Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kill switch and arm/disarm logic #10133

Closed
RicardoM17 opened this issue Aug 3, 2018 · 20 comments · Fixed by #10175
Closed

Kill switch and arm/disarm logic #10133

RicardoM17 opened this issue Aug 3, 2018 · 20 comments · Fixed by #10175

Comments

@RicardoM17
Copy link
Contributor

RicardoM17 commented Aug 3, 2018

From the discussion that started on the PX4 slack this issue is opened so the discussion on the relation between the kill switch and arm/disarm logic.

As proposed by me and also other users on the slack the kill switch (KS) should also automatically disarm the platform. Right now if the KS is activated, props stop immediatly (good for emergencies) but if the user/dev forgets to disarm before deactivating the KS props immediatly turn on with a high probabiliy of a crash (because the control loop is still on).

So proposal #1 is when the Killswitch is activated the platform is immediatly and automatically disarmed aswell.

This would improve security.

Another proposal that was discussed was the possible addition of a arm/disarm switch.

@RicardoM17
Copy link
Contributor Author

Paging @dagar @lamping7 and @DonLakeFlyer .

@DonLakeFlyer
Copy link
Contributor

I originally brought up this issue because it really caught my by surprised. Luckily none of my body parts where in the way when I disengaged the kill switch and the props spun back up.

My use of the kill switch tends to come from unstable landings. I drag around an antenna under the landing gear which can screw up landings at time. So when I see the vehicle possibly tipping over I hit the kill switch. For this purpose a disarm switch would not work since at least for me "disarm" does not always work. It only seems to work if I first change the flight mode to manual. Otherwise it says "not landed or not in manual mode" and won't disarm. So I use the kill switch on the transmitter as a faster version of the Emergency Stop slider in QGC.

@dagar
Copy link
Member

dagar commented Aug 3, 2018

The kill switch is a great safety feature, but I'm a bit concerned with the way I commonly see it being used more or less like a fast arm switch. The controllers don't know the kill switch is active and can easily get into a bad state.

I think having the kill switch also auto-disarm would alleviate the problems without reducing its intended use as a safety feature.

@RicardoM17
Copy link
Contributor Author

The kill switch is a great safety feature, but I'm a bit concerned with the way I commonly see it being used more or less like a fast arm switch.

The first proposal would fix this. Because then it wouldn't auto-arm or fast arm like you call it.

And I agree. The kill switch is for emergency scenarios, not as a workaround.

@dagar
Copy link
Member

dagar commented Aug 3, 2018

Sounds good to me, but what's the flip side?

Why shouldn't we have the kill switch simply force disarm immediately? After disengaging the kill switch you'd have to rearm.

@bkueng @MaEtUgR thoughts?

@DonLakeFlyer
Copy link
Contributor

Why shouldn't we have the kill switch simply force disarm immediately? After disengaging the kill switch you'd have to rearm.

That's certainly how I would have expected it to work.

@lamping7
Copy link
Member

lamping7 commented Aug 3, 2018

I agree with what @dager said. From my experience, it's used as @DonLakeFlyer explained.

@RicardoM17
Copy link
Contributor Author

RicardoM17 commented Aug 3, 2018

The whole issue is basically to see if there any reasons why using the kill switch doesn't automatically force disarm. Here we have reasons why it should, so if the more experienced users don't have any reasons why it is the way it is now it's an easy decision.

@dagar
Copy link
Member

dagar commented Aug 3, 2018

I'll wait for any dissenting opinions, but I would then propose.

  1. PX4 kill switch becomes force disarm (@dagar)
  2. QGC consider changing the language for kill switch (something about emergency?) and promote arm/disarm switch configuration to the same level. (@DonLakeFlyer)

@ndepal
Copy link
Contributor

ndepal commented Aug 3, 2018

Why shouldn't we have the kill switch simply force disarm immediately?

One thing that I can think of is accidental kills in air that can quickly be reverted in the current implementation but would most likely lead to a crash in the new proposal.
However, I don't know if that's something that happens to people.

@dagar
Copy link
Member

dagar commented Aug 3, 2018

Thanks for the input @ndepal. I actually think that's consistent with my goal here. Make it clear that the kill switch is intended for emergencies only. Beyond development and early testing the kill switch shouldn't be part of a normal users setup.

There are a few different cases for this overall issue, but the common one I keep seeing is people using the PX4 kill switch like the arming switch in Betaflight. A lot of this would go away if QGC had an arming switch configuration next to (or in place of) the kill switch configuration. There's already logic for quick rearm in air for manual flight situations. We can work on improving this use case once we get the kill switch abuse under control.

@DonLakeFlyer
Copy link
Contributor

QGC consider changing the language for kill switch

This comes from parameter meta data.

@bkueng
Copy link
Member

bkueng commented Aug 6, 2018

I'm ok with the kill switch leading to a forced disarm as well - but the disarming must not depend on any checks to pass.
And we must ensure that the logic on the implementation-level is kept really simple for the kill-switch, which might mean we keep the existing kill-switch logic and and just add a force-disarm call in addition (because the existing disarm logic is not simple).

@MaEtUgR
Copy link
Member

MaEtUgR commented Aug 6, 2018

Another proposal that was discussed was the possible addition of a arm/disarm switch.

It's there and should be used for any normal usage to disable the drone by switch. That was actually my first feature contribution: #6093 and was recently improved by @bkueng #9622. I think we should offer it also via UI.

One thing that I can think of is accidental kills in air that can quickly be reverted in the current implementation

@ndepal I agree, that's very likely the reason why it never disarmed at the same time. If that's a concern we could give the force disarm command a short delay to still enable reverting. I think noone accidentally disables the kill switch within 4 seconds of enabling it. What do you think?

As dagar said the kill switch is intended for emergencies only. I would say: Only use it for very dangerous bringup tests or when you see a crash unavoidable anymore. I also blame the disarm logic being to complicated. It should be configurable if the vehicle can be disarmed without landing detection because while enabling it is a good default and useful for products it's sometimes undesirable for developers.

And we must ensure that the logic on the implementation-level is kept really simple for the kill-switch

That's key, that's its only purpose.

@ndepal
Copy link
Contributor

ndepal commented Aug 6, 2018

give the force disarm command a short delay to still enable reverting. I think noone accidentally disables the kill switch within 4 seconds of enabling it.

That sounds like a good idea.

@dagar
Copy link
Member

dagar commented Aug 6, 2018

Initial PR #10175

@hamishwillee
Copy link
Contributor

hamishwillee commented Aug 6, 2018

Just FYI, the arm/disarm switch is documented: https://docs.px4.io/en/config/safety.html#arming_switch

@DonLakeFlyer If you add this as an option in the UI for RC switch selection, can you please let me know so I can update corresponding docs.

@DonLakeFlyer
Copy link
Contributor

If you add this as an option in the UI for RC switch selection, can you please let me know so I can update corresponding docs.

Support for that has been in for a while now.

@dagar
Copy link
Member

dagar commented Aug 8, 2018

On the dev call it was suggested that (with a safe debounce) the emergency kill switch should latch and require a reboot to clear.

Separately I'd like to push on any remaining arm/disarm switch and arming state machine annoyances that stand in the way of people using this mechanism directly. For example, @DonLakeFlyer if you still have the logs I'd be interested in reviewing any situations where disarm didn't work as you needed it to as well as any log where you needed to trigger kill.

@stale
Copy link

stale bot commented Feb 4, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants