search

dronekit / dronekit-python

DroneKit-Python library for communicating with Drones via MAVLink.
https://readthedocs.org/projects/dronekit-python/
Apache License 2.0
1.5k stars 1.42k forks source link

Make HEARTBEAT handler ignore non-vehicle HEARTBEATs #1188

Closed morzack closed 1 year ago

morzack commented 1 year ago

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 https://github.com/ArduPilot/pymavlink/pull/769 currently on their master.

peterbarker commented 1 year ago

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 commented 1 year ago

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

morzack commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@peterbarker Ping ^^^^

peterbarker commented 1 year ago

Merged it, thanks!