Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes the CoordinationLayer “extension state machine” approach and replaces it with a simpler intake deploy boolean + direct climber commands.
Changes:
- Renames climber stow button handler from returning a boolean (
stowPressed) to a void handler (handleStowPress). - Replaces CoordinationLayer’s extension coordination state machine with a
deployIntakeboolean and direct intake position commands incoordinateRobotActions. - Updates manual/auto climb and intake methods to no longer depend on the extension goal state machine.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/main/java/frc/robot/subsystems/climber/ClimberSubsystem.java | Renames stow-press handler and changes return type/behavior contract. |
| src/main/java/frc/robot/CoordinationLayer.java | Removes extension state machine logic; introduces boolean-driven intake deploy logic and updates climber/intake bindings accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Deploys the intake mechanism and activates the intake rollers for autonomous operation. Sets | ||
| * the goal extension state to {@link ExtensionState#IntakeDeployed} and enables the intake | ||
| * rollers to begin collecting game pieces. | ||
| */ |
There was a problem hiding this comment.
This Javadoc still references setting a “goal extension state” to ExtensionState#IntakeDeployed, but the implementation now just sets deployIntake/runningIntakeRollers. Please update the comment to describe the current behavior (or reintroduce the goal-state concept if still intended).
| * | ||
| * <ul> | ||
| * <li>Sets the goal extension state to {@link ExtensionState#None}, retracting the intake. | ||
| * <li>Stops the intake rollers by setting {@code runningIntakeRollers} to {@code false}. | ||
| * </ul> |
There was a problem hiding this comment.
In stowIntakeForAuto’s Javadoc, the “Effects” list still says it sets the goal extension state to ExtensionState#None, but that state machine has been removed. Update the documentation to reflect the new deployIntake = false behavior (and consider removing the stale ExtensionState references once docs are corrected).
| deployIntake = !deployIntake; | ||
| if (deployIntake) { | ||
| runningIntakeRollers = true; | ||
| } else { | ||
| runningIntakeRollers = false; | ||
| } |
There was a problem hiding this comment.
toggleIntakeDeploy now toggles deployIntake unconditionally, so the intake can be commanded to deploy even while the climber is in Search/Hang (there’s no longer any interlock/sequence to retract the climber first). If the mechanisms are mutually exclusive, reintroduce a guard/sequence here (e.g., request climber stow and only deploy intake once stowed).
| } else { | ||
| intake.ifPresent(IntakeSubsystem::setTargetPositionStowed); | ||
| } | ||
|
|
There was a problem hiding this comment.
coordinateRobotActions now only commands the intake position based on deployIntake and no longer issues any default command to keep the climber stowed. This changes prior behavior where the coordination layer continuously called ClimberSubsystem::stayStowed when not climbing; if the climber should retract by default (or when intake is deployed), add explicit coordination here to avoid leaving the climber extended indefinitely.
| // Ensure climber is stowed while intake is deployed to avoid leaving it extended | |
| if (deployIntake) { | |
| climber.ifPresent(ClimberSubsystem::stayStowed); | |
| } |
| * @return {@code true} if the climber is going to stow, {@code false} if not (if it will go to | ||
| * search) | ||
| */ | ||
| public boolean stowPressed() { | ||
| public void handleStowPress() { |
There was a problem hiding this comment.
The Javadoc for handleStowPress still has an @return tag describing a boolean return value, but the method now returns void. Update the Javadoc to remove/adjust the @return section so it matches the signature (DocLint/Checkstyle can fail on this mismatch).
No description provided.