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

AP_Arming: Check that sticks are neutral #13937

Merged

Conversation

WickedShell
Copy link
Contributor

This is designed to catch the situation where someone has slowly nudged the trim sticks over by accident and has not noticed it. The problem with this is that they will be taking off and can be commanding a large input when they think they are neutral, which leads to erratic (and scary) flights.

This checks that the roll/pitch/throttle/yaw channels are inside of their deadzone before allowing arming. It's done as an arm rather then prearm check because I needed access to the method to check if the user was attempting to rudder arm or not.

This was bench tested with real hardware and behaved as expected.

libraries/AP_Arming/AP_Arming.cpp Outdated Show resolved Hide resolved
@tridge
Copy link
Contributor

tridge commented Mar 30, 2020

also, throttle trim is otherwise unused, so should be ignored even without sprung throttle

@tridge
Copy link
Contributor

tridge commented Mar 30, 2020

needs a check for RC failsafe

@rmackay9
Copy link
Contributor

rmackay9 commented Mar 30, 2020

For the record, I think this is going to lead to too many false positives and cause user pain and support calls which will likely fall on my plate :-(. Still, for the sake of keeping the vehicles consistent I'll compromise and let it in but as discussed I reserve the right to say, "I told you so" when we hit troubles during the next beta period.

I think we should make the check less strict. So perhaps 2x the deadzone would be better. I.e. we catch really large deviations from center but not just a single PWM value off from center.

@WickedShell WickedShell force-pushed the wickedshell/arming-rc-neutral branch from c2d266b to 1e1dbc4 Compare March 31, 2020 02:09
@WickedShell
Copy link
Contributor Author

WickedShell commented Mar 31, 2020

Made the changes from the call, this no longer checks throttle by default and will check for a failsafe. This does allow you to explicitly check the throttle if you want to via a RC_OPTIONS bit.

EDIT: Also fixed the missing doc from f299a4a

@WickedShell WickedShell force-pushed the wickedshell/arming-rc-neutral branch from 1e1dbc4 to 8304ae7 Compare March 31, 2020 02:15
@rmackay9
Copy link
Contributor

rmackay9 commented Mar 31, 2020

The "has valid input" check timesout and becomes false if there's been no input in a while? I'm thinking of the case where the user turns on the receiver, then turns it off before arming (from the GCS). I'm also thinking of the two types of receivers we have, the ones that that output no pulses and the ones that pull the throttle low.

@WickedShell WickedShell force-pushed the wickedshell/arming-rc-neutral branch from a77f0f5 to 5891b9f Compare April 2, 2020 21:12
@WickedShell
Copy link
Contributor Author

@rmackay9 So I wanted to say yes, it just does that, however it appears that based on all the vehicles that's not actually the case. has_valid_input only looks at if it has an RC failsafe or not, and ignores the no RC receiver case entirely. I think this might need some more fixups, as the has_valid_input call is used in AP_Tuning, RC_Channels::read_aux_all, RC_Channels::read_mode_switch() which is a bit worrisome. To work around this in the AP_Arming code I added a check to ensure the last RC input was received within the last second, and ever. But this is a bit of an odd patch I admit.

@tridge
Copy link
Contributor

tridge commented Apr 7, 2020

make it 2 option bits, one for RPY, one for throttle, RPY option bit should be on by default

@WickedShell WickedShell force-pushed the wickedshell/arming-rc-neutral branch from 5891b9f to 03cea29 Compare April 7, 2020 01:56
if (c == nullptr) {
continue;
}
if (!c->in_trim_dz()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Michael and I are in agreement this should probably by looking at control-in rather than in-dz. But this is what was agreed upon at the devcall, so don't think this should block this going in.

@WickedShell WickedShell merged commit 73c5c2e into ArduPilot:master Apr 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants