×

INDI Library v1.9.6 Released (21 May 2022)

Bi-monthly INDI Library released with new drivers and bug fixes.

Pulsar2 driver updated, need advice on deployment

  • Posts: 98
  • Thank you received: 22
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
1 year 8 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: 1 year 8 months ago by Greg.
1 year 8 months ago #61882
Attachments:

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

  • Posts: 98
  • Thank you received: 22
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: 1 year 8 months ago by Camiel Severijns.
1 year 8 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
1 year 8 months ago #61956
Attachments:

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

  • Posts: 98
  • Thank you received: 22
Hi Gregory,

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

Regards,
Camiel
The following user(s) said Thank You: Greg
1 year 8 months ago #61960

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

Time to create page: 2.498 seconds