made zeroing for intake and hood - #165
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements motor zeroing functionality for the intake pivot and hood subsystems in an FRC robot. Zeroing is a critical initialization procedure that establishes a known reference position for mechanisms without absolute encoders by driving them to a hard stop and resetting the sensor position.
Changes:
- Added support for both automatic and manual zeroing modes with velocity-based stall detection
- Integrated zeroing commands into robot lifecycle (autonomous, teleop, and disabled modes)
- Extended Motion subsystem with voltage control, sensor position reset, and software limit management
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| Motion.java | Added voltage control methods, velocity getters, sensor position reset, software limit control, and zeroing state flags |
| ConstMotion.java | Defined zeroing constants including voltages, velocity thresholds, timeouts, and zero positions (with TODOs) |
| ZeroIntake.java | New command for automatic intake pivot zeroing using velocity-based stall detection |
| ZeroHood.java | New command for automatic hood zeroing using velocity-based stall detection |
| ManualZeroIntake.java | New command for manual intake pivot zeroing with user-controlled motion and delta velocity detection |
| ManualZeroHood.java | New command for manual hood zeroing with user-controlled motion and delta velocity detection |
| RobotContainer.java | Created composed zeroing commands (zeroSubsystems and manualZeroSubsystems) and added allZeroed() helper |
| Robot.java | Integrated zeroing into robot lifecycle: manual zeroing in disabled mode, automatic zeroing before autonomous/teleop if needed |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public void setIntakePivotSoftwareLimits(boolean reverseLimitEnable, boolean forwardLimitEnable) { | ||
| ConstMotion.INTAKE_PIVOT_CONFIGURATION.SoftwareLimitSwitch.ReverseSoftLimitEnable = reverseLimitEnable; | ||
| ConstMotion.INTAKE_PIVOT_CONFIGURATION.SoftwareLimitSwitch.ForwardSoftLimitEnable = forwardLimitEnable; | ||
| intakePivot.getConfigurator().apply(ConstMotion.INTAKE_PIVOT_CONFIGURATION); | ||
| } |
There was a problem hiding this comment.
Modifying shared configuration objects is not thread-safe and may have unintended side effects. The methods directly mutate ConstMotion.INTAKE_PIVOT_CONFIGURATION which is a shared constant. If multiple instances or threads access this, it could lead to race conditions. Consider creating a copy of the configuration, modifying it, and then applying it to avoid mutating shared state.
| boolean deltaRotorVelocity = RobotContainer.motionInstance.getIntakePivotRotorVelocity() | ||
| .minus(lastRotorVelocity) | ||
| .lte(ConstMotion.MANUAL_ZEROING_DELTA_VELOCITY); | ||
|
|
||
| if (deltaRotorVelocity && lastRotorVelocity.lte(Units.RotationsPerSecond.of(0))) { | ||
| zeroingSuccess = true; | ||
| } else { | ||
| lastRotorVelocity = RobotContainer.motionInstance.getIntakePivotRotorVelocity(); |
There was a problem hiding this comment.
The logic for detecting zeroing completion appears incorrect. The condition checks if the velocity delta is less than or equal to MANUAL_ZEROING_DELTA_VELOCITY and if lastRotorVelocity is less than or equal to 0. However, the delta calculation on line 53-55 computes (current - last), which will be negative if velocity is decreasing. This means deltaRotorVelocity will be true when velocity decreases by more than 5 RPS, not when it's stable. Consider using the absolute value of the delta or reversing the logic to properly detect when the mechanism has stalled against a hard stop.
| boolean deltaRotorVelocity = RobotContainer.motionInstance.getIntakePivotRotorVelocity() | |
| .minus(lastRotorVelocity) | |
| .lte(ConstMotion.MANUAL_ZEROING_DELTA_VELOCITY); | |
| if (deltaRotorVelocity && lastRotorVelocity.lte(Units.RotationsPerSecond.of(0))) { | |
| zeroingSuccess = true; | |
| } else { | |
| lastRotorVelocity = RobotContainer.motionInstance.getIntakePivotRotorVelocity(); | |
| AngularVelocity currentRotorVelocity = RobotContainer.motionInstance.getIntakePivotRotorVelocity(); | |
| boolean deltaRotorVelocity = lastRotorVelocity | |
| .minus(currentRotorVelocity) | |
| .lte(ConstMotion.MANUAL_ZEROING_DELTA_VELOCITY); | |
| if (deltaRotorVelocity && lastRotorVelocity.lte(Units.RotationsPerSecond.of(0))) { | |
| zeroingSuccess = true; | |
| } else { | |
| lastRotorVelocity = currentRotorVelocity; |
| boolean deltaRotorVelocity = RobotContainer.motionInstance.getHoodRotorVelocity() | ||
| .minus(lastRotorVelocity) | ||
| .lte(ConstMotion.MANUAL_ZEROING_DELTA_VELOCITY); | ||
|
|
||
| if (deltaRotorVelocity && lastRotorVelocity.lte(Units.RotationsPerSecond.of(0))) { | ||
| zeroingSuccess = true; | ||
| } else { | ||
| lastRotorVelocity = RobotContainer.motionInstance.getHoodRotorVelocity(); |
There was a problem hiding this comment.
The logic for detecting zeroing completion appears incorrect. The condition checks if the velocity delta is less than or equal to MANUAL_ZEROING_DELTA_VELOCITY and if lastRotorVelocity is less than or equal to 0. However, the delta calculation on line 53-55 computes (current - last), which will be negative if velocity is decreasing. This means deltaRotorVelocity will be true when velocity decreases by more than 5 RPS, not when it's stable. Consider using the absolute value of the delta or reversing the logic to properly detect when the mechanism has stalled against a hard stop.
| boolean deltaRotorVelocity = RobotContainer.motionInstance.getHoodRotorVelocity() | |
| .minus(lastRotorVelocity) | |
| .lte(ConstMotion.MANUAL_ZEROING_DELTA_VELOCITY); | |
| if (deltaRotorVelocity && lastRotorVelocity.lte(Units.RotationsPerSecond.of(0))) { | |
| zeroingSuccess = true; | |
| } else { | |
| lastRotorVelocity = RobotContainer.motionInstance.getHoodRotorVelocity(); | |
| AngularVelocity currentRotorVelocity = RobotContainer.motionInstance.getHoodRotorVelocity(); | |
| AngularVelocity deltaRotorVelocityMeasure = lastRotorVelocity.minus(currentRotorVelocity); | |
| boolean deltaRotorVelocity = deltaRotorVelocityMeasure | |
| .lte(ConstMotion.MANUAL_ZEROING_DELTA_VELOCITY); | |
| if (deltaRotorVelocity && lastRotorVelocity.lte(Units.RotationsPerSecond.of(0))) { | |
| zeroingSuccess = true; | |
| } else { | |
| lastRotorVelocity = currentRotorVelocity; |
| () -> stateMachineInstance.tryState(RobotState.NONE)); | ||
|
|
||
| Command zeroSubsystems = new ParallelCommandGroup( | ||
| new ZeroHood().withTimeout(ConstMotion.ZEROING_TIMEOUT.in(Units.Seconds)), |
There was a problem hiding this comment.
Missing condition for intake pivot zeroing. Unlike ZeroHood, ZeroIntake includes an onlyIf check to skip zeroing if already zeroed (line 100). However, ZeroHood at line 99 has no such check and will always attempt to zero even if hasHoodZeroed is true. This creates an inconsistency and unnecessary wear on the hood mechanism. Consider adding .onlyIf(() -> !RobotContainer.motionInstance.hasHoodZeroed) to the ZeroHood command for consistency.
| new ZeroHood().withTimeout(ConstMotion.ZEROING_TIMEOUT.in(Units.Seconds)), | |
| new ZeroHood().onlyIf(() -> !RobotContainer.motionInstance.hasHoodZeroed) | |
| .withTimeout(ConstMotion.ZEROING_TIMEOUT.in(Units.Seconds)), |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Pull request was converted to draft
No description provided.