Unexpected Flight Termination/Lockdown

Last week during one of our flights, after taking off many times before with great results the drone suddenly shut off, very shortly after take-off, with all motors stopped into a crash.

Relevant log:
https://logs.px4.io/plot_app?log=7671448f-a107-48d7-94dd-0e4b81a5ef2b

As it can be seen there is an ERROR message at 0:12:25 [commander] Critical failure detected: lockdown .

Now I’ve did some digging around and it looks like the cause of this is that the drone went over the 60deg Roll offset for more than 0.3s which then triggers the lockdown, all actuators to failsafe and consequent crash. We’re running in offboard and the very agressive behaviour is mostly intended. There will be some improvements in that sense but to me it looks like this should be happening.

As we’re currently setting the CBRK_FLIGHTTERM parameter to enabled in theory this shouldn’t happen. From the parameters documentation specifically for FD_FAIL_P and FD_FAIL_R it specifically mentions

FailureDetector triggers the attitude_failure flag If flight termination is enabled (@CBRK_FLIGHTTERM set to 0)

which is not the case, however looking into commander.cpp it seems that this parameter is not taken into account during the first 3 secs of take-off. Is this correct??

I just want to confirm this first, and if correct I can

  1. Quickly fix the issue on my side by setting FD_FAIL_R and FD_FAIL_P to zero to completely remove this check. (as I’m also running a forked version of PX4, but rebased 1/2 months ago).

  2. Raise an issue on Github

  3. Work on a more robust fix. Imo just changing the if statement to

     			if ((hrt_elapsed_time(&_time_at_takeoff) < 3_s) &&
     		    !_lockdown_triggered && !status_flags.circuit_breaker_flight_termination_disabled) {
    

Should do the trick. If so I can make the PR today Unless it should be done in a separate parameter. This would also mean no changes to documentation as it would work as it states it will. If not at least a special mention for the take-off phase should be there.

Lastly this looks to have been introduced in https://github.com/PX4/Firmware/pull/14428 so maybe @bresch care to chime in with some input?

After some discussion on the PX4 slack it does seem intended behaviour as it is specifies clearly here:
https://docs.px4.io/master/en/config/safety.html#failure_detector

However in the parameter page it’s misleading so that should be fixed.

IMO it doesn’t make much sense to have this behaviour by default and/or bypassing CBRK_FLIGHTTERM. However this is all subjective and can be easily fixed on my side for my given scenario but feel free to comment on this behaviour.