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

Limit horizontal velocity during landing #13561

Merged
merged 5 commits into from
Dec 11, 2019
Merged

Limit horizontal velocity during landing #13561

merged 5 commits into from
Dec 11, 2019

Conversation

bresch
Copy link
Member

@bresch bresch commented Nov 22, 2019

Usually, a user doesn't need and want to fly at high speed close to ground. We currently already reduce the vertical speed between MPC_LAND_ALT{1/2} so I added the same limitation for the horizontal speed.
This also allows more precise control during landing as the scaling on the sticks is reduced.

Additional changes:

  • At the same time I centralized the estimation of _dist_to_ground in the base FlightTask such that the ManualPosition, AutoMapper(2) and AutoLine can use the same logic without having to recompute it locally.
  • I also re-introduced the velocity ramp between alt1 and alt2 that was not implemented in AutoMapper2.

@bresch bresch added this to the Release v1.11.0 milestone Nov 22, 2019
@bresch bresch requested a review from MaEtUgR November 22, 2019 10:06
@bresch bresch self-assigned this Nov 22, 2019
@bresch bresch added this to In progress in Multicopter via automation Nov 22, 2019
@MaEtUgR
Copy link
Member

MaEtUgR commented Dec 3, 2019

I allowed myself to rebase the pr and apply some nit-pick suggestions. What is missing for me is a way to disable the functionality. It works well and the implementation is good but there will for sure be someone who wants the legacy behavior because of a missing distance sensor or special use case.

@MaEtUgR
Copy link
Member

MaEtUgR commented Dec 3, 2019

Added a comment in my additional commit on how to disable.

Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice implementation. Rebased, reviewed, SITL tested. From my side ready.

Multicopter automation moved this from In progress to Reviewer approved Dec 3, 2019
@bresch
Copy link
Member Author

bresch commented Dec 4, 2019

Thanks @MaEtUgR .
@PX4/testflights can you please test this PR and verify that when you are in position control mode, the speed is limited when close to the ground? Thanks

@Tony3dr Tony3dr added this to Ready for testing in Test Flights Dec 5, 2019
@dannyfpv
Copy link

dannyfpv commented Dec 6, 2019

Tested on Pixhawk 4 v5 f-450
Position mode: no issues

log: https://review.px4.io/plot_app?log=359b3c8c-ccea-4949-a389-ef3a4065f354

PixRacer v4 f-450
position mode: no issues

log: https://review.px4.io/plot_app?log=4b200565-eb8f-49d5-8f04-b88d46987463

Observations:
We noticed when closest to the ground the speed would reduce or be limited when the altitude increased the speed increased. Regardless of the altitude, the vehicle seemed at a lower speed than normal when not close to the ground.

@Junkim3DR
Copy link

Junkim3DR commented Dec 9, 2019

Tested on NXP FMUK66 v3

Mode Tested: Position Mode.

Procedure: Flight in Position mode and notice ground velocity change proporcional to the altitude of the vehicle.

Notes: No issue.

Log: https://review.px4.io/plot_app?log=b0be1815-e64c-4a6a-808f-b8adce9486db

Tested on Pixhawk Pro v4

Mode Tested: Position Mode.

Procedure: Flight in Position mode and notice ground velocity change proporcional to the altitude of the vehicle.

Notes: No issue.

Log: https://review.px4.io/plot_app?log=adab6003-a537-43c3-9c2e-4b31d6003962

Observations:
We noticed when closest to the ground the speed would reduce or be limited when the altitude increased the speed increased. Regardless of the altitude, the vehicle seemed at a lower speed than normal when not close to the ground.

@MaEtUgR
Copy link
Member

MaEtUgR commented Dec 11, 2019

I also did some real-world flight testing and if the altitudes MPC_LAND_ALT1, MPC_LAND_ALT2 are not super close like less than a meter to each other it works super nice. If they are too close the maximum allowed speed changes very quickly but that's a question of configuration. I consider this a good feature since especially with a downward-facing distance sensor manual landing even when coming from flying fast forward gets so easy.

@MaEtUgR
Copy link
Member

MaEtUgR commented Dec 11, 2019

I just confirmed an issue: If you go full stick while flying up the allowed velocity gets faster and faster but you acceleration is much lower than when you start flying on the same altitude even after you are long over MPC_LAND_ALT1
slowdown

Full stick while crossing the altitudes looks like exactly 0.5m/s^2 while when starting in high altitude it looks like exactly 3m/s^2.

@MaEtUgR MaEtUgR merged commit a3d30fc into master Dec 11, 2019
Test Flights automation moved this from Ready for testing to Done Dec 11, 2019
Multicopter automation moved this from Reviewer approved to Done Dec 11, 2019
@MaEtUgR MaEtUgR deleted the dev-xy-slow-land branch December 11, 2019 13:03
@MaEtUgR
Copy link
Member

MaEtUgR commented Dec 11, 2019

We found that it's not this pr creating the behavior that is shown above but the problem already existed before and it was only found by thoroughly testing this pr. @bresch will create another pr to get rid of this legacy logic. See https://github.com/PX4/Firmware/blob/a3d30fc970262a589ed2ac94c0c5bf18343162c0/src/lib/flight_tasks/tasks/ManualPosition/FlightTaskManualPosition.cpp#L104-L114

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Multicopter
  
Done
Test Flights
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants