MCAttitudeController: remove reset of yaw_sp when landed

[sfuhrer]

Solved Problem

No manual yaw control when landed. This e.g. makes preflight checks for yaw actuators impossible in any attitude-controlled mode (for hovering vehicles with yaw control not through differential thrust like tiltrotors or tailsitters). It's also quite confusing to have actuation of roll/pitch but not yaw.

Solution

Remove _landed check for _reset_yaw_sp.

Note that the integrals still get reset independently in the rate controller (for all axes).

I wonder if we the other two conditions (attitude_setpoint_generated and vtol_in_transition_mode) are really required as well. Why do you only want to reset yaw if (!attitude_setpoint_generated), and is the yaw reset during VTOL transition not only required for tailsitters, and there handled in the tailsitter.cpp (as the comment hints as well)?

Test coverage

Minimal SITL testing, would need some flight testing as well.

Context

Was also reported on discord: https://discord.com/channels/1022170275984457759/1063239941825503324

[dagar]

I think you're right about the checks/testing, but I'd worry about potential yawing right at takeoff and other side effects depending on heading estimation and mag issues.

Maybe there's a line in between where we could respect the input while it's active (eg non-zero yaw stick), but otherwise keep the yaw setpoint tracking the estimate.

[sfuhrer]

Maybe there's a line in between where we could respect the input while it's active (eg non-zero yaw stick), but otherwise keep the yaw setpoint tracking the estimate.

Could make sense right. Though if you have heading estimation issues then they will now just show right when landed=false, which I think is usually very early in a takeoff sequence.

[dagar]

It's not great either way, we also probably need to factor in heading_good_for_control here as well.

What if we only did the yaw speed FF (straight from your yaw stick in manual/stabilized mode) out to the vehicle_rate_setpoint?

[PrometheusTransport]

If the issue is windup when on the ground, could we simply reset on the transition?

It seems like this is a different use case than say ignoring yaw during VTOL transition.

[MaEtUgR]

I'm completely with you on this issue. It's annoying for any check of yaw actuation that does not spin up a motor but uses the multicopter attitude controller e.g. tricopeter yaw servo, helicopter tail servo, tailsitter surface servos.

The issue with just removing the landed check is that it then updates the absolute heading setpoint all the time and upon arming or whenever ground friction allows control control the vehicle to yaw with all effort which imposes a high risk.

I think to solve this we should keep the landed or better even heading_good_for_control check but only for the absolute heading setpoint and still generate a yaw rate setpoint. Currently _reset_yaw_sp does not just reset the absolute yaw but also suppress any yaw rate setpoint generation. I'm looking into a good way to achieve that solution and test it in simulation.

[MaEtUgR]

I pushed my suggestion and tested in SITL (only!) Any feedback, test on hardware welcome.
Note: The yaw rate setpoint now also goes through when having the throttle stick at zero but it doesn't build up an absolute yaw error.

[sfuhrer]

Thanks @MaEtUgR for the needed additions. Makes all sense to me. Let's get some testing in.

[sfuhrer]

Tested on a small quad, didn't notice anything weird.

What I tested:

• normal arming with yaw stick gesture --> nothing obvious
• slowly increase throttle on ground after arming with yaw stick fully and half deflected -->even with high min throttle my vehicle had too much ground friction and would never spin on the ground. But ofc, it now will try to follow the yawrate setpoint, while before it didn't as long as landed=true. As the rate controller though disables integral action while landed (unchanged with this PR) I see very little risk of this causing an issue in general.
  https://review.px4.io/plot_app?log=11bfae35-8dde-4e01-9584-eb24739b237b
• artificially introduce yawing disturbance (set the kms of one motor diagonal axis to different values than the other one), and removing yawrate_I and reduce yawrate_P
  https://review.px4.io/plot_app?log=a3f9090a-992e-48d0-90b4-ca2795907c01
  
• enabling yaw airmode and arm with switch, then slowly takeoff and/or move yawing stick while still on the ground

[MaEtUgR]

@sfuhrer Thanks so much for addressing this, looking through my suggestions and testing it out in detail 🙏

[dagar]

Avoiding the dependency on vehicle_local_position might have been nice, how about adding equivalent metadata to vehicle_attitude instead?

[MaEtUgR]

how about adding equivalent metadata to vehicle_attitude instead?

Would make sense yes. The flag needs to be in both then because the position controller doesn't have a dependency on vehicle_attitude.