Sfr algae #81
Sfr algae #81
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…2025 into sfr-comp-bot
…ic instead of in setGoalState. Still needs testing.
WalkthroughThis pull request updates version control constants in BuildConstants, adjusts vision and distance constants in Constants, and integrates new algae management subsystems into the robot. It revises the default states for algae systems in RobotState, enhances pivot control with PID integration, and refines roller state management. Additionally, a new command (DeployRunAlgae) and updated button mappings in RobotContainer have been introduced, alongside modifications to the climber’s state condition. Changes
Sequence Diagram(s)sequenceDiagram
participant O as Operator
participant RC as RobotContainer
participant CMD as DeployRunAlgae
participant AP as AlgaePivot
participant AR as AlgaeRoller
O->>RC: Press D-pad button
RC->>CMD: Schedule DeployRunAlgae
CMD->>AP: setGoalState(DEPLOY)
CMD->>AR: setGoalState(SPINNING)
Note over CMD, AR: Command remains active
CMD-->>AR: On end/interrupt, setGoalState(STOPPED)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/main/java/org/team5924/frc2025/commands/algae/DeployRunAlgae.java (1)
49-53: Good cleanup in end method.The end method properly stops the roller when the command ends or is interrupted, which is essential for safe operation. However, consider whether the pivot should also be reset to STOW when the command ends.
@Override public void end(boolean interrupted) { algaeRoller.setGoalState(AlgaeRollerState.STOPPED); + algaePivot.setGoalState(AlgaePivotState.STOW); }src/main/java/org/team5924/frc2025/subsystems/pivot/AlgaePivotIOTalonFX.java (2)
66-67: New PID controller implementation with fixed gains.The PID controller is created with fixed gains (3, 0, 0) which means proportional-only control. Consider using the tunable numbers already defined in the class instead of hardcoded values to allow for runtime tuning.
-private final PIDController pid = new PIDController(3, 0, 0); +private final PIDController pid = new PIDController( + algaePivotMotorkP.getAsDouble(), + algaePivotMotorkI.getAsDouble(), + algaePivotMotorkD.getAsDouble());
131-132: Added motor position re-zeroing during initialization.This is a good practice to ensure consistent behavior across robot restarts, especially for mechanisms without absolute encoders. However, consider adding a log statement to confirm the zeroing was successful.
// re-zeros the algae pivot motor on initialiation algaePivotTalon.setPosition(0); +Logger.recordOutput("AlgaePivotIOTalonFX/InitializationZeroSet", true);src/main/java/org/team5924/frc2025/RobotContainer.java (1)
332-354: Added button mappings for algae subsystems.The button mappings for the algae pivot and roller are well-organized:
- POV Up (0): Deploy the algae pivot
- POV Down (180): Stow the algae pivot
- POV Left (270): Toggle the algae roller between spinning and stopped
This provides intuitive controls for the operator. The implementation correctly handles both button press and release states for the roller.
Consider adding a comment above line 346 to indicate that this section is for the Algae Roller, similar to the comment on line 332 for the Algae Pivot.
src/main/java/org/team5924/frc2025/subsystems/pivot/AlgaePivot.java (1)
37-40: Simplified AlgaePivotState to two essential states.The AlgaePivotState enum has been simplified from multiple states to just DEPLOY and STOW, which helps streamline the control logic. However, the LoggedTunableNumber names don't match the state names, which could be confusing.
public enum AlgaePivotState { - DEPLOY(new LoggedTunableNumber("AlgaePivotIntakeHighRads", -13.5)), - STOW(new LoggedTunableNumber("AlgaePivotIntakeLowRads", .1)); + DEPLOY(new LoggedTunableNumber("AlgaePivotDeployRads", -13.5)), + STOW(new LoggedTunableNumber("AlgaePivotStowRads", .1));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/org/team5924/frc2025/BuildConstants.java(1 hunks)src/main/java/org/team5924/frc2025/Constants.java(3 hunks)src/main/java/org/team5924/frc2025/RobotContainer.java(6 hunks)src/main/java/org/team5924/frc2025/RobotState.java(1 hunks)src/main/java/org/team5924/frc2025/commands/algae/DeployRunAlgae.java(1 hunks)src/main/java/org/team5924/frc2025/subsystems/climber/Climber.java(1 hunks)src/main/java/org/team5924/frc2025/subsystems/pivot/AlgaePivot.java(4 hunks)src/main/java/org/team5924/frc2025/subsystems/pivot/AlgaePivotIOTalonFX.java(4 hunks)src/main/java/org/team5924/frc2025/subsystems/rollers/Algae/AlgaeRoller.java(2 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
src/main/java/org/team5924/frc2025/subsystems/pivot/AlgaePivotIOTalonFX.java (1)
src/main/java/org/team5924/frc2025/Constants.java (1)
Constants(41-405)
src/main/java/org/team5924/frc2025/RobotState.java (2)
src/main/java/org/team5924/frc2025/subsystems/climber/Climber.java (1)
Setter(31-172)src/main/java/org/team5924/frc2025/subsystems/rollers/Algae/AlgaeRoller.java (1)
Setter(28-83)
src/main/java/org/team5924/frc2025/subsystems/pivot/AlgaePivot.java (1)
src/main/java/org/team5924/frc2025/Constants.java (1)
Constants(41-405)
🔇 Additional comments (23)
src/main/java/org/team5924/frc2025/subsystems/climber/Climber.java (1)
87-93: Updated condition aligns with new AlgaePivot state management.The condition has been updated to check for
AlgaePivotState.STOWinstead of the previousAlgaePivotState.INTAKE_FLOOR. This change properly aligns with the updated state management in the AlgaePivot subsystem and RobotState class.src/main/java/org/team5924/frc2025/RobotState.java (2)
74-76: Default AlgaePivotState updated to STOW.This change properly aligns with the modifications in the AlgaePivot class and ensures consistency with the climber subsystem's condition checking. Good update to maintain state coherence across subsystems.
79-81: Default AlgaeRollerState renamed to STOPPED.The updated default state aligns with the renamed states in the AlgaeRoller class. This ensures proper initialization and consistent state management across the robot's subsystems.
src/main/java/org/team5924/frc2025/subsystems/rollers/Algae/AlgaeRoller.java (3)
34-41: Renamed AlgaeRollerState enum values for clarity.The state names have been updated from
NO_ALGAE/INTAKINGtoSTOPPED/SPINNING, which better represents their functional meaning. This improves code readability and provides more intuitive state management.
46-46: Updated default goalState to match renamed enum.Default state initialization correctly updated to use the renamed
STOPPEDstate instead of the previousNO_ALGAE.
66-69: Updated switch case logic to use new state names.The switch cases have been properly updated to use the new state names, ensuring consistent state management between the AlgaeRoller and RobotState classes.
src/main/java/org/team5924/frc2025/commands/algae/DeployRunAlgae.java (3)
25-36: New command class for algae management.This command correctly manages both the AlgaeRoller and AlgaePivot subsystems and properly declares its requirements, ensuring command scheduler exclusivity when running.
38-43: Well-structured initialization method.The initialize method correctly sets up both subsystems for algae operation by deploying the pivot and spinning the roller.
55-59: Command runs until explicitly interrupted.The command is designed to run indefinitely until interrupted, which is appropriate for this type of operation where the algae system needs to remain active until manually stopped.
src/main/java/org/team5924/frc2025/subsystems/pivot/AlgaePivotIOTalonFX.java (2)
37-37: Added PIDController import for new control implementation.The import is necessary for the new positional control strategy that's being implemented.
68-68:Details
❓ Verification inconclusive
Updated kP value for improved position control.
The proportional gain has been updated from 0 to 2, which should provide better position-holding capability. This seems reasonable, but note this value doesn't appear to be used in the new PID implementation (hardcoded value is used instead).
Verify that changing this value has the expected effect on the system behavior:
🏁 Script executed:
#!/bin/bash # Search for places where algaePivotMotorkP is used rg "algaePivotMotorkP" -A 3 -B 3Length of output: 1934
Action: Verify proper usage of the updated kP tunable value in the PID implementation
- In
src/main/java/org/team5924/frc2025/subsystems/pivot/AlgaePivotIOTalonFX.java, the tunable numberalgaePivotMotorkPhas been updated to a default value of 2, and it is later applied tocontrollerConfig.kP.- However, the PIDController is instantiated using a hardcoded value of 3 (
new PIDController(3, 0, 0)), which may override or delay the effect of the tunable value.- Please verify that the current design correctly incorporates the updated kP value into the active PID configuration. If the intent is for the tunable value to be used during PIDController instantiation, consider refactoring to pass
algaePivotMotorkP.getAsDouble()directly to the constructor or ensure that post-construction updates are handled appropriately.src/main/java/org/team5924/frc2025/RobotContainer.java (2)
53-54: Added imports for algae management subsystems.Clean addition of the necessary imports for the new algae subsystems.
Also applies to: 59-60
78-79: Added new algae subsystem instances.Good declaration of the AlgaePivot and AlgaeRoller subsystems as private final fields, consistent with other subsystems in the class.
src/main/java/org/team5924/frc2025/subsystems/pivot/AlgaePivot.java (6)
57-57: Updated default state to STOW.Changed the default goal state to STOW, which aligns with the simplified state model and makes sense as a starting position for the mechanism.
74-75: Added additional logging for pivot control.Good addition of logging for the difference in radians and whether the pivot is at the setpoint, which will be helpful for debugging and tuning.
79-80: Improved position control efficiency.The position control is now only called when the pivot is not at the setpoint, which is more efficient. This prevents unnecessary position commands when the pivot is already where it needs to be.
85-90: Added helper method for calculating position difference.Good refactoring to create a helper method for calculating the difference between the goal and current positions, which improves code readability and maintainability.
97-97: Updated setpoint checking to use new helper method.Modified the setpoint check to use the new getDifferenceRads method, which makes the code more consistent and maintainable.
107-112: Simplified state management for RobotState.Updated the setGoalState method to handle just the two new states, making it cleaner and more concise. The implementation correctly updates the RobotState singleton.
src/main/java/org/team5924/frc2025/BuildConstants.java (1)
24-29: Updated version control constants.These automated updates to the build constants reflect the latest changes in the codebase. The GIT_BRANCH value "sfr-algae" aligns with the PR name and purpose.
src/main/java/org/team5924/frc2025/Constants.java (3)
179-179: Verify shooting position adjustment impactThe distance from center to robot center for shooting has been increased by 10cm (from 1.22m to 1.32m). This will affect all calculated shooting positions around the reef.
Ensure this adjustment has been tested with the algae subsystem to confirm it improves shooting accuracy. The distance change will move all shooting positions further from the reef centers.
313-329: Good consistency improvement in red alliance calculationsAdding the
offsetCorrectionparameter to the red alliance branch shoot calculations brings consistency with the blue alliance calculations. This ensures both alliances have the same behavior.
139-143:Details
❓ Verification inconclusive
Significant camera offset changes require verification
The back Limelight camera offsets have been substantially updated, with particularly notable changes to:
- Forward offset changing from positive to negative direction
- Vertical height increase of approximately 9.8 inches (from 16.145" to 25.955")
- Pitch angle tripling from 15° to 45°
These changes will significantly affect vision-based pose estimation. Ensure these values have been calibrated and validated with real-world testing.
🏁 Script executed:
#!/bin/bash # Check for usage of these constants to understand impact echo "Checking for usage of Limelight constants in the codebase..." rg -A 2 "BACK_LIMELIGHT_OFF_" --type javaLength of output: 2022
Verify Updated Vision Offset Constants with Real-World Calibration
The updates in the back Limelight offsets are confirmed to be in active use (see their usage in
src/main/java/org/team5924/frc2025/subsystems/vision/VisionIOLimelight.java). However, these significant changes—especially the reversal of the forward offset, the increased vertical offset, and the tripled pitch angle—can substantially affect vision-based pose estimation. Please ensure that these new values are rigorously validated with real-world testing and calibration.
- Check: Confirm that the updated offsets correctly reflect the actual hardware configuration.
- Action: Re-validate the vision pose estimation accuracy during on-field tests.
| public void setPosition(double rads) { | ||
| algaePivotTalon.setControl(positionControl.withPosition(rads)); | ||
| algaePivotTalon.setVoltage( | ||
| pid.calculate( | ||
| algaePivotPosition.getValue().in(Radians) / Constants.MOTOR_TO_ALGAE_PIVOT_REDUCTION, | ||
| rads)); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Replaced built-in position control with custom PID implementation.
This change replaces the TalonFX's built-in position control with a custom PID controller. This gives you more direct control over the motor voltage but bypasses some of the built-in safety features and optimization of the motor controller.
Some considerations:
- The calculation correctly converts the motor position to pivot position using the reduction ratio
- The implementation is simpler and more direct
However, this implementation doesn't use any feedforward terms, which may impact control performance, especially under varying loads.
Consider adding a small feedforward term or using the built-in kS parameter:
algaePivotTalon.setVoltage(
pid.calculate(
algaePivotPosition.getValue().in(Radians) / Constants.MOTOR_TO_ALGAE_PIVOT_REDUCTION,
- rads));
+ rads) + Math.signum(rads - algaePivotPosition.getValue().in(Radians) / Constants.MOTOR_TO_ALGAE_PIVOT_REDUCTION) * algaePivotMotorkS.getAsDouble());Verify that the pivot movement is smooth and accurate with this control approach:
🏁 Script executed:
#!/bin/bash
# Check how this position control method is used throughout the codebase
rg "algaePivot.*setPosition" -A 2 -B 2Length of output: 522
Feedforward Addition Recommended for Custom PID in AlgaePivot
The conversion from motor position to pivot position is correct and the custom PID implementation is more direct. However, bypassing the TalonFX’s built-in safety and feedforward features might affect performance under varying loads. We recommend testing the following change to incorporate a minimal feedforward term, which could improve the control response:
algaePivotTalon.setVoltage(
pid.calculate(
algaePivotPosition.getValue().in(Radians) / Constants.MOTOR_TO_ALGAE_PIVOT_REDUCTION,
- rads));
+ rads) + Math.signum(rads - algaePivotPosition.getValue().in(Radians) / Constants.MOTOR_TO_ALGAE_PIVOT_REDUCTION) * algaePivotMotorkS.getAsDouble());Notes:
- Our grep output indicates that
setPositionis primarily used for zeroing the motor. Make sure that the change supports both initialization and dynamic pivot control. - Verify that the pivot movement remains smooth and accurate, especially under load variations, after incorporating the feedforward term.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void setPosition(double rads) { | |
| algaePivotTalon.setControl(positionControl.withPosition(rads)); | |
| algaePivotTalon.setVoltage( | |
| pid.calculate( | |
| algaePivotPosition.getValue().in(Radians) / Constants.MOTOR_TO_ALGAE_PIVOT_REDUCTION, | |
| rads)); | |
| public void setPosition(double rads) { | |
| algaePivotTalon.setVoltage( | |
| pid.calculate( | |
| algaePivotPosition.getValue().in(Radians) / Constants.MOTOR_TO_ALGAE_PIVOT_REDUCTION, | |
| rads) + Math.signum(rads - algaePivotPosition.getValue().in(Radians) / Constants.MOTOR_TO_ALGAE_PIVOT_REDUCTION) * algaePivotMotorkS.getAsDouble()); | |
| } |
| algaePivot = new AlgaePivot(new AlgaePivotIOTalonFX()); | ||
| algaeRoller = new AlgaeRoller(new AlgaeRollerIOKrakenFOC()); |
There was a problem hiding this comment.
Initialized algae subsystems for all robot modes.
The AlgaePivot and AlgaeRoller subsystems are consistently initialized across all operation modes (REAL, SIM, DEFAULT). However, in SIM and DEFAULT modes, you're still using the hardware IO implementations which may not be appropriate for simulation.
Consider creating sim-specific IO implementations for these subsystems:
// For SIM mode
-algaePivot = new AlgaePivot(new AlgaePivotIOTalonFX());
-algaeRoller = new AlgaeRoller(new AlgaeRollerIOKrakenFOC());
+algaePivot = new AlgaePivot(new AlgaePivotIOSim()); // Create this class
+algaeRoller = new AlgaeRoller(new AlgaeRollerIOSim()); // Create this class
// For DEFAULT mode
-algaePivot = new AlgaePivot(new AlgaePivotIOTalonFX());
-algaeRoller = new AlgaeRoller(new AlgaeRollerIOKrakenFOC());
+algaePivot = new AlgaePivot(new AlgaePivotIO() {});
+algaeRoller = new AlgaeRoller(new AlgaeRollerIO() {});Also applies to: 122-123, 139-140
Summary by CodeRabbit
Chores
New Features
Refactor