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

(Bug) Deser function in MavFrame/MavHeader not working properly #205

Closed
IhsenBouallegue opened this issue Nov 20, 2023 · 11 comments · Fixed by #206
Closed

(Bug) Deser function in MavFrame/MavHeader not working properly #205

IhsenBouallegue opened this issue Nov 20, 2023 · 11 comments · Fixed by #206

Comments

@IhsenBouallegue
Copy link
Contributor

IhsenBouallegue commented Nov 20, 2023

While using rust mavlink to serialize MavFrames and send them over a LoRa link, I noticed some weird inconsistencies.
I noticed a weird problem. Not sure if I am the only one having it.
Here are my findings, if you can point me I will be happy to fix it and open a PR.

image
All the values inside the MavMessage are wrong. But not random! so it's not a problem with the LoRa link. I also printed the buffers out and confirmed that they are identical. The problem happens when deserialising.
I noticed something peculiar
20 is 0000000000010100 while 5120 is 0001010000000000
1792 is 0000011100000000 while 0000000000000111
It is consistent. At first, I suspected it has something to do with big/little endian. But maybe the bits don't quite align for that? Not sure what it can be, because I can't find the code responsible for serializing and deserializing the message anywhere in the repo. Is it being imported from somewhere?
I would appreciate any hint 😃

@patrickelectric
Copy link
Member

patrickelectric commented Nov 20, 2023

Merged and available on latest release: https://github.com/mavlink/rust-mavlink/releases/tag/0.12.1

@IhsenBouallegue
Copy link
Contributor Author

hey @patrickelectric thank you for releasing this fast. But this issue, while related to PR #204, is not the same. That PR fixes another related problem (I didn't bother creating an issue for it, because it was straightforward).
As far as I understood, the problem with the parsing of the MavMessage (discussed here) was not fixed yet. So this issue remains open.
Let me know if I missed something.

@patrickelectric
Copy link
Member

Hi @IhsenBouallegue, sorry for closing the issue early.

It's a weird problem, to be sure, what kind of hardware is sending this messages ? is it x86 ? is it ARM ? It's probably not an issue with the LoRa link, but it's the first time that I'm seeing such problem.

@patrickelectric
Copy link
Member

And how the companion part is running ? Is it using rust-mavlink as well ?

@IhsenBouallegue
Copy link
Contributor Author

@patrickelectric I threw together a reproduction repo (no lora, just ser and then deser, and still getting the same output)
https://github.com/IhsenBouallegue/mavlink-rust-test
This also means it has nothing to do with the hardware. But here is what I tried it on:

  • on companion computer and ground station (Raspberry pi 4) running Pi OS 64Bit and compiling with target arch aarch64-unknown-linux-gnu
  • Windows 11 compiling with x86_64-pc-windows-msvc. Here is the output (basically the same)
MavFrame {
    header: MavHeader {
        system_id: 1,
        component_id: 1,
        sequence: 84,
    },
    msg: LINK_NODE_STATUS(
        LINK_NODE_STATUS_DATA {
            timestamp: 92197916,
            tx_rate: 53,
            rx_rate: 20,
            messages_sent: 47,
            messages_received: 21,
            messages_lost: 0,
            rx_parse_err: 0,
            tx_overflows: 0,
            rx_overflows: 0,
            tx_buf: 0,
            rx_buf: 0,
        },
    ),
    protocol_version: V2,
}
MavFrame {
    header: MavHeader {
        system_id: 1,
        component_id: 1,
        sequence: 84,
    },
    msg: LINK_NODE_STATUS(
        LINK_NODE_STATUS_DATA {
            timestamp: 23602666496,
            tx_rate: 13568,
            rx_rate: 5120,
            messages_sent: 12032,
            messages_received: 5376,
            messages_lost: 0,
            rx_parse_err: 0,
            tx_overflows: 0,
            rx_overflows: 0,
            tx_buf: 0,
            rx_buf: 0,
        },
    ),
    protocol_version: V2,
}

(On a good note, the issue with the sequence is fixed 😄)

@patrickelectric
Copy link
Member

Thanks for creating your repository @IhsenBouallegue.

It should be fixed by #206 now, I just used in your repository and it indeed solves the problem.

I'll add your example as a test.

@IhsenBouallegue
Copy link
Contributor Author

Hey @patrickelectric, most welcome! Glad to see it fixed.
Would you be so kind and do a quick hotfix release, so I can use it in my full repo?

@patrickelectric
Copy link
Member

Hey @patrickelectric, most welcome! Glad to see it fixed. Would you be so kind and do a quick hotfix release, so I can use it in my full repo?

Soon after merging a second test, I'll update you.

@patrickelectric
Copy link
Member

@IhsenBouallegue tag done, should be available soon.

@IhsenBouallegue
Copy link
Contributor Author

@patrickelectric Yup it works! Can't thank you enough :D You literally saved my thesis 😆 You, my friend, will be going into the acknowledgements 🏆

@patrickelectric
Copy link
Member

@patrickelectric Yup it works! Can't thank you enough :D You literally saved my thesis 😆 You, my friend, will be going into the acknowledgements 🏆

wow, Thank you 😆

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

Successfully merging a pull request may close this issue.

2 participants