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

RC_Channel: put options in numerical order and add missing options for Rover #12804

Merged
merged 4 commits into from Nov 12, 2019

Conversation

@rmackay9
Copy link
Contributor

rmackay9 commented Nov 9, 2019

This PR includes two changes:

  • a non-functional reorder of the case statement in each of the RC_Channel::init_aux_function() methods for the RC_Channel library and each vehicle. This re-order may cause some annoyance as we try to backport options to the stable branches but long lists (of functions, cases, etc) in seemingly random order make spotting errors difficult. Let's try and use either numerical or alphabetical ordering.
  • add 3 missing options to the RC_Channel_Rover::init_aux_function() handler. This error was discovered as part of Rover-4.0 testing (discussion)

This has been tested on a CubeOrange:

  • set RC6_OPTION = 4 (RTL), 42 (SmartRTL) and 59 (Simple)
  • loaded Rover-4.0.0-rc3 and checked that the, "Config Error: failed to init" message appeared
  • loaded this PR and checked that the message no longer appeared, then used the channel to set the mode to each of the 3 listed
@rmackay9 rmackay9 referenced this pull request Nov 9, 2019
2 of 9 tasks complete
@peterbarker

This comment has been minimized.

Copy link
Contributor

peterbarker commented Nov 10, 2019

@rmackay9

This comment has been minimized.

Copy link
Contributor Author

rmackay9 commented Nov 11, 2019

I'm happy with either alphabetical or numerical. We might also consider simply removing the lines for options that don't need any initialisation.

@rmackay9

This comment has been minimized.

Copy link
Contributor Author

rmackay9 commented Nov 11, 2019

as discussed on the dev call I change the order in the case statement to be alphabetical

@rmackay9 rmackay9 force-pushed the rmackay9:rover-rc-option-fix branch 3 times, most recently from 8762565 to 48635c4 Nov 12, 2019
@rmackay9

This comment has been minimized.

Copy link
Contributor Author

rmackay9 commented Nov 12, 2019

I've rebased this PR and put the case statements in alphabetical order now

@rmackay9 rmackay9 force-pushed the rmackay9:rover-rc-option-fix branch from 48635c4 to aff8836 Nov 12, 2019
@rmackay9

This comment has been minimized.

Copy link
Contributor Author

rmackay9 commented Nov 12, 2019

Rebased on master following merge conflict with this PR #12757

@peterbarker peterbarker merged commit 762b2c9 into ArduPilot:master Nov 12, 2019
4 checks passed
4 checks passed
ArduPilot.ardupilot Build #20191112.17 succeeded
Details
ArduPilot.ardupilot (Cygwin SITL build) Cygwin SITL build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
semaphoreci The build passed on Semaphore.
Details
@peterbarker

This comment has been minimized.

Copy link
Contributor

peterbarker commented Nov 12, 2019

Merged, before I create another conflict :-)

@rmackay9

This comment has been minimized.

Copy link
Contributor Author

rmackay9 commented Nov 12, 2019

haha, thanks!!

@rmackay9 rmackay9 deleted the rmackay9:rover-rc-option-fix branch Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Rover 4.0 backports
Awaiting triage
4 participants
You can’t perform that action at this time.