Crash/tumble detection should not re-enable motors when flipped back

[fredg02]

Crash/tumble detection (#248 ) is really helpful and prevents damage, but I think it can be improved a little bit. It's still potentially dangerous to re-enable the motors when the copter is flipped back up. If setpoints are still sent (e.g. a flow sequence is still running), the motors can start spinning again violently. This can cause (minor) injuries or lead to another crash, when the copter is dropped/flies away.

It would be best to cancel the running sequence or not accept setpoints until some kind of "reset"-packet is sent. Since this would be rather hard to implement (a flow sequence is most likely sent by a client and can't be canceled on the copter side), I'd recommend to kill the connection. There might be use-cases where this is inconvenient, but it's the safest option IMHO.

[krichardsson]

Agree, some form of freeze mode that requires a reset to fly again.

[ntamas]

A less intrusive option to the current codebase would be to disable the high-level commander when a tumble is detected. This would not solve the problem when a client is continuously setting new setpoints, but it would solve the cases when the Crazyflie is flying a pre-programmed sequence autonomously with the high-level commander.

I think that a patch like this would essentially need to add a function to commander.c to disable the high-level commander, and the situation awareness module would need to call this function when a tumble is detected. Would you accept such a PR?

[krichardsson]

Pull requests are always welcome :-)
Another option could be to simply turn off the motors, regardless of using the HL commander or not.

[ntamas]

This seems to do the trick for me; if I understand correctly the thrust is already set to zero, which should turn off the motors, all that we need is to ensure that the HL commander does not turn them back on when the drone is picked up from the floor:

diff --git a/src/modules/src/sitaw.c b/src/modules/src/sitaw.c
index 87a9707..a3699a2 100644
--- a/src/modules/src/sitaw.c
+++ b/src/modules/src/sitaw.c
@@ -32,7 +32,7 @@
 #include "param.h"
 #include "trigger.h"
 #include "sitaw.h"
-#include "commander.h"
+#include "crtp_commander_high_level.h"

 /* Trigger object used to detect Free Fall situation. */
 static trigger_t sitAwFFAccWZ;
@@ -138,6 +138,9 @@ static void sitAwPreThrustUpdateCallOut(setpoint_t *setpoint)
         setpoint->mode.y = modeDisable;
         setpoint->mode.z = modeDisable;
         setpoint->thrust = 0;
+
+        /* Also stop the high-level commander */
+        crtpCommanderHighLevelStop();
       }
 #endif

[krichardsson]

@ntamas, this looks like a really good addition to the current functionality. If you issue a pull request I'll merge it.

If you want to put more time into it, the original issue was that the motors may start spinning again if a crashed CF (with stopped motors) is picked up.

[ntamas]

I'll create a PR soon. Stopping the HL controller will at least solve the part of the problem that happens when the setpoints are generated on-board (which is my use-case). Setpoints sent from an external controller are still an issue. I could add some kind of a boolean flag on top of the patch proposed above that also prevents the crash detector from allowing non-zero thrust until the Crazyflie is reset completely.

[ntamas]

Thinking aloud: if we add a boolean flag that cuts power to the motors, would it be okay to expose it as a settable parameter as well? This way it could be used to restore normal operation after a crash if the operator thinks it is safe to do so, without forcing a reset.

[krichardsson]

Sounds good! Just one small modification:
The parameter framework is designed to only allow modifications of values from the client. There is one exception which we call triggers: parameters that trigger a functionality when set to 1 (from the client) and that are reset to 0 by the firmware.

So in this case, the boolean flag should not be exposed directly as a parameter, instead there should be a parameter (a trigger) to reset it. The bool itself should probably be exposed as a log though, to expose the current state of the system.

[NicksonYap]

Hi @ntamas, I just noticed your PR and does it solve the issue where during flight controlled by the python lib and if a tumble occurs, the CF will still try to take off to desired position?
Currently, It will only stop if the CF is flipped 180 deg (bottom up)
And if I grab it and flip it back to the right orientation, the CF will take off again

[ntamas]

Yes, that's the problem that the PR attempts to solve. Basically, once a tumble is detected, an internal flag is triggered in the firmware that prevents non-zero thrust from reaching the motors until the flag is reset by setting a certain parameter to 1. The high-level controller is also stopped when a tumble is detected. So, no matter whether you are using the high-level controller or just sending roll / pitch / yaw setpoints continuously, this should prevent the motors from spinning up again.

Right now the patch has a problem, though: the same flag is triggered also when the CF is turned upside down and then back without the motors running (e.g., when you hold it in your hand). This could be a bit annoying and I am looking for guidance in the PR comments about how to resolve this in the least annoying way.

[NicksonYap]

maybe this flag is required to be enabled as a param? manually using the client or within the python lib

so when flying manually, it won't annoy the user
but when using setpoints, we can turn it on for better safety,
alternatively, we can override the flag and allow the CF to fly again upon next setpoint

[krichardsson]

@NicksonYap You have a good point in that there are conflicting properties when flying manually and from a script. The firmware (as far as I can see) will not be able to understand if it is flown manually or by script so the parameter way you are suggesting would be a way to solve the problem.

[knmcguire]

fixed in #481