15 basic shooter roller and hood with superstructure#26
Merged
Conversation
…tainer with a trigger
Contributor
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds new shooter subsystems and wiring: ShooterRoller and ShooterHood (types, IO interfaces, TalonFX and sim IOs), a SuperShooter coordinator mapping high-level shooter states to hood/roller goals, constants/config updates, and RobotState additions. RobotContainer only received whitespace edits. Changes
Sequence Diagram(s)sequenceDiagram
participant Driver as "Driver"
participant RC as "RobotContainer"
participant SS as "SuperShooter"
participant SR as "ShooterRoller"
participant SH as "ShooterHood"
participant MH as "Motor HW"
Driver->>RC: request AUTO_SHOOTING
RC->>SS: setGoalState(AUTO_SHOOTING)
SS->>SR: setGoalState(AUTO_SHOOTING)
SS->>SH: setGoalState(AUTO_SHOOTING)
SR->>MH: runVolts(shooter_volts)
SH->>MH: runVolts(hood_volts)
MH-->>SR: shooter feedback (velocity/connection)
MH-->>SH: hood feedback (position/velocity/temperature)
SR->>RC: update RobotState.shooterRollerState
SH->>RC: update RobotState.shooterHoodState
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@src/main/java/org/team5924/frc2026/Constants.java`:
- Around line 128-171: The ShooterHood and ShooterRoller hardware constants
currently use 0 for CAN_ID and BEAM_BREAK_PORT which will collide with other
devices; update the static fields (ShooterHood.CAN_ID, ShooterRoller.CAN_ID,
ShooterRoller.BEAM_BREAK_PORT and any other port constants in those classes) to
the actual hardware CAN IDs/DIO port numbers used on the robot, ensure each
value is unique across the Constants file, and if the real value is not yet
known use a clear sentinel (e.g., -1) or add a TODO comment so deployment cannot
accidentally use 0.
In
`@src/main/java/org/team5924/frc2026/subsystems/rollers/shooterRoller/ShooterRoller.java`:
- Around line 32-49: The static field beamBreak is initialized only in the
ShooterRoller constructor which can lead to an NPE if isGamePieceDetected() (or
any static access to beamBreak) is called before an instance is constructed; fix
by guarding access or changing initialization: either make beamBreak non-static
and initialize it in the ShooterRoller constructor, or keep it static but lazily
initialize it (e.g., check for null in isGamePieceDetected() and create new
DigitalInput(Constants.ShooterRoller.BEAM_BREAK_PORT) if needed) and ensure any
close/cleanup semantics are handled; update references to beamBreak and the
ShooterRoller constructor accordingly so all usages are safe.
- Around line 51-55: periodic() in ShooterRoller currently calls
((ShooterRollerIO) io).runVolts(goalState.getVoltageSupplier().getAsDouble())
and then calls super.periodic(), which causes duplicate voltage commands; remove
the explicit io.runVolts(...) call from ShooterRoller.periodic and let
super.periodic() perform the single runVolts call (or if the superclass needs
the voltage value, pass goalState.getVoltageSupplier().getAsDouble() into the
superclass pathway instead), referencing ShooterRoller.periodic, io.runVolts,
goalState.getVoltageSupplier, and super.periodic when making the change.
In
`@src/main/java/org/team5924/frc2026/subsystems/rollers/shooterRoller/ShooterRollerIOSim.java`:
- Around line 25-28: Replace the hardcoded simulation parameters that reference
Constants.ExampleRoller with the shooter-specific constants so the sim matches
the real subsystem: in ShooterRollerIOSim's constructor (the super(...) call
that currently uses Constants.ExampleRoller.REDUCTION and
Constants.ExampleRoller.SIM_MOI) swap those references to the corresponding
ShooterRoller constants (e.g., ShooterRoller.REDUCTION and ShooterRoller.SIM_MOI
or whatever static names are defined on the ShooterRoller class), and add an
import for ShooterRoller if necessary.
In `@src/main/java/org/team5924/frc2026/subsystems/shooterHood/ShooterHood.java`:
- Around line 98-109: In setGoalState (ShooterHood.java) the default branch
always sets RobotState.getInstance().setShooterHoodState(ShooterHoodState.OFF),
causing AUTO/BUMPER/SHUFFLING commands to be reported as OFF; change the logic
so that for non-MANUAL and non-MOVING cases you set RobotState to the requested
goalState (e.g., RobotState.getInstance().setShooterHoodState(goalState)) before
calling io.setPosition(goalState.rads.getAsDouble()), keep the MOVING branch
error/reporting as-is, and ensure enums like AUTO/BUMPER/SHUFFLING are preserved
when updating RobotState.
In
`@src/main/java/org/team5924/frc2026/subsystems/shooterHood/ShooterHoodIOSim.java`:
- Around line 31-36: The sim in ShooterHoodIOSim constructor uses
Constants.Example.SIM_MOI and Constants.Example.REDUCTION which can diverge from
the hood's real parameters; update the call to
LinearSystemId.createDCMotorSystem inside the ShooterHoodIOSim() constructor to
use the ShooterHood-specific constants (e.g., ShooterHood.SIM_MOI and
ShooterHood.REDUCTION) so the DCMotorSim and gearbox instantiation match the
ShooterHood configuration.
In
`@src/main/java/org/team5924/frc2026/subsystems/shooterHood/ShooterHoodIOTalonFX.java`:
- Around line 41-45: ShooterHoodIOTalonFX is missing an override of the
ShooterHoodIO default setPosition(double rads), so position control never uses
the PositionVoltage instance; add a public `@Override` setPosition(double rads)
method in ShooterHoodIOTalonFX that converts the requested radians to the
motor/encoder units your PositionVoltage expects and calls
positionOut.setPosition(...) (and/or positionOut.withTarget(...) depending on
API), then ensure the motor update loop reads positionOut to apply outputs;
reference the class ShooterHoodIOTalonFX, the method setPosition(double), and
the field positionOut when implementing this fix.
In
`@src/main/java/org/team5924/frc2026/subsystems/superShooter/SuperShooter.java`:
- Around line 46-57: The code never updates or uses goalState and always
switches on the unused private field state, so initialize and use goalState: set
its default to ShooterState.OFF, add a public setter like
setGoalState(ShooterState goal) that sets this.goalState, and change periodic()
to switch on goalState (or fallback to OFF if null) instead of state so the
shooter can transition; alternatively remove the unused private field state if
you prefer to drive behavior solely from goalState.
- Around line 55-76: The switch in periodic() over state is missing break
statements, causing fall-through so every branch executes and ends up using the
HOOD_MOVING actions; update the switch in SuperShooter.periodic() to add a break
after each case block (OFF, AUTO_SHOOTING, BUMPER_SHOOTING, NEUTRAL_SHUFFLING,
OPPONENT_SHUFFLING, HOOD_MOVING) so that each branch only invokes the intended
roller.setGoalState(...) and hood.setGoalState(...) calls and then exits the
switch.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@src/main/java/org/team5924/frc2026/subsystems/rollers/shooterRoller/ShooterRoller.java`:
- Around line 32-49: The static field beamBreak is reassigned in the
ShooterRoller constructor which can leak DIO resources if multiple ShooterRoller
instances are created; fix by either making beamBreak an instance field (remove
static) and initialize it in the ShooterRoller(ShooterRollerIO) constructor, or
if it must remain static, guard re-initialization by checking if beamBreak is
null before creating a new DigitalInput and, if not null, call its close() (or
equivalent) before assigning a new DigitalInput created with
Constants.ShooterRoller.BEAM_BREAK_PORT; update any usages to reference the
chosen scope.
In `@src/main/java/org/team5924/frc2026/subsystems/shooterHood/ShooterHood.java`:
- Around line 74-76: The Logger is recording the LoggedTunableNumber object
(goalState.rads) instead of its numeric value; update the ShooterHood logging
call that writes "ShooterHood/TargetRads" to use the LoggedTunableNumber
accessor (e.g., goalState.rads.get()) so a double is logged, leaving the other
Logger.recordOutput calls unchanged.
- Around line 87-89: getShooterHoodPositionRads() is dividing
inputs.shooterHoodPositionRads by Constants.ShooterHood.REDUCTION even though
ShooterHoodIOTalonFX.updateInputs() and ShooterHoodIOSim.updateInputs() already
provide a reduced value; remove the redundant division in
getShooterHoodPositionRads() so it simply returns
inputs.shooterHoodPositionRads, which will correct comparisons in
isAtSetpoint(). Ensure only getShooterHoodPositionRads() is changed and do not
alter the updateInputs() implementations.
In
`@src/main/java/org/team5924/frc2026/subsystems/shooterHood/ShooterHoodIOTalonFX.java`:
- Around line 43-48: Position control via PositionVoltage is using the motor's
internal PID Slot0 but Constants.ShooterHood.CONFIG currently lacks Slot0 gains;
update the CONFIG used in ShooterHoodIOTalonFX (where
shooterHoodTalon.getConfigurator().apply(Constants.ShooterHood.CONFIG) is
called) to include a .withSlot0(...) block supplying tuned kP/kD/kS/kV (using
Slot0Configs and its withKP/withKD/withKS/withKV setters) so the internal PID
loop can produce non-zero position output.
♻️ Duplicate comments (1)
src/main/java/org/team5924/frc2026/Constants.java (1)
128-170: CAN/DIO IDs still at placeholder values.
ShooterHood.CAN_ID,ShooterRoller.CAN_ID, andBEAM_BREAK_PORTare all0. While the TODO comments acknowledge this, these will cause runtime collisions if deployed. Ensure real hardware assignments are made before testing on the robot.
🧹 Nitpick comments (2)
src/main/java/org/team5924/frc2026/subsystems/shooterHood/ShooterHoodIOSim.java (1)
26-59: Simulation lackssetPositionoverride.
ShooterHood.setGoalState()callsio.setPosition()for position control. Without overridingsetPositionin the sim, position commands are no-ops and the simulated hood won't move to target angles. Consider implementing position control via a simple P-controller or motion profile.♻️ Proposed implementation sketch
public class ShooterHoodIOSim implements ShooterHoodIO { private final DCMotorSim sim; private final DCMotor gearbox = DCMotor.getKrakenX60Foc(1); private double appliedVoltage = 0.0; + private double targetPositionRads = 0.0; + private static final double kP = 12.0; // Tune as needed // ... constructor and other methods ... + `@Override` + public void setPosition(double rads) { + targetPositionRads = rads; + } `@Override` public void updateInputs(ShooterHoodIOInputs inputs) { - if (DriverStation.isDisabled()) runVolts(0.0); + if (DriverStation.isDisabled()) { + runVolts(0.0); + } else { + // Simple P-control for position + double error = targetPositionRads - sim.getAngularPositionRad(); + runVolts(error * kP); + } sim.update(Constants.LOOP_PERIODIC_SECONDS); // ... rest of method }src/main/java/org/team5924/frc2026/subsystems/superShooter/SuperShooter.java (1)
31-44: Unusedradsfield inShooterStateenum.Each
ShooterStatestores aLoggedTunableNumber radsbut it's never read—theShooterHooduses its ownShooterHoodState.radsfor positioning. This creates redundant tunable entries and potential confusion.♻️ Remove unused field or document its purpose
public enum ShooterState { - OFF(new LoggedTunableNumber("Shooter/Off", Math.toRadians(0))), - AUTO_SHOOTING(new LoggedTunableNumber("Shooter/Auto_Shooting", 0)), - BUMPER_SHOOTING(new LoggedTunableNumber("Shooter/Bumper_Shooting", Math.toRadians(90))), - NEUTRAL_SHUFFLING(new LoggedTunableNumber("Shooter/Neutral_Shuffling", Math.toRadians(90))), - OPPONENT_SHUFFLING(new LoggedTunableNumber("Shooter/Opponent_Shuffling", Math.toRadians(90))), - HOOD_MOVING(new LoggedTunableNumber("Shooter/Hood_Moving", -1)); - - private final LoggedTunableNumber rads; - - ShooterState(LoggedTunableNumber rads) { - this.rads = rads; - } + OFF, + AUTO_SHOOTING, + BUMPER_SHOOTING, + NEUTRAL_SHUFFLING, + OPPONENT_SHUFFLING, + HOOD_MOVING }
…2026 into 15-shooter_roller-and-hood
thatnerdjack
requested changes
Jan 24, 2026
| private final ShooterHood hood; | ||
|
|
||
| public enum ShooterState { | ||
| OFF(new LoggedTunableNumber("Shooter/Off", Math.toRadians(0))), |
Member
There was a problem hiding this comment.
Edit: whoops forgot this was superstructure
We don't need to include tunable numbers here if there's nothing to tune. These can be ()->0.0
| .withNeutralMode(NeutralModeValue.Brake)) | ||
| .withSlot0( | ||
| new Slot0Configs() | ||
| .withKP(0) |
Member
There was a problem hiding this comment.
These should be tunable numbers, with the LoggedTunableNumber being instantiated inside ShooterHood and overriding these defaults.
TL;DR PID and related are always LoggedTunableNumbers
marisa07
requested changes
Jan 24, 2026
| private final ShooterHoodIOInputsAutoLogged inputs = new ShooterHoodIOInputsAutoLogged(); | ||
|
|
||
| public enum ShooterHoodState { | ||
| OFF(new LoggedTunableNumber("ShooterHood/Off", Math.toRadians(0))), |
Contributor
There was a problem hiding this comment.
can just use 0 instead of toRadians(0)
| private final ShooterHood hood; | ||
|
|
||
| public enum ShooterState { | ||
| OFF(new LoggedTunableNumber("Shooter/Off", Math.toRadians(0))), |
Contributor
There was a problem hiding this comment.
these default values should ideally just be set in the base classes for hood/roller
| // Dashboard inputs | ||
| private final LoggedDashboardChooser<Command> autoChooser; | ||
|
|
||
| // Trigger for Beam Break |
Contributor
There was a problem hiding this comment.
is this meant for testing while there's not a beamBreak in the system? If it's just meant to update the value, it can be done somewhere periodically without a button
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/org/team5924/frc2026/Constants.java`:
- Line 19: Remove the unused imports CANcoderConfiguration and
SensorDirectionValue from the Constants.java import list; locate the import
statements that reference CANcoderConfiguration and SensorDirectionValue and
delete those lines so the file no longer imports unused symbols.
♻️ Duplicate comments (1)
src/main/java/org/team5924/frc2026/Constants.java (1)
175-192: LGTM on configuration structure.The
ShooterRollerconfiguration follows the established pattern. The CAN ID and beam break port placeholders have already been flagged in prior reviews.
🧹 Nitpick comments (5)
src/main/java/org/team5924/frc2026/Constants.java (1)
148-153: Inconsistent indentation in builder chain.The
.withSlot0()and its contents have inconsistent indentation compared to the rest of the builder chain. This affects readability.♻️ Suggested formatting fix
.withMotorOutput( new MotorOutputConfigs() .withInverted(InvertedValue.CounterClockwise_Positive) .withNeutralMode(NeutralModeValue.Brake)) - .withSlot0( - new Slot0Configs() - .withKP(0) - .withKD(0) - .withKS(0) - .withKV(0)); // TODO: TUNE THESE VALUES + .withSlot0( + new Slot0Configs() + .withKP(0) + .withKD(0) + .withKS(0) + .withKV(0)); // TODO: TUNE THESE VALUESsrc/main/java/org/team5924/frc2026/subsystems/superShooter/SuperShooter.java (3)
30-40: Remove unnecessary empty enum constructor.The
ShooterStateenum has an empty constructor that serves no purpose. Java enums get implicit no-arg constructors when no fields are present.♻️ Simplified enum
public enum ShooterState { OFF(), AUTO_SHOOTING(), BUMPER_SHOOTING(), NEUTRAL_SHUFFLING(), OPPONENT_SHUFFLING(), - HOOD_MOVING(); - - ShooterState() { - } + HOOD_MOVING }
44-46: Consider updating RobotState when goal changes.
ShooterHood.setGoalStateandShooterRoller.setGoalStateboth update the globalRobotStatewith their current states. For consistency,SuperShooter.setGoalStateshould also updateRobotState.getInstance().setShooterState(goalState)to maintain global state visibility.♻️ Proposed fix
+import org.team5924.frc2026.RobotState; ... public void setGoalState(ShooterState goalState) { this.goalState = goalState; + RobotState.getInstance().setShooterState(goalState); }
55-80: Add a default case and fix indentation.The switch statement is missing a
defaultcase, which could cause silent failures if theShooterStateenum is extended. Also, the indentation forNEUTRAL_SHUFFLING,OPPONENT_SHUFFLING, andHOOD_MOVINGcases is inconsistent with the others.♻️ Proposed fix
switch (goalState) { case OFF: roller.setGoalState(ShooterRollerState.OFF); hood.setGoalState(ShooterHoodState.OFF); break; case AUTO_SHOOTING: roller.setGoalState(ShooterRollerState.SHOOTING); hood.setGoalState(ShooterHoodState.AUTO_SHOOTING); break; case BUMPER_SHOOTING: roller.setGoalState(ShooterRollerState.SHOOTING); hood.setGoalState(ShooterHoodState.BUMPER_SHOOTING); break; - case NEUTRAL_SHUFFLING: + case NEUTRAL_SHUFFLING: roller.setGoalState(ShooterRollerState.SHOOTING); hood.setGoalState(ShooterHoodState.NEUTRAL_SHUFFLING); break; - case OPPONENT_SHUFFLING: + case OPPONENT_SHUFFLING: roller.setGoalState(ShooterRollerState.SHOOTING); hood.setGoalState(ShooterHoodState.OPPONENT_SHUFFLING); break; - case HOOD_MOVING: + case HOOD_MOVING: roller.setGoalState(ShooterRollerState.OFF); hood.setGoalState(ShooterHoodState.MOVING); break; + default: + roller.setGoalState(ShooterRollerState.OFF); + hood.setGoalState(ShooterHoodState.OFF); + break; }src/main/java/org/team5924/frc2026/subsystems/shooterHood/ShooterHood.java (1)
35-35: Consider making tolerance field private.
ShooterHoodPivotToleranceis declared aspublic, which exposes it for uncontrolled modification. Following encapsulation best practices, this should beprivatewith a getter if external access is needed.♻️ Proposed fix
- public LoggedTunableNumber ShooterHoodPivotTolerance = new LoggedTunableNumber("ShooterHoodPivotToleranceRads", .02); + private final LoggedTunableNumber shooterHoodPivotTolerance = new LoggedTunableNumber("ShooterHoodPivotToleranceRads", 0.02);Then update
isAtSetpoint()to use the new field name.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/org/team5924/frc2026/RobotContainer.java`:
- Around line 19-32: Remove the unresolved Git conflict markers (<<<<<<< HEAD,
=======, >>>>>>>) in RobotContainer.java and keep only the correct set of import
statements; specifically delete the entire conflict block shown and ensure
imports for classes referenced later (e.g., DriveCommands, TunerConstants,
Drive, GyroIO, GyroIOPigeon2, ModuleIO, ModuleIOSim, ModuleIOTalonFX and add
import for ShooterRoller if the class is used where referenced) remain exactly
once so the file compiles without duplicate or conflicted imports.
♻️ Duplicate comments (3)
src/main/java/org/team5924/frc2026/Constants.java (2)
19-19: Remove unused import.
CANcoderConfigurationis imported but not used in this file.
32-33: Remove unused import.
SensorDirectionValueis imported but not used in this file.src/main/java/org/team5924/frc2026/RobotContainer.java (1)
71-72: UnusedTriggerfield.gamePieceTriggeris instantiated but never bound to any command (no.onTrue(),.whileTrue(), etc.). ATriggeris "polled by the default scheduler button loop", but binding buttons to commands is done declaratively—the library checks the button state and schedules (or cancels) the command as needed. Without a binding, this trigger does nothing useful.Either bind a command to this trigger or remove it until it's needed.
🧹 Nitpick comments (2)
src/main/java/org/team5924/frc2026/Constants.java (2)
150-155: Inconsistent indentation in Slot0 configuration.The
.withSlot0()chain and itsSlot0Configsblock have inconsistent indentation compared to the other chained method calls. Align for readability.🔧 Suggested formatting
.withMotorOutput( new MotorOutputConfigs() .withInverted(InvertedValue.CounterClockwise_Positive) .withNeutralMode(NeutralModeValue.Brake)) - .withSlot0( - new Slot0Configs() - .withKP(0) - .withKD(0) - .withKS(0) - .withKV(0)); // TODO: TUNE THESE VALUES + .withSlot0( + new Slot0Configs() + .withKP(0) + .withKD(0) + .withKS(0) + .withKV(0)); // TODO: TUNE THESE VALUES
184-193: Supply current limit should be 60A.Per team guidance, the default minimum for supply current is 60A. The
ShooterRoller.CONFIGhaswithSupplyCurrentLimit(35)which is below the recommended threshold.🔧 Suggested fix
public static final TalonFXConfiguration CONFIG = new TalonFXConfiguration() .withCurrentLimits( new CurrentLimitsConfigs() - .withSupplyCurrentLimit(35) + .withSupplyCurrentLimit(60) .withStatorCurrentLimit(35)) .withMotorOutput( new MotorOutputConfigs() .withInverted(InvertedValue.CounterClockwise_Positive) .withNeutralMode(NeutralModeValue.Brake));
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/org/team5924/frc2026/Constants.java`:
- Around line 134-155: ShooterHood's Slot0 PID gains in the TalonFXConfiguration
(static CONFIG in class ShooterHood) are all zero, so position control (used by
setGoalState() → io.setPosition() which uses PositionVoltage) will produce no
output; update Slot0Configs (the .withKP/.withKD/.withKS/.withKV calls on
CONFIG) to sensible non‑zero tuning values (at minimum set KP > 0) or add a
gating/flag in setGoalState()/io.setPosition() to disable position mode until
those gains are tuned so the hood can actually move to setpoints.
…2026 into 15-shooter_roller-and-hood
arcadeArchitect
approved these changes
Jan 25, 2026
This was referenced Feb 7, 2026
This was referenced Feb 14, 2026
Merged
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
created basic shooter roller based off of example roller and basic shooter hood based off of 2025 pivot, along with a basic super shooter superstructure, also has one cancoder in the roller subsystem