Skip to content

RC_Channel: config error on mode channel and RCn_OPTION conflict - #34600

Closed
Hwurzburg wants to merge 4 commits into
ArduPilot:masterfrom
Hwurzburg:rc-mode-channel-config-error
Closed

Hwurzburg wants to merge 4 commits into
ArduPilot:masterfrom
Hwurzburg:rc-mode-channel-config-error

Conversation

@Hwurzburg

@Hwurzburg Hwurzburg commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Alternate to #34592. Instead of making it a periodic pre-arm check, a flight mode channel that also has an RCn_OPTION set is reported at boot as a config error (Config Error: Mode channel and RC8_OPTION conflict). Like other config errors, the vehicle stays in the config error loop until the parameter is fixed and the vehicle is rebooted. The existing arm-time check is kept, so a conflict created after boot still blocks arming.

Classification & Testing (check all that apply and add your own)

  • Checked by a human programmer

  • Non-functional change

  • No-binary change

  • Infrastructure change (e.g. unit tests, helper scripts)

  • Automated test(s) verify changes (e.g. unit test, autotest)

  • Tested manually, description below (e.g. SITL)

  • Tested on hardware

  • Logs attached

  • Logs available on request

  • New Rover autotest RCModeChannelOptionConflict:

    • sets MODE_CH=8, RC8_OPTION=4 and reboots
    • waits for Config Error: Mode channel and RC8_OPTION conflict
    • sets RC8_OPTION=0 from inside the config error loop, reboots and waits for ready-to-arm
    • sets RC8_OPTION=4 again and checks that arming fails with Mode channel and RC8_OPTION conflict

    It passes on this branch. With the RC_Channels.cpp change reverted, it fails because the config error text is never received.

  • test.Rover.RCDuplicateOptionsExist and test.Rover.ConfigErrorLoop still pass.

  • SITL builds of copter, plane, rover, sub, blimp and antennatracker succeed.

  • The AP_SUB_RC_ENABLED=0 branch of the Sub change was not compiled; it only changes a returned constant.

Description

The check is done in RC_Channels::init(), before init_aux_all(), using the existing flight_mode_channel_conflicts_with_rc_option(). The arm-time checks in AP_Arming::rc_arm_checks() are unchanged.

Behaviour changes:

  • At boot, the conflict can't be skipped with ARMING_SKIP_CHK, and the check doesn't depend on ARMING_REQUIRE or RC input. The vehicle doesn't finish booting until the conflict is fixed. A vehicle without a receiver (GCS or companion control) that has a leftover RCn_OPTION on its mode channel could previously arm, and will now stop at boot after upgrading.
  • On Plane, rc().init() runs before gcs().setup_uarts(), so during the config error loop only the console port (normally USB) has MAVLink. Existing Plane config errors raised before setup_uarts(), such as barometer calibration, behave the same way.

The arm check could be removed, since the boot check catches any conflict present at startup. That saves 44 bytes of flash (measured on MatekF405 Copter and Plane). The cost is that a conflict created by changing parameters after boot would go unreported until the next reboot. On the other hand, a user can make the vehicle dangerous after boot through parameter changes in any number of ways, so reviewers may prefer to save the bytes. This PR keeps the arm check.

Tracker has no flight mode channel, but flight_mode_channel_number() returned 1, so a non-zero RC1_OPTION would put Tracker into the config error loop. Sub's AP_SUB_RC_ENABLED=0 fallback had the same placeholder. Both now return 0, which RC_Channels::flight_mode_channel() already treats as "no mode channel". These two commits are the same as in #34592.

Related: #34592 (pre-arm approach; only one of the two should be merged), #34427, #32749 (replaces FLTMODE_CH with an RC option; whichever PR merges second needs rebasing).

AI-assisted: implementation, tests and description were drafted with Claude Code.

🤖 Generated with Claude Code

Tracker has no flight mode channel, but flight_mode_channel_number()
returned 1, so RC_Channels::flight_mode_channel() returned RC1 and a
non-zero RC1_OPTION would be reported as a mode channel conflict.
Return 0, which flight_mode_channel() already treats as no channel.
With AP_SUB_RC_ENABLED=0 flight_mode_channel_number() returned 1,
making RC1 look like a mode channel. Return 0, which
flight_mode_channel() already treats as no channel.
A flight mode channel that also has an RCn_OPTION set is a
configuration error. Report it at boot via the config error loop, so
the user must fix the parameters and reboot, like other config errors.
Check the conflict is a config error at boot, and that a conflict
created after boot is still caught at arming.
@AP-Review

Copy link
Copy Markdown

Automated review note — AI-generated (Claude), validated against the live diff. Please sanity-check before acting.
Verdict: COMMENT

Reviewed at head df1f3ab2a4.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/ardupilot/34600/1.html#pr34600

Thanks, this is a sensible tightening, and the Tracker/Sub change to return 0 is needed so that RC1_OPTION doesn't trip the new check. The new Rover autotest passes at head and fails with the RC_Channel change reverted. Two nonblocking points:

  1. The new config error in RC_Channels::init (https://github.com/ardupilot/ardupilot/pull/34600/files#diff-9fe83d8a1f3ccb4c9107664d6c6ae867193a141f4f76221d6bcc1248d2279334R64) runs whether or not RC is present. The arming check it mirrors only ran with the RC arming check enabled and valid, recent RC input. A receiver-less, GCS/companion-driven vehicle with a leftover RCn_OPTION on its mode channel used to arm; after this change it stops at boot. I reproduced this in Rover SITL with RC8_OPTION=4 and ARMING_SKIPCHK=64. It can be fixed over the GCS while in the error loop, so I'm not treating it as a blocker. Please either mention the upgrade impact in the PR description or only raise the error when RC is actually in use.

  2. On Plane, rc().init() runs before gcs().setup_uarts() (ArduPlane/system.cpp:16 vs :45), so when this error fires only the first configured MAVLink port gets the heartbeat and the error text. A Plane connected only over a telemetry radio on a secondary port looks dead. Doing this check after setup_uarts would avoid that.

Not verified: an AP_SUB_RC_ENABLED=0 Sub build, real hardware, and CI (pending at review time).

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

Global Size Summary (Total Flash delta in bytes)

Built from df1f3ab rebased onto e204ca7

Board Periph tracker blimp bootloader copter heli iofw plane rover sub
DrotekP3Pro _ _ _ * _ _ _ _ _ _
Durandal _ +28 +28 _ +28 +28 _ +28 +28 +28
Hitec-Airspeed * _ _ _ _ _ _ _ _ _
KakuteF7 _ +28 +28 _ +28 +28 _ +28 +28 +28
MambaH743v4 _ +52 +52 _ +52 +52 _ +52 +52 +52
MatekF405-CAN _ _ _ * _ _ _ _ _ _
MatekF405 _ +28 +28 _ +28 +28 _ +28 +28 +28
MatekH743-bdshot _ +28 +28 _ +28 +28 _ +28 +28 +28
Pixhawk1-1M _ +28 +28 _ +28 +28 _ +28 +28 +28
disco _ +56 +56 _ +38 +56 _ +56 +56 +60
f103-GPS * _ _ _ _ _ _ _ _ _

@Hwurzburg

Copy link
Copy Markdown
Contributor Author

closing this since config errors are only used if the bd will crash if it continues to boot...the pre-arm check is the better approach

@Hwurzburg Hwurzburg closed this Oct 4, 2026
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.

3 participants