Port migration-branch features into main architecture - #52
Port migration-branch features into main architecture#52YehudaRothstein wants to merge 1 commit into
Conversation
Cherry-picks selected features from feature/robot-migration into the existing frc.excalib library without adopting excalib2 or changing mechanism/swerve concepts. Telemetry (frc.excalib.telemetry, DogLog-based, runs alongside Monologue): - Telemetry: DogLog + Phoenix SignalLogger + PDH logging lifecycle - LoopTimer: loop/max/overrun timing - FaultReporter: per-TalonFX fault + RIO CAN health alerts (replaces CANHealthMonitor) - TunableNumber / TunableGains: live /Tuning values, off by default; wired as an example into the Shooter hood PID StateMachine (frc.excalib.statemachine): typed transition table with guards + DogLog. Vision: LimelightMegaTag2 client wired into Swerve.periodic() (MT2 replaces MegaTag1), distance/tag-count-scaled std-devs; turret conversion + isFast gating preserved. Motor library (TalonFXMotor): CurrentBudget record + applyCurrentBudget, configureFusedCANcoder API (not live-wired), current-budget config. Phoenix CAN optimization: PhoenixSignalHub runs optimizeBusUtilizationForAll on all motors + swerve/shooter CANcoders once at startup (optimizeAll was previously dead code); fault + encoder-position signals protected first. refreshAll moved to top of robotPeriodic. Pigeon left full-rate deliberately. Also: Shooter DogLog keys, Test-mode bring-up harness in RobotContainer, DogLog vendordep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds Phoenix motor and signal management, MegaTag2 vision fusion, a guarded enum state machine, runtime telemetry and tuning utilities, and robot lifecycle/test-control integrations. ChangesMotor control and Phoenix signal management
MegaTag2 vision fusion
Enum state machine
Runtime telemetry and bring-up
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Swerve
participant LimelightMegaTag2
participant Odometry
Swerve->>LimelightMegaTag2: Set robot orientation
Swerve->>LimelightMegaTag2: Read latest estimate
LimelightMegaTag2-->>Swerve: Return pose, timestamp, and tag data
Swerve->>Odometry: Add converted pose with standard deviations
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Ports selected feature/robot-migration utilities into the existing frc.excalib architecture by adding DogLog-based telemetry, a reusable typed state machine, MegaTag2 Limelight vision fusion for swerve, and Phoenix 6 CAN signal optimization hooks.
Changes:
- Added DogLog vendordep + telemetry utilities (Telemetry lifecycle, loop timing, fault reporting, tunable gains).
- Replaced swerve vision fusion path with a minimal NT-only MegaTag2 client and per-measurement vision std-dev weighting.
- Added Phoenix 6 CAN status-signal optimization hub and extended motor wrapper APIs (current budgeting + fused CANcoder config), plus a Test-mode bring-up harness.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vendordeps/DogLog.json | Adds DogLog vendordep for DogLog-based telemetry. |
| src/main/java/frc/robot/subsystems/shooter/Shooter.java | Adds DogLog telemetry + live-tunable hood PID gains; registers CANcoders for optimization. |
| src/main/java/frc/robot/RobotContainer.java | Adds Test-mode-only bring-up bindings and exposes PDH for telemetry init. |
| src/main/java/frc/robot/Robot.java | Initializes telemetry + CAN optimization; moves CAN refresh earlier; adds loop timing + fault polling. |
| src/main/java/frc/robot/Constants.java | Registers swerve absolute CANcoders with PhoenixSignalHub for optimization. |
| src/main/java/frc/excalib/telemetry/TunableNumber.java | Introduces tunable NT-backed numeric constants (opt-in). |
| src/main/java/frc/excalib/telemetry/TunableGains.java | Wraps Gains as a bundle of TunableNumbers with poll/apply helper. |
| src/main/java/frc/excalib/telemetry/Telemetry.java | Central DogLog + Phoenix SignalLogger + PDH logging lifecycle. |
| src/main/java/frc/excalib/telemetry/LoopTimer.java | Adds per-loop timing + overrun telemetry to DogLog. |
| src/main/java/frc/excalib/telemetry/FaultReporter.java | Adds periodic TalonFX fault scanning + RIO CAN health telemetry/alerts. |
| src/main/java/frc/excalib/swerve/vision/LimelightMegaTag2.java | Adds minimal NT-only MegaTag2 Limelight client with std-dev scaling. |
| src/main/java/frc/excalib/swerve/Swerve.java | Switches vision fusion to MegaTag2 pipeline and uses std-dev-weighted measurements. |
| src/main/java/frc/excalib/statemachine/StateMachine.java | Adds typed transition-table state machine with guard/pending semantics and DogLog telemetry. |
| src/main/java/frc/excalib/slam/mapper/Odometry.java | Adds pose-estimator vision measurement overload accepting std devs. |
| src/main/java/frc/excalib/control/motor/PhoenixSignalHub.java | Adds central Phoenix optimize pass + CANcoder registration helpers. |
| src/main/java/frc/excalib/control/motor/motor_specs/CurrentBudget.java | Adds per-mechanism current budget record with Phoenix config conversion. |
| src/main/java/frc/excalib/control/motor/controllers/TalonFXMotor.java | Auto-registers for fault reporting + optimization; adds current budget + fused CANcoder configuration APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| test.and(primary.triangle()).whileTrue(superstructure.setStateCommand(RobotState.NO_INTAKE_AIM_HUB)); | ||
| test.and(primary.cross()).whileTrue(superstructure.setStateCommand(RobotState.IDLE)); |
| public Command requestAndWaitCommand(S target) { | ||
| return Commands.runOnce(() -> request(target)) | ||
| .andThen(Commands.waitUntil(() -> current == target)) | ||
| .withName(name + " -> " + target + " (wait)"); | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/frc/excalib/swerve/Swerve.java (1)
526-530: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove the extra turret transform from the vision pose —
src/main/java/frc/excalib/swerve/Swerve.java:513-529
LimelightMegaTag2.getEstimate()already readsbotpose_orb_wpiblue, soestimate.pose()is the robot pose in field coordinates.turretToRobot()is treating that robot pose like a turret pose and applies the turret offset a second time, biasing every vision update. Feedestimate.pose()directly intoaddVisionMeasurement()and keep the turret offset only insetRobotOrientation(...).🤖 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/excalib/swerve/Swerve.java` around lines 526 - 530, Update the vision-estimate handling to pass estimate.pose() directly to addVisionMeasurement() without calling turretToRobot(). Remove the now-unneeded turretToRobot() transform, while retaining the turret offset exclusively in setRobotOrientation(...).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/frc/excalib/control/motor/controllers/TalonFXMotor.java`:
- Around line 234-267: Update the Javadoc for configureFusedCANcoder to
explicitly state that when SensorToMechanismRatio is configured,
getMotorPosition() already returns mechanism units and
setPositionConversionFactor must remain 1.0 to prevent double conversion. Keep
the existing fused-feedback behavior and configuration unchanged.
In `@src/main/java/frc/excalib/statemachine/StateMachine.java`:
- Around line 136-139: Move the pending-state reset before transitionTo(target)
in both request() and periodic(), matching the existing forceState() ordering.
Preserve any new pending request or forceState set by the target state's onEnter
actions, while keeping the existing transition and return behavior unchanged.
- Around line 151-155: Update requestAndWaitCommand so its wait condition
completes when the target is reached or the requested transition becomes
impossible, including denied requests and current-state drift caused by
forceState. Preserve waiting while the target remains reachable pending guards,
and leave callers to verify success via getCurrentState().
- Around line 233-236: Update StateMachine’s whileIn command handling so
re-entering a state creates a fresh Command instance rather than reusing the
previously stored one. Replace the stored Command value with a Supplier<Command>
and invoke it before CommandScheduler.getInstance().schedule(), preserving the
existing scheduling behavior.
In `@src/main/java/frc/excalib/swerve/Swerve.java`:
- Around line 63-64: Remove the hardcoded LIMELIGHT_NAME constant from Swerve
and accept the robot-specific Limelight name through Swerve’s constructor, then
use that parameter when initializing megaTag2. Update all Swerve construction
sites to provide the configured name while preserving existing behavior.
- Around line 506-524: Update the coding guideline for Swerve vision fusion to
describe the LimelightMegaTag2 path, including turret-frame conversion, stdDevs
weighting, and the TurretOffsetGetter.instance.isFast() guard instead of
LimelightHelpers. In updateMegaTag2Vision, replace the hardcoded zero yaw rate
passed to megaTag2.setRobotOrientation with the actual IMU yaw rate converted to
degrees per second, while preserving the existing heading and vision-fusion
behavior.
In `@src/main/java/frc/excalib/swerve/vision/LimelightMegaTag2.java`:
- Around line 77-81: Update LimelightMegaTag2.stdDevs by extracting the tuning
literals 0.4 and 30.0 into clearly named constants or DogLog tunables, then use
those symbols in the base calculation so on-robot tuning does not require
editing the method logic.
In `@src/main/java/frc/excalib/telemetry/FaultReporter.java`:
- Around line 62-71: Update FaultReporter.scan() to replace the four individual
refresh() calls on hardwareFault, deviceTempFault, bootDuringEnable, and
undervoltage with a single BaseStatusSignal.refreshAll(...) batch refresh. Keep
the existing update(...) calls and fault-processing order unchanged.
In `@src/main/java/frc/robot/Robot.java`:
- Around line 56-61: Move TalonFXMotor.refreshAll() in Robot.robotPeriodic() to
execute after Monologue.updateAll(), preserving the required order of
ShiftUtil.update(), RobotContainer.periodic(), PeriodicScheduler,
CommandScheduler.run(), Monologue.updateAll(), and then
TalonFXMotor.refreshAll(). Keep the other periodic calls unchanged.
In `@src/main/java/frc/robot/RobotContainer.java`:
- Line 84: Re-enable the registerCommands() call in the RobotContainer
constructor by removing its comment marker, ensuring named commands such as
"idle", "shoot", and "intake" are registered before autonomous routines run.
Leave configureTestBindings() unchanged.
---
Outside diff comments:
In `@src/main/java/frc/excalib/swerve/Swerve.java`:
- Around line 526-530: Update the vision-estimate handling to pass
estimate.pose() directly to addVisionMeasurement() without calling
turretToRobot(). Remove the now-unneeded turretToRobot() transform, while
retaining the turret offset exclusively in setRobotOrientation(...).
🪄 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: aef3a277-9859-4f52-806f-ede0e619d0e3
📒 Files selected for processing (17)
src/main/java/frc/excalib/control/motor/PhoenixSignalHub.javasrc/main/java/frc/excalib/control/motor/controllers/TalonFXMotor.javasrc/main/java/frc/excalib/control/motor/motor_specs/CurrentBudget.javasrc/main/java/frc/excalib/slam/mapper/Odometry.javasrc/main/java/frc/excalib/statemachine/StateMachine.javasrc/main/java/frc/excalib/swerve/Swerve.javasrc/main/java/frc/excalib/swerve/vision/LimelightMegaTag2.javasrc/main/java/frc/excalib/telemetry/FaultReporter.javasrc/main/java/frc/excalib/telemetry/LoopTimer.javasrc/main/java/frc/excalib/telemetry/Telemetry.javasrc/main/java/frc/excalib/telemetry/TunableGains.javasrc/main/java/frc/excalib/telemetry/TunableNumber.javasrc/main/java/frc/robot/Constants.javasrc/main/java/frc/robot/Robot.javasrc/main/java/frc/robot/RobotContainer.javasrc/main/java/frc/robot/subsystems/shooter/Shooter.javavendordeps/DogLog.json
| /** | ||
| * Applies a full {@link CurrentBudget} (stator + supply limits, optional time-based supply | ||
| * reduction, and TorqueCurrentFOC output peaks) in one call. Prefer this over | ||
| * {@link #setCurrentLimit} when the mechanism's allocation is kept in a named budget table. | ||
| */ | ||
| public void applyCurrentBudget(CurrentBudget budget) { | ||
| applyChecked(super.getConfigurator().apply(budget.toCurrentLimitsConfigs()), "applyCurrentBudget(limits)"); | ||
| applyChecked(super.getConfigurator().apply(budget.toTorqueCurrentConfigs()), "applyCurrentBudget(torque)"); | ||
| } | ||
|
|
||
| /** | ||
| * Fuses a remote CANcoder into this TalonFX's feedback (Phoenix 6 {@code FusedCANcoder}): | ||
| * the absolute CANcoder sets the mechanism position while the integrated rotor supplies | ||
| * high-rate velocity, so {@link #getMotorPosition()} returns the absolute mechanism angle | ||
| * at the rotor's update rate. Used for swerve steering and any absolute-referenced pivot. | ||
| * | ||
| * <p>After this call, mechanism position is reported directly by the device — do <b>not</b> | ||
| * also seed it with {@link #setMotorPosition}. The CANcoder's magnet offset must be burned | ||
| * into the encoder (this does not set it). ⚠ Verify inversion and ratios on the robot. | ||
| * | ||
| * @param cancoderId CAN id of the CANcoder (same bus as this motor) | ||
| * @param rotorToSensorRatio gear ratio from motor rotor to the CANcoder (steer gearing) | ||
| * @param sensorToMechanismRatio gear ratio from the CANcoder to the mechanism (usually 1.0) | ||
| */ | ||
| public void configureFusedCANcoder(int cancoderId, double rotorToSensorRatio, double sensorToMechanismRatio) { | ||
| var feedback = new FeedbackConfigs(); | ||
| if (!refreshChecked(super.getConfigurator().refresh(feedback), "configureFusedCANcoder")) return; | ||
| feedback.FeedbackSensorSource = FeedbackSensorSourceValue.FusedCANcoder; | ||
| feedback.FeedbackRemoteSensorID = cancoderId; | ||
| feedback.RotorToSensorRatio = rotorToSensorRatio; | ||
| feedback.SensorToMechanismRatio = sensorToMechanismRatio; | ||
| applyChecked(super.getConfigurator().apply(feedback), "configureFusedCANcoder"); | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Document conversion-factor interaction with FusedCANcoder.
When FusedCANcoder is the feedback source with SensorToMechanismRatio set, posSignal already reports mechanism units. But getMotorPosition() (line 320) multiplies by m_positionConversionFactor. If a caller sets a non-1.0 position conversion factor (as the coding guidelines require after construction), positions will be double-converted. The Javadoc says "mechanism position is reported directly by the device" but should explicitly warn that setPositionConversionFactor must remain 1.0 when using fused-CANcoder feedback.
📝 Suggested Javadoc addition
* <p>After this call, mechanism position is reported directly by the device — do <b>not</b>
* also seed it with {`@link` `#setMotorPosition`}. The CANcoder's magnet offset must be burned
- * into the encoder (this does not set it). ⚠ Verify inversion and ratios on the robot.
+ * into the encoder (this does not set it). ⚠ Verify inversion and ratios on the robot.
+ * Because the device already applies {`@code` SensorToMechanismRatio}, keep
+ * {`@link` `#setPositionConversionFactor`} at 1.0 to avoid double-conversion of position.📝 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.
| /** | |
| * Applies a full {@link CurrentBudget} (stator + supply limits, optional time-based supply | |
| * reduction, and TorqueCurrentFOC output peaks) in one call. Prefer this over | |
| * {@link #setCurrentLimit} when the mechanism's allocation is kept in a named budget table. | |
| */ | |
| public void applyCurrentBudget(CurrentBudget budget) { | |
| applyChecked(super.getConfigurator().apply(budget.toCurrentLimitsConfigs()), "applyCurrentBudget(limits)"); | |
| applyChecked(super.getConfigurator().apply(budget.toTorqueCurrentConfigs()), "applyCurrentBudget(torque)"); | |
| } | |
| /** | |
| * Fuses a remote CANcoder into this TalonFX's feedback (Phoenix 6 {@code FusedCANcoder}): | |
| * the absolute CANcoder sets the mechanism position while the integrated rotor supplies | |
| * high-rate velocity, so {@link #getMotorPosition()} returns the absolute mechanism angle | |
| * at the rotor's update rate. Used for swerve steering and any absolute-referenced pivot. | |
| * | |
| * <p>After this call, mechanism position is reported directly by the device — do <b>not</b> | |
| * also seed it with {@link #setMotorPosition}. The CANcoder's magnet offset must be burned | |
| * into the encoder (this does not set it). ⚠ Verify inversion and ratios on the robot. | |
| * | |
| * @param cancoderId CAN id of the CANcoder (same bus as this motor) | |
| * @param rotorToSensorRatio gear ratio from motor rotor to the CANcoder (steer gearing) | |
| * @param sensorToMechanismRatio gear ratio from the CANcoder to the mechanism (usually 1.0) | |
| */ | |
| public void configureFusedCANcoder(int cancoderId, double rotorToSensorRatio, double sensorToMechanismRatio) { | |
| var feedback = new FeedbackConfigs(); | |
| if (!refreshChecked(super.getConfigurator().refresh(feedback), "configureFusedCANcoder")) return; | |
| feedback.FeedbackSensorSource = FeedbackSensorSourceValue.FusedCANcoder; | |
| feedback.FeedbackRemoteSensorID = cancoderId; | |
| feedback.RotorToSensorRatio = rotorToSensorRatio; | |
| feedback.SensorToMechanismRatio = sensorToMechanismRatio; | |
| applyChecked(super.getConfigurator().apply(feedback), "configureFusedCANcoder"); | |
| } | |
| /** | |
| * Applies a full {`@link` CurrentBudget} (stator + supply limits, optional time-based supply | |
| * reduction, and TorqueCurrentFOC output peaks) in one call. Prefer this over | |
| * {`@link` `#setCurrentLimit`} when the mechanism's allocation is kept in a named budget table. | |
| */ | |
| public void applyCurrentBudget(CurrentBudget budget) { | |
| applyChecked(super.getConfigurator().apply(budget.toCurrentLimitsConfigs()), "applyCurrentBudget(limits)"); | |
| applyChecked(super.getConfigurator().apply(budget.toTorqueCurrentConfigs()), "applyCurrentBudget(torque)"); | |
| } | |
| /** | |
| * Fuses a remote CANcoder into this TalonFX's feedback (Phoenix 6 {`@code` FusedCANcoder}): | |
| * the absolute CANcoder sets the mechanism position while the integrated rotor supplies | |
| * high-rate velocity, so {`@link` `#getMotorPosition`()} returns the absolute mechanism angle | |
| * at the rotor's update rate. Used for swerve steering and any absolute-referenced pivot. | |
| * | |
| * <p>After this call, mechanism position is reported directly by the device — do <b>not</b> | |
| * also seed it with {`@link` `#setMotorPosition`}. The CANcoder's magnet offset must be burned | |
| * into the encoder (this does not set it). ⚠ Verify inversion and ratios on the robot. | |
| * Because the device already applies {`@code` SensorToMechanismRatio}, keep | |
| * {`@link` `#setPositionConversionFactor`} at 1.0 to avoid double-conversion of position. | |
| * | |
| * `@param` cancoderId CAN id of the CANcoder (same bus as this motor) | |
| * `@param` rotorToSensorRatio gear ratio from motor rotor to the CANcoder (steer gearing) | |
| * `@param` sensorToMechanismRatio gear ratio from the CANcoder to the mechanism (usually 1.0) | |
| */ | |
| public void configureFusedCANcoder(int cancoderId, double rotorToSensorRatio, double sensorToMechanismRatio) { | |
| var feedback = new FeedbackConfigs(); | |
| if (!refreshChecked(super.getConfigurator().refresh(feedback), "configureFusedCANcoder")) return; | |
| feedback.FeedbackSensorSource = FeedbackSensorSourceValue.FusedCANcoder; | |
| feedback.FeedbackRemoteSensorID = cancoderId; | |
| feedback.RotorToSensorRatio = rotorToSensorRatio; | |
| feedback.SensorToMechanismRatio = sensorToMechanismRatio; | |
| applyChecked(super.getConfigurator().apply(feedback), "configureFusedCANcoder"); | |
| } |
🤖 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/excalib/control/motor/controllers/TalonFXMotor.java` around
lines 234 - 267, Update the Javadoc for configureFusedCANcoder to explicitly
state that when SensorToMechanismRatio is configured, getMotorPosition() already
returns mechanism units and setPositionConversionFactor must remain 1.0 to
prevent double conversion. Keep the existing fused-feedback behavior and
configuration unchanged.
| if (guard.getAsBoolean()) { | ||
| transitionTo(target); | ||
| pending = null; | ||
| return true; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
pending is cleared after transitionTo, clobbering any new request set by onEnter actions.
Both request() (line 138) and periodic() (line 195) execute pending = null after transitionTo(target). If the new state's onEnter runnable calls request() or forceState() — a natural pattern for cascading transitions — that newly set pending is immediately overwritten to null, silently dropping the follow-up transition.
🔧 Proposed fix: clear `pending` before `transitionTo`
if (guard.getAsBoolean()) {
+ pending = null;
transitionTo(target);
- pending = null;
return true;
} } else if (guard.getAsBoolean()) {
+ pending = null;
transitionTo(target);
- pending = null;
}forceState (line 160) already does this correctly — pending = null precedes transitionTo. The same pattern should be applied consistently here.
Also applies to: 193-195
🤖 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/excalib/statemachine/StateMachine.java` around lines 136 -
139, Move the pending-state reset before transitionTo(target) in both request()
and periodic(), matching the existing forceState() ordering. Preserve any new
pending request or forceState set by the target state's onEnter actions, while
keeping the existing transition and return behavior unchanged.
| public Command requestAndWaitCommand(S target) { | ||
| return Commands.runOnce(() -> request(target)) | ||
| .andThen(Commands.waitUntil(() -> current == target)) | ||
| .withName(name + " -> " + target + " (wait)"); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
requestAndWaitCommand hangs forever on illegal or stale transitions.
If request(target) returns false because no declared edge exists, current will never become target and Commands.waitUntil never resolves. The same happens if a forceState clears the pending request mid-wait. The command blocks indefinitely, which can stall autonomous sequences or command groups.
🔧 Proposed fix: finish when the transition becomes impossible
public Command requestAndWaitCommand(S target) {
return Commands.runOnce(() -> request(target))
- .andThen(Commands.waitUntil(() -> current == target))
+ .andThen(Commands.waitUntil(() -> current == target || guardFor(current, target) == null))
.withName(name + " -> " + target + " (wait)");
}This lets the command complete as soon as the edge becomes illegal (denied request, or state drifted via forceState), while still waiting for pending guards to pass. Callers should check getCurrentState() after the command finishes to confirm success.
📝 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.
| public Command requestAndWaitCommand(S target) { | |
| return Commands.runOnce(() -> request(target)) | |
| .andThen(Commands.waitUntil(() -> current == target)) | |
| .withName(name + " -> " + target + " (wait)"); | |
| } | |
| public Command requestAndWaitCommand(S target) { | |
| return Commands.runOnce(() -> request(target)) | |
| .andThen(Commands.waitUntil(() -> current == target || guardFor(current, target) == null)) | |
| .withName(name + " -> " + target + " (wait)"); | |
| } |
🤖 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/excalib/statemachine/StateMachine.java` around lines 151 -
155, Update requestAndWaitCommand so its wait condition completes when the
target is reached or the requested transition becomes impossible, including
denied requests and current-state drift caused by forceState. Preserve waiting
while the target remains reachable pending guards, and leave callers to verify
success via getCurrentState().
| Command command = whileIn.get(state); | ||
| if (command != null) { | ||
| CommandScheduler.getInstance().schedule(command); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check WPILib CommandScheduler.schedule() for already-scheduled / cancelled-but-not-ended commands.
# Look for the isScheduled() guard or rescheduling logic.
rg -nC5 'public.*schedule\b' --type java $(fd -t f CommandScheduler.java)Repository: ExcaliburFRC/Rebuilt2026
Length of output: 162
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant state machine code and search for whileIn usage.
git ls-files 'src/main/java/frc/excalib/statemachine/StateMachine.java' 'src/main/java/frc/excalib/**/*.java' | sed -n '1,40p'
echo '--- StateMachine.java (around requested lines) ---'
cat -n src/main/java/frc/excalib/statemachine/StateMachine.java | sed -n '200,280p'
echo '--- whileIn references ---'
rg -n 'whileIn|onEnter|onExit|cancel\(|schedule\(' src/main/java/frc/excalib/statemachine/StateMachine.java src/main/java/frc/excalib -g'*.java'Repository: ExcaliburFRC/Rebuilt2026
Length of output: 11227
🌐 Web query:
WPILib CommandScheduler.schedule canceled but not yet processed isScheduled behavior same tick reschedule command
💡 Result:
In the WPILib CommandScheduler, commands that are canceled or scheduled during the scheduler's run loop are handled using a deferred processing mechanism to avoid concurrent modification issues [1]. 1. Deferred Execution: When you call CommandScheduler.schedule or CommandScheduler.cancel while the scheduler is actively running its loop (i.e., m_inRunLoop is true), the commands are not processed immediately [1][2]. Instead, they are added to internal queues (m_toSchedule, m_toCancelCommands) [1][2]. 2. isScheduled Behavior: The isScheduled method checks if a command is currently in the set of active commands (m_scheduledCommands) [1][3]. Because cancellation is deferred when inside the run loop, a command that has been "canceled" but not yet processed will still return true for isScheduled until the end of the current scheduler iteration [1]. 3. Rescheduling in the Same Tick: If you attempt to reschedule a command that has been "canceled" but not yet processed (i.e., it is currently in the cancellation queue during the same tick), the scheduler's schedule method will see that isScheduled still returns true [1][2]. Consequently, the scheduler will treat the command as already running and effectively perform a no-op, meaning the reschedule attempt will fail to add the command to the schedule for the current tick [4][2]. To ensure a command is correctly rescheduled after being canceled, you must allow the scheduler to complete its current iteration so that it can clear the canceled command from its internal state. Rescheduling typically works as expected in the subsequent iteration of the robot loop [4][5].
Citations:
- 1: https://github.com/wpilibsuite/allwpilib/blob/master/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/CommandScheduler.java
- 2: https://github.com/wpilibsuite/allwpilib/blob/v2024.3.2/wpilibNewCommands/src/main/java/edu/wpi/first/wpilibj2/command/CommandScheduler.java
- 3: https://github.wpilib.org/allwpilib/docs/release/java/edu/wpi/first/wpilibj2/command/Command.html
- 4: https://docs.wpilib.org/en/latest/docs/software/commandbased/commands-v2/command-scheduler.html
- 5: https://docs.wpilib.org/en/latest/docs/software/commandbased/command-scheduler.html
whileIn commands need a fresh instance on re-entry
StateMachine reuses the same Command object for a state. If a state is exited and re-entered before the scheduler processes the cancel, schedule() will treat it as still running and won’t restart it. Store a Supplier<Command> instead, or document that same-tick re-entry is unsupported.
🤖 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/excalib/statemachine/StateMachine.java` around lines 233 -
236, Update StateMachine’s whileIn command handling so re-entering a state
creates a fresh Command instance rather than reusing the previously stored one.
Replace the stored Command value with a Supplier<Command> and invoke it before
CommandScheduler.getInstance().schedule(), preserving the existing scheduling
behavior.
| private static final String LIMELIGHT_NAME = "limelight-turret"; | ||
| private final LimelightMegaTag2 megaTag2 = new LimelightMegaTag2(LIMELIGHT_NAME); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Consider making LIMELIGHT_NAME configurable rather than hardcoding it in the library layer.
Swerve is in frc.excalib and cannot reference frc.robot.Constants, but the Limelight name is robot-specific. Passing it via the constructor would keep the library reusable across robots.
🤖 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/excalib/swerve/Swerve.java` around lines 63 - 64, Remove
the hardcoded LIMELIGHT_NAME constant from Swerve and accept the robot-specific
Limelight name through Swerve’s constructor, then use that parameter when
initializing megaTag2. Update all Swerve construction sites to provide the
configured name while preserving existing behavior.
| /** | ||
| * MegaTag2 vision fusion. The camera is turret-mounted, so MT2 is fed the turret's field | ||
| * heading (robot heading + turret offset) and its returned pose is converted from turret | ||
| * frame to robot frame by {@link #turretToRobot}. Vision is skipped while the turret/robot | ||
| * is rotating too fast for a reliable pose; measurements are weighted by distance and tag | ||
| * count via {@link LimelightMegaTag2#stdDevs}. | ||
| */ | ||
| private void updateMegaTag2Vision() { | ||
| Rotation2d turretFieldHeading = getRotation2D().plus(TurretOffsetGetter.instance.getTurretOffset()); | ||
| megaTag2.setRobotOrientation(turretFieldHeading, 0.0); | ||
| if (TurretOffsetGetter.instance.isFast()) { | ||
| return; | ||
| } | ||
| megaTag2.getEstimate().ifPresent(estimate -> | ||
| m_odometry.addVisionMeasurement( | ||
| turretToRobot(estimate.pose()), | ||
| estimate.timestampSeconds(), | ||
| LimelightMegaTag2.stdDevs(estimate))); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Coding guideline violation: LimelightMegaTag2 replaces LimelightHelpers without updating the guideline.
As per coding guidelines: "In Swerve.periodic(), read Limelight pose data from LimelightHelpers and fuse it into odometry with addVisionMeasurement; skip vision updates when TurretOffsetGetter.instance.isFast() is true." The code now uses LimelightMegaTag2 instead of LimelightHelpers. While the PR objectives document this as intentional ("MegaTag2 replaces the previous MegaTag1 path"), the guideline has not been updated. Please update the coding guideline to reflect the new MegaTag2 approach.
Additionally, yawRateDegreesPerSecond is hardcoded to 0.0 on Line 515. MT2 uses this value to compensate for rotation during image capture. Since isFast() only filters when combined rotation exceeds π rad/s, moderate rotation still processes estimates without yaw-rate compensation. Consider passing the actual IMU yaw rate (converted to degrees per second) to improve pose quality during moderate rotation.
♻️ Suggested fix for yaw rate
- megaTag2.setRobotOrientation(turretFieldHeading, 0.0);
+ megaTag2.setRobotOrientation(turretFieldHeading, Math.toDegrees(m_imu.getYawRateRadPerSec()));📝 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.
| /** | |
| * MegaTag2 vision fusion. The camera is turret-mounted, so MT2 is fed the turret's field | |
| * heading (robot heading + turret offset) and its returned pose is converted from turret | |
| * frame to robot frame by {@link #turretToRobot}. Vision is skipped while the turret/robot | |
| * is rotating too fast for a reliable pose; measurements are weighted by distance and tag | |
| * count via {@link LimelightMegaTag2#stdDevs}. | |
| */ | |
| private void updateMegaTag2Vision() { | |
| Rotation2d turretFieldHeading = getRotation2D().plus(TurretOffsetGetter.instance.getTurretOffset()); | |
| megaTag2.setRobotOrientation(turretFieldHeading, 0.0); | |
| if (TurretOffsetGetter.instance.isFast()) { | |
| return; | |
| } | |
| megaTag2.getEstimate().ifPresent(estimate -> | |
| m_odometry.addVisionMeasurement( | |
| turretToRobot(estimate.pose()), | |
| estimate.timestampSeconds(), | |
| LimelightMegaTag2.stdDevs(estimate))); | |
| } | |
| /** | |
| * MegaTag2 vision fusion. The camera is turret-mounted, so MT2 is fed the turret's field | |
| * heading (robot heading + turret offset) and its returned pose is converted from turret | |
| * frame to robot frame by {`@link` `#turretToRobot`}. Vision is skipped while the turret/robot | |
| * is rotating too fast for a reliable pose; measurements are weighted by distance and tag | |
| * count via {`@link` LimelightMegaTag2#stdDevs}. | |
| */ | |
| private void updateMegaTag2Vision() { | |
| Rotation2d turretFieldHeading = getRotation2D().plus(TurretOffsetGetter.instance.getTurretOffset()); | |
| megaTag2.setRobotOrientation(turretFieldHeading, Math.toDegrees(m_imu.getYawRateRadPerSec())); | |
| if (TurretOffsetGetter.instance.isFast()) { | |
| return; | |
| } | |
| megaTag2.getEstimate().ifPresent(estimate -> | |
| m_odometry.addVisionMeasurement( | |
| turretToRobot(estimate.pose()), | |
| estimate.timestampSeconds(), | |
| LimelightMegaTag2.stdDevs(estimate))); | |
| } |
🤖 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/excalib/swerve/Swerve.java` around lines 506 - 524, Update
the coding guideline for Swerve vision fusion to describe the LimelightMegaTag2
path, including turret-frame conversion, stdDevs weighting, and the
TurretOffsetGetter.instance.isFast() guard instead of LimelightHelpers. In
updateMegaTag2Vision, replace the hardcoded zero yaw rate passed to
megaTag2.setRobotOrientation with the actual IMU yaw rate converted to degrees
per second, while preserving the existing heading and vision-fusion behavior.
Source: Coding guidelines
| public static Matrix<N3, N1> stdDevs(Estimate estimate) { | ||
| double base = 0.4 * (1.0 + estimate.avgTagDistanceMeters() * estimate.avgTagDistanceMeters() / 30.0); | ||
| double xy = estimate.tagCount() >= 2 ? base / 2.0 : base; | ||
| return VecBuilder.fill(xy, xy, Double.MAX_VALUE); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Name the tuning constants in stdDevs for on-robot adjustability.
0.4 and 30.0 are tuning parameters that will likely need adjustment during hardware testing. Extract them as named constants (or DogLog tunables) so they can be modified without code edits.
♻️ Proposed refactor
+ private static final double BASE_STD_DEV = 0.4;
+ private static final double DISTANCE_SQUARED_SCALE = 30.0;
+
public static Matrix<N3, N1> stdDevs(Estimate estimate) {
- double base = 0.4 * (1.0 + estimate.avgTagDistanceMeters() * estimate.avgTagDistanceMeters() / 30.0);
+ double base = BASE_STD_DEV * (1.0 + estimate.avgTagDistanceMeters() * estimate.avgTagDistanceMeters() / DISTANCE_SQUARED_SCALE);
double xy = estimate.tagCount() >= 2 ? base / 2.0 : base;
return VecBuilder.fill(xy, xy, Double.MAX_VALUE);
}📝 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.
| public static Matrix<N3, N1> stdDevs(Estimate estimate) { | |
| double base = 0.4 * (1.0 + estimate.avgTagDistanceMeters() * estimate.avgTagDistanceMeters() / 30.0); | |
| double xy = estimate.tagCount() >= 2 ? base / 2.0 : base; | |
| return VecBuilder.fill(xy, xy, Double.MAX_VALUE); | |
| } | |
| private static final double BASE_STD_DEV = 0.4; | |
| private static final double DISTANCE_SQUARED_SCALE = 30.0; | |
| public static Matrix<N3, N1> stdDevs(Estimate estimate) { | |
| double base = BASE_STD_DEV * (1.0 + estimate.avgTagDistanceMeters() * estimate.avgTagDistanceMeters() / DISTANCE_SQUARED_SCALE); | |
| double xy = estimate.tagCount() >= 2 ? base / 2.0 : base; | |
| return VecBuilder.fill(xy, xy, Double.MAX_VALUE); | |
| } |
🤖 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/excalib/swerve/vision/LimelightMegaTag2.java` around lines
77 - 81, Update LimelightMegaTag2.stdDevs by extracting the tuning literals 0.4
and 30.0 into clearly named constants or DogLog tunables, then use those symbols
in the base calculation so on-robot tuning does not require editing the method
logic.
| void scan() { | ||
| hardwareFault.refresh(); | ||
| deviceTempFault.refresh(); | ||
| bootDuringEnable.refresh(); | ||
| undervoltage.refresh(); | ||
| update(hardwareAlert, hardwareFault, "HardwareFault"); | ||
| update(tempAlert, deviceTempFault, "DeviceTempFault"); | ||
| update(bootAlert, bootDuringEnable, "BootDuringEnable"); | ||
| update(undervoltageAlert, undervoltage, "Undervoltage"); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
Use BaseStatusSignal.refreshAll() for batch fault-signal refresh.
Individual refresh() calls generate separate CAN reads per signal. Batching them with BaseStatusSignal.refreshAll() reduces CAN traffic and aligns with the project's batch-refresh pattern.
♻️ Proposed refactor
void scan() {
- hardwareFault.refresh();
- deviceTempFault.refresh();
- bootDuringEnable.refresh();
- undervoltage.refresh();
+ BaseStatusSignal.refreshAll(hardwareFault, deviceTempFault, bootDuringEnable, undervoltage);
update(hardwareAlert, hardwareFault, "HardwareFault");
update(tempAlert, deviceTempFault, "DeviceTempFault");
update(bootAlert, bootDuringEnable, "BootDuringEnable");
update(undervoltageAlert, undervoltage, "Undervoltage");
}📝 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.
| void scan() { | |
| hardwareFault.refresh(); | |
| deviceTempFault.refresh(); | |
| bootDuringEnable.refresh(); | |
| undervoltage.refresh(); | |
| update(hardwareAlert, hardwareFault, "HardwareFault"); | |
| update(tempAlert, deviceTempFault, "DeviceTempFault"); | |
| update(bootAlert, bootDuringEnable, "BootDuringEnable"); | |
| update(undervoltageAlert, undervoltage, "Undervoltage"); | |
| } | |
| void scan() { | |
| BaseStatusSignal.refreshAll(hardwareFault, deviceTempFault, bootDuringEnable, undervoltage); | |
| update(hardwareAlert, hardwareFault, "HardwareFault"); | |
| update(tempAlert, deviceTempFault, "DeviceTempFault"); | |
| update(bootAlert, bootDuringEnable, "BootDuringEnable"); | |
| update(undervoltageAlert, undervoltage, "Undervoltage"); | |
| } |
🤖 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/excalib/telemetry/FaultReporter.java` around lines 62 - 71,
Update FaultReporter.scan() to replace the four individual refresh() calls on
hardwareFault, deviceTempFault, bootDuringEnable, and undervoltage with a single
BaseStatusSignal.refreshAll(...) batch refresh. Keep the existing update(...)
calls and fault-processing order unchanged.
| LoopTimer.start(); | ||
|
|
||
| // Batch-refresh CAN signals before commands run so they act on fresh data. | ||
| TalonFXMotor.refreshAll(); | ||
| FaultReporter.poll(); | ||
| Telemetry.logPowerDistribution(); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
TalonFXMotor.refreshAll() must be called after Monologue.updateAll(), not before ShiftUtil.update().
The coding guideline requires robotPeriodic() to call ShiftUtil.update(), RobotContainer.periodic(), PeriodicScheduler, CommandScheduler.run(), Monologue.updateAll(), and TalonFXMotor.refreshAll() in that order. The current code places TalonFXMotor.refreshAll() first (line 59), violating this sequence. If the earlier placement is intentional (per the PR's "earlier refreshAll() execution" objective), update the coding guideline to reflect the new contract; otherwise, move refreshAll() to after Monologue.updateAll().
As per coding guidelines: "Robot.robotPeriodic() must call ShiftUtil.update(), RobotContainer.periodic(), PeriodicScheduler, CommandScheduler.run(), Monologue.updateAll(), and TalonFXMotor.refreshAll() in that order."
🔧 Proposed fix: move refreshAll() to end of robotPeriodic()
`@Override`
public void robotPeriodic() {
// Record loop start time for performance tracking
performanceMetricsTracker.recordLoopStart();
LoopTimer.start();
- // Batch-refresh CAN signals before commands run so they act on fresh data.
- TalonFXMotor.refreshAll();
FaultReporter.poll();
Telemetry.logPowerDistribution();
ShiftUtil.update();
robotContainer.periodic();
PeriodicScheduler.PERIOD.MILLISECONDS_20.run();
Threads.setCurrentThreadPriority(true, 99);
CommandScheduler.getInstance().run();
Monologue.updateAll();
performanceMetricsTracker.recordLoopEnd();
LoopTimer.end();
+
+ // Batch-refresh CAN signals for the next loop.
+ TalonFXMotor.refreshAll();
}📝 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.
| LoopTimer.start(); | |
| // Batch-refresh CAN signals before commands run so they act on fresh data. | |
| TalonFXMotor.refreshAll(); | |
| FaultReporter.poll(); | |
| Telemetry.logPowerDistribution(); | |
| LoopTimer.start(); | |
| FaultReporter.poll(); | |
| Telemetry.logPowerDistribution(); | |
| ShiftUtil.update(); | |
| robotContainer.periodic(); | |
| PeriodicScheduler.PERIOD.MILLISECONDS_20.run(); | |
| Threads.setCurrentThreadPriority(true, 99); | |
| CommandScheduler.getInstance().run(); | |
| Monologue.updateAll(); | |
| performanceMetricsTracker.recordLoopEnd(); | |
| LoopTimer.end(); | |
| // Batch-refresh CAN signals for the next loop. | |
| TalonFXMotor.refreshAll(); |
🤖 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/Robot.java` around lines 56 - 61, Move
TalonFXMotor.refreshAll() in Robot.robotPeriodic() to execute after
Monologue.updateAll(), preserving the required order of ShiftUtil.update(),
RobotContainer.periodic(), PeriodicScheduler, CommandScheduler.run(),
Monologue.updateAll(), and then TalonFXMotor.refreshAll(). Keep the other
periodic calls unchanged.
Source: Coding guidelines
| //registerCommands(); | ||
| setAutoChooser(); | ||
| configureBindings(); | ||
| configureTestBindings(); // on-robot bring-up harness — active only in DriverStation Test mode |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Re-enable registerCommands() in the constructor.
Line 81 still has //registerCommands(); commented out. The coding guidelines for this file explicitly require re-enabling this call. If any PathPlanner autonomous routine references the named commands ("idle", "shoot", "intake") registered in registerCommands(), they will silently fail at runtime since NamedCommands.registerCommand() is never called.
As per coding guidelines: "Register PathPlanner named commands in RobotContainer.registerCommands() if any autonomous routine depends on them, and re-enable the constructor call that currently comments out registration."
🔧 Proposed fix
//registerCommands();
+ registerCommands();
setAutoChooser();
configureBindings();
configureTestBindings(); // on-robot bring-up harness — active only in DriverStation Test mode📝 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.
| configureTestBindings(); // on-robot bring-up harness — active only in DriverStation Test mode | |
| registerCommands(); | |
| setAutoChooser(); | |
| configureBindings(); | |
| configureTestBindings(); // on-robot bring-up harness — active only in DriverStation Test mode |
🤖 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/RobotContainer.java` at line 84, Re-enable the
registerCommands() call in the RobotContainer constructor by removing its
comment marker, ensuring named commands such as "idle", "shoot", and "intake"
are registered before autonomous routines run. Leave configureTestBindings()
unchanged.
Source: Coding guidelines
Cherry-picks selected features from
feature/robot-migrationinto the existingfrc.excaliblibrary without adoptingexcalib2or changing mechanism/swerve concepts. Verified with./gradlew compileJava(BUILD SUCCESSFUL). Not hardware-tested.What's included
Telemetry (
frc.excalib.telemetry, DogLog-based, runs alongside existing Monologue@Log.NT)Telemetry— DogLog + PhoenixSignalLogger+ PDH logging lifecycleLoopTimer— loop/max/overrun timingFaultReporter— per-TalonFX fault + RIO CAN-health alerts (replacesCANHealthMonitor)TunableNumber/TunableGains— live/Tuning/values, off by default; wired as an example into the Shooter hood PIDStateMachine (
frc.excalib.statemachine) — typed transition table with guards + enter/while/exit actions + DogLog telemetry.Vision —
LimelightMegaTag2wired intoSwerve.periodic(); MT2 replaces the old MegaTag1 path, with distance/tag-count-scaled std-devs. Turret conversion andisFast()gating preserved.Motor library (
TalonFXMotor) —CurrentBudgetrecord +applyCurrentBudget,configureFusedCANcoderAPI (not live-wired).Phoenix CAN optimization —
PhoenixSignalHubrunsoptimizeBusUtilizationForAllon all motors + swerve/shooter CANcoders once at startup (optimizeAllwas previously dead code). Fault + encoder-position signals are protected first so the optimize pass doesn't disable them.refreshAll()moved to the top ofrobotPeriodic. Pigeon left full-rate deliberately.Bring-up — Test-mode-gated harness in
RobotContainer(coast, module SysId, state checks) + DogLog vendordep.Behavior changes to verify on hardware
refreshAll()moved earlier in the loop (commands now act on fresh CAN data).Deliberately deferred (APIs delivered, not live-wired)
configureFusedCANcoder(cancoderId, 26.09, 1.0)inSwerveModule; needs on-robot offset/ratio/inversion check.TalonFXMotor; adopting it means replacing the hood/turret hand-tuned software PID (re-opens tuning).Recommended follow-up
Bench-test on the robot first (confirm DogLog keys, fault alerts, CAN utilization drop, MT2 pose) before flipping on FusedCANcoder or Motion Magic.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Monitoring