Testing/feb 21

[y0shi]

No description provided.

[github-actions]

AI Code Review

Hey team!

This is a really substantial pull request with a lot of great changes, especially in organizing the PathPlanner autos and paths, and beginning to refine the motor control for various subsystems. It's awesome to see so much progress!

Positive Highlights

• Excellent PathPlanner Organization: The use of folders for autos and paths (e.g., Comp Autos, Simple Autos, Basic Comp Paths - LEFT/RIGHT) is a fantastic practice for clarity and managing different autonomous routines. This will make it much easier to select and modify autos.
• Motor ID Management: Adding the motor-ids.txt file is a very helpful practice! It provides a quick reference for CAN IDs, which is super useful for avoiding conflicts and debugging hardware issues.
• SysId Integration and Debugging Tools: Integrating SignalLogger controls into RobotContainer and adding SysIdRoutine commands for the shooter is excellent! These tools are invaluable for debugging, logging, and accurately tuning the robot's performance, setting us up for success.

Suggestions

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

Code Quality & FRC/WPILib Best Practices

1. src/main/deploy/pathplanner/paths/Forward Simple.path and Rotation Simple.path - Folder Name Typo:

   • Suggestion: In the folder field for these PathPlanner paths, it looks like "Simple Pahs" might be a typo for "Simple Paths".
   • Why it matters: While it won't break anything, consistent and correctly spelled folder names make it easier for everyone on the team to navigate and understand the project structure.

2. src/main/java/frc/robot/RobotContainer.java - setExtendNoPID().repeatedly() usage:

   • Suggestion: For the intake extension and retraction commands (e.g., driverJoystick.rightTrigger().whileTrue(intake.setExtendNoPID().repeatedly())), consider making the setExtendNoPID() and setRetractNoPID() methods return a run(() -> extensionMotor.set(percent)) command directly, rather than runOnce().
   • Why it matters: runOnce() creates a command that runs its initialize method once and then immediately finishes. When used with repeatedly() and whileTrue(), it effectively re-initializes and finishes on every scheduler tick while the button is held. While this works for setting a constant percent output, run() is designed for continuous execution and is generally clearer and potentially more efficient for actions that should persist as long as the command is scheduled.

3. src/main/java/frc/robot/subsystems/indexer/IndexerSubsystem.java - Re-enable PID control:

   • Suggestion: Just a friendly reminder to uncomment and re-enable the PID control for the indexer motor in the periodic() method (indexerMotor.setControl(...)) once you are done with "NoPID" testing.
   • Why it matters: PID control is crucial for precise and consistent motor movements, which will be necessary for reliable robot performance during matches.

4. src/main/java/frc/robot/subsystems/shooter/ShooterConstants.java - Flywheel Motor Inversion:

   • Suggestion: The InvertedValue for the leader and follower flywheel motors has been swapped in the createLeaderMotorOutputConfigs() and createFollowerMotorOutputConfigs() methods. Please double-check the physical wiring and the desired direction of rotation on the robot to ensure these InvertedValue settings are correct.
   • Why it matters: Incorrect motor inversion can lead to unexpected behavior, motors fighting each other, or even potential damage to the mechanism if not caught early.

5. src/main/java/frc/robot/subsystems/shooter/SimpleShooterSubsystem.java - Flywheel Follower in launchLemonsCommandNoPID:

   • Potential Bug: In launchLemonsCommandNoPID(), it looks like flywheelMotorFollower is not being set to the flywheelLaunchPercent.getValue(). Only the flywheelMotorLeader is set.
   • Why it matters: If both motors are meant to spin the flywheel, the follower motor needs to receive a control command (or be explicitly set to follow the leader's percent output, if that's the intent for percent control). Otherwise, the flywheel might not spin correctly or efficiently, potentially affecting shot consistency.

6. src/main/java/frc/robot/subsystems/shooter/SimpleShooterSubsystem.java - Hood Command Robustness:

   • Suggestion: The setHoodCommand() in SimpleShooterSubsystem currently sets the target but doesn't wait for the hood to reach that position. Consider adding andThen(Commands.waitUntil(() -> atHoodSetpoint())) to this command, similar to how it's done in the IntakeSubsystem's setIntakeExtensionCommand().
   • Why it matters: For consistent shot accuracy, the hood should ideally reach its target position before the flywheel starts spinning or before a note is launched. Without waiting, the robot might attempt to shoot while the hood is still moving, leading to inconsistent results.

7. src/main/java/frc/robot/subsystems/vision/CameraConstants.java - Camera Transform Validation:

   • Suggestion: The camera transform values have been updated. Please ensure these new values (-0.1651, 0, 0.7191 for translation and -20 degrees for rotation) are thoroughly validated on the robot using a known AprilTag or target.
   • Why it matters: Accurate camera transforms are fundamental for reliable pose estimation and vision-based targeting. Even small inaccuracies can significantly impact robot performance in autonomous and teleoperated modes.

Java Standards

1. src/main/java/frc/robot/subsystems/shooter/SimpleShooterSubsystem.java - run vs runOnce:
   • Suggestion: In launchLemonsCommandNoPID(), the flywheel motors are set using run(() -> { ... }). However, in stopLaunchLemonsNoPIDCommand(), they are also set using run(() -> { ... }). For actions that should only happen once when a command starts, runOnce() is typically preferred for clarity. If the intent is for the motor to continuously apply that percent output as long as the command is scheduled, run() is correct. Given the andThen structure, runOnce might be more semantically appropriate if the motors are meant to hold that percent output until a subsequent command (like stopping) is scheduled.
   • Why it matters: Using runOnce for instantaneous actions and run for continuous actions improves code readability and makes the intent of the command clearer to other developers.

Questions

• Could you share the reasoning behind commenting out the driverJoystick.a().whileTrue(drivetrain.applyRequest(() -> brake)); and driverJoystick.b().whileTrue(...) commands in RobotContainer.java? Are these features being temporarily removed, or is there a new approach planned?
• What is the primary goal of using SimpleShooterSubsystem over ShooterSubsystem at this stage? Is it purely for basic testing, or are there plans to re-integrate the MappedLaunchRequestBuilder logic into the SimpleShooterSubsystem later?

Keep up the fantastic work! This is great progress and

---

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!

[github-actions]

✓ Build successful and code formatting check passed!