Menu

Topic-icon 2 more API suggestions and possibly another bug

More
10 months 2 weeks ago #29534495 by vector
API's incorporated into other applications can often be disruptive if/when they internally write to standard output. I suggest wrapping all your cout statements in a compiler directive with a default value that evaluates to false. Like #define ENABLE_STDOUT 0 or something like that.

Also, I would suggest moving the common code that retrieves the firmware version into it's own method outside of the platform dependent connect methods. That would help reduce maintenance and has the added benefit that it can be called by an application as well as internally within the API.

When the firmware version substring is retrieved, it outputs "v1.8" but I think what is wanted is 1.8d so the revision is captured as well so instead of this:
cout << response.substr(8, 4) << "." << endl;

should it be?
cout << response.substr(9, 4) << "." << endl;

or maybe this?
cout << response.substr(8, 5) << "." << endl;

so the output is v1.8d if including the 'v' was intentional.

These are all changes I am making and will have to be careful about merging any future API updates. I would be very happy to contribute. I could post patch sets but it would be a whole lot easier if it was in an online repo somewhere.

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

  • Gabriel_Isko
  • Gabriel_Isko's Avatar
10 months 2 weeks ago #29534496 by Gabriel_Isko
Replied by Gabriel_Isko on topic 2 more API suggestions and possibly another bug
Alright,

I ended up mirroring the API on my github . So, fork that, submit pull requests if you think it is appropriate, and I'll try to take a look at it personally.

We are trying to consider some of our software development routines here at RoboteQ, but for now this will have to do. I will advocate for appropriate changes to be upstreamed, but it's ultimately not up to me.

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

Moderators: tonysantoni
Time to create page: 0.099 seconds
Go to top