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

Sub: refactor aux servos #28801

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Williangalvani
Copy link
Contributor

This refactors Sub's servo functions. they are now called actuators.
This now allows them to be placed on any outputs, rather than the hard-coded servo9, 10, and 11.
This also exposes parameters for setting both the initial increment step and an acceleration factor.

The main goal was to introduce the user-adjustable increment and acceleration, but as things were a mess, I felt I had to try to make it better.

The "Actuator" choice was so that we could easily plug handling of MAV_CMD_DO_SET_ACTUATOR in on a following PR.

I'm considering setting the default servo9-11 functions to the corresponding actuators in order for this to be no change to users upgrading.

I swear we do want to stop relying on these hardcoded button settings, but some things are easier to migrate away than others 🥲

@Williangalvani Williangalvani force-pushed the aux_refactor branch 5 times, most recently from c339cf0 to e39bc88 Compare December 6, 2024 00:34
@peterbarker
Copy link
Contributor

[10:01 AM]Peter Barker: Why do you wish to continue to do the integration of the actuator output position deltas on the flight controller?  We generally frown on that as our primary transport (mavlink) is assumed to be lossy, so "go 2m/s faster is a terrible way to control the vehicle.  I'm also not convinced of the need to add a mapping from "actuator" to "servo output".  I'm suggesting that having the GCS know what the absolute position should be might be a better way to go here....

[10:49 AM]Willian Galvani: On Sub, if MAVLink ever gets lossy, we'll have bigger issues as the video feed will have dropped a while ago  .
This is addressing an immediate need of fine-tuning an output, and along the way I decided to get rid of the hard-coded servo9-servo11 thing.

I disagree about the adding the mapping from actuator to "Servo output". I'd really prefer behaviors to be consistent whenever possible.
Using "Disabled" channels for functions feels like a hack to me.
The fact that the previous code changed the channel regardless of it being disabled feels even worse.
This change also makes it clear for users what each channel is doing just by looking at SERVOn_FUNCTION, without having to look elsewhere for DO_SET_SERVO indexes.

About having the GCS know what the absolute position should be, I agree, but I'd like the control the absolute position of an actuator or system, instead of a "raw channel".
I do want to move away from JSButton and friends pretty soon, and I'm hoping CMD_DO_SET_ACTUATOR could replace at least this "servo" section of our functionality.

@Williangalvani Williangalvani force-pushed the aux_refactor branch 2 times, most recently from dcbd3f8 to 37ce97a Compare February 3, 2025 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants