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.
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
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).
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:
// Skip response strings consisting of a single termination character
if (nbytes_read_total == 0 && response == Termination)
response = LX200Pulsar2::Null;
nbytes_read_total += nbytes_read;
done = true;
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:
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!