-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
FlightTaskManualAltitude: Follow altitude limit given by the estimator #13958
Conversation
_updateConstraintsFromEstimator(); | ||
|
||
_max_speed_up = _constraints.speed_up; | ||
_min_speed_down = _constraints.speed_down; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_min_speed_down = _constraints.speed_down; | |
_max_speed_down = _constraints.speed_down; |
The name confuses me since if I read min_speed_down I'm assuming you have to fly a certain minimum speed to descend... well, it wasn't your decision.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally agree, I made a 2nd commit to rename that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MaEtUgR Can you approve again please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks correct to me, seems to work looking at the plot.
…s it is the maximum allowed downward speed
@PX4/testflights Can you fly that please?
Thanks in advance. |
Ping @PX4/testflights |
@bresch, unfortunately, we don't have a setup with optical flow or range finder. |
Codecov Report
@@ Coverage Diff @@
## master #13958 +/- ##
==========================================
+ Coverage 35.97% 36.14% +0.17%
==========================================
Files 574 545 -29
Lines 48714 46887 -1827
==========================================
- Hits 17523 16947 -576
+ Misses 31191 29940 -1251
|
This had enough testing on my side |
This fixes #13350 |
Describe problem solved by this pull request
The altitude agl is limited by the sonar range when taking off without GPS aiding, even if it is gained in air because the constraint is only updated on activate.
Describe your solution
Continuously track the constraint sent by the estimator
Describe possible alternatives
This function should be virtual and the horizontal limits updated in FlightTaskManualPosition
Test data / coverage

SITL tests with flow and intermittent GPS
https://logs.px4.io/plot_app?log=f60d771e-3008-4b44-8d96-2476e7c38c8b