bipropellant / bipropellant-protocol

defines and implements a low level protocol shared between a few different hoverboard repositories
MIT License
20 stars 17 forks source link

fix subscriptions messages in protocol.c #36

Closed LucFabresse closed 4 years ago

LucFabresse commented 4 years ago

subscription mechanism was not working for me and this change fixes it. So I share it ;-)

Basically, in protocol.c line 507, the subscription code is erased. So, I made the registration of the subscription at PRE_WRITE time instead of POST_WRITE time.

p-h-a-i-l commented 4 years ago

Can you try with the updated protocol if the subscriptions problem is still there? If so, we can adapt this PR to match the latest branch.

LucFabresse commented 4 years ago

I wanted to test it but the latest version of the firmware does not compile with this new version of the protocol. This is because the firmware (cf. src/main.c:911) accesses a var named enable_immediate that has been moved into a struct by this new PR. I did not investigate more to do the required changes.

p-h-a-i-l commented 4 years ago

You are right, wanted to merge all repos at once but I underestimated the effort needed :) Master branch is merged now, please note that the robustness of the serial protocol is way better now, but it is not compatible anymore. I'll update the Arduino Library soon.

LucFabresse commented 4 years ago

I messed up with the pull request branch on my own fork and it closes the pull request...

We always underestimate the effort, otherwise we would not start ;-) I tried this new version of the protocol and subscriptions does not work for me. I also compiled the firmware with this new protocol and tried the ASCII protocol and it is weird. For example the Immediate Speed "Is" mode continuously increase speed even if I release "w".