-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
Fix Altitude Limitation #11211
Fix Altitude Limitation #11211
Conversation
Are there any disagreements on this fix? @Stifael mentioned that @priseborough would prefer a perfectly continuous implementation which would be possible but the current implementation is not even working properly and can get your vehicle trapped at the maximum altitude so we really need this fix in my opinion. |
f29f78b
to
cb452e0
Compare
And rebased in a second step such that you can see the changes under the first for-pushed link. |
cb452e0
to
583116a
Compare
Before if you were above the maximum altitude you could not command to go down anymore until the position controller had overshoot to under the maximum altitude again.
583116a
to
bbfc4ef
Compare
@julianoes So rebased (1st force-push) and improved naming and comments to be hopefully very clear now (2nd force-push). My SITL test still works perfectly fine. |
Good to go, now that thenames and comments are clear? |
Describe problem solved by the proposed pull request
Before if you were above the maximum altitude you could command to go down anymore until the position controller had control overshoot and the vehicle slowly got below the maximum altitude again.
Describe your preferred solution
I tried to simplify the logic:
This results in "bouncing off" the limitation.
Test data / coverage
SITL testing