Raise CAN device and bus health alerts, with a chain break hint - #57
Open
nlaverdure wants to merge 8 commits into
Open
nlaverdure wants to merge 8 commits into
nlaverdure wants to merge 8 commits into
Conversation
Each bus in Constants declares a CANChain and one CANChain.Device per line, in daisy-chain order from the SystemCore. Java runs static initializers in the order written (JLS 12.4.2), so line order is chain order. Each bus class also holds the one Phoenix CANBus for its port. CANChainMonitor raises a HIGH alert naming the broken link once a clean split (at least two devices down) holds for 2.5 s. It reads a single map from device to connection state, which Robot builds from Drive.canConnections() and LoggedPowerDistribution.canConnections(). Both buses start untraced (CHAIN_ORDER_TRACED = null), so the monitor stays off and silent until the wiring is traced. LoggedPowerDistribution logs a debounced "voltage > 0" as a replayable connected input, raises PD/disconnected, and skips the other reads while the PDH is missing. This carries PR 55 forward, simplified per the 2026-09-27 review: no chain freezing, no per-port filtering or duplicate-source check, and module hardware keeps using SC1.BUS with the IDs from its constants. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Phoenix 6 sends its automatic alerts straight to MrcLib, so AdvantageKit never logs them (#47). Each module now raises WPILib alerts from logged inputs, so replay shows the same alerts: - turnEncoderDisconnected, next to the motor disconnect alerts. - setupFailed, one per module, listing every failed setup step (drive config and position reset, turn config, CANcoder read and write). - firmwareBlocked, when setControl returns FirmwareTooOld or ApiTooOld, which is how Phoenix reports that it is blocking output. A failed CANcoder config read leaves cancoderConfig holding defaults. The constructor then skips the write, and Module neither seeds nor overwrites the turn-zero Preference, on the first loop or from the zero-encoders button. ModuleIOTalonFXBase.setTurnZero also refuses to apply, as a backstop. Firmware versions are read without blocking or error reports until each first arrives (Phoenix sends them at 4 Hz), then logged as inputs. Phoenix .hoot logging is off unless FeatureFlags.HOOT_LOGGING_ENABLED. The setup and firmware text builders are pure static functions, tested in ModuleHealthTest without the HAL. Refs #47, #52 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
LoggedCANBus now runs one bus-health step per loop: 1. Copy the latest status sample into logged inputs. A background thread reads CANBus.getStatus() every 400 ms in REAL mode, because CTRE documents that call as blocking for up to 1 ms. The new #50 fields (BusErrorCount, ArbitrationLostCount, RestartCount, State, Status) are logged with it, and SampleCount lets CANBusHealth detect a stalled reader. 2. CANBusHealth raises HIGH for ErrorPassive, BusOff, Stopped, a failed status read, a rising bus-off or restart count, or a stalled reader, and MEDIUM for ErrorWarning, each held 0.5 s. 3. The chain monitor runs with the bus fault flag. While the bus alert is HIGH, a break further along the chain is not shown, since a bus fault can drop devices anywhere. A break at the SystemCore end still shows: "check the SystemCore port and plug" fits a bus fault too. Everything comes from logged inputs, so replay reproduces the alerts. Refs #50 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Before the first Module.periodic(), the inputs hold their defaults, so turnEncoderRefreshStatus reads "OK" even for an encoder whose config read failed. A zero request in that window got past the refresh check and wrote the turn-zero Preference from default values. The device write was still refused by ModuleIOTalonFXBase.setTurnZero. Found in sim with an injected CANcoder read failure on FrontLeft and a zero call before the first loop. With this change, FrontLeft's Preference stays unset and its setTurnZero is never called, both before and after the first loop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Sep 27, 2026
A missing PDH still sent one Driver Station error per loop: PowerDistributionJNI.getVoltage reports its own failed read (PowerDistributionJNI.cpp:153, allwpilib v2027.0.0-alpha-7), and the quiet getVoltageNoError needs the handle that PowerDistribution keeps private. While the module is missing, LoggedPowerDistribution now reads it once per second, so it sends about one error per second, and a returning module is noticed at the next read. shouldRead is a pure static function, tested without the HAL. DriveConstants.MODULE_DEVICES is now an array in Drive's module order (FL, FR, BL, BR). It replaces the IdentityHashMap lookup from each module's SwerveModuleConstants and keeps the per-device buses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both follow the patterns already in the codebase: - LoggedCANBus reads CANBus.getStatus() from a WPILib Notifier every 0.4 s, like CanandgyroThread, SparkOdometryThread and VisionThread, instead of a hand-rolled Thread and sleep loop. - LoggedPowerDistribution times its retry of a missing module with Timer.advanceIfElapsed, like RobotStats, instead of comparing timestamps by hand. The retry logic now lives in WPILib's Timer, so its unit test is gone rather than bootstrapping the HAL clock. Verified in sim: with the reader started, SampleCount rose 2.508 per second on both buses with no alerts. With the PD forced to 0 V, it was read every loop through the 0.5 s debounce, then every 1.000 s (+/- 0.02 s). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A public static final array fixes only the reference; any class could still overwrite its entries. List.of makes the list itself immutable, and a test checks that set() throws. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CANChainMonitor.update fills one boolean[] field each loop instead of allocating a new array. CANChain.hint uses name directly instead of a local alias. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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
This PR merges PR 54 (Phoenix device and CAN bus alerts) and PR 55 (CAN chain break hint and PD connection) into one change. It follows section 4 of the 2026-09-27 simplification review. The two PRs built overlapping per-device and per-bus logic, and they conflicted in
ModuleIOTalonFXBase,LoggedCANBusandRobot. Here that logic is built once. When this merges, PR 54 and PR 55 close in its favor.Every alert is computed from logged inputs, so a replayed log shows the same alerts.
Basis marks: ✔ checked in code, sim or tests · R read, not rechecked · D derived · 🤖 needs a robot.
What it does
Per-device health (per module)
turnEncoderDisconnectedjoins the drive and turn disconnect alerts.setupFailed: one alert that lists every failed setup step: drive config and position reset, turn config, and CANcoder config read and write.firmwareBlocked: raised whensetControlreturnsFirmwareTooOldorApiTooOld, which is how Phoenix reports that it is blocking output (R:ParentDevice.setControlPrivate).cancoderConfigholds default values. Three guards follow from that:Modulenever seeds or overwrites the turn-zero Preference: not on the first loop, not from the zero-encoders action, and not before the first loop has read the inputs.ModuleIOTalonFXBase.setTurnZeroalso refuses to apply, as a backstop..hootlogging is off unlessFeatureFlags.HOOT_LOGGING_ENABLEDis set.CAN topology and PD
Constantsdeclares aCANChain, then oneCANChain.Deviceper line in daisy-chain order. Java runs static initializers in the order written (JLS §12.4.2), so line order is chain order. Each bus class holds the singleCANBusinstance for its port.CANChainMonitorraises a HIGH alert naming the broken link once a clean split holds for 2.5 s. A clean split means devices0..k-1respond and at least two devices fromkon don't.Robotbuilds fromDriveandLoggedPowerDistribution.LoggedPowerDistributionlogs a debouncedvoltage > 0as a replayableconnectedinput and raisesPD/disconnected. While the PDH is missing, it skips the other reads and reads the voltage only once a second, timed withTimer.advanceIfElapsedasRobotStatsdoes. Each failed read sends a Driver Station error (✔PowerDistributionJNI.cpp:153), so that limits the errors to about one per second instead of one per loop.One bus-health step per loop (
LoggedCANBus.log())NotifierreadsCANBus.getStatus()every 400 ms in REAL mode, like the other background readers. CTRE documents that call as blocking for up to 1 ms (R: CTRE Javadoc). This is a documented worst case, not a measurement.SampleCountlands because the stale-reader check needs it. Revisit both after that timing (Refs Log Phoenix CAN bus health fields that AdvantageKit SystemStats doesn't cover #50).CANBusHealthraises HIGH for ErrorPassive, BusOff, Stopped, a failed status read, a rising bus-off or restart count, or a stalled reader. It raises MEDIUM for ErrorWarning. Each alert is held for 0.5 s.Simplifications from the review
CANChainhas no freeze and no late-add throw. Class initialization already runs everyaddbefore any other class can read the chain.Util.setAlert.firstError,Drive/ConstructMs(apply it as a temporary patch for the robot timing), and the per-loop firmware refreshes.Verified
./gradlew buildruns 254 tests (✔). The only failure isVisionFilterTest > yawConsistency, which fails the same way onmain-2027-alpha7(review bug 4). The new logic is tested without the HAL:ModuleHealthTest: turn-zero guard, setup text, blocked text.CANBusHealthTest: a parameterized severity table plus the time-based cases.CANChainTest: a table offindBreakcases, the hint text, andvalidate.CANChainMonitorTest: hold time, the skew case, and two bus-fault cases (k=3 hidden, k=0 shown).DriveConstantsTest: every module ID equals main's./PD/Connectedstays true. No.hootsession directory. No chain entries.ModuleIOSimTalonFX): driveConfigFailedand CANcoder refreshTxFailed.26.70.0.0on the Phoenix sim devices.SampleCountrose 2.508 per second on both buses, with no alerts.ChainBreakIndexis -1 on both buses. On SC0 only the gyro is down, and a single device at the end is not a break.Robot session
Termination and tracing (disabled, on blocks)
CHAIN_ORDER_TRACED.Chain and bus behavior
OK/ErrorActive. Record State, TEC and REC with the PD and the gyro unplugged.restartRosejust duplicatesbusOffRose./SystemStats/Network/CAN<n>/FDfor both buses. CAN FD is less tolerant of a missing terminator.Devices
setupFailedandturnEncoderDisconnectedalerts appear, and no zero is saved./PD/Connectedgoes false within about 0.6 s,PD/disconnectedappears, and the DS console shows about one PD error per second rather than one per loop.firmwareBlockedalert.Timing and logging
CANBus.getStatus()on each bus. Also compare/RealOutputs/LoggedRobot/UserCodeMSp50 and p99 with a log from before this change.CANBus/SC*/SampleCountrises about 2.5 per second.CANBus/SC<n>/BusUtilization(0–1) withSystemStats/Network/CAN<n>/Utilization(percent).BusErrorCountwithRX/ErrorsandTX/Errors.periodic(), and a zero-encoders press.ParentConfigurator.java:31, used byCANcoderConfigurator.apply(configs)). A slow device could hold the first loop for up to about 0.4 s across four modules (D).Drive/ConstructMstiming patch. Record it with a healthy bus and with SC1 unplugged at boot./U/logs/session_Ncontains only.wpilogfiles.Closes #52
Refs #50, #47
🤖 Generated with Claude Code