Code Review: Critical Bugs Identified

[DugboTek]

• Code review identifying critical bugs in robot subsystem implementations

issues Found
See inline review comments on the following files:

1. IntakeIOHardware.java - CAN ID swap & input overwrite bugs
2. ShooterIOHardware.java - Wrong controller used for hood
3. Shooter.java - Voltage method calls percentage method

[DugboTek]

Critical Bug #1: IntakeIOHardware.java:17-18 - CAN IDs Swapped

The leader motor is assigned the follower CAN ID and vice versa:

// Current (BROKEN):
protected final SparkFlex m_pivotLeader = new SparkFlex(IntakeConstants.kPivotFollowerCAN, ...);  // Wrong!
protected final SparkFlex m_pivotFollower = new SparkFlex(IntakeConstants.kPivotLeaderCAN, ...);  // Wrong!

Fix: Swap the CAN ID constants:

protected final SparkFlex m_pivotLeader = new SparkFlex(IntakeConstants.kPivotLeaderCAN, ...);
protected final SparkFlex m_pivotFollower = new SparkFlex(IntakeConstants.kPivotFollowerCAN, ...);

[DugboTek]

Critical Bug #3: ShooterIOHardware.java:97 - Wrong Controller for Hood

setHoodPosition() uses m_flywheelController instead of m_hoodController. Calling this will move the flywheel instead of the hood!

// Current (BROKEN):
public void setHoodPosition (double position) {
    m_flywheelController.setSetpoint(position, ControlType.kPosition, ClosedLoopSlot.kSlot0);
}

Fix:

public void setHoodPosition (double position) {
    m_hoodController.setSetpoint(position, ControlType.kPosition, ClosedLoopSlot.kSlot0);
}

[DugboTek]

#4: Shooter.java:99 - Voltage Method Calls Percentage

SysId routines call setFlywheelVoltage() expecting voltage control (0-12V), but it passes through to setFlywheelPercentage(). A 12V command would be interpreted as 1200%!

// Current (BROKEN):
public void setFlywheelVoltage(double magnitude) {
    this.m_io.setFlywheelPercentage(magnitude);  // Wrong!
}

Fix:

public void setFlywheelVoltage(double magnitude) {
    this.m_io.setFlywheelVoltage(magnitude);
}

Also need to implement setFlywheelVoltage() in ShooterIOHardware.java:

@Override
public void setFlywheelVoltage(double voltage) {
    m_flywheelLeader.setVoltage(voltage);
}

[DugboTek]

Issue: Velocity Units Mismatch: RPM vs Rotations Per Second

REV SparkFlex encoders return velocity in RPM by default, but WPILib's SysId RotationsPerSecond unit expects rotations per second.

Current code:
config.encoder
.velocityConversionFactor(IntakeConstants.kRollerGearing)

This only applies the gear ratio, not the RPM to RPS conversion.

Expected:
config.encoder
.velocityConversionFactor(IntakeConstants.kRollerGearing / 60.0)

The /60.0 converts RPM to rotations per second (RPM ÷ 60 = RPS).

Affected subsystems:

• IntakeIOHardware
• SpindexerIOHardware
• FeederIOHardware
• ShooterIOHardware
• ClimberIOHardware

Impact:
SysId logs velocity data with incorrect units, causing characterization to produce wrong kV/kA values (off by a factor of 60).

References:

• https://docs.revrobotics.com/revlib/spark/closed-loop/units - "Encoder Velocity: RPM"
• WPILib SysId expects RotationsPerSecond