Land detector: Remove LNDMC_TRIG_TIME parameter #25251
Open
+31
−63
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
Documentation 📑
…factor() function
MaEtUgR
force-pushed
the
maetugr/remove-land-trigger-time-parameter
branch
from
a4aeea7 to
bdec183
Compare
dakejahl
previously approved these changes
Jul 18, 2025
docs/assets/airframes/multicopter/amovlab_f410/amovlabf410_drone_v1.15.4.params
Outdated
Show resolved
Hide resolved
| @@ -44,7 +44,7 @@ | |||
| *State 2 (=maybe_landed): | |||
| *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. | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Plus it is actually 300_ms below.
| hrt_abstime land_detection_time = 1_s; | ||
|
|
||
| if (_dist_bottom_is_observable && !_vehicle_local_position.dist_bottom_valid) { | ||
| land_detection_time = 3_s; |
There was a problem hiding this comment.
I wonder if this logic is really needed or even useful
|
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 🐱 |
Co-authored-by: Jacob Dahl <[email protected]>
|
No flaws found |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Solved Problem
When examining the land detector, I found that:
_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
Test coverage
No testing performed, this is a refactor aside from removing the parameter.