Hotfix/champs

[Orcasphynx]

No description provided.

[github-actions]

✓ Build successful and code formatting check passed!

[github-actions]

AI Code Review

Hey team!

Great job on this hotfix for champs! It looks like you've been busy tuning and refining your autonomous routines and making some really smart architectural improvements. Let's dive into the details.

Summary

This pull request introduces a significant number of updates to your PathPlanner autonomous routines and paths, along with some excellent code refactoring for subsystem tests and new driver controls. I'm particularly impressed with the detailed tuning in your PathPlanner paths and the adoption of command factory methods! There's one critical area related to threading that we need to address to ensure robot reliability.

Positive Highlights

1. PathPlanner Organization & Tuning: You've done a fantastic job organizing your auto and path files into logical folders (Half + Full Sweep Autos, Double Half Sweep Autos, Half Sweep + Swipe Autos). The use of constraintZones and custom globalConstraints in many paths (e.g., Delayed Swipe Start - RIGHT.path, Side Wall - LEFT.path, Start to Overbump - RIGHT.path) shows a deep understanding of PathPlanner's capabilities for fine-tuning robot behavior, which is awesome for competition performance!
2. Command Factory Methods: The change in RobotContainer.java from new WheelSlipTest(drivetrain) to drivetrain.wheelSlipTest() is a perfect example of following the "Command Factory Methods" guideline. This keeps the command logic encapsulated within the DrivetrainSubsystem, making RobotContainer cleaner and the commands easier to maintain.
3. Enhanced Driver Controls and Safety: Adding driverJoystick.x().onTrue(intake.stopRollerNoPID()); and operatorJoystick.y().onTrue(intake.stopRollerNoPID()); provides immediate and intuitive ways for drivers to stop the intake, which is a great safety and control feature. Using withTimeout(1) for intake.collectCommand() in autonomous is also a smart way to prevent indefinite running.

Suggestions

FRC/WPILib Best Practices & Safety

• Critical: Avoid New Threads in DrivetrainSubsystem.java (wheelSlipTest() method)
  • File: src/main/java/frc/robot/subsystems/drive/DrivetrainSubsystem.java
  • Location: wheelSlipTest() method, lines 413-447 (specifically ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); and scheduler.scheduleAtFixedRate(...)).
  • Explanation: Creating a ScheduledExecutorService introduces a new thread to your robot code. WPILib's framework is designed around a single main robot thread (the one that calls periodic() methods). Introducing additional threads can lead to unpredictable behavior, race conditions, difficult-to-debug timing issues, and potentially cause the robot's main loop to miss its 20ms cycle. This is a significant risk for reliability during a match.
  • Recommendation: Instead of using a separate scheduler thread to update state.step, consider updating this value directly within the Commands.run block using Timer.getFPGATimestamp() to calculate elapsed time, or by incrementing a counter each periodic cycle. This keeps all robot logic on the main thread, ensuring deterministic and safe operation. If you need a specific delay, Commands.waitSeconds() is the preferred WPILib approach.
  • Example (Conceptual, for state.step update):

    // Inside Commands.run()
    // double elapsedTime = Timer.getFPGATimestamp() - commandStartTime;
    // state.step = initialStep + (elapsedTime / interval) * stepIncrement;
    // Or if you want discrete steps:
    // state.step = Math.min(MAX_STEP, state.step + STEP_INCREMENT_PER_PERIODIC);

  • This is a high-priority fix before competition to ensure robot stability.

Code Quality & FRC/WPILib Best Practices

• PathPlanner Path Configuration (Minor Issue)
  • Files: src/main/deploy/pathplanner/paths/Half Second Sweep - LEFT.path, src/main/deploy/pathplanner/paths/Second Half Sweep to Shoot - LEFT.path
  • Location: constraintZones array.
  • Explanation: In these paths, you have constraintZones defined, but their minWaypointRelativePos and maxWaypointRelativePos are both set to 0. This effectively makes the constraint zone inactive. Additionally, useDefaultConstraints: true is present, which can be confusing if you intend to use custom constraints.
  • Recommendation:
    • If you intend to apply specific constraints to a segment of the path, ensure minWaypointRelativePos and maxWaypointRelativePos define a valid range (e.g., 0.0 to 1.0 for the whole path, or a smaller segment).
    • If you don't need custom constraints for these paths, you can remove the constraintZones block entirely and set useDefaultConstraints: true to clearly indicate that global defaults (from settings.json) are used.
    • If you do want custom constraints, set useDefaultConstraints: false (as you've done in other paths) and ensure your globalConstraints are explicitly defined for the path, or that your constraintZones cover the intended segments.

Code Readability

• PathPlanner JSON Newline Characters (Minor Style)
  • Files: src/main/deploy/pathplanner/autos/Depot Auto.auto, Double Half Sweep - LEFT.auto, Double Half Sweep - RIGHT.auto, Half Sweep + Swipe Auton - LEFT.auto, Half Sweep + Swipe Auton - RIGHT.auto, Simple Auto.auto, and several .path files.
  • Explanation: Several of the newly added PathPlanner .auto and .path files are missing a newline character at the very end of the file (indicated by \ No newline at end of file in the diff). While not a functional bug, it's a common style convention for text files.
  • Recommendation: Consider adding a newline character at the end of these files. Your Spotless formatter would typically handle this for Java files, and keeping consistent file endings is generally good practice.

Questions

• In RobotContainer.java, you've commented out the operatorJoystick.y().whileTrue(uiFeedback.manualRumbleCommand(...)) block. Was this temporarily disabled for champs, or is it a feature that's being removed permanently? A quick note on its status could be helpful for future reference!

You've put a lot of hard work into these autonomous routines, and it really shows! Addressing the threading concern will make your robot even more robust. Keep up the fantastic work, and good luck at champs!

---

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