-
Notifications
You must be signed in to change notification settings - Fork 111
Accurately reflect PWM output configuration #3507
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?
Accurately reflect PWM output configuration #3507
Conversation
Reviewer's GuideThis PR refactors gripper-related logic by moving configuration checks into computed properties and a new Vuex module, replaces hardcoded channel detection with a dynamic loop over expanded channel definitions, and enhances the UI with validation messaging and layout constraints. Class diagram for new ArdupilotCapabilitiesStore Vuex moduleclassDiagram
class ArdupilotCapabilitiesStore {
+firmware_supports_actuators: boolean
}
ArdupilotCapabilitiesStore <|-- ardupilot_capabilities: instance
ArdupilotCapabilitiesStore ..> autopilot_data: uses
Class diagram for updated gripper.vue component logicclassDiagram
class Gripper {
+gripper: number | null
+boardType: string | undefined
+configured_actuators(): number[]
+misconfigured_gripper(): string | null
+toBoardFriendlyChannel(servo: string): string
}
Gripper ..> ardupilot_capabilities: uses
Gripper ..> autopilot_data: uses
Gripper ..> autopilot: uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
} | ||
|
||
get firmware_supports_light_functions(): boolean { | ||
// TODO: light functions were introduced at the same time as actuators |
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.
// TODO: light functions were introduced at the same time as actuators | |
// light functions were introduced at the same time as actuators |
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.
Not sure if you wanted reviews yet, since it's still marked as draft, but you did bring it up in the meeting so figured I'd at least take an initial look :-)
BTN_FUNCTION.SERVO_1_MIN_MOMENTARY, | ||
BTN_FUNCTION.SERVO_1_MAX_MOMENTARY, | ||
BTN_FUNCTION.SERVO_1_MIN_TOGGLE, | ||
BTN_FUNCTION.SERVO_1_MAX_TOGGLE, | ||
BTN_FUNCTION.SERVO_1_MIN, | ||
BTN_FUNCTION.SERVO_1_MAX, |
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.
Haven't these been renamed now to actuator_n_*
? Not sure how that's best handled from a display perspective, given they were/are called servo_n_*
for older firmware versions 🤷♂️
BTN_FUNCTION.SERVO_5_MAX_TOGGLE, | ||
BTN_FUNCTION.SERVO_5_MIN, | ||
BTN_FUNCTION.SERVO_5_MAX, | ||
] |
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.
What about Actuator 6?
<v-card-text v-if="misconfigured_gripper" class="red--text"> | ||
{{ misconfigured_gripper }} |
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.
This seems worth expanding to more general checks of missing outputs corresponding to assigned button functions:
- Lights N
- Actuator N (with gripper mentioned as a potential intended output)
- Relay N
but that's perhaps out of scope for this PR 🤷♂️
|
||
get firmware_supports_light_functions(): boolean { | ||
// TODO: light functions were introduced at the same time as actuators | ||
return autopilot_data.parameter('ACTUATOR1_INC') !== undefined |
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.
Is there pre-filtering somewhere, that makes this apply only to ArduSub?
IIRC Actuator outputs are more general ArduPilot, but Lights is a Sub thing - I could be wrong on both accounts though 🤷♂️
Relevant to #3501 |
Needs to include outputs displaying as RCIN9/10 when using a firmware that supports the new lights outputs (if the output parameters are not using Lights1/2 values) |
mixed feelings about the store. thoughts?
the idea is to slowly move logic away from .vue files
Summary by Sourcery
Extract actuator capability detection to a Vuex module and enhance the gripper component with generic channel mapping and misconfiguration warnings.
New Features:
Enhancements: