Skip to content

Hotfix/dalton#48

Merged
y0shi merged 32 commits intodevelopmentfrom
hotfix/dalton
Mar 8, 2026
Merged

Hotfix/dalton#48
y0shi merged 32 commits intodevelopmentfrom
hotfix/dalton

Conversation

@Orcasphynx
Copy link
Collaborator

No description provided.

@Orcasphynx Orcasphynx requested a review from y0shi March 8, 2026 14:16
@github-actions
Copy link

github-actions bot commented Mar 8, 2026

✓ Build successful and code formatting check passed!

@github-actions
Copy link

github-actions bot commented Mar 8, 2026

AI Code Review

Hey team!

Overall, this pull request introduces a fantastic set of new autonomous routines and makes several thoughtful refinements to your command structure and subsystem logic. It's clear a lot of effort went into enhancing your robot's autonomous capabilities, which is awesome!

Positive Highlights:

  • PathPlanner Integration: The numerous new PathPlanner auto routines and the robust use of NamedCommands for seamless integration are excellent! This will greatly enhance your robot's autonomous capabilities.
  • Command Design: The change in IndexerSubsystem.stopFullIndexingNoPID() from run to runOnce is a great improvement for command lifecycle management. Also, the use of WaitCommand in collectNoPIDCommand() and stowNoPIDCommand() in IntakeSubsystem is a smart way to handle non-PID controlled movements, ensuring actions have time to complete.
  • Architecture Refinement: Moving the configureSubsystemDefaultCommands() and configureTeleopBindings() calls to the RobotContainer constructor in Robot.java is a significant architectural improvement, ensuring these are set up correctly and only once.
  • Data Mapping: The simplification of MappedLaunchRequestBuilder to use InterpolatingDoubleTreeMap directly for hood angles is a clean and efficient update for your shooter logic.

Suggestions:

  • File Management (networktables.json.bck):

    • networktables.json.bck: It looks like this file might be a backup of your NetworkTables preferences. If it's not intended to be part of the active project, consider removing it from the repository or adding it to your .gitignore file. Committing .bck files can sometimes lead to confusion about which file is the active configuration.
  • Constants Clarity (Constants.java):

    • src/main/java/frc/robot/Constants.java (line 45): The change to BLUE_LEFT_PASS from (7, 1) to (1, 7) is a substantial change in coordinates (X and Y values swapped). It might be helpful to add a comment explaining why this change was made, or confirm it's the desired field position.
  • Command Bindings & Redundancy (RobotContainer.java):

    • src/main/java/frc/robot/RobotContainer.java (lines 173, 201): The driverJoystick.a() button is bound in both configureTestBindings() and configureTeleopBindings(). While configureTestBindings() is only called in test mode, it's generally good practice to ensure there are no unintended overlaps or that one binding doesn't accidentally override another if the robot transitions modes in an unexpected way. Maybe consider using different buttons for test mode or explicitly clearing bindings when entering test mode.
    • src/main/java/frc/robot/RobotContainer.java (lines 206, 211): Both operatorJoystick.leftTrigger() and operatorJoystick.leftBumper() are configured to call shooter.spinFlywheelRanged(). Is this intended redundancy, or was one of these meant to trigger a different command?
    • src/main/java/frc/robot/RobotContainer.java (line 250): The withRotationalDeadband was removed from getDriveAndLaunchRequest. Was this intentional? Sometimes a small rotational deadband can help prevent slight drift, especially when drivers aren't actively rotating.
  • Code Cleanup (IntakeSubsystem.java):

    • src/main/java/frc/robot/subsystems/intake/IntakeSubsystem.java (lines 64-68): There's commented-out code in the periodic() method. It's generally a good idea to remove commented-out code that is no longer needed to keep the codebase clean and easy to read.
  • Shooter Hardcoding (ShooterSubsystem.java):

    • src/main/java/frc/robot/subsystems/shooter/ShooterSubsystem.java (lines 161-165): The spinFlywheelHardCoded() command hardcodes specific velocity and hood targets. This is perfectly fine for specific autonomous sequences or testing, but it's worth remembering that these values won't be easily tunable through NetworkTables or preferences. If these values are critical for competition performance, consider if they should be exposed as preferences or constants.

Questions:

  1. Regarding networktables.json.bck, is this file intended to be part of the repository, or could it be removed/ignored?
  2. Could you confirm the coordinate change for BLUE_LEFT_PASS in Constants.java (X and Y swap) is correct for the field layout?
  3. Are the duplicate joystick bindings in RobotContainer.java for driverJoystick.a() and operatorJoystick.leftTrigger()/leftBumper() intentional?
  4. Was the removal of withRotationalDeadband from getDriveAndLaunchRequest in RobotContainer.java intentional, and if so, what was the reasoning?

Keep up the great work, team! This PR shows a lot of thoughtful improvements and new features for autonomous. Looking forward to seeing them in action on the robot!


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

@y0shi y0shi merged commit 4917cd6 into development Mar 8, 2026
2 checks passed
@y0shi y0shi deleted the hotfix/dalton branch March 9, 2026 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants