Multicopter land detector - Add robustifying condition

[bresch]

Description

This shortcut comes from the fact that "When landed and that the thrust is low, the drone has to be on the ground"

This also fixes a bug in jMavSim where the takeoff delay parameter MPC_IDLE_TKO was ignored due to false takeoff detection.

False takeoff detection also occurs in real life when the baro and/or GPS wrongly drive the velocity estimate to a value above the detection threshold.

SITL tests

Manual position control
Before:

As soon as the vehicle is armed, takeoff is detected

After:

Even the large vz estimate error, takeoff is only detected when the throttle is high enough.

Mission
Before:

Note that the delay between idle (actuator outputs at 900ms [0.9 on the graph]) and takeoff is ignored. We don't even see the actuators going to 900ms.
After:

Note the delay of 3 seconds between idle and takeoff. The actuator output first goes to 0.9 (900ms) during those 3 seconds defined by MPC_IDLE_TKO.

[bresch]

As you are currently in the land detector with #11172 can you check this PR as well? Thanks

[dannyfpv]

tested on pixhawk4 mini v5
https://review.px4.io/plot_app?log=0e1c3ecf-5951-4cbc-9d7f-119f67e911a3

pixhawk v4 pro
https://review.px4.io/plot_app?log=1970a922-a286-4406-bbb5-37b7a8a67519

pixhawk race v4
https://review.px4.io/plot_app?log=4bc9a548-d0df-4889-af1f-a73328a28857

[MaEtUgR]

I totally see how this can make sense, still I'm constantly thinking about how it can be simplified or what is broken such that it's needed.
It's basically introducing different conditions to fall out of the landed state compared to switching into it. In detection you require the estimated speeds to be within thresholds to fall out only the thrust.
Do we generally need that? Can we structure the code simpler to make these cases clear?

I still don't really understand

[MaEtUgR]

Your thorough description of how you found out with MPC_IDLE_TKO lead me to do tests myself. I don't think it's false takeoff detection but rather false application of the hysteresis and hence a bug in the position controller.

I sometimes see the problem in jMAVSim and I'd even have a possible fix but I want to be able to test that it's fixed for sure but can't reproduce the problem all the time.

[MaEtUgR]

@bresch I think I fixed the root cause in #11206, can you please check?

[Stifael]

@bresch I like this change.
As a side comment: with

This shortcut comes from the fact that "When ground has been detected and that the thrust is low, the drone has to be on the ground"

I guess you meant "When landed was detected and the thrust is low, then the drone has to be on ground". That is also what you have implemented here

[bresch]

@MaEtUgR I moved the condition to _get_landed_state()

[bresch]

@MaEtUgR Can we merge that? @RomanBapst Tested it and it seems to reject bad velocity estimates that would trigger a takeoff detection.

[LorenzMeier]

Can you please rebase so that CI is re-run?

[MaEtUgR]

I can accept it because it's not wrong. I'm only hesitant because it could cover up other fundamental issues that have more impact when we have to discover them in a different way.

For example:

• the issue you describe in your pr description is not caused by the land detector but the cause is solved in Fix idle delay race-condition #11206
• Multicopter takes off in HOLD on ground #11197

I think both might go unnoticed and would later cause other unexpected symptoms.

[MaEtUgR]

There's a new example: #11331 (comment)
It looks like he manages to get all the way into landed state and if you then stay there until there's a thrust command which is only given under certain takeoff conditions because it's in landed state there's a very good chance the drone would fall from the sky...

[bresch]

In those cases, freefall detection (that works really well) saves you from falling like a stone. Remember that the freefall detection has the highest priority.

[MaEtUgR]

We can try.

• As I mentioned it's not wrong and has the big advantage that there's no way the vehicle kicks out of the landed state while no takeoff action happens e.g. when someone knocks the drone over while it's armed on the ground and estimates overshoot it will not suddenly try to go into hover.

• We should just be aware of a certain risk because falling out of a false positive landed state in air is not given anymore.
• Also it might cover some existing or upcoming problems with the takeoff.

[MaEtUgR]

@bresch Freefall failed on me once before back in #6550.
The drone fell 15 meters until I finally reacted by manually pushing the throttle up again just before it hit the ground. It might work today though... did you test? 😉

[dagar]

@MaEtUgR @bresch what would you like to do here?

[MaEtUgR]

The problem @bresch described in the description was solved by #11206. I see it like the case this pr helps is when you arm the drone, want to let it sit there in idle and it takes off prematurely because it gets moved hard enough. But the logic is this way to prevent a crash when false detecting a landing in air. So I'm concerned about a case the drone detects landing to the last stage in air and doesn't get out anymore because the position controller is holding the thrust low thinking it's landed.

[bresch]

I will close this PR since it's not really needed yet. If we discover that we have some false positive takeoff detections, we can still reopen it or discuss about an other solution.