Drip working - #129
Merged
Merged
Conversation
…rol-FuelPhysicsSim
This reverts commit d5efa71.
Reorganize spectrumLib into clearer subpackages and apply multiple safety/bug fixes across the codebase. Key changes: - Move core spectrumLib classes into new packages (framework, hardware, telemetry, mechanism, etc.) and update all imports accordingly. - Refactor Mechanism: cleanup docs, reorganize fields, improve cached sensor accessors and initialization, and update imports to TalonFXFactory/util locations. - Replace usages of frc.spectrumLib.Telemetry with frc.spectrumLib.telemetry.Telemetry across the project. - Fix sign/logic bugs: normalize reverseTorqueCurrentLimit usages (removed negative literals) in Hood, IntakeExtension, Launcher, and other mechanisms. - Add guards around ShotCalculator parameter use (params.isValid()) in Hood and Launcher to avoid invalid targeting. - Remove debug prints from Tag align controllers and adjust controller.reset() usage. - Improve Robot auton startup: handle empty path list (log a warning) and remove an unnecessary clearCommandsAndButtons call in disabledInit. - Delete unused HomeOffsets class. - Add VSCode Java VM args to .vscode/settings.json to improve editor JVM behavior. These changes are primarily structural (package reorganization) and safety-oriented to prevent invalid operations and improve code clarity/telemetry integration.
Replace the minimal README with a comprehensive usage and API overview for the spectrumLib package. The new README documents package structure and modules (framework, hardware, telemetry, util, mechanism, gamepads, leds, sim, swerve, vision), enumerates key classes and responsibilities, lists dependencies, and provides usage/installation guidance to help developers onboard and reuse library components across robot projects.
Large refactor of the Vision subsystem: introduce VisionConfig and VisionFieldPoseEstimate classes; add per-camera VisionLogger instances and bulk telemetry logging; implement robust MT1/MT2 pose pipelines with detailed rejection checks, std-dev selection, and imu-mode caching; expose utility methods (getBestLimelight, hasAccuratePose, tagsInView, triggerRewindCaptureForAllCameras, setLimelightPipelines, pose reset with stricter sanity checks) and improved commands for LED control. Also reorganized periodic logic and cleaned up many null/units/angle checks and timestamp handling. Additionally moved/normalized Telemetry import positions across multiple mechanism and robot subsystem classes and applied small Javadoc/formatting tweaks in Mechanism.
Delete the locally modified Trigger implementation and its README, and update all call sites to use the standard Trigger API. Replaced deprecated .not() calls with .negate(), converted multi-argument .and()/.or() usages into chained .and()/.or() calls to match the available API, and updated related call sites (Pilot, Operator, Gamepad, SwerveStates, SpectrumState, PilotStates, OperatorStates, RobotStates). Also adjusted some command names and a pilotAimDrive parameter name (degrees -> radians) and updated a WaitCommand.onlyWhile(...) call to use negate(). These changes restore compatibility with the external WPILib Trigger implementation and align the codebase with its API.
Adjust pilot input bindings and formatting, resize simulator GUI, and tidy imports: - Swap and rewire LT/RT trigger handlers (onTrue/onFalse) and update the combined trigger order for launch/intake state transitions in RobotStates. - Resize simgui window from 2256x1415 to 1920x1009 and reduce widget sizes/positions for Addressable LEDs and Joysticks in simgui-window.json. - Reorder the Telemetry import in LauncherStates (style-only) and collapse the multi-line Trigger composition into a single expression in PilotStates.
Replace the previous interpolation/map-based shot lookup with a degree-3 polynomial surface and an iterative "virtual-target" solver in ShotCalculator. Introduces a ShootingParameters record, clamped fitted ranges, MPS/RPM scaling constants, and a 1690-style iterative lookahead that accounts for radial and tangential launcher velocities to compute exit speed, launch angle, yaw offset, converged lookahead distance and time-of-flight. Adds constants for phase delay, loop period, and filters; moves hood offset/drive offset handling into runtime-adjustable fields; and expands Telemetry outputs for debugging. Also updates RobotSim to use the computed exit speed (m/s) directly instead of deriving a simulated launcher speed from the old flywheel RPM mapping. Misc: removes dependence on interpolating maps (and related imports), and consolidates hood/flywheel/drive computations around the new polynomial-based solver.
Contributor
|
🐇 ✅ Action performedFull review finished. |
coderabbitai
Bot
added
the
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (4)
src/main/java/frc/spectrumLib/README.md (1)
9-21: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLine 17 still describes the
ledspackage as anAddressableLEDwrapper.Line 99 documents
SpectrumLEDsas a CTRE CANdle subsystem. The package-structure listing contradicts it. Also add a language to the fenced block at line 9, which markdownlint flags as MD040.📝 Proposed fix
-``` +```text frc.spectrumLib ├── framework/ Base classes and interfaces for robot and subsystem architecture @@ -├── leds/ AddressableLED wrapper with built-in pattern library +├── leds/ CTRE CANdle wrapper with built-in pattern library🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/spectrumLib/README.md` around lines 9 - 21, Update the package-structure fenced block in the README to specify the text language and change the leds entry to describe the CTRE CANdle wrapper, keeping it consistent with the SpectrumLEDs documentation.Source: Linters/SAST tools
src/main/java/frc/robot/subsystems/vision/Vision.java (1)
444-452: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe heading std-dev overrides narrow the variance and miss clockwise rotation.
Both assignments overwrite
degStdsunconditionally. The tiers "Close integration", "Proximity integration", and "Stable integration" setdegStds = config.getKLargeVariance()to discard the heading. Lines 446 and 451 then reduce that value to 15 or 50, so a low-confidence heading is fused. Line 450 also omitsMath.abs, so clockwise rotation never triggers the override;rejectionCheckusesMath.absfor the same quantity at line 598.🐛 Proposed fix
// Widen heading std-dev when ambiguity is moderate if (highestAmbiguity > 0.5) { - degStds = 15; + degStds = Math.max(degStds, 15); } // Discard heading during fast rotation (MegaTag1 heading unreliable while spinning) - if (robotSpeed.omegaRadiansPerSecond >= 0.5) { - degStds = 50; + if (Math.abs(robotSpeed.omegaRadiansPerSecond) >= 0.5) { + degStds = Math.max(degStds, 50); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/robot/subsystems/vision/Vision.java` around lines 444 - 452, Update the heading variance logic near the ambiguity and rotation checks so these overrides cannot reduce a previously selected large variance: apply the 15 and 50 values only when they are greater than the existing degStds, or otherwise preserve the larger value. Use Math.abs(robotSpeed.omegaRadiansPerSecond) for the rotation threshold so both rotation directions trigger the same behavior, consistent with rejectionCheck.src/main/java/frc/spectrumLib/gamepads/Gamepad.java (1)
118-143: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winInitialize the composite modifier triggers to
kFalse.The class documentation at lines 42-43 states that all triggers remain
falsewhenConfig.isAttached()returnsfalse. These nine fields do not follow that contract. The constructor assigns them only inside theif (config.attached)block at lines 311-319. If the controller is not attached, they staynull, and a binding such aspilot.noModifiers.and(...)throws aNullPointerExceptionduring binding configuration. The individual button fields already default tokFalse.🛡️ Proposed fix
- public Trigger noBumpers; + public Trigger noBumpers = kFalse; @@ - public Trigger leftBumperOnly; + public Trigger leftBumperOnly = kFalse; @@ - public Trigger rightBumperOnly; + public Trigger rightBumperOnly = kFalse; @@ - public Trigger bothBumpers; + public Trigger bothBumpers = kFalse; @@ - public Trigger noTriggers; + public Trigger noTriggers = kFalse; @@ - public Trigger leftTriggerOnly; + public Trigger leftTriggerOnly = kFalse; @@ - public Trigger rightTriggerOnly; + public Trigger rightTriggerOnly = kFalse; @@ - public Trigger bothTriggers; + public Trigger bothTriggers = kFalse; @@ - public Trigger noModifiers; + public Trigger noModifiers = kFalse;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/spectrumLib/gamepads/Gamepad.java` around lines 118 - 143, Initialize the nine composite modifier triggers—noBumpers, leftBumperOnly, rightBumperOnly, bothBumpers, noTriggers, leftTriggerOnly, rightTriggerOnly, bothTriggers, and noModifiers—to kFalse before the config.attached conditional in Gamepad’s constructor, while preserving their existing attached-controller assignments so bindings remain safe when the controller is detached.src/main/java/frc/robot/subsystems/leds/Leds.java (1)
27-42: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe
NUM_LEDScomment contradicts the configured index range.Line 27 states the 20 external LEDs occupy indices 8–27. The static block does not call
setStartIdx(8), so the configuration addresses indices 0–19, which includes the 8 onboard CANdle LEDs. Lines 30-34 describe both options, so the intent is unclear. Align the comment with the configured value, or setstartIdxto 8.📝 Proposed documentation fix
- /** Number of external LEDs attached to the CANdle output (indices 8–27 on the device). */ + /** + * Number of LEDs driven by this configuration. With the current {`@code` startIdx} of 0 this + * addresses device indices 0–19 (the 8 onboard CANdle LEDs plus the first 12 external LEDs). + */ public static final int NUM_LEDS = 20;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/robot/subsystems/leds/Leds.java` around lines 27 - 42, Align the LED range documentation with the actual configuration in the Leds static initializer: since ledsConfig does not call setStartIdx(8), document NUM_LEDS as covering indices 0–19 and clarify that the current configuration includes onboard LEDs. Remove the contradictory external-strip description without changing the configured start index.
🧹 Nitpick comments (13)
src/main/java/frc/robot/RobotSim.java (2)
46-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the injected
SuperStructureinstead of the static accessor.
simLaunching()readsRobot.getSuperStructure()three times, and line 161 repeats the pattern, while the class already storesrobotSuperStructure. Extract a local variable inside the lambda, and prefer the instance field where the method is not static.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/robot/RobotSim.java` around lines 46 - 57, Update simLaunching() to capture the injected robotSuperStructure in a local variable inside the Trigger lambda and use it for all current-super-state checks instead of calling Robot.getSuperStructure(). Apply the same replacement at the other repeated access near line 161, using the instance field directly where the method is non-static.
196-229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the four identical lane loops, and handle the remainder.
The four blocks differ only by the lane offset. Build them in a loop over the lane offsets.
numToLaunchPerLane = fuelCount / numOfLanesalso uses integer division, so up tonumOfLanes - 1intaked balls never launch.♻️ Proposed change
return Commands.defer( () -> { int fuelCount = ballSim.getTotalIntaked(); - int numToLaunchPerLane = fuelCount / numOfLanes; - SequentialCommandGroup group1 = - new SequentialCommandGroup(Commands.waitSeconds(Math.random() * 0.3)); - for (int i = 0; i < numToLaunchPerLane; i++) { - group1.addCommands( - createSimBallLaunch(lane1), - Commands.waitSeconds(timeBetweenBallLaunches)); - } - SequentialCommandGroup group2 = - new SequentialCommandGroup(Commands.waitSeconds(Math.random() * 0.3)); - for (int i = 0; i < numToLaunchPerLane; i++) { - group2.addCommands( - createSimBallLaunch(lane2), - Commands.waitSeconds(timeBetweenBallLaunches)); - } - SequentialCommandGroup group3 = - new SequentialCommandGroup(Commands.waitSeconds(Math.random() * 0.3)); - for (int i = 0; i < numToLaunchPerLane; i++) { - group3.addCommands( - createSimBallLaunch(lane3), - Commands.waitSeconds(timeBetweenBallLaunches)); - } - SequentialCommandGroup group4 = - new SequentialCommandGroup(Commands.waitSeconds(Math.random() * 0.3)); - for (int i = 0; i < numToLaunchPerLane; i++) { - group4.addCommands( - createSimBallLaunch(lane4), - Commands.waitSeconds(timeBetweenBallLaunches)); - } - return Commands.parallel(group1, group2, group3, group4) + double[] lanes = {lane1, lane2, lane3, lane4}; + Command[] groups = new Command[lanes.length]; + for (int lane = 0; lane < lanes.length; lane++) { + // Distribute the remainder across the first lanes + int ballsForLane = + fuelCount / numOfLanes + (lane < fuelCount % numOfLanes ? 1 : 0); + SequentialCommandGroup group = + new SequentialCommandGroup( + Commands.waitSeconds(Math.random() * 0.3)); + for (int i = 0; i < ballsForLane; i++) { + group.addCommands( + createSimBallLaunch(lanes[lane]), + Commands.waitSeconds(timeBetweenBallLaunches)); + } + groups[lane] = group; + } + return Commands.parallel(groups) .withName("RobotSim.ballSimLaunchFuel");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/robot/RobotSim.java` around lines 196 - 229, In the deferred launch command, replace the four duplicated lane-specific loops with a loop over the available lane offsets, creating one sequential command group per lane and passing each offset to createSimBallLaunch. Ensure every intaked ball launches by distributing the fuel remainder across lanes or otherwise accounting for fuelCount % numOfLanes, while preserving the existing randomized startup delays, inter-launch waits, and parallel execution.src/main/deploy/elastic-layout.json (1)
174-186: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the widget title to match its topic. The title is
Intake/SystemState, but the topic is/Robot/FuelIntake/SystemState. All sibling widgets use the subsystem name. UseFuelIntake/SystemStatefor consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/deploy/elastic-layout.json` around lines 174 - 186, Update the title of the widget identified by topic "/Robot/FuelIntake/SystemState" from "Intake/SystemState" to "FuelIntake/SystemState", leaving its topic and other properties unchanged.src/main/java/frc/rebuilt/ShotCalculator.java (1)
452-452: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the telemetry key. The key is
ShotCalc/HubPolyModel, but the value is now the active model, which isFEED_MODELduring feed shots. UseShotCalc/ActivePolyModel. Update the dashboard layout if it references the old key.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/rebuilt/ShotCalculator.java` at line 452, Rename the telemetry key in ShotCalculator from ShotCalc/HubPolyModel to ShotCalc/ActivePolyModel while continuing to log model.name() as the active model. Update any dashboard layout references from the old key to the new key.src/main/java/frc/rebuilt/FuelPhysicsSim.java (2)
2316-2323: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winGate the conservation computation behind its config flag.
publishPositionscallscomputeConservationQuantities()on every tick. That is an extra full pass over all balls, plusgetNorm()per ball, on the 20 ms loop.stepSubtickalready computes it whenconfig.conservationMonitoris true, so the work is duplicated in that case and unwanted otherwise.Publish the cached value, and recompute only when the monitor is enabled.
♻️ Proposed change
- computeConservationQuantities(); - totalEnergyPub.set(totalKE + totalPE); + if (!config.conservationMonitor) { + computeConservationQuantities(); + } + totalEnergyPub.set(totalKE + totalPE);If per-tick energy telemetry is not needed, publish only when
config.conservationMonitoris true.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/rebuilt/FuelPhysicsSim.java` around lines 2316 - 2323, Update the stats publishing block in publishPositions to remove the unconditional computeConservationQuantities() call and publish the cached totalKE + totalPE value. Ensure conservation quantities are recomputed only through the existing config.conservationMonitor-gated logic in stepSubtick, and avoid publishing per-tick energy telemetry when that monitor is disabled.
1421-1429: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueClamp grid indices for negative coordinates. A ball just outside the field on the negative side, for example
x = -0.1, produces(int) (-0.1 / 0.25) == 0. The bounds check passes, and the ball is hashed into column 0 together with balls atx ∈ [0, 0.25). The same truncation applies ingenerateBallBallContacts, so a ball atx = -0.1and a ball atx = 0.1are treated as neighbours in the same cell. Contacts are still validated by distance, so the effect is a small amount of wasted narrowphase work, not a wrong result.Use
Math.floorDiv-style flooring, or skip balls with negative X or Y.Also applies to: 1440-1441
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/rebuilt/FuelPhysicsSim.java` around lines 1421 - 1429, Replace truncating coordinate-to-cell conversion in the grid-building loop and generateBallBallContacts with floor-based indexing, such as Math.floorDiv-style behavior, so negative positions map outside the valid grid instead of column or row zero. Preserve the existing bounds checks and skip out-of-range cells, ensuring balls at negative X or Y are not grouped with balls inside the field.src/main/java/frc/robot/subsystems/fuelIntake/FuelIntake.java (1)
111-113: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueEach
applyStates()allocates a new supplier every loop.All four state machines capture a local
final doublein a lambda and pass it to theMechanismsetter. A capturing lambda cannot be cached by the JVM, so every 20 ms loop allocates a new object per mechanism. Store the setpoint in a field and reuse one supplier instance instead. This is an optional cleanup; the current code is correct.
src/main/java/frc/robot/subsystems/fuelIntake/FuelIntake.java#L111-L113: hold the wanted voltage in a field and pass a singleDoubleSupplierfield tosetVoltageOutput.src/main/java/frc/robot/subsystems/hood/Hood.java#L114-L116: hold the wanted position in a field and pass a single supplier tosetMMPositionFoc.src/main/java/frc/robot/subsystems/indexerBed/IndexerBed.java#L95-L96: hold the wanted RPM in a field and pass a single supplier tosetVelocityTCFOCrpm.src/main/java/frc/robot/subsystems/indexerTower/IndexerTower.java#L98-L99: apply the same change for itssetVelocityTCFOCrpmcall.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/robot/subsystems/fuelIntake/FuelIntake.java` around lines 111 - 113, Replace the per-loop capturing suppliers in FuelIntake.java lines 111-113, Hood.java lines 114-116, IndexerBed.java lines 95-96, and IndexerTower.java lines 98-99: add a field for each desired setpoint, update that field in the corresponding applyStates() method, and reuse a single DoubleSupplier field with each Mechanism setter (setVoltageOutput, setMMPositionFoc, or setVelocityTCFOCrpm).src/main/java/frc/robot/subsystems/SuperStructure.java (1)
177-317: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider a table-driven state application.
The 11 state methods repeat the same seven
setWantedStatecalls with different constants. A single record or map of per-state setpoints would remove the repetition and make omissions, such as the missingintakeExtensioncall inlaunchWithoutSqueeze, visible. This is optional and can wait until the state set stabilizes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/robot/subsystems/SuperStructure.java` around lines 177 - 317, Optionally refactor the repeated state-setting logic in SuperStructure methods such as applyIdle, intakeFuel, launchWithSqueeze, and launchWithoutSqueeze into a shared record or map of per-state setpoints with one application method. Ensure every state specifies all relevant subsystem commands, including intakeExtension for launchWithoutSqueeze, while preserving each state’s existing behavior.src/main/java/frc/robot/subsystems/swerve/Swerve.java (1)
297-302: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueReuse a single
SwerveRequest.Idleinstance.
applyStates()allocates a newSwerveRequest.Idleon every loop while the state isIDLE. The other requests in this class are cached fields. Cache this one as well to avoid per-loop allocation on the RIO.♻️ Proposed refactor
+ private static final SwerveRequest.Idle IDLE_REQUEST = new SwerveRequest.Idle(); + private void applyStates() { switch (systemState) { default: case IDLE: - setControl(new SwerveRequest.Idle()); + setControl(IDLE_REQUEST); break;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/robot/subsystems/swerve/Swerve.java` around lines 297 - 302, Cache a single SwerveRequest.Idle instance alongside the other request fields in Swerve, then update applyStates() to pass that cached instance to setControl() for the IDLE state instead of constructing one on each loop.src/main/java/frc/spectrumLib/framework/SpectrumRobot.java (1)
24-33: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winInclude the reflection exception details in the warning. WPILib 2026.1.1 uses
IterativeRobotBase.m_watchdog, andWatchdog.setTimeout(double)is available. If reflection fails, report the exception details so the missing timeout is diagnosable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/spectrumLib/framework/SpectrumRobot.java` around lines 24 - 33, Update the reflection failure handling around IterativeRobotBase.m_watchdog and Watchdog.setTimeout in SpectrumRobot so DriverStation.reportWarning includes the caught exception details along with the existing failure context, while preserving the current fallback behavior.src/main/java/frc/spectrumLib/sim/LinearConfig.java (1)
142-146: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the stray empty statement in
setStaticLength.Line 144 contains a lone
;after the assignment.♻️ Proposed refactor
public LinearConfig setStaticLength(double lengthInches) { this.staticLength = Units.inchesToMeters(lengthInches); - ; return this; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/spectrumLib/sim/LinearConfig.java` around lines 142 - 146, Remove the stray standalone semicolon from setStaticLength after assigning staticLength, leaving the conversion assignment and fluent return unchanged.src/main/java/frc/spectrumLib/sim/ArmConfig.java (1)
103-106: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd Javadoc to
setSimulatedGravity.Every other public member in this file has Javadoc. This new fluent setter has none.
♻️ Proposed refactor
+ /** + * Sets whether the physics simulation applies gravity to the arm. + * + * `@param` simulateGravity {`@code` true} to apply gravitational force + * `@return` this config for chaining + */ public ArmConfig setSimulatedGravity(boolean simulateGravity) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/spectrumLib/sim/ArmConfig.java` around lines 103 - 106, Add Javadoc to the public fluent setter setSimulatedGravity in ArmConfig, documenting the simulateGravity parameter and that the method returns this configuration instance for chaining. Keep the setter behavior unchanged.src/main/java/frc/spectrumLib/mechanism/Mechanism.java (1)
119-128: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReconsider 250 Hz for all status signals, especially device temperature.
Each motor now publishes 8 signals at 250 Hz, which is 2000 frames per second per device. Followers add the same load. A
Mechanism.Configcan target the nativeriobus (1 Mbps classic CAN), where this rate plus swerve traffic can saturate the bus. Device temperature changes slowly and does not need 250 Hz.Consider keeping control-relevant signals at a high rate and setting
getDeviceTemp()to a low rate, for example 4 Hz.♻️ Proposed refactor
BaseStatusSignal.setUpdateFrequencyForAll( 250, motor.getDutyCycle(), motor.getMotorVoltage(), motor.getTorqueCurrent(), motor.getStatorCurrent(), motor.getSupplyCurrent(), motor.getPosition(), - motor.getVelocity(), - motor.getDeviceTemp()); + motor.getVelocity()); + motor.getDeviceTemp().setUpdateFrequency(4);Also applies to: 138-147
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/spectrumLib/mechanism/Mechanism.java` around lines 119 - 128, Update the status-signal frequency configuration in Mechanism so control-relevant signals retain the high update rate while motor.getDeviceTemp() uses a separate low frequency such as 4 Hz. Apply the same change to every corresponding signal-group setup, including follower motors, without changing the existing signal selection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/frc/rebuilt/FuelPhysicsSim.java`:
- Around line 1036-1039: Swap the “Blue-side depot” and “Red-side depot”
comments in the depot setup near fillBallGrid so they match the coordinates: the
call using X 0.37 is blue, and the call using X FIELD_LENGTH - 0.37 is red.
Leave the coordinates and fillBallGrid calls unchanged.
- Around line 117-129: Replace the local FIELD_LENGTH, FIELD_WIDTH,
BLUE_HUB_CENTER, and RED_HUB_CENTER values in FuelPhysicsSim with the canonical
Field.fieldLength and Field.fieldWidth constants, and derive both hub centers
from Field.BlueHub. Update all references to use these shared geometry
definitions while leaving unrelated bump and wall constants unchanged.
In `@src/main/java/frc/rebuilt/ShotCalculator.java`:
- Around line 365-372: Guard the unit-vector calculations in the
radial/tangential decomposition before dividing by distanceNoLookahead. When the
distance is zero or below a small epsilon, use zero radial and tangential
velocity components; otherwise retain the existing ux/uy-based calculations so
solveVirtualTarget receives finite values.
In `@src/main/java/frc/robot/RobotSim.java`:
- Around line 140-163: Correct the intake-zone bounds in configBallSimRobot: use
simRobotLength and intakeLength to calculate intakeXMin and intakeXMax, and use
simRobotWidth and intakeWidth for intakeYMin and intakeYMax, preserving the
existing coordinate orientation and intake-state predicate.
In `@src/main/java/frc/robot/subsystems/hood/Hood.java`:
- Around line 23-24: Remove the `@Setter` annotations from maxRotations and
minRotations in Hood, since changing these fields does not update the motor’s
configured soft limits; keep the constructor’s existing limit configuration
behavior unchanged.
In `@src/main/java/frc/robot/subsystems/SuperStructure.java`:
- Around line 238-246: Update launchWithoutSqueeze to explicitly command
intakeExtension with the intended IntakeExtension.WantedState, matching the
state-setting pattern used by the other SuperStructure methods and preventing
the previous superstate’s command from persisting.
In `@src/main/java/frc/robot/subsystems/vision/Vision.java`:
- Around line 466-467: Update the count calculation in the vision method around
getRawFiducial() so tags.length is only accessed when tags is non-null;
otherwise use a count of zero. Remove the `@SuppressWarnings`("null") annotation
rather than masking the nullable path.
- Around line 591-596: Update the stale-timestamp validation in the surrounding
vision pose-processing method to compare the raw Limelight/WPILib timestamp from
the appropriate getter directly against Timer.getFPGATimestamp(), removing the
Utils.fpgaToCurrentTime conversion. When validating an MT2 pose, use
getMegaTag2PoseTimestamp(); retain the existing rejection threshold and
invalid-status behavior.
In `@src/main/java/frc/spectrumLib/telemetry/BatteryLogger.java`:
- Around line 113-130: Update the parent-key construction loop in
BatteryLogger’s aggregation logic so each prefix is merged without a trailing
separator. Build and merge the current prefix first, then append "/" only before
adding the next segment, ensuring keys such as "Drive" remain consistent across
multi-segment and two-segment reports.
In `@src/main/java/frc/spectrumLib/telemetry/Telemetry.java`:
- Around line 67-99: Update the Javadoc for Telemetry.start so the parameter
descriptions match the method signature: document captureDs before captureNt,
describe captureDs as capturing Driver Station data, and retain the
NetworkTables description for captureNt.
---
Duplicate comments:
In `@src/main/java/frc/robot/subsystems/leds/Leds.java`:
- Around line 27-42: Align the LED range documentation with the actual
configuration in the Leds static initializer: since ledsConfig does not call
setStartIdx(8), document NUM_LEDS as covering indices 0–19 and clarify that the
current configuration includes onboard LEDs. Remove the contradictory
external-strip description without changing the configured start index.
In `@src/main/java/frc/robot/subsystems/vision/Vision.java`:
- Around line 444-452: Update the heading variance logic near the ambiguity and
rotation checks so these overrides cannot reduce a previously selected large
variance: apply the 15 and 50 values only when they are greater than the
existing degStds, or otherwise preserve the larger value. Use
Math.abs(robotSpeed.omegaRadiansPerSecond) for the rotation threshold so both
rotation directions trigger the same behavior, consistent with rejectionCheck.
In `@src/main/java/frc/spectrumLib/gamepads/Gamepad.java`:
- Around line 118-143: Initialize the nine composite modifier
triggers—noBumpers, leftBumperOnly, rightBumperOnly, bothBumpers, noTriggers,
leftTriggerOnly, rightTriggerOnly, bothTriggers, and noModifiers—to kFalse
before the config.attached conditional in Gamepad’s constructor, while
preserving their existing attached-controller assignments so bindings remain
safe when the controller is detached.
In `@src/main/java/frc/spectrumLib/README.md`:
- Around line 9-21: Update the package-structure fenced block in the README to
specify the text language and change the leds entry to describe the CTRE CANdle
wrapper, keeping it consistent with the SpectrumLEDs documentation.
---
Nitpick comments:
In `@src/main/deploy/elastic-layout.json`:
- Around line 174-186: Update the title of the widget identified by topic
"/Robot/FuelIntake/SystemState" from "Intake/SystemState" to
"FuelIntake/SystemState", leaving its topic and other properties unchanged.
In `@src/main/java/frc/rebuilt/FuelPhysicsSim.java`:
- Around line 2316-2323: Update the stats publishing block in publishPositions
to remove the unconditional computeConservationQuantities() call and publish the
cached totalKE + totalPE value. Ensure conservation quantities are recomputed
only through the existing config.conservationMonitor-gated logic in stepSubtick,
and avoid publishing per-tick energy telemetry when that monitor is disabled.
- Around line 1421-1429: Replace truncating coordinate-to-cell conversion in the
grid-building loop and generateBallBallContacts with floor-based indexing, such
as Math.floorDiv-style behavior, so negative positions map outside the valid
grid instead of column or row zero. Preserve the existing bounds checks and skip
out-of-range cells, ensuring balls at negative X or Y are not grouped with balls
inside the field.
In `@src/main/java/frc/rebuilt/ShotCalculator.java`:
- Line 452: Rename the telemetry key in ShotCalculator from
ShotCalc/HubPolyModel to ShotCalc/ActivePolyModel while continuing to log
model.name() as the active model. Update any dashboard layout references from
the old key to the new key.
In `@src/main/java/frc/robot/RobotSim.java`:
- Around line 46-57: Update simLaunching() to capture the injected
robotSuperStructure in a local variable inside the Trigger lambda and use it for
all current-super-state checks instead of calling Robot.getSuperStructure().
Apply the same replacement at the other repeated access near line 161, using the
instance field directly where the method is non-static.
- Around line 196-229: In the deferred launch command, replace the four
duplicated lane-specific loops with a loop over the available lane offsets,
creating one sequential command group per lane and passing each offset to
createSimBallLaunch. Ensure every intaked ball launches by distributing the fuel
remainder across lanes or otherwise accounting for fuelCount % numOfLanes, while
preserving the existing randomized startup delays, inter-launch waits, and
parallel execution.
In `@src/main/java/frc/robot/subsystems/fuelIntake/FuelIntake.java`:
- Around line 111-113: Replace the per-loop capturing suppliers in
FuelIntake.java lines 111-113, Hood.java lines 114-116, IndexerBed.java lines
95-96, and IndexerTower.java lines 98-99: add a field for each desired setpoint,
update that field in the corresponding applyStates() method, and reuse a single
DoubleSupplier field with each Mechanism setter (setVoltageOutput,
setMMPositionFoc, or setVelocityTCFOCrpm).
In `@src/main/java/frc/robot/subsystems/SuperStructure.java`:
- Around line 177-317: Optionally refactor the repeated state-setting logic in
SuperStructure methods such as applyIdle, intakeFuel, launchWithSqueeze, and
launchWithoutSqueeze into a shared record or map of per-state setpoints with one
application method. Ensure every state specifies all relevant subsystem
commands, including intakeExtension for launchWithoutSqueeze, while preserving
each state’s existing behavior.
In `@src/main/java/frc/robot/subsystems/swerve/Swerve.java`:
- Around line 297-302: Cache a single SwerveRequest.Idle instance alongside the
other request fields in Swerve, then update applyStates() to pass that cached
instance to setControl() for the IDLE state instead of constructing one on each
loop.
In `@src/main/java/frc/spectrumLib/framework/SpectrumRobot.java`:
- Around line 24-33: Update the reflection failure handling around
IterativeRobotBase.m_watchdog and Watchdog.setTimeout in SpectrumRobot so
DriverStation.reportWarning includes the caught exception details along with the
existing failure context, while preserving the current fallback behavior.
In `@src/main/java/frc/spectrumLib/mechanism/Mechanism.java`:
- Around line 119-128: Update the status-signal frequency configuration in
Mechanism so control-relevant signals retain the high update rate while
motor.getDeviceTemp() uses a separate low frequency such as 4 Hz. Apply the same
change to every corresponding signal-group setup, including follower motors,
without changing the existing signal selection.
In `@src/main/java/frc/spectrumLib/sim/ArmConfig.java`:
- Around line 103-106: Add Javadoc to the public fluent setter
setSimulatedGravity in ArmConfig, documenting the simulateGravity parameter and
that the method returns this configuration instance for chaining. Keep the
setter behavior unchanged.
In `@src/main/java/frc/spectrumLib/sim/LinearConfig.java`:
- Around line 142-146: Remove the stray standalone semicolon from
setStaticLength after assigning staticLength, leaving the conversion assignment
and fluent return unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 427d8532-1eb7-4216-aa82-e1f0a0dbb861
📒 Files selected for processing (114)
.vscode/settings.jsonadvantagescope-custom-assets/README.txtadvantagescope-custom-assets/Robot_FM/config.jsonadvantagescope-custom-assets/Robot_FM/model.glbadvantagescope-custom-assets/Robot_FM/model_0.glbadvantagescope-custom-assets/Robot_FM/model_1.glbadvantagescope-custom-assets/Robot_PM/config.jsonadvantagescope-custom-assets/Robot_PM/model.glbadvantagescope-custom-assets/Robot_PM/model_0.glbadvantagescope-custom-assets/Robot_PM/model_1.glbsimgui-window.jsonsrc/main/deploy/elastic-layout.jsonsrc/main/deploy/pathplanner/autos/TBTB Full.autosrc/main/deploy/pathplanner/settings.jsonsrc/main/java/edu/wpi/first/wpilibj2/command/button/README.mdsrc/main/java/edu/wpi/first/wpilibj2/command/button/Trigger.javasrc/main/java/frc/rebuilt/Field.javasrc/main/java/frc/rebuilt/FieldHelpers.javasrc/main/java/frc/rebuilt/FuelPhysicsSim.javasrc/main/java/frc/rebuilt/RobotBumpSim.javasrc/main/java/frc/rebuilt/ShiftHelpers.javasrc/main/java/frc/rebuilt/ShotCalculator.javasrc/main/java/frc/rebuilt/TagProperties.javasrc/main/java/frc/rebuilt/Zones.javasrc/main/java/frc/rebuilt/launchingMaps/AndyMarkMap.javasrc/main/java/frc/rebuilt/launchingMaps/HomeMap.javasrc/main/java/frc/rebuilt/offsets/HomeOffsets.javasrc/main/java/frc/rebuilt/targetFactories/FeedTargetFactory.javasrc/main/java/frc/robot/Coordinator.javasrc/main/java/frc/robot/Robot.javasrc/main/java/frc/robot/RobotSim.javasrc/main/java/frc/robot/RobotStates.javasrc/main/java/frc/robot/State.javasrc/main/java/frc/robot/auton/Auton.javasrc/main/java/frc/robot/fuelIntake/FuelIntakeStates.javasrc/main/java/frc/robot/hood/Hood.javasrc/main/java/frc/robot/hood/HoodStates.javasrc/main/java/frc/robot/indexerBed/IndexerBed.javasrc/main/java/frc/robot/indexerBed/IndexerBedStates.javasrc/main/java/frc/robot/indexerTower/IndexerTower.javasrc/main/java/frc/robot/indexerTower/IndexerTowerStates.javasrc/main/java/frc/robot/intakeExtension/IntakeExtension.javasrc/main/java/frc/robot/intakeExtension/IntakeExtensionStates.javasrc/main/java/frc/robot/launcher/Launcher.javasrc/main/java/frc/robot/launcher/LauncherStates.javasrc/main/java/frc/robot/leds/CANdleLeds.javasrc/main/java/frc/robot/leds/LedStates.javasrc/main/java/frc/robot/operator/Operator.javasrc/main/java/frc/robot/operator/OperatorStates.javasrc/main/java/frc/robot/pilot/Pilot.javasrc/main/java/frc/robot/pilot/PilotStates.javasrc/main/java/frc/robot/subsystems/SuperStructure.javasrc/main/java/frc/robot/subsystems/fuelIntake/FuelIntake.javasrc/main/java/frc/robot/subsystems/hood/Hood.javasrc/main/java/frc/robot/subsystems/indexerBed/IndexerBed.javasrc/main/java/frc/robot/subsystems/indexerTower/IndexerTower.javasrc/main/java/frc/robot/subsystems/intakeExtension/IntakeExtension.javasrc/main/java/frc/robot/subsystems/launcher/Launcher.javasrc/main/java/frc/robot/subsystems/leds/Leds.javasrc/main/java/frc/robot/subsystems/swerve/Swerve.javasrc/main/java/frc/robot/subsystems/swerve/SwerveConfig.javasrc/main/java/frc/robot/subsystems/vision/Vision.javasrc/main/java/frc/robot/swerve/SwerveStates.javasrc/main/java/frc/robot/swerve/controllers/RotationController.javasrc/main/java/frc/robot/swerve/controllers/TagCenterAlignController.javasrc/main/java/frc/robot/swerve/controllers/TagDistanceAlignController.javasrc/main/java/frc/robot/swerve/controllers/TranslationXController.javasrc/main/java/frc/robot/swerve/controllers/TranslationYController.javasrc/main/java/frc/robot/vision/Vision.javasrc/main/java/frc/robot/vision/VisionStates.javasrc/main/java/frc/robot/vision/VisionSystem.javasrc/main/java/frc/spectrumLib/BatteryLogger.javasrc/main/java/frc/spectrumLib/README.mdsrc/main/java/frc/spectrumLib/SpectrumCANcoderConfig.javasrc/main/java/frc/spectrumLib/SpectrumRobot.javasrc/main/java/frc/spectrumLib/SpectrumServo.javasrc/main/java/frc/spectrumLib/SpectrumSubsystem.javasrc/main/java/frc/spectrumLib/TuneValue.javasrc/main/java/frc/spectrumLib/framework/SpectrumRobot.javasrc/main/java/frc/spectrumLib/framework/SpectrumState.javasrc/main/java/frc/spectrumLib/gamepads/Gamepad.javasrc/main/java/frc/spectrumLib/hardware/Rio.javasrc/main/java/frc/spectrumLib/hardware/SpectrumCANcoder.javasrc/main/java/frc/spectrumLib/hardware/SpectrumCANcoderConfig.javasrc/main/java/frc/spectrumLib/hardware/SpectrumServo.javasrc/main/java/frc/spectrumLib/hardware/TalonFXFactory.javasrc/main/java/frc/spectrumLib/leds/SpectrumLEDs.javasrc/main/java/frc/spectrumLib/mechanism/Mechanism.javasrc/main/java/frc/spectrumLib/sim/ArmConfig.javasrc/main/java/frc/spectrumLib/sim/ArmSim.javasrc/main/java/frc/spectrumLib/sim/Circle.javasrc/main/java/frc/spectrumLib/sim/LinearConfig.javasrc/main/java/frc/spectrumLib/sim/LinearSim.javasrc/main/java/frc/spectrumLib/sim/Mount.javasrc/main/java/frc/spectrumLib/sim/Mountable.javasrc/main/java/frc/spectrumLib/sim/RollerConfig.javasrc/main/java/frc/spectrumLib/sim/RollerSim.javasrc/main/java/frc/spectrumLib/swerve/MapleSimSwerveDrivetrain.javasrc/main/java/frc/spectrumLib/swerve/SysID.javasrc/main/java/frc/spectrumLib/telemetry/BatteryLogger.javasrc/main/java/frc/spectrumLib/telemetry/Telemetry.javasrc/main/java/frc/spectrumLib/telemetry/TuneValue.javasrc/main/java/frc/spectrumLib/util/CachedDouble.javasrc/main/java/frc/spectrumLib/util/CanDeviceId.javasrc/main/java/frc/spectrumLib/util/CrashTracker.javasrc/main/java/frc/spectrumLib/util/ExpCurve.javasrc/main/java/frc/spectrumLib/util/Network.javasrc/main/java/frc/spectrumLib/util/Trio.javasrc/main/java/frc/spectrumLib/util/Util.javasrc/main/java/frc/spectrumLib/util/exceptions/KillRobotException.javasrc/main/java/frc/spectrumLib/vision/Limelight.javasrc/main/java/frc/spectrumLib/vision/VisionLogger.javavendordeps/Phoenix6-26.3.0.jsonvendordeps/photonlib.json
💤 Files with no reviewable changes (41)
- src/main/java/frc/spectrumLib/SpectrumSubsystem.java
- src/main/java/frc/rebuilt/offsets/HomeOffsets.java
- src/main/java/frc/spectrumLib/TuneValue.java
- src/main/java/frc/robot/operator/OperatorStates.java
- src/main/java/frc/robot/indexerTower/IndexerTower.java
- src/main/java/frc/robot/leds/CANdleLeds.java
- src/main/java/frc/robot/intakeExtension/IntakeExtension.java
- src/main/java/frc/robot/indexerTower/IndexerTowerStates.java
- src/main/java/frc/rebuilt/launchingMaps/AndyMarkMap.java
- src/main/java/frc/robot/RobotStates.java
- src/main/java/frc/robot/intakeExtension/IntakeExtensionStates.java
- src/main/java/frc/robot/hood/Hood.java
- src/main/java/edu/wpi/first/wpilibj2/command/button/README.md
- src/main/java/frc/robot/swerve/controllers/TranslationXController.java
- src/main/java/frc/robot/swerve/controllers/TagCenterAlignController.java
- src/main/java/frc/robot/launcher/Launcher.java
- src/main/java/frc/robot/indexerBed/IndexerBed.java
- src/main/java/frc/rebuilt/TagProperties.java
- src/main/java/frc/robot/State.java
- src/main/java/frc/robot/fuelIntake/FuelIntakeStates.java
- src/main/java/frc/robot/hood/HoodStates.java
- src/main/java/frc/spectrumLib/SpectrumServo.java
- src/main/java/frc/rebuilt/launchingMaps/HomeMap.java
- src/main/java/frc/robot/pilot/PilotStates.java
- src/main/java/frc/spectrumLib/SpectrumCANcoderConfig.java
- src/main/java/frc/robot/vision/VisionStates.java
- src/main/java/frc/robot/swerve/controllers/RotationController.java
- src/main/java/frc/rebuilt/Zones.java
- src/main/java/edu/wpi/first/wpilibj2/command/button/Trigger.java
- src/main/java/frc/robot/indexerBed/IndexerBedStates.java
- src/main/java/frc/rebuilt/FieldHelpers.java
- src/main/java/frc/robot/vision/Vision.java
- src/main/java/frc/robot/swerve/controllers/TagDistanceAlignController.java
- src/main/java/frc/spectrumLib/BatteryLogger.java
- src/main/java/frc/robot/vision/VisionSystem.java
- src/main/java/frc/robot/Coordinator.java
- src/main/java/frc/robot/launcher/LauncherStates.java
- src/main/java/frc/spectrumLib/SpectrumRobot.java
- src/main/java/frc/robot/leds/LedStates.java
- src/main/java/frc/robot/swerve/SwerveStates.java
- src/main/java/frc/robot/swerve/controllers/TranslationYController.java
👮 Files not reviewed due to content moderation or server errors (1)
- advantagescope-custom-assets/Robot_FM/model_1.glb
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 10
♻️ Duplicate comments (4)
src/main/java/frc/spectrumLib/README.md (1)
9-21: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLine 17 still describes the
ledspackage as anAddressableLEDwrapper.Line 99 documents
SpectrumLEDsas a CTRE CANdle subsystem. The package-structure listing contradicts it. Also add a language to the fenced block at line 9, which markdownlint flags as MD040.📝 Proposed fix
-``` +```text frc.spectrumLib ├── framework/ Base classes and interfaces for robot and subsystem architecture @@ -├── leds/ AddressableLED wrapper with built-in pattern library +├── leds/ CTRE CANdle wrapper with built-in pattern library🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/spectrumLib/README.md` around lines 9 - 21, Update the package-structure fenced block in the README to specify the text language and change the leds entry to describe the CTRE CANdle wrapper, keeping it consistent with the SpectrumLEDs documentation.Source: Linters/SAST tools
src/main/java/frc/robot/subsystems/vision/Vision.java (1)
444-452: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe heading std-dev overrides narrow the variance and miss clockwise rotation.
Both assignments overwrite
degStdsunconditionally. The tiers "Close integration", "Proximity integration", and "Stable integration" setdegStds = config.getKLargeVariance()to discard the heading. Lines 446 and 451 then reduce that value to 15 or 50, so a low-confidence heading is fused. Line 450 also omitsMath.abs, so clockwise rotation never triggers the override;rejectionCheckusesMath.absfor the same quantity at line 598.🐛 Proposed fix
// Widen heading std-dev when ambiguity is moderate if (highestAmbiguity > 0.5) { - degStds = 15; + degStds = Math.max(degStds, 15); } // Discard heading during fast rotation (MegaTag1 heading unreliable while spinning) - if (robotSpeed.omegaRadiansPerSecond >= 0.5) { - degStds = 50; + if (Math.abs(robotSpeed.omegaRadiansPerSecond) >= 0.5) { + degStds = Math.max(degStds, 50); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/robot/subsystems/vision/Vision.java` around lines 444 - 452, Update the heading variance logic near the ambiguity and rotation checks so these overrides cannot reduce a previously selected large variance: apply the 15 and 50 values only when they are greater than the existing degStds, or otherwise preserve the larger value. Use Math.abs(robotSpeed.omegaRadiansPerSecond) for the rotation threshold so both rotation directions trigger the same behavior, consistent with rejectionCheck.src/main/java/frc/spectrumLib/gamepads/Gamepad.java (1)
118-143: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winInitialize the composite modifier triggers to
kFalse.The class documentation at lines 42-43 states that all triggers remain
falsewhenConfig.isAttached()returnsfalse. These nine fields do not follow that contract. The constructor assigns them only inside theif (config.attached)block at lines 311-319. If the controller is not attached, they staynull, and a binding such aspilot.noModifiers.and(...)throws aNullPointerExceptionduring binding configuration. The individual button fields already default tokFalse.🛡️ Proposed fix
- public Trigger noBumpers; + public Trigger noBumpers = kFalse; @@ - public Trigger leftBumperOnly; + public Trigger leftBumperOnly = kFalse; @@ - public Trigger rightBumperOnly; + public Trigger rightBumperOnly = kFalse; @@ - public Trigger bothBumpers; + public Trigger bothBumpers = kFalse; @@ - public Trigger noTriggers; + public Trigger noTriggers = kFalse; @@ - public Trigger leftTriggerOnly; + public Trigger leftTriggerOnly = kFalse; @@ - public Trigger rightTriggerOnly; + public Trigger rightTriggerOnly = kFalse; @@ - public Trigger bothTriggers; + public Trigger bothTriggers = kFalse; @@ - public Trigger noModifiers; + public Trigger noModifiers = kFalse;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/spectrumLib/gamepads/Gamepad.java` around lines 118 - 143, Initialize the nine composite modifier triggers—noBumpers, leftBumperOnly, rightBumperOnly, bothBumpers, noTriggers, leftTriggerOnly, rightTriggerOnly, bothTriggers, and noModifiers—to kFalse before the config.attached conditional in Gamepad’s constructor, while preserving their existing attached-controller assignments so bindings remain safe when the controller is detached.src/main/java/frc/robot/subsystems/leds/Leds.java (1)
27-42: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe
NUM_LEDScomment contradicts the configured index range.Line 27 states the 20 external LEDs occupy indices 8–27. The static block does not call
setStartIdx(8), so the configuration addresses indices 0–19, which includes the 8 onboard CANdle LEDs. Lines 30-34 describe both options, so the intent is unclear. Align the comment with the configured value, or setstartIdxto 8.📝 Proposed documentation fix
- /** Number of external LEDs attached to the CANdle output (indices 8–27 on the device). */ + /** + * Number of LEDs driven by this configuration. With the current {`@code` startIdx} of 0 this + * addresses device indices 0–19 (the 8 onboard CANdle LEDs plus the first 12 external LEDs). + */ public static final int NUM_LEDS = 20;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/robot/subsystems/leds/Leds.java` around lines 27 - 42, Align the LED range documentation with the actual configuration in the Leds static initializer: since ledsConfig does not call setStartIdx(8), document NUM_LEDS as covering indices 0–19 and clarify that the current configuration includes onboard LEDs. Remove the contradictory external-strip description without changing the configured start index.
🧹 Nitpick comments (13)
src/main/java/frc/robot/RobotSim.java (2)
46-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the injected
SuperStructureinstead of the static accessor.
simLaunching()readsRobot.getSuperStructure()three times, and line 161 repeats the pattern, while the class already storesrobotSuperStructure. Extract a local variable inside the lambda, and prefer the instance field where the method is not static.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/robot/RobotSim.java` around lines 46 - 57, Update simLaunching() to capture the injected robotSuperStructure in a local variable inside the Trigger lambda and use it for all current-super-state checks instead of calling Robot.getSuperStructure(). Apply the same replacement at the other repeated access near line 161, using the instance field directly where the method is non-static.
196-229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the four identical lane loops, and handle the remainder.
The four blocks differ only by the lane offset. Build them in a loop over the lane offsets.
numToLaunchPerLane = fuelCount / numOfLanesalso uses integer division, so up tonumOfLanes - 1intaked balls never launch.♻️ Proposed change
return Commands.defer( () -> { int fuelCount = ballSim.getTotalIntaked(); - int numToLaunchPerLane = fuelCount / numOfLanes; - SequentialCommandGroup group1 = - new SequentialCommandGroup(Commands.waitSeconds(Math.random() * 0.3)); - for (int i = 0; i < numToLaunchPerLane; i++) { - group1.addCommands( - createSimBallLaunch(lane1), - Commands.waitSeconds(timeBetweenBallLaunches)); - } - SequentialCommandGroup group2 = - new SequentialCommandGroup(Commands.waitSeconds(Math.random() * 0.3)); - for (int i = 0; i < numToLaunchPerLane; i++) { - group2.addCommands( - createSimBallLaunch(lane2), - Commands.waitSeconds(timeBetweenBallLaunches)); - } - SequentialCommandGroup group3 = - new SequentialCommandGroup(Commands.waitSeconds(Math.random() * 0.3)); - for (int i = 0; i < numToLaunchPerLane; i++) { - group3.addCommands( - createSimBallLaunch(lane3), - Commands.waitSeconds(timeBetweenBallLaunches)); - } - SequentialCommandGroup group4 = - new SequentialCommandGroup(Commands.waitSeconds(Math.random() * 0.3)); - for (int i = 0; i < numToLaunchPerLane; i++) { - group4.addCommands( - createSimBallLaunch(lane4), - Commands.waitSeconds(timeBetweenBallLaunches)); - } - return Commands.parallel(group1, group2, group3, group4) + double[] lanes = {lane1, lane2, lane3, lane4}; + Command[] groups = new Command[lanes.length]; + for (int lane = 0; lane < lanes.length; lane++) { + // Distribute the remainder across the first lanes + int ballsForLane = + fuelCount / numOfLanes + (lane < fuelCount % numOfLanes ? 1 : 0); + SequentialCommandGroup group = + new SequentialCommandGroup( + Commands.waitSeconds(Math.random() * 0.3)); + for (int i = 0; i < ballsForLane; i++) { + group.addCommands( + createSimBallLaunch(lanes[lane]), + Commands.waitSeconds(timeBetweenBallLaunches)); + } + groups[lane] = group; + } + return Commands.parallel(groups) .withName("RobotSim.ballSimLaunchFuel");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/robot/RobotSim.java` around lines 196 - 229, In the deferred launch command, replace the four duplicated lane-specific loops with a loop over the available lane offsets, creating one sequential command group per lane and passing each offset to createSimBallLaunch. Ensure every intaked ball launches by distributing the fuel remainder across lanes or otherwise accounting for fuelCount % numOfLanes, while preserving the existing randomized startup delays, inter-launch waits, and parallel execution.src/main/deploy/elastic-layout.json (1)
174-186: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the widget title to match its topic. The title is
Intake/SystemState, but the topic is/Robot/FuelIntake/SystemState. All sibling widgets use the subsystem name. UseFuelIntake/SystemStatefor consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/deploy/elastic-layout.json` around lines 174 - 186, Update the title of the widget identified by topic "/Robot/FuelIntake/SystemState" from "Intake/SystemState" to "FuelIntake/SystemState", leaving its topic and other properties unchanged.src/main/java/frc/rebuilt/ShotCalculator.java (1)
452-452: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the telemetry key. The key is
ShotCalc/HubPolyModel, but the value is now the active model, which isFEED_MODELduring feed shots. UseShotCalc/ActivePolyModel. Update the dashboard layout if it references the old key.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/rebuilt/ShotCalculator.java` at line 452, Rename the telemetry key in ShotCalculator from ShotCalc/HubPolyModel to ShotCalc/ActivePolyModel while continuing to log model.name() as the active model. Update any dashboard layout references from the old key to the new key.src/main/java/frc/rebuilt/FuelPhysicsSim.java (2)
2316-2323: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winGate the conservation computation behind its config flag.
publishPositionscallscomputeConservationQuantities()on every tick. That is an extra full pass over all balls, plusgetNorm()per ball, on the 20 ms loop.stepSubtickalready computes it whenconfig.conservationMonitoris true, so the work is duplicated in that case and unwanted otherwise.Publish the cached value, and recompute only when the monitor is enabled.
♻️ Proposed change
- computeConservationQuantities(); - totalEnergyPub.set(totalKE + totalPE); + if (!config.conservationMonitor) { + computeConservationQuantities(); + } + totalEnergyPub.set(totalKE + totalPE);If per-tick energy telemetry is not needed, publish only when
config.conservationMonitoris true.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/rebuilt/FuelPhysicsSim.java` around lines 2316 - 2323, Update the stats publishing block in publishPositions to remove the unconditional computeConservationQuantities() call and publish the cached totalKE + totalPE value. Ensure conservation quantities are recomputed only through the existing config.conservationMonitor-gated logic in stepSubtick, and avoid publishing per-tick energy telemetry when that monitor is disabled.
1421-1429: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueClamp grid indices for negative coordinates. A ball just outside the field on the negative side, for example
x = -0.1, produces(int) (-0.1 / 0.25) == 0. The bounds check passes, and the ball is hashed into column 0 together with balls atx ∈ [0, 0.25). The same truncation applies ingenerateBallBallContacts, so a ball atx = -0.1and a ball atx = 0.1are treated as neighbours in the same cell. Contacts are still validated by distance, so the effect is a small amount of wasted narrowphase work, not a wrong result.Use
Math.floorDiv-style flooring, or skip balls with negative X or Y.Also applies to: 1440-1441
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/rebuilt/FuelPhysicsSim.java` around lines 1421 - 1429, Replace truncating coordinate-to-cell conversion in the grid-building loop and generateBallBallContacts with floor-based indexing, such as Math.floorDiv-style behavior, so negative positions map outside the valid grid instead of column or row zero. Preserve the existing bounds checks and skip out-of-range cells, ensuring balls at negative X or Y are not grouped with balls inside the field.src/main/java/frc/robot/subsystems/fuelIntake/FuelIntake.java (1)
111-113: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueEach
applyStates()allocates a new supplier every loop.All four state machines capture a local
final doublein a lambda and pass it to theMechanismsetter. A capturing lambda cannot be cached by the JVM, so every 20 ms loop allocates a new object per mechanism. Store the setpoint in a field and reuse one supplier instance instead. This is an optional cleanup; the current code is correct.
src/main/java/frc/robot/subsystems/fuelIntake/FuelIntake.java#L111-L113: hold the wanted voltage in a field and pass a singleDoubleSupplierfield tosetVoltageOutput.src/main/java/frc/robot/subsystems/hood/Hood.java#L114-L116: hold the wanted position in a field and pass a single supplier tosetMMPositionFoc.src/main/java/frc/robot/subsystems/indexerBed/IndexerBed.java#L95-L96: hold the wanted RPM in a field and pass a single supplier tosetVelocityTCFOCrpm.src/main/java/frc/robot/subsystems/indexerTower/IndexerTower.java#L98-L99: apply the same change for itssetVelocityTCFOCrpmcall.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/robot/subsystems/fuelIntake/FuelIntake.java` around lines 111 - 113, Replace the per-loop capturing suppliers in FuelIntake.java lines 111-113, Hood.java lines 114-116, IndexerBed.java lines 95-96, and IndexerTower.java lines 98-99: add a field for each desired setpoint, update that field in the corresponding applyStates() method, and reuse a single DoubleSupplier field with each Mechanism setter (setVoltageOutput, setMMPositionFoc, or setVelocityTCFOCrpm).src/main/java/frc/robot/subsystems/SuperStructure.java (1)
177-317: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider a table-driven state application.
The 11 state methods repeat the same seven
setWantedStatecalls with different constants. A single record or map of per-state setpoints would remove the repetition and make omissions, such as the missingintakeExtensioncall inlaunchWithoutSqueeze, visible. This is optional and can wait until the state set stabilizes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/robot/subsystems/SuperStructure.java` around lines 177 - 317, Optionally refactor the repeated state-setting logic in SuperStructure methods such as applyIdle, intakeFuel, launchWithSqueeze, and launchWithoutSqueeze into a shared record or map of per-state setpoints with one application method. Ensure every state specifies all relevant subsystem commands, including intakeExtension for launchWithoutSqueeze, while preserving each state’s existing behavior.src/main/java/frc/robot/subsystems/swerve/Swerve.java (1)
297-302: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueReuse a single
SwerveRequest.Idleinstance.
applyStates()allocates a newSwerveRequest.Idleon every loop while the state isIDLE. The other requests in this class are cached fields. Cache this one as well to avoid per-loop allocation on the RIO.♻️ Proposed refactor
+ private static final SwerveRequest.Idle IDLE_REQUEST = new SwerveRequest.Idle(); + private void applyStates() { switch (systemState) { default: case IDLE: - setControl(new SwerveRequest.Idle()); + setControl(IDLE_REQUEST); break;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/robot/subsystems/swerve/Swerve.java` around lines 297 - 302, Cache a single SwerveRequest.Idle instance alongside the other request fields in Swerve, then update applyStates() to pass that cached instance to setControl() for the IDLE state instead of constructing one on each loop.src/main/java/frc/spectrumLib/framework/SpectrumRobot.java (1)
24-33: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winInclude the reflection exception details in the warning. WPILib 2026.1.1 uses
IterativeRobotBase.m_watchdog, andWatchdog.setTimeout(double)is available. If reflection fails, report the exception details so the missing timeout is diagnosable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/spectrumLib/framework/SpectrumRobot.java` around lines 24 - 33, Update the reflection failure handling around IterativeRobotBase.m_watchdog and Watchdog.setTimeout in SpectrumRobot so DriverStation.reportWarning includes the caught exception details along with the existing failure context, while preserving the current fallback behavior.src/main/java/frc/spectrumLib/sim/LinearConfig.java (1)
142-146: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the stray empty statement in
setStaticLength.Line 144 contains a lone
;after the assignment.♻️ Proposed refactor
public LinearConfig setStaticLength(double lengthInches) { this.staticLength = Units.inchesToMeters(lengthInches); - ; return this; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/spectrumLib/sim/LinearConfig.java` around lines 142 - 146, Remove the stray standalone semicolon from setStaticLength after assigning staticLength, leaving the conversion assignment and fluent return unchanged.src/main/java/frc/spectrumLib/sim/ArmConfig.java (1)
103-106: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd Javadoc to
setSimulatedGravity.Every other public member in this file has Javadoc. This new fluent setter has none.
♻️ Proposed refactor
+ /** + * Sets whether the physics simulation applies gravity to the arm. + * + * `@param` simulateGravity {`@code` true} to apply gravitational force + * `@return` this config for chaining + */ public ArmConfig setSimulatedGravity(boolean simulateGravity) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/spectrumLib/sim/ArmConfig.java` around lines 103 - 106, Add Javadoc to the public fluent setter setSimulatedGravity in ArmConfig, documenting the simulateGravity parameter and that the method returns this configuration instance for chaining. Keep the setter behavior unchanged.src/main/java/frc/spectrumLib/mechanism/Mechanism.java (1)
119-128: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReconsider 250 Hz for all status signals, especially device temperature.
Each motor now publishes 8 signals at 250 Hz, which is 2000 frames per second per device. Followers add the same load. A
Mechanism.Configcan target the nativeriobus (1 Mbps classic CAN), where this rate plus swerve traffic can saturate the bus. Device temperature changes slowly and does not need 250 Hz.Consider keeping control-relevant signals at a high rate and setting
getDeviceTemp()to a low rate, for example 4 Hz.♻️ Proposed refactor
BaseStatusSignal.setUpdateFrequencyForAll( 250, motor.getDutyCycle(), motor.getMotorVoltage(), motor.getTorqueCurrent(), motor.getStatorCurrent(), motor.getSupplyCurrent(), motor.getPosition(), - motor.getVelocity(), - motor.getDeviceTemp()); + motor.getVelocity()); + motor.getDeviceTemp().setUpdateFrequency(4);Also applies to: 138-147
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/spectrumLib/mechanism/Mechanism.java` around lines 119 - 128, Update the status-signal frequency configuration in Mechanism so control-relevant signals retain the high update rate while motor.getDeviceTemp() uses a separate low frequency such as 4 Hz. Apply the same change to every corresponding signal-group setup, including follower motors, without changing the existing signal selection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/frc/rebuilt/FuelPhysicsSim.java`:
- Around line 1036-1039: Swap the “Blue-side depot” and “Red-side depot”
comments in the depot setup near fillBallGrid so they match the coordinates: the
call using X 0.37 is blue, and the call using X FIELD_LENGTH - 0.37 is red.
Leave the coordinates and fillBallGrid calls unchanged.
- Around line 117-129: Replace the local FIELD_LENGTH, FIELD_WIDTH,
BLUE_HUB_CENTER, and RED_HUB_CENTER values in FuelPhysicsSim with the canonical
Field.fieldLength and Field.fieldWidth constants, and derive both hub centers
from Field.BlueHub. Update all references to use these shared geometry
definitions while leaving unrelated bump and wall constants unchanged.
In `@src/main/java/frc/rebuilt/ShotCalculator.java`:
- Around line 365-372: Guard the unit-vector calculations in the
radial/tangential decomposition before dividing by distanceNoLookahead. When the
distance is zero or below a small epsilon, use zero radial and tangential
velocity components; otherwise retain the existing ux/uy-based calculations so
solveVirtualTarget receives finite values.
In `@src/main/java/frc/robot/RobotSim.java`:
- Around line 140-163: Correct the intake-zone bounds in configBallSimRobot: use
simRobotLength and intakeLength to calculate intakeXMin and intakeXMax, and use
simRobotWidth and intakeWidth for intakeYMin and intakeYMax, preserving the
existing coordinate orientation and intake-state predicate.
In `@src/main/java/frc/robot/subsystems/hood/Hood.java`:
- Around line 23-24: Remove the `@Setter` annotations from maxRotations and
minRotations in Hood, since changing these fields does not update the motor’s
configured soft limits; keep the constructor’s existing limit configuration
behavior unchanged.
In `@src/main/java/frc/robot/subsystems/SuperStructure.java`:
- Around line 238-246: Update launchWithoutSqueeze to explicitly command
intakeExtension with the intended IntakeExtension.WantedState, matching the
state-setting pattern used by the other SuperStructure methods and preventing
the previous superstate’s command from persisting.
In `@src/main/java/frc/robot/subsystems/vision/Vision.java`:
- Around line 466-467: Update the count calculation in the vision method around
getRawFiducial() so tags.length is only accessed when tags is non-null;
otherwise use a count of zero. Remove the `@SuppressWarnings`("null") annotation
rather than masking the nullable path.
- Around line 591-596: Update the stale-timestamp validation in the surrounding
vision pose-processing method to compare the raw Limelight/WPILib timestamp from
the appropriate getter directly against Timer.getFPGATimestamp(), removing the
Utils.fpgaToCurrentTime conversion. When validating an MT2 pose, use
getMegaTag2PoseTimestamp(); retain the existing rejection threshold and
invalid-status behavior.
In `@src/main/java/frc/spectrumLib/telemetry/BatteryLogger.java`:
- Around line 113-130: Update the parent-key construction loop in
BatteryLogger’s aggregation logic so each prefix is merged without a trailing
separator. Build and merge the current prefix first, then append "/" only before
adding the next segment, ensuring keys such as "Drive" remain consistent across
multi-segment and two-segment reports.
In `@src/main/java/frc/spectrumLib/telemetry/Telemetry.java`:
- Around line 67-99: Update the Javadoc for Telemetry.start so the parameter
descriptions match the method signature: document captureDs before captureNt,
describe captureDs as capturing Driver Station data, and retain the
NetworkTables description for captureNt.
---
Duplicate comments:
In `@src/main/java/frc/robot/subsystems/leds/Leds.java`:
- Around line 27-42: Align the LED range documentation with the actual
configuration in the Leds static initializer: since ledsConfig does not call
setStartIdx(8), document NUM_LEDS as covering indices 0–19 and clarify that the
current configuration includes onboard LEDs. Remove the contradictory
external-strip description without changing the configured start index.
In `@src/main/java/frc/robot/subsystems/vision/Vision.java`:
- Around line 444-452: Update the heading variance logic near the ambiguity and
rotation checks so these overrides cannot reduce a previously selected large
variance: apply the 15 and 50 values only when they are greater than the
existing degStds, or otherwise preserve the larger value. Use
Math.abs(robotSpeed.omegaRadiansPerSecond) for the rotation threshold so both
rotation directions trigger the same behavior, consistent with rejectionCheck.
In `@src/main/java/frc/spectrumLib/gamepads/Gamepad.java`:
- Around line 118-143: Initialize the nine composite modifier
triggers—noBumpers, leftBumperOnly, rightBumperOnly, bothBumpers, noTriggers,
leftTriggerOnly, rightTriggerOnly, bothTriggers, and noModifiers—to kFalse
before the config.attached conditional in Gamepad’s constructor, while
preserving their existing attached-controller assignments so bindings remain
safe when the controller is detached.
In `@src/main/java/frc/spectrumLib/README.md`:
- Around line 9-21: Update the package-structure fenced block in the README to
specify the text language and change the leds entry to describe the CTRE CANdle
wrapper, keeping it consistent with the SpectrumLEDs documentation.
---
Nitpick comments:
In `@src/main/deploy/elastic-layout.json`:
- Around line 174-186: Update the title of the widget identified by topic
"/Robot/FuelIntake/SystemState" from "Intake/SystemState" to
"FuelIntake/SystemState", leaving its topic and other properties unchanged.
In `@src/main/java/frc/rebuilt/FuelPhysicsSim.java`:
- Around line 2316-2323: Update the stats publishing block in publishPositions
to remove the unconditional computeConservationQuantities() call and publish the
cached totalKE + totalPE value. Ensure conservation quantities are recomputed
only through the existing config.conservationMonitor-gated logic in stepSubtick,
and avoid publishing per-tick energy telemetry when that monitor is disabled.
- Around line 1421-1429: Replace truncating coordinate-to-cell conversion in the
grid-building loop and generateBallBallContacts with floor-based indexing, such
as Math.floorDiv-style behavior, so negative positions map outside the valid
grid instead of column or row zero. Preserve the existing bounds checks and skip
out-of-range cells, ensuring balls at negative X or Y are not grouped with balls
inside the field.
In `@src/main/java/frc/rebuilt/ShotCalculator.java`:
- Line 452: Rename the telemetry key in ShotCalculator from
ShotCalc/HubPolyModel to ShotCalc/ActivePolyModel while continuing to log
model.name() as the active model. Update any dashboard layout references from
the old key to the new key.
In `@src/main/java/frc/robot/RobotSim.java`:
- Around line 46-57: Update simLaunching() to capture the injected
robotSuperStructure in a local variable inside the Trigger lambda and use it for
all current-super-state checks instead of calling Robot.getSuperStructure().
Apply the same replacement at the other repeated access near line 161, using the
instance field directly where the method is non-static.
- Around line 196-229: In the deferred launch command, replace the four
duplicated lane-specific loops with a loop over the available lane offsets,
creating one sequential command group per lane and passing each offset to
createSimBallLaunch. Ensure every intaked ball launches by distributing the fuel
remainder across lanes or otherwise accounting for fuelCount % numOfLanes, while
preserving the existing randomized startup delays, inter-launch waits, and
parallel execution.
In `@src/main/java/frc/robot/subsystems/fuelIntake/FuelIntake.java`:
- Around line 111-113: Replace the per-loop capturing suppliers in
FuelIntake.java lines 111-113, Hood.java lines 114-116, IndexerBed.java lines
95-96, and IndexerTower.java lines 98-99: add a field for each desired setpoint,
update that field in the corresponding applyStates() method, and reuse a single
DoubleSupplier field with each Mechanism setter (setVoltageOutput,
setMMPositionFoc, or setVelocityTCFOCrpm).
In `@src/main/java/frc/robot/subsystems/SuperStructure.java`:
- Around line 177-317: Optionally refactor the repeated state-setting logic in
SuperStructure methods such as applyIdle, intakeFuel, launchWithSqueeze, and
launchWithoutSqueeze into a shared record or map of per-state setpoints with one
application method. Ensure every state specifies all relevant subsystem
commands, including intakeExtension for launchWithoutSqueeze, while preserving
each state’s existing behavior.
In `@src/main/java/frc/robot/subsystems/swerve/Swerve.java`:
- Around line 297-302: Cache a single SwerveRequest.Idle instance alongside the
other request fields in Swerve, then update applyStates() to pass that cached
instance to setControl() for the IDLE state instead of constructing one on each
loop.
In `@src/main/java/frc/spectrumLib/framework/SpectrumRobot.java`:
- Around line 24-33: Update the reflection failure handling around
IterativeRobotBase.m_watchdog and Watchdog.setTimeout in SpectrumRobot so
DriverStation.reportWarning includes the caught exception details along with the
existing failure context, while preserving the current fallback behavior.
In `@src/main/java/frc/spectrumLib/mechanism/Mechanism.java`:
- Around line 119-128: Update the status-signal frequency configuration in
Mechanism so control-relevant signals retain the high update rate while
motor.getDeviceTemp() uses a separate low frequency such as 4 Hz. Apply the same
change to every corresponding signal-group setup, including follower motors,
without changing the existing signal selection.
In `@src/main/java/frc/spectrumLib/sim/ArmConfig.java`:
- Around line 103-106: Add Javadoc to the public fluent setter
setSimulatedGravity in ArmConfig, documenting the simulateGravity parameter and
that the method returns this configuration instance for chaining. Keep the
setter behavior unchanged.
In `@src/main/java/frc/spectrumLib/sim/LinearConfig.java`:
- Around line 142-146: Remove the stray standalone semicolon from
setStaticLength after assigning staticLength, leaving the conversion assignment
and fluent return unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 427d8532-1eb7-4216-aa82-e1f0a0dbb861
📒 Files selected for processing (114)
.vscode/settings.jsonadvantagescope-custom-assets/README.txtadvantagescope-custom-assets/Robot_FM/config.jsonadvantagescope-custom-assets/Robot_FM/model.glbadvantagescope-custom-assets/Robot_FM/model_0.glbadvantagescope-custom-assets/Robot_FM/model_1.glbadvantagescope-custom-assets/Robot_PM/config.jsonadvantagescope-custom-assets/Robot_PM/model.glbadvantagescope-custom-assets/Robot_PM/model_0.glbadvantagescope-custom-assets/Robot_PM/model_1.glbsimgui-window.jsonsrc/main/deploy/elastic-layout.jsonsrc/main/deploy/pathplanner/autos/TBTB Full.autosrc/main/deploy/pathplanner/settings.jsonsrc/main/java/edu/wpi/first/wpilibj2/command/button/README.mdsrc/main/java/edu/wpi/first/wpilibj2/command/button/Trigger.javasrc/main/java/frc/rebuilt/Field.javasrc/main/java/frc/rebuilt/FieldHelpers.javasrc/main/java/frc/rebuilt/FuelPhysicsSim.javasrc/main/java/frc/rebuilt/RobotBumpSim.javasrc/main/java/frc/rebuilt/ShiftHelpers.javasrc/main/java/frc/rebuilt/ShotCalculator.javasrc/main/java/frc/rebuilt/TagProperties.javasrc/main/java/frc/rebuilt/Zones.javasrc/main/java/frc/rebuilt/launchingMaps/AndyMarkMap.javasrc/main/java/frc/rebuilt/launchingMaps/HomeMap.javasrc/main/java/frc/rebuilt/offsets/HomeOffsets.javasrc/main/java/frc/rebuilt/targetFactories/FeedTargetFactory.javasrc/main/java/frc/robot/Coordinator.javasrc/main/java/frc/robot/Robot.javasrc/main/java/frc/robot/RobotSim.javasrc/main/java/frc/robot/RobotStates.javasrc/main/java/frc/robot/State.javasrc/main/java/frc/robot/auton/Auton.javasrc/main/java/frc/robot/fuelIntake/FuelIntakeStates.javasrc/main/java/frc/robot/hood/Hood.javasrc/main/java/frc/robot/hood/HoodStates.javasrc/main/java/frc/robot/indexerBed/IndexerBed.javasrc/main/java/frc/robot/indexerBed/IndexerBedStates.javasrc/main/java/frc/robot/indexerTower/IndexerTower.javasrc/main/java/frc/robot/indexerTower/IndexerTowerStates.javasrc/main/java/frc/robot/intakeExtension/IntakeExtension.javasrc/main/java/frc/robot/intakeExtension/IntakeExtensionStates.javasrc/main/java/frc/robot/launcher/Launcher.javasrc/main/java/frc/robot/launcher/LauncherStates.javasrc/main/java/frc/robot/leds/CANdleLeds.javasrc/main/java/frc/robot/leds/LedStates.javasrc/main/java/frc/robot/operator/Operator.javasrc/main/java/frc/robot/operator/OperatorStates.javasrc/main/java/frc/robot/pilot/Pilot.javasrc/main/java/frc/robot/pilot/PilotStates.javasrc/main/java/frc/robot/subsystems/SuperStructure.javasrc/main/java/frc/robot/subsystems/fuelIntake/FuelIntake.javasrc/main/java/frc/robot/subsystems/hood/Hood.javasrc/main/java/frc/robot/subsystems/indexerBed/IndexerBed.javasrc/main/java/frc/robot/subsystems/indexerTower/IndexerTower.javasrc/main/java/frc/robot/subsystems/intakeExtension/IntakeExtension.javasrc/main/java/frc/robot/subsystems/launcher/Launcher.javasrc/main/java/frc/robot/subsystems/leds/Leds.javasrc/main/java/frc/robot/subsystems/swerve/Swerve.javasrc/main/java/frc/robot/subsystems/swerve/SwerveConfig.javasrc/main/java/frc/robot/subsystems/vision/Vision.javasrc/main/java/frc/robot/swerve/SwerveStates.javasrc/main/java/frc/robot/swerve/controllers/RotationController.javasrc/main/java/frc/robot/swerve/controllers/TagCenterAlignController.javasrc/main/java/frc/robot/swerve/controllers/TagDistanceAlignController.javasrc/main/java/frc/robot/swerve/controllers/TranslationXController.javasrc/main/java/frc/robot/swerve/controllers/TranslationYController.javasrc/main/java/frc/robot/vision/Vision.javasrc/main/java/frc/robot/vision/VisionStates.javasrc/main/java/frc/robot/vision/VisionSystem.javasrc/main/java/frc/spectrumLib/BatteryLogger.javasrc/main/java/frc/spectrumLib/README.mdsrc/main/java/frc/spectrumLib/SpectrumCANcoderConfig.javasrc/main/java/frc/spectrumLib/SpectrumRobot.javasrc/main/java/frc/spectrumLib/SpectrumServo.javasrc/main/java/frc/spectrumLib/SpectrumSubsystem.javasrc/main/java/frc/spectrumLib/TuneValue.javasrc/main/java/frc/spectrumLib/framework/SpectrumRobot.javasrc/main/java/frc/spectrumLib/framework/SpectrumState.javasrc/main/java/frc/spectrumLib/gamepads/Gamepad.javasrc/main/java/frc/spectrumLib/hardware/Rio.javasrc/main/java/frc/spectrumLib/hardware/SpectrumCANcoder.javasrc/main/java/frc/spectrumLib/hardware/SpectrumCANcoderConfig.javasrc/main/java/frc/spectrumLib/hardware/SpectrumServo.javasrc/main/java/frc/spectrumLib/hardware/TalonFXFactory.javasrc/main/java/frc/spectrumLib/leds/SpectrumLEDs.javasrc/main/java/frc/spectrumLib/mechanism/Mechanism.javasrc/main/java/frc/spectrumLib/sim/ArmConfig.javasrc/main/java/frc/spectrumLib/sim/ArmSim.javasrc/main/java/frc/spectrumLib/sim/Circle.javasrc/main/java/frc/spectrumLib/sim/LinearConfig.javasrc/main/java/frc/spectrumLib/sim/LinearSim.javasrc/main/java/frc/spectrumLib/sim/Mount.javasrc/main/java/frc/spectrumLib/sim/Mountable.javasrc/main/java/frc/spectrumLib/sim/RollerConfig.javasrc/main/java/frc/spectrumLib/sim/RollerSim.javasrc/main/java/frc/spectrumLib/swerve/MapleSimSwerveDrivetrain.javasrc/main/java/frc/spectrumLib/swerve/SysID.javasrc/main/java/frc/spectrumLib/telemetry/BatteryLogger.javasrc/main/java/frc/spectrumLib/telemetry/Telemetry.javasrc/main/java/frc/spectrumLib/telemetry/TuneValue.javasrc/main/java/frc/spectrumLib/util/CachedDouble.javasrc/main/java/frc/spectrumLib/util/CanDeviceId.javasrc/main/java/frc/spectrumLib/util/CrashTracker.javasrc/main/java/frc/spectrumLib/util/ExpCurve.javasrc/main/java/frc/spectrumLib/util/Network.javasrc/main/java/frc/spectrumLib/util/Trio.javasrc/main/java/frc/spectrumLib/util/Util.javasrc/main/java/frc/spectrumLib/util/exceptions/KillRobotException.javasrc/main/java/frc/spectrumLib/vision/Limelight.javasrc/main/java/frc/spectrumLib/vision/VisionLogger.javavendordeps/Phoenix6-26.3.0.jsonvendordeps/photonlib.json
💤 Files with no reviewable changes (41)
- src/main/java/frc/spectrumLib/SpectrumSubsystem.java
- src/main/java/frc/rebuilt/offsets/HomeOffsets.java
- src/main/java/frc/spectrumLib/TuneValue.java
- src/main/java/frc/robot/operator/OperatorStates.java
- src/main/java/frc/robot/indexerTower/IndexerTower.java
- src/main/java/frc/robot/leds/CANdleLeds.java
- src/main/java/frc/robot/intakeExtension/IntakeExtension.java
- src/main/java/frc/robot/indexerTower/IndexerTowerStates.java
- src/main/java/frc/rebuilt/launchingMaps/AndyMarkMap.java
- src/main/java/frc/robot/RobotStates.java
- src/main/java/frc/robot/intakeExtension/IntakeExtensionStates.java
- src/main/java/frc/robot/hood/Hood.java
- src/main/java/edu/wpi/first/wpilibj2/command/button/README.md
- src/main/java/frc/robot/swerve/controllers/TranslationXController.java
- src/main/java/frc/robot/swerve/controllers/TagCenterAlignController.java
- src/main/java/frc/robot/launcher/Launcher.java
- src/main/java/frc/robot/indexerBed/IndexerBed.java
- src/main/java/frc/rebuilt/TagProperties.java
- src/main/java/frc/robot/State.java
- src/main/java/frc/robot/fuelIntake/FuelIntakeStates.java
- src/main/java/frc/robot/hood/HoodStates.java
- src/main/java/frc/spectrumLib/SpectrumServo.java
- src/main/java/frc/rebuilt/launchingMaps/HomeMap.java
- src/main/java/frc/robot/pilot/PilotStates.java
- src/main/java/frc/spectrumLib/SpectrumCANcoderConfig.java
- src/main/java/frc/robot/vision/VisionStates.java
- src/main/java/frc/robot/swerve/controllers/RotationController.java
- src/main/java/frc/rebuilt/Zones.java
- src/main/java/edu/wpi/first/wpilibj2/command/button/Trigger.java
- src/main/java/frc/robot/indexerBed/IndexerBedStates.java
- src/main/java/frc/rebuilt/FieldHelpers.java
- src/main/java/frc/robot/vision/Vision.java
- src/main/java/frc/robot/swerve/controllers/TagDistanceAlignController.java
- src/main/java/frc/spectrumLib/BatteryLogger.java
- src/main/java/frc/robot/vision/VisionSystem.java
- src/main/java/frc/robot/Coordinator.java
- src/main/java/frc/robot/launcher/LauncherStates.java
- src/main/java/frc/spectrumLib/SpectrumRobot.java
- src/main/java/frc/robot/leds/LedStates.java
- src/main/java/frc/robot/swerve/SwerveStates.java
- src/main/java/frc/robot/swerve/controllers/TranslationYController.java
👮 Files not reviewed due to content moderation or server errors (1)
- advantagescope-custom-assets/Robot_FM/model_1.glb
🛑 Comments failed to post (2)
src/main/java/frc/spectrumLib/telemetry/BatteryLogger.java (1)
113-130: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the trailing-slash bug in parent-key aggregation.
The loop appends the "/" separator immediately after adding a key segment, then merges on the same iteration. For a 3+ segment key, for example
"Drive/FrontLeft/Motor1", the first-level parent key becomes"Drive/"instead of"Drive". This key never merges with a"Drive"key produced by a 2-segment report. Subsystem-level current, power, and energy rollups for that parent become inconsistent.Build each prefix key before merging, and append the separator only when adding the next segment.
🐛 Proposed fix for parent-key aggregation
- String subkey = ""; - for (int i = 0; i < keys.length - 1; i++) { - subkey += keys[i]; - - if (i < keys.length - 2) { - subkey += "/"; - } - - subsystemCurrents.merge(subkey, totalAmps, Double::sum); - subsystemPowers.merge(subkey, power, Double::sum); - subsystemEnergies.merge(subkey, energy, Double::sum); - maxSubsystemCurrents.merge(subkey, totalAmps, Math::max); - } + StringBuilder subkey = new StringBuilder(keys[0]); + subsystemCurrents.merge(subkey.toString(), totalAmps, Double::sum); + subsystemPowers.merge(subkey.toString(), power, Double::sum); + subsystemEnergies.merge(subkey.toString(), energy, Double::sum); + maxSubsystemCurrents.merge(subkey.toString(), totalAmps, Math::max); + for (int i = 1; i < keys.length - 1; i++) { + subkey.append("/").append(keys[i]); + subsystemCurrents.merge(subkey.toString(), totalAmps, Double::sum); + subsystemPowers.merge(subkey.toString(), power, Double::sum); + subsystemEnergies.merge(subkey.toString(), energy, Double::sum); + maxSubsystemCurrents.merge(subkey.toString(), totalAmps, Math::max); + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.String[] keys = key.split("/|-"); if (keys.length < 2) { return; } StringBuilder subkey = new StringBuilder(keys[0]); subsystemCurrents.merge(subkey.toString(), totalAmps, Double::sum); subsystemPowers.merge(subkey.toString(), power, Double::sum); subsystemEnergies.merge(subkey.toString(), energy, Double::sum); maxSubsystemCurrents.merge(subkey.toString(), totalAmps, Math::max); for (int i = 1; i < keys.length - 1; i++) { subkey.append("/").append(keys[i]); subsystemCurrents.merge(subkey.toString(), totalAmps, Double::sum); subsystemPowers.merge(subkey.toString(), power, Double::sum); subsystemEnergies.merge(subkey.toString(), energy, Double::sum); maxSubsystemCurrents.merge(subkey.toString(), totalAmps, Math::max); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/spectrumLib/telemetry/BatteryLogger.java` around lines 113 - 130, Update the parent-key construction loop in BatteryLogger’s aggregation logic so each prefix is merged without a trailing separator. Build and merge the current prefix first, then append "/" only before adding the next segment, ensuring keys such as "Drive" remain consistent across multi-segment and two-segment reports.src/main/java/frc/spectrumLib/telemetry/Telemetry.java (1)
67-99: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -eu printf '%s\n' '--- Telemetry.java ---' sed -n '55,110p' src/main/java/frc/spectrumLib/telemetry/Telemetry.java printf '%s\n' '--- DogLog references and dependency declarations ---' rg -n -i --glob '!build/**' --glob '!node_modules/**' 'DogLogOptions|withCaptureDs|captureDs|doglog|dog-log' .Repository: Spectrum3847/2026-Spectrum
Length of output: 2047
🌐 Web query:
DogLogOptions withCaptureDs captureDs DogLog Java💡 Result:
The
captureDsoption in the DogLog Java library is a configuration setting used to control whether Driver Station (DS) data—such as the robot's enabled state, autonomous mode status, and joystick inputs—is included in the log files [1][2]. Key details regardingcaptureDs: Configuration: It is configured through theDogLogOptionsclass using the.withCaptureDs(boolean)method [3][2]. To apply this configuration, you must pass aDogLogOptionsobject toDogLog.setOptions()within your robot's initialization (typically in theRobotorRobotContainerconstructor) [1][4][5]. Example usage: DogLog.setOptions(new DogLogOptions.withCaptureDs(true)); [1][4] Important Limitations: 1. Permanence: Due to limitations within the underlying WPILibDataLogManager, oncecaptureDsis enabled, it cannot be disabled during the same execution session [1][2]. 2. NetworkTables: Data captured via this option cannot be published over NetworkTables [1][2]. 3. Default State: By default, driver station capture is disabled [1][5][2]. Ensure you explicitly enable it if you require this data in your logs [4][5].Citations:
- 1: https://doglog.dev/reference/configuring
- 2: jonahsnider/doglog@2024.5.6...2024.5.7
- 3: https://javadoc.doglog.dev/dev/doglog/DogLogOptions.html
- 4: https://doglog.dev/getting-started/usage
- 5: https://context7.com/websites/doglog_dev/llms.txt
🌐 Web query:
github Spectrum3847 2026-Spectrum DogLog version build.gradle💡 Result:
For the 2026 FRC season, Spectrum 3847 maintains their robot code in the GitHub repository Spectrum3847/2026-Spectrum [1]. DogLog is a recognized logging library used by FRC teams, including Spectrum 3847 [2]. In FRC projects, library dependencies such as DogLog are typically managed in the build.gradle file [3][4]. While the specific content of the build.gradle file in the 2026-Spectrum repository may vary as the code is actively developed, standard integration of the DogLog library is typically implemented by adding it as a dependency [4]: dependencies { implementation "com.github.jonahsnider:doglog:2026.x.x" } As of February 2026, the latest release of DogLog is version 2026.5.0 [5]. Teams often include such dependencies within the dependencies block of their build.gradle file [4], often alongside WPILib dependencies [6][4]. For the most accurate and up-to-date configuration, you should consult the build.gradle file directly within the main branch of the Spectrum3847/2026-Spectrum repository [1].
Citations:
- 1: https://github.com/Spectrum3847/2026-Spectrum
- 2: https://doglog.dev/getting-started/faq
- 3: https://context7.com/jonahsnider/doglog/llms.txt
- 4: https://github.com/df1317/2026-rebuilt/blob/818c02383aac42e80a95ceb0f0a612b7f3f32d7c/build.gradle
- 5: https://github.com/jonahsnider/doglog
- 6: https://github.com/Spectrum3847/2025-Spectrum/blob/main/build.gradle
🌐 Web query:
DogLog DogLogOptions withCaptureNt NetworkTables entries Java💡 Result:
In DogLog, the withCaptureNt method was used to control whether all NetworkTables entries were included in the robot logs [1][2]. When enabled, this feature captured all NetworkTables values, which could significantly increase log file size and system overhead [1][2]. By default, NetworkTables capture was disabled in DogLog [1][2]. Please be aware that recent changes to the DogLog library have removed the withCaptureNt and ntPublish options entirely [3]. Current versions of the library manage logging behavior automatically, generally aligning with the standard WPILib DataLogManager behavior [4]. If you are working with a newer version of DogLog, you will no longer find these specific methods in the DogLogOptions class [3].
Citations:
- 1: https://doglog.dev/reference/configuring
- 2: https://context7.com/websites/doglog_dev/llms.txt
- 3: jonahsnider/doglog@28ab236
- 4: jonahsnider/doglog#182
Correct the
start()parameter documentation.Place
captureDsbeforecaptureNtand describecaptureDsas Driver Station data capture, not SmartDashboard capture.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/frc/spectrumLib/telemetry/Telemetry.java` around lines 67 - 99, Update the Javadoc for Telemetry.start so the parameter descriptions match the method signature: document captureDs before captureNt, describe captureDs as capturing Driver Station data, and retain the NetworkTables description for captureNt.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
|
@A2A1x you can ping coderabbit with feedback and it will update its memory with the repositories preferences |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
A2A1x
approved these changes
Aug 4, 2026
Contributor
|
@A2A1x can you rebase this branch to main to make sure there no conflicts with the merge |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/coding-conventions/class-generation.md`:
- Line 11: Update the fenced code block in class-generation.md to include a
valid neutral language identifier, such as text, and preserve Spotless
formatting for the Markdown file.
- Line 18: The Config mutability guidance in the class-generation documentation
conflicts with build-tools guidance. Update the Config description to clarify
that immutable getter-only fields apply to mechanism defaults, while
call-site-assembled *Config inner classes may use `@Setter` for chainable
overrides, matching the actual classes and docs.
In `@docs/coding-conventions/code-style.md`:
- Line 19: Update the enum example near the Classes / Interfaces convention to
replace State.LAUNCH_WITH_SQUEEZE with
SuperStructure.WantedSuperState.LAUNCH_WITH_SQUEEZE, keeping the example
consistent with the new state owner.
In `@docs/dependencies/overview.md`:
- Line 18: Remove the stale PhotonLib dependency entry from README.md, keeping
the dependency listing aligned with the Limelight-based implementation and
ensuring no PhotonLib row remains.
In `@docs/frc-software-basics/applied-to-frc.md`:
- Line 33: Standardize the state-transition method reference across both
documentation pages by replacing the inconsistent handleStateTransitions() usage
with the implementation’s handleStateTransition() identifier, while preserving
the surrounding explanation.
- Around line 22-25: Align the explanation following the AButton.onTrue example
with the actual onTrue lifecycle: describe the command triggering once and the
state-machine transition rather than claiming Trigger.whileTrue() hold behavior.
Alternatively, change the example call to whileTrue() only if the intended
behavior is to run continuously while held.
In `@docs/frc-software-basics/classes-methods-objects.md`:
- Around line 38-48: Update the remaining *States references in this
documentation page to match the public instance-method control model used by
SuperStructure. Remove claims that companion *States files or *States classes
are the static API, unless the text explicitly identifies the specific
mechanisms that still use that pattern.
In `@docs/other-guides/tips.md`:
- Around line 26-32: Update the explanation around the AIM_AT_TARGET example so
it accurately describes the captured flywheel setpoint: wantedRPM is read once
and finalWantedRPM causes the DoubleSupplier to return that fixed value on each
control loop. Do not describe it as following later ShotCalculator changes
unless the lambda is changed to read ShotCalculator dynamically.
In `@docs/tools/build-tools.md`:
- Around line 59-60: Update the JavaDoc link lists in docs/tools/build-tools.md
lines 59-60 and docs/tools/gradle.md line 42 to use the exact external-links
list and wording configured by build.gradle’s setLinks(...), keeping both
documentation locations identical.
In `@docs/tools/leds.md`:
- Around line 5-19: Update the LED documentation and runtime wiring
consistently: inspect the Robot initialization and trigger-binding code around
the Leds subsystem, then either uncomment and enable the Leds construction and
its bindings or revise docs/tools/leds.md to state that the CANdle
implementation is inactive. Ensure the documented “live” status and trigger
behavior match the actual runtime configuration.
In `@docs/tools/logging.md`:
- Around line 52-56: Update the Telemetry.log(...) example to use the returned
decorated command by passing it to a binding method such as onTrue(...) or
returning it from a command factory, rather than discarding it after creation.
In `@docs/tools/vision.md`:
- Line 29: Update the See Also entry in the vision documentation to replace the
outdated autoUpdatePose identifier with Auton.autonPoseUpdate, matching the
current pose-update trigger used elsewhere.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 60158c1d-00f2-4e69-b1ca-d4a103237427
📒 Files selected for processing (32)
build.gradledocs/coding-conventions/class-generation.mddocs/coding-conventions/code-style.mddocs/coding-conventions/commits-pull-requests.mddocs/coding-conventions/documentation-and-comments.mddocs/dependencies/doglog.mddocs/dependencies/overview.mddocs/dependencies/pathplanner.mddocs/dependencies/phoenix6.mddocs/dependencies/photonvision.mddocs/dependencies/wpilib.mddocs/frc-software-basics/applied-to-frc.mddocs/frc-software-basics/arrays.mddocs/frc-software-basics/classes-methods-objects.mddocs/frc-software-basics/logic-operators.mddocs/frc-software-basics/loops.mddocs/index.mddocs/other-guides/2026-season-specific.mddocs/other-guides/photon-guide-to-programming.mddocs/other-guides/tips.mddocs/tools/auton.mddocs/tools/build-tools.mddocs/tools/gradle.mddocs/tools/leds.mddocs/tools/logging.mddocs/tools/phoenix-tuner-x.mddocs/tools/pid-tuning.mddocs/tools/simulation.mddocs/tools/vision.mdsrc/main/java/frc/robot/Robot.javasrc/main/java/frc/robot/subsystems/hood/Hood.javasrc/main/java/frc/robot/subsystems/vision/Vision.java
💤 Files with no reviewable changes (3)
- docs/index.md
- build.gradle
- docs/dependencies/photonvision.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/frc/robot/subsystems/hood/Hood.java
- src/main/java/frc/robot/subsystems/vision/Vision.java
- src/main/java/frc/robot/Robot.java
| Every mechanism in `frc.robot` is three pieces in one folder: | ||
| Every mechanism in `frc.robot.subsystems` is one file holding the subsystem plus its config inner class: | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the fenced block.
markdownlint-cli2 reports MD040 because this fence has no language. Use text or another valid neutral language.
As per coding guidelines: Keep Markdown and .gitignore files formatted by Spotless.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/coding-conventions/class-generation.md` at line 11, Update the fenced
code block in class-generation.md to include a valid neutral language
identifier, such as text, and preserve Spotless formatting for the Markdown
file.
Sources: Coding guidelines, Linters/SAST tools
| ``` | ||
|
|
||
| The subsystem class extends `Mechanism` (from `frc.spectrumLib.mechanism`) and owns the motors and sensors. The `Config` inner class holds every tunable value — gear ratios, current limits, voltages, target poses — annotated with `@Getter`/`@Setter`. Each per-robot config file (`FM2026`, `PM2026`, …) mutates those defaults during construction so a single codebase covers different physical robots. | ||
| The subsystem class extends `Mechanism` (from `frc.spectrumLib.mechanism`) and owns the motors and sensors. The `Config` inner class holds every tunable value — gear ratios, current limits, voltages, target poses — as `@Getter private final` fields (no `@Setter`; the values are constants). Each per-robot config file (`FM2026`, `PM2026`, …) selects which mechanisms exist on that robot with `setAttached(true/false)` and supplies robot-specific calibration like `configEncoderOffsets(...)`, so a single codebase covers different physical robots. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align the Config mutability guidance.
This line says mechanism Config classes are immutable and have no setters. docs/tools/build-tools.md says @Getter and @Setter are used on *Config inner classes for chainable overrides. Narrow that rule to call-site-assembled config objects, or update this section to match the actual classes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/coding-conventions/class-generation.md` at line 18, The Config
mutability guidance in the class-generation documentation conflicts with
build-tools guidance. Update the Config description to clarify that immutable
getter-only fields apply to mechanism defaults, while call-site-assembled
*Config inner classes may use `@Setter` for chainable overrides, matching the
actual classes and docs.
| ## Naming | ||
|
|
||
| * **Classes / Interfaces:** `UpperCamelCase` — `Launcher`, `LauncherConfig`, `RobotStates`. | ||
| * **Classes / Interfaces:** `UpperCamelCase` — `Launcher`, `LauncherConfig`, `SuperStructure`. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the enum example to the new state owner.
This change replaces the old ownership example with SuperStructure, but Line 22 still uses State.LAUNCH_WITH_SQUEEZE. Use SuperStructure.WantedSuperState.LAUNCH_WITH_SQUEEZE to avoid documenting the obsolete API.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/coding-conventions/code-style.md` at line 19, Update the enum example
near the Classes / Interfaces convention to replace State.LAUNCH_WITH_SQUEEZE
with SuperStructure.WantedSuperState.LAUNCH_WITH_SQUEEZE, keeping the example
consistent with the new state owner.
Comment on lines
+22
to
25
| pilot.AButton.onTrue(superStructure.setStateCommand(WantedSuperState.UNJAM)); | ||
| ``` | ||
|
|
||
| The `Trigger.whileTrue()` call handles the "keep doing this while the condition holds" logic. When the trigger goes false, the command ends automatically. No loop, no manual state management. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match the explanation to the onTrue example.
The example uses onTrue, not Trigger.whileTrue(). It does not demonstrate a command that runs while the button is held. Update the explanation to describe the state-machine behavior, or change the example to whileTrue() if that lifecycle is intended.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/frc-software-basics/applied-to-frc.md` around lines 22 - 25, Align the
explanation following the AButton.onTrue example with the actual onTrue
lifecycle: describe the command triggering once and the state-machine transition
rather than claiming Trigger.whileTrue() hold behavior. Alternatively, change
the example call to whileTrue() only if the intended behavior is to run
continuously while held.
Comment on lines
+26
to
+32
| // Launcher.applyStates(), AIM_AT_TARGET case | ||
| double wantedRPM = ShotCalculator.getInstance().getParameters().flywheelSpeed(); | ||
| final double finalWantedRPM = wantedRPM; | ||
| setVelocityTCFOCrpm(() -> finalWantedRPM); | ||
| ``` | ||
|
|
||
| `config::getIdlingRPM` is a method reference — a `DoubleSupplier` — so if `idlingRPM` is changed at runtime (say, from a per-robot config override), the running command picks up the new value. More on this in [Class Generation](../coding-conventions/class-generation.md#methods). | ||
| The `() -> ...` lambda is a `DoubleSupplier` re-read by the control request each loop, so as the shot calculator's flywheel target tracks the live distance, the command follows it instead of freezing the value at scheduling time. More on this in [Class Generation](../coding-conventions/class-generation.md#methods). |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not describe a captured value as live.
Line 27 reads the flywheel speed once. Lines 28-29 capture that value in finalWantedRPM, so the lambda returns the same value on every call. It does not follow later ShotCalculator changes. Update the explanation to describe a captured setpoint, or move the calculator read inside the lambda if live tracking is intended.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/other-guides/tips.md` around lines 26 - 32, Update the explanation
around the AIM_AT_TARGET example so it accurately describes the captured
flywheel setpoint: wantedRPM is read once and finalWantedRPM causes the
DoubleSupplier to return that fixed value on each control loop. Do not describe
it as following later ShotCalculator changes unless the lambda is changed to
read ShotCalculator dynamically.
Comment on lines
+59
to
60
| WPILib, REV, Phoenix v5, Phoenix 6, PathPlanner, DogLog, MapleSim, Java 17 | ||
| ``` |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the JavaDoc link list consistent across both pages.
These paragraphs document different dependency sets. Use the exact build.gradle setLinks(...) list in both locations.
docs/tools/build-tools.md#L59-L60: document the configured external links.docs/tools/gradle.md#L42-L42: use the same list and wording.
📍 Affects 2 files
docs/tools/build-tools.md#L59-L60(this comment)docs/tools/gradle.md#L42-L42
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/tools/build-tools.md` around lines 59 - 60, Update the JavaDoc link
lists in docs/tools/build-tools.md lines 59-60 and docs/tools/gradle.md line 42
to use the exact external-links list and wording configured by build.gradle’s
setLinks(...), keeping both documentation locations identical.
Comment on lines
+5
to
+19
| The robot's LEDs are live: [`Leds`](../../src/main/java/frc/robot/subsystems/leds/Leds.java) (`frc.robot.subsystems.leds`) extends `SpectrumLEDs` and drives a Phoenix 6 CANdle — device ID 1 on the CANivore, a 20-LED RGB external strip. | ||
|
|
||
| ## Library: `SpectrumLEDs` | ||
|
|
||
| [`frc.spectrumLib.leds.SpectrumLEDs`](../../src/main/java/frc/spectrumLib/leds/SpectrumLEDs.java) is our wrapper around WPILib's addressable-LED stack. It implements `SpectrumSubsystem`, owns: | ||
| [`frc.spectrumLib.leds.SpectrumLEDs`](../../src/main/java/frc/spectrumLib/leds/SpectrumLEDs.java) is our wrapper around a **Phoenix 6 `CANdle`** (not the WPILib `AddressableLED` stack). It `implements Subsystem` and owns: | ||
|
|
||
| * An `AddressableLED` (PWM port, set by `Config.port`). | ||
| * An `AddressableLEDBuffer` of fixed length. | ||
| * An `AddressableLEDBufferView` — a windowed slice of the buffer so multiple `SpectrumLEDs` instances can drive different sections of the same physical strip independently. | ||
| * A `CANdle` (device ID + CAN bus, set by `Config`). | ||
| * A `CANdleConfiguration` for strip type, brightness, and loss-of-signal behavior. | ||
| * An animation slot the CANdle uses for hardware animations (strobe, fade, rainbow, …). | ||
|
|
||
| The constructor takes a `Config` either by buffer size (in which case it allocates its own LED + buffer and is the "main view") or by sharing an existing buffer with a start/end index (a sub-view that doesn't own the hardware). Only the main view actually pushes data to the strip in `periodic()`, so creating sub-views is free. | ||
| The constructor takes a `Config` either by device id + LED count (it owns the CANdle) or by sharing an existing `CANdle` with a start index and count (a sub-view that addresses a slice of the same physical strip without owning the hardware). | ||
|
|
||
| ## Patterns | ||
|
|
||
| `SpectrumLEDs` ships with pattern factories that return WPILib `LEDPattern` objects: | ||
| `SpectrumLEDs` ships with pattern factories that return `CANdlePattern` objects (some backed by hardware CANdle animations, some by per-LED color writes): |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align the LED documentation with runtime wiring.
The supplied src/main/java/frc/robot/Robot.java context at Lines 110-206 still comments out leds = new Leds();. If that is the current wiring, the LEDs are not live and the documented trigger bindings cannot run. Either enable the subsystem and bindings or state that the CANdle implementation is currently inactive.
Also applies to: 50-52
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/tools/leds.md` around lines 5 - 19, Update the LED documentation and
runtime wiring consistently: inspect the Robot initialization and
trigger-binding code around the Leds subsystem, then either uncomment and enable
the Leds construction and its bindings or revise docs/tools/leds.md to state
that the CANdle implementation is inactive. Ensure the documented “live” status
and trigger behavior match the actual runtime configuration.
Comment on lines
+52
to
56
| `Telemetry.log(Command cmd)` returns a decorated command that logs `Commands: Init: <name>` when scheduled and `Commands: End: <name>` when it ends. Wrap a command in it wherever you want its lifecycle in the log: | ||
|
|
||
| ```java | ||
| // LauncherStates.java | ||
| private static Command log(Command cmd) { | ||
| return Telemetry.log(cmd); | ||
| } | ||
| Telemetry.log(superStructure.setStateCommand(WantedSuperState.INTAKE_FUEL)); | ||
| ``` |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Show the decorated command being scheduled.
Telemetry.log(...) returns a command, but the example discards that command. If copied into Robot.configureBindings(), no command is scheduled and no lifecycle is logged. Show the returned command passed to a binding, such as onTrue(...), or returned from a command factory.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/tools/logging.md` around lines 52 - 56, Update the Telemetry.log(...)
example to use the returned decorated command by passing it to a binding method
such as onTrue(...) or returning it from a command factory, rather than
discarding it after creation.
|
|
||
| * While **disabled**, both MT1 and MT2 from the best Limelight are integrated (good for pre-match auto-zeroing). | ||
| * While **enabled** (teleop, auton-launching, or when `RobotStates.autoUpdatePose` is asserted), only MT1 is integrated. | ||
| * While **enabled** (teleop or when `Auton.autonPoseUpdate` is asserted), only MT1 is integrated. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use one identifier for the pose-update trigger.
Line 29 uses Auton.autonPoseUpdate, but the See Also entry at Line 86 still says autoUpdatePose. Update the See Also entry to use the current identifier.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/tools/vision.md` at line 29, Update the See Also entry in the vision
documentation to replace the outdated autoUpdatePose identifier with
Auton.autonPoseUpdate, matching the current pose-update trigger used elsewhere.
A2A1x
approved these changes
Aug 4, 2026
Contributor
|
@coderabbitai why did you add the do not merge label? have you found any real blocking issues @A2A1x has not addressed? |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="Spectrum3847/2026-Spectrum"
pr="129"
echo "=== Current PR labels ==="
gh pr view "$pr" --repo "$repo" --json labels \
--jq '.labels[] | {name, description, color}'
echo "=== Label timeline events ==="
gh api --paginate "repos/$repo/issues/$pr/timeline?per_page=100" \
-H "Accept: application/vnd.github+json" \
--jq '.[] | select(.event == "labeled" or .event == "unlabeled") | {event, created_at, actor: .actor.login, label: .label.name}'
echo "=== CodeRabbit review comments ==="
gh api --paginate "repos/$repo/pulls/$pr/comments?per_page=100" \
--jq '.[] | select(.user.login == "coderabbitai") | {id, path, line, side, created_at, body, html_url}'
echo "=== CodeRabbit reviews ==="
gh api --paginate "repos/$repo/pulls/$pr/reviews?per_page=100" \
--jq '.[] | select(.user.login == "coderabbitai") | {id, state, submitted_at, body, html_url}'Length of output: 1571
I cannot verify a valid blocking reason for that label. GitHub shows no current CodeRabbit review comments or reviews on You removed the label on August 5 at 00:35 UTC. The PR now has only the I am sorry for the incorrect You are interacting with an AI system. |
Project516
approved these changes
Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
put new state machiine in main
Summary by CodeRabbit
New Features
Bug Fixes
Documentation