HL->LL config packet with "DFP is 5V" implementation

[Apehaenger]

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 :-))

[ClemensElflein]

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

[Apehaenger]

That looks good to me. Yes, forcing a low level update will probably lead to many questions on discord.

Yes :-/ ... and a more complicated synchronized HL-ROS & LL-FW update procedure

Why does the ROS need the volume setting? To show to the user as feedback via MQTT or something probably?

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.

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

I'm halting with the new message type because I don't wanna create a new message type for every stupid idea :-/
Packet or protocol version in heartbeat is a good idea, but wouldn't this also result initially in a bad crc till all updated their LL-FW?

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.
Afterwards we should find/implement some kind of protocol selection handling before changing any packets. I could do that in a separate PR... if you help me to find/decide for the most easy and clever solution ;-)

[ClemensElflein]

That looks good to me. Yes, forcing a low level update will probably lead to many questions on discord.

Yes :-/ ... and a more complicated synchronized HL-ROS & LL-FW update procedure
Yes, with v2 hardware this shall be automatic then. But for now, people will need to keep it in sync manually.

Why does the ROS need the volume setting? To show to the user as feedback via MQTT or something probably?

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.

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

I'm halting with the new message type because I don't wanna create a new message type for every stupid idea :-/ Packet or protocol version in heartbeat is a good idea, but wouldn't this also result initially in a bad crc till all updated their LL-FW?

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.

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. Afterwards we should find/implement some kind of protocol selection handling before changing any packets. I could do that in a separate PR... if you help me to find/decide for the most easy and clever solution ;-)

I'm also OK with that.

[Apehaenger]

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.

Had a similar thought, but yours is much simpler, smarter and easier!
How's putting it all together, because it's one initial configuration announcement (and add some spare bytes for further brain farts because it's inexpensive). I.e.:

#pragma pack(push, 1)
struct ll_high_level_config
{
    uint8_t type;           // Type of this message. Has to be PACKET_ID_LL_HIGH_LEVEL_CONFIG
    uint8_t version = 0;    // Increasing comms version number for packet compatibility (n > 0)
    uint8_t config_bitmask; // See LL_HIGH_LEVEL_CONFIG_BIT_*
    uint8_t reserved1 = 0;  // Reserved for future use
    uint16_t reserved2 = 0; // Reserved for future use
    uint16_t crc;
} __attribute__((packed));
#pragma pack(pop)

[ClemensElflein]

yes that looks good to me! you can add to this PR

[Apehaenger]

Main changes are:

1. Added comms_version to config package
2. Moved volume and language to config packet as they fit logically better to there (in my opinion)
3. Double used config package with two packet.types. Either as
   • PACKET_ID_LL_HIGH_LEVEL_CONFIG_RSP will only response with a config packet, whereas
   • PACKET_ID_LL_HIGH_LEVEL_CONFIG_REQ response with a config packet but also requests the other side to _RSP with a config packet.

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?:
PACKET_ID_LL_HIGH_LEVEL_CONFIG and
PACKET_ID_LL_HIGH_LEVEL_CONFIG_REQ

[ClemensElflein]

Thanks for the update! Looks good to me

[ClemensElflein]

RE naming: I think it's fine as is but I get that "response" is not 100% accurate.