-
Notifications
You must be signed in to change notification settings - Fork 125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HL->LL config packet with "DFP is 5V" implementation #79
HL->LL config packet with "DFP is 5V" implementation #79
Conversation
That looks good to me. Yes, forcing a low level update will probably lead to many questions on discord. Why does the ROS need the volume setting? To show to the user as feedback via MQTT or something probably? What we can do is implement a protocol version in the heartbeat and then send back the correct version of the package, alternatively we can create a new message type as you suggested |
Yes :-/ ... and a more complicated synchronized HL-ROS & LL-FW update procedure
Yes sure we could also publish the volume in mower/status but my main intention was that I assumed that we'll need it for future WebGUI possibilties. I.e. increase/decrease volume also in WebGUI and for this we would need the current volume.
I'm halting with the new message type because I don't wanna create a new message type for every stupid idea :-/ So with any change in the protocol we do have the problem with packet changes if not both sides get updated ^^. I'm also unsure what get changed/installed more often by the user: The LL-FW or OM-ROS/Image? Think we should drop my new ll_status.volume member for now because this PR (without .volume) wouldn't affect an old LL-FW, except of a handful of "unknown packet" LL-errors due to the unknown ll_high_level_config packet, but once emergency released (and old LL-FW will release without waiting for config packet) no config packet is send anymore. |
What we could do in theory is introducing a new packet for ROS to inform the LowLevel which comms version it supports, if no such package arrives, LL always sends the oldest version. As soon as the packet arrives, LL sends the packets with that required version. Wouldn't be too crazy of a change and keep things compatible.
I'm also OK with that. |
Had a similar thought, but yours is much simpler, smarter and easier!
|
yes that looks good to me! you can add to this PR |
Main changes are:
Just asking myself if the typo suffix '_RSP' is that clever, because a response is normally only sent after a request, which is not always the case. Probably better?: |
Thanks for the update! Looks good to me |
RE naming: I think it's fine as is but I get that "response" is not 100% accurate. |
Related to ClemensElflein/OpenMower#80
This is how I would implement the ROS part about what we discusses here:
ClemensElflein/OpenMower#80 (comment)
Where I'm undetermined is:
In this proposal I added volume feedback to ll_status struct which would make a LL FW update mandatory.
I added it there, to not create another new packet and because it's the right place for it as it's a "status" (even if unimportant)
As an alternative we could add another new packet like ll_highlevel_misc :-))