Move rudder arming up into RC_Channel library #16091

peterbarker · 2020-12-17T21:23:18Z

This will make rudder-arming consistent across our vehicles.

One known behaviour change is that the rover arm interval has moved from 2 to 3 seconds (to be like Copter and Plane).

Other known changes in behaviour:

Copter and Rover will try to arm if your throttle is non-zero, and you will get a warning that your throttle is non-zero. In master we don't try to arm. I think this new behaviour is far preferable from a user's standpoint as they're told why they can't arm right now in the case they meant to arm! The new code has the same behaviour as Plane in master

~~We can't move the rudder-specific arming checks into the RC_Channels library as ToyMode also arms with a reason of "rudder" and presumably wants the same checks~~ We did some surgery to fix this

peterbarker · 2021-04-01T14:05:17Z

Tested in SITL - standard arming and RUDDER_DISARM values.

ArduCopter/Copter.h

ArduCopter/RC_Channel.cpp

libraries/RC_Channel/RC_Channels.cpp

tridge · 2021-04-07T07:43:22Z

@rmackay9 also noted on the call that he's happy for the 10s trim feature to be removed if whoever removes it ensures docs are updated

rmackay9 · 2021-04-07T07:44:29Z

Docs for autotrim are here: https://ardupilot.org/copter/docs/autotrim.html and here: https://ardupilot.org/copter/docs/arming_the_motors.html. The 2nd link in particular has a warning that could be updated to be clear it only applies to 4.0 (and earlier)

stops uwarning the user instantly

…ING_RUDDER=0

we do not run the arming checks if ARMING_CHECK is zero

this was ineffective as Blimp was not enforcing the parameter. We do now

… throttle

as required by PR feedback (ArduPilot#16091) and my DevCall decision.

it was decided that your throttle-is-zero check was really part of the gesture and thus belonged in RC_Channels, not in AP_Arming.

as determined at DevCall

- does it really matter if the user does this when they can just throttle up to take off? - stops spreading oymode stuff through the code - kind of weird doing this in Arming anyway

as opposed to trying to arm and have the arming library say no

for setting a single bit

rmackay9

I'm happy for this to be merged once @andyp1per has completed his testing and has approved

andyp1per

Think its ok, just a couple of questions

Rover/AP_Arming_Rover.cpp

andyp1per · 2025-07-04T16:02:01Z

libraries/RC_Channel/RC_Channels.cpp

+        // only permit arming if the vehicle isn't being commanded to
+        // move via RC input
+        const auto &c = rc().get_throttle_channel();
+        if (c.get_control_in() != 0) {


don't you want to use the deadzone version here?

I don't think so - this is pilot stick input we want to be inactive. It would be really, really difficult for a Rover user to get zero input on a mid-stick-is-zero throttle input without a dz :-)

andyp1per · 2025-07-04T16:06:21Z

ArduCopter/AP_Arming_Copter.cpp

    }

+    if (method == AP_Arming::Method::RUDDER) {
+        if (!copter.flightmode->has_manual_throttle() && !copter.ap.land_complete) {


I'm wondering whether we should remove the manual throttle check here. Disarming mid-air at zero throttle, full yaw seems bad.

Thanks AndyP very much for helping test this important PR

we should certainly be careful about changes in behaviour because it is also bad to be unable to disarm the vehicle waiting for the land-complete to become true. That can lead to vehicles flopping around on the ground.

I haven't given this enough thought one way or the other. I absolutely don't want to change something so fundamental/dangerous in this PR.

I do agree mid-air-accidental-disarms sound very bad. But we might end up with people not able to disarm depending on how the landing detector modifies its behaviour based on manual throttle etc etc.

We could document our way out of it - suggest people doing inverted yaw spins may want to set the disarm-via-stick to false. We could potentially change the default value to disallow it in minimize-fpv-osd setups. We could say that if you've armed-with-a-switch-with-airmode then we don't do it. Lots of options, lots of thought, a very small amount of coding, a lot of testing!

I'd also say that I've never actually heard of someone managing to hit this problem - I do find that a bit of a surprise.

tldr; sure, we need to think about it, but I do not want to do it as part of this PR.

I was thinking that perhaps the right check was manual throttle without airmode, that would probably cover the majority case in a safe way.

I was thinking that perhaps the right check was manual throttle without airmode, that would probably cover the majority case in a safe way.

Probably right - but not for this PR.

Are you OK with this one, @andyp1per ?

as required by PR feedback (#16091) and my DevCall decision.

peterbarker added WIP NeedsTesting labels Dec 17, 2020

peterbarker mentioned this pull request Dec 17, 2020

Plane: move rudder-arming arm checks into Plane's AP_Arming #16008

Merged

peterbarker force-pushed the pr/rudder-arming-up branch 3 times, most recently from 5570908 to f71ac3b Compare December 18, 2020 03:13

peterbarker force-pushed the pr/rudder-arming-up branch 4 times, most recently from e7c327b to 827ff0d Compare January 1, 2021 01:55

peterbarker force-pushed the pr/rudder-arming-up branch from 827ff0d to 8bc7e33 Compare January 12, 2021 02:17

peterbarker force-pushed the pr/rudder-arming-up branch from 8bc7e33 to 65df351 Compare February 3, 2021 10:48

peterbarker force-pushed the pr/rudder-arming-up branch 2 times, most recently from 157f4c3 to 9ef65ee Compare March 27, 2021 01:51

peterbarker removed the WIP label Apr 1, 2021

peterbarker force-pushed the pr/rudder-arming-up branch from 9ef65ee to 24377cf Compare April 1, 2021 14:04

peterbarker removed the NeedsTesting label Apr 1, 2021

peterbarker added the DevCallTopic label Apr 1, 2021

CraigElder removed the DevCallTopic label Apr 5, 2021

rmackay9 reviewed Apr 5, 2021

View reviewed changes

ArduCopter/Copter.h Outdated Show resolved Hide resolved

rmackay9 reviewed Apr 5, 2021

View reviewed changes

ArduCopter/RC_Channel.cpp Outdated Show resolved Hide resolved

tridge requested changes Apr 5, 2021

View reviewed changes

libraries/RC_Channel/RC_Channels.cpp Outdated Show resolved Hide resolved

peterbarker force-pushed the pr/rudder-arming-up branch 3 times, most recently from ab489a5 to 2815824 Compare April 6, 2021 06:41

peterbarker added the DevCallEU label Apr 7, 2021

tridge removed the DevCallEU label Apr 7, 2021

peterbarker added 8 commits June 22, 2025 10:13

autotest: add test for QuadPlane auto-takeoff wait-for-rudder-release

b38fbd3

Plane: set timer for rudder post-arming

8a7991b

stops uwarning the user instantly

autotest: ensure we can't arm with rudder with ARMING_CHECK=0 and ARM…

24f6d00

…ING_RUDDER=0

AP_Arming: move no-rudder-arming check into ::arm

9cd0066

we do not run the arming checks if ARMING_CHECK is zero

autotest: remove default value for ARMING_RUDDER in blimp

1e98d8a

this was ineffective as Blimp was not enforcing the parameter. We do now

AP_Arming: ignore attempts to disarm if rudder-disarming and non-zero…

58f708d

… throttle

AP_Arming: fail arm attempt silently if arming is non-zero

7b0d8ed

as required by PR feedback (ArduPilot#16091) and my DevCall decision.

AP_Arming: move throttle-is-zero check into RC_Channels library

650a951

it was decided that your throttle-is-zero check was really part of the gesture and thus belonged in RC_Channels, not in AP_Arming.

peterbarker force-pushed the pr/rudder-arming-up branch from c6ef41f to 90a1319 Compare June 22, 2025 00:14

peterbarker added 7 commits June 22, 2025 11:11

RC_Channel: move throttle-is-zero check into RC_Channels library

791508e

it was decided that your throttle-is-zero check was really part of the gesture and thus belonged in RC_Channels, not in AP_Arming.

AP_Arming: add AP_Arming::Method::TOYMODE

bf189c4

as determined at DevCall

ArduCopter: add AP_Arming::Method::TOYMODE

49872f8

as determined at DevCall

Copter: remove restriction on arming with rudder in toymode

9e9fd1e

- does it really matter if the user does this when they can just throttle up to take off? - stops spreading oymode stuff through the code - kind of weird doing this in Arming anyway

RC_Channels: do not attempt to arm if ARMING_RUDDER

2b919ae

as opposed to trying to arm and have the arming library say no

autotest: add quadplane test for ARMING_CHECK_THROTTLE unset

dd3384f

autotest: add set_parameter_bit method

f511676

for setting a single bit

peterbarker force-pushed the pr/rudder-arming-up branch from 90a1319 to f511676 Compare June 22, 2025 01:15

peterbarker added the DevCallEU label Jun 23, 2025

rmackay9 approved these changes Jun 25, 2025

View reviewed changes

peterbarker requested a review from rmackay9 June 25, 2025 07:41

tridge approved these changes Jun 25, 2025

View reviewed changes

peterbarker removed the DevCallEU label Jun 25, 2025

andyp1per reviewed Jul 4, 2025

View reviewed changes

andyp1per approved these changes Jul 7, 2025

View reviewed changes

peterbarker merged commit 02a6ba0 into ArduPilot:master Jul 7, 2025
107 of 108 checks passed

peterbarker added a commit that referenced this pull request Jul 7, 2025

AP_Arming: fail arm attempt silently if arming is non-zero

5abf3e1

as required by PR feedback (#16091) and my DevCall decision.

github-project-automation bot moved this to Done in Peter's ArduPilot 4.7 Queue Jul 7, 2025

peterbarker deleted the pr/rudder-arming-up branch July 7, 2025 22:54

peterbarker removed this from Peter's ArduPilot 4.7 Queue Jul 29, 2025

Move rudder arming up into RC_Channel library #16091

Move rudder arming up into RC_Channel library #16091

Uh oh!

Conversation

peterbarker commented Dec 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peterbarker commented Apr 1, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tridge commented Apr 7, 2021

Uh oh!

rmackay9 commented Apr 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rmackay9 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andyp1per left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andyp1per Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

peterbarker Jul 5, 2025

Choose a reason for hiding this comment

Uh oh!

andyp1per Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

rmackay9 Jul 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterbarker Jul 5, 2025

Choose a reason for hiding this comment

Uh oh!

andyp1per Jul 5, 2025

Choose a reason for hiding this comment

Uh oh!

peterbarker Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

peterbarker commented Dec 17, 2020 •

edited

Loading

rmackay9 commented Apr 7, 2021 •

edited

Loading

rmackay9 left a comment •

edited

Loading

rmackay9 Jul 5, 2025 •

edited

Loading