Skip to content

Citris Auton#67

Open
Orcasphynx wants to merge 3 commits intodevelopmentfrom
feature/citrus
Open

Citris Auton#67
Orcasphynx wants to merge 3 commits intodevelopmentfrom
feature/citrus

Conversation

@Orcasphynx
Copy link
Copy Markdown
Collaborator

No description provided.

@Orcasphynx Orcasphynx requested a review from y0shi March 18, 2026 00:19
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 18, 2026

AI Code Review

Hello team!

This is a great pull request, showing fantastic progress on your autonomous routines and incorporating some new paths. It's exciting to see the robot's capabilities expanding!

Positive Highlights

  1. Effective Use of PathPlanner: The addition of many new autonomous paths and sequences demonstrates a solid understanding of how to leverage PathPlanner for building complex and precise autonomous routines. This is a crucial skill for FRC.
  2. Command Composition in Autos: The HP Shoot.auto file shows thoughtful use of parallel and sequential command composition. This is a powerful feature of WPILib's command-based framework, allowing you to coordinate multiple robot actions efficiently.
  3. Named Commands for Integration: Registering commands with NamedCommands in RobotContainer is an excellent practice. It seamlessly integrates your custom robot behaviors into PathPlanner sequences, making your autonomous logic modular and readable.

Suggestions

Here are a few suggestions to make these changes even better and more robust:

Code Quality & Maintenance

  • Resolve Merge Conflicts in Path Files:
    • File: src/main/deploy/pathplanner/paths/Back To Shoot.path
    • File: src/main/deploy/pathplanner/paths/Back to HP Shot.path
    • Suggestion: It looks like these path files have unresolved merge conflict markers (<<<<<<< HEAD, =======, >>>>>>> development). This typically happens when changes from different branches are merged without fully resolving all conflicts. It would be helpful to resolve these to ensure PathPlanner can correctly read and execute the paths without unexpected behavior.
  • File Naming Consistency for Autos:
    • File: src/main/deploy/pathplanner/autos/Citris copy.auto
    • Suggestion: The name "Citris copy.auto" suggests this might be a temporary or duplicated file. To keep your auto folder organized and clear, consider renaming it to something more descriptive and permanent (e.g., "Citris_3Piece.auto" or "Citris_Test.auto" if it's still experimental). This helps avoid confusion about which auto to use in competition.

FRC/WPILib Best Practices

  • Clarity of AutonShoot Timing in HP Shoot.auto:
    • File: src/main/deploy/pathplanner/autos/HP Shoot.auto
    • Suggestion: In the HP Shoot.auto sequence, you have a parallel command that runs AutonShoot alongside a sequential command (which waits for 3 seconds, then calls Stow Intake).
      • It's worth considering the expected duration of your AutonShoot command. If AutonShoot is designed to be an instant command (e.g., just starts the shooter and immediately finishes), or if it finishes much faster than 3 seconds, the Stow Intake might occur long after the actual shot, or potentially too early if AutonShoot takes longer.
      • If AutonShoot is intended to complete its firing sequence before the intake stows, you might consider having AutonShoot internally handle its timing and then Stow Intake as a subsequent command, or ensure AutonShoot is a command that only finishes once the shot is truly complete. This helps ensure the robot's actions are perfectly coordinated.
  • Mowing Command Definition for Autonomous:
    • File: src/main/java/frc/robot/RobotContainer.java (line 106)
    • Suggestion: The Mowing named command is currently defined as intake.collectNoPIDCommand(), and the comment // make a parameter that runs this action for a set amount of time when at mid-field indicates a desire for a timed action.
      • If "Mowing" in autonomous is indeed intended to run for a specific duration, it would be beneficial to explicitly add a withTimeout(duration) to the command when registering it. For example: intake.collectNoPIDCommand().withTimeout(Constants.MOWING_DURATION). This way, the auto sequence will reliably move on after the specified time.

Java Standards

  • Clarity of Mowing Command Name:
    • File: src/main/java/frc/robot/RobotContainer.java (line 106)
    • Suggestion: While "Mowing" might be a fun and descriptive name internally, for general readability and maintainability, especially for new team members or external reviewers, a command name that more directly describes the robot's action could be beneficial. Something like "CollectForTimedDuration" or "MidfieldCollect" might be more immediately understandable in the context of an autonomous routine.

Questions

  • Could you clarify the intended behavior of the AutonShoot command in HP Shoot.auto? Does it complete quickly (e.g., just starts the shooter), or does it represent a longer action (e.g., spins up, shoots, then stops)? Understanding this will help ensure the parallel Stow Intake command executes at the right time.
  • For the "Mowing" command in RobotContainer, is the intention for intake.collectNoPIDCommand() to run for a specific duration, or until interrupted? The comment suggests a timed action, but the current code runs it indefinitely.

Overall, this is a really valuable set of changes for your robot's autonomous capabilities! Addressing these points will make your auto modes even more robust and easier to understand. Keep up the excellent work!


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

@github-actions
Copy link
Copy Markdown

✓ Build successful and code formatting check passed!

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

✓ Build successful and code formatting check passed!

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.

2 participants