-
Notifications
You must be signed in to change notification settings - Fork 18.6k
Add support for second transmitter-tuning control channel #30042
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
base: master
Are you sure you want to change the base?
Add support for second transmitter-tuning control channel #30042
Conversation
libraries/RC_Channel/RC_Channel.h
Outdated
@@ -283,6 +283,8 @@ class RC_Channel { | |||
MOUNT2_YAW = 217, // mount4 yaw input | |||
LOWEHEISER_THROTTLE= 218, // allows for throttle on slider | |||
TRANSMITTER_TUNING = 219, // use a transmitter knob or slider for in-flight tuning | |||
TRANSMITTER_TUNING2 = 220, // use another transmitter knob or slider for in-flight tuning | |||
TRANSMITTER_TUNING3 = 221, // use yet another transmitter knob or slider for in-flight tuning |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we don't need TRANSMITTER_TUNING3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we don't need TRANSMITTER_TUNING3?
I decided to reserve an additional value just to keep things consecutive if someone did want another one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. until muramura comes along in 5 minutes and removes it :-). We have a golden rule in AP that we don't think about the future too much because it's hard to predict so let's remove it. users don't care about the numbers much anyway now that we alphabetise the param values that are shown in the GCSs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. until muramura comes along in 5 minutes and removes it :-). We have a golden rule in AP that we don't think about the future too much because it's hard to predict so let's remove it. users don't care about the numbers much anyway now that we alphabetise the param values that are shown in the GCSs
Fair enough, I've removed it.
Note I'm not wedded to adding this second channel, but it's always vexed me that there was only one (and that it was fixed at CH_6, of course ;-) )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peterbarker, thanks for your efforts. If you really want it then I'm happy to go along with that. Can I ask though when was the last time that you (or anyone else on the CanberraUAV team) actually wanted to manually tune a copter from the transmitter? I personally never tune using transmitter knobs anymore, I always just do the updates from the GCS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peterbarker, thanks for your efforts. If you really want it then I'm happy to go along with that. Can I ask though when was the last time that you (or anyone else on the CanberraUAV team) actually wanted to manually tune a copter from the transmitter? I personally never tune using transmitter knobs anymore, I always just do the updates from the GCS.
Weelll.... I don't think the CanberraUAV people are typical fliers :-)
I don't think I've personally used this stuff in a very long time. Mostly because I do usually have a friendly GCS operator to change values while I'm flying.
I have seen tridge use transmitter-based tuning within the last couple of years - but not this sort. The sort that's only present in Plane and allows you to rotate through PID values using a switch.
If we want to start to ease this out of the code instead we can do that. I think this would make sense as a build-server option anyway. Still useful on 1MB boards but not vital.
This PR costs 136 bytes on CubeOrange.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, so the short answer is that even CanberraUAV won't use this feature. If we have time, let's ask at the dev call what other devs think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... compiling the feature out saves 1,400 bytes. I've added defines for that in this PR now.
Txs, the issue linked is from 2015 though so I wonder if perhaps things haven't moved on so much as to make this unnecessary. Back in those days we had limited interface options for most users (e.g. less users of telemetry, no lua scripting, less sophisticated GCSs). I'm not blocking this but I think we should carefully consider whether we really want this |
I believe people still do actually use these. |
64897eb
to
2e1dbd2
Compare
reboot is now required. Also verify that we are zeroing the DZ
these are all about to change names, so simplify the logic first
…zero much the same effect, but 0 may not be the default value of a channel.
hopefully a clearer name
3a97d8f
to
c070e6f
Compare
There are several pre-cursor PRs to merge before this one.
Closes #2125