-
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
Add terrain hold altitude control mode #9854
Conversation
@RomanBapst FYI |
* with terrain height variation. Requires a distance to ground sensor. The height controller will | ||
* revert to using height above origin if the distance to ground estimate becomes invalid as indicated | ||
* by the local_position.distance_bottom_valid message being false. | ||
* Set to 2 to control height relative to earth frame origin when stationary and relative to ground |
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.
@priseborough Shouldn't it be the other way around? Control height relative to earth frame when moving and relative to ground frame when stationary?
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.
will fix
@priseborough I will test this using a distance sensor but with no optical flow activated due to PX4/PX4-ECL#474 |
300e6e2
to
84c72b4
Compare
@priseborough I tested this outdoors and it seems to work fine. I forgot to upload the log but will do it tomorrow. |
I need to rebase this on the latest flight task PR |
@priseborough Here's a log from the flight I did. The performance was actually not really nice because the baro is very much affected by wind when moving horizontally. However, I verified that the functionality was as described. @Stifael @MaEtUgR This is the same log in which I saw the yaw jump at the very beginning. |
4b627ef
to
ac8e75d
Compare
@MaEtUgR I have added you as a reviewer given it is modifying your PR |
if (MPC_ALT_MODE.get() == 2) { | ||
// terrain hold | ||
float spd_xy = Vector2f(&_velocity(0)).length(); | ||
bool stick_input = Vector2f(&_velocity_setpoint(0)).length() > 0.5f * MPC_ALT_MODE_SPD.get(); |
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.
can we not use _sticks
instead (https://github.com/PX4/Firmware/pull/9731/files#diff-7dfb661344d42aeefa71f4afd9b85be8R60), which would make it scaling independent.
In addition, there is already a parameter for horizontal direction that defines the speed threshold at which the vehicle is considered at rest: https://github.com/PX4/Firmware/blob/master/src/modules/mc_pos_control/mc_pos_control_params.c#L375
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.
Good idea. Will fix.
if (PX4_ISFINITE(_constraints.max_distance_to_ground)) { | ||
_constraints.speed_up = math::gradual(_dist_to_bottom, min_distance_to_ground, _constraints.max_distance_to_ground, | ||
_max_speed_up, 0.0f); | ||
_constraints.speed_up = math::constrain(MPC_Z_P.get() * (_constraints.max_distance_to_ground - _dist_to_bottom), |
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.
I personally prefer to avoid core-position control parameters like MPC_Z_P. Why can we not just limit the speed upwards linearly from max velocity to 0 at maximum altitude?
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.
Because that is a de-facto position error gain than can either be too small/sluggish if the distance between min alt and max alt is large or too high resulting in overshoot if the distance is small. By using the gain we guarantee the fastest ascent possible without excessive overshoot and consistent behaviour regardless of height limit separation.
I don't know how to proceed. Should we wait with merge until |
If we don't merge this now, then it will rot given the churn in pr-flight-task-iteration. Working this feature into master requires a different implementation and I can do that as a separate PR if pr-flight-task-iteration is going to take too long. We can then include the patches required to revert that PR in pr-flight-task-iteration |
e3f92e9
to
da794e7
Compare
655d130
to
57f6d2d
Compare
@priseborough, I prefer to merge first pr-flight-task-iteration, then merge this PR and @mrivi PR for avoidance on top. Otherwise pr-flight-task-iteration never comes to an end |
32084b7
to
7917a1f
Compare
This new mode of altitude control uses terrain following when holding position and normal altitude control when moving.
…2 squashed commits) Squashed commits: [ed2a243] FlightTasks: Preserve control loop tuning when applying max altitude limit [b33b947] FlightTasks: Add terrain hold function This new mode of altitude control uses terrain following when holding position and normal altitude control when moving.
Use baro for height in EKF. Use terrain hold mode in height controller.
Use pre-existing MPC_HOLD_MAX_XY parameter for speed threshold. Use _stick_expo variable for stick movement check. Update documentation.
57f6d2d
to
638ce2d
Compare
Rebased on master now that pr-flight-task-iteration has been merged. |
This PR adds a height control mode that uses a ground reference when not moving horizontally and a local position datum reference when moving. An example application is when operating indoors using optical flow. The vehicle can be flown horizontally over obstacles without height increases that could cause it to to collide with overhead obstacles.
Relevant to #9793.
The upper altitude limit has been modified so that the height error gain is consistent with control loop tuning.
This has been tested in SITL using the following branch which contains modifications to simulate baro drift and print transitions: https://github.com/priseborough/Firmware/tree/altControlMode-wip
Test log here: https://logs.px4.io/plot_app?log=05d598f6-181d-421e-a686-164225065079
Note: The upper optical flow height limit is not applied when the vehicle is moving horizontally. This behaviour can be changed, however it will cause a terrain following behaviour under some conditions if we do so.
The reason for adding this altitude control mode is that although an equivalent behaviour can be achieved via use of the EKF2_RNG_AID parameter, this should be avoided if possible. EKF2_RNG_AID was originally developed for an application where the ground effect interference from rotor wash was so severe that it affected vertical velocity estimates and safe takeoffs and landings were not possible.
Range finders are accurate, but not reliable sensors (they can experience large random readings), the terrain under the vehicle cannot be relied on to be flat and although the EKF can handle slow drift and variation in height reference, anything more than that can cause estimation problems that go beyond the vertical position state.
For these reasons, unless baro errors are severe, we should be meeting the requirement for accurate ground relative height control in the position controller when possible.