FlightTasks: do not adjust tilt limit of the position control #14702
Conversation
This file contains 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
Adjusting the tilt limit can lead to diverging position control and should only be used by setting a sanity limit for the controller and not to adjust during the descent phase of a Land or RTL. Otherwise it leads to flyaways in important failsafe modes when there's stronger disturbance e.g. wind.
bresch
approved these changes
Apr 20, 2020
|
When it does get approved, why are we not merging it? |
|
I assumed we'd need some real-world tests but everything I read from the problematic logs so far speaks for removing the limit. Making it part of the acceptance test for the release is probably more efficient. |
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.
Describe problem solved by this pull request
Adjusting the tilt limit can lead to diverging position control and should only be used by setting a sanity limit for the controller and not to adjust during the descent phase of a Land or RTL. Otherwise it leads to flyaways in important failsafe modes when there's stronger disturbance e.g. wind.
Describe your solution
I removed all occurances where flight tasks sets a tilt limit. All of them except for the one in AutoMapper's
_prepareLandSetpoints()were setting the maximum redundantly. The auto land point one was causing the position control divergence on RTL and Land descend.Describe possible alternatives
As discussed with @RomanBapst the goal landing strategy is to relax xy position control and ramp down the thrust gradually on ground contact as a next step of improvement.
Test data / coverage
I did quite some tests in SITL using different wind strengths made possible by @Jaeyoung-Lim 's efforts like PX4/PX4-SITL_gazebo-classic#461
It has to be said that before #12324 this tilt limit was already missing for a long time and hence we already know it's not breaking the user experience.
Additional context
fixes #13956