Skip to content
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

Project is not truncating trailing zeros for payload #188

Open
patrickelectric opened this issue Aug 7, 2023 · 5 comments
Open

Project is not truncating trailing zeros for payload #188

patrickelectric opened this issue Aug 7, 2023 · 5 comments

Comments

@patrickelectric
Copy link
Member

This is a must for MAVLink2
https://mavlink.io/en/guide/serialization.html#payload_truncation

reference: bluerobotics/BlueOS#1740
reference: mavlink/mavlink2rest#80

@hamishwillee
Copy link

This is a must for MAVLink2

Yes. FWIW this came up in a real implementation, and we decided on "must". But you'll note that receivers are supposed to "cope" with messages that don't truncate if they are otherwise valid.

@patrickelectric
Copy link
Member Author

Thanks for pointing that out @hamishwillee

@clydemcqueen
Copy link

This is a bit complicated because QGC receives the messages just fine, but writes them to tlog files incorrectly: truncated, but with the CRC computed from the non-truncated payload. This is caused by a bug in mavlink_helpers.h: ArduPilot/pymavlink#237

The result is quite a few messages sent by rust-mavlink and logged by QGC will be treated as BAD_DATA by pymavlink.

@hamishwillee do you have an opinion for how to fix this? Perhaps the best way is to submit a PR to fix the mavlink_helpers.h bug?

@hamishwillee
Copy link

@clydemcqueen The original PR submitted to fix this was https://github.com/ArduPilot/pymavlink/pull/241/files which as I understand it recalculates the checksum. Does this do what you need?

That stalled, for reasons given in ArduPilot/pymavlink#241 (comment) . I could try re-energise?

If that does not do what you want, then yes, a new PR. However this would likely stall in the same way unless a test case can be created along with the patch. Its all a bit frustrating since however you look at it this is a bug. Unfortunately ArduPilot/Pymavlink moves at its own schedule.

Happy to try get this pushed again, as long as we're all clear that the fix we push is the right one.

@amsmith-pro
Copy link
Contributor

amsmith-pro commented Aug 22, 2024

This is a problem when receiving as well. Made an issue at the PX4 forum about a missing byte from a message, at the time wasn't aware of this mavlink v2 truncation.

As an example, when receiving a truncated packet for OPEN_DRONE_ID_LOCATION (missing the final timestamp_accuracy field if it's set to zero), MavFrame::deser fails to deserialize it because it expects that enum and instead gets whatever value the first checksum byte is. To get around this, I currently do a check on the ID and payload length, and manually insert an extra byte before the checksum before passing it to MavFrame::deser... maybe there's a way I'm missing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants