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

Add a mapping of ArduPilot custom flight modes #61

Merged
merged 1 commit into from
Jan 27, 2018

Conversation

WickedShell
Copy link

This maps out all currently supported flight modes that are reported via the custom_mode field of heartbeat.

@lvale
Copy link
Member

lvale commented Jan 13, 2018

ROVER_FLIGHT_MODE ROVER_FLIGHT_MODE_INITIALISING

I don't see the need for the repetition of the _FLIGHT_MODE part

@OXINARF
Copy link
Member

OXINARF commented Jan 15, 2018

@lvale The macro names don't have the enum name so if you remove the FLIGHT_MODE part, it will be just ROVER_INITIALISING which doesn't say it is a flight mode.

@WickedShell
Copy link
Author

I can see the arguments either way. I suspect that _FLIGHT could be removed from the middle without losing any clarity, (and would actually make more sense for rover, sub, tracker without hurting copter or plane).

@WickedShell
Copy link
Author

WickedShell commented Jan 16, 2018

Renamed to <vehicle>_MODE_<mode>. If someone can just review that this matches the content of all the vehicles defines.h this should be good to go.

Copy link

@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.

LGTM. The Z vs S thing doesn't bother me too much.

<entry value="12" name="PLANE_MODE_LOITER"/>
<entry value="14" name="PLANE_MODE_AVOID_ADSB"/>
<entry value="15" name="PLANE_MODE_GUIDED"/>
<entry value="16" name="PLANE_MODE_INITIALISING"/>

Choose a reason for hiding this comment

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

Shold be INITIALIZING for consistency.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

<entry value="2" name="PLANE_MODE_STABILIZE"/>
<entry value="3" name="PLANE_MODE_TRAINING"/>
<entry value="4" name="PLANE_MODE_ACRO"/>
<entry value="5" name="PLANE_MODE_FLY_BY_WIRE_A"/>

Choose a reason for hiding this comment

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

Maybe a little wordy.

Copy link
Author

Choose a reason for hiding this comment

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

I'd prefer the clarity/lack of ambiguity of spelling it out here.

@OXINARF
Copy link
Member

OXINARF commented Jan 25, 2018

Has anyone confirmed this matches the ArduPilot values?

@peterbarker
Copy link

peterbarker commented Jan 25, 2018 via email

@OXINARF OXINARF merged commit be9f426 into master Jan 27, 2018
@OXINARF
Copy link
Member

OXINARF commented Jan 27, 2018

Thanks @peterbarker! Merged!

@OXINARF OXINARF deleted the wickedshell/mavlink-modes branch January 27, 2018 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants