Repository navigation
perf(arm): cut the bus reads a waypoint stream spends on its own pacing - #45
Merged
Merged
Conversation
…ving together Two cuts to the bus traffic a waypoint stream generates. Reads were costing about 0.6s of a 6.9s nod replay -- the smaller of its two overheads, but the one that also contends with concurrent gripper writes on the shared bus. The dwell already returns where the arm actually was. So when the next goal is already inside the lookahead of that position, the dwell's first read would return immediately and the read buys nothing: skip it. On a densified stream that is the common case, since 8x-interpolated 10 Hz frames give sub-degree segments against a multi-degree lookahead. Measured against the loop's own bus cost, reads drop ~8x at the 25 deg/s profile a recorded nod derives and ~16x at an 80 deg/s arm -- 544 reads become about 35. The bound has to hold without a fresh read, so it adds speed*elapsed: the furthest any joint could have moved in any direction since that read, since a joint may have reversed. That can only overstate how far the arm still has to go, so a skip is taken only where the real dwell would also have returned immediately, and both terms grow as waypoints are skipped so it stops skipping and reads again on its own. This is only safe because of MinExecutableSpeedDegsPerSec. Handing the commanded waypoint forward as the next travel reference is exactly what CLAUDE.md warned against: a joint whose travel concentrated earlier in the path showed a near-zero delta and was floored to 1 step/s while its goal was still far away. The speed floor makes that unreachable, which is what unlocked this. Second, when a read IS needed it now fetches position and Moving in one transaction. Present_Position is at 56 and Moving at 66, so an 11-byte SyncRead covers both, and every feetech transaction pays enforceCommandGap's ~1ms while holding the bus lock -- payload size is nearly free, a second transaction is not. That deleted a failure mode rather than making one fatal. With two reads, position could succeed while Moving failed, which the dwell had to treat as "assume still moving"; one read cannot land in that state, so the leniency and its test are gone. Moving is still only consulted from the second poll onward, since it takes ~2ms to rise after a goal write, but it now rides along for free on every poll. Neither cut touches the larger overhead: a nod's base motion is still 1.29x its recorded duration for reasons unexplained, and the reversal-ramp model for it failed its test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C2UqFZQ4wdgRdUxEcnm6o8
Five cuts from a complexity-only review pass, no behaviour change:
- The merged state read duplicated ~20 lines of GetJointPositionsForServos's
decode loop. Both now go through busSession.jointRadians: they differ only
in how they obtain raw ticks, not in how those ticks are interpreted.
- The Present_Position-to-Moving offset and read width were recomputed from
register addresses at every call, for a layout the control table fixes.
Hoisted to package vars (not consts -- feetech's addresses are struct
fields, so the expression is not constant).
- A 16-line comment on the 4-line skip branch, restating the commit body.
Ten lines: what the skip is, why the bound is conservative, and the
MinExecutableSpeedDegsPerSec dependency.
- CLAUDE.md's merged-read entry restated a datasheet fact and a deleted code
path at length. Halved.
- TestSkippedWaypointsCannotStarveAJoint tested CoordinatedProfiles' speed
floor rather than the skip, duplicating internal/servo's own coverage, and
carried a `_ = arm.MoveOptions{}` that existed only to keep an import
alive. Deleted -- but its WARNING was the only thing in this package that
fails loudly when the speed floor is removed, and that is worth more than
its coverage, so the assertion is folded into the skip test it belongs to
at two lines instead of nine.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C2UqFZQ4wdgRdUxEcnm6o8
The attributes table was followed by ~220 lines on calibration priority, coordinated speed scaling, waypoint pacing, orientation planning, manual mode and serial communication before reaching the command reference. Someone looking up a command's payload had to scroll past all of it. Reference first: Attributes, then DoCommand and the servo_* family, then the explanations. Pure reorder -- no prose changed except two positional references that inverted, which are now anchor links rather than "below" and so cannot rot again on the next reshuffle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C2UqFZQ4wdgRdUxEcnm6o8
Matches arm.md. A reader looking up a command's payload should not scroll past the feature and limitation prose to reach it. - gripper.md: DoCommand now follows Attributes, ahead of the TCP reference frame, Communication, and the arm-dependency failure notes. - calibration.md: Available Commands follows Configuration, ahead of Communication, Usage, the state machine and the file format. Its `port` row said "see Communication section below", which inverted -- now an anchor link. - simulated.md: DoCommand moved above Behavior in the simulated arm section. The simulated-gripper section was already in this order. teleop.md already had it. discovery.md, setup-app.md and troubleshooting.md have no attributes-plus-commands structure to reorder. Pure reorder -- verified by comparing the sorted line sets before and after, which differ only by the one anchor-link edit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C2UqFZQ4wdgRdUxEcnm6o8
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
1 participant
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.
Follow-on to #44. Playback is still slower and stuttier than the recording. This removes most
of the bus traffic the dwell spends discovering it has nothing to wait for. Pacing behaviour
is unchanged.
Also reorganizes the resource docs so the command reference sits above the explanations.
1. Skip a dwell that is already satisfiable
dwellUntilNearreturns where the arm was. If the next goal is already inside the lookaheadof that position, the dwell's first read would return immediately — so skip it.
Common case on a densified stream, where sub-degree segments sit against a multi-degree
lookahead:
speed_degs_per_sec: 80The estimate adds
speed × elapsed, bounding how far any joint could have moved in anydirection since the last read (a joint may have reversed). So it can only overstate what
remains, and a skip happens only where the dwell would also have returned immediately. Both
terms grow as waypoints are skipped, so it reads again on its own.
Depends on
MinExecutableSpeedDegsPerSecfrom #44. Using the commanded waypoint as thetravel reference instead of a live read is what starved low-
kjoints before that floorexisted.
TestDenseStreamSkipsDwellReadsWithinTheLookaheadasserts the floor still binds.2. Read position and Moving in one transaction
They are 11 bytes apart in the control table, and each transaction holds the bus lock for a
round trip plus
enforceCommandGap's ~1 ms. Payload size is nearly free; a second transactionis not.
Side effect: position-succeeds-while-Moving-fails can no longer happen, so the dwell's
leniency for that case is gone.
Movingis still only consulted from the second poll onward(it takes ~2 ms to rise after a goal write), just read earlier for free.
3. Doc reorganization
Every resource doc now runs config attributes → command reference → feature and limitation
prose. Previously
arm.mdput ~220 lines of explanation between its attributes table and itsDoCommandsection.arm.mdDoCommand+ servo commands above calibration priority, motion, orientation, manual mode, communicationgripper.mdDoCommandabove the TCP reference frame, communication, arm-dependency notescalibration.mdsimulated.mdDoCommandabove Behavior in the simulated-arm sectionteleop.mdPure reorders — verified by comparing the sorted line sets before and after. The only prose
edits are three positional references that inverted ("see below" pointing at a section that
moved above), now anchor links.
Scope
Reads were ~0.6 s of nod's ~2.6 s excess. The larger term — base motion at 1.29× the
recording — is untouched and still unexplained. Expect this to help the stutter (less
contention with gripper writes) more than the slow; unverified on hardware.
Second commit is a complexity pass: shared decode via
busSession.jointRadians, registeroffsets out of the per-call path, one redundant test folded down.
🤖 Generated with Claude Code