Conversation
📝 WalkthroughWalkthroughAdds ~35 new trajectory/choreo data files, removes one legacy trajectory, updates several existing trajectories, enhances autonomous event-marker registration and per-trajectory event binding, makes swerve odometry thread-safe with a high-frequency notifier, updates PID gains and build metadata, and tweaks sim GUI settings. Changes
Sequence Diagram(s)sequenceDiagram
participant Chooser as AutoChooser
participant AutoUtils
participant Registry as EventRegistry
participant Builder as TrajectoryBuilder
participant Factory as ChoreoFactory
participant Runner as TrajectoryCommand
Chooser->>AutoUtils: request chooser/factory
AutoUtils->>Registry: registerEventMarker("score"/"climb"/"intake", Supplier/Command)
Chooser->>Builder: trajectory(name)
Builder->>Registry: withAllEventMarkers() / bindEvent(eventName, marker)
Builder->>Factory: build(trajectoryData)
Factory-->>Runner: trajectory command (with bound event hooks)
Runner->>Runner: sample trajectory over time
Runner->>Registry: check triggered events
Registry-->>Runner: provide Command/Supplier when event fires
Runner->>Runner: execute bound command
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java (3)
248-249:⚠️ Potential issue | 🔴 CriticalHeading and omega flip logic appears inverted.
When
isFlippedisfalse(no alliance flip needed), the heading and omega are negated. WhenisFlippedistrue, they're passed through unchanged. This is backwards compared to the single-argumentdriveToSample(line 228) which usessample.headingdirectly without negation.// Current (likely inverted): Rotation2d heading = isFlipped ? new Rotation2d(sample.heading) : new Rotation2d(-sample.heading); Rotation2d omega = isFlipped ? new Rotation2d(sample.omega) : new Rotation2d(-sample.omega);Expected: the
isFlippedbranch should transform heading/omega, and the non-flipped branch should preserve them:🐛 Proposed fix
- Rotation2d heading = isFlipped ? new Rotation2d(sample.heading) : new Rotation2d(-sample.heading); - Rotation2d omega = isFlipped ? new Rotation2d(sample.omega) : new Rotation2d(-sample.omega); + Rotation2d heading = isFlipped ? new Rotation2d(-sample.heading) : new Rotation2d(sample.heading); + Rotation2d omega = isFlipped ? new Rotation2d(-sample.omega) : new Rotation2d(sample.omega);
115-117:⚠️ Potential issue | 🟠 MajorExposing
SwerveDrivePoseEstimatordirectly underminesodometryLocksynchronization.
getSwerveDrivePoseEstimator()hands out the raw estimator reference, allowing external callers (e.g., vision subsystem adding measurements) to mutate it without acquiringodometryLock. This creates a race with the notifier thread'supdate()call. Either remove this accessor or provide a synchronized wrapper method for external operations (e.g.,addVisionMeasurement(...)).
377-394:⚠️ Potential issue | 🔴 CriticalGyro data is updated in
periodic()but consumed asynchronously by the Notifier — no happens-before guarantee.
gyro.updateData()(line 382) writes togyroDataon the main thread. The notifier thread readsgyroData.orientation(line 303) at 200 Hz independently. Without a memory barrier (synchronized block, volatile, or atomic), the notifier thread may see stale or partially written orientation data. This is the read-side of the race condition flagged on the constructor.Consider either:
- Moving
gyro.updateData()into the notifier thread (so it owns both the read and write), or- Protecting
gyroDataaccess withodometryLockon both sides.2026/src/main/java/frc/robot/commands/auto/AutoUtils.java (2)
125-135:⚠️ Potential issue | 🔴 CriticalNullPointerException when event marker is not registered.
When
markeris not found ineventMarkerCommands, line 128 prints a warning but execution continues to line 133, whereeventMarkerCommands.get(marker)returnsnull. Calling.get()onnullthrows an NPE.🐛 Proposed fix — add `continue` after the warning
public static void applyEventMarkers(AutoTrajectory traj, String... markers) { for (String marker : markers) { if (!eventMarkerCommands.containsKey(marker)) { System.out.println("[AutoUtils]: No command found for event marker: " + marker); + continue; } traj.atPose(marker, Accuracy.DRIVE_TRANSLATE_TOLERANCE.in(Meters), Accuracy.DRIVE_ROTATION_TOLERANCE.in(Radians)) .onTrue(eventMarkerCommands.get(marker).get()); } }
87-89:⚠️ Potential issue | 🟠 MajorCommand reuse risk in
registerEventMarker(String, Command).Wrapping a single
Commandinstance in() -> commandreturns the same object every time the supplier is called. In WPILib, aCommandinstance cannot be scheduled if it's already running or hasn't been properly reset. If this event marker fires across multiple auto runs (or multiple trajectories in one auto), the second invocation will silently fail or throw.The
Supplier<Command>overload (line 79) is the safe variant. Consider deprecating or removing theCommandoverload to prevent misuse, or wrapping it properly.🛡️ Option: Remove the unsafe overload or document the single-use constraint
- public static void registerEventMarker(String name, Command command) { - eventMarkerCommands.put(name, () -> command); - }If you keep it, at minimum wrap it to create a fresh instance each time (though that's only possible with a
Supplier). Alternatively, add a Javadoc warning:/** * Registers a global event marker command that can be used across all * trajectories. + * <p><b>Warning:</b> The same Command instance is reused. Only use for + * commands that are safe to re-schedule (e.g., InstantCommands). Prefer + * the {`@code` Supplier<Command>} overload for commands with state.</p> */ public static void registerEventMarker(String name, Command command) {
🤖 Fix all issues with AI agents
In `@2026/NewPath.traj`:
- Around line 1-78: The trajectory file NewPath.traj is misplaced at the
repository root and duplicates an existing "New Path.traj" under the deploy
directory; either move NewPath.traj into the deploy folder used by Choreo
(src/main/deploy/choreo/) so the runtime can load it, or delete NewPath.traj if
it is a leftover test artifact to avoid duplication and confusion.
In `@2026/src/main/deploy/choreo/Climb.traj`:
- Around line 21-27: The trajectory object in Climb.traj is empty (the
"trajectory" field has config=null, sampleType=null and empty
waypoints/samples/splits), so runtime will have nothing to follow; regenerate
the trajectory in Choreo to produce non-empty samples/waypoints/splits (or
export a proper motion profile) and replace Climb.traj, or if this file is
intended only as a single-goal pose, populate a minimal waypoint entry and
update the consumer code to read that waypoint instead of expecting samples;
look for the "trajectory" JSON object and its keys (config, sampleType,
waypoints, samples, splits) when making the fix.
In `@2026/src/main/deploy/choreo/LeftFarIntake.traj`:
- Around line 21-27: The trajectory block in LeftFarIntake.traj is missing
generated data (trajectory.config and trajectory.sampleType are null and
trajectory.waypoints/trajectory.samples are empty), so either mark this file
explicitly as a placeholder or regenerate the trajectory in Choreo; open
LeftFarIntake.traj in Choreo, run the trajectory generation for the defined
waypoint(s), and update the file so trajectory.config and trajectory.sampleType
are non-null and trajectory.waypoints/trajectory.samples contain the generated
data (or, if intentional, add a clear placeholder comment/flag in the file and
any runtime loader to skip/handle placeholder trajectories).
In `@2026/src/main/java/frc/robot/commands/auto/AutoUtils.java`:
- Around line 64-70: The chooser currently builds an inline "Sample" command
with chooser.addCmd(...) calling
trajectory("Sample").bindEvent("score","score").holdFinalPose(false).build(),
which duplicates and conflicts with the registered trajectory created by
AutoUtils.trajectory("Sample").register() in registerAutos(); to fix, remove the
inline "Sample" entry from setupChooser() so chooser.addCmd does not recreate
the trajectory and the chooser will use the registered Sample command from
registerAutos() (update any references in setupChooser or chooser.addCmd to use
the registered name instead); alternatively, if the inline configuration is the
intended one, delete the AutoUtils.trajectory("Sample").register() call in
registerAutos() so only one canonical "Sample" trajectory exists.
- Around line 188-190: The bindEvent(String, Command) overload captures and
reuses a single Command instance via () -> command which can break when commands
hold state or are scheduled multiple times; change the API to accept a
Supplier<Command> (add or replace bindEvent(String, Supplier<Command>)) and
update call sites to pass a supplier that constructs a fresh Command each
invocation (or, if you must keep the existing overload, make it call the new
Supplier-based method by wrapping the Command in a supplier that clones/creates
a new instance); reference TrajectoryBuilder.bindEvent(String, Command) and the
new bindEvent(String, Supplier<Command>) when making the change.
- Around line 296-300: The build() method is adding event bindings to a static
shared factory (factory.bind(...) for each entry in namedEventBindings) causing
bindings to accumulate across builds and create cross-trajectory coupling;
change build() to avoid mutating global state by either clearing or isolating
bindings before binding: e.g., call a reset/clear method on the factory (or
create a fresh factory instance) at the start of AutoUtils.build(), or scope
bindings to a local/factory copy so factory.bind(...) does not permanently
register them; update references to factory, build(), namedEventBindings, and
factory.bind(...) accordingly to ensure bindings are per-build or documented as
intentionally additive.
In `@2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java`:
- Around line 143-155: getRotation() currently reads gyroData.orientation.getZ()
without synchronization while getPose() uses odometryLock; protect getRotation()
by synchronizing on the same odometryLock (wrap the return of
Rotation2d.fromRadians(gyroData.orientation.getZ()) in a
synchronized(odometryLock) block) so reads cannot race with notifier or update
threads, and fix the method indentation so its braces align with the class
style; alternatively, if you decide it's safe to remain unsynchronized, add a
clear comment explaining why (e.g., underlying fields are atomic) and then
correct the indentation.
- Around line 111-112: The odometry thread reads shared sensor state without
locking; wrap all accesses to gyroData.orientation and module encoder state with
the same odometryLock to prevent races: acquire odometryLock inside
updateOdometry() around the sensor reads (gyroData.orientation and
getModulePositions()) and estimator.update call, and likewise make periodic()
acquire odometryLock while it calls gyro.updateData() and each module.periodic()
(or at minimum while it writes the sensor fields read by updateOdometry());
alternatively replace the reads with a signal-synchronized read strategy, but
ensure the chosen approach uses odometryLock consistently around both readers
(updateOdometry) and writers (periodic, gyro.updateData, module.periodic).
- Around line 44-46: The Notifier started in the Swerve constructor is never
stopped; add a close() implementation on the Swerve class that safely stops and
disposes odometryNotifier: add a public void close() (or implement
AutoCloseable) that synchronizes on odometryLock, checks if odometryNotifier !=
null, calls odometryNotifier.close(), sets odometryNotifier = null, and returns;
ensure any places that re-create or dispose the subsystem call this close() to
avoid leaking the Notifier thread.
🧹 Nitpick comments (12)
2026/src/main/deploy/choreo/LeftFarIntakeThrough_Shooter_copy1.traj (1)
1-151: Consider whether this_copy1trajectory file should be committed permanently.The filename
LeftFarIntakeThrough_Shooter_copy1.trajsuggests this is a duplicate or test variant of the existingLeftFarIntakeThrough_Shooter.traj. Since the PR description mentions "test auto paths," confirm whether this copy is intentionally kept as a distinct path variant or is a leftover from experimentation. If it's no longer needed, removing it avoids stale data files accumulating in the deploy directory.The trajectory data itself is well-formed Choreo v3 JSON and the
Shootevent correctly targets the final waypoint timestamp (2.48967 s).2026/src/main/java/frc/robot/subsystems/swerve/gyro/GyroIO.java (2)
14-16: Inconsistent indentation withinGyroDataclass.The field on line 14 has extra spacing before
Rotation3d, line 15 has excessive leading whitespace, and the closing brace on line 16 is misaligned. These should all align to the same indentation level.🔧 Suggested fix
- public Rotation3d orientation = new Rotation3d(0, 0, 0); - public double lastUpdateS = 0.0; - } + public Rotation3d orientation = new Rotation3d(0, 0, 0); + public double lastUpdateS = 0.0; + }
18-24: Remove unusedgetGyroRotationmethod and fix indentation inGyroDataclass.The
getGyroRotationmethod is never called—all callers directly useRotation2d.fromRadians(gyroData.orientation.getZ())(at lines 144, 200, 303, 323 inSwerve.java). Remove the dead code.Additionally,
GyroData(lines 13–16) and thegetGyroRotationmethod body (lines 18–24) have inconsistent indentation. Fix indentation to be consistent with the interface structure (4 spaces for method, 8 for body content).If you decide to keep the method, consider making it
staticsince it doesn't depend on instance state—default interface methods that take their own data class as a parameter are unusual.2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java (3)
289-305: Remove commented-out code blocks.There are two commented-out implementations of
updateOdometry(lines 289-293 and 297-301). These add noise and are preserved in version control. Remove them to keep the codebase clean.
313-337: Indentation:reset()body is misaligned with the class.The method declaration starts at column 0 and the body has inconsistent indentation. This is a pattern throughout the file — several methods (
getRotation,getPose,setOdometry,updateOdometry,reset) have their bodies at varying indent levels. A single formatting pass would improve readability significantly.
237-243: Debug telemetry with informal key names — consider cleanup.
"getpointx"and"getpointy"look like debug artifacts. The same data is logged to bothSmartDashboardandLogger, which is redundant. Consider removing theSmartDashboardcalls and keeping only the structuredLogger.recordOutputentries (or vice versa), and use consistent naming (e.g.,"Swerve/Auto/TargetX").2026/src/main/deploy/choreo/New Path.traj (1)
1-35: Placeholder trajectory — consider removing before merge.This file appears to be a default Choreo template ("New Path" with a space in the filename). The trajectory section is empty (no config, no samples), and the generic name suggests it's not an intentional autonomous path. Leftover placeholder files add clutter to the deploy directory.
2026/src/main/java/frc/robot/commands/auto/Autos.java (2)
13-16: Duplicate event marker registration — static defaults inAutoUtilsare silently overwritten.
AutoUtilshas a static initializer block (lines 35–41 of AutoUtils.java) that registers"score","climb", and"intake"with placeholder print commands. ThenregisterEventMarkers()here overwrites"score"and"intake"with different messages. This makes the static defaults dead code and creates confusion about which registration is authoritative.Consider removing the defaults from the
AutoUtilsstatic block entirely, and makingAutos.registerEventMarkers()the single source of truth for all event markers (including"climb"if needed).
43-59:tripleThreatAutosequencing looks correct, but consider consistency onresetOdometry.
trajectory1relies on the defaultresetOdometry = true(set inTrajectoryBuilder), whiletrajectory2andtrajectory3explicitly set.resetOdometry(false). This is functionally correct — the first trajectory resets odometry and subsequent ones don't — but the implicit default fortrajectory1is easy to miss.Consider adding
.resetOdometry(true)explicitly ontrajectory1for readability and to make the intent clear to future readers.Suggested change for clarity
Command trajectory1 = AutoUtils.trajectory("Start1_LeftNearIntake") + .resetOdometry(true) .holdFinalPose(false) .build();2026/src/main/java/frc/robot/commands/auto/AutoUtils.java (3)
33-41: Static initializer defaults conflict withAutos.registerEventMarkers().These default registrations for
"score","climb", and"intake"are overwritten byAutos.registerEventMarkers()(for"score"and"intake"), making them dead code. The"climb"marker remains with the placeholder"climbed!!!"message, which appears to be a leftover from testing.Consider removing this static block and making
Autos.registerEventMarkers()the sole registration point for all markers.♻️ Proposed cleanup
- private static Map<String, Supplier<Command>> eventMarkerCommands = new HashMap<>(); - - static { - // Initialize default event markers - eventMarkerCommands.put("score", () -> Commands.print("scored!!!")); - eventMarkerCommands.put("climb", () -> Commands.print("climbed!!!")); - eventMarkerCommands.put("intake", () -> Commands.print("intake!!!")); - - } + private static final Map<String, Supplier<Command>> eventMarkerCommands = new HashMap<>();
330-336: Logging initial/final poses with AdvantageKit is a nice observability addition.The
Logger.recordOutput(...)calls provide good telemetry for debugging auto paths.Minor note:
traj.getInitialPose().orElse(Pose2d.kZero)and thefinalPosefallback toPose2d.kZeroon line 317 — if a trajectory file is missing or malformed, this silently logs(0,0,0)rather than surfacing the error. Consider logging a warning whengetInitialPose()orgetFinalPose()returns empty.
232-234: Same command-reuse pattern inatPose(String, Command)andatTime(double, Command).These wrap a single
Commandin() -> command, same pattern as discussed above. Consistent with the rest of the API but carries the same single-use risk. If you address the pattern inbindEvent/registerEventMarker, apply the same treatment here.
| { | ||
| "name":"NewPath", | ||
| "version":3, | ||
| "snapshot":{ | ||
| "waypoints":[ | ||
| {"x":3.495450019836426, "y":4.112559795379639, "heading":3.141592653589793, "intervals":20, "split":false, "fixTranslation":true, "fixHeading":true, "overrideIntervals":false}, | ||
| {"x":1.526960015296936, "y":3.5083699226379395, "heading":3.141592653589793, "intervals":40, "split":false, "fixTranslation":true, "fixHeading":true, "overrideIntervals":false}], | ||
| "constraints":[ | ||
| {"from":"first", "to":null, "data":{"type":"StopPoint", "props":{}}, "enabled":true}, | ||
| {"from":"last", "to":null, "data":{"type":"StopPoint", "props":{}}, "enabled":true}, | ||
| {"from":"first", "to":"last", "data":{"type":"KeepInRectangle", "props":{"x":0.0, "y":0.0, "w":16.541, "h":8.0692}}, "enabled":false}], | ||
| "targetDt":0.05 | ||
| }, | ||
| "params":{ | ||
| "waypoints":[ | ||
| {"x":{"exp":"3.495450019836426 m", "val":3.495450019836426}, "y":{"exp":"4.112559795379639 m", "val":4.112559795379639}, "heading":{"exp":"3.141592653589793 rad", "val":3.141592653589793}, "intervals":20, "split":false, "fixTranslation":true, "fixHeading":true, "overrideIntervals":false}, | ||
| {"x":{"exp":"1.526960015296936 m", "val":1.526960015296936}, "y":{"exp":"3.5083699226379395 m", "val":3.5083699226379395}, "heading":{"exp":"3.141592653589793 rad", "val":3.141592653589793}, "intervals":40, "split":false, "fixTranslation":true, "fixHeading":true, "overrideIntervals":false}], | ||
| "constraints":[ | ||
| {"from":"first", "to":null, "data":{"type":"StopPoint", "props":{}}, "enabled":true}, | ||
| {"from":"last", "to":null, "data":{"type":"StopPoint", "props":{}}, "enabled":true}, | ||
| {"from":"first", "to":"last", "data":{"type":"KeepInRectangle", "props":{"x":{"exp":"0 m", "val":0.0}, "y":{"exp":"0 m", "val":0.0}, "w":{"exp":"16.541 m", "val":16.541}, "h":{"exp":"8.0692 m", "val":8.0692}}}, "enabled":false}], | ||
| "targetDt":{ | ||
| "exp":"0.05 s", | ||
| "val":0.05 | ||
| } | ||
| }, | ||
| "trajectory":{ | ||
| "config":{ | ||
| "frontLeft":{ | ||
| "x":0.2794, | ||
| "y":0.2794 | ||
| }, | ||
| "backLeft":{ | ||
| "x":-0.2794, | ||
| "y":0.2794 | ||
| }, | ||
| "mass":68.0388555, | ||
| "inertia":6.0, | ||
| "gearing":6.5, | ||
| "radius":0.0508, | ||
| "vmax":628.3185307179587, | ||
| "tmax":1.2, | ||
| "cof":1.5, | ||
| "bumper":{ | ||
| "front":0.4064, | ||
| "side":0.4064, | ||
| "back":0.4064 | ||
| }, | ||
| "differentialTrackWidth":0.5588 | ||
| }, | ||
| "sampleType":"Swerve", | ||
| "waypoints":[0.0,0.95602], | ||
| "samples":[ | ||
| {"t":0.0, "x":3.49545, "y":4.11256, "heading":3.14159, "vx":0.0, "vy":0.0, "omega":0.0, "ax":-8.62188, "ay":-2.64632, "alpha":0.0, "fx":[-146.65577,-146.65577,-146.65577,-146.65577], "fy":[-45.01315,-45.01315,-45.01315,-45.01315]}, | ||
| {"t":0.0478, "x":3.4856, "y":4.10954, "heading":3.14159, "vx":-0.41214, "vy":-0.1265, "omega":0.0, "ax":-8.62099, "ay":-2.64605, "alpha":0.0, "fx":[-146.64056,-146.64056,-146.64056,-146.64056], "fy":[-45.00848,-45.00848,-45.00848,-45.00848]}, | ||
| {"t":0.0956, "x":3.45605, "y":4.10047, "heading":3.14159, "vx":-0.82423, "vy":-0.25298, "omega":0.0, "ax":-8.61986, "ay":-2.6457, "alpha":0.0, "fx":[-146.62131,-146.62131,-146.62131,-146.62131], "fy":[-45.00257,-45.00257,-45.00257,-45.00257]}, | ||
| {"t":0.1434, "x":3.4068, "y":4.08535, "heading":3.14159, "vx":-1.23627, "vy":-0.37945, "omega":0.0, "ax":-8.61838, "ay":-2.64524, "alpha":0.0, "fx":[-146.59613,-146.59613,-146.59613,-146.59613], "fy":[-44.99484,-44.99484,-44.99484,-44.99484]}, | ||
| {"t":0.1912, "x":3.33786, "y":4.06419, "heading":3.14159, "vx":-1.64823, "vy":-0.50589, "omega":0.0, "ax":-8.61636, "ay":-2.64462, "alpha":0.0, "fx":[-146.56179,-146.56179,-146.56179,-146.56179], "fy":[-44.9843,-44.9843,-44.9843,-44.9843]}, | ||
| {"t":0.23901, "x":3.24923, "y":4.03699, "heading":3.14159, "vx":-2.0601, "vy":-0.63231, "omega":0.0, "ax":-8.61344, "ay":-2.64373, "alpha":0.0, "fx":[-146.51219,-146.51219,-146.51219,-146.51219], "fy":[-44.96908,-44.96908,-44.96908,-44.96908]}, | ||
| {"t":0.28681, "x":3.14091, "y":4.00374, "heading":3.14159, "vx":-2.47184, "vy":-0.75868, "omega":0.0, "ax":-8.60886, "ay":-2.64232, "alpha":0.0, "fx":[-146.43426,-146.43426,-146.43426,-146.43426], "fy":[-44.94516,-44.94516,-44.94516,-44.94516]}, | ||
| {"t":0.33461, "x":3.01292, "y":3.96446, "heading":3.14159, "vx":-2.88335, "vy":-0.88499, "omega":0.0, "ax":-8.60062, "ay":-2.63979, "alpha":0.0, "fx":[-146.29401,-146.29401,-146.29401,-146.29401], "fy":[-44.90211,-44.90211,-44.90211,-44.90211]}, | ||
| {"t":0.38241, "x":2.86527, "y":3.91914, "heading":3.14159, "vx":-3.29447, "vy":-1.01117, "omega":0.0, "ax":-8.5814, "ay":-2.63389, "alpha":0.0, "fx":[-145.96709,-145.96709,-145.96709,-145.96709], "fy":[-44.80177,-44.80177,-44.80177,-44.80177]}, | ||
| {"t":0.43021, "x":2.69799, "y":3.86779, "heading":3.14159, "vx":-3.70467, "vy":-1.13708, "omega":0.0, "ax":-8.48582, "ay":-2.60456, "alpha":0.0, "fx":[-144.34134,-144.34134,-144.34134,-144.34134], "fy":[-44.30278,-44.30278,-44.30278,-44.30278]}, | ||
| {"t":0.47801, "x":2.51121, "y":3.81046, "heading":3.14159, "vx":-4.1103, "vy":-1.26158, "omega":0.0, "ax":8.48582, "ay":2.60456, "alpha":0.0, "fx":[144.34134,144.34134,144.34134,144.34134], "fy":[44.30278,44.30278,44.30278,44.30278]}, | ||
| {"t":0.52581, "x":2.32442, "y":3.75314, "heading":3.14159, "vx":-3.70467, "vy":-1.13708, "omega":0.0, "ax":8.5814, "ay":2.63389, "alpha":0.0, "fx":[145.96709,145.96709,145.96709,145.96709], "fy":[44.80177,44.80177,44.80177,44.80177]}, | ||
| {"t":0.57361, "x":2.15714, "y":3.70179, "heading":3.14159, "vx":-3.29447, "vy":-1.01117, "omega":0.0, "ax":8.60062, "ay":2.63979, "alpha":0.0, "fx":[146.29401,146.29401,146.29401,146.29401], "fy":[44.90211,44.90211,44.90211,44.90211]}, | ||
| {"t":0.62141, "x":2.00949, "y":3.65647, "heading":3.14159, "vx":-2.88335, "vy":-0.88499, "omega":0.0, "ax":8.60886, "ay":2.64232, "alpha":0.0, "fx":[146.43426,146.43426,146.43426,146.43426], "fy":[44.94516,44.94516,44.94516,44.94516]}, | ||
| {"t":0.66921, "x":1.8815, "y":3.61719, "heading":3.14159, "vx":-2.47184, "vy":-0.75868, "omega":0.0, "ax":8.61344, "ay":2.64373, "alpha":0.0, "fx":[146.51219,146.51219,146.51219,146.51219], "fy":[44.96908,44.96908,44.96908,44.96908]}, | ||
| {"t":0.71702, "x":1.77318, "y":3.58394, "heading":3.14159, "vx":-2.0601, "vy":-0.63231, "omega":0.0, "ax":8.61636, "ay":2.64462, "alpha":0.0, "fx":[146.56179,146.56179,146.56179,146.56179], "fy":[44.9843,44.9843,44.9843,44.9843]}, | ||
| {"t":0.76482, "x":1.68455, "y":3.55674, "heading":3.14159, "vx":-1.64823, "vy":-0.50589, "omega":0.0, "ax":8.61838, "ay":2.64524, "alpha":0.0, "fx":[146.59613,146.59613,146.59613,146.59613], "fy":[44.99484,44.99484,44.99484,44.99484]}, | ||
| {"t":0.81262, "x":1.61561, "y":3.53558, "heading":3.14159, "vx":-1.23627, "vy":-0.37945, "omega":0.0, "ax":8.61986, "ay":2.6457, "alpha":0.0, "fx":[146.62131,146.62131,146.62131,146.62131], "fy":[45.00257,45.00257,45.00257,45.00257]}, | ||
| {"t":0.86042, "x":1.56636, "y":3.52046, "heading":3.14159, "vx":-0.82423, "vy":-0.25298, "omega":0.0, "ax":8.62099, "ay":2.64605, "alpha":0.0, "fx":[146.64056,146.64056,146.64056,146.64056], "fy":[45.00848,45.00848,45.00848,45.00848]}, | ||
| {"t":0.90822, "x":1.53681, "y":3.51139, "heading":3.14159, "vx":-0.41214, "vy":-0.1265, "omega":0.0, "ax":8.62188, "ay":2.64632, "alpha":0.0, "fx":[146.65577,146.65577,146.65577,146.65577], "fy":[45.01315,45.01315,45.01315,45.01315]}, | ||
| {"t":0.95602, "x":1.52696, "y":3.50837, "heading":3.14159, "vx":0.0, "vy":0.0, "omega":0.0, "ax":0.0, "ay":0.0, "alpha":0.0, "fx":[0.0,0.0,0.0,0.0], "fy":[0.0,0.0,0.0,0.0]}], | ||
| "splits":[0] | ||
| }, | ||
| "events":[] | ||
| } |
There was a problem hiding this comment.
Misplaced trajectory file — should be under src/main/deploy/choreo/ or removed.
This file sits at 2026/NewPath.traj (repository root level) rather than in 2026/src/main/deploy/choreo/ where the rest of the trajectory files reside. The Choreo library and deploy pipeline typically load .traj files from the deploy directory, so this file likely won't be found at runtime. Additionally, a similar New Path.traj already exists in the correct location.
Consider moving this into the deploy directory if it's needed, or deleting it if it's a leftover test artifact.
🤖 Prompt for AI Agents
In `@2026/NewPath.traj` around lines 1 - 78, The trajectory file NewPath.traj is
misplaced at the repository root and duplicates an existing "New Path.traj"
under the deploy directory; either move NewPath.traj into the deploy folder used
by Choreo (src/main/deploy/choreo/) so the runtime can load it, or delete
NewPath.traj if it is a leftover test artifact to avoid duplication and
confusion.
| "trajectory":{ | ||
| "config":null, | ||
| "sampleType":null, | ||
| "waypoints":[], | ||
| "samples":[], | ||
| "splits":[] | ||
| }, |
There was a problem hiding this comment.
Empty trajectory — this file has no generated path data.
config, sampleType, waypoints, samples, and splits are all null/empty. If the autonomous routine attempts to follow this trajectory at runtime, ChoreoLib will have no samples to track, likely resulting in a no-op or error. If this is a single-waypoint goal pose used only for its position data (e.g., read the waypoint and drive-to-pose), this is fine — otherwise, regenerate the trajectory in Choreo before deploying.
🤖 Prompt for AI Agents
In `@2026/src/main/deploy/choreo/Climb.traj` around lines 21 - 27, The trajectory
object in Climb.traj is empty (the "trajectory" field has config=null,
sampleType=null and empty waypoints/samples/splits), so runtime will have
nothing to follow; regenerate the trajectory in Choreo to produce non-empty
samples/waypoints/splits (or export a proper motion profile) and replace
Climb.traj, or if this file is intended only as a single-goal pose, populate a
minimal waypoint entry and update the consumer code to read that waypoint
instead of expecting samples; look for the "trajectory" JSON object and its keys
(config, sampleType, waypoints, samples, splits) when making the fix.
| "trajectory":{ | ||
| "config":null, | ||
| "sampleType":null, | ||
| "waypoints":[], | ||
| "samples":[], | ||
| "splits":[] | ||
| }, |
There was a problem hiding this comment.
Trajectory has not been generated — config and samples are empty.
This file defines a single waypoint but has null config, null sampleType, and empty samples/waypoints arrays in the trajectory section. It appears the trajectory was never generated in Choreo. If this path is referenced at runtime, the trajectory follower will likely fail or produce a no-op. Confirm this is intentional (placeholder for future use) or regenerate the trajectory before merging.
🤖 Prompt for AI Agents
In `@2026/src/main/deploy/choreo/LeftFarIntake.traj` around lines 21 - 27, The
trajectory block in LeftFarIntake.traj is missing generated data
(trajectory.config and trajectory.sampleType are null and
trajectory.waypoints/trajectory.samples are empty), so either mark this file
explicitly as a placeholder or regenerate the trajectory in Choreo; open
LeftFarIntake.traj in Choreo, run the trajectory generation for the defined
waypoint(s), and update the file so trajectory.config and trajectory.sampleType
are non-null and trajectory.waypoints/trajectory.samples contain the generated
data (or, if intentional, add a clear placeholder comment/flag in the file and
any runtime loader to skip/handle placeholder trajectories).
| chooser.addCmd("Taxi", () -> Autos.getTaxiAuto()); | ||
| chooser.addCmd("Sample", () -> trajectory("Sample") | ||
| .bindEvent("score", "score") // Bind the "score" event from trajectory to the "score" command | ||
| .holdFinalPose(false) | ||
| .build()); | ||
| chooser.addCmd("TripleThreatAuto", () -> Autos.tripleThreatAuto()); | ||
| chooser.addCmd("getStart1LeftNear", () -> Autos.getStart1LeftNear()); |
There was a problem hiding this comment.
Inline Sample trajectory in chooser duplicates/bypasses the registration in registerAutos().
registerAutos() already registers "Sample" via AutoUtils.trajectory("Sample").register(). Here in setupChooser(), a different Sample trajectory command is built inline with .bindEvent("score", "score") and .holdFinalPose(false) — different configuration than what's registered. This creates two potentially conflicting "Sample" entries, depending on ordering.
If the intent is to have the chooser use the registered version, remove this inline definition. If the inline version is the desired one, remove the registration in registerAutos().
🤖 Prompt for AI Agents
In `@2026/src/main/java/frc/robot/commands/auto/AutoUtils.java` around lines 64 -
70, The chooser currently builds an inline "Sample" command with
chooser.addCmd(...) calling
trajectory("Sample").bindEvent("score","score").holdFinalPose(false).build(),
which duplicates and conflicts with the registered trajectory created by
AutoUtils.trajectory("Sample").register() in registerAutos(); to fix, remove the
inline "Sample" entry from setupChooser() so chooser.addCmd does not recreate
the trajectory and the chooser will use the registered Sample command from
registerAutos() (update any references in setupChooser or chooser.addCmd to use
the registered name instead); alternatively, if the inline configuration is the
intended one, delete the AutoUtils.trajectory("Sample").register() call in
registerAutos() so only one canonical "Sample" trajectory exists.
| public TrajectoryBuilder bindEvent(String eventName, Command command) { | ||
| return bindEvent(eventName, () -> command); | ||
| } |
There was a problem hiding this comment.
Same command-reuse concern applies to TrajectoryBuilder.bindEvent(String, Command).
() -> command captures a single Command instance. Same risk as registerEventMarker(String, Command) — if the command has internal state or is scheduled more than once, it will fail. This is the downstream manifestation of the same pattern.
🤖 Prompt for AI Agents
In `@2026/src/main/java/frc/robot/commands/auto/AutoUtils.java` around lines 188 -
190, The bindEvent(String, Command) overload captures and reuses a single
Command instance via () -> command which can break when commands hold state or
are scheduled multiple times; change the API to accept a Supplier<Command> (add
or replace bindEvent(String, Supplier<Command>)) and update call sites to pass a
supplier that constructs a fresh Command each invocation (or, if you must keep
the existing overload, make it call the new Supplier-based method by wrapping
the Command in a supplier that clones/creates a new instance); reference
TrajectoryBuilder.bindEvent(String, Command) and the new bindEvent(String,
Supplier<Command>) when making the change.
| public Command build() { | ||
| // Bind all named events from the trajectory file to their commands | ||
| for (Map.Entry<String, Supplier<Command>> entry : namedEventBindings.entrySet()) { | ||
| factory.bind(entry.getKey(), entry.getValue().get()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the AutoUtils.java file
find . -name "AutoUtils.java" -type fRepository: Team-Optix-3749/Team-3749-2026
Length of output: 133
🏁 Script executed:
# Also search for the factory field declaration to understand its scope
rg -n "factory\s*=" --type java -A 2 -B 2 | head -50Repository: Team-Optix-3749/Team-3749-2026
Length of output: 537
🏁 Script executed:
# Check if there are any unbind/clear binding methods in the codebase
rg -n "unbind|clearBind|resetBind|clearBindings" --type javaRepository: Team-Optix-3749/Team-3749-2026
Length of output: 56
🏁 Script executed:
# Let's read the AutoUtils.java file to understand the full context
wc -l ./2026/src/main/java/frc/robot/commands/auto/AutoUtils.javaRepository: Team-Optix-3749/Team-3749-2026
Length of output: 137
🏁 Script executed:
# Read the beginning of the file to see class declaration and field declarations
head -100 ./2026/src/main/java/frc/robot/commands/auto/AutoUtils.javaRepository: Team-Optix-3749/Team-3749-2026
Length of output: 3586
🏁 Script executed:
# Read around line 296-300 to see the build() method context
sed -n '280,320p' ./2026/src/main/java/frc/robot/commands/auto/AutoUtils.javaRepository: Team-Optix-3749/Team-3749-2026
Length of output: 1782
🏁 Script executed:
# Search for where build() is called to understand the usage pattern
rg -n "\.build\(\)" ./2026/src/main/java/frc/robot/commands/auto/AutoUtils.java -B 3 -A 1Repository: Team-Optix-3749/Team-3749-2026
Length of output: 610
🏁 Script executed:
# Search for tripleThreatAuto to see how multiple trajectories are built
rg -n "tripleThreatAuto" --type java -A 20 | head -60Repository: Team-Optix-3749/Team-3749-2026
Length of output: 4257
🏁 Script executed:
# Check for any Choreo library version or documentation in the repo
rg -n "choreo" --type gradle -iRepository: Team-Optix-3749/Team-3749-2026
Length of output: 56
🏁 Script executed:
# Look for any comments or documentation around factory.bind usage
rg -n "factory\.bind|namedEventBindings" --type java -B 3 -A 3Repository: Team-Optix-3749/Team-3749-2026
Length of output: 3820
🏁 Script executed:
# Check the TrajectoryBuilder class to understand namedEventBindings
rg -n "class TrajectoryBuilder\|namedEventBindings" --type java -A 5Repository: Team-Optix-3749/Team-3749-2026
Length of output: 56
🏁 Script executed:
# Search for multiple bind calls in a single auto routine to see if there's accumulation
rg -n "bindEvent" --type java -B 2 -A 2Repository: Team-Optix-3749/Team-3749-2026
Length of output: 3476
🏁 Script executed:
# Verify if factory.bind() methods exist and understand their signature
rg -n "\.bind\(" ./2026/src/main/java/frc/robot/commands/auto/ -B 2 -A 2Repository: Team-Optix-3749/Team-3749-2026
Length of output: 653
🏁 Script executed:
# Check if there are any other auto routines that might bind overlapping event names
rg -n "\.bindEvent\(" ./2026/src/main/java/frc/robot/commands/auto/ -B 1 | head -60Repository: Team-Optix-3749/Team-3749-2026
Length of output: 878
factory.bind() accumulates bindings across all build() calls — shared static state creates implicit cross-trajectory dependencies.
factory is a static singleton initialized once and never cleared. Each call to factory.bind() in the build() method registers an event binding on it permanently. Currently, the "score" event is bound in both the "Sample" auto and trajectory3 of tripleThreatAuto(), creating an undocumented shared dependency. If bindings are later changed or trajectory files add overlapping event names, the factory will silently use stale or incorrect bindings without warning.
Consider either:
- Clearing bindings per
build()call or scoping them to specific routines, or - Documenting this additive behavior and the implicit cross-trajectory coupling it creates.
🤖 Prompt for AI Agents
In `@2026/src/main/java/frc/robot/commands/auto/AutoUtils.java` around lines 296 -
300, The build() method is adding event bindings to a static shared factory
(factory.bind(...) for each entry in namedEventBindings) causing bindings to
accumulate across builds and create cross-trajectory coupling; change build() to
avoid mutating global state by either clearing or isolating bindings before
binding: e.g., call a reset/clear method on the factory (or create a fresh
factory instance) at the start of AutoUtils.build(), or scope bindings to a
local/factory copy so factory.bind(...) does not permanently register them;
update references to factory, build(), namedEventBindings, and factory.bind(...)
accordingly to ensure bindings are per-build or documented as intentionally
additive.
| private final Object odometryLock = new Object(); | ||
|
|
||
| private Notifier odometryNotifier; |
There was a problem hiding this comment.
Notifier is never stopped or closed — potential thread leak.
odometryNotifier is started in the constructor but never stopped. While FRC subsystems typically live for the full robot lifecycle, if code is hot-restarted or the subsystem is re-instantiated during testing, the old notifier thread will continue running. Consider implementing close() (WPILib Notifier is AutoCloseable) to stop it gracefully.
🤖 Prompt for AI Agents
In `@2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java` around lines 44 -
46, The Notifier started in the Swerve constructor is never stopped; add a
close() implementation on the Swerve class that safely stops and disposes
odometryNotifier: add a public void close() (or implement AutoCloseable) that
synchronizes on odometryLock, checks if odometryNotifier != null, calls
odometryNotifier.close(), sets odometryNotifier = null, and returns; ensure any
places that re-create or dispose the subsystem call this close() to avoid
leaking the Notifier thread.
| odometryNotifier = new Notifier(this::updateOdometry); | ||
| odometryNotifier.startPeriodic(0.005); // 200 Hz |
There was a problem hiding this comment.
Thread-safety: Notifier thread reads shared mutable state without synchronization.
The Notifier runs updateOdometry() on a separate JNI thread. Inside updateOdometry (line 303), you read gyroData.orientation and call getModulePositions() (which reads each module's encoder state). However, both of these are written to on the main robot thread by periodic() → gyro.updateData() (line 382) and module.periodic() (line 380). The odometryLock only synchronizes access to the pose estimator itself, not the gyro data or module positions being fed into it.
This is a data race that can produce corrupted orientation/position readings and non-deterministic pose estimation errors (which may explain the odometry regressions mentioned in commit history).
Typical FRC patterns for high-frequency odometry either:
- Use
odometryLockaround both the sensor reads and the estimator update (and also around the writes inperiodic), or - Use signal-synchronized reads (e.g., Phoenix
BaseStatusSignal.waitForAllon the odometry thread, so the thread owns the sensor reads entirely).
🤖 Prompt for AI Agents
In `@2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java` around lines 111
- 112, The odometry thread reads shared sensor state without locking; wrap all
accesses to gyroData.orientation and module encoder state with the same
odometryLock to prevent races: acquire odometryLock inside updateOdometry()
around the sensor reads (gyroData.orientation and getModulePositions()) and
estimator.update call, and likewise make periodic() acquire odometryLock while
it calls gyro.updateData() and each module.periodic() (or at minimum while it
writes the sensor fields read by updateOdometry()); alternatively replace the
reads with a signal-synchronized read strategy, but ensure the chosen approach
uses odometryLock consistently around both readers (updateOdometry) and writers
(periodic, gyro.updateData, module.periodic).
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java (1)
117-119:⚠️ Potential issue | 🟡 MinorRemove
getSwerveDrivePoseEstimator()— it's unused and leaks an unsynchronized reference to a lock-protected object.
swerveDrivePoseEstimatoris mutated underodometryLockinupdateOdometry,setOdometry, andreset. This method has no callers in the codebase and returns a raw reference that bypasses synchronization. Note:getPoseEstimator()(line 162) is a duplicate with the same issue. Consider removing both or exposing only synchronized accessors likegetPose().
🤖 Fix all issues with AI agents
In `@2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java`:
- Around line 352-353: Fix the Javadoc typo in the Swerve class Javadoc comment:
replace the misspelled phrase "telemetrpdate" with "telemetry data" in the
comment that reads "Logs all swerve output telemetrpdate y data (non-input
data)" so the Javadoc correctly states "Logs all swerve output telemetry data
(non-input data)" near the logging method in Swerve.
- Around line 318-342: The call to gyro.reset() currently occurs outside the
synchronized(odometryLock) block and can race with the notifier's
updateOdometry()/gyro.updateData(), causing the subsequent read of
gyroData.orientation used in swerveDrivePoseEstimator.resetPosition(...) to be
stale; move gyro.reset() inside the synchronized(odometryLock) block immediately
before computing gyroHeading and calling
swerveDrivePoseEstimator.resetPosition(...) so the gyro reset and pose-estimator
reset are atomic relative to updateOdometry(), then leave syncEncoderPositions()
and resetAutoControllers(getPose()) after the synchronized block.
- Around line 113-114: The Notifier (odometryNotifier) is started inside the
Swerve constructor which can fire updateOdometry() before Robot.swerve is
assigned, causing an NPE in GyroSim.updateData() when it calls
Robot.swerve.getChassisSpeeds(); fix by deferring
odometryNotifier.startPeriodic(...) until after Robot.swerve is assigned (move
the start call into a new init method such as Swerve.initialize() and call it
right after setting Robot.swerve), or alternatively add a defensive null-check
in GyroSim.updateData() around Robot.swerve (return early if null) so
getChassisSpeeds() is not invoked until the swerve reference is valid.
🧹 Nitpick comments (5)
2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java (4)
145-160: Good:getRotation()andgetPose()are now synchronized — address indentation.The synchronization fixes the previously flagged race on
gyroData.orientation. However, the indentation is inconsistent:getRotation()andgetPose()have theirsynchronizedblocks and closing braces at mismatched indent levels (e.g., line 149, 156, 159). This pattern repeats throughout the file (setOdometry,updateOdometry,reset).Please normalize the indentation to match the class style (4-space indent for method bodies, 6-space for synchronized block bodies).
293-298: Remove commented-out code blocks.There are multiple stale commented-out blocks: the old
updateOdometry(lines 293–297), theupdateWithTimevariant (lines 301–305), the// updateOdometry()call inperiodic(line 388), and the encoder-sync block (lines 395–398). These are preserved in version control and add noise.
241-247: Debug telemetry — consolidate or removeSmartDashboardcalls.
SmartDashboard.putNumber(lines 242–243) andLogger.recordOutput(lines 245–246) log identical data. Since the codebase uses AdvantageKit'sLoggerfor structured telemetry, theSmartDashboardcalls appear to be debug artifacts. Also, keys like"getpointx"/"getpointy"are vague — consider more descriptive names if these are kept.
372-374: CachegetPose()to avoid redundant lock acquisitions.
getPose()is called three times inlogData(), each acquiringodometryLockseparately. The pose could theoretically differ between calls, and the repeated locking is unnecessary overhead at 200 Hz logging frequency.Proposed fix
- Logger.recordOutput("Swerve/Pose", getPose()); - Logger.recordOutput("Swerve/ActualPosition/x",getPose().getX()); - Logger.recordOutput("Swerve/ActualPosition/y",getPose().getY()); + Pose2d currentPose = getPose(); + Logger.recordOutput("Swerve/Pose", currentPose); + Logger.recordOutput("Swerve/ActualPosition/x", currentPose.getX()); + Logger.recordOutput("Swerve/ActualPosition/y", currentPose.getY());2026/src/main/java/frc/robot/subsystems/swerve/gyro/GyroSim.java (1)
25-33: Consider computing actual elapsed time instead of using a hardcoded constant.
deltaTis hardcoded toHIGH_FREQ_LOOP_TIME_S(5 ms), butNotifierscheduling is best-effort and may jitter. Sincedata.lastUpdateSalready stores the previous timestamp, you could compute the real elapsed time for more accurate yaw integration. This is more impactful at 200 Hz where small timing errors compound quickly.Suggested approach
`@Override` public void updateData() { - double deltaT = RobotConfig.General.HIGH_FREQ_LOOP_TIME_S; + double now = Timer.getTimestamp(); + double deltaT = (data.lastUpdateS > 0) ? (now - data.lastUpdateS) : RobotConfig.General.HIGH_FREQ_LOOP_TIME_S; double angleDiffRad = Robot.swerve.getChassisSpeeds().omegaRadiansPerSecond * deltaT; yaw = yaw.plus(Rotation2d.fromRadians(angleDiffRad)); data.orientation = new Rotation3d(0, 0, yaw.getRadians()); - data.lastUpdateS = Timer.getTimestamp(); + data.lastUpdateS = now; }
| odometryNotifier = new Notifier(this::updateOdometry); | ||
| odometryNotifier.startPeriodic(RobotConfig.General.HIGH_FREQ_LOOP_TIME_S); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find where Robot.swerve is assigned
find . -type f -name "*.java" | head -20
rg -n 'Robot\.swerve\s*=' --type=java -C3Repository: Team-Optix-3749/Team-3749-2026
Length of output: 1346
🏁 Script executed:
#!/bin/bash
# Locate the Swerve.java file and check surrounding context
fd -n "Swerve.java" --type fRepository: Team-Optix-3749/Team-3749-2026
Length of output: 304
🏁 Script executed:
#!/bin/bash
# Find Robot.java to understand initialization order
fd -n "Robot.java" --type fRepository: Team-Optix-3749/Team-3749-2026
Length of output: 304
🏁 Script executed:
#!/bin/bash
# Find the exact location of key files
fd "Swerve.java" --type f
fd "Robot.java" --type f
fd "GyroSim.java" --type fRepository: Team-Optix-3749/Team-3749-2026
Length of output: 237
🏁 Script executed:
#!/bin/bash
# Find Robot.swerve assignments
rg 'Robot\.swerve\s*=' --type=javaRepository: Team-Optix-3749/Team-3749-2026
Length of output: 56
🏁 Script executed:
#!/bin/bash
# Search for swerve field declaration and initialization more broadly
rg 'swerve\s*=' --type=java -B2 -A2 | head -50Repository: Team-Optix-3749/Team-3749-2026
Length of output: 381
🏁 Script executed:
#!/bin/bash
# Read Robot.java to understand the structure
cat -n 2026/src/main/java/frc/robot/Robot.java | head -100Repository: Team-Optix-3749/Team-3749-2026
Length of output: 4350
🏁 Script executed:
#!/bin/bash
# Read Swerve.java constructor and related code
cat -n 2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java | head -150Repository: Team-Optix-3749/Team-3749-2026
Length of output: 6327
🏁 Script executed:
#!/bin/bash
# Read GyroSim.updateData() to verify the claim
cat -n 2026/src/main/java/frc/robot/subsystems/swerve/gyro/GyroSim.javaRepository: Team-Optix-3749/Team-3749-2026
Length of output: 1330
🏁 Script executed:
#!/bin/bash
# Find the updateOdometry method in Swerve.java
rg -n "updateOdometry" 2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java -A 20Repository: Team-Optix-3749/Team-3749-2026
Length of output: 2080
🏁 Script executed:
#!/bin/bash
# Get more of Swerve.java to see the updateOdometry method
tail -n +150 2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java | head -100Repository: Team-Optix-3749/Team-3749-2026
Length of output: 2842
Notifier starts before Robot.swerve is assigned — NPE risk in SIM mode.
The Notifier is created and started in the Swerve constructor (lines 113-114), but Robot.swerve is assigned after the constructor completes. updateOdometry() calls gyro.updateData(), which in GyroSim.java line 28 unconditionally calls Robot.swerve.getChassisSpeeds() without a null check. If the notifier fires before the assignment to the static field completes (which can happen within 5ms in a 5ms-period loop), this results in an NPE.
Consider deferring odometryNotifier.startPeriodic(...) to a separate initialization method called after Robot.swerve is fully assigned, or add a null-guard in GyroSim.updateData().
🤖 Prompt for AI Agents
In `@2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java` around lines 113
- 114, The Notifier (odometryNotifier) is started inside the Swerve constructor
which can fire updateOdometry() before Robot.swerve is assigned, causing an NPE
in GyroSim.updateData() when it calls Robot.swerve.getChassisSpeeds(); fix by
deferring odometryNotifier.startPeriodic(...) until after Robot.swerve is
assigned (move the start call into a new init method such as Swerve.initialize()
and call it right after setting Robot.swerve), or alternatively add a defensive
null-check in GyroSim.updateData() around Robot.swerve (return early if null) so
getChassisSpeeds() is not invoked until the swerve reference is valid.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java`:
- Around line 299-310: updateOdometry() still races with module state writes
because getModulePositions() (and gyro.updateData() -> getChassisSpeeds() ->
modules[i].getState()/getPosition()) reads module encoder state while
module.periodic() writes it without holding odometryLock; fix by synchronizing
access: either wrap the module.periodic() loop in periodic() inside
synchronized(odometryLock) so all reads in updateOdometry() see a consistent
snapshot, or adopt the signal-synchronized pattern where the notifier thread
exclusively performs all sensor reads (move
getModulePositions()/gyro.updateData() to that owner and block periodic() until
reads complete); reference methods: updateOdometry(), periodic(),
gyro.updateData(), getModulePositions(), module.periodic(),
modules[i].getPosition(), modules[i].getState().
🧹 Nitpick comments (5)
2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java (5)
145-160: Inconsistent indentation across several synchronized methods.The closing braces of
getRotation()(line 149) andgetPose()(line 159) are at column 0 instead of being aligned with the method declaration. The same pattern repeats insetOdometry(),updateOdometry(), andreset(). This makes the class harder to read and may cause merge conflicts.A quick auto-format pass would clean this up.
241-247: Debug telemetry left indriveToSampleoverload.
SmartDashboard.putNumbercalls on lines 242–243 duplicate theLogger.recordOutputcalls on 245–246. SmartDashboard puts are more expensive (NetworkTables overhead) and look like debugging artifacts. Consider removing the SmartDashboard calls and keeping only the AdvantageKit logger output.
293-310: Remove commented-out code; considerupdateWithTimefor high-frequency odometry.The old
updateOdometrybody (lines 293–297) and theupdateWithTimevariant (lines 301–305) are dead code. Clean these up.Worth noting:
swerveDrivePoseEstimator.update()internally usesTimer.getFPGATimestamp()at call time. For a high-frequency notifier, timestamps from the sensor sample time (gyroData.lastUpdateS) would be more accurate. The commented-outupdateWithTimeapproach is actually the better choice here — consider using it with the gyro'slastUpdateStimestamp instead ofTimer.getFPGATimestamp().Proposed cleanup
- // public void updateOdometry() { - // swerveDrivePoseEstimator.update( - // Rotation2d.fromRadians(gyroData.orientation.getZ()), - // getModulePositions()); - // } - public void updateOdometry() { synchronized (odometryLock) { - // swerveDrivePoseEstimator.updateWithTime( - // Timer.getFPGATimestamp(), - // Rotation2d.fromRadians(gyroData.orientation.getZ()), - // getModulePositions() - // ); - gyro.updateData(); - swerveDrivePoseEstimator.update(Rotation2d.fromRadians(gyroData.orientation.getZ()), getModulePositions()); + swerveDrivePoseEstimator.updateWithTime( + gyroData.lastUpdateS, + Rotation2d.fromRadians(gyroData.orientation.getZ()), + getModulePositions() + ); } }
372-374: CachegetPose()to avoid repeated lock acquisition.
getPose()acquiresodometryLockon each call. Here it's called three times in succession. Cache the result in a local variable.Proposed fix
- Logger.recordOutput("Swerve/Pose", getPose()); - Logger.recordOutput("Swerve/ActualPosition/x",getPose().getX()); - Logger.recordOutput("Swerve/ActualPosition/y",getPose().getY()); + Pose2d currentPose = getPose(); + Logger.recordOutput("Swerve/Pose", currentPose); + Logger.recordOutput("Swerve/ActualPosition/x", currentPose.getX()); + Logger.recordOutput("Swerve/ActualPosition/y", currentPose.getY());
385-399: Clean up commented-out code inperiodic().Lines 388 and 395–398 are dead code. Remove them or convert to a tracked TODO if the encoder sync logic is intended to return.
| public void updateOdometry() { | ||
| swerveDrivePoseEstimator.update( | ||
| Rotation2d.fromRadians(gyroData.orientation.getZ()), | ||
| getModulePositions()); | ||
| synchronized (odometryLock) { | ||
| // swerveDrivePoseEstimator.updateWithTime( | ||
| // Timer.getFPGATimestamp(), | ||
| // Rotation2d.fromRadians(gyroData.orientation.getZ()), | ||
| // getModulePositions() | ||
| // ); | ||
|
|
||
| gyro.updateData(); | ||
| swerveDrivePoseEstimator.update(Rotation2d.fromRadians(gyroData.orientation.getZ()), getModulePositions()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "SwerveModule.java" --type fRepository: Team-Optix-3749/Team-3749-2026
Length of output: 138
🏁 Script executed:
cat -n "2026/src/main/java/frc/robot/subsystems/swerve/SwerveModule.java"Repository: Team-Optix-3749/Team-3749-2026
Length of output: 11608
🏁 Script executed:
find . -name "SwerveModuleDataAutoLogged.java" -type fRepository: Team-Optix-3749/Team-3749-2026
Length of output: 56
🏁 Script executed:
rg "SwerveModuleDataAutoLogged" --type java -lRepository: Team-Optix-3749/Team-3749-2026
Length of output: 285
🏁 Script executed:
rg "class SwerveModuleDataAutoLogged" --type javaRepository: Team-Optix-3749/Team-3749-2026
Length of output: 56
🏁 Script executed:
rg "SwerveModuleDataAutoLogged" --type java -A 3 -B 3 | head -50Repository: Team-Optix-3749/Team-3749-2026
Length of output: 4852
🏁 Script executed:
find . -name "*.java" -type f | xargs grep -l "class.*SwerveModuleDataAutoLogged" 2>/dev/null | head -5Repository: Team-Optix-3749/Team-3749-2026
Length of output: 56
🏁 Script executed:
find . -name "*SwerveModuleData*" -type fRepository: Team-Optix-3749/Team-3749-2026
Length of output: 56
🏁 Script executed:
cat -n "2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java" | sed -n '380,415p'Repository: Team-Optix-3749/Team-3749-2026
Length of output: 774
🏁 Script executed:
cat -n "2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java" | sed -n '299,330p'Repository: Team-Optix-3749/Team-3749-2026
Length of output: 1115
🏁 Script executed:
rg "getModulePositions\(\)" -A 3 "2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java"Repository: Team-Optix-3749/Team-3749-2026
Length of output: 873
🏁 Script executed:
rg "private SwerveModulePosition\[\] getModulePositions" -A 5 "2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java"Repository: Team-Optix-3749/Team-3749-2026
Length of output: 279
🏁 Script executed:
cat -n "2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java" | sed -n '1,50p'Repository: Team-Optix-3749/Team-3749-2026
Length of output: 2281
🏁 Script executed:
rg "getChassisSpeeds\(\)" -A 5 "2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java"Repository: Team-Optix-3749/Team-3749-2026
Length of output: 843
🏁 Script executed:
find . -name "GyroSim.java" -type fRepository: Team-Optix-3749/Team-3749-2026
Length of output: 140
🏁 Script executed:
cat -n "2026/src/main/java/frc/robot/subsystems/swerve/gyro/GyroSim.java"Repository: Team-Optix-3749/Team-3749-2026
Length of output: 1330
Remaining data race on module state: reads in updateOdometry() race with writes in periodic().
Moving gyro.updateData() inside synchronized(odometryLock) fixes the gyro data race — good. However, getModulePositions() and the getChassisSpeeds() call within gyro.updateData() both read module encoder state (via modules[i].getPosition() and modules[i].getState()). These reads are protected by the lock in updateOdometry(), but the writes happen in module.periodic() (line 385–387 in the main thread's periodic() method) without holding odometryLock.
This means the notifier thread can observe a partially-updated set of module positions/velocities mid-write from periodic().
Either wrap the module.periodic() calls in periodic() under odometryLock as well, or adopt the signal-synchronized pattern where the notifier thread owns sensor reads entirely (blocking on hardware signals).
🤖 Prompt for AI Agents
In `@2026/src/main/java/frc/robot/subsystems/swerve/Swerve.java` around lines 299
- 310, updateOdometry() still races with module state writes because
getModulePositions() (and gyro.updateData() -> getChassisSpeeds() ->
modules[i].getState()/getPosition()) reads module encoder state while
module.periodic() writes it without holding odometryLock; fix by synchronizing
access: either wrap the module.periodic() loop in periodic() inside
synchronized(odometryLock) so all reads in updateOdometry() see a consistent
snapshot, or adopt the signal-synchronized pattern where the notifier thread
exclusively performs all sensor reads (move
getModulePositions()/gyro.updateData() to that owner and block periodic() until
reads complete); reference methods: updateOdometry(), periodic(),
gyro.updateData(), getModulePositions(), module.periodic(),
modules[i].getPosition(), modules[i].getState().
2 participants
made test auto paths
added high frequency odometry
Summary by CodeRabbit
New Features
Configuration Updates
Improvements