search

PX4 / PX4-Autopilot

PX4 Autopilot Software
https://px4.io
BSD 3-Clause "New" or "Revised" License
8.58k stars 13.56k forks source link

standard_modes: add vehicle-type specific standard modes #24011

Open bkueng opened 1 week ago

bkueng commented 1 week ago

See https://mavlink.io/en/messages/development.html#MAV_STANDARD_MODE. The only standard mode that is not set is MAV_STANDARD_MODE_SAFE_RECOVERY, as PX4 uses RTL for that (with configuration parameters).

Changelog Entry

For release notes:

Implement vehicle-type specific MAVLink standard modes.
github-actions[bot] commented 1 week ago

FLASH Analysis

px4_fmu-v5x ``` FILE SIZE VM SIZE -------------- -------------- +0.0% +726 [ = ] 0 .debug_loc +0.0% +444 [ = ] 0 [section .debug_loc] +0.2% +282 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +0.0% +290 [ = ] 0 .debug_line +0.5% +161 [ = ] 0 ../../src/modules/commander/Commander.cpp +0.1% +129 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +0.0% +160 +0.0% +160 .text +0.1% +100 +0.1% +100 src/modules/mavlink/modules__mavlink_unity.cpp +0.4% +72 +0.4% +72 ../../src/modules/commander/Commander.cpp -0.0% -12 -0.0% -12 [section .text] +0.0% +98 [ = ] 0 .debug_info +0.0% +56 [ = ] 0 ../../src/modules/commander/Commander.cpp +0.0% +42 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +0.0% +48 [ = ] 0 .symtab +0.1% +64 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +1.1% +48 [ = ] 0 ../../src/modules/commander/Commander.cpp -0.1% -64 [ = ] 0 [section .symtab] +0.0% +25 [ = ] 0 .strtab +0.1% +48 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp -0.0% -23 [ = ] 0 [section .strtab] +0.0% +16 [ = ] 0 .debug_frame +0.0% +16 [ = ] 0 .debug_ranges +0.0% +40 [ = ] 0 [section .debug_ranges] -0.1% -24 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +0.0% +8 [ = ] 0 .debug_aranges -0.6% -155 [ = ] 0 [Unmapped] +0.0% +1.20Ki +0.0% +160 TOTAL ```
px4_fmu-v6x ``` FILE SIZE VM SIZE -------------- -------------- +0.0% +696 [ = ] 0 .debug_loc +0.0% +440 [ = ] 0 [section .debug_loc] +0.2% +269 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp -0.0% -13 [ = ] 0 ../../src/modules/commander/Commander.cpp +0.0% +290 [ = ] 0 .debug_line +0.5% +161 [ = ] 0 ../../src/modules/commander/Commander.cpp +0.1% +129 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +0.0% +160 +0.0% +160 .text +0.1% +100 +0.1% +100 src/modules/mavlink/modules__mavlink_unity.cpp +0.4% +72 +0.4% +72 ../../src/modules/commander/Commander.cpp -0.0% -12 -0.0% -12 [section .text] +0.0% +98 [ = ] 0 .debug_info +0.0% +56 [ = ] 0 ../../src/modules/commander/Commander.cpp +0.0% +42 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +0.0% +48 [ = ] 0 .symtab +0.1% +64 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +1.1% +48 [ = ] 0 ../../src/modules/commander/Commander.cpp -0.1% -64 [ = ] 0 [section .symtab] +0.0% +25 [ = ] 0 .strtab +0.1% +48 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp -0.0% -23 [ = ] 0 [section .strtab] +0.0% +16 [ = ] 0 .debug_frame +0.0% +16 [ = ] 0 .debug_ranges +0.0% +40 [ = ] 0 [section .debug_ranges] -0.1% -24 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +0.0% +8 [ = ] 0 .debug_aranges -0.2% -157 [ = ] 0 [Unmapped] +0.0% +1.17Ki +0.0% +160 TOTAL ```
github-actions[bot] commented 6 days ago

FLASH Analysis

px4_fmu-v5x ``` FILE SIZE VM SIZE -------------- -------------- +0.0% +711 [ = ] 0 .debug_loc +0.0% +442 [ = ] 0 [section .debug_loc] +0.2% +282 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp -0.0% -13 [ = ] 0 ../../src/modules/commander/Commander.cpp +0.0% +290 [ = ] 0 .debug_line +0.5% +161 [ = ] 0 ../../src/modules/commander/Commander.cpp +0.1% +129 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +0.0% +152 +0.0% +152 .text +0.1% +100 +0.1% +100 src/modules/mavlink/modules__mavlink_unity.cpp +0.3% +68 +0.3% +68 ../../src/modules/commander/Commander.cpp -0.0% -16 -0.0% -16 [section .text] +0.0% +98 [ = ] 0 .debug_info +0.0% +56 [ = ] 0 ../../src/modules/commander/Commander.cpp +0.0% +42 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +0.0% +48 [ = ] 0 .symtab +0.1% +64 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +1.1% +48 [ = ] 0 ../../src/modules/commander/Commander.cpp -0.1% -64 [ = ] 0 [section .symtab] +0.0% +25 [ = ] 0 .strtab +0.1% +48 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp -0.0% -23 [ = ] 0 [section .strtab] +0.0% +16 [ = ] 0 .debug_frame +0.0% +16 [ = ] 0 .debug_ranges +0.0% +40 [ = ] 0 [section .debug_ranges] -0.1% -24 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +0.0% +8 [ = ] 0 .debug_aranges -0.6% -144 [ = ] 0 [Unmapped] +0.0% +1.19Ki +0.0% +152 TOTAL ```
px4_fmu-v6x ``` FILE SIZE VM SIZE -------------- -------------- +0.0% +696 [ = ] 0 .debug_loc +0.0% +440 [ = ] 0 [section .debug_loc] +0.2% +269 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp -0.0% -13 [ = ] 0 ../../src/modules/commander/Commander.cpp +0.0% +290 [ = ] 0 .debug_line +0.5% +161 [ = ] 0 ../../src/modules/commander/Commander.cpp +0.1% +129 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +0.0% +152 +0.0% +152 .text +0.1% +100 +0.1% +100 src/modules/mavlink/modules__mavlink_unity.cpp +0.3% +68 +0.3% +68 ../../src/modules/commander/Commander.cpp -0.0% -16 -0.0% -16 [section .text] +0.0% +98 [ = ] 0 .debug_info +0.0% +56 [ = ] 0 ../../src/modules/commander/Commander.cpp +0.0% +42 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +0.0% +48 [ = ] 0 .symtab +0.1% +64 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +1.1% +48 [ = ] 0 ../../src/modules/commander/Commander.cpp -0.1% -64 [ = ] 0 [section .symtab] +0.0% +25 [ = ] 0 .strtab +0.1% +48 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp -0.0% -23 [ = ] 0 [section .strtab] +0.0% +16 [ = ] 0 .debug_frame +0.0% +16 [ = ] 0 .debug_ranges +0.0% +40 [ = ] 0 [section .debug_ranges] -0.1% -24 [ = ] 0 src/modules/mavlink/modules__mavlink_unity.cpp +0.0% +8 [ = ] 0 .debug_aranges -0.2% -145 [ = ] 0 [Unmapped] +0.0% +1.18Ki +0.0% +152 TOTAL ```
mrpollo commented 6 days ago

FYI - tests are passing now

hamishwillee commented 2 days ago

The only standard mode that is not set is MAV_STANDARD_MODE_SAFE_RECOVERY, as PX4 uses RTL for that (with configuration parameters).

So my thinking on this was that MAV_STANDARD_MODE_SAFE_RECOVERY is "smart RTL", and that would be the mode you would set if you were doing an RTL that had anything other than home via straight line configure (mission, rally point etc). So if any of the params were set to non-RTL then instead of RTL you'd set this.

Are you saying that is too hard? (not arguing, trying to understand the logic).

bkueng commented 2 days ago

Are you saying that is too hard?

Ok that's possible, we have to evaluate the param then. If you look at the param, which values would you map to which type? https://github.com/PX4/PX4-Autopilot/blob/main/src/modules/navigator/rtl_params.c#L107-L117

hamishwillee commented 1 day ago

Are you saying that is too hard?

Ok that's possible, we have to evaluate the param then. If you look at the param, which values would you map to which type? https://github.com/PX4/PX4-Autopilot/blob/main/src/modules/navigator/rtl_params.c#L107-L117

Not that easy unfortunately - because the parameters set a mode that then depends on the existence of rally points or mission landing.

So basically the vehicle is doing Smart RTL if it is heading to a rally point/mission landing/following a mission path. You might not actually know which of those it is doing until the RTL is triggered. All of them AFAIK fall back to RTL.

What is possible here? Can we set that the smart mode is available, but only make it active conditional on the actual return target?

Either way, it sounds like we shouldn't block this PR on any of my comments.

bkueng commented 1 day ago

What is possible here? Can we set that the smart mode is available, but only make it active conditional on the actual return target?

It's possible, but currently that information isn't exposed. Probably it would not be much effort to add it. So you would indicate both standard modes as available (which means the same internal mode needs to be sent out twice for both standard modes, and the GCS needs to handle that), then for mode change accept both, and as the current mode emit dynamically based on rally point vs home? Isn't that unexpected for a GCS?