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

Update deprecated MAV_TYPE enum for tailsitter #889

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Dennis-Johnson
Copy link

Hi,

This fixes #571.

With latest Pymavlink (master), the mavgen step works, but on trying to import mavutil, I get this error.
Screenshot 2023-12-15 at 1 46 44 PM

It seems that these two enums were changed, but weren't reflected in mavutil.py

  • MAV_TYPE_VTOL_TAILSITTER_QUADROTOR was previously MAV_TYPE_VTOL_QUADROTOR
  • MAV_TYPE_VTOL_TAILSITTER_DUOROTOR was previously MAV_TYPE_VTOL_DUOROTOR

I believe there is still a reference to the old enum name in Swift generator tests pymavlink/generator/swift/Tests/MAVLinkTests/Testdata/common.xml, but I haven't run that locally.

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would break ArduPilot's CI as we have not adopted the name-change into ardupilot/mavlink.

@hamishwillee what process was gone through to ensure renaming this enumeration entry would be smooth? i.e. MAV_TYPE_VTOL_QUADROTOR -> MAV_TYPE_VTOL_TAILSITTER_QUADROTOR ?

I'm not actually sure the rename is actually valid - the description does not precluse the use of MAV_TYPE_VTOL_DUOROTOR for belly-sitters as well as tailsitters.

@hamishwillee
Copy link
Contributor

So it was discussed with @auturgy @DonLakeFlyer @meee1 both in mavlink/mavlink#1818 and also offline. The reasoning for the change is mostly here: mavlink/mavlink#1808 (comment)

  • the description does not precluse the use of MAV_TYPE_VTOL_DUOROTOR for belly-sitters as well as tailsitters.

W.r.t. there is no such thing. Though note that I see https://mavlink.io/en/messages/minimal.html#MAV_TYPE_VTOL_TAILSITTER is misdocumented in the description to mention that removed concept. It should say MAV_TYPE_VTOL_TAILSITTER_DUOROTOR.

For a two rotor bellysitter you'd have to use MAV_TYPE_VTOL_FIXEDROTOR or a more specific MAV_TYPE_VTOL_FIXEDROTOR_DUO_ROTOR or similar. If that is invalid then yes, more thinking needed.

@peterbarker
Copy link
Contributor

I've created a PR with an alternative approach here: #898

That will let mavutil.py work with both old and new message definitions.

Why has this only just now become a problem? It looks like these renames happened quite some time ago, which should have led to the mavutil.py AttributeErrors?!?

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.

Problem with latest pymavlink - mavutil
3 participants