Switch arming is completely broken in multiple ways

[mlyle]

In renatus originally, but probably won't stay here.

Things to think about:

• Failsafe and what happens to armed state (especially in conjunction with +Delay modes)
• Funny interaction with low throttle timeout-- especially in conjunction with +Delay modes)
• Funniness of ARMED_STILL_HOLDING state
• Edge vs. level triggered behavior
• ManualControl's enclosing state machine

[mlyle]

Also of note-- it should take more than 1 frame to arm/disarm via switch. I know we want to switch arm/disarm quickly... but we don't want to get fooled by glitches.

[henn1001]

As someone who is used to mw/cleanflight, the failsafe behavior here seems to be a total mess. It´s very confusing. I tested Stick arming with Arming timeout of 2 seconds.

Switch+Throttle: Turning off or unplugging my receivers disarms the quad after 2 seconds. (good) After arming and even in flight the quad will disarm after 2 seconds if i don´t raise my throttle over the neutral position (wtf - why) ?

Switch+Throttle+Delay: Turning off or unplugging my receivers disarms the quad after 2 seconds. (good) After arming, the quad can sit there indefinitely without disarming (good). After i raise my throttle and go back below the neutral position it disarms again after 2 seconds... and then it rearms itself (double wtf D: )

This whole auto disarm while below throttle neutral seems very alien to me. For the meantime i probably use really low throttle neutral + some kind of idle up switch that forces my min throttle command above neutral while armed. No disarming + working pids = win

[tracernz]

The armed timeout defaults to 30 seconds, did you change it to 2 seconds or has a bug been introduced?

[henn1001]

No i changed it to 2 seconds.. First i thought the failsafe behavior is completely broken. Keep in mind i´m using "spin at neutral while armed". In case of a failsafe the motors would spin for 30 seconds at neutral without disarming.. who wants that? Wanna buy new motors/escs if the quad "crashes" into deep grass?

This whole feature seems to be designed around the people who don´t use "spin at neutral".

[tracernz]

The default is to not spin while armed. Leaving an armed vehicle with no visible indication that it is armed for an indefinite period is very dangerous, hence the timeout by default.

[tracernz]

You can disable the timeout by setting it to 0 I believe (can confirm by looking at the code if necessary).

[henn1001]

But with 0 it will never disarm in case of a failsafe. tried that already.

[tracernz]

That is broken and needs fixed, I think there's an open issue for that.

[henn1001]

Seems to be here: https://github.com/d-ronin/dRonin/blob/874b310a9bfaba85bdf8dcbceeb5456996d06bc6/flight/Modules/ManualControl/transmitter_control.c#L693 ? if settings->ArmedTimeout == 0 then it will never be disarmed.

It should probably something like this:

if ((settings->ArmedTimeout != 0) && (timeDifferenceMs(armedDisarmStart, lastSysTime) > settings->ArmedTimeout))
            arm_state = ARM_STATE_DISARMED;
else if (!valid && settings->ArmedTimeout == 0)
            arm_state = ARM_STATE_DISARMED;

beh - just guessing here without know the codebase lol. gotta fix my compiler errors to test it myself

[dustin]

I crashed in grass a few times today. ESCs seemed to do a good job of giving up.

On Sat, Jan 9, 2016 at 4:59 PM henn1001 notifications@github.com wrote:

Seems to be here: https://github.com/d-ronin/dRonin/blob/874b310a9bfaba85bdf8dcbceeb5456996d06bc6/flight/Modules/ManualControl/transmitter_control.c#L693 ? if settings->ArmedTimeout != 0 then it will never be disarmed.

— Reply to this email directly or view it on GitHub https://github.com/d-ronin/dRonin/issues/357#issuecomment-170298230.

[tracernz]

@henn1001 That code will immediately disarm when the RC signal is invalid and the ArmedTimeout is set to 0.

-edit- I see that's the code you wrote! Oops. It looks good to me.

[henn1001]

Works as advertised? :D But yes - the failsafe code needs some buffer/bad frame counter. There were some notes about that in failsafe_control.c Problem is that the code is using the same time variable for 2 different (in my opinion) scenarios.

[tracernz]

Agreed, it should be a seperate timeout for failsafe, and should apply to ArmedTimeout=0 too.

[mlyle]

        if (!valid || (low_throttle && check_throttle)) {
            if ((settings->ArmedTimeout != 0) && (timeDifferenceMs(armedDisarmStart, lastSysTime) > settings->ArmedTimeout))
                arm_state = ARM_STATE_DISARMED;
        } else {
            armedDisarmStart = lastSysTime;
        }

I believe this code is correct. This code says:

• If the transmitter is disconnected, or the throttle is low:
  • If the armed timeout is not disabled, and it's been an excessive time that we've been at low throttle
  • Then disarm.
• Else, reset the time.

This is not the code used for switch disarm. It is used purely for long-disconnect-disarm and long-low-throttle-disarm.

The next code handles switch disarming:

                bool disarm = disarming_position(cmd, settings) && valid;
                if (disarm && (settings->Arming == MANUALCONTROLSETTINGS_ARMING_SWITCH ||
                                settings->Arming == MANUALCONTROLSETTINGS_ARMING_SWITCHDELAY ||
                                settings->Arming == MANUALCONTROLSETTINGS_ARMING_SWITCHTHROTTLE ||
                                settings->Arming == MANUALCONTROLSETTINGS_ARMING_SWITCHTHROTTLEDELAY)) {
                        arm_state = ARM_STATE_DISARMED;
                } else if (disarm) {
                        armedDisarmStart = lastSysTime;
                        arm_state = ARM_STATE_DISARMING;
                }

armedDisarmStart is set here, too, but the state changes; the variable has a different meaning in that state.

During DISARMING 'armedDisarmStart' is used for the time we entered that state. During ARMED 'armedDisarmStart' is used for the low-throttle-inactivity-timer.

[tracernz]

Why should the disarming behaviour be different when the RC signal is invalid?

[mlyle]

It's not? (Other than the difference with autonomous modes)

[henn1001]

@mlyle Yes, but what happens if the ArmedTimeout is euqal to 0? Then the code will never disarm in case of failsafe!? Is this a bug now or not?

[mluessi]

I think we should introduce a second timeout, failsafe timeout, i.e., a time after which it disarms if no valid RC input is received. The default should be much shorter, 2s or so, so it can survive short rx failsafes.

[mlyle]

I think ArmedTimeout of 0 is dumb except for planes. That's what armedtimeout == 0 is for-- craft that can be flown for long times at zero throttle or that you might need to keep armed for a long time while figuring out hand-launch, etc.

IMO, ArmedTimeout == 0 + multirotor + spin while armed is a very bad configuration.

I'm wary of introducing new parameters here. People already misunderstand the ones we have. Worse, since parameters get reset during upgrades, it's very easy for someone to end up with a surprising bad parameter (e.g. failsafe timeout == 2 would be really bad for planes).

[henn1001]

Well maybe the misunderstanding comes from the fact that this parameter is used for 2 different use cases. I googled the parameter and read something about auto disarming after x seconds while at min throttle. Thats completely fine and the default value of 30 seconds is ok for that. While the acro scene does have air hang-times with throttle at 0 ( requires active pids but this is another story) for 2-5 seconds, i doubt anyone does that for 30. So no problem here with auto disarming mid flight.

The problem occurs that this dumb parameter is combined with failsafe. A good failsafe behavior for mini/racequads is, after detecting signal loss, to completely cut the motors and just let it drop like a stone! Preferably after 1-2 seconds so you can filter out small signal cut/losses.

Im just repeating myself now, but who wants a craft to fart around for another 30 seconds and risk damaging motors/escs while it stalls motors somewhere in a tree or a bush?

But this whole thing goes obviously hand in hand with #406 and #228 .

[mlyle]

I disagree on almost all counts.

But-- if you want-- open a new issue where this can be discussed. You have hijacked issue 357 which is about the switch arming state machine.

[henn1001]

Yes, sorry about that :) I´ll do that.

[DTFUHF]

IMO, failsafe should be done in the receiver. The failsafe values are sent to the fc, and the fc handles the case that there is no input signal at all. For example, i prefer the failsafe setting of zero throttle but still armed, (and no motors spinning when armed) so i can recover quickly and without thinking when link is regained. (Disarming on failsafe has caused me to crash more than failsafe-ing continuously into the ground) Until failsafe functionality is greatly expanded, an idea which met with great resistance in taulabs, this is the most usable setup.

Some serial protocols such as sumd send a flag that radio link has been lost (independent of rssi) which could be used in these future, advanced failsafe modes. On Jan 10, 2016 13:28, "henn1001" notifications@github.com wrote:

Yes, sorry about that :) I´ll do that.

— Reply to this email directly or view it on GitHub https://github.com/d-ronin/dRonin/issues/357#issuecomment-170378501.

[mlyle]

Done to Tanto standards.