MAVLink protocol improvements

[BluemarkInnovations]

Currently I'm working on ArduPilot integration ArduPilot/ardupilot#21075 and working on a MAVLink OpenDroneID device. Here, I want to share my experience with the current MAVLink OpenDroneID messages, what is missing. And what I propose to solve it.

I would appreciate feedback. Any thoughts? @friissoren @gabrielcox

Current implementation

In MAVLink messages, a CRC is added to ensure message integrity (https://en.wikipedia.org/wiki/MAVLink#CRC_field). This means that if the definition of an OpenDroneID MAVLink message changes, the CRC checksum will change as well. For instance the OPEN_DRONE_ID_SYSTEM message defintion was recently changed. It means that devices with an older MAVLink library won't be able to receive messages from the current MAVlink library and vice versa.

Proposed solution: keep the current MAVLink message definitions fixed. in case of an updated message definition, define a new message instead.

Dual or multiple IDs

The current OPEN_DRONE_ID_BASIC_ID messages does not allow to differentiate between multiple Basic ID definitions. Current implementation is okay, just want to raise the subject to raise awareness

Proposed solution: keep the current OPEN_DRONE_ID_BASIC_ID as is. In case of two IDs, multiple OPEN_DRONE_ID_BASIC_ID messages can be send, for each ID one message. The transponder can decode those messages accordingly and decide how to handle it.

Controlling the radio transmission

The current MAVLink messages don't allow to control the radio transmission. It can be useful to disable radio transmission when the UAV is on the ground. Or to change the radio transmission protocol (WiFi Beacon/NAN, BLE legacy/long range) depending on the region where the device is flying.

Proposed solution: Attached is a proposal for such a MAVLink CMD message.

ACK packets

As Remote ID evolves, it is likely that there will be devices with different MAVLink library versions with not the same OpenDrone ID message definitions. (See the OPEN_DRONE_ID_SYSTEM message example earlier.) Having an ACK message helps to identify:

• problems in the MAVLink interface. If ACK messages are not received, the communication line is faulty. Or wrong UART parameters are used. Based on ACK packets, an auto pilot systems can signal those issues.
• compatibility issues. Say there are two Basic ID message definitions. An auto pilot system can send both the legacy and new message definition. Based on the ACK response, it knows which message types are supported by the transponder. (And adjust the MAVLink messages accordingly). Also having ACK messages ,the auto pilot system knows in general that the OpenDrone ID messages have been accepted by the transponder.

Proposed solution: Attached is a proposal for such a MAVLink ACK message.

I zipped the attachment, because XML is not accepted
mavlink_opendroneid.xml.zip

(edited to fix some typo's and minor additions for clarification)
.

[friissoren]

Sorry for the slow reply. Vacation got a bit in the way :-)

---

CRC:
I don't think that is how the CRC works. It is purely a transmission protection mechanism and doesn't have anything directly to do with the layout of each message. The CRC is calculated over the entire header + payload of the message and will depend on the exact bits and bytes of the payload. I.e. just by changing values in the payload, the CRC will change.

If we get the WIP markers removed from the MAVLink message, I do agree with you that we should not in the future "break" any backwards compatibility. I.e. if additional fields are added, they must be added at the end of the message and the length/interpretation of the existing fields must not be changed.

Since both the ASTM and ASD-STAN standards are now basically done, the chance that there will be future changes is quite small.

---

Multiple Basic ID:
Transmitting two messages is how I intended it to be done. They can be distinguished by having different MAV_ODID_ID_TYPE values.

---

Radio Control:
Good point. That is completely missing.

I am not sure if the id_or_mac field is needed? What would be the usage?

The updated frequency could be a byte or float field instead of an enum. It would be more flexible.

For consistency, all of the enum values should start with "MAV_ODID".

---

Acknowledge:
I am not sure if the id_or_mac field is needed? What would be the usage?

Where does seq_number come from?

I didn't really understand the "other commands could be accepted" part of OPEN_DRONE_ID_ACK_ERR_NOT_SUPPORTED?

Adding this type of acknowledge messages is probably something that would need to be discussed with the MAVLink maintainers as well. I am not really sure how they in general expects things to be done. That discussion can be started by uploading a PR to add the acknowledge message and there ask for their opinion. Hamish Willee gave me great feedback when I was developing the documentation.

[BluemarkInnovations]

CRC: I don't think that is how the CRC works...

No, your understanding is incorrect. See this page: https://mavlink.io/en/guide/serialization.html
Part of the message is the 2-byte checksum. It say on the checksum "Includes CRC_EXTRA byte." More info here on CRC_EXTRA: https://mavlink.io/en/guide/serialization.html#crc_extra

When the MAVLink code generator runs, it takes a checksum of the XML structure for each message and creates an array define MAVLINK_MESSAGE_CRCS. This is used to initialise the mavlink_message_crcs[] array in the C/C++ implementation, and is similarly used in the Python (or any other, such as the C# and JavaScript) implementation.

When the sender calculates the checksum for a message it adds the CRC_EXTRA byte onto the end of the data that the checksum is calculated over. The recipient calculates a checksum for the received message and adds its own CRC_EXTRA for the particular message id. If the CRC_EXTRA for the sender and receiver are different the checksums will not match.

This approach ensures that only messages where the sender and recipient are using the same message structure will be decoded (or at least it makes a mistake much more unlikely, as for any checksum application).

Basically a pre-computed byte is added in the CRC calculation to ensure sender and receiver agree on the data structure.
It means that if you change anything (data types) or add/remove fields, this CRC_EXTRA byte will change.

Radio Control: Good point. That is completely missing.

I am not sure if the id_or_mac field is needed? What would be the usage?

Can be removed, it is just copying the field from existing message defintions.

The updated frequency could be a byte or float field instead of an enum. It would be more flexible.
Let's make it a uint16_t value indicating the ms duration/period. Values between 100 ms and 1000ms

For consistency, all of the enum values should start with "MAV_ODID".

Okay will do.

Acknowledge: I am not sure if the id_or_mac field is needed? What would be the usage?

Can be removed.

Where does seq_number come from?

See https://mavlink.io/en/guide/serialization.html, there is a field seq, a byte indicating the message counter. Used to detect packet loss. Components increment value for each message sent. Typically in an ACK message, you also include the received package counter.

I didn't really understand the "other commands could be accepted" part of OPEN_DRONE_ID_ACK_ERR_NOT_SUPPORTED?

Well say you have transponder 1 who doesn't support message packs messages, whereas transponder 2 only support message pack messages. So this error type may be used by the transponder to signal that it can't process these messages.

Adding this type of acknowledge messages is probably something that would need to be discussed with the MAVLink maintainers as well. I am not really sure how they in general expects things to be done. That discussion can be started by uploading a PR to add the acknowledge message and there ask for their opinion. Hamish Willee gave me great feedback when I was developing the documentation.

Well there is also the hearbeat message, MAVLink devices need to send those as well every second. So I agree that if the messages are fixed and likely won't change in future, and the heartbeat message may be used also to see if the device/link works, this ACK may not be useful. But let's first agree on the new messages, then wait for their opinion.

Attached new definition.
mavlink_opendroneid.xml.zip

[friissoren]

No, your understanding is incorrect.

Thanks for educating me. I did not know about that :-) This certainly means that the messages should not change any more. That is not likely to happen anyway, once one full system implementation has been done and verified.

there is a field seq

Do you know if that is auto-incremented by some lower level MAVLink sender code or does the sender of the message need to do that manually? If manual, we need to update the documentation to mandate that this is incremented.

Still about the enum names. Please change also the name of the "main enum struct" so that the main struct name is the prefix of each specific enum value. See the existing definitions. This is a general MAVLink convention/requirement.

transmission_period_ms should have units="ms" added to its definition. And I would extend the allowed range up to at least 3 seconds.

[BluemarkInnovations]

there is a field seq

Do you know if that is auto-incremented by some lower level MAVLink sender code or does the sender of the message need to do that manually? If manual, we need to update the documentation to mandate that this is incremented.

This is done automatically by the lower level MAVLink sender code. No need to update documentation.

Still about the enum names. Please change also the name of the "main enum struct" so that the main struct name is the prefix of each specific enum value. See the existing definitions. This is a general MAVLink convention/requirement.

done.

transmission_period_ms should have units="ms" added to its definition. And I would extend the allowed range up to at least 3 seconds.

done, but the location message period always needs to be 1 second maximum as defined here, footnote 4. As most transmission protocols pack all information into one message (except BT4), it means that in real life you are never above 1 second period. (And for BT4 you also need to send at least every 1 second the location message.) So basically this field means, transmit the location messages at this interval (and make sure other information is sent as well within the allowed period range set by the standard.)

A potential solution could be to add a period for both dynamic and static messages. However, as stated above, most transmission protocols use one message to transmit both dynamic and static information.

Attached updated definition.
mavlink_opendroneid.xml.zip

[friissoren]

Great. Looks pretty good to me.

Will you do the changes to the MAVLink common.xml and create a PR?

Depending on how the discussion with the MAVLink maintainers go, it will probably also be necessary at some point to update the documentation and describe how the ACK message is supposed to be used.

[BluemarkInnovations]

Will you do the changes to the MAVLink common.xml and create a PR?

Yes, done. mavlink/mavlink#1865

[friissoren]

Closing this issue, since the discussion continued in the MAVLink PR.