Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions canonical_port_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package so_arm

import (
"os"
"path/filepath"
"testing"
)

func TestCanonicalPortKey(t *testing.T) {
dir := t.TempDir()
device := filepath.Join(dir, "ttyUSB0")
if err := os.WriteFile(device, nil, 0o600); err != nil {
t.Fatalf("create fake device: %v", err)
}
link := filepath.Join(dir, "by-id-usb-serial")
if err := os.Symlink(device, link); err != nil {
t.Fatalf("symlink: %v", err)
}

// A symlink and its target resolve to the same canonical key.
if got, want := canonicalPortKey(link), canonicalPortKey(device); got != want {
t.Errorf("symlink and target should canonicalize equal: %q vs %q", got, want)
}

// An unresolvable path falls back to the original string (best-effort).
missing := filepath.Join(dir, "does-not-exist")
if got := canonicalPortKey(missing); got != missing {
t.Errorf("unresolvable path should fall back to original: got %q, want %q", got, missing)
}
}

// TestRegistry_SymlinkPortSharesController verifies that a consumer acquiring a
// port via a /dev/serial/by-id-style symlink reuses the controller a prior
// consumer created via the real device path, instead of opening a second bus
// on the same UART. Regression guard for the silent double-open footgun.
func TestRegistry_SymlinkPortSharesController(t *testing.T) {
dir := t.TempDir()
device := filepath.Join(dir, "ttyUSB0")
if err := os.WriteFile(device, nil, 0o600); err != nil {
t.Fatalf("create fake device: %v", err)
}
link := filepath.Join(dir, "by-id-usb-serial")
if err := os.Symlink(device, link); err != nil {
t.Fatalf("symlink: %v", err)
}

realKey := canonicalPortKey(device)

registry := NewControllerRegistry()
// Pre-inject an entry keyed by the canonical device path, as if the arm
// acquired first via the real path. No live bus is needed here:
// getExistingController touches no bus method on the shared-acquire path.
registry.entries[realKey] = &ControllerEntry{
controller: &SafeSoArmController{},
config: testConfig(realKey),
calibration: DefaultSO101FullCalibration,
refCount: 1,
}

// A second consumer acquires the same device via the symlink spelling.
if _, err := registry.GetController(link, testConfig(link), DefaultSO101FullCalibration, false); err != nil {
t.Fatalf("acquire via symlink: %v", err)
}

// It must reuse the existing entry rather than create a second one...
if n := len(registry.entries); n != 1 {
t.Errorf("expected 1 shared registry entry, got %d", n)
}
// ...bumping the shared refcount instead of opening a second bus.
if rc := registry.entries[realKey].refCount; rc != 2 {
t.Errorf("expected refCount 2 after shared acquire, got %d", rc)
}
}
2 changes: 1 addition & 1 deletion manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ func configsEqual(a, b *SoArm101Config) bool {
if a == nil || b == nil {
return false
}
return a.Port == b.Port &&
return canonicalPortKey(a.Port) == canonicalPortKey(b.Port) &&
a.Baudrate == b.Baudrate &&
a.Timeout == b.Timeout
}
Expand Down
32 changes: 31 additions & 1 deletion registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package so_arm
import (
"context"
"fmt"
"path/filepath"
"runtime"
"strings"
"sync"
Expand All @@ -12,6 +13,24 @@ import (
"github.com/hipsterbrown/feetech-servo/feetech"
)

// canonicalPortKey resolves a serial-port path to a stable device identity so
// that two spellings of the same physical port — e.g. "/dev/ttyUSB0" and a
// "/dev/serial/by-id/usb-..." symlink pointing at it — map to a single shared
// controller instead of two feetech buses contending for one UART.
//
// It is best-effort: filepath.EvalSymlinks resolves symlinks and cleans the
// path, but if the path can't be resolved (device not present yet, or a
// platform without symlink semantics) the original string is returned so the
// caller still gets a usable key. The result is used as the registry map key
// and for port-equality comparisons; the bus is still opened with the caller's
// original config.Port, so an unresolved fallback preserves today's behavior.
func canonicalPortKey(port string) string {
if resolved, err := filepath.EvalSymlinks(port); err == nil {
return resolved
}
return port
}

type ControllerEntry struct {
controller *SafeSoArmController
config *SoArm101Config
Expand All @@ -38,6 +57,13 @@ func NewControllerRegistry() *ControllerRegistry {
}

func (r *ControllerRegistry) GetController(portPath string, config *SoArm101Config, calibration SO101FullCalibration, fromFile bool) (*SafeSoArmController, error) {
// Collapse different spellings of the same device (e.g. /dev/ttyUSB0 and a
// /dev/serial/by-id symlink) to one registry key so both consumers share a
// single bus instead of contending. Only the key is normalized -- the caller's
// config is left untouched (it may be read concurrently via GetControllerStatus),
// so configsEqual and trackCaller canonicalize port strings at comparison time.
portPath = canonicalPortKey(portPath)

r.mu.RLock()
entry, exists := r.entries[portPath]
r.mu.RUnlock()
Expand Down Expand Up @@ -93,7 +119,9 @@ func (r *ControllerRegistry) getExistingController(entry *ControllerEntry, confi
}

atomic.AddInt64(&entry.refCount, 1)
r.trackCaller(entry.config.Port)
// Track against the canonical key so releaseFromCaller can find the entry
// even when this consumer spelled the port differently than the creator.
r.trackCaller(canonicalPortKey(entry.config.Port))

return &SafeSoArmController{
bus: entry.controller.bus,
Expand Down Expand Up @@ -316,6 +344,8 @@ func (r *ControllerRegistry) GetControllerStatus(portPath string) (int64, bool,
}

func (r *ControllerRegistry) GetCurrentCalibration(portPath string) SO101FullCalibration {
portPath = canonicalPortKey(portPath)

r.mu.RLock()
entry, exists := r.entries[portPath]
r.mu.RUnlock()
Expand Down