fix(registry): key shared controller by canonical device path#26
Open
Nick Hehr (HipsterBrown) wants to merge 1 commit into
Open
fix(registry): key shared controller by canonical device path#26Nick Hehr (HipsterBrown) wants to merge 1 commit into
Nick Hehr (HipsterBrown) wants to merge 1 commit into
Conversation
The shared-controller registry keyed on the raw `port` string, so an arm configured as "/dev/ttyUSB0" and a gripper configured via a "/dev/serial/by-id/usb-..." symlink to the same device were treated as two different ports — opening two feetech buses that silently contend for one UART (garbled frames, intermittent timeouts). This is footgun #1 from the shared-controller architecture review (#25). Resolve the port to a stable device identity (filepath.EvalSymlinks, best-effort with fallback to the raw string) and use that as the registry map key. Only the key is normalized — the caller's config is left untouched because it can be read concurrently via GetControllerStatus (mutating it tripped the race detector) — so configsEqual and trackCaller canonicalize port strings at comparison time instead. Result: different spellings of one device collapse to a single shared bus. Tests (no hardware): canonicalPortKey resolves a symlink to its target and falls back for unresolvable paths; a consumer acquiring via a symlink reuses the entry created via the real path (one entry, refcount 2) rather than opening a second bus. Full package passes under -race, including the existing TestConcurrentRegistryAccess. Refs #25. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01JNvy6dGUT6wBHLNBv8YPYv
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.
First hardening step from the shared-controller architecture review (#25) — the highest-value, lowest-risk footgun.
Problem
The registry that lets the arm (servos 1-5) and gripper (servo 6) share one serial bus keys on the raw
portstring. An arm configured/dev/ttyUSB0and a gripper configured via a/dev/serial/by-id/usb-...symlink to the same device are treated as two different ports → twofeetech.NewBusconnections silently contending for one UART (garbled frames, intermittent timeouts). It fails at the hardware level with no error, which is the hardest kind to diagnose.Fix
Resolve the port to a stable device identity with
filepath.EvalSymlinks(best-effort: falls back to the raw string if unresolvable — device not present yet, or a platform without symlink semantics) and use that as the registry map key.Only the key is normalized — the caller's
configis deliberately not mutated, because it can be read concurrently viaGetControllerStatus(mutatingconfig.Porttripped the race detector inTestConcurrentRegistryAccess). Instead,configsEqualand the release-trackingtrackCallercanonicalize port strings at comparison time. Net effect: different spellings of one device collapse to a single shared bus, and a differing spelling no longer trips the "conflict: different config" check.The bus itself is still opened with the caller's original
config.Port(a valid path to the same device), so behavior is unchanged when paths are already canonical.Testing (no hardware)
TestCanonicalPortKey— a symlink and its target canonicalize equal; an unresolvable path falls back to the original string.TestRegistry_SymlinkPortSharesController— a consumer acquiring via aby-idsymlink reuses the entry created via the real device path (one registry entry, refcount 2) instead of opening a second bus.go test -race ./cmd/module/ .— fullso_armpackage green, including the existingTestConcurrentRegistryAccess.Scope
Independent of #24 (close-state race fix) — both branch off
mainand touch different parts ofregistry.go; whichever merges second is a trivial rebase. Addresses footgun #1 of #25; the lifecycle cleanup and conflict-diagnostics items there remain follow-ups.Refs #25.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JNvy6dGUT6wBHLNBv8YPYv