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

landdetector groundcontact: adjust climbrate if landing speed is low #7831

Merged
merged 1 commit into from Aug 21, 2017

Conversation

Stifael
Copy link
Contributor

@Stifael Stifael commented Aug 21, 2017

The threshold for no vertical movement is a constant value of 0.5. If the parameter landspeed is set low (for instance below 0.5), then the landdetector will always detect no vertical movement. This PR is a protection for having low landspeed.

Fixes #7811

@LorenzMeier
Copy link
Member

@PX4TestFlights Please cross test by flying very low and very slow with low land speed in position control mode.


// Check if we are moving vertically - this might see a spike after arming due to
// throttle-up vibration. If accelerating fast the throttle thresholds will still give
// an accurate in-air indication.
bool verticalMovement = fabsf(_vehicleLocalPosition.vz) > _params.maxClimbRate * landThresholdFactor;
bool verticalMovement;
Copy link
Member

Choose a reason for hiding this comment

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

I would initialize this to true, just for any future errors in the code.

if (hrt_elapsed_time(&_landed_time) < LAND_DETECTOR_LAND_PHASE_TIME_US) {
landThresholdFactor = 2.5f;
}
// land speed threshold
Copy link
Member

Choose a reason for hiding this comment

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

this comment says nothing, a descripion even if only with keywords would help more then duplicating the title.

// Adjust maxClimbRate if land_speed is lower than 2x maxClimbrate
float maxClimbRate = ((land_speed_threshold * 0.5f) < _params.maxClimbRate) ? (0.5f * land_speed_threshold) :
_params.maxClimbRate;
verticalMovement = fabsf(_vehicleLocalPosition.vz) > maxClimbRate;
Copy link
Member

Choose a reason for hiding this comment

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

If I calculate this through with a very low land speed I get vz will be compared to 0.045 m/s.
Was that the intention? Seems like a too low value to me...

Copy link
Contributor Author

@Stifael Stifael Aug 21, 2017

Choose a reason for hiding this comment

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

what is a very low land speed?
yes, it is intended. the land speed should be limited to the ekf resolution (ekf can have an error of +-0.3m/s). If you go below such a land speed, then you cannot expect that landdection works. therefore it is better to have no landed at all (as opposed to false positive).

@Stifael
Copy link
Contributor Author

Stifael commented Aug 21, 2017

@bkueng, can you please retest with the Intel Aero?

Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

I tested this branch on the aero. It does not have any thrust dropouts anymore while flying downwards at higher or lower altitudes and higher or lower stick inputs. Landing detection still worked consistently the two times I flew. Therefore it fixes #7811

@LorenzMeier LorenzMeier merged commit e39b38b into master Aug 21, 2017
@LorenzMeier LorenzMeier deleted the pr-MClanddetector_catch_low_speed branch August 21, 2017 18:38
MaEtUgR added a commit that referenced this pull request Oct 24, 2022
… speed threshold

It's very important that the in descend detection is always
at a strictly higher velocity than the vertical movement check.
This combination is basically used to check for vertical downwards
velocity tracking. Desired descend, no movement -> ground

If in descend threshold is lower than vertical movement it is
by definition even with perfect tracking the case that with
any velocity between the two thresholds there is no movement
even though a descend is commanded.

See first fix of this problem #7831
e39b38b
MaEtUgR added a commit that referenced this pull request Oct 27, 2022
… speed threshold

It's very important that the in descend detection is always
at a strictly higher velocity than the vertical movement check.
This combination is basically used to check for vertical downwards
velocity tracking. Desired descend, no movement -> ground

If in descend threshold is lower than vertical movement it is
by definition even with perfect tracking the case that with
any velocity between the two thresholds there is no movement
even though a descend is commanded.

See first fix of this problem #7831
e39b38b
MaEtUgR added a commit that referenced this pull request Oct 31, 2022
… speed threshold

It's very important that the in descend detection is always
at a strictly higher velocity than the vertical movement check.
This combination is basically used to check for vertical downwards
velocity tracking. Desired descend, no movement -> ground

If in descend threshold is lower than vertical movement it is
by definition even with perfect tracking the case that with
any velocity between the two thresholds there is no movement
even though a descend is commanded.

See first fix of this problem #7831
e39b38b
sfuhrer pushed a commit that referenced this pull request Nov 1, 2022
… speed threshold

It's very important that the in descend detection is always
at a strictly higher velocity than the vertical movement check.
This combination is basically used to check for vertical downwards
velocity tracking. Desired descend, no movement -> ground

If in descend threshold is lower than vertical movement it is
by definition even with perfect tracking the case that with
any velocity between the two thresholds there is no movement
even though a descend is commanded.

See first fix of this problem #7831
e39b38b
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.

None yet

4 participants