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

Fix Altitude Limitation #11211

Merged
merged 1 commit into from
Feb 25, 2019
Merged

Fix Altitude Limitation #11211

merged 1 commit into from
Feb 25, 2019

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Jan 14, 2019

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:

  • We're allowed to reach a maximum altitude of home + parameter
  • If we ever exceed this altitude
    • make sure we cannot get further up
    • only allow downwards speed

This results in "bouncing off" the limitation.

Test data / coverage
SITL testing

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Jan 26, 2019

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.

@MaEtUgR MaEtUgR force-pushed the fix-altitude-limitation branch 2 times, most recently from f29f78b to cb452e0 Compare February 8, 2019 13:24
@MaEtUgR
Copy link
Member Author

MaEtUgR commented Feb 8, 2019

And rebased in a second step such that you can see the changes under the first for-pushed link.

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.
@MaEtUgR
Copy link
Member Author

MaEtUgR commented Feb 21, 2019

@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.

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Feb 22, 2019

Good to go, now that thenames and comments are clear?

@julianoes
Copy link
Contributor

@dagar does this need test flying by the flight testing team (@Tony3dr) or is that good to be merged?

@MaEtUgR MaEtUgR merged commit db0283e into master Feb 25, 2019
@MaEtUgR MaEtUgR deleted the fix-altitude-limitation branch February 25, 2019 17:53
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.

None yet

2 participants