Conversation
1104c8b to
6d4df0b
Compare
There was a problem hiding this comment.
Pull Request Overview
Add gear ratio selection and handling for Winder2 so puller speeds, UI controls, and emitted state correctly reflect the selected gearbox.
- Introduces GearRatio in backend, applies multiplier in control and divides in emission, and exposes a new SetPullerGearRatio mutation.
- Extends frontend schemas, state, and hooks; adds settings control and presets integration; adjusts control max speed based on gear ratio.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/machines/winder2/puller_speed_controller.rs | Adds GearRatio enum, stores it in controller, applies multiplier in update_speed. |
| server/src/machines/winder2/emit.rs | Emits gear_ratio and divides motor speed by ratio to get material speed; adds setter API. |
| server/src/machines/winder2/api.rs | Adds SetPullerGearRatio mutation and gear_ratio in PullerState. |
| electron/src/machines/winder/winder2/winder2Namespace.ts | Adds GearRatio schema/type and getGearRatioMultiplier; updates puller state schema. |
| electron/src/machines/winder/winder2/useWinder.ts | Adds request and setter for gear ratio with optimistic update and server calls. |
| electron/src/machines/winder/winder2/Winder2Settings.tsx | Adds UI selection for gear ratio in settings. |
| electron/src/machines/winder/winder2/Winder2PresetsPage.tsx | Shows gear ratio in preview; attempts to restore ratio on load. |
| electron/src/machines/winder/winder2/Winder2ControlPage.tsx | Computes max target speed from gear ratio multiplier and uses it in the slider. |
| const setPullerGearRatio = (gearRatio: GearRatio) => { | ||
| updateStateOptimistically( | ||
| (current) => { | ||
| current.data.puller_state.gear_ratio = gearRatio; | ||
| // Reset target speed to 0 to prevent sudden speed changes | ||
| current.data.puller_state.target_speed = 0; | ||
| }, | ||
| async () => { | ||
| // First set the gear ratio | ||
| await requestPullerSetGearRatio({ | ||
| machine_identification_unique: machineIdentification, | ||
| data: { SetPullerGearRatio: gearRatio }, | ||
| }); | ||
| // Then set the target speed to 0 | ||
| await requestPullerSetTargetSpeed({ | ||
| machine_identification_unique: machineIdentification, | ||
| data: { SetPullerTargetSpeed: 0 }, | ||
| }); | ||
| }, | ||
| ); | ||
| }; | ||
|
|
There was a problem hiding this comment.
serverRequest is declared as () => void in updateStateOptimistically, but an async () => Promise is passed here, which violates the type and will cause a TS error. Also, sending SetPullerGearRatio before zeroing the target speed can momentarily spike motor speed. Wrap the async sequence in a void IIFE and reverse the order so the target speed is set to 0 first, then the gear ratio is changed.
| setPullerTargetSpeed(preset.data?.puller_state?.target_speed ?? 1.0); | ||
| setPullerGearRatio(preset.data?.puller_state?.gear_ratio ?? "OneToOne"); |
There was a problem hiding this comment.
setPullerGearRatio triggers a follow-up request that resets target speed to 0; calling it after setPullerTargetSpeed can leave the machine at 0 instead of the preset speed. Call setPullerGearRatio before setPullerTargetSpeed so the final command sets the intended preset speed.
| setPullerTargetSpeed(preset.data?.puller_state?.target_speed ?? 1.0); | |
| setPullerGearRatio(preset.data?.puller_state?.gear_ratio ?? "OneToOne"); | |
| setPullerGearRatio(preset.data?.puller_state?.gear_ratio ?? "OneToOne"); | |
| setPullerTargetSpeed(preset.data?.puller_state?.target_speed ?? 1.0); |
| const maxMotorSpeed = 75; // Maximum motor speed in m/min | ||
| const maxTargetSpeed = maxMotorSpeed / gearRatioMultiplier; |
There was a problem hiding this comment.
Avoid the magic number 75; surface this value from backend state, configuration, or a shared constant to keep UI limits consistent with machine capabilities.
| onChange={(value) => | ||
| setPullerGearRatio( | ||
| value as "OneToOne" | "OneToFive" | "OneToTen", | ||
| ) | ||
| } |
There was a problem hiding this comment.
Avoid duplicating the string literal union; use the shared GearRatio type to keep types in sync (e.g., cast to GearRatio or type onChange accordingly).
| pub const fn set_gear_ratio(&mut self, gear_ratio: GearRatio) { | ||
| self.gear_ratio = gear_ratio; | ||
| } | ||
|
|
||
| pub const fn get_gear_ratio(&self) -> GearRatio { | ||
| self.gear_ratio | ||
| } |
There was a problem hiding this comment.
const fn on these methods provides no practical benefit and is unusual for a mutating setter; consider removing const to avoid implying they are usable in const contexts.
| // Apply gear ratio multiplier | ||
| let speed = base_speed * self.gear_ratio.multiplier(); | ||
|
|
There was a problem hiding this comment.
Please add unit tests covering gear ratio application in update_speed (e.g., verifying that a base target speed results in motor speed scaled by 1:1, 1:5, 1:10, and direction handling) to prevent regressions.
| // Divide by gear ratio to get actual puller/material speed | ||
| let puller_speed = motor_speed / self.puller_speed_controller.get_gear_ratio().multiplier(); | ||
|
|
There was a problem hiding this comment.
Add tests asserting that emitted puller_speed is motor_speed divided by the selected gear ratio (and matches 1.0 for OneToOne) to validate the telemetry conversion.
|
Code looks good. Needs test on machine tho |
- Introduced gear ratio enum with options for 1:5 and 1:10. - Updated puller speed controller to include gear ratio state. - Implemented gear ratio selection in Winder2Settings component. - Added corresponding mutation and state management in useWinder2 hook.
…ed calculation based on gear ratio
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
64bb86c to
9357960
Compare
This pull request adds support for selecting and managing the puller gear ratio on the Winder2 machine, ensuring that speed calculations and UI controls reflect the chosen gear ratio. The changes span both the frontend (TypeScript/React) and backend (Rust) codebases, introducing a new
GearRatioenum, updating state management, and adjusting speed calculations to account for the gear ratio.Backend (Rust) changes:
Gear Ratio support in machine logic:
GearRatioenum and related methods topuller_speed_controller.rs, including a multiplier function and default implementation. The gear ratio is now part of thePullerSpeedControllerstate, with getter and setter methods. Speed calculations now apply the gear ratio multiplier, and the gear ratio is included in state emission. [1] [2] [3] [4] [5] [6]PullerStatestruct. The main machine logic routes the new mutation to the appropriate handler. [1] [2] [3] [4] [5]Frontend (TypeScript/React) changes:
State and schema updates:
gearRatioSchemaandGearRatiotype inwinder2Namespace.ts, along with a utility function for obtaining the gear ratio multiplier. The puller state schema now includesgear_ratio. [1] [2]useWinder2hook now manages the gear ratio, including an optimistic state update and API call for changing the gear ratio. When the gear ratio is changed, the target speed is reset to zero to prevent sudden speed changes. [1] [2] [3] [4]UI and control improvements:
SelectionGroupcontrol. [1] [2]Summary of most important changes:
Backend: Gear Ratio logic and API
GearRatioenum and multiplier logic to backend, updated speed calculations to use the gear ratio, and included gear ratio in machine state and API mutations. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]Frontend: State management and schema
GearRatiotype and schema, updated puller state, and extended theuseWinder2hook to support gear ratio changes and ensure safe speed resets. [1] [2] [3] [4] [5] [6]Frontend: UI enhancements
Frontend: Safety and control