Feature/holonomic controllers

[Orcasphynx]

No description provided.

[github-actions]

✓ Build successful and code formatting check passed!

[github-actions]

AI Code Review

Hello team! This looks like a really significant and exciting pull request, bringing in the new WPILib 2026 and CTRE Phoenix 6 Swerve setup, alongside robust logging and path planning. Great work on tackling such a foundational change!

Positive Highlights

• Modernization: It's fantastic to see the update to WPILib 2026 and CTRE Phoenix 6. This sets us up with the latest features and best practices for the upcoming season.
• Comprehensive Logging: The integration of Epilogue and SignalLogger in Robot.java is excellent! This will provide invaluable data for debugging, analysis, and deterministic replay, which is crucial for a competitive FRC team.
• PathPlanner Integration: The inclusion of PathPlanner auto routines and paths demonstrates a strong commitment to advanced autonomous capabilities, which is a huge asset.
• Clear Telemetry: The new Telemetry.java class is a great step towards organized and visual feedback on the robot's state, especially for the swerve drive.

Suggestions

FRC/WPILib Best Practices

• Command Factory Methods for setDrivePower: In src/main/java/frc/robot/commands/WheelSlipTest.java (line 52), you're directly calling m_drive.setDrivePower(step). While this works, for consistency with the "Command Factory Methods" guideline, it might be more idiomatic to expose this as a command from the DrivetrainSubsystem (e.g., drivetrain.driveAtPowerCommand(power)). This helps keep subsystem methods focused on state and commands on behavior.
• Thread Management in WheelSlipTest: In src/main/java/frc/robot/commands/WheelSlipTest.java (lines 33-39), you're using a ScheduledExecutorService to increment the step variable. While this is valid Java, in the FRC command-based framework, it's generally simpler and safer to manage timing and state changes within the command's execute() method or by composing commands (e.g., using WaitCommand or Commands.sequence). Introducing a separate thread can complicate debugging and introduce potential thread-safety issues if not carefully managed.
  • Educational Context: The WPILib scheduler runs all command execute() methods on the main robot thread. This simplifies concurrency management. When you create new threads, you need to be mindful of volatile keywords, synchronized blocks, and potential race conditions if the main thread and your custom thread access shared resources. Sticking to the main thread for robot control often makes the code easier to reason about in an FRC context.

Java Standards & Code Quality

• Consistency in Naming Conventions:
  • In src/main/java/frc/robot/RobotContainer.java, you have joystick and then drivetrain, intake, indexer, climber. While joystick is a local variable, drivetrain and the other subsystems are public final. Our guidelines suggest camelCase for variables. It might be helpful to be consistent with m_ prefix for member variables (e.g., m_joystick, m_drivetrain) or to remove it entirely for all member variables if the team prefers that style (e.g., drivetrain, intake are good examples of no m_).
  • src/main/java/frc/robot/commands/DriveBySpeed.java (line 21): There's a double semicolon ;; at the end of the line. It's a minor syntax issue but worth cleaning up for clarity.
• Javadoc for New Classes: It would be great to add Javadoc comments to the new classes (Telemetry, DriveBySpeed, DriveToPose, WheelSlipTest, BooleanPreference, DoublePreference) and their public methods. This helps other team members understand their purpose, parameters, and return values at a glance.
  • Educational Context: Javadoc is a standard way to document Java code, making it easier for others (and your future self!) to understand how to use your classes and methods without diving into the implementation details. It acts as a contract for your code.
• PID Controller Initialization in DriveToPose: In src/main/java/frc/robot/commands/DriveToPose.java, the xControl, yControl, and rotControl PID controllers are initialized in initialize(). While this is fine, they are not explicitly reset or configured with setpoints/tolerances here. If this command is re-scheduled, the PID controllers will pick up from their last state.
  • Consider adding xControl.setSetpoint(targetPose.getX()); and similar for Y and rotation, along with xControl.setTolerance(...) if specific tolerances are desired, within the initialize() method. This ensures the PID controllers are always in a known state when the command starts.
  • For the rotational PID, rotControl.calculate(driveState.getCurrentDriveStats().Pose.getRotation().getDegrees(), targetPose.getRotation().getDegrees()) (lines 40-41) is using degrees for the target. Ensure the rotControl PID is configured for degrees if that's the desired unit, or convert to radians for consistency with WPILib's Rotation2d class if it's radian-based. Using Rotation2d.getRadians() for both might be more robust.

Build & Configuration

• build.gradle - Spotless Application: You've added compileJava.dependsOn 'spotlessApply' to build.gradle (line 134). This means that every time you compile Java code, Spotless will attempt to apply formatting changes, potentially modifying files.
  • Recommendation: It's usually preferred to have spotlessCheck run as part of CI/CD or a pre-commit hook to verify formatting, and spotlessApply run manually or as a separate task (e.g., gradle spotlessApply) when developers want to fix formatting. Running spotlessApply on every compile can be unexpected and might interfere with IDE auto-formatting.
  • Educational Context: Separating checking from applying helps maintain a consistent code style across the team without constantly modifying files during routine builds, which can sometimes lead to merge conflicts if not everyone's IDE is perfectly aligned with Spotless.

Robot Safety

• Current Limiting for Drive Motors: In src/main/java/frc/robot/generated/TunerConstants.java, the steerInitialConfigs explicitly set a StatorCurrentLimit (lines 46-51), which is great! However, the driveInitialConfigs (line 44) are an empty new TalonFXConfiguration().
  • Recommendation: It might be helpful to explicitly add current limits for the drive motors as well, to protect them from excessive currents during high-load situations. Even if there are default limits, explicitly setting them makes it clear in the code.
  • Educational Context: Motors, especially drive motors, can draw very high currents under stall or heavy load. Current limiting protects the motors from overheating and damage, and helps prevent robot brownouts by managing overall power draw.

Questions

• Backup Files (networktables.json.bck, simgui-ds.json): I noticed networktables.json.bck and simgui-ds.json were added. Are these intended to be committed, or are they temporary files/backups? It's generally good practice to keep generated or temporary files out of version control.
• BuildConstants.java in .gitignore: src/main/java/frc/robot/BuildConstants.java was added to .gitignore, but the file itself doesn't appear in the PR. Is this a generated file that will be created by a new Gradle task, or is it a placeholder for future build information? The Robot.java file seems to reference it, so it's good to understand its origin.

This is a fantastic set of changes that will significantly improve the robot's capabilities and maintainability. Keep up the great work!

---

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