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

Standard methods for parsing available flight mode for PX4 and APM #12254

Merged
merged 2 commits into from
Jan 3, 2025

Conversation

bytesByHarsh
Copy link
Collaborator

Update APM and PX4 flight mode parsing methods

Description

  • Removed use of direct strings to change flight mode.
  • Case-sensitive issues in flight mode in APM.
  • Internal Mapping to flight mode names.
  • QGC Custom Build support has also been added.
  • Add New Modes discovered via Available Mode packets received from the flight controller.

Related Issue

#12191
#12128

Update APM and PX4 flight mode parsing methods
@DonLakeFlyer
Copy link
Contributor

Working my way through this. Found some raw flight mode strings here: https://github.com/mavlink/qgroundcontrol/blob/master/src/Vehicle/Vehicle.cc#L1267. Not sure what that is used for.

@DonLakeFlyer
Copy link
Contributor

Shouldn't this method: https://github.com/mavlink/qgroundcontrol/blob/master/src/FirmwarePlugin/FirmwarePlugin.cc#L54 be updated. A non-PX4/ArduPilot vehicle could still support the new standard modes protocol. In that case the real names should be available. Seems like it should check to see if the list is populated and use that if available, otherwise do it the old way.

@DonLakeFlyer
Copy link
Contributor

Seems odd that standard modes don't come across with names: https://github.com/mavlink/qgroundcontrol/blob/master/src/Vehicle/StandardModes.cc#L47. @bkueng That said shouldn't these be translated?

@DonLakeFlyer
Copy link
Contributor

@bkueng This is interesting as well: https://github.com/mavlink/qgroundcontrol/blob/master/src/Vehicle/StandardModes.cc#L75. Not sure it makes sense for these to not be available for usage from the flight modes menu. They don't cause a problem if used from there do they?

@DonLakeFlyer
Copy link
Contributor

@bytesByHarsh If you search for .flightMode in .qml files you'll find a couple places which are still using raw flight mode strings. Those need to be cleaned up as well.

Copy link
Contributor

@DonLakeFlyer DonLakeFlyer left a comment

Choose a reason for hiding this comment

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

Mainly looks great. Just a couple questions.

src/FirmwarePlugin/PX4/PX4FirmwarePlugin.cc Outdated Show resolved Hide resolved
src/FirmwarePlugin/FirmwarePlugin.cc Outdated Show resolved Hide resolved
src/FirmwarePlugin/FirmwarePlugin.cc Outdated Show resolved Hide resolved
* Remove loose flight mode strings
* Force All Enums in switch cases
* Update function calls to parse flight mode list
if(WIN32)
add_compile_options(/w44265 /we44265)
else()
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wswitch -Werror=switch")
Copy link
Contributor

Choose a reason for hiding this comment

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

@HTRamsey This won't be needed with the switch to -Wall right?

@DonLakeFlyer DonLakeFlyer merged commit 81d3cc1 into mavlink:master Jan 3, 2025
8 checks passed
@rubenp02
Copy link
Contributor

rubenp02 commented Jan 4, 2025

The flight modes seem to get added twice to the flight mode list in APM. It only seems to happen with master, in Plane 4.5 it's fine.

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.

3 participants