Feature/shooter

[TheGreatPintoJ]

No description provided.

[github-actions]

AI Code Review

Hey team!

This is a great start on the shooter subsystem, and it's awesome to see so many of our team's best practices already being incorporated!

Positive Highlights

• Type-Safe Units: You're doing a fantastic job using WPILib's Units system for Distance, Angle, Current, and AngularVelocity constants and variables. This helps prevent many common conversion errors!
• CTRE Configuration: Using static factory methods in ShooterConstants to create and apply CTRE configs (MotorOutputConfigs, Slot0Configs, SoftwareLimitSwitchConfigs) is an excellent way to keep your code organized and reusable.
• Command Factory Methods: The subsystem exposes well-named command factory methods like spinFlywheelCommand(), setHoodCommand(), and stowCommand(), which is a cornerstone of good command-based programming.
• Automatic Homing: Implementing a homeShooterCommand that uses stator current to detect a stall for homing is a robust and safe approach for mechanisms without absolute encoders.

Suggestions

Here are a few suggestions to consider as you continue to refine the shooter:

src/main/java/frc/robot/subsystems/shooter/ShooterConstants.java

• FRC Best Practices / Motor ID Conflict: I noticed that HOOD_MOTOR_ID is set to 5, which is the same as FLYWHEEL_LEADER_MOTOR_ID. Motor IDs on the CAN bus must be unique. It might be helpful to double-check your robot's actual motor IDs in Tuner X and update these constants to ensure they are distinct.
• Type-Safe Units for Limits: HOOD_FORWARD_LIMIT, HOOD_REVERSE_LIMIT, and ALLOWABLE_HOOD_ERROR are currently double values. Since these represent angles (rotations), it might be more robust to define them using Angle units (e.g., Rotations.of(100)) to maintain consistency and prevent unit mix-ups.
• Safety & Error Handling / Enable Software Limits: In createHoodSoftwareLimitSwitchConfigs(), ForwardSoftLimitEnable and ReverseSoftLimitEnable are currently set to false. To fully utilize the software limits you've defined, you'll want to set these to true. This provides an important safety mechanism for your hood.
• Code Quality / Constant Clarification:
  • ROTATIONS_PER_LAUNCH_DEGREE is set to Rotations.of(1). If this represents a gear ratio or conversion, its name could be more specific (e.g., HOOD_GEAR_RATIO). Also, 1 rotation per degree seems like an unusual ratio; you might want to verify this value.
  • FLYWHEEL_RADIUS is Inch.of(1). This seems quite small for a flywheel, so it might be a placeholder. Just something to confirm!
  • OFFSET_DISTANCE is Meter.of(1). A comment clarifying what this offset refers to (e.g., shooter's position relative to robot center) would be helpful for future understanding.
  • For SAFE_HOMING_EFFORT, you've correctly used DutyCycleOut, but its .Output field is a double. While this works for hoodMotor.set(), consider if it might be clearer to define SAFE_HOMING_DUTY_CYCLE directly as a double constant if it's primarily used for raw motor setting.

src/main/java/frc/robot/subsystems/shooter/ShooterPreferences.java

• Type-Safe Units for Preferences: flywheelLaunchSpeed and hoodLaunchAngle are DoublePreference objects. While their comments indicate units, it might be beneficial to retrieve them as Measure<AngularVelocity> and Measure<Angle> directly (e.g., RotationsPerSecond.of(flywheelLaunchSpeed.getValue())) when using them, to leverage the WPILib Units system more fully and prevent implicit conversions.

src/main/java/frc/robot/subsystems/shooter/ShooterSubsystem.java

• FRC Best Practices / Motor ID Conflict: As mentioned in ShooterConstants, the HOOD_MOTOR_ID is likely conflicting with a flywheel motor ID. This will need to be resolved to ensure proper hardware functionality.
• FRC Best Practices / Command Composition: In launchLemonsCommand(), you have setHoodCommand(...).andThen(spinFlywheelCommand()). This means the hood will reach its target position before the flywheel starts spinning. Often, it's more efficient to spin the flywheel while the hood is moving to save time. You could consider Commands.parallel(...) or Commands.race(...) if these actions can happen concurrently, or even spinning the flywheel first if it takes longer to reach speed.
• FRC Best Practices / Periodic Method Logic:
  • You have launchRequestBuilder.createLaunchRequest(); in periodic(). This method calculates launch parameters but doesn't seem to apply them to velocityTarget or hoodTarget in the subsystem. If the intention is for these calculated values to constantly update the motor targets, you might want to explicitly set velocityTarget and hoodTarget within periodic() using the LaunchRequest results. Otherwise, it might be a wasted computation if the results aren't immediately used.
  • There appears to be a type mismatch in periodic(): hoodMotor.setControl(hoodControl.withVelocity(hoodTarget.in(Rotations))). Your hoodControl is a PositionTorqueCurrentFOC, meaning it expects a position as its target. However, withVelocity is called, which expects an angular velocity. You should likely use hoodControl.withPosition(hoodTarget.in(Rotations)) here to correctly apply the position control.
• FRC Best Practices / launchLemonsCommandNoPID(): This command directly uses hoodMotor.set() and flywheelMotorLeader.set(), bypassing the subsystem's PID control loops and established command factory methods. While useful for testing, it's generally recommended to use the higher-level command factory methods (like setHoodCommand and spinFlywheelCommand) that leverage the configured PID control. If this is a specific test mode, consider adding comments to explain its purpose. Also, ensure hoodMotor.set(...) uses an Angle unit (e.g., Rotations.of(...)) rather than a raw double if that's what the set method expects.
• Code Quality / atHoodSetpoint() Constant: In atHoodSetpoint(), you're using IntakeConstants.ALLOWABLE_EXTENSION_ERROR. This seems like a copy-paste error; it would be better to use ShooterConstants.ALLOWABLE_HOOD_ERROR for clarity and correctness.
• Code Quality / SysId Routine: You've correctly initialized m_sysIdRoutineFlywheel, which is great for future tuning! Just a reminder that you'll need to call its runSysIdRoutine() method from a command to actually execute the SysId tests.
• Code Quality / Typo: There's a small typo in MappedLauchRequestBuilder (should be MappedLaunchRequestBuilder).

Questions

• Could you clarify the purpose of OFFSET_DISTANCE in ShooterConstants.java?
• Is launchRequestBuilder.createLaunchRequest(); in periodic() intended to continuously update the velocityTarget and hoodTarget variables, or is its output meant to be used differently?

Great work on this feature! The structure is solid, and you're already thinking about important aspects like logging, homing, and type safety. Keep up the excellent work!

---

This review was automatically generated by AI. Please use your judgment and feel free to discuss any suggestions!

[github-actions]

✓ Build successful and code formatting check passed!