Main Catchup

[y0shi]

No description provided.

[github-actions]

AI Code Review

Hey team!

This pull request looks like a fantastic catch-up, bringing in a lot of new autonomous routines and refining some core robot behaviors. It's great to see so much progress in a single PR!

Positive Highlights

• PathPlanner Integration: The numerous additions and modifications to PathPlanner auto files and paths show a strong commitment to robust autonomous capabilities. Using NamedCommands is an excellent way to integrate custom robot actions into your sequences.
• Type-Safe Units: I noticed consistent use of WPILib's Units system (e.g., Meters.of(), Rotations.of()) which is a best practice for preventing common conversion errors and making your code more readable and robust. Great job!
• Command Factory Methods: Subsystems like IntakeSubsystem and ShooterSubsystem are effectively using command factory methods (collectNoPIDCommand(), spinFlywheelRanged()), which keeps the internal logic encapsulated and makes commands easier to compose.
• Preferences System: The use of DoublePreference for tuning values like autoAim_kP and various intake/shooter parameters is excellent for quick iteration and testing without recompiling.

Suggestions

Code Quality & Organization

• networktables.json.bck: I noticed this file in the changes. Typically, .bck files are backups and might not be intended for version control.
  • Consider: If this file is meant to be the active NetworkTables configuration, it might be clearer to rename it to networktables.json. If it's a backup, it would be helpful to add *.bck to your .gitignore file to prevent accidental commits.
• RobotContainer.java - Command Cleanup: There are a few commented-out blocks in configureTeleopBindings() (e.g., for driverJoystick.a() and driverJoystick.b()) that seem to be remnants of previous logic.
  • Consider: Removing these commented-out sections once you've confirmed they are no longer needed. This helps keep the code clean and focused on the current implementation.
• IndexerSubsystem.java - stopFullIndexingNoPID(): Changing this from run to runOnce is a good correction! It ensures the motors are set to zero once and the command finishes.

FRC/WPILib Best Practices

• Robot.java / RobotContainer.java - Initialization Timing: You've moved the calls to configureSubsystemDefaultCommands() and configureTeleopBindings() into the RobotContainer constructor.
  • Context: While this works for initial setup, the previous approach of calling them in teleopInit() ensured that default commands and bindings were re-applied if, for example, the robot transitioned from autonomous to teleop, or from test mode to teleop. This can be important if commands modify subsystem states or if you ever dynamically change bindings.
  • Consider: Re-evaluating if you'd like these to re-initialize at the start of teleop. For default commands, having them active ensures subsystems always have a "do nothing" state. For bindings, it means controls are always fresh. If you don't anticipate dynamic changes, the current approach is fine, but it's worth understanding the implications.
• IntakeSubsystem.java - PID Control in periodic(): I noticed the PID control lines for rollerMotor and extensionMotor in the periodic() method are commented out.
  • Context: This means these motors are currently controlled directly by set() commands (open-loop) rather than through their configured PID loops and control requests. This is evident in the new collectNoPIDCommand() and stowNoPIDCommand().
  • Consider: Confirming if this is a temporary change for testing (which is perfectly fine!) or a permanent shift away from PID control for these mechanisms. If it's temporary, it might be helpful to add a comment explaining that the PID is disabled for now. If permanent, we might want to remove the PID configuration and associated _control objects if they're not used.
• Robot.java - Thread Priority: The guidelines mention elevating critical thread priority.
  • Consider: Adding Thread.currentThread().setPriority(4); in Robot.java's constructor or robotInit() method to help ensure consistent control loop timing, especially on busy robots.

Java Standards

• Constants.java - BLUE_LEFT_PASS Coordinates: The coordinates for BLUE_LEFT_PASS changed from (7, 1) to (1, 7).
  • Question: Was this an intentional coordinate change, perhaps a correction or a new target location? Just wanted to double-check the intent here.
• MappedLaunchRequestBuilder.java - Hood Angle Map: You've simplified hoodAngleMap to use InterpolatingDoubleTreeMap directly with double values.
  • Context: This is a clean simplification if Rotation2d interpolation wasn't adding specific value for the map itself.
  • Question: Was the change to InterpolatingDoubleTreeMap from InterpolatingTreeMap<Double, Rotation2d> an intentional simplification due to how Rotation2d was being used, or just a refactor? It looks good either way, just curious about the thought process!

Performance and Efficiency

• RobotContainer.java - getDriveAndLaunchRequest() Deadband: The withRotationalDeadband was removed from the SwerveRequest.FieldCentric in getDriveAndLaunchRequest().
  • Question: Was this removal intentional? If so, what was the reasoning? Sometimes a rotational deadband can prevent minor controller stick drift from causing unwanted robot rotation.

Questions

• networktables.json.bck: What is the purpose of this file? Is it meant to be kept in version control, or should it be ignored?
• IntakeSubsystem.java periodic() commented code: Is the commenting out of PID control for the intake motors in periodic() temporary for testing, or a permanent design decision to use open-loop control for these?

Overall, this is a very productive pull request! Keep up the great work on refining these systems and building robust autonomous capabilities!

Awesome job, team!

---

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!