MAVLink 2 Packet Format includes non existent fields
WickedShell opened this issue · 5 comments
https://mavlink.io/en/protocol/overview.html describes the MAVLink 2 Packet Format as follows:
uint8_t magic; ///< protocol magic marker
uint8_t len; ///< Length of payload
uint8_t incompat_flags; ///< flags that must be understood
uint8_t compat_flags; ///< flags that can be ignored if not understood
uint8_t seq; ///< Sequence of packet
uint8_t sysid; ///< ID of message sender system/aircraft
uint8_t compid; ///< ID of the message sender component
uint8_t msgid 0:7; ///< first 8 bits of the ID of the message
uint8_t msgid 8:15; ///< middle 8 bits of the ID of the message
uint8_t msgid 16:23; ///< last 8 bits of the ID of the message
uint8_t target_sysid; ///< Optional field for point-to-point messages, used for payload else
uint8_t target_compid; ///< Optional field for point-to-point messages, used for payload else
uint8_t payload[max 253]; ///< A maximum of 253 payload bytes
uint16_t checksum; ///< X.25 CRC
uint8_t signature[13]; ///< Signature which allows ensuring that the link is tamper-proof
target_sysid
and target_compid
are not real protocol details, no parser handles the concept, it was never in any of the MAVLink 2.0 standard proposals, there is no way in a protocol level to tell if they are being used for this or not, and all existent parsers treat the payload length as 255 bytes. More importantly there are exsisting messages that need a 255 byte payload, you can't steal from this without breaking the ability to encode some messages. This needs to be removed from the documentation.
Thanks very much. I also questioned this recently when updating the serialisation doc. There is an action for me to update that doc and the one on serialization to clarify.
The summary is that this is not wrong, but is confusing. The fields exist (and have always existed) for point to point messages in the payload for MAVLink messages. They are always in that first position for those sorts of commands.
Apparently MAVLink 2 generated libraries provides additional support/accessors to make that explicit.
I have not yet worked out how to communicate this (and I need to do a little more research to verify what happens in the generated libraries). Obviously I will need review/discussion on this and will loop you in.
My leaning though is to include those fields in both MAVLink 1 and MAVLink 2 packets but explain the difference in what the generators do.
The summary is that this is not wrong, but is confusing. The fields exist (and have always existed) for point to point messages in the payload for MAVLink messages. They are always in that first position for those sorts of commands.
This isn't actually true. Messages are packed according to the field size, which typically means these are sent near the end of a selected message. (There also aren't any fields called target_sysid
target_compid
in any of the exsisting messages, it's all target_system
or target_component
. The net effect is that the fields typically are never located immediately after the other headers, so this is still wrong.
They are always in that first position for those sorts of commands.
No, they aren't, check PING message for example. Also, some messages only send target_system.
Apparently MAVLink 2 generated libraries provides additional support/accessors to make that explicit.
Yes, the parser checks out if fields with these names exist and gets the info where in the wire frame they are located - it's not a fixed position.
Thanks guys, that is helpful. It does sound wrong. I will read up on this, update in the coming week and ping you on the PR.
OK, so just a quick update - I've looked at the code now and I agree that the docs are "clearly incorrect".
The target system and component id might be before the rest of the payload because MAVLink 2 messages with id >255 are ordered by declaration position in XML. However, as you say, for messages with id <255 the order will be different, and the fields might not be defined at all. So clearly wrong.
The generator does have some useful tools to get these fields from the message without having to read the payload (the offset is in the MAVLINK_MESSAGE_CRCS) but I will make sure that gets documented in the different libraries.
I'll try tidy this up.