Pulsar2 driver updated, need advice on deployment

Camiel Severijns
Offline
Premium Member

Posts: 105
Thank you received: 23

Replied by Camiel Severijns on topic Pulsar2 driver updated, need advice on deployment

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 1 month ago #61853

Attachments:

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

Greg
Topic Author
Offline
Senior Member

Posts: 47
Thank you received: 2

Replied by Greg on topic Pulsar2 driver updated, need advice on deployment

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 1 month ago by Greg.

3 years 1 month ago #61882

Attachments:

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

Camiel Severijns
Offline
Premium Member

Posts: 105
Thank you received: 23

Replied by Camiel Severijns on topic Pulsar2 driver updated, need advice on deployment

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 1 month ago by Camiel Severijns.

3 years 1 month ago #61896

Attachments:

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

Greg
Topic Author
Offline
Senior Member

Posts: 47
Thank you received: 2

Replied by Greg on topic Pulsar2 driver updated, need advice on deployment

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 1 month ago #61956

Attachments:

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

Camiel Severijns
Offline
Premium Member

Posts: 105
Thank you received: 23

Replied by Camiel Severijns on topic Pulsar2 driver updated, need advice on deployment

Hi Gregory,

I'll test the new version and let you know the result.

Regards,
Camiel

The following user(s) said Thank You: Greg

3 years 1 month ago #61960

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

INDI Library v2.0.5 is Released (Yesterday)