Completed Intake Logic

[SaiJavaGuy]

Intake Subsystem has been fleshed out

[bczdog]

Intake.java appears to import from the capital T Turret package. Build failed:
/__w/2026_Rebuilt/2026_Rebuilt/CompBot/src/main/java/frc/robot/subsystems/intake/Intake.java:12: error: package frc.robot.subsystems.Turret.TurretIO does not exist
import frc.robot.subsystems.Turret.TurretIO.TurretIOInputs;

Task :compileJava FAILED

[dkbrown8]

PR Review: Completed Intake Logic

Good progress on the intake subsystem! Here's my review:

Critical Issues 🔴

1. setVoltage() in IntakeIOHardware Doesn't Control the Motor

public void setVoltage(Voltage voltage){
    this.voltage = voltage;
}

This just stores the voltage in a field but never applies it to the motor. The intakeFuel() and reverse() commands won't actually do anything. You need to actually send the voltage to the motor:

public void setVoltage(Voltage voltage) {
    motor.setVoltage(voltage.baseUnitMagnitude());
}

2. Unused Parameter in detectJam()

public Trigger detectJam(boolean hasFuel) {
    return new Trigger(() ->  
        (inputs.supplyCurrent.in(Amps) > IntakeConstants.kStatorCurrentLimit) && 
        (inputs.hasFuel));  // Uses inputs.hasFuel, not the parameter!
}

The hasFuel parameter is never used. Either remove it or use it:

public Trigger detectJam() {
    return new Trigger(() ->  
        (inputs.supplyCurrent.in(Amps) > IntakeConstants.kStatorCurrentLimit) && 
        inputs.hasFuel);
}

3. Commands Don't Stop Motor When Finished

Same issue as PR #42 - using runOnce() sets the voltage and completes, but the motor keeps running:

public Command intakeFuel() {
    return runOnce(() -> io.setVoltage(IntakeConstants.kIntakeVoltage));    
}

Use startEnd() to ensure cleanup:

public Command intakeFuel() {
    return startEnd(
        () -> io.setVoltage(IntakeConstants.kIntakeVoltage),
        () -> io.setVoltage(Volts.zero())
    );
}

4. Confusing Private Method

private void setVoltage(Current voltage) {
    io.setCurrent(voltage);
}

This method is named setVoltage, takes a parameter of type Current named voltage, and calls setCurrent(). This is very confusing. Also, this method is never used - the commands call io.setVoltage() directly, not this private method.

Minor Issues ⚠️

5. Unused Imports in IntakeIO.java

These imports are added but never used:

• StatusSignal
• Units
• VelocityUnit
• Distance
• LinearVelocity
• Velocity

6. Unused Import in Intake.java

import frc.robot.Robot; is not used.

7. Naming Convention: kcanrangeID

Should follow Java conventions: kCanRangeId or kCANRangeID

8. IntakeIOSim.setVoltage() Doesn't Simulate Anything

public void setVoltage(Voltage voltage){
    this.voltage = voltage;
}

This should actually affect the simulated motor behavior, similar to how setCurrent() works.

9. Unnecessary super() Call

public Intake(IntakeIO io) {
    super();  // This is redundant
    this.io = io;
    inputs = new IntakeIOInputs();
}

10. Extra Blank Lines

Multiple unnecessary blank lines at the end of IntakeConstants.java.

What Looks Good ✅

• Good use of CANrange for fuel detection
• Voltage constants are reasonable (5V is sensible for intake)
• Following the IO pattern for hardware abstraction
• Added velocity tracking to inputs

Summary

The main issue is that setVoltage() in the hardware implementation doesn't actually control the motor - this needs to be fixed or the intake won't work at all. Also please address the unused parameter and motor cleanup issues.

---

🤖 Generated with Claude Code

[therekrab]

@dkbrown8 I looked over this code and Claude's review, and although there are several (good) things it did pick up, it missed one crucial oversight: the IntakeIOHardware class never calls BaseStatusSignal.registerXXXSignals() with the caches status signal objects, and then the code doesn't refresh them automatically. Not refreshing is idea, but you must remember to register your status signals or they will never be updated.

If you want to tell it what's important or what to look for, tell it to make sure that all HardwareIO-style classes register their signals.

[therekrab]

lgtm

[dkbrown8]

Code Review

Good progress fleshing out the intake. A few real bugs to address before merging.

---

Bugs

RobotBindings is never bound

RobotContainer declares robotBinder but never calls robotBinder.bind(superstructure). All of RobotBindings (auto-shoot on shootReady, climber re-extension on teleop start) is dead code on the robot.

// RobotContainer.java
public final Binder robotBinder = new RobotBindings();  // created...

public RobotContainer() {
    superstructure = new Superstructure();
    driverBinder.bind(superstructure);
    // robotBinder.bind(superstructure) is never called!

canrangeConnected is never set; canrangeDetected is overwritten

In IntakeIOHardware.updateInputs():

inputs.canrangeDetected = BaseStatusSignal.isAllGood(...);  // should be canrangeConnected
inputs.canrangeDistance = canrange.getDistance(false).getValue();
inputs.canrangeDetected = canrange.getIsDetected(false).getValue();  // overwrites above

The first line should assign inputs.canrangeConnected, not inputs.canrangeDetected. As written, canrangeConnected stays false forever, and the connection check result is immediately discarded.

Intake.go() never updates reference, so deployAtPositoin() always checks against Stow

private DeployPosition reference = DeployPosition.Stow;

public Command go(DeployPosition state) {
    return Commands.sequence(
        runOnce(() -> io.setDeployPosition(state.position)),  // sends command
        Commands.waitUntil(this::deployAtPositoin)            // but checks against Stow!
    );
}

private boolean deployAtPositoin() {
    return Math.abs(inputs.deployPosition.minus(reference.position)...);
    //                                              ^^ always Stow (0 rotations)
}

reference needs to be updated to state inside the runOnce. Without this, calling go(Deployed) will complete the waitUntil the moment the motor is near 0 rotations, which is immediately at startup.

IntakeIOSim.setDeployPosition() is a no-op, so go() hangs in sim

public void setDeployPosition(Angle angle) {}  // position never updated

inputs.deployPosition never changes, so deployAtPositoin() never returns true, and any command using go() will hang indefinitely in simulation.

---

Design Concerns

detectJam() checks supply current against a threshold named kJamStatorThreshold

inputs.intakeSupplyCurrent.in(Amps) > IntakeConstants.kJamStatorThreshold.in(Amps)

Either the field should be kJamSupplyThreshold, or it should be checking intakeStatorCurrent. Stator current is usually more appropriate for jam detection since it reflects actual mechanical load.

RunIntake starts rollers and deploy in parallel

Rollers start spinning before the arm is fully deployed. Depending on the mechanism geometry, this could cause issues. Consider sequencing deploy before intake, or at least document it as intentional.

CANrange ID comment left in constants

protected static final int kcanrangeID = 25; // Do we really have a CANrange?

This is a hardware question that should be resolved before competition.

---

Nitpicks

• Typo in method name: deployAtPositoin -> deployAtPosition
• Typo in ClimbPosition constructor param: posiiton -> position (double-i, copy-pasted from old ClimberPositions)
• Deploy PID gains are all 0 in kDeployMotorConfig — expected for now but worth noting before testing

---

What's Good

• Clean IO interface pattern (IntakeIO / IntakeIOHardware / IntakeIOSim)
• Comprehensive logging in IntakeIOInputs
• Aiming constraints moved out of StateManager and into Constants.AimConstants
• RobotBindings is a nice pattern for robot-automated behaviors (auto-shoot, etc.) separate from driver bindings
• Climber API cleanup (ClimberPositions -> ClimbPosition, climb() -> go(), ready() -> at()) is cleaner

[bczdog]

Something to consider, rename the canrange to reflect its purpose, e.g. intakeFullCanRange or similar (I'm guessing at why we have it).