×

INDI Library v2.0.6 is Released (02 Feb 2024)

Bi-monthly release with minor bug fixes and improvements

Pulsar2 driver updated, need advice on deployment

  • Posts: 105
  • Thank you received: 23
Hi Gregory,

The units for the non-guide speeds are reported correctly for my old Pulsar.
I hope to do some longer test runs under the stars soon but I think there are no major issues with the new driver.

Concerning the -1 bug, I can only think of two sources for this problem:
1. the conversion of the declination in string format as send with the slew command
to an internal (binary) format goes wrong, or
2. the conversion from this internal format back to the string that is displayed on
the hand controller and/or returned by the get position command contains an error.
In the first case the pointing is off, in the second case the pointing is correct.
In both cases a fix in the driver is possible although it will be somewhat simpler in the second case than in the first.
Since the bug is an old one, most Pulsar controllers will have it, so correcting it in the driver has the advantage that
every Pulsar user can profit.

Regards,
Camiel
The following user(s) said Thank You: Greg
3 years 5 months ago #61602

Please Log in or Create an account to join the conversation.

  • Posts: 47
  • Thank you received: 2
Hello Camiel,

Hooray, the units and display are correct for your controller for
the non-guide speeds!

Thanks so much for verifying that -- hopefully "set" commands
above 999 will also work. (Those are undocumented at this
point, based on the documents I have.)

On the -1 bug: I haven't tested this, but let's say (for the sake of
the discussion :-) ) that the OTA does indeed end up pointing at
the wrong place, which is a likely outcome. I did a little experiment,
and for the controller commands (or hand controller button presses)
that *move* the position, they do move the position quite happily
into that "forbidden" band. I just can't use any command that
sends coordinates, or an Altitude position (I tried that also).

Please check my knowledge on this, but I *think* that all I have
available to me to correct the position would be the LX200 move
commands, which move in one of the cardinal directions at a
given pace. That pace is the currently-selected "slew" rate
(center, find, slew, goto).

The amount of time would be dependent on the system clock,
and therefore dependent upon the responsiveness of the
system to issuing tty commands at a given interval. This
is because, as far as I know, there is no command to move
for a given period of time (except for pulse guiding, which is
not universally available, and which operates only on the
current guide rate, which is much too slow for the correction).

So, if the target Dec position were in the "forbidden zone",
I would have to detect the end of that slew, then:

1) Pick a rate (e.g., the "find" rate), change it to a known value
(adjusted for distance) and calculate the amount of time necessary
to go twice the distance, or a bit less, to the celestial equator at
that rate;
2) Start that southward motion;
3) At the end of that given time (say, an alert event), issue the
command to stop the motion;
4) Check the current Dec position, and if it is off, go through a
cycle at a slower rate (say, a guide rate) to correct the position.
For this, I *could* use the pulse guide facility if available, and if
not, just cycle for a reasonable time until reasonably close. :-)
5) Return the find rate to its former value.

Even after all this, any command that is issued that specifies
a Dec or altitude position will undo all that good work!

Please let me know if I have completely missed some much
easier way to do this. :blush:

Regards,

Gregory
3 years 5 months ago #61667

Please Log in or Create an account to join the conversation.

  • Posts: 105
  • Thank you received: 23
Hi Gregory,

So your test result indicates that we have the first of the two cases to workaround.
I'll do the same test with my controller.

There are three different types of movements that we can distinguish:
1. a motion from outside to inside the problematic declination range,
2. a motion within this range,
3. a motion out of this range.
The last one causes no problems of course.

To deal with the first one, we can issue a goto command just North or South of the range
depending on which is closer to the desired position and then apply the procedure that you describe.

For the second type of movement, we can use the same procedure for large movements. For small
movements, such as issued when kstars adjusts the telescope pointing based on astrometric data,
it may be better to use just move commands without goto.

Regards,
Camiel
The following user(s) said Thank You: Greg
3 years 5 months ago #61673

Please Log in or Create an account to join the conversation.

  • Posts: 47
  • Thank you received: 2
Hello Camiel,

Well, it's time for the most sheepish look I can muster. :oops:

It turns out that the -1 bug was not in the controller at all, but rather
in the driver code. I was doing a lazy thing, and taking the output
that was being logged to the INDI driver page as *the* output.
It turns out that the command actually being sent to the controller
was formatted incorrectly, which I discovered as I rolled up my
sleeves to do a lot more work...it was actually just a couple of
lines to change.

Then the great news is: the problem is fixed! (I sincerely hope).
It only remains for an expert such as yourself to try the attached
code, and let me know the results.

Now I will have to send my apologies to Andras Dan at GTD!

Very very grateful for your consistent input, and most thorough
testing!!

Regards,

Gregory
3 years 5 months ago #61727
Attachments:

Please Log in or Create an account to join the conversation.

  • Posts: 105
  • Thank you received: 23
Hi Gregory,

Good to hear that you found the cause of the bug. The driver code
is quite complex which doesn't help bug solving.

It looks like this evening there will be some gaps between the clouds
so that I'll hopefully be able to test the driver in an operational setting.

Regards,
Camiel
The following user(s) said Thank You: Greg
3 years 5 months ago #61737

Please Log in or Create an account to join the conversation.

  • Posts: 105
  • Thank you received: 23
Hi Gregory,

There is just one small problem with the driver that I encountered: after the :Sr and :Sd commands,
and before the RA/Decl. are given, there should be a space. My old Pulsar controller needs it to properly
process the command. Furthermore, no more '-1 bug' for me either.

An alternative for your code which does not require hard-coding of the position of the + or - sign and allows
the format string to be a constant is:

bool setObjectDEC(const int fd, const double dec)
{
int d, m, s;
getSexComponents(dec, &d, &m, &s);
char full_cmd[32];
snprintf(full_cmd, sizeof(full_cmd), "#:Sd %c%02d:%02d:%02d#",( dec < 0.0 ? '-' : '+' ), abs(d), m, s);
char response;
return (confirmed(fd, full_cmd, response) && response == '1');
}

When I think about this, it seems that the bug is actually in the function getSexComponents().
It does not (or no longer?) return a signed zero value in the argument 'd'.
A quick 'grep' for it shows this function is used in many drivers. Maybe something to report to Yasem?

Regards,
Camiel
The following user(s) said Thank You: Greg
Last edit: 3 years 5 months ago by Camiel Severijns.
3 years 5 months ago #61749

Please Log in or Create an account to join the conversation.

  • Posts: 47
  • Thank you received: 2
Hello Camiel,

Yes, good call! Less work to do at run-time, and more compact.
Those changes are copy/pasted into the attached version, along
with the spaces, which I removed only because I was looking at
the Meade documentation. The spaces seem to work just as well
for my more recent firmware version.

I will attempt to pass on the information related to getSexComponents()
you describe to Jasem, but (based on ancient memory) I also wonder
if getSexComponents() is as it is because support for a signed zero
(also sometimes called "dirty zero") has varied between compilers and
compiler versions. Maybe that's just not a factor any more, and things
have become standardized -- I don't really know. At least what we
have in the driver seems relatively "bullet proof" (which is why I used
abs(d)).

There were two or three things I had hoped to add (e.g., UMode
Speed Limit, Pole Crossing direction, etc.), but Andras at GTD said
that, since the project is an old one, more information may not be
forthcoming from the developer. He did volunteer to post the
Delphi-based source to the Pulsar Commander, though, which
may have more information in it.

So, do you think we have a "release candidate" here, or would it be
better to wait for the Delphi source, in order to add the two or three
items, if they happen to be there? That may take a while, and those
features could be added later in another release, since what we have
at this point in version 1.2 seems like a valuable upgrade for other
Pulsar2 users.

Best regards, and thanks again,

Gregory
Last edit: 3 years 5 months ago by Greg.
3 years 5 months ago #61818
Attachments:

Please Log in or Create an account to join the conversation.

  • Posts: 105
  • Thank you received: 23
Hi Gregory,

I'll try the new code.

It would be useful if the solution for the sign problem in the driver could be moved to the getSexComponents() function.
This requires changes in many drivers but a this would be required anyways to fix this problem.

I think the driver is good enough to be released .

Next to adding a few more commands, a bit of code cleanup would be good as well. E.g., it would be better to get all
code involving the mutex localized by introducing a function sendOnly(). The low level send and receive functions could
then be hidden in a private scope. At the time, the serial port code of the standard LX200 driver didn't work well. It may be
worth to check whether this is still the case. If not, we could use the generic routines which saves some lines of code.

Regards,
Camiel
The following user(s) said Thank You: Greg
3 years 5 months ago #61839

Please Log in or Create an account to join the conversation.

  • Posts: 105
  • Thank you received: 23
Hi Gregory,

The code works except for one small issue that I noticed while scanning the log files in more detail:
the old Pulsar apparently doesn't support the setting of the user rate. The driver tries to read
the response (which never comes) after which a resynchronisation is done. This doesn't
prevent the driver from working in other respects.

I have attached a version of the driver code in which the mutex is controlled by std::unique_lock.
This is safer since it will always release the mutex automatically when the scope is closed:
on a normal exit, an early exit, and even when a C++ exception is thrown.

I also renamed functions that I think should become private with
a leading and trailing underscore. The resynchronize function needs some more work.
And the mutex should remain lock when the reading is completed fully instead of unlocking/relocking
it between the call to sendReceive() and subsequent reading of the planetary data.

Regards,
Camiel
The following user(s) said Thank You: Greg
3 years 5 months ago #61853
Attachments:

Please Log in or Create an account to join the conversation.

  • Posts: 47
  • Thank you received: 2
Hello Camiel,

Thanks for the contributions!

I have looked at your modifications, and attempted to incorporate
them (or just copy wholesale). Especially true for std::unique_lock,
which (as you know) is far better/safer than manipulating the mutex
directly. So, take a look at the attached code, which also includes
a fix for a bug that allowed a "Sync()" while the scope was slewing,
apparently a *very* bad thing for the controller!. I *think* I also
caught what you were referring to about calls that do multiple
receives, and their locking strategy. There were also a small
handful of additional methods that I have made "private".

I did the "private" modification through an anonymous namespace,
but if you think a private class would be better, I can also make
that change. Also, feel free to make other changes and additions
as you see fit. I will hold off further changes until I hear from
you!

The UserRate setting stuff does not work with the firmware version
I am using either. I have tried to put comments in the code, until (if
ever) we can get an answer from the developer, or from another
means. Othewise, even for me, it's just too dangerous to use, since
the controller I have supports a rather "invisible" rate that is not rate
1, 2, or 3! It could cause minor havoc :) So I have tried to opt-out
the "active" portions of the code dealing with that parameter.

And, please, if you have further thoughts about how to improve the
resynchronization routine, that would be great -- update me with the
code or with the idea(s).

Outstanding collaboration, thanks again,

Regards,

Gregory
Last edit: 3 years 5 months ago by Greg.
3 years 5 months ago #61882
Attachments:

Please Log in or Create an account to join the conversation.

  • Posts: 105
  • Thank you received: 23
Hi Gregory,

The new version works for me.
If noticed that a bug that I fixed earlier, didn't make it in the version that was put into the repository.
From line 226 the code should read:
<code>
{
// Skip response strings consisting of a single termination character
if (nbytes_read_total == 0 && response[0] == Termination)
response[0] = LX200Pulsar2::Null;
else {
nbytes_read_total += nbytes_read;
done = true;
}
}
}
</code>

I have also been thinking about a way to ensure that the mutex is always locked and unlocked
properly. In the attached source file (a modified version of your code from this weekend), I have
introduced the Transaction class for this purpose.

We can make the sending and receiving more similar to C++ iostreams. An example of what this
could look like is:
class Transaction {
private:
  static std::mutex dev_mutex;
 
  const int fd;
  const std::lock_guard<std::mutex> lock;
  bool is_valid;
 
  ...
 
public:
  Transaction(const int _fd)
    : fd(_fd), lock(dev_mutex), is_valid(true)
  {}
 
  ~Transaction(void) {}
 
  operator bool(void) const { return is_valid; }
 
  Transaction& operator<<(const char* _command) { is_valid = send(_command); return *this; }
  Transaction& operator>>(bool&       _okay   ) { is_valid = receive_a_single_one_or_zero(_okay);   return *this; }
  Transaction& operator>>(char*       _string ) { is_valid = read_a_response_upto_marker (_string); return *this; }
  Transaction& operator>>(int&         _value ) { char s[32]; is_valid = ( read_a_response_upto_marker (s) && convert_to_int(s,_value) ); return *this; }
 
  ...
 
};
This class can be used as follows:
bool getVersion(const int fd, char response[])
{
    Transaction t(fd);
    bool success = ( t << ":YV#" );
    if (success)  success = ( t >> response );
    return success;
}
 
const char* setIntCommand(const int v) { // Crude implementation!!
  static char buffer[32];
  sprintf(buffer,"%d",v);
  return buffer;
}
 
bool setInt(fd,const int value) {
    Transaction t(fd);
    return ( t << setIntCommand(value) );
}
Let me know what you think.

Regards,
Camiel
The following user(s) said Thank You: Greg
Last edit: 3 years 5 months ago by Camiel Severijns.
3 years 5 months ago #61896
Attachments:

Please Log in or Create an account to join the conversation.

  • Posts: 47
  • Thank you received: 2
Hello Camiel,

Yes, a Transaction class is certainly more elegant. If you'd like
to add it to my latest version (attached here), that would be a
very sophisticated addition.

There is no difference between this and my last posting, except
that your "bug fix" for the single-termination-character return is
included. I suggest starting with this only because it further
confines the TX calls, such that even the Transaction class
would not have to propagate much throughout the code.

This will have to be my last tested version for a while, since
I am sending my mount base back for repair, and the mount
maker requires that I send the Pulsar controller along with
it :-(, no doubt for testing.

Your help has certainly made a much better product!

Many thanks again,

Gregory
3 years 5 months ago #61956
Attachments:

Please Log in or Create an account to join the conversation.

Time to create page: 1.997 seconds