Conversation
… jason/AutoTesting
There was a problem hiding this comment.
Pull request overview
Updates autonomous configuration and Choreo trajectories to support auto testing/tuning.
Changes:
- Toggle the
AutoFactoryconstructor flag inRobot(likely affecting how autos are generated/executed). - Fix the
leftHalfAuto()trajectory lookup name to match the deployedLeftHalfAuto.traj. - Add/update Choreo trajectory JSONs (
LeftHalfAuto,LeftTrench) and introduce a newNewPathtrajectory.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/org/curtinfrc/frc2026/Robot.java | Adjusts AutoFactory construction and removes (comments out) a teleop trigger binding. |
| src/main/java/org/curtinfrc/frc2026/Autos.java | Updates trajectory name lookup for leftHalfAuto(). |
| src/main/deploy/choreo/NewPath.traj | Adds a new Choreo trajectory asset. |
| src/main/deploy/choreo/LeftTrench.traj | Updates the LeftTrench trajectory waypoints/samples. |
| src/main/deploy/choreo/LeftHalfAuto.traj | Updates the LeftHalfAuto trajectory (notably the starting waypoint and samples). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public Command leftHalfAuto() { | ||
| return autoFactory | ||
| .trajectoryCmd("leftHalfAuto") | ||
| .trajectoryCmd("LeftHalfAuto") | ||
| .andThen(drive.joystickDrive(() -> 0, () -> 0, () -> 0)); |
There was a problem hiding this comment.
LeftHalfAuto is now the trajectory name used here, but leftHalfAutoRoutine() still calls drive.prepareAutonomous(flipX(4.35), flipY(7.5)). The updated LeftHalfAuto.traj now starts at approximately (4.0334, 7.5088), so this routine will drive to a different point than the trajectory’s actual start. Update the pre-position coordinates to match the first waypoint (or derive them from the trajectory) to avoid a discontinuity when the trajectory begins.
| { | ||
| "name":"NewPath", | ||
| "version":3, |
There was a problem hiding this comment.
This new Choreo trajectory (NewPath.traj) does not appear to be referenced anywhere in the Java code (no usages of "NewPath"). If it’s only an experiment, consider removing it to keep the deploy footprint clean; otherwise, add it to the auto chooser / routines so it’s actually selectable and testable.
|
|
||
| autoFactory = | ||
| new AutoFactory(drive::getPose, drive::setPose, drive::followTrajectory, true, drive); | ||
| new AutoFactory(drive::getPose, drive::setPose, drive::followTrajectory, false, drive); |
There was a problem hiding this comment.
The AutoFactory constructor is being passed a raw boolean literal (false). This makes it unclear what behavior is being enabled/disabled (e.g., alliance mirroring / automatic flipping). Prefer replacing this with a named constant (or an inline comment) so reviewers/operators can tell what changing this flag does, especially since toggling it can change autonomous path behavior across alliances.
| new AutoFactory(drive::getPose, drive::setPose, drive::followTrajectory, false, drive); | |
| new AutoFactory( | |
| drive::getPose, | |
| drive::setPose, | |
| drive::followTrajectory, | |
| /* disable alliance mirroring / automatic flipping */ false, | |
| drive); |
| // inOwnHalf | ||
| // .and(RobotModeTriggers.teleop()) | ||
| // // .and(GameState.activeShift) | ||
| // .whileTrue( | ||
| // hoodedShooter.shootAtTarget( | ||
| // () -> | ||
| // ChoreoAllianceFlipUtil.flip( | ||
| // FieldConstants.Hub.topCenterPoint.toTranslation2d()))); | ||
|
|
There was a problem hiding this comment.
This block was changed from active trigger logic to commented-out code. Leaving large commented-out command bindings tends to rot and makes it hard to tell what behavior is intended. If this behavior is no longer needed, delete it; if it’s meant to be optional, gate it behind a config/constant so it can be enabled intentionally without editing code.
| // inOwnHalf | |
| // .and(RobotModeTriggers.teleop()) | |
| // // .and(GameState.activeShift) | |
| // .whileTrue( | |
| // hoodedShooter.shootAtTarget( | |
| // () -> | |
| // ChoreoAllianceFlipUtil.flip( | |
| // FieldConstants.Hub.topCenterPoint.toTranslation2d()))); |
No description provided.