Avoid acting on first RC input from receiver #13599
Conversation
|
.... the second change in the PR is that we must see a transition on (currently all) functions rather than acting on the first level we see. |
…ctions This is needed to ensure we get a consistent vehicle state with no race conditions on boot.
…tions This is needed to ensure we get a consistent vehicle state with no race conditions on boot.
…ctions This is needed to ensure we get a consistent vehicle state with no race conditions on boot.
…tions This is needed to ensure we get a consistent vehicle state with no race conditions on boot.
peterbarker
force-pushed
the
mode-switch-unknown
branch
from
2b5d2d8
to
aa650bf
Compare
|
So this is cool. The ADSB test in ArduPlane is failing because of the changes here. We reboot the vehicle with the RC input asserted on the channel with the "enable ADSB" on it. The change in this PR means that we will not act on that switch - we require a transition for all options at the moment. There is a method in here which allows you to whitelist options so that we will react to the first RC input level received - so fixing this particular test is a one-liner on top of the current PR. My question, 'though - does this make for a too-complicated user story? There's no feedback to the user that we haven't acted on the switch. |
|
This sounds like it makes the user experience really messy and harder to reason about. I would have assumed that if I had a switch set to do something and turned the transmitter on with the switch in that position we should execute the action. We literally just requested it over the radio... I understand the desire to protect the user from mistakes, but I think this brings you into a much worse realm where it's harder to actually get the desired action, and as you noted the lack of confirmation means that if you turn on the transmitter when it's far enough away you may have trouble getting enough packets through to the vehicle to assert the no action then the action state. Basically this is adding enough complication that I think it decreases the reliability of knowing what the vehicle will do, and has a net effect of being less safe overall. |
|
I agree with MdB.
Perhaps consider leaving the current behaviour as default, but add a couple of params to optionally blacklist some channels (throttle, arm and mode for instance).
As is I think this will cause a lot of user issues: we shouldn’t assume everyone reads release notes when they update.
… On 23 Feb 2020, at 8:30 am, WickedShell ***@***.***> wrote:
This sounds like it makes the user experience really messy and harder to reason about. I would have assumed that if I had a switch set to do something and turned the transmitter on with the switch in that position we should execute the action. We literally just requested it over the radio...
I understand the desire to protect the user from mistakes, but I think this brings you into a much worse realm where it's harder to actually get the desired action, and as you noted the lack of confirmation means that if you turn on the transmitter when it's far enough away you may have trouble getting enough packets through to the vehicle to assert the no action then the action state. Basically this is adding enough complication that I think it decreases the reliability of knowing what the vehicle will do, and has a net effect of being less safe overall.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
I agree with MDB The taranis and other radios had startup switch position checks to protect from such concerns. On HereLink, we have just moved to a new scheme that leaves mode switching to mavlink commands, and so doesn’t use RC at all for this, reducing risk again. |
|
We've decided to not go ahead with this PR. Some people are actively using the exact behaviour we'd be removing here. |
This should be considered unbelievably-wip at this point.
I said I'd reopen the old PR, but the old branch was long gone, so I recreated from scratch. Old PR is here: https://github.com/ArduPilot/ardupilot/pull/10166/files
Initiating issue is here: https://discuss.ardupilot.org/t/crash-following-rcinput-decoding-sbus/52290/7
There are two separate but related changes here.
The first is that we do not assume that we have radio input when initialising any auxillary functions.
The practical effect of that is that if you start with your transmitter off then when you turn it on we will no longer react to some inputs - so if you vehicle is armed we will not disarm it. If you have a mode-on-aux-switch switch asserted then we will not act on that (so if you are trying to recover a vehicle by turning the transmitter on and your signal is marginal you may have to hammer that switch until the vehicle sees a transition! OTOH, you might not want the vehicle to (e.g.) RTL the first time it sees the transmitter, so this may be intended behaviour).
Notes: