Pr auto disarm landing - #120
Conversation
… 400 and 0.8 respectively
9bdfc54 to
fd07d95
Compare
Greptile SummaryThis PR adjusts auto-disarm and landing detection parameters following analysis of a mid-air disarm event observed during back-transition (VTOL→fixed-wing). It makes three interrelated changes: tightens the landing altitude-change threshold for the Pixhawk6C-bdshot hardware (1.5 m → 0.8 m), replaces two hardcoded timing constants in the land-detector with tunable parameters (
Confidence Score: 3/5The landing state-machine change is intentional and addresses a real stuck-state bug, but it unconditionally accepts any mid-flight disarm as 'land complete' without verifying the aircraft is actually on the ground. The fix in check_land_complete() solves the observed deadlock, but the new early-return path fires for every disarm source — not just the back-transition scenario. A failsafe disarm during QPOS_LAND_FINAL (e.g., brief RC loss mid-descent) will immediately set QPOS_LAND_COMPLETE and skip all altitude-stability and time-window checks. The consequences of a false 'land complete' declaration while airborne — suppressed motor commands, mode transitions, mission progression — depend on callers that aren't modified in this PR and warrant careful verification before merging. ArduPlane/quadplane.cpp — specifically the check_land_complete() change and how callers respond to an early QPOS_LAND_COMPLETE transition. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["check_land_complete() called"] --> B{"poscontrol state\n== QPOS_LAND_FINAL?"}
B -- No --> C["return false"]
B -- Yes --> D{"motors->armed()?"}
D -- "No (NEW PATH)" --> E["poscontrol.set_state(QPOS_LAND_COMPLETE)\nreturn true"]
D -- Yes --> F["Start / update disarm watchdog"]
F --> G["land_detector()"]
G --> H{"land_detector\nreturns true?"}
H -- No --> I["return false"]
H -- Yes --> J["poscontrol.set_state(QPOS_LAND_COMPLETE)\nplane.arming.disarm(LANDED)"]
J --> K["return true"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["check_land_complete() called"] --> B{"poscontrol state\n== QPOS_LAND_FINAL?"}
B -- No --> C["return false"]
B -- Yes --> D{"motors->armed()?"}
D -- "No (NEW PATH)" --> E["poscontrol.set_state(QPOS_LAND_COMPLETE)\nreturn true"]
D -- Yes --> F["Start / update disarm watchdog"]
F --> G["land_detector()"]
G --> H{"land_detector\nreturns true?"}
H -- No --> I["return false"]
H -- Yes --> J["poscontrol.set_state(QPOS_LAND_COMPLETE)\nplane.arming.disarm(LANDED)"]
J --> K["return true"]
Reviews (1): Last reviewed commit: "default values of parameters, LND_RELAX_..." | Re-trigger Greptile |
| if (!motors->armed()) { | ||
| return false; | ||
| poscontrol.set_state(QPOS_LAND_COMPLETE); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Disarm-triggered land-complete bypasses all ground-detection checks
Any unintended disarm event (RC failsafe, battery failsafe, GCS disarm command) while the state machine is in QPOS_LAND_FINAL will now immediately set QPOS_LAND_COMPLETE and return true, regardless of whether the aircraft is actually on the ground. Previously the function returned false and kept the state machine in QPOS_LAND_FINAL; the new path skips the altitude-change guard (detect_alt_change) and both time-window checks in land_detector(). If a failsafe disarm fires mid-descent (e.g., short RC loss), the system declares landing complete while still airborne, potentially stopping motor commands or suppressing any subsequent recovery attempt.
|
|
||
| AP_GROUPINFO("DARM_WDG_T", 41, QuadPlane, landing_detect.wdg_timeout_s, 10.0), | ||
|
|
||
| // @Param: LND_RELAX_MS | ||
| // @DisplayName: Landing relax debounce time | ||
| // @Description: Time in milliseconds that throttle must be at lower limit continuously before should_relax() returns true | ||
| // @Units: ms | ||
| // @Range: 100 1000 | ||
| // @Increment: 50 | ||
| // @User: Advanced | ||
| AP_GROUPINFO("LND_RELAX_MS", 42, QuadPlane, landing_detect.relax_debounce_ms, 200), | ||
|
|
||
| // @Param: LND_LLIM_MS | ||
| // @DisplayName: Landing lower limit extra time | ||
| // @Description: Extra time in milliseconds added to land detection timeout for lower limit guard in land_detector() | ||
| // @Units: ms | ||
| // @Range: 100 1000 | ||
| // @Increment: 50 | ||
| // @User: Advanced | ||
| AP_GROUPINFO("LND_LLIM_MS", 43, QuadPlane, landing_detect.lower_limit_extra_ms, 200), | ||
|
|
||
| AP_GROUPEND | ||
| }; | ||
|
|
There was a problem hiding this comment.
Code-level default for
LND_RELAX_MS (200 ms) diverges from analysis recommendation (400 ms)
The AP_GROUPINFO default is 200 ms, but the defaults.parm for Pixhawk6C-bdshot sets 400 ms, and the PR description cites 400 ms as the validated safe value. Any board that does not load this hardware defaults file will get 200 ms — half the debounce time — which was not tested against the mid-air disarm dataset. Consider either raising the compile-time default to 400 ms or documenting that 200 ms is intentionally conservative for general hardware.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| AP_Int16 relax_debounce_ms; | ||
| AP_Int16 lower_limit_extra_ms; |
There was a problem hiding this comment.
Trailing whitespace on the
relax_debounce_ms declaration — minor cleanup.
| AP_Int16 relax_debounce_ms; | |
| AP_Int16 lower_limit_extra_ms; | |
| AP_Int16 relax_debounce_ms; | |
| AP_Int16 lower_limit_extra_ms; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Based on the historical analysis and the mid-air disarm observed during back transition, the default values of LAND_ALTCHG and LAND_RELAX_MS have been changed to 0.8 m and 400 ms, respectively.
https://app.notion.com/p/airbound/Auto-disarm-during-the-flight-midair-analysis-38621adf4be98058a17ed728a5d2dab2?source=copy_link