`MavFrame::deser` incorrectly parsing

[amsmith-pro]

deser is returning 'Unknown message with ID ' on all V2 packets received in a packet forward from QGroundControl, for example: Unknown message with ID 93952: fd 0a 00 00 6f 01 01 2a 00 00 3f 00 40 00 00 00 46 3c d6 3c 80 06

I believe the issue lies here: https://github.com/mavlink/rust-mavlink/blob/63b6af9fe59eb5cdea6aa385d0c3f4f3d2f2303b/mavlink-core/src/lib.rs#L177-L179

deser reads the first three bytes of the frame into seq, sys_id, and comp_id. However the V2 packet has four bytes preceding those fields (starting with the packet start marker 0xFD):

More Serialization Info Here

Lopping off the first four bytes of the buffer first then makes deser work.

If this isn't intended behavior, I can fork and fix

[joaoantoniocardoso]

yeah, I believe we need to differentiate between V1 and V2 messages to deserialize

[joaoantoniocardoso]

@amsmith-pro your diff makes sense, just needs to be that only for V2:

if matches!(version, MavlinkVersion::V2) {
    buf.get_u32_le();
}

[amsmith-pro]

@amsmith-pro your diff makes sense, just needs to be that only for V2:

if matches!(version, MavlinkVersion::V2) {
    buf.get_u32_le();
}

Would this also be the same issue for V1? There's still an STX and len field in V1, two bytes instead of the four we're ignoring in the V2 case

[pv42]

I think MAVFrame in this context only starts with the SEQ byte in both MAVLink versions. I agree that the name is confusing as MAVLink Frame generally includes all bytes in the frame. But looking at https://github.com/mavlink/rust-mavlink/blob/63b6af9fe59eb5cdea6aa385d0c3f4f3d2f2303b/mavlink/tests/mav_frame_tests.rs#L9-L27 it very much seems intentional.

[amsmith-pro]

https://github.com/mavlink/rust-mavlink/blob/63b6af9fe59eb5cdea6aa385d0c3f4f3d2f2303b/mavlink-core/src/lib.rs#L503-L540

MAVLinkV2MessageRaw seems to be what I expected MavFrame to be without the deser functionality, just multiple serialize functions

[amsmith-pro]

I agree that MavFrame and this implementation is a bit misleading as it's not the whole frame

However, changing MavFrame ser and deser to account for these extra preceding bytes seems likely to break a lot of stuff downstream, so for the time being I'd settle for adding an explicit note in the MavFrame docs for the MavFrame struct itself and the ser and deser functions. If that's alright I'll update my branch to simply amend the comments

Maybe a later major version can account for them, though. Seems like a breaking change