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

Takeoff: check if the desired velocity is finite when generating the takeoff velocity ramp #13973

Closed
wants to merge 1 commit into from

Conversation

dusan19
Copy link
Contributor

@dusan19 dusan19 commented Jan 17, 2020

Describe problem solved by this pull request
Drone takes off, executes an auto mission, lands, but never detects landing, instead keeps high thrust indefinitely.

When the mission starts (with a command) the flight task doesn't update for one iteration (#13971 #13668). If this happens while the takeoff state machine is in state rampup, the controller constraints remain NAN as initialized and Takeoff::updateRamp would set _takeoff_ramp_vz to NAN. With _takeoff_ramp_vz being NAN, the takeoff state machine can not leave the rampup state which leads to limit_thrust_during_landing never being called and thrust never set to 0, even though ground contact is true. Maybe landed requires low thrust, so landed is never detected.

Screenshot from 2020-01-17 20-05-06

Describe your solution
Proposed solution is to check whether the constraint that is used in ramp computation is NAN. While it is NAN the ramp is not updated.

@MaEtUgR
Copy link
Member

MaEtUgR commented Mar 7, 2020

Ouch, that's a good find! Thanks a lot for the detailed problem description and fix. 👍 I was just thinking about the case where the limit parameter takeoff_desired_vz stays NAN indefinitely then it would skip the ramp but stay in rampup indefinitely? I have to check that case and otherwise we can merge.

@@ -115,6 +116,10 @@ float Takeoff::updateRamp(const float dt, const float takeoff_desired_vz)
}

if (_takeoff_state == TakeoffState::rampup) {
if (!PX4_ISFINITE(takeoff_desired_vz)) {
return _takeoff_ramp_vz;
Copy link
Member

Choose a reason for hiding this comment

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

I think the ramp should continue to the maximum speed constraint to avoid getting stuck in case takeoff_desired_vz stays always NaN. I can make a suggestion.

Copy link
Member

Choose a reason for hiding this comment

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

Here's the suggestion: #14339

MaEtUgR added a commit that referenced this pull request Mar 10, 2020
MaEtUgR added a commit that referenced this pull request Mar 17, 2020
bresch pushed a commit that referenced this pull request Mar 17, 2020
@MaEtUgR
Copy link
Member

MaEtUgR commented Mar 17, 2020

Hey @dusan19 I hope it's ok if I close this pr in favor of #14339 as we discussed. Thanks a lot for spotting the bug and all your help!

@MaEtUgR MaEtUgR closed this Mar 17, 2020
bkueng pushed a commit that referenced this pull request May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants