mc_pos_control: handle landed and takeoff setpoint limiting the same way

[MaEtUgR]

Describe problem solved by this pull request
@jkflying reported an effect I've seen before:
During a normal landing procedure the thrust gets first cut by the reaction to ground detected and then cut by the takeoff logic because the vehicle is back ready for another takeoff.

The problem is that the thrust and attitude limit while landing is in the loop order after the position controller ran and directly operates on the attitude setpoint skipping the position controllers minimum thrust limit. The takeoff thrust limitation on the other had is before the position controller and amends the input to it.

Describe your solution
I'm unifying the thrust and attitude limit logic to be in one place where it already was for the takeoff. Both logics for takeoff and landing do these things in common:

• cut thrust
• set roll pitch level
• set yawspeed to zero and yaw setpoint to current yaw
• reset position control integral

I still have to check if reactivating flight task and reseting the hover thrust is also desired otherwise they can be conditional for just the takeoff.

Describe possible alternatives
It's not 100% clear to me if MPC_THR_MIN should be the minimum thrust at all times even during landing and takeoff or if the takeoff ramp should always start at zero thrust and thrust should go down to zero after landing.

Test data / coverage
Quick SITL test, needs more testing and review.

[LorenzMeier]

On this question:

It's not 100% clear to me if MPC_THR_MIN should be the minimum thrust at all times even during landing and takeoff or if the takeoff ramp should always start at zero thrust and thrust should go down to zero after landing.

From a UX perspective ramping from and to 0 would be preferable, because the minimum throttle is the throttle that enables the system to be safely descending, while in these situations a smooth spool-up and spool-down of the rotors creates trust of the user.

Otherwise every flight starts with a scary "twitch" because the props go from 0 to idle throttle in a short amount of time. Same for landing: Running smoothly to zero is nicer compared to holding throttle and just switching off.

[MaEtUgR]

Otherwise every flight starts with a scary "twitch" because the props go from 0 to idle throttle in a short amount of time. Same for landing: Running smoothly to zero is nicer compared to holding throttle and just switching off.

Exactly what I thought. Twitch at the beginning and less useful land detection at the end because it doesn't go down to zero. I'm working on a way to have that possible.

[MaEtUgR]

because the minimum throttle is the throttle that enables the system to be safely descending

It cannot be considered a guarantee for safe descending because of the minimum thrust resulting in a downwards acceleration. So it depends on how long it's applied if the vertical end speed is still safe. I see it more as the guard to keep rotational control authority when not using airmode in the mixer (which is disabled by default for safety reasons).

[LorenzMeier]

Sorry, that’s what I meant with “safe”.

[MaEtUgR]

I revisited the implementation and rebased on master. I also did quite extensive SITL testing.

It now:

• solves the original issue that the thrust went up again during landing
• starts the takeoff ramp at zero thrust
• removes the deprecated thrust setpoint takeoff logic for stabilized (because it was int the way)
• handles setpoint limiting for takeoff and landing the same way before the position controller

There is no thrust ramp down for ground contact yet, that's for another pr.

Ready for review and testing.

[bresch]

Nice

[jorge789]

Tested on PixRacer V4

Indoor Flight Test
Modes Tested
Stabilized Mode: Good.

Procedure
Arm and Take off in stabilized mode.

Notes
No issues noted, good flight in general.

Log https://review.px4.io/plot_app?log=28108a2a-8640-4416-a817-cd7bdd94087f

Tested on CUAV nano V5

Indoor Flight Test
Modes Tested
Stabilized Mode: Good.

Procedure
Arm and Take off in stabilized mode.

Notes
No issues noted, good flight in general.

Log https://review.px4.io/plot_app?log=b8e12d7a-3476-462a-946f-d9c26a9b8c37

[Junkim3DR]

Tested on NXP FMUK66 v3

Indoor Flight
Mode tested

• Stabilized

Procedure

• Takeoff and landing in Stabilized

Note

• No issue noted.

Log
https://review.px4.io/plot_app?log=5cd75156-b675-4399-acdf-d4e6c2e22c09

[MaEtUgR]

@PX4/testflights With this one it's most important to test takeoffs and landings in Altitude, Position and Auto Takeoff/Landing modes. Thanks in advance!

[Tony3dr]

@PX4/testflights With this one it's most important to test takeoffs and landings in Altitude, Position and Auto Takeoff/Landing modes. Thanks in advance!

@MaEtUgR We will test this on the field once the rain stops. Monday the rain should clear.

[MaEtUgR]

I rebased on master after the conflicts with #14378 and #14339.

[bresch]

Still GTM

[MaEtUgR]

I'm quoting the test report by @fury1895:

I did several takeoffs and landings with F450 in altitude, position, and auto flight modes. No issues showed up. Looks like small changes but the improvement in UX is great!

Thanks for helping with testing!