Land detector: Remove LNDMC_TRIG_TIME parameter #25251

MaEtUgR · 2025-07-18T12:44:56Z

Solved Problem

When examining the land detector, I found that:

Removed the unused LNDMC_TRIG_TIME parameter added by me in this commit to simplify configuration.
Cleaned up _set_hysteresis_factor() and related logic, which were global but only relevant for multicopters. This was added in land_detector: robustify land detection by using distance to ground info #17403)

Changelog Entry

Land detector: Remove LNDMC_TRIG_TIME parameter

Test coverage

No testing performed, this is a refactor aside from removing the parameter.

docs/assets/airframes/multicopter/amovlab_f410/amovlabf410_drone_v1.15.4.params

docs/en/advanced_config/land_detector.md

dakejahl · 2025-07-18T18:20:44Z

src/modules/land_detector/MulticopterLandDetector.cpp

 *maybe_landed can only occur if the internal ground_contact hysteresis state is true. maybe_landed criteria requires to have no motion in x and y,
 *no rotation and a thrust below 0.1 of the thrust_range (thrust_hover - thrust_min). In addition, the mc_pos_control turns off the thrust_sp in
- *body frame along x and y which helps to detect maybe_landed. The criteria for maybe_landed needs to be true for (LNDMC_TRIG_TIME / 3) seconds.
+ *body frame along x and y which helps to detect maybe_landed. The criteria for maybe_landed needs to be true for 1/3 seconds.


Suggested change

*body frame along x and y which helps to detect maybe_landed. The criteria for maybe_landed needs to be true for 1/3 seconds.

*body frame along x and y which helps to detect maybe_landed. The criteria for maybe_landed needs to be true for 300ms.

idk why but 1/3 of a second feels weird to me lol

Plus it is actually 300_ms below.

dakejahl · 2025-07-18T18:22:52Z

src/modules/land_detector/MulticopterLandDetector.cpp

+	hrt_abstime land_detection_time = 1_s;
+
+	if (_dist_bottom_is_observable && !_vehicle_local_position.dist_bottom_valid) {
+		land_detection_time = 3_s;


I wonder if this logic is really needed or even useful

dakejahl · 2025-07-18T18:27:54Z

I took a quick look at the full MulticopterLandDetector.cpp while reviewing this -- it could probably use a larger cleanup to improve and simplify the logic flow and use of member variables. If you're feeling frisky 🐱

…factor() function

Co-authored-by: Jacob Dahl <37091262+dakejahl@users.noreply.github.com>

github-actions · 2025-10-14T05:38:06Z

No flaws found

github-actions · 2025-10-14T05:45:09Z

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: -328 byte (-0.02 %)]

    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%     +19  [ = ]       0    .debug_abbrev
-0.0%     -24  [ = ]       0    .debug_aranges
-0.0%     -72  [ = ]       0    .debug_frame
-0.0% -4.23Ki  [ = ]       0    .debug_info
-0.0%    -457  [ = ]       0    .debug_line
  [DEL]      -2  [ = ]       0    [Unmapped]
  -0.0%    -455  [ = ]       0    [section .debug_line]
-0.0%    -466  [ = ]       0    .debug_loclists
-0.0%     -57  [ = ]       0    .debug_rnglists
  [NEW]      +2  [ = ]       0    [Unmapped]
  -0.0%     -59  [ = ]       0    [section .debug_rnglists]
-0.0% -1.58Ki  [ = ]       0    .debug_str
-0.0%    -184  [ = ]       0    .strtab
 -51.6%     -16  [ = ]       0    __ioctl_veneer
   +70%     +16  [ = ]       0    __nxsem_trywait_veneer
  [DEL]     -66  [ = ]       0    land_detector::AirshipLandDetector::_set_hysteresis_factor()
  [DEL]     -68  [ = ]       0    land_detector::FixedwingLandDetector::_set_hysteresis_factor()
  [DEL]     -70  [ = ]       0    land_detector::MulticopterLandDetector::_set_hysteresis_factor()
  [NEW]     +84  [ = ]       0    land_detector::RoverLandDetector::RoverLandDetector()
  [DEL]     -64  [ = ]       0    land_detector::RoverLandDetector::_set_hysteresis_factor()
-0.0%     -80  [ = ]       0    .symtab
   +50%     +16  [ = ]       0    ConstLayer::contains()
 -33.3%     -16  [ = ]       0    ConstLayer::store()
  -0.3%     -32  [ = ]       0    [section .symtab]
 -40.0%     -32  [ = ]       0    __ioctl_veneer
   +67%     +32  [ = ]       0    __nxsem_trywait_veneer
   +50%     +16  [ = ]       0    __nxsig_ismember_veneer
 -33.3%     -16  [ = ]       0    __stm32_i2c_modifyreg32_veneer
  [DEL]     -32  [ = ]       0    land_detector::AirshipLandDetector::_set_hysteresis_factor()
  [DEL]     -16  [ = ]       0    land_detector::FixedwingLandDetector::_set_hysteresis_factor()
 -25.0%     -16  [ = ]       0    land_detector::MulticopterLandDetector::_get_ground_contact_state()
  [DEL]     -48  [ = ]       0    land_detector::MulticopterLandDetector::_set_hysteresis_factor()
   +33%     +16  [ = ]       0    land_detector::MulticopterLandDetector::_update_topics()
  [NEW]     +64  [ = ]       0    land_detector::RoverLandDetector::RoverLandDetector()
   +25%     +16  [ = ]       0    land_detector::RoverLandDetector::_get_landed_state()
  [DEL]     -48  [ = ]       0    land_detector::RoverLandDetector::_set_hysteresis_factor()
 -33.3%     -16  [ = ]       0    land_detector::RoverLandDetector::updateParamsImpl()
  +100%     +16  [ = ]       0    param_for_index
  +7.7%     +16  [ = ]       0    param_get_cplusplus()
  +100%     +16  [ = ]       0    param_get_index
 -33.3%     -16  [ = ]       0    px4::parameters_volatile
+4.1%    +328  [ = ]       0    [Unmapped]
-0.1%     -16  -0.1%     -16    .ramfunc
   +14%      +1   +14%      +1    __nxsig_ismember_veneer
 -12.5%      -1 -12.5%      -1    __stm32_i2c_modifyreg32_veneer
  -1.7%      -4  -1.7%      -4    param_get
  -1.4%      -4  -1.4%      -4    param_get_default_value
 -25.0%      -4 -25.0%      -4    param_get_index
  -8.3%      -4  -8.3%      -4    param_get_system_default_value
-0.0%    -312  -0.0%    -312    .text
  [NEW]    +128  [NEW]    +128    land_detector::RoverLandDetector::RoverLandDetector()
   +89%    +100   +89%    +100    land_detector::MulticopterLandDetector::_update_topics()
  +0.0%     +12  +0.0%     +12    [section .text]
  +4.8%      +4  +4.8%      +4    FlightTask
   +44%      +4   +44%      +4    g_nullstring
 -25.0%      -4 -25.0%      -4    ConstLayer::contains()
 -14.3%      -4 -14.3%      -4    ConstLayer::get()
  -2.9%      -4  -2.9%      -4    land_detector::AirshipLandDetector
  -2.9%      -4  -2.9%      -4    land_detector::FixedwingLandDetector
  [DEL]      -4  [DEL]      -4    land_detector::FixedwingLandDetector::_set_hysteresis_factor()
  -2.9%      -4  -2.9%      -4    land_detector::LandDetector
  -0.6%      -4  -0.6%      -4    land_detector::LandDetector::LandDetector()
  -0.1%      -8  -0.1%      -8    px4::parameters
  -2.7%     -12  -2.7%     -12    land_detector::MulticopterLandDetector::MulticopterLandDetector()
 -15.0%     -12 -15.0%     -12    land_detector::MulticopterLandDetector::updateParamsImpl()
  [DEL]     -16  [DEL]     -16    land_detector::RoverLandDetector::_set_hysteresis_factor()
  -6.4%     -56  -6.4%     -56    land_detector::LandDetector::Run()
 -102.5%     -64 -102.5%     -64    [18 Others]
  -0.0%     -76  -0.0%     -76    g_cromfs_image
 -26.4%    -116 -26.4%    -116    land_detector::LandDetector::task_spawn()
  [DEL]    -172  [DEL]    -172    land_detector::MulticopterLandDetector::_set_hysteresis_factor()
-0.0% -7.10Ki  -0.0%    -328    TOTAL

px4_fmu-v6x [Total VM Diff: -272 byte (-0.01 %)]

    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%     +19  [ = ]       0    .debug_abbrev
-0.0%     -24  [ = ]       0    .debug_aranges
-0.0%     -72  [ = ]       0    .debug_frame
-0.0% -4.17Ki  [ = ]       0    .debug_info
-0.0%    -415  [ = ]       0    .debug_line
  +200%      +4  [ = ]       0    [Unmapped]
  -0.0%    -419  [ = ]       0    [section .debug_line]
-0.0%    -340  [ = ]       0    .debug_loclists
-0.0%     -55  [ = ]       0    .debug_rnglists
-0.0% -1.58Ki  [ = ]       0    .debug_str
-0.0%    -184  [ = ]       0    .strtab
  [DEL]     -66  [ = ]       0    land_detector::AirshipLandDetector::_set_hysteresis_factor()
  [DEL]     -68  [ = ]       0    land_detector::FixedwingLandDetector::_set_hysteresis_factor()
  [DEL]     -70  [ = ]       0    land_detector::MulticopterLandDetector::_set_hysteresis_factor()
  [NEW]     +84  [ = ]       0    land_detector::RoverLandDetector::RoverLandDetector()
  [DEL]     -64  [ = ]       0    land_detector::RoverLandDetector::_set_hysteresis_factor()
-0.0%     -80  [ = ]       0    .symtab
  [DEL]     -32  [ = ]       0    land_detector::AirshipLandDetector::_set_hysteresis_factor()
  [DEL]     -16  [ = ]       0    land_detector::FixedwingLandDetector::_set_hysteresis_factor()
 -25.0%     -16  [ = ]       0    land_detector::MulticopterLandDetector::_get_ground_contact_state()
  [DEL]     -48  [ = ]       0    land_detector::MulticopterLandDetector::_set_hysteresis_factor()
   +33%     +16  [ = ]       0    land_detector::MulticopterLandDetector::_update_topics()
  [NEW]     +64  [ = ]       0    land_detector::RoverLandDetector::RoverLandDetector()
   +25%     +16  [ = ]       0    land_detector::RoverLandDetector::_get_landed_state()
  [DEL]     -48  [ = ]       0    land_detector::RoverLandDetector::_set_hysteresis_factor()
 -33.3%     -16  [ = ]       0    land_detector::RoverLandDetector::updateParamsImpl()
+6.3%    +272  [ = ]       0    [Unmapped]
-0.0%    -272  -0.0%    -272    .text
  [NEW]    +128  [NEW]    +128    land_detector::RoverLandDetector::RoverLandDetector()
   +89%    +100   +89%    +100    land_detector::MulticopterLandDetector::_update_topics()
  +4.8%      +4  +4.8%      +4    FlightTask
 -101.3%      -4 -101.3%      -4    [3 Others]
  -2.9%      -4  -2.9%      -4    land_detector::AirshipLandDetector
  -2.9%      -4  -2.9%      -4    land_detector::FixedwingLandDetector
  [DEL]      -4  [DEL]      -4    land_detector::FixedwingLandDetector::_set_hysteresis_factor()
  -2.9%      -4  -2.9%      -4    land_detector::LandDetector
  -0.6%      -4  -0.6%      -4    land_detector::LandDetector::LandDetector()
  -2.9%      -4  -2.9%      -4    land_detector::MulticopterLandDetector
  -2.9%      -4  -2.9%      -4    land_detector::RoverLandDetector
  -2.9%      -4  -2.9%      -4    land_detector::VtolLandDetector
  -0.0%      -8  -0.0%      -8    [section .text]
  -0.1%      -8  -0.1%      -8    px4::parameters
  -2.7%     -12  -2.7%     -12    land_detector::MulticopterLandDetector::MulticopterLandDetector()
 -15.0%     -12 -15.0%     -12    land_detector::MulticopterLandDetector::updateParamsImpl()
  [DEL]     -16  [DEL]     -16    land_detector::RoverLandDetector::_set_hysteresis_factor()
  -6.4%     -56  -6.4%     -56    land_detector::LandDetector::Run()
  -0.0%     -68  -0.0%     -68    g_cromfs_image
 -26.4%    -116 -26.4%    -116    land_detector::LandDetector::task_spawn()
  [DEL]    -172  [DEL]    -172    land_detector::MulticopterLandDetector::_set_hysteresis_factor()
-0.0% -6.87Ki  -0.0%    -272    TOTAL

Updated: 2025-10-14T05:45:07

MaEtUgR requested a review from sfuhrer July 18, 2025 12:44

MaEtUgR self-assigned this Jul 18, 2025

MaEtUgR added the Admin: Cleanup (housekeeping) 🧹 label Jul 18, 2025

github-actions bot added the Documentation 📑 label Jul 18, 2025

MaEtUgR force-pushed the maetugr/remove-land-trigger-time-parameter branch from a4aeea7 to bdec183 Compare July 18, 2025 12:47

dakejahl previously approved these changes Jul 18, 2025

View reviewed changes

hamishwillee dismissed dakejahl’s stale review via 0a18d8e July 23, 2025 06:34

github-actions bot added the stale label Aug 23, 2025

mrpollo removed Admin: Cleanup (housekeeping) 🧹 labels Sep 12, 2025

dakejahl previously approved these changes Oct 14, 2025

View reviewed changes

dakejahl dismissed their stale review via 46590f0 October 14, 2025 05:24

dakejahl self-assigned this Oct 14, 2025

github-actions bot added the Documentation 📑 label Oct 14, 2025

dakejahl previously approved these changes Oct 14, 2025

View reviewed changes

MaEtUgR and others added 5 commits October 13, 2025 21:35

LandDetector: remove unused LNDMC_TRIG_TIME parameter

1406a71

LandDetector: remove unnecessarily complicated global set_hysteresis_…

33d99e7

…factor() function

Apply suggestions from code review

a436cb9

Co-authored-by: Jacob Dahl <37091262+dakejahl@users.noreply.github.com>

Apply suggestions from code review

a54fbbc

remove param

6da58e8

dakejahl dismissed their stale review via 6da58e8 October 14, 2025 05:37

dakejahl force-pushed the maetugr/remove-land-trigger-time-parameter branch from 46590f0 to 6da58e8 Compare October 14, 2025 05:37

dakejahl approved these changes Oct 14, 2025

View reviewed changes

dakejahl enabled auto-merge (squash) October 14, 2025 05:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Land detector: Remove LNDMC_TRIG_TIME parameter #25251

Land detector: Remove LNDMC_TRIG_TIME parameter #25251

MaEtUgR commented Jul 18, 2025

Uh oh!

Uh oh!

Uh oh!

dakejahl Jul 18, 2025

Uh oh!

hamishwillee Jul 23, 2025

Uh oh!

dakejahl Jul 18, 2025

Uh oh!

dakejahl commented Jul 18, 2025

Uh oh!

github-actions bot commented Oct 14, 2025

Uh oh!

github-actions bot commented Oct 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	*body frame along x and y which helps to detect maybe_landed. The criteria for maybe_landed needs to be true for 1/3 seconds.
	*body frame along x and y which helps to detect maybe_landed. The criteria for maybe_landed needs to be true for 300ms.

Land detector: Remove LNDMC_TRIG_TIME parameter #25251

Are you sure you want to change the base?

Land detector: Remove LNDMC_TRIG_TIME parameter #25251

Conversation

MaEtUgR commented Jul 18, 2025

Solved Problem

Changelog Entry

Test coverage

Uh oh!

Uh oh!

Uh oh!

dakejahl Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

hamishwillee Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

dakejahl Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

dakejahl commented Jul 18, 2025

Uh oh!

github-actions bot commented Oct 14, 2025

Uh oh!

github-actions bot commented Oct 14, 2025

🔎 FLASH Analysis

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants