Multicopter takes off in HOLD on ground

[julianoes]

I found a regression while running the Dronecode SDK integration tests. Before arming, the SDK sets PX4 into HOLD mode. That way we make sure that a drone does not shoot up right after being armed but always sits on ground and just spins the motors in idle, so this is a safety precaution.

However, after 5887a23 the drone suddenly starts hovering right after arming even though it's in HOLD mode.

The SDK command to test this is:

make && build/default/integration_tests/integration_tests_runner --gtest_filter="SitlTest.ActionHoverAsync"

FYI: @Stifael

https://logs.px4.io/plot_app?log=c387cdf8-120a-4aab-90ca-bcfb559a5ea5

[Stifael]

It seems that the issue is caused by an other bug, but the changes from 5887a23 makes it now visible.
For whatever reason, if the commander arms in Hold, then Flighttask sends out a thrust-setpoint of 0 instead of NAN.

[julianoes]

Ok, what do you suggest to fix it?

[Stifael]

No suggestion, except of finding the bug. I only spent a couple minutes on trying to find the bug, so I hope that on monday I will figure it out. If it is a time critical issue, then we could revert it

[julianoes]

Ok thanks. Maybe @MaEtUgR has an idea too.

[MaEtUgR]

@julianoes I found this issue as well when I did some SDK tests such that I learn how to eventually add orbit. Can you check if #11206 fixes it?
EDIT: Just checked, I think it doesn't.

[Stifael]

@MaEtUgR, as i wrote above, the reason is a thrust-setpoint of 0. If you are in hold, the thrust setpoint should be NAN, otherwise the logic breaks.

[Stifael]

By the way, if you are in hold during flight, the thrust setpoint is NAN as expected. This, however, is not the case if the drone is put into Hold mode before arming...

[MaEtUgR]

This, however, is not the case if the drone is put into Hold mode before arming...

Strange 🤔

I found the minimal revert hotfix: #11207

[Stifael]

Ok, I know now what module causes it.
The default for auto is now the jerk-limited Flighttask (https://github.com/PX4/Firmware/blob/master/src/modules/mc_pos_control/mc_pos_control_params.c#L681-L688). With this default, there is a thrust and velocity setpoint at the same time, which is not supported. Changing MPC_AUTO_MODE back to Default line tracking (which, by the way is not default anymore), the issue is gone.

@bresch, can you look into it. I think this bug justifies my concern of having two code paths for Auto (AutoMapper vs AutoMapper2) despite sharing similar or even the same functionalities.

[Stifael]

By the way, this kind of bug can be captured with the integration-test ulog pipeline.

[MaEtUgR]

@bresch It's because the zero thrust point of https://github.com/PX4/Firmware/blob/master/src/lib/FlightTasks/tasks/AutoMapper2/FlightTaskAutoMapper2.cpp#L120 gets through when you arm in hold mode. The same line also exists in AutoMapper(1) but for some reason it doesn't get executed in that situation. We should have only one "mapper" implementation if they are copies of each other with the same goal (#11209).

[Stifael]

@bresch @MaEtUgR in Automapper, the thrust will be set to NAN if the state changes from idle to any other state: https://github.com/PX4/Firmware/blob/master/src/lib/FlightTasks/tasks/AutoMapper2/FlightTaskAutoMapper2.cpp#L61-L62

[bresch]

@Stifael The issue was not in the AutoMapper but in how the AutoLineSmoothVel FlightTask handled the case where a position setpoint and a velocity setpoint is NAN. That was considered as an unknown command and the velocity setpoint was forced to zero.
https://github.com/PX4/Firmware/blob/ef54bff4ed45755b4762ef58db21fa37d612d44e/src/lib/FlightTasks/tasks/AutoLineSmoothVel/FlightTaskAutoLineSmoothVel.cpp#L178-L181
I made a quick fix to solve it and I'll open a PR soon.