Report per-joint drive faults, and make the status rate settable - #39
Open
Jepson2k wants to merge 2 commits into
Open
Report per-joint drive faults, and make the status rate settable#39Jepson2k wants to merge 2 commits into
Jepson2k wants to merge 2 commits into
Conversation
The drivers report fault bits over the serial link rather than analog temperature or current registers, so drive_health carries faults and nothing else here: one label tuple per joint, empty when that drive is healthy. A list of empty tuples is deliberately not the same as an empty list — the first is an all-clear, the second is a backend with no fault reporting at all, and a display that cannot tell them apart will show a bus it never asked about as healthy. The label view is rebuilt only when a bit moves, so the 100 Hz path allocates nothing while the drives are fine, and the wire entry is replaced only on change so a snapshot's shallow copy keeps the labels current when it was taken. SET_STATUS_RATE makes the broadcast rate a session knob rather than a boot constant, which is what lets a capture or a tuning run get resolution the 50 Hz default cannot. Status is emitted every Nth control tick, so the achievable rates are the divisors of the control rate; the STATUS_RATE query reports that loop rate rather than a list of legal values, letting a caller compute the set itself. A rate that does not divide evenly is refused with a remedy naming the ones that do, never rounded to a neighbour — a capture taken at a rate nobody asked for is wrong in a way nothing reports. The control loop re-derives its broadcast interval when the rate moves instead of capturing it once at startup, and the status cache adopts the new period too, since it differentiates TCP position against it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DsMBr94VMTsLJtPJq7h6A5
The validator divided before it screened: control % int(hz) ran ahead of the
integrality check, so 0.5 Hz floored to a zero divisor and NaN could not be
made an int at all. Both escaped as a generic tick failure, which is the one
thing a refusal must not do -- whoever asked for 0.5 Hz got a crash instead
of the rates that would have worked. The guard now establishes finite, in
range and integral before the modulo sees anything, and refuses through the
command protocol rather than an exception the executor has to unwrap.
make_error() formatted title and cause only, so the one template that puts
its parameter in the remedy -- the remedy being where a refusal says what
would have worked -- reached the client reading "Pick a rate that divides
{control} Hz: {allowed}". All four fields format now.
The rate also had more owners than one. STATUS_BROADCAST_INTERVAL was
computed at import from the boot rate, the status cache kept a
_status_rate_hz shadow of its own, and StatusBroadcaster took a rate_hz it
never read. ControllerState owns it now: status_broadcast_interval(hz)
derives the tick count on demand, and the TCP-speed derivative divides by
the live broadcast period while keeping the period its previous sample was
taken at, so the frame that straddles a rate change is not differentiated
against a period it never spanned.
Drive-fault labels are a pre-built lookup keyed by the two bits, because
that path runs at 100 Hz and the hot path does not allocate.
Co-Authored-By: Claude Opus 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
2 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.
Moved from the Jepson2k fork, unchanged: Waldo-Commander's CI branch-matches parol6 against this repository, so the branch has to live here for its diagnostics tests to pick it up.
What
drive_health.faults: one tuple of labels per joint, empty when that drive is healthy. The drivers report fault bits over the serial link rather than temperature or current registers, so faults is alldrive_healthholds. A list of empty tuples is an all-clear; an empty list is a backend with no fault reporting at all, and the two are deliberately distinct so a display cannot show a bus it never asked about as healthy. The label view is rebuilt only when a bit moves, so the 100 Hz path allocates nothing while the drives are fine, and the wire entry is replaced only on change so a snapshot's shallow copy keeps the labels current when it was taken.SET_STATUS_RATE/STATUS_RATE: the broadcast rate is a session knob rather than a boot constant, for a capture or tuning run that needs resolution the 50 Hz default cannot give. Status is emitted every Nth control tick, so the achievable rates are the divisors of the control rate;STATUS_RATEreports the loop rate so a caller computes the set itself. A rate that does not divide is refused with a remedy naming the ones that do, never rounded to a neighbour.Companions
Same branch name on waldoctl (Jepson2k/waldoctl#23), par6 (Jepson2k/par6#62) and Waldo-Commander (Jepson2k/Waldo-Commander#46). CI installs waldoctl from the matching branch until it is tagged.
Verification
tests/integration/test_drive_faults.pyandtests/integration/test_status_rate.pycover the fault labels and the divisor refusal against the fake-serial controller. The commit is the fork's 337237f as-is, one commit ahead of currentmain; the suite was not re-run for the move, so CI here is the check.— written by Claude on behalf of @Jepson2k
🤖 Generated with Claude Code