Simpit-team / KerbalSimpitRevamped-Arduino

BSD 2-Clause "Simplified" License
18 stars 19 forks source link

[Feature] Revamp the throttle msg #1

Closed PBechon closed 3 years ago

PBechon commented 3 years ago

I feel that sending a throttle command is unnecessarily more complicated than other command (like rotation and translation) since there is no struct defined. One has to figure out for instance than sending for sending one value the correct message size (in byte) is 2 and that the number should be send as uint16.

I would suggest to create a struct for throttle only, containing only one value, and all the needed helper to be able to call

throttleMessage throttle;
throttle.throttle = 100;
mySimpit.send(THROTTLE_MESSAGE, trans);
LRTNZ commented 3 years ago

I see your point - will take a look into this.

PBechon commented 3 years ago

In fact the modification was quite small. I did a pull request to put it in, and I added an example with it (that I used to test it).

LRTNZ commented 3 years ago

Thanks! Will test it shortly, along with the target fix on the other repo.

LRTNZ commented 3 years ago

Just waiting on a couple of tweaks and answers to my questions/suggestions 😄

PBechon commented 3 years ago

What tweaks and suggestion ? I did not see those I think, I might have missed them.

LRTNZ commented 3 years ago

What tweaks and suggestion ? I did not see those I think, I might have missed them.

Sorry, I forgot to hit submit on the review I did 😅

LRTNZ commented 3 years ago

This has been solved!