Sim docs#113
Draft
Project516 wants to merge 72 commits into
Draft
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.
…s; update RobotSim to draw turret circle
…tedRPM for INDEX_MAX state in Spindexer class
…2026-Spectrum into 2026-offseason-bot
…ng; adjust launcher RPM settings
… simulation handling
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR restructures robot control around ChangesRobot platform and control architecture
Simulation and reusable library
Supporting configuration and documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Robot as Robot
participant SuperStructure as SuperStructure
participant RobotSim as RobotSim
participant FuelPhysicsSim as FuelPhysicsSim
participant AdvantageScope as AdvantageScope
Robot->>SuperStructure: setWantedSuperState()
SuperStructure->>RobotSim: expose current state
RobotSim->>FuelPhysicsSim: launchBall() or tick()
FuelPhysicsSim->>AdvantageScope: publish positions, arcs, scores, and stats
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
@coderabbitai resolve merge conflict |
|
✅ Merge conflicts resolved successfully! Resolved 1 conflict file(s). Commit: 126 file operation(s)
View agent analysis |
Resolved conflicts in: - docs/tools/simulation.md (content) Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
Author
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 65
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
docs/tools/simulation.md (1)
76-77: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRetain a MapleSim reference.
The new section relies on MapleSim behavior in Lines 35, 46, and 72, but this change removes the MapleSim dependency link. Keep a current link or add one inline at the first MapleSim reference.
🤖 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/simulation.md` around lines 76 - 77, Update the simulation documentation around the PathPlanner dependency reference to retain a current MapleSim link, placing it inline at the first MapleSim mention if no dedicated dependency link is appropriate. Ensure the existing MapleSim references remain navigable without changing unrelated documentation.src/main/java/frc/robot/subsystems/launcher/Launcher.java (2)
143-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing
WantedState/SystemStatetelemetry.Every sibling subsystem in this PR (
IndexerTower,IntakeExtension,Swerve) logs both keys; the launcher — the one mechanism whose state is hardest to infer from voltage alone — logs neither.📊 Proposed addition
systemState = handleStateTransition(); logBatteryUsage(); applyStates(); + Telemetry.log("Launcher/WantedState", wantedState.toString()); + Telemetry.log("Launcher/SystemState", systemState.toString()); Telemetry.log("Launcher/CurrentCommand", getCurrentCommandName());🤖 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/launcher/Launcher.java` around lines 143 - 154, Update Launcher.periodic() to log both the current WantedState and SystemState telemetry, matching the corresponding keys and value style used by sibling subsystems such as IndexerTower, IntakeExtension, and Swerve. Add these logs alongside the existing launcher telemetry without changing the state-transition or actuator flow.
23-40: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winTwo competing sets of current-limit fields.
LauncherSupplyCurrent(30) andLauncherStatorCurrent(85) are settable but never applied — the constructor configures the motor fromsupplyCurrentLimit(80) /statorCurrentLimit(100). Calling the generated setters at runtime silently does nothing. Remove the unused pair or make the constructor use them.🤖 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/launcher/Launcher.java` around lines 23 - 40, Remove the unused settable LauncherSupplyCurrent and LauncherStatorCurrent fields from Launcher, or consolidate the configuration so the constructor and runtime setters use the same current-limit values; ensure no competing field pair remains and generated setters cannot silently have no effect.src/main/java/frc/spectrumLib/framework/SpectrumState.java (1)
180-189: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
toggle()duplicatessetStateinstead of delegating.The lambda re-implements the three lines of
setState(boolean); any future change tosetState(logging, guards) silently skips the toggle path.♻️ Proposed fix
public Command toggle() { - return Commands.runOnce( - () -> { - value = !value; - alert.set(value); - setCondition(name, value); - }) + return Commands.runOnce(() -> setState(!value)) .ignoringDisable(true) .withName(name + " state: Toggle"); }🤖 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/SpectrumState.java` around lines 180 - 189, Update toggle() to delegate its state change to the existing setState(boolean) method instead of directly mutating value, updating alert, and calling setCondition. Preserve the current command behavior and toggle the state by passing the inverse of the current value to setState.src/main/java/frc/spectrumLib/sim/Circle.java (1)
59-78: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winApply the requested initial color after delegation.
The delegated constructor draws all ligaments with the default black color before Line 77 assigns
this.color; this overload therefore ignores itscolorargument visually.Proposed fix
this(backgroundLines, diameterInches, name, root, mech); this.color = color; + setBackgroundColor(color);🤖 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/Circle.java` around lines 59 - 78, Update the Circle constructor overload that accepts Color8Bit so the requested color is applied to every already-created background ligament after delegating construction; assigning this.color alone is insufficient. Reuse the existing ligament collection and color-application mechanism in Circle, ensuring all radial lines visibly use the provided color immediately.src/main/java/frc/spectrumLib/framework/SpectrumRobot.java (1)
20-34: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGate the watchdog widening to simulation.
SpectrumRobotwidens both loop-overrun watchdogs to 200 ms on everyRobotstartup, so real hardware can miss genuine 20 ms loop overruns. If this is only for sim physics, wrap the reflection block andCommandScheduler.setPeriod(0.20)inif (isSimulation())so the robot keeps the normal timing diagnostics.🤖 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 20 - 34, Update the SpectrumRobot constructor so the watchdog reflection block and CommandScheduler.getInstance().setPeriod(0.20) execute only when isSimulation() is true; preserve the existing warning behavior and leave real-hardware loop timing unchanged.
🤖 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 @.vscode/settings.json:
- Line 62: Update the java.jdt.ls.vmargs setting in the workspace configuration
to replace the tracked -Xmx8G maximum heap with a substantially smaller default,
while preserving the other JVM arguments. Users should still be able to override
the heap size in their personal settings.
- Line 61: Update the java.debug.settings.onBuildFailureProceed setting in the
VS Code configuration to false so debugging does not continue after a failed
build.
In `@docs/tools/simulation.md`:
- Line 44: Update the simulation documentation sentence to name the actual
AdvantageScope 3D Field topics: replace FuelInFlight with Sim/Fuel/InFlight,
identify its type as Translation3d[], and mention Sim/Fuel/LastShotArc as the
other relevant topic.
In `@simgui-window.json`:
- Around line 4-8: Normalize the saved Glass dock layout in simgui-window.json
so all window and dock positions remain within the 1920×974 main-window bounds,
especially the panes positioned beyond the right edge. Adjust the affected Pos
and Size values while preserving the existing dock hierarchy and split
relationships.
In `@src/main/java/frc/rebuilt/FuelPhysicsSim.java`:
- Around line 984-986: Prevent launchBall in FuelPhysicsSim from decrementing
totalIntaked below zero, or introduce a separate held-ball count while
preserving totalIntaked as a monotonic telemetry counter. In RobotSim, clamp the
totalIntaked snapshot with Math.max(0, ...) before dividing by numOfLanes, and
return group1 directly instead of wrapping the single command in
Commands.parallel; apply these changes at FuelPhysicsSim.java:984-986 and
RobotSim.java:240-251.
- Around line 722-737: Update shouldIntake so it only computes and returns
whether the ball is eligible, removing the callback.run side effect. Move
intake.callback invocation into handleIntakePickup, executing it only after
shouldIntake returns true during actual pickup handling.
- Around line 289-292: Extract the shared tower geometry values used by both
static blocks in FuelPhysicsSim into named constants, including blue/red tower X
and Y positions and halfSpacing. Replace the inline literals and duplicate local
declarations in the AABB upright and rung-cylinder setup with those constants so
all tower geometry remains synchronized.
- Around line 1192-1227: Move the per-frame bookkeeping from stepSubtick into
advancePhysics after the subtick loop: updateSleepState, the finite/position
bounds scan, and elevatedSlowCounter handling must each run once per 20 ms frame
rather than once per subtick. Preserve the existing checks, thresholds, and
outOfBounds behavior while removing their execution from the subtick path.
In `@src/main/java/frc/rebuilt/RobotBumpSim.java`:
- Line 294: Update the contactFactor calculation in RobotBumpSim to use the
documented diagonal behavior, yielding approximately 0.71 at 45° instead of
zero; preserve the absolute-value handling and ensure diagonal collisions still
apply ramp deceleration.
- Around line 418-427: Update the ramp-collision correction around the
projected-distance calculation to use signed separation from the ramp rather
than unsigned dist. Preserve the existing normal computation, derive the signed
distance using the normal, and apply only the Z correction through normalZ so
modules below the ramp are pushed out even at contact; keep the non-intersection
check based on the appropriate radius condition.
In `@src/main/java/frc/rebuilt/ShotCalculator.java`:
- Around line 193-206: Remove the offset applications from ShotCalculator’s
calculation flow: stop adding TURRET_ANGLE_OFFSET_DEGREES to turretAngle and
stop applying FLYWHEEL_SPEED_OFFSET to flywheelSpeed. Preserve the raw
map-derived angle and speed so Turret and Launcher remain the sole consumers
applying their respective offsets.
- Around line 120-128: Fix the feed condition in the ShotCalculator feed-target
logic by replacing the tautological OR of negated launch-state checks with a
guard that excludes all three launch states using the appropriate AND semantics.
Preserve the isRobotInFeedZone() requirement while ensuring feed is false for
LAUNCH_WITH_SQUEEZE, LAUNCH_WITHOUT_SQUEEZE, and
LAUNCH_WITH_SQUEEZE_WITH_NO_DELAY.
- Line 217: Update the telemetry key in the ShotCalculator logging statement for
turretAngularVelocityRotPerSec to identify rotations per second rather than
radians per second, while leaving the computed value and formatting unchanged.
- Around line 152-162: Correct the tangential-velocity calculation in the turret
velocity logic by updating turretVelocityX to use the negative omega
contribution of the rotated offset, specifically -omega * (dx*sinθ + dy*cosθ).
Leave turretVelocityY unchanged.
- Line 192: Update the turret-angle calculation in ShotCalculator so the
field-relative target direction is converted to a mechanism-relative angle by
subtracting the robot heading before returning or commanding it. Preserve the
existing compensatedTurretTranslation calculation and ensure the resulting angle
matches Turret.setMMPositionFoc’s expected reference frame.
In `@src/main/java/frc/robot/auton/Auton.java`:
- Around line 108-114: Update the command returned by launch() so interruption
or cancellation also restores WantedSuperState.IDLE and clears autonLaunching,
rather than relying only on the sequence’s trailing commands. Preserve the
normal 2.5-second launch sequence while adding cleanup that runs when the
command is interrupted.
In `@src/main/java/frc/robot/configs/FM2026.java`:
- Around line 19-20: Update mechanism attachment flags consistently: in
src/main/java/frc/robot/configs/FM2026.java lines 19-20, attach turret alongside
spindexer (or explicitly document why FM has no turret); in
src/main/java/frc/robot/configs/PHOTON2026.java line 19, replace the commented
indexerBed attachment with spindexer.setAttached(true) and
turret.setAttached(true).
In `@src/main/java/frc/robot/configs/OM2026.java`:
- Around line 9-10: Update the robot configuration selection in Robot.java so
UNKNOWN roboRIO serials do not use OM2026’s zero encoder offsets: either
introduce and select a distinct simulation-only configuration for SIM while
rejecting UNKNOWN explicitly, or otherwise fail loudly before starting with
UNKNOWN. Keep OM2026 available only for the intended robot hardware.
In `@src/main/java/frc/robot/configs/PM2026.java`:
- Line 20: Remove the commented-out indexerBed attach call from PM2026.java and
apply the same change to XM2026.java; if indexerBed remains an active mechanism,
replace the commented calls in both configs with indexerBed.setAttached(false)
instead.
In `@src/main/java/frc/robot/Robot.java`:
- Around line 484-487: Remove the commented-out simulation reset block near the
autonomous-start logic. If resetting field balls on autonomous start remains
required, restore that behavior using RobotBase.isSimulation() instead of the
removed Utils reference.
- Around line 179-184: Invoke configureSimBindings() within the
RobotBase.isSimulation() branch after creating robotSim, while retaining
configureBindings() for common bindings. Ensure the RobotSim.simLaunching() to
ballSimLaunchFuel() binding becomes active only during simulation.
In `@src/main/java/frc/robot/RobotSim.java`:
- Around line 104-108: Define a named constant for the 9-degree turret
visualization offset alongside SHOOTER_TURRET_PIVOT_POINT, then update the
turretPose3d rotation calculation to use that constant instead of the hard-coded
subtraction. Preserve the existing offset value and behavior.
- Around line 71-76: Update the lane1 initialization in RobotSim to remove the
misleading -2 * laneWidth / 2 expression: use the general lane-position formula
based on the lane index and numOfLanes, or express the single-lane offset
plainly as -laneWidth. Preserve the intended lane positioning behavior.
- Around line 212-219: Update the launch kinematics in the RobotSim
shot-creation block so params.flywheelSpeed() is converted from RPM to exit
velocity using the flywheel surface-speed conversion and efficiency before
constructing Translation3d. Use params.turretAngle() directly as the
field-relative launch yaw, and replace its use as pitch with the configured
hood/launch elevation constant; keep launchRotation and launchVelocity
consistent with these corrected components.
- Around line 122-129: Update drawTurretCircle to pass the top-view dimensions
to topView.getRoot in the same x/y order as drawTopRobot, using topViewWidth for
x and topViewHeight for y. Store the created Circle in a field instead of a
local variable so it is retained for future updates, and remove the
now-unnecessary unused suppression.
- Around line 240-251: Update the deferred command construction in the RobotSim
fuel-launch flow to clamp the value from ballSim.getTotalIntaked() to a
non-negative launch count before the loop builds waits and launches. Return
group1 directly and apply withName("RobotSim.ballSimLaunchFuel") to it instead
of wrapping it with Commands.parallel.
- Around line 186-191: Correct the intake-zone axis mapping in the visible
extent calculations: use the robot length/X extent when computing intakeXMax,
and use the intake width/Y extent for the Y bounds. Update the variable names
and conversions around intakeWidth and intakeLength as needed so the
calculations align with FuelPhysicsSim.configureRobot and handleRobotCollision.
In `@src/main/java/frc/robot/subsystems/fuelIntake/FuelIntake.java`:
- Around line 52-80: Collapse the duplicate WantedState/SystemState enums in
FuelIntake.java (lines 52-80) into one enum, update state handling to use it
directly, and remove handleStateTransition() while transitions remain identity
mappings. Apply the same change to Spindexer.java (lines 46-74) so both
subsystems use a single state enum without transition indirection.
In `@src/main/java/frc/robot/subsystems/intakeExtension/IntakeExtension.java`:
- Around line 199-203: Update setInitialPosition to convert
config.getInitPosition() with percentToRotations instead of degreesToRotations,
matching the subsystem’s other setpoints and maxRotations-based representation
while preserving the existing attachment check.
- Around line 184-232: Remove the unused inSpringyMode field and its
getter/setter from IntakeExtension, remove the corresponding
springyMode*CurrentLimit configuration values, and delete the
IntakeExtension/InSpringyMode telemetry entry in periodic(). Do not add new
state-machine behavior unless wiring SLOW_CLOSE to this flag is required by
existing design.
- Around line 205-215: Update resetCurrentPositionToMax() to call
motor.setPosition(config.getMaxRotations()) only when isAttached() is true,
matching setInitialPosition(). Change resetCurrentPositionToMaxCommand() and
resetToInitialPos() from run(...) to runOnce(...) so both reset commands execute
once and finish.
- Around line 157-182: Update applyStates() to use the existing config.fullOut,
config.home, and config.squeeze values instead of hardcoded 100, 0, and 25
setpoints. Replace the slow-close profile literals 4.0, 20.0, and 1000.0 with
appropriately named configuration constants, adding them to config if they do
not already exist.
In `@src/main/java/frc/robot/subsystems/launcher/Launcher.java`:
- Around line 103-121: Remove the obsolete `@SuppressWarnings`("unused")
annotations from handleStateTransition() and applyStates(), since periodic()
invokes both methods. In applyStates(), replace the hardcoded idle RPM value of
700 with config.getIdlingRPM() so the configured setter value controls IDLE_PREP
behavior.
- Around line 123-126: The ShotCalculator preference offsets are applied twice
by the launcher and turret consumers. In
src/main/java/frc/robot/subsystems/launcher/Launcher.java lines 123-126, update
the LAUNCH handling to assign params.flywheelSpeed() directly; in
src/main/java/frc/robot/subsystems/turret/Turret.java lines 143-152, use
params.turretAngle().getDegrees() directly. Remove both raw offset additions
while preserving the existing parameter retrieval and control flow.
In `@src/main/java/frc/robot/subsystems/leds/Leds.java`:
- Around line 27-42: Update the static initialization of ledsConfig to set the
documented startIdx offset of 8, so NUM_LEDS addresses the external LED strip at
device indices 8–27; keep the existing LED configuration settings unchanged and
ensure the Javadoc matches the resulting behavior.
In `@src/main/java/frc/robot/subsystems/spindexer/Spindexer.java`:
- Around line 76-95: Replace the guessed RPM setpoints in Spindexer.applyStates
with calibrated, hardware-validated values for INDEX_MAX, SLOW_INDEX, and UNJAM
before enabling them on the built robot; remove the placeholder TODO once the
values are sourced from characterization or an approved configuration.
- Around line 19-23: Make the Spindexer configuration fields supplyCurrentLimit,
statorCurrentLimit, velocityKp, velocityKv, and velocityKs final and remove
their setters, matching the immutable configuration pattern used by
FuelIntakeConfig. Preserve the existing constructor-based application of these
values to the Talon configuration.
In `@src/main/java/frc/robot/subsystems/SuperStructure.java`:
- Around line 275-285: Remove the duplicated
spindexer.setWantedState(Spindexer.WantedState.OFF) call in forceHome(), leaving
a single OFF state transition unless the surrounding subsystem initialization
identifies a missing intended call.
- Around line 106-122: Update handleStateTransitions() so Util.disabled forces
CurrentSuperState.AUTON_IDLE before evaluating wantedSuperState, while
preserving the existing autonomous-IDLE behavior and normal wantedSuperState
mappings when enabled. Ensure disabled mode cannot pass through launcher,
turret, or drive commands from any non-IDLE wanted state.
In `@src/main/java/frc/robot/subsystems/swerve/Swerve.java`:
- Around line 577-584: Update the overloaded isAtDesiredRotation(double
toleranceRadians) method to require systemState ==
SystemState.PILOT_AIM_AT_TARGET before evaluating
HeadingController.getPositionError(). Return false in all other states, while
preserving the existing tolerance comparison during pilot aim.
- Around line 258-277: Update applyStates() so the IDLE case issues a neutral
drivetrain control request via setControl, clearing any previously commanded
translational and rotational speeds. Preserve the existing behavior for
PILOT_AIM_AT_TARGET and TELEOP_DRIVE.
- Around line 211-230: Synchronize the bump-simulation state access in the
periodic() simulation block with the simNotifier thread that runs
mapleSimSwerveDrivetrain::update and SimulatedArena.simulationPeriodic().
Introduce or reuse a common simLock, wrapping the reads of simulated pose and
chassis speeds, robotBumpSim.update, and setSimulationWorldPose in synchronized
(simLock); ensure the notifier callback uses the same lock around its simulation
step.
- Around line 413-446: Update the zone-bound constants near NEUTRAL_ZONE and
ENEMY_ALLIANCE_ZONE to use Field.fieldLength instead of the hard-coded
FIELD_LENGTH_METERS value, while preserving the existing field-width handling
and zone geometry. Ensure isInEnemyAllianceZone remains aligned with
FieldHelpers.flipXifRed’s field-length mirror.
- Around line 263-270: Update the PILOT_AIM_AT_TARGET branch in Swerve.java so
DRIVE_AT_ANGLE_REQUEST.withTargetDirection uses a chassis heading computed from
the robot pose toward the target, rather than params.turretAngle(). Keep the
turret shot-angle calculation separate and preserve the existing joystick
velocity inputs.
In `@src/main/java/frc/robot/subsystems/swerve/SwerveConfig.java`:
- Around line 47-51: Update the comments above linearSpeedAt12Volts and
angularSpeedAt12Volts to describe their actual units: meters per second for
translational speed and degrees per second for rotational speed, without
changing the field types or values.
- Around line 102-106: Remove the redundant withSupplyCurrentLowerLimit call
from the current-limit configuration in SwerveConfig, since it duplicates
withSupplyCurrentLimit. Leave the 40A supply limit and its enable setting
unchanged.
In `@src/main/java/frc/robot/subsystems/turret/Turret.java`:
- Around line 155-162: Remove the unused TrapezoidProfile scaffolding from
Turret, including profile, turretSetpoint, and turretRequest, along with the
TurretConfig.turretConstraints construction and their assignments in the turret
initialization/update flow. Preserve the existing Talon Motion Magic control
through setMMPositionFoc.
- Around line 21-23: Reorder the imports in Turret.java so
frc.rebuilt.ShotCalculator appears before the Lombok import, matching the
project's formatter and allowing spotlessCheck to pass.
In `@src/main/java/frc/robot/subsystems/vision/Vision.java`:
- Around line 544-548: Enforce VisionConfig.kMaxTimeDeltaSeconds for both MT1
and MT2 pose-estimate paths before returning or submitting measurements to the
pose estimator, comparing each getMegaTagXPoseTimestamp() against the current
FPGA time and rejecting stale measurements. Apply the same age-gating behavior
at both estimate sites, or remove the unused configuration field if no gating is
intended.
- Around line 705-714: Update the Javadoc for Vision.tagsInView() to state that
an unknown alliance falls back to Blue, matching the existing
DriverStation.getAlliance().orElse(DriverStation.Alliance.Blue) behavior; leave
the implementation unchanged.
- Around line 876-877: Update the VisionFieldPoseEstimate nested class
declaration to be static, preserving its immutable data-holder behavior and
existing Lombok-generated accessors; do not change how callers construct or
consume estimates.
- Around line 448-451: Update the heading-discard condition in Vision’s rotation
handling to use the absolute value of robotSpeed.omegaRadiansPerSecond, matching
the Math.abs-based check in rejectionCheck(), so fast rotation in either
direction sets degStds to 50.
- Around line 794-796: Update resetPoseToVision() so the poseTimestamp passed to
addVisionMeasurement is converted with Utils.fpgaToCurrentTime(...), matching
the timestamp handling in the other estimate pipelines. Keep the existing
integratedPose and measurement uncertainty behavior unchanged.
In `@src/main/java/frc/spectrumLib/hardware/Rio.java`:
- Around line 27-28: Update the Rio enum serial registration and checkID() flow
to prevent blank or null serials from colliding in the IDs map: populate IDs
only for valid serial values, and make checkID() explicitly return SIM whenever
RobotBase.isReal() is false. Preserve real-hardware lookup so FM_2026 and
OM_2026 resolve to their own enum values and remain reachable by Robot().
In `@src/main/java/frc/spectrumLib/hardware/SpectrumServo.java`:
- Around line 10-19: Update the SpectrumServo constructor to register the
subsystem with the scheduler after initializing the Servo superclass. Ensure
every SpectrumServo instance invokes the inherited Subsystem registration
behavior so periodic methods and default commands are scheduler-managed.
In `@src/main/java/frc/spectrumLib/leds/SpectrumLEDs.java`:
- Around line 692-716: Update the edges method to handle clampedLen <= 0 before
constructing edge SolidColor ranges. In that case, configure a single SolidColor
covering the entire segment from startIdx through startIdx + numLeds - 1 with
black, then preserve the existing edge and optional-center behavior for positive
lengths.
- Around line 404-419: Document in the CANdlePattern javadoc that pattern
instances are stateful, capture the first applyTo segment and enclosing
animation slot, and must not be reused across SpectrumLEDs instances or
different segments; alternatively, update the lazy holder in blink and other
pattern factories to cache animations per incoming startIdx/numLeds and slot.
- Around line 661-680: Update the software patterns gradient, ombre, wave,
countdown, and switchCountdown to reduce CAN traffic by grouping contiguous LEDs
with identical colors into range-based SolidColor controls instead of calling
setControl once per LED on each loop. Preserve each pattern’s visual output, and
consider a hardware animation where equivalent.
- Around line 261-292: Honor config.isAttached() in SpectrumLEDs: only
create/configure the CANdle when the strip is attached, and prevent the default
command or pattern execution from calling candle.setControl(...) when it is
absent. Update the relevant default-command/write path while preserving
isAttached() as the exposed status check.
In `@src/main/java/frc/spectrumLib/README.md`:
- Line 17: Update src/main/java/frc/spectrumLib/README.md lines 17-17 to
describe leds as the CANdle pattern library, and update lines 99-106 to document
the current CANdle API and CTRE animation behavior instead of the removed
AddressableLED and buffer-view details.
In `@src/main/java/frc/spectrumLib/sim/ArmSim.java`:
- Line 50: Update the TurretSim construction/configuration to disable gravity
explicitly rather than using the default true value. Keep ArmSim’s
config.isSimulateGravity() forwarding unchanged, and ensure the TurretSim path
passes false to its SingleJointedArmSim setup.
In `@src/main/java/frc/spectrumLib/swerve/SysID.java`:
- Around line 20-21: Remove the commented-out swerve field from SysID and delete
the corresponding dead assignment in its constructor; keep using the constructor
parameter directly.
- Line 9: Remove the robot-specific Swerve dependency from SysID and change its
API to accept a reusable CTRE SwerveDrivetrain/Subsystem handle, or a minimal
interface exposing setControl(SwerveRequest). Update SysID’s construction and
control calls to use that abstraction while preserving its existing behavior.
In `@src/main/java/frc/spectrumLib/vision/Limelight.java`:
- Around line 175-179: Update the Limelight constructor to enforce one camera
identity: validate that the supplied name matches config.getName(), or derive
cameraName directly from config.getName() instead of storing an independent
value. Ensure pose methods and target-space methods consistently query the same
configured Limelight.
- Around line 402-407: Update getRawFiducial() to check isAttached() before
querying LimelightHelpers.getBotPoseEstimate_wpiBlue. Return an empty
RawFiducial array immediately when the camera is detached, while preserving the
existing null-estimate handling for attached cameras.
---
Outside diff comments:
In `@docs/tools/simulation.md`:
- Around line 76-77: Update the simulation documentation around the PathPlanner
dependency reference to retain a current MapleSim link, placing it inline at the
first MapleSim mention if no dedicated dependency link is appropriate. Ensure
the existing MapleSim references remain navigable without changing unrelated
documentation.
In `@src/main/java/frc/robot/subsystems/launcher/Launcher.java`:
- Around line 143-154: Update Launcher.periodic() to log both the current
WantedState and SystemState telemetry, matching the corresponding keys and value
style used by sibling subsystems such as IndexerTower, IntakeExtension, and
Swerve. Add these logs alongside the existing launcher telemetry without
changing the state-transition or actuator flow.
- Around line 23-40: Remove the unused settable LauncherSupplyCurrent and
LauncherStatorCurrent fields from Launcher, or consolidate the configuration so
the constructor and runtime setters use the same current-limit values; ensure no
competing field pair remains and generated setters cannot silently have no
effect.
In `@src/main/java/frc/spectrumLib/framework/SpectrumRobot.java`:
- Around line 20-34: Update the SpectrumRobot constructor so the watchdog
reflection block and CommandScheduler.getInstance().setPeriod(0.20) execute only
when isSimulation() is true; preserve the existing warning behavior and leave
real-hardware loop timing unchanged.
In `@src/main/java/frc/spectrumLib/framework/SpectrumState.java`:
- Around line 180-189: Update toggle() to delegate its state change to the
existing setState(boolean) method instead of directly mutating value, updating
alert, and calling setCondition. Preserve the current command behavior and
toggle the state by passing the inverse of the current value to setState.
In `@src/main/java/frc/spectrumLib/sim/Circle.java`:
- Around line 59-78: Update the Circle constructor overload that accepts
Color8Bit so the requested color is applied to every already-created background
ligament after delegating construction; assigning this.color alone is
insufficient. Reuse the existing ligament collection and color-application
mechanism in Circle, ensuring all radial lines visibly use the provided color
immediately.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 63327ba1-62cd-40f6-b61d-939f6d56c5ac
📒 Files selected for processing (115)
.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.glbdocs/tools/simulation.mdsimgui-ds.jsonsimgui-window.jsonsrc/main/deploy/pathplanner/autos/TBTB Full.autosrc/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/configs/FM2026.javasrc/main/java/frc/robot/configs/OM2026.javasrc/main/java/frc/robot/configs/PHOTON2026.javasrc/main/java/frc/robot/configs/PM2026.javasrc/main/java/frc/robot/configs/XM2026.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/IntakeExtensionStates.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/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/spindexer/Spindexer.javasrc/main/java/frc/robot/subsystems/swerve/Swerve.javasrc/main/java/frc/robot/subsystems/swerve/SwerveConfig.javasrc/main/java/frc/robot/subsystems/turret/Turret.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/README.mdsrc/main/java/frc/spectrumLib/SpectrumCANcoderConfig.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 (38)
- src/main/java/frc/rebuilt/Zones.java
- src/main/java/frc/robot/swerve/controllers/TranslationXController.java
- src/main/java/frc/spectrumLib/SpectrumSubsystem.java
- src/main/java/frc/robot/swerve/controllers/TagDistanceAlignController.java
- src/main/java/frc/rebuilt/offsets/HomeOffsets.java
- src/main/java/edu/wpi/first/wpilibj2/command/button/README.md
- src/main/java/frc/rebuilt/launchingMaps/AndyMarkMap.java
- src/main/java/frc/rebuilt/launchingMaps/HomeMap.java
- src/main/java/frc/robot/swerve/controllers/TranslationYController.java
- src/main/java/frc/robot/swerve/controllers/RotationController.java
- src/main/java/frc/robot/vision/VisionStates.java
- src/main/java/frc/spectrumLib/SpectrumServo.java
- src/main/java/frc/spectrumLib/TuneValue.java
- src/main/java/frc/robot/swerve/SwerveStates.java
- src/main/java/frc/robot/leds/CANdleLeds.java
- src/main/java/frc/robot/swerve/controllers/TagCenterAlignController.java
- src/main/java/frc/robot/State.java
- src/main/java/frc/robot/indexerTower/IndexerTowerStates.java
- src/main/java/frc/spectrumLib/SpectrumCANcoderConfig.java
- src/main/java/frc/robot/RobotStates.java
- src/main/java/frc/robot/operator/OperatorStates.java
- src/main/java/frc/robot/fuelIntake/FuelIntakeStates.java
- src/main/java/frc/robot/indexerTower/IndexerTower.java
- src/main/java/edu/wpi/first/wpilibj2/command/button/Trigger.java
- src/main/java/frc/robot/pilot/PilotStates.java
- src/main/java/frc/robot/vision/VisionSystem.java
- src/main/java/frc/robot/hood/Hood.java
- src/main/java/frc/robot/hood/HoodStates.java
- src/main/java/frc/robot/intakeExtension/IntakeExtensionStates.java
- src/main/java/frc/robot/launcher/LauncherStates.java
- src/main/java/frc/robot/indexerBed/IndexerBedStates.java
- src/main/java/frc/rebuilt/FieldHelpers.java
- vendordeps/photonlib.json
- src/main/java/frc/robot/vision/Vision.java
- src/main/java/frc/rebuilt/TagProperties.java
- src/main/java/frc/robot/indexerBed/IndexerBed.java
- src/main/java/frc/robot/leds/LedStates.java
- src/main/java/frc/robot/Coordinator.java
👮 Files not reviewed due to content moderation or server errors (1)
- advantagescope-custom-assets/Robot_FM/model_1.glb
| ], | ||
| "java.compile.nullAnalysis.mode": "automatic", | ||
| "java.debug.settings.onBuildFailureProceed": true | ||
| "java.debug.settings.onBuildFailureProceed": true, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== .vscode/settings.json (around line 61) ==\n'
nl -ba .vscode/settings.json | sed -n '45,75p'
printf '\n== Search for related Java debug/build settings ==\n'
rg -n '"java\.debug\.settings\.onBuildFailureProceed"|"java\.jdt\.ls\.vmargs"|"java\.debug\.settings\.' .vscode settings.json . || trueRepository: Spectrum3847/2026-Spectrum
Length of output: 250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== .vscode/settings.json (around line 61) ==\n'
awk 'NR>=45 && NR<=75 { printf "%4d %s\n", NR, $0 }' .vscode/settings.json
printf '\n== Search for related Java debug/build settings ==\n'
rg -n '"java\.debug\.settings\.onBuildFailureProceed"|"java\.jdt\.ls\.vmargs"|"java\.debug\.settings\.' .vscode . || trueRepository: Spectrum3847/2026-Spectrum
Length of output: 1257
🌐 Web query:
Java debug settings onBuildFailureProceed last successful artifact after build failure documentation
💡 Result:
The setting java.debug.settings.onBuildFailureProceed is a configuration option specifically for the Visual Studio Code Debugger for Java extension [1][2]. It is unrelated to retrieving artifacts after a build failure [3]. ### Understanding java.debug.settings.onBuildFailureProceed This setting controls whether the VS Code Java debugger automatically continues and launches the program even if the workspace build (compilation) fails [1]. * When set to true: The debugger suppresses the "Build failed, do you want to continue?" prompt and proceeds with the launch using the previously compiled (and potentially outdated) classes [1][3]. * When set to false (default): The debugger displays a warning prompt when a build failure occurs, allowing the user to choose whether to continue, always continue (which updates the setting to true), or fix the errors [1][4][3]. This setting does not manage or retrieve build artifacts; it solely dictates the debugger's behavior in the presence of compiler errors within the IDE [3]. ### Managing Artifacts After Build Failures If you are looking to retrieve artifacts from a "last successful build" after a failure, this functionality is typically found in CI/CD platforms rather than local Java debugger settings [5][6][7]. * Jenkins: The Copy Artifact plugin allows you to select artifacts from the "Last successful build" of a project [6]. Pipeline scripts can use selectors like lastSuccessful() to target these builds [8][9]. * TeamCity: Artifact dependencies can be configured to import artifacts from the "Latest successful build" specifically [7]. If you are encountering build failures in a local VS Code environment, the java.debug.settings.onBuildFailureProceed setting will not help you access a previous successful build; it will only force the debugger to run the current, broken state of your code [3]. If you need a stable version, you should revert your source code to the last known good commit using your version control system (e.g., Git) [1][4].
Citations:
- 1: https://github.com/microsoft/vscode-java-debug/blob/main/Configuration.md
- 2: https://marketplace.visualstudio.com/items?itemName=vscjava.vscode-java-debug
- 3: https://github.com/microsoft/vscode-java-debug/blob/9304faaf/src/build.ts
- 4: https://stackoverflow.com/questions/79522813/restore-build-failed-warning
- 5: https://stackoverflow.com/questions/42829710/jenkins-if-a-build-fails-deploy-the-artifacts-of-the-last-successful-build
- 6: https://plugins.jenkins.io/copyartifact
- 7: https://www.jetbrains.com/help/teamcity/artifact-dependencies.html
- 8: [JENKINS-71231] lastSuccessful does not get the last successful artifact jenkinsci/copyartifact-plugin#559
- 9: [JENKINS-20370] CopyArtifact - get last artifact from self jenkinsci/copyartifact-plugin#398
Do not proceed after a failed build
java.debug.settings.onBuildFailureProceed should be false here; when enabled, the debugger skips the build-failed prompt and can launch with stale classes, which makes validation results misleading.
Suggested setting
- "java.debug.settings.onBuildFailureProceed": true,
+ "java.debug.settings.onBuildFailureProceed": false,📝 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.
Suggested change
| "java.debug.settings.onBuildFailureProceed": true, | |
| "java.debug.settings.onBuildFailureProceed": false, |
🤖 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 @.vscode/settings.json at line 61, Update the
java.debug.settings.onBuildFailureProceed setting in the VS Code configuration
to false so debugging does not continue after a failed build.
| "java.compile.nullAnalysis.mode": "automatic", | ||
| "java.debug.settings.onBuildFailureProceed": true | ||
| "java.debug.settings.onBuildFailureProceed": true, | ||
| "java.jdt.ls.vmargs": "-XX:+UseParallelGC -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90 -Dsun.zip.disableMemoryMapping=true -Xmx8G -Xms100m -Xlog:disable" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'FILE: .vscode/settings.json\n'
wc -l .vscode/settings.json
printf '\n--- lines around 62 ---\n'
sed -n '50,75p' .vscode/settings.json
printf '\n--- occurrences of java.jdt.ls.vmargs ---\n'
rg -n '"java\.jdt\.ls\.vmargs"' -S .
printf '\n--- workspace docs mentioning VS Code / Java settings ---\n'
rg -n 'VS Code|vscode|java\.jdt\.ls\.vmargs|JDT LS|Gradle|Glass' -S README* .github .vscode docs . 2>/dev/null | sed -n '1,200p'Repository: Spectrum3847/2026-Spectrum
Length of output: 805
Lower the tracked JDT LS heap. .vscode/settings.json sets java.jdt.ls.vmargs to -Xmx8G, which every contributor inherits; on smaller machines it can consume a large share of RAM. Use a smaller workspace default and let users raise it in their own settings if needed.
🤖 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 @.vscode/settings.json at line 62, Update the java.jdt.ls.vmargs setting in
the workspace configuration to replace the tracked -Xmx8G maximum heap with a
substantially smaller default, while preserving the other JVM arguments. Users
should still be able to override the heap size in their personal settings.
| 4. If an auto is selected, drag `SmartDashboard/Field2d/Auto Routine` in as a trajectory/poses object so the planned path and simulated robot motion can be compared. | ||
|
|
||
| You can also visualize flying fuel projectiles: `RobotSim.createFuelProjectile()` publishes them to NetworkTables under `Sim/Fuel/Positions`, and you can drag those entries into the 3D field to watch shots in flight. | ||
| You can also drive things like FuelInFlight and whatnot to simulate flying fuel. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the referenced documentation and any mentions of FuelInFlight / AdvantageScope / NetworkTables.
git ls-files | rg '(^|/)docs/tools/simulation\.md$|FuelInFlight|AdvantageScope|NetworkTables|simulation'Repository: Spectrum3847/2026-Spectrum
Length of output: 189
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the documentation file around the referenced line.
file="docs/tools/simulation.md"
wc -l "$file"
cat -n "$file" | sed -n '1,120p'Repository: Spectrum3847/2026-Spectrum
Length of output: 6845
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for the specific term and nearby implementation details.
rg -n -C 3 'FuelInFlight|flying fuel|AdvantageScope|NetworkTables|Network Table|NT' .Repository: Spectrum3847/2026-Spectrum
Length of output: 50385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read only the relevant documentation and search for implementation references.
printf '\n=== docs/tools/simulation.md ===\n'
wc -l docs/tools/simulation.md
cat -n docs/tools/simulation.md | sed -n '1,120p'
printf '\n=== FuelInFlight references ===\n'
rg -n -C 4 'FuelInFlight|flying fuel|fuel in flight|AdvantageScope|NetworkTables|Network Table|NT' . || trueRepository: Spectrum3847/2026-Spectrum
Length of output: 50385
🌐 Web query:
"FuelInFlight AdvantageScope NetworkTables path pose format simulation"
💡 Result:
Based on current technical documentation, there is no tool or standard feature named "FuelInFlight" related to AdvantageScope, NetworkTables, or robot simulation [1][2][3]. AdvantageScope is a widely used FRC data visualization and log analysis tool that supports NetworkTables (NT4), WPILOG files, and Driver Station logs [1][3][4]. For visualizing poses (such as robot odometry or trajectories) in AdvantageScope, the standard approach involves publishing structured data to NetworkTables [5][6]. Key aspects of this process include: 1. Data Format: AdvantageScope supports standard WPILib struct and protobuf serialization [5]. Developers typically use the StructPublisher class in WPILib to publish Pose2d or Pose3d objects to a specific NetworkTables path [5][6]. 2. Path Usage: You define a NetworkTables path (e.g., /MyPose) and associate it with the appropriate struct (e.g., Pose2d.struct) [5][6]. 3. Simulation Integration: AdvantageScope can connect directly to a robot simulator or read.wpilog files generated during simulation [1][3][4]. Simulation frameworks (like AdvantageKit) can automatically generate these logs, which AdvantageScope then renders for 2D or 3D visualization [7][2][4]. If "FuelInFlight" is a custom project, library, or specific simulation component, it does not appear in standard FRC or AdvantageScope documentation. You may wish to check the specific documentation for your proprietary codebase or simulation environment.
Citations:
- 1: https://docs.wpilib.org/es/stable/docs/software/dashboards/advantagescope.html
- 2: https://docs.advantagescope.org/overview/live-sources/
- 3: https://docs.wpilib.org/en/stable/docs/software/dashboards/advantagescope.html
- 4: https://www.chiefdelphi.com/t/advantagescope-2023-beta-live-visualization-log-analysis-with-wpilog-nt4/417424
- 5: https://www.chiefdelphi.com/t/advantagescope-2024-log-analysis-robot-telemetry-continued/442658/12
- 6: https://context7.com/mechanical-advantage/advantagescope/llms.txt
- 7: https://docs.advantagekit.org/javadoc/org/littletonrobotics/junction/wpilog/WPILOGWriter.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the simulation code that publishes the fuel/flying-ball data.
ast-grep outline src/main/java/frc/robot/RobotSim.java --view expanded || true
printf '\n=== RobotSim fuel-related slice ===\n'
cat -n src/main/java/frc/robot/RobotSim.java | sed -n '1,260p'
printf '\n=== fuel / ball physics references ===\n'
rg -n -C 3 'FuelPhysicsSim|ballSim|getTotalIntaked|publish.*NT|NetworkTables|FuelInFlight|Sim/Fuel' src/main/java/frc/robot src/main/java/frc/spectrumLibRepository: Spectrum3847/2026-Spectrum
Length of output: 33657
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the fuel physics simulation class and any published topic names.
fuel_file=$(git ls-files | rg 'FuelPhysicsSim\.java$' | head -n 1)
printf 'Fuel file: %s\n' "$fuel_file"
wc -l "$fuel_file"
cat -n "$fuel_file" | sed -n '1,260p'
printf '\n=== references to topic/path strings in fuel simulation ===\n'
rg -n '"[^"]*(Fuel|fuel|Ball|ball|Sim/)"|Telemetry\.log|StructPublisher|Field2d|NetworkTables' "$fuel_file" src/main/java/frc/robot/Robot.java src/main/java/frc/robot/RobotSim.java || trueRepository: Spectrum3847/2026-Spectrum
Length of output: 17597
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Identify any simulation-time publications for the ball/fuel visualization.
rg -n -C 4 'Telemetry\.log\("Sim/Fuel"|Sim/Fuel|createSimBallLaunch|FuelPhysicsSim|getBallSim|getTotalIntaked|NetworkTables|StructPublisher|Field2d|Pose2d|Pose3d' src/main/java/frc/robot src/main/java/frc/spectrumLibRepository: Spectrum3847/2026-Spectrum
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the publishing helpers in FuelPhysicsSim around the NT API and the tick method.
file="src/main/java/frc/rebuilt/FuelPhysicsSim.java"
sed -n '760,860p' "$file"
printf '\n=== tick / publishing section ===\n'
sed -n '2260,2325p' "$file"Repository: Spectrum3847/2026-Spectrum
Length of output: 6717
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for the exact topic names and data types used for the ball visualization.
rg -n -C 3 'StructArrayPublisher|DoublePublisher|IntegerPublisher|publish|create.*Publisher|tableKey|balls|Pose3d|Pose2d|trajectory|FuelInFlight' src/main/java/frc/rebuilt/FuelPhysicsSim.javaRepository: Spectrum3847/2026-Spectrum
Length of output: 28420
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
text = Path("src/main/java/frc/rebuilt/FuelPhysicsSim.java").read_text()
for needle in [
"StructArrayPublisher",
"tableKey",
"Pose3d",
"Pose2d",
"launchBall(",
"publish",
"Sim/Fuel",
"ballPos",
"ballPositions",
]:
idx = text.find(needle)
print(f"--- {needle} ---")
if idx == -1:
print("not found")
else:
start = max(0, idx-400)
end = min(len(text), idx+1200)
print(text[start:end])
print()
PYRepository: Spectrum3847/2026-Spectrum
Length of output: 9973
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If the NT publisher names are difficult to spot in the full file, extract the exact methods around the publishing section.
file="src/main/java/frc/rebuilt/FuelPhysicsSim.java"
awk 'NR>=779 && NR<=860 {print NR ":" $0}' "$file"
printf '\n=== around 2288 ===\n'
awk 'NR>=2280 && NR<=2335 {print NR ":" $0}' "$file"Repository: Spectrum3847/2026-Spectrum
Length of output: 6202
Name the actual sim topics. Replace FuelInFlight with Sim/Fuel/InFlight (Translation3d[]) in AdvantageScope’s 3D Field flow; Sim/Fuel/LastShotArc is the other relevant topic here.
🤖 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/simulation.md` at line 44, Update the simulation documentation
sentence to name the actual AdvantageScope 3D Field topics: replace FuelInFlight
with Sim/Fuel/InFlight, identify its type as Translation3d[], and mention
Sim/Fuel/LastShotArc as the other relevant topic.
Comment on lines
+4
to
+8
| "DockNode ID=0x00000001 Pos=4,942 Size=983,285 Selected=0x48913727", | ||
| "DockNode ID=0x00000003 Pos=983,443 Size=877,514 Split=X", | ||
| "DockNode ID=0x00000004 Parent=0x00000003 SizeRef=238,497 Selected=0xFA2A8FFA", | ||
| "DockNode ID=0x00000006 Parent=0x00000003 SizeRef=235,497 Selected=0xA6A62DF3", | ||
| "DockNode ID=0x00000005 Pos=9,29 Size=2242,790 Split=X", |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="simgui-window.json"
echo "== File info =="
wc -l "$file"
echo
echo "== Relevant lines =="
cat -n "$file" | sed -n '1,120p'
echo
echo "== Coordinate check =="
python3 - <<'PY'
import json, re, sys, pathlib
path = pathlib.Path("simgui-window.json")
text = path.read_text()
# Extract all quoted strings and look for common ImGui layout fields.
strings = re.findall(r'"([^"]*)"', text)
nums = []
for s in strings:
if any(k in s for k in ("Pos=", "Size=", "SizeRef=", "MainWindow", "DockNode")):
nums.append(s)
print("Matched strings:", len(nums))
for s in nums:
print(s)
# Parse simple numeric extents where possible
def parse_kv(s):
out = {}
for key in ("Pos", "Size", "SizeRef"):
m = re.search(rf'{key}=(-?\d+),(-?\d+)', s)
if m:
out[key] = (int(m.group(1)), int(m.group(2)))
return out
xs, ys = [], []
for s in strings:
kv = parse_kv(s)
if "Pos" in kv:
x, y = kv["Pos"]
xs.append(x); ys.append(y)
if "Size" in kv:
w, h = kv["Size"]
# only for strings that also have Pos
if "Pos" in kv:
x, y = kv["Pos"]
xs.append(x + w); ys.append(y + h)
print()
if xs and ys:
print("Observed x range:", min(xs), "to", max(xs))
print("Observed y range:", min(ys), "to", max(ys))
PYRepository: Spectrum3847/2026-Spectrum
Length of output: 6360
Keep the Glass layout within the window bounds. simgui-window.json stores dock/window positions past the 1920×974 main window (for example, the Alerts and Scheduler panes sit beyond the right edge), so this layout can reopen off-screen on a single-monitor setup. Normalize the saved coordinates before committing.
🤖 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 `@simgui-window.json` around lines 4 - 8, Normalize the saved Glass dock layout
in simgui-window.json so all window and dock positions remain within the
1920×974 main-window bounds, especially the panes positioned beyond the right
edge. Adjust the affected Pos and Size values while preserving the existing dock
hierarchy and split relationships.
Comment on lines
+193
to
+206
| turretAngle = turretAngle.plus(Rotation2d.fromDegrees(TURRET_ANGLE_OFFSET_DEGREES)); | ||
|
|
||
| // Drive angular velocity (rad/s) for your position controller feedforward | ||
| if (lastDriveAngle == null) lastDriveAngle = driveAngle; | ||
| // Turret angular velocity (rot/s) for your position controller feedforward | ||
| if (lastTurretAngle == null) lastTurretAngle = turretAngle; | ||
| double deltaRot = | ||
| MathUtil.inputModulus(driveAngle.minus(lastDriveAngle).getRotations(), -0.5, 0.5); | ||
|
|
||
| double rawDriveOmega = deltaRot / loopPeriodSecs; | ||
| double driveAngularVelocity = driveAngleFilter.calculate(rawDriveOmega); | ||
| lastDriveAngle = driveAngle; | ||
|
|
||
| // Hood angle from map with offset | ||
| double hoodAngle = hoodAngleMap.get(lookaheadDistance); | ||
| if (Double.isNaN(lastHoodAngle)) lastHoodAngle = hoodAngle; | ||
| double hoodVelocity = | ||
| hoodAngleFilter.calculate((hoodAngle - lastHoodAngle) / loopPeriodSecs); | ||
| lastHoodAngle = hoodAngle; | ||
| MathUtil.inputModulus(turretAngle.minus(lastTurretAngle).getRotations(), -0.5, 0.5); | ||
|
|
||
| // Apply hood angle offset | ||
| hoodAngle += HOOD_ANGLE_OFFSET; | ||
| double rawOmega = deltaRot / loopPeriodSecs; | ||
| double turretAngularVelocityRotPerSec = turretOmegaFilter.calculate(rawOmega); | ||
| lastTurretAngle = turretAngle; | ||
|
|
||
| // Flywheel from map + preference offset (%) | ||
| double flywheelSpeed = launcherSpeedMap.get(lookaheadDistance); | ||
| double flywheelSpeed = shotFlywheelSpeedMap.get(lookaheadDistance); | ||
| flywheelSpeed += flywheelSpeed * (FLYWHEEL_SPEED_OFFSET / 100.0); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Tuning offsets are applied twice — once here, once at the consumers.
TURRET_ANGLE_OFFSET_DEGREES is added into turretAngle here (line 193), but per the relevant_code_snippets, Turret.java adds ShotCalculator.TURRET_ANGLE_OFFSET_DEGREES again on top of params.turretAngle().getDegrees() — doubling the same degree offset. Similarly, FLYWHEEL_SPEED_OFFSET is applied here as a percentage multiplier (line 206), but Launcher.java adds the raw FLYWHEEL_SPEED_OFFSET value again as an RPM increment on top of params.flywheelSpeed() — compounding the offset in two different units/semantics. Either remove the internal application here (leaving the offset entirely to consumers) or remove the redundant re-application in Launcher.java/Turret.java, but not both.
🤖 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` around lines 193 - 206, Remove
the offset applications from ShotCalculator’s calculation flow: stop adding
TURRET_ANGLE_OFFSET_DEGREES to turretAngle and stop applying
FLYWHEEL_SPEED_OFFSET to flywheelSpeed. Preserve the raw map-derived angle and
speed so Turret and Launcher remain the sole consumers applying their respective
offsets.
Comment on lines
+794
to
+796
| Robot.getSwerve() | ||
| .addVisionMeasurement( | ||
| integratedPose, poseTimestamp, VecBuilder.fill(0.00001, 0.00001, 0.00001)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Pose-reset measurement uses a raw FPGA timestamp instead of the Phoenix time base.
Both estimate pipelines convert with Utils.fpgaToCurrentTime(...) before calling addVisionMeasurement (Lines 461 and 546), but here poseTimestamp — sourced straight from ll.getMegaTag1PoseTimestamp() in resetPoseToVision() (Line 744) — is passed through unconverted. Phoenix's pose estimator interprets it on its own clock, so the reset is applied with an arbitrary latency offset and can be discarded or mis-interpolated.
⏱️ Proposed fix
Robot.getSwerve()
.addVisionMeasurement(
- integratedPose, poseTimestamp, VecBuilder.fill(0.00001, 0.00001, 0.00001));
+ integratedPose,
+ Utils.fpgaToCurrentTime(poseTimestamp),
+ VecBuilder.fill(0.00001, 0.00001, 0.00001));📝 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.
Suggested change
| Robot.getSwerve() | |
| .addVisionMeasurement( | |
| integratedPose, poseTimestamp, VecBuilder.fill(0.00001, 0.00001, 0.00001)); | |
| Robot.getSwerve() | |
| .addVisionMeasurement( | |
| integratedPose, | |
| Utils.fpgaToCurrentTime(poseTimestamp), | |
| VecBuilder.fill(0.00001, 0.00001, 0.00001)); |
🤖 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 794 -
796, Update resetPoseToVision() so the poseTimestamp passed to
addVisionMeasurement is converted with Utils.fpgaToCurrentTime(...), matching
the timestamp handling in the other estimate pipelines. Keep the existing
integratedPose and measurement uncertainty behavior unchanged.
Comment on lines
+876
to
+877
| @Getter | ||
| public class VisionFieldPoseEstimate { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Make VisionFieldPoseEstimate a static nested class.
It's a pure immutable data holder but is declared as an inner class, so every estimate (one or two per loop) captures an implicit Vision.this reference for no reason. A record would be even tighter.
♻️ Proposed refactor
- `@Getter`
- public class VisionFieldPoseEstimate {
+ `@Getter`
+ public static class VisionFieldPoseEstimate {📝 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.
Suggested change
| @Getter | |
| public class VisionFieldPoseEstimate { | |
| `@Getter` | |
| public static class VisionFieldPoseEstimate { |
🤖 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 876 -
877, Update the VisionFieldPoseEstimate nested class declaration to be static,
preserving its immutable data-holder behavior and existing Lombok-generated
accessors; do not change how callers construct or consume estimates.
| ├── util/ General-purpose utilities and math helpers | ||
| ├── mechanism/ Abstract TalonFX-driven mechanism base class | ||
| ├── gamepads/ Xbox controller abstraction with deadbanding and rumble | ||
| ├── leds/ AddressableLED wrapper with built-in pattern library |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Migrate LED documentation from AddressableLED to CANdle. The library now uses CANdle patterns and CTRE animations, but both sections document the removed AddressableLED model.
src/main/java/frc/spectrumLib/README.md#L17-L17: describeledsas the CANdle pattern library.src/main/java/frc/spectrumLib/README.md#L99-L106: replace AddressableLED and buffer-view details with the current CANdle API and behavior.
📍 Affects 1 file
src/main/java/frc/spectrumLib/README.md#L17-L17(this comment)src/main/java/frc/spectrumLib/README.md#L99-L106
🤖 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` at line 17, Update
src/main/java/frc/spectrumLib/README.md lines 17-17 to describe leds as the
CANdle pattern library, and update lines 99-106 to document the current CANdle
API and CTRE animation behavior instead of the removed AddressableLED and
buffer-view details.
Comment on lines
175
to
179
| public Limelight(String name, int pipeline, LimelightConfig config) { | ||
| this(name); | ||
| cameraName = name; | ||
| this.config = config; | ||
| setLimelightPipeline(pipeline); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use one camera identity consistently.
Pose methods query config.getName(), while target-space methods query cameraName. This constructor stores them independently, so mismatched inputs silently combine pose data from one Limelight with aiming data from another. Derive cameraName from config, or reject unequal names.
🤖 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/vision/Limelight.java` around lines 175 - 179,
Update the Limelight constructor to enforce one camera identity: validate that
the supplied name matches config.getName(), or derive cameraName directly from
config.getName() instead of storing an independent value. Ensure pose methods
and target-space methods consistently query the same configured Limelight.
Comment on lines
402
to
+407
| public RawFiducial[] getRawFiducial() { | ||
| return LimelightHelpers.getBotPoseEstimate_wpiBlue(config.name).rawFiducials; | ||
| PoseEstimate est = LimelightHelpers.getBotPoseEstimate_wpiBlue(config.name); | ||
| if (est == null) { | ||
| return new RawFiducial[0]; | ||
| } | ||
| return est.rawFiducials; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Honor the detached-camera empty-array contract.
getRawFiducial() bypasses isAttached(), unlike the neighboring pose methods. A detached camera can therefore query stale NetworkTables data instead of returning the documented empty array.
Proposed fix
public RawFiducial[] getRawFiducial() {
+ if (!isAttached()) {
+ return new RawFiducial[0];
+ }
PoseEstimate est = LimelightHelpers.getBotPoseEstimate_wpiBlue(config.name);📝 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.
Suggested change
| public RawFiducial[] getRawFiducial() { | |
| return LimelightHelpers.getBotPoseEstimate_wpiBlue(config.name).rawFiducials; | |
| PoseEstimate est = LimelightHelpers.getBotPoseEstimate_wpiBlue(config.name); | |
| if (est == null) { | |
| return new RawFiducial[0]; | |
| } | |
| return est.rawFiducials; | |
| public RawFiducial[] getRawFiducial() { | |
| if (!isAttached()) { | |
| return new RawFiducial[0]; | |
| } | |
| PoseEstimate est = LimelightHelpers.getBotPoseEstimate_wpiBlue(config.name); | |
| if (est == null) { | |
| return new RawFiducial[0]; | |
| } | |
| return est.rawFiducials; | |
| } |
🤖 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/vision/Limelight.java` around lines 402 - 407,
Update getRawFiducial() to check isAttached() before querying
LimelightHelpers.getBotPoseEstimate_wpiBlue. Return an empty RawFiducial array
immediately when the camera is detached, while preserving the existing
null-estimate handling for attached cameras.
Contributor
Author
|
@coderabbitai rate limit |
|
Your plan includes PR reviews subject to rate limits. Reviews are available now. |
Comment on lines
+289
to
+292
| double blueTowerX = 1.067; | ||
| double redTowerX = 15.494; | ||
| double blueTowerY = 4.039; | ||
| double redTowerY = 4.318; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Extract the duplicated tower geometry literals into constants.
blueTowerY = 4.039 / redTowerY = 4.318 (and halfSpacing) are re-declared verbatim in the second static block (Lines 420-422), and tower X positions are inline literals. If one copy is retuned the AABB uprights and the rung cylinders silently disagree.
♻️ Suggested extraction
+ private static final double BLUE_TOWER_X = 1.067;
+ private static final double RED_TOWER_X = 15.494;
+ private static final double BLUE_TOWER_Y = 4.039;
+ private static final double RED_TOWER_Y = 4.318;
+ private static final double TOWER_HALF_SPACING = TOWER_UPRIGHT_SPACING / 2.0;🤖 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 289 - 292,
Extract the shared tower geometry values used by both static blocks in
FuelPhysicsSim into named constants, including blue/red tower X and Y positions
and halfSpacing. Replace the inline literals and duplicate local declarations in
the AABB upright and rung-cylinder setup with those constants so all tower
geometry remains synchronized.
Comment on lines
+722
to
+737
| boolean shouldIntake(SimBall ball, Pose2d robotPose, double bumperHeight) { | ||
| if (!active.getAsBoolean() || ball.pos.getZ() > bumperHeight) return false; | ||
| Translation2d relPos = | ||
| new Pose2d(ball.pos.toTranslation2d(), Rotation2d.kZero) | ||
| .relativeTo(robotPose) | ||
| .getTranslation(); | ||
| boolean inside = | ||
| relPos.getX() >= xMin | ||
| && relPos.getX() <= xMax | ||
| && relPos.getY() >= yMin | ||
| && relPos.getY() <= yMax; | ||
| if (inside) { | ||
| callback.run(); | ||
| } | ||
| return inside; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
shouldIntake mutates state (runs callback) from a predicate.
A should* query firing the pickup callback is surprising and will misfire if a future caller ever probes without consuming. Prefer returning the decision and letting handleIntakePickup invoke intake.callback.
🤖 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 722 - 737, Update
shouldIntake so it only computes and returns whether the ball is eligible,
removing the callback.run side effect. Move intake.callback invocation into
handleIntakePickup, executing it only after shouldIntake returns true during
actual pickup handling.
Comment on lines
+984
to
+986
| totalLaunched++; | ||
| totalIntaked--; | ||
| lastLaunchSpeed = vel.getNorm(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
totalIntaked is used as both a lifetime counter and the live hopper count, and it can go negative. launchBall decrements it unconditionally while handleIntakePickup compares it against hopperSize and RobotSim reads it to decide how many balls to fire; once it drifts below zero the hopper gate and the launch count are both wrong.
src/main/java/frc/rebuilt/FuelPhysicsSim.java#L984-L986: stop decrementing below zero (or introduce a separateheldBallsfield, keepingtotalIntakedmonotonic for telemetry).src/main/java/frc/robot/RobotSim.java#L240-L251: clamp the snapshot withMath.max(0, ballSim.getTotalIntaked())before dividing bynumOfLanes, and returngroup1directly instead of wrapping a single command inCommands.parallel.
📍 Affects 2 files
src/main/java/frc/rebuilt/FuelPhysicsSim.java#L984-L986(this comment)src/main/java/frc/robot/RobotSim.java#L240-L251
🤖 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 984 - 986,
Prevent launchBall in FuelPhysicsSim from decrementing totalIntaked below zero,
or introduce a separate held-ball count while preserving totalIntaked as a
monotonic telemetry counter. In RobotSim, clamp the totalIntaked snapshot with
Math.max(0, ...) before dividing by numOfLanes, and return group1 directly
instead of wrapping the single command in Commands.parallel; apply these changes
at FuelPhysicsSim.java:984-986 and RobotSim.java:240-251.
Comment on lines
+1192
to
+1227
| if (config.sleepingEnabled) { | ||
| for (int i = 0; i < balls.size(); i++) { | ||
| updateSleepState(balls.get(i)); | ||
| } | ||
| } | ||
|
|
||
| // Out-of-bounds cleanup (includes NaN guard, upper Z limit, and stuck-on-obstacle removal) | ||
| for (int i = 0; i < balls.size(); i++) { | ||
| SimBall ball = balls.get(i); | ||
| if (!Double.isFinite(ball.pos.getX()) | ||
| || !Double.isFinite(ball.pos.getY()) | ||
| || !Double.isFinite(ball.pos.getZ()) | ||
| || !Double.isFinite(ball.vel.getX()) | ||
| || !Double.isFinite(ball.vel.getY()) | ||
| || !Double.isFinite(ball.vel.getZ()) | ||
| || !Double.isFinite(ball.omega.getX()) | ||
| || !Double.isFinite(ball.omega.getY()) | ||
| || !Double.isFinite(ball.omega.getZ()) | ||
| || ball.pos.getX() < -2.0 | ||
| || ball.pos.getX() > FIELD_LENGTH + 2.0 | ||
| || ball.pos.getY() < -2.0 | ||
| || ball.pos.getY() > FIELD_WIDTH + 2.0 | ||
| || ball.pos.getZ() < -1.0 | ||
| || ball.pos.getZ() > 15.0) { | ||
| ball.outOfBounds = true; | ||
| } | ||
| // Remove balls stuck on elevated obstacles | ||
| if (ball.pos.getZ() > BALL_RADIUS + 0.3 && ball.vel.getNorm() < 0.5) { | ||
| ball.elevatedSlowCounter++; | ||
| if (ball.elevatedSlowCounter > 250) { | ||
| ball.outOfBounds = true; | ||
| } | ||
| } else { | ||
| ball.elevatedSlowCounter = 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Per-frame bookkeeping is running per subtick (5× per 20 ms tick).
updateSleepState, the NaN/out-of-bounds scan, and elevatedSlowCounter all run inside stepSubtick, so sleepFrameThreshold = 10 ("consecutive frames") actually settles after 2 robot ticks, and the full-list finite/bounds scan over every ball is executed 5× per tick for no benefit. Move these passes into advancePhysics after the subtick loop.
♻️ Suggested move
for (int tick = 0; tick < ticks; tick++) {
stepSubtick(subDt);
}
+
+ if (config.sleepingEnabled) {
+ for (int i = 0; i < balls.size(); i++) {
+ updateSleepState(balls.get(i));
+ }
+ }
+ cullInvalidBalls(); // NaN / OOB / stuck-elevated pass, extracted from stepSubtick
// Remove flagged balls
removeFlaggedBalls();🤖 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 1192 - 1227, Move
the per-frame bookkeeping from stepSubtick into advancePhysics after the subtick
loop: updateSleepState, the finite/position bounds scan, and elevatedSlowCounter
handling must each run once per 20 ms frame rather than once per subtick.
Preserve the existing checks, thresholds, and outOfBounds behavior while
removing their execution from the subtick path.
| // yaw = 0° -> |cos(0)| = 1.0 -> full deceleration (straight-on, hardest to cross) | ||
| // yaw = 45° -> |cos(90°)| = 0.71 -> reduced deceleration (diagonal, easier) | ||
| // yaw = 90° -> |cos(180°)| = 1.0 -> full deceleration (sideways approach also hard) | ||
| double contactFactor = Math.abs(Math.cos(2 * robotPose2d.getRotation().getRadians())); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix the diagonal contact factor.
At 45°, abs(cos(2θ)) is 0, not the documented 0.71; this eliminates ramp deceleration entirely and permits diagonal ghosting.
Proposed fix
- double contactFactor = Math.abs(Math.cos(2 * robotPose2d.getRotation().getRadians()));
+ double yaw = robotPose2d.getRotation().getRadians();
+ double contactFactor = Math.max(Math.abs(Math.cos(yaw)), Math.abs(Math.sin(yaw)));📝 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.
Suggested change
| double contactFactor = Math.abs(Math.cos(2 * robotPose2d.getRotation().getRadians())); | |
| double yaw = robotPose2d.getRotation().getRadians(); | |
| double contactFactor = Math.max(Math.abs(Math.cos(yaw)), Math.abs(Math.sin(yaw))); |
🤖 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/RobotBumpSim.java` at line 294, Update the
contactFactor calculation in RobotBumpSim to use the documented diagonal
behavior, yielding approximately 0.71 at 45° instead of zero; preserve the
absolute-value handling and ensure diagonal collisions still apply ramp
deceleration.
Comment on lines
+661
to
680
| public CANdlePattern gradient(Color color1, Color color2) { | ||
| SolidColor[][] holder = new SolidColor[1][]; | ||
| return (candle, startIdx, numLeds) -> { | ||
| if (holder[0] == null) { | ||
| holder[0] = new SolidColor[numLeds]; | ||
| for (int i = 0; i < numLeds; i++) { | ||
| double ratio = (numLeds <= 1) ? 0.0 : (double) i / (numLeds - 1); | ||
| int r = (int) (color1.red * 255 * (1 - ratio) + color2.red * 255 * ratio); | ||
| int g = (int) (color1.green * 255 * (1 - ratio) + color2.green * 255 * ratio); | ||
| int b = (int) (color1.blue * 255 * (1 - ratio) + color2.blue * 255 * ratio); | ||
| holder[0][i] = | ||
| new SolidColor(startIdx + i, startIdx + i) | ||
| .withColor(new RGBWColor(r, g, b, 0)); | ||
| } | ||
| } | ||
| for (SolidColor req : holder[0]) { | ||
| candle.setControl(req); | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find how many LEDs are configured for SpectrumLEDs instances in this repo
rg -nP --type=java -C4 'new SpectrumLEDs\.Config|new SpectrumLEDs\('Repository: Spectrum3847/2026-Spectrum
Length of output: 164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map SpectrumLEDs and inspect the relevant methods.
FILE="src/main/java/frc/spectrumLib/leds/SpectrumLEDs.java"
wc -l "$FILE"
ast-grep outline "$FILE" --view expanded | sed -n '1,240p'
echo '--- relevant ranges ---'
sed -n '620,780p' "$FILE"Repository: Spectrum3847/2026-Spectrum
Length of output: 10054
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for SpectrumLEDs construction and pattern usages.
rg -n --type=java 'new SpectrumLEDs|SpectrumLEDs\.' src/main/java src/test/java 2>/dev/null || trueRepository: Spectrum3847/2026-Spectrum
Length of output: 164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/main/java/frc/spectrumLib/leds/SpectrumLEDs.java"
echo '--- class and helper overview ---'
ast-grep outline "$FILE" --view expanded | sed -n '1,260p'
echo '--- surrounding pattern methods ---'
sed -n '560,780p' "$FILE"
echo '--- any docs/comments mentioning one-shot or loop resend ---'
rg -n -C2 'one-shot|resent each loop|setControl|SolidColor' "$FILE"Repository: Spectrum3847/2026-Spectrum
Length of output: 25267
Reduce per-LED CAN traffic in software patterns
gradient, ombre, wave, countdown, and switchCountdown send one setControl(...) request per LED on every loop. That scales linearly with strip length and can consume a lot of CAN bandwidth; collapse contiguous runs where possible or move the effect to a hardware animation.
🤖 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/leds/SpectrumLEDs.java` around lines 661 - 680,
Update the software patterns gradient, ombre, wave, countdown, and
switchCountdown to reduce CAN traffic by grouping contiguous LEDs with identical
colors into range-based SolidColor controls instead of calling setControl once
per LED on each loop. Preserve each pattern’s visual output, and consider a
hardware animation where equivalent.
Comment on lines
+692
to
716
| public CANdlePattern edges(Color color, int length) { | ||
| RGBWColor rgbw = toRGBW(color); | ||
| SolidColor[][] holder = new SolidColor[1][]; | ||
| return (candle, startIdx, numLeds) -> { | ||
| if (holder[0] == null) { | ||
| int clampedLen = Math.min(length, numLeds / 2); | ||
| int centerStart = startIdx + clampedLen; | ||
| int centerEnd = startIdx + numLeds - clampedLen - 1; | ||
| // left edge, right edge, (optional) center black | ||
| holder[0] = (centerStart <= centerEnd) ? new SolidColor[3] : new SolidColor[2]; | ||
| holder[0][0] = new SolidColor(startIdx, startIdx + clampedLen - 1).withColor(rgbw); | ||
| holder[0][1] = | ||
| new SolidColor(startIdx + numLeds - clampedLen, startIdx + numLeds - 1) | ||
| .withColor(rgbw); | ||
| if (holder[0].length == 3) { | ||
| holder[0][2] = | ||
| new SolidColor(centerStart, centerEnd) | ||
| .withColor(new RGBWColor(0, 0, 0, 0)); | ||
| } | ||
| } | ||
| for (SolidColor req : holder[0]) { | ||
| candle.setControl(req); | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
edges produces an inverted SolidColor range for tiny segments.
clampedLen = Math.min(length, numLeds / 2) is 0 when numLeds < 2 (or when length == 0), making Line 702 construct new SolidColor(startIdx, startIdx - 1) — an end index below the start. Guard for clampedLen <= 0 and fall back to filling the whole segment black.
🤖 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/leds/SpectrumLEDs.java` around lines 692 - 716,
Update the edges method to handle clampedLen <= 0 before constructing edge
SolidColor ranges. In that case, configure a single SolidColor covering the
entire segment from startIdx through startIdx + numLeds - 1 with black, then
preserve the existing edge and optional-center behavior for positive lengths.
| config.getMinAngle(), | ||
| config.getMaxAngle(), | ||
| false, // Simulate gravity (change back to true) | ||
| config.isSimulateGravity(), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
ast-grep outline src/main/java/frc/robot/subsystems/turret/Turret.java --items all
rg -n -C 5 'new ArmConfig|setSimulatedGravity' src/main/java/frc/robot/subsystems/turret/Turret.java
rg -n -C 3 'boolean simulateGravity|setSimulatedGravity' src/main/java/frc/spectrumLib/sim/ArmConfig.javaRepository: Spectrum3847/2026-Spectrum
Length of output: 2526
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
ast-grep outline src/main/java/frc/spectrumLib/sim/ArmSim.java --items all
sed -n '1,220p' src/main/java/frc/spectrumLib/sim/ArmSim.java
sed -n '220,290p' src/main/java/frc/robot/subsystems/turret/Turret.javaRepository: Spectrum3847/2026-Spectrum
Length of output: 7963
Disable gravity for TurretSim ArmSim now forwards simulateGravity into SingleJointedArmSim, and TurretSim still uses the default true. A turret yaw axis should not be gravity-loaded, so this will make the sim bias toward settling at an angle/limit.
🤖 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/ArmSim.java` at line 50, Update the
TurretSim construction/configuration to disable gravity explicitly rather than
using the default true value. Keep ArmSim’s config.isSimulateGravity()
forwarding unchanged, and ensure the TurretSim path passes false to its
SingleJointedArmSim setup.
| import edu.wpi.first.wpilibj2.command.Command; | ||
| import edu.wpi.first.wpilibj2.command.sysid.SysIdRoutine; | ||
| import frc.robot.swerve.Swerve; | ||
| import frc.robot.subsystems.swerve.Swerve; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
frc.spectrumLib now depends on frc.robot — the dependency direction is inverted.
This PR moves SpectrumLib into structured, reusable packages, but SysID imports the robot-specific frc.robot.subsystems.swerve.Swerve, so the library can no longer be lifted into another project. The class only needs setControl(SwerveRequest) and a Subsystem handle; accept a CTRE SwerveDrivetrain/Subsystem (or a small interface) instead.
🤖 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/swerve/SysID.java` at line 9, Remove the
robot-specific Swerve dependency from SysID and change its API to accept a
reusable CTRE SwerveDrivetrain/Subsystem handle, or a minimal interface exposing
setControl(SwerveRequest). Update SysID’s construction and control calls to use
that abstraction while preserving its existing behavior.
Comment on lines
20
to
21
| public class SysID { | ||
| // private Swerve swerve; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Drop the commented-out swerve field and assignment.
Lines 21 and 55 are dead code; the constructor parameter is used directly.
🤖 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/swerve/SysID.java` around lines 20 - 21, Remove
the commented-out swerve field from SysID and delete the corresponding dead
assignment in its constructor; keep using the constructor parameter directly.
Contributor
Author
|
@JediMasterLucy55 you want to work on this? |
Contributor
|
sure i prbly cant until after drip |
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
4 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.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation