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

Make HEARTBEAT handler ignore non-vehicle HEARTBEATs #1188

Merged

Conversation

morzack
Copy link
Contributor

@morzack morzack commented Apr 14, 2023

Hi, this fixes #1187 . Check the issue for more details.

TL;DR (the two lines :) ): use mavutil's probably_vehicle_heartbeat function to avoid trying to parse non-vehicle HEARTBEATs.

Do note that this should not be merged in until pymavlink releases a new version. It relies on logic introduced in ArduPilot/pymavlink#769 currently on their master.

@peterbarker
Copy link
Contributor

Thanks for this!

Please ping me when you think we should merge this!

... we probably want something in the requirements.txt to require a version of pymavlink....

@bestaps
Copy link

bestaps commented Apr 16, 2023

verified this. its okay to merge this PR to main release @peterbarker

@morzack
Copy link
Contributor Author

morzack commented Apr 16, 2023

Please ping me when you think we should merge this!

Ah yeah, I was wrong about there needing to be any waiting, the function already exists in the version of pymavlink used by dronekit. The new functionality won't be exposed until a new pymavlink release is made, however.

It can be merged at any time.

@morzack
Copy link
Contributor Author

morzack commented May 6, 2023

Hey! Just a quick follow-up since the most recent release of pymavlink implements the extra logic used by this commit. This is ready to be merged in whenever.

... we probably want something in the requirements.txt to require a version of pymavlink....

The minimum version of pymavlink specified in requirements.txt and setup.py has the probably_vehicle_heartbeat() function implemented, and pip will be default download the latest version of pymavlink when getting dronekit.

If you want, I can make a separate commit/PR to bump the minimum version of pymavlink to something newer, but that's not necessary for this PR to work.

@peterbarker if this looks good to you, feel free to merge when you get a chance. LMK if it needs changes.

@hamishwillee
Copy link
Contributor

@peterbarker Ping ^^^^

@peterbarker peterbarker merged commit 3ee89b7 into dronekit:master Jun 15, 2023
@peterbarker
Copy link
Contributor

Merged it, thanks!

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 this pull request may close these issues.

HEARTBEAT handler failing to handle non-vehicle HEARTBEATs
4 participants