fix(local-agent): Rescan 序號身分比對替換 stale session + 鎖外 Disconnect
- serialIdentity() 三態比對（空/假序號 0x00000000 正規化為無身分、對齊 ADR-018 R2）：Rescan 對既存合成 ID 比對身分——同一顆保留 live session、 換位/拔插以新 info 替換 session，serial 索引重建即正確 - stale drivers 改釋放鎖後 Disconnect（不再阻塞 GetDevice 路由）+ Disconnect 錯誤改 WARNING log - 測試 seam：detectFn/newDriverFn 注入 + newSessionDriverLocked 統一建構 （Start 行為零變動）；新 4 支行為級測試（拔除位移/換位 rebind/ 同序號保 session/無身分回歸） - evidence：build/vet/test 全綠 + -race ok；review 通過 0C/0M/0Mi/4Sug （wp0-serial-routing-review.md「Minor #1 修復審查」章節） - WP-C 阻擋項解除；觀察項：多裝置實測留意 changed identity log （偵測順序抖動時解法在 detector 端排序） Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
b9ee184586
commit
12b1fe3bad
@ -34,6 +34,13 @@ type Manager struct {
|
||||
scriptPath string
|
||||
logBroadcaster *logger.Broadcaster
|
||||
mu sync.RWMutex
|
||||
|
||||
// detectFn / newDriverFn are seams so Rescan behavior (unplug / slot
|
||||
// swap of positional synthetic IDs) can be tested without spawning the
|
||||
// Python bridge. Production wiring (set in NewManager) uses the kneron
|
||||
// implementations.
|
||||
detectFn func(scriptPath string) []driver.DeviceInfo
|
||||
newDriverFn func(info driver.DeviceInfo, scriptPath string) driver.DeviceDriver
|
||||
}
|
||||
|
||||
func NewManager(registry *DriverRegistry, scriptPath string) *Manager {
|
||||
@ -43,6 +50,10 @@ func NewManager(registry *DriverRegistry, scriptPath string) *Manager {
|
||||
serialToLocalID: make(map[string]string),
|
||||
eventBus: make(chan DeviceEvent, 100),
|
||||
scriptPath: scriptPath,
|
||||
detectFn: kneron.DetectDevices,
|
||||
newDriverFn: func(info driver.DeviceInfo, scriptPath string) driver.DeviceDriver {
|
||||
return kneron.NewKneronDriver(info, scriptPath)
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@ -60,9 +71,23 @@ func (m *Manager) SetLogBroadcaster(b *logger.Broadcaster) {
|
||||
}
|
||||
}
|
||||
|
||||
// newSessionDriverLocked builds a driver for a detected device and attaches
|
||||
// the log broadcaster when the concrete driver supports it. Caller must hold
|
||||
// m.mu (write lock) — it only reads m.logBroadcaster/scriptPath, but is kept
|
||||
// locked-only for consistency with its callers.
|
||||
func (m *Manager) newSessionDriverLocked(info driver.DeviceInfo) driver.DeviceDriver {
|
||||
d := m.newDriverFn(info, m.scriptPath)
|
||||
if m.logBroadcaster != nil {
|
||||
if kd, ok := d.(*kneron.KneronDriver); ok {
|
||||
kd.SetLogBroadcaster(m.logBroadcaster)
|
||||
}
|
||||
}
|
||||
return d
|
||||
}
|
||||
|
||||
func (m *Manager) Start() {
|
||||
// Detect real Kneron devices (KL520, KL720, etc.) via Python bridge.
|
||||
devices := kneron.DetectDevices(m.scriptPath)
|
||||
devices := m.detectFn(m.scriptPath)
|
||||
if len(devices) == 0 {
|
||||
log.Println("No Kneron devices detected")
|
||||
return
|
||||
@ -71,20 +96,36 @@ func (m *Manager) Start() {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
for _, info := range devices {
|
||||
d := kneron.NewKneronDriver(info, m.scriptPath)
|
||||
if m.logBroadcaster != nil {
|
||||
d.SetLogBroadcaster(m.logBroadcaster)
|
||||
}
|
||||
m.sessions[info.ID] = NewSession(d)
|
||||
m.sessions[info.ID] = NewSession(m.newSessionDriverLocked(info))
|
||||
log.Printf("Registered Kneron device: %s (%s, type=%s)", info.Name, info.ID, info.Type)
|
||||
}
|
||||
m.rebuildSerialIndexLocked()
|
||||
}
|
||||
|
||||
// Rescan re-detects connected Kneron devices. New devices are registered,
|
||||
// removed devices are cleaned up, and existing devices are left untouched.
|
||||
// serialIdentity normalizes a serial for identity comparison: empty and the
|
||||
// fake pyusb-fallback serial both mean "no identity" (ADR-018 R2), so they
|
||||
// compare equal to each other and never equal to a real kn_number.
|
||||
func serialIdentity(serial string) string {
|
||||
if serial == fakeSerialNumber {
|
||||
return ""
|
||||
}
|
||||
return serial
|
||||
}
|
||||
|
||||
// Rescan re-detects connected Kneron devices. New devices are registered and
|
||||
// removed devices are cleaned up. For an existing synthetic ID the session is
|
||||
// kept only when the detected serial identity matches the session's; on a
|
||||
// mismatch the session is replaced with a fresh driver built from the
|
||||
// detected info.
|
||||
//
|
||||
// Why the identity check (WP-0 review Minor #1): synthetic IDs are
|
||||
// positional ("kl520-0" = first KL520 found). With multiple same-chip
|
||||
// dongles, unplugging or re-ordering shifts which physical device occupies a
|
||||
// slot. Keeping the old session would make rebuildSerialIndexLocked() index
|
||||
// a stale serial → the serial route would hit a removed/relocated device.
|
||||
// The index must always reflect what detection currently sees.
|
||||
func (m *Manager) Rescan() []driver.DeviceInfo {
|
||||
detected := kneron.DetectDevices(m.scriptPath)
|
||||
detected := m.detectFn(m.scriptPath)
|
||||
|
||||
// Build a set of detected device IDs.
|
||||
detectedIDs := make(map[string]driver.DeviceInfo, len(detected))
|
||||
@ -92,37 +133,60 @@ func (m *Manager) Rescan() []driver.DeviceInfo {
|
||||
detectedIDs[info.ID] = info
|
||||
}
|
||||
|
||||
// Drivers to disconnect after releasing the lock: a real KneronDriver
|
||||
// Disconnect can take seconds, and holding the write lock would block
|
||||
// every GetDevice (inference / flash / camera routing) meanwhile.
|
||||
type staleDriver struct {
|
||||
id string
|
||||
drv driver.DeviceDriver
|
||||
}
|
||||
var stale []staleDriver
|
||||
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
|
||||
// Remove devices that are no longer present.
|
||||
for id, s := range m.sessions {
|
||||
if _, exists := detectedIDs[id]; !exists {
|
||||
log.Printf("Device removed: %s", id)
|
||||
s.Driver.Disconnect()
|
||||
stale = append(stale, staleDriver{id: id, drv: s.Driver})
|
||||
delete(m.sessions, id)
|
||||
}
|
||||
}
|
||||
|
||||
// Add newly detected devices.
|
||||
// Register newly detected devices and replace sessions whose slot is now
|
||||
// occupied by a different physical device.
|
||||
for _, info := range detected {
|
||||
if _, exists := m.sessions[info.ID]; !exists {
|
||||
d := kneron.NewKneronDriver(info, m.scriptPath)
|
||||
if m.logBroadcaster != nil {
|
||||
d.SetLogBroadcaster(m.logBroadcaster)
|
||||
if s, exists := m.sessions[info.ID]; exists {
|
||||
old := s.Driver.Info().SerialNumber
|
||||
if serialIdentity(old) == serialIdentity(info.SerialNumber) {
|
||||
// Same physical device (or no identity on either side, e.g.
|
||||
// the single-dongle no-SDK demo) — keep the live session and
|
||||
// its connection state untouched.
|
||||
continue
|
||||
}
|
||||
m.sessions[info.ID] = NewSession(d)
|
||||
log.Printf("Registered Kneron device: %s (%s, type=%s)", info.Name, info.ID, info.Type)
|
||||
log.Printf("Device %s changed identity (serial %q -> %q); replacing session", info.ID, old, info.SerialNumber)
|
||||
stale = append(stale, staleDriver{id: info.ID, drv: s.Driver})
|
||||
delete(m.sessions, info.ID)
|
||||
}
|
||||
m.sessions[info.ID] = NewSession(m.newSessionDriverLocked(info))
|
||||
log.Printf("Registered Kneron device: %s (%s, type=%s)", info.Name, info.ID, info.Type)
|
||||
}
|
||||
|
||||
m.rebuildSerialIndexLocked()
|
||||
|
||||
// Return current list.
|
||||
// Snapshot current list while still holding the lock.
|
||||
devices := make([]driver.DeviceInfo, 0, len(m.sessions))
|
||||
for _, s := range m.sessions {
|
||||
devices = append(devices, s.Driver.Info())
|
||||
}
|
||||
m.mu.Unlock()
|
||||
|
||||
for _, sd := range stale {
|
||||
if err := sd.drv.Disconnect(); err != nil {
|
||||
log.Printf("WARNING: disconnect of stale device %s failed: %v", sd.id, err)
|
||||
}
|
||||
}
|
||||
|
||||
return devices
|
||||
}
|
||||
|
||||
|
||||
@ -7,8 +7,9 @@ import (
|
||||
)
|
||||
|
||||
type testDriver struct {
|
||||
info driver.DeviceInfo
|
||||
connected bool
|
||||
info driver.DeviceInfo
|
||||
connected bool
|
||||
disconnects int
|
||||
}
|
||||
|
||||
func (d *testDriver) Info() driver.DeviceInfo { return d.info }
|
||||
@ -19,6 +20,7 @@ func (d *testDriver) Connect() error {
|
||||
}
|
||||
func (d *testDriver) Disconnect() error {
|
||||
d.connected = false
|
||||
d.disconnects++
|
||||
d.info.Status = driver.StatusDisconnected
|
||||
return nil
|
||||
}
|
||||
@ -197,6 +199,162 @@ func TestManager_SerialIndex_SkipsEmptyAndFakeSerial(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// ==========================================================================
|
||||
// Rescan identity check (WP-0 review Minor #1): positional synthetic IDs vs
|
||||
// serial identity — the serial index must reflect current detection.
|
||||
// ==========================================================================
|
||||
|
||||
// newRescanManager returns a Manager whose detection and driver factory are
|
||||
// stubbed: Rescan sees whatever *detected currently holds and builds
|
||||
// testDrivers instead of spawning the Python bridge.
|
||||
func newRescanManager(detected *[]driver.DeviceInfo) *Manager {
|
||||
mgr := NewManager(NewRegistry(), "")
|
||||
mgr.detectFn = func(string) []driver.DeviceInfo { return *detected }
|
||||
mgr.newDriverFn = func(info driver.DeviceInfo, _ string) driver.DeviceDriver {
|
||||
return &testDriver{info: info}
|
||||
}
|
||||
return mgr
|
||||
}
|
||||
|
||||
func TestManager_Rescan_UnplugShiftsPositionalID(t *testing.T) {
|
||||
// Two same-chip dongles: kl520-0 = serial A, kl520-1 = serial B.
|
||||
detected := []driver.DeviceInfo{
|
||||
{ID: "kl520-0", Type: "KL520", SerialNumber: "0xAAAA0001"},
|
||||
{ID: "kl520-1", Type: "KL520", SerialNumber: "0xBBBB0002"},
|
||||
}
|
||||
mgr := newRescanManager(&detected)
|
||||
mgr.Rescan()
|
||||
|
||||
oldSession, err := mgr.GetDevice("kl520-0")
|
||||
if err != nil {
|
||||
t.Fatalf("GetDevice(kl520-0) error = %v", err)
|
||||
}
|
||||
oldDriver := oldSession.Driver.(*testDriver)
|
||||
|
||||
// Unplug A: detection now sees a single device, and the positional ID
|
||||
// kl520-0 is occupied by the dongle with serial B.
|
||||
detected = []driver.DeviceInfo{
|
||||
{ID: "kl520-0", Type: "KL520", SerialNumber: "0xBBBB0002"},
|
||||
}
|
||||
mgr.Rescan()
|
||||
|
||||
// Removed serial A must no longer be routable.
|
||||
if _, err := mgr.GetDevice("0xAAAA0001"); err == nil {
|
||||
t.Error("GetDevice(0xAAAA0001) expected error after unplug, got nil (stale serial route)")
|
||||
}
|
||||
// Serial B must route to its new positional slot kl520-0.
|
||||
s, err := mgr.GetDevice("0xBBBB0002")
|
||||
if err != nil {
|
||||
t.Fatalf("GetDevice(0xBBBB0002) error = %v", err)
|
||||
}
|
||||
if got := s.Driver.Info().ID; got != "kl520-0" {
|
||||
t.Errorf("serial 0xBBBB0002 resolved to %q, want kl520-0", got)
|
||||
}
|
||||
if got := s.Driver.Info().SerialNumber; got != "0xBBBB0002" {
|
||||
t.Errorf("session kl520-0 serial = %q, want 0xBBBB0002 (Device Info not refreshed)", got)
|
||||
}
|
||||
// The vacated positional slot kl520-1 must be gone.
|
||||
if _, err := mgr.GetDevice("kl520-1"); err == nil {
|
||||
t.Error("GetDevice(kl520-1) expected error after unplug, got nil")
|
||||
}
|
||||
// The replaced stale driver (old occupant of kl520-0) was disconnected.
|
||||
if oldDriver.disconnects == 0 {
|
||||
t.Error("stale driver for old kl520-0 occupant was not disconnected")
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_Rescan_SwapRebindsSerials(t *testing.T) {
|
||||
detected := []driver.DeviceInfo{
|
||||
{ID: "kl520-0", Type: "KL520", SerialNumber: "0xAAAA0001"},
|
||||
{ID: "kl520-1", Type: "KL520", SerialNumber: "0xBBBB0002"},
|
||||
}
|
||||
mgr := newRescanManager(&detected)
|
||||
mgr.Rescan()
|
||||
|
||||
// Replug in the opposite order: slots swap occupants.
|
||||
detected = []driver.DeviceInfo{
|
||||
{ID: "kl520-0", Type: "KL520", SerialNumber: "0xBBBB0002"},
|
||||
{ID: "kl520-1", Type: "KL520", SerialNumber: "0xAAAA0001"},
|
||||
}
|
||||
mgr.Rescan()
|
||||
|
||||
sA, err := mgr.GetDevice("0xAAAA0001")
|
||||
if err != nil {
|
||||
t.Fatalf("GetDevice(0xAAAA0001) error = %v", err)
|
||||
}
|
||||
if got := sA.Driver.Info().ID; got != "kl520-1" {
|
||||
t.Errorf("serial 0xAAAA0001 resolved to %q, want kl520-1 (swap not reflected)", got)
|
||||
}
|
||||
sB, err := mgr.GetDevice("0xBBBB0002")
|
||||
if err != nil {
|
||||
t.Fatalf("GetDevice(0xBBBB0002) error = %v", err)
|
||||
}
|
||||
if got := sB.Driver.Info().ID; got != "kl520-0" {
|
||||
t.Errorf("serial 0xBBBB0002 resolved to %q, want kl520-0 (swap not reflected)", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_Rescan_SameDeviceKeepsSession(t *testing.T) {
|
||||
// Single-dongle regression: same serial across rescans → session (and
|
||||
// its connection state) must be left untouched.
|
||||
detected := []driver.DeviceInfo{
|
||||
{ID: "kl520-0", Type: "KL520", SerialNumber: "0x1A2B3C4D"},
|
||||
}
|
||||
mgr := newRescanManager(&detected)
|
||||
mgr.Rescan()
|
||||
|
||||
go func() {
|
||||
for range mgr.Events() {
|
||||
}
|
||||
}()
|
||||
if err := mgr.Connect("kl520-0"); err != nil {
|
||||
t.Fatalf("Connect() error = %v", err)
|
||||
}
|
||||
before, _ := mgr.GetDevice("kl520-0")
|
||||
|
||||
mgr.Rescan()
|
||||
|
||||
after, err := mgr.GetDevice("kl520-0")
|
||||
if err != nil {
|
||||
t.Fatalf("GetDevice(kl520-0) after rescan error = %v", err)
|
||||
}
|
||||
if before != after {
|
||||
t.Error("session was replaced on rescan although the device did not change")
|
||||
}
|
||||
if td := after.Driver.(*testDriver); !td.connected {
|
||||
t.Error("connection state lost across rescan of an unchanged device")
|
||||
}
|
||||
if s, err := mgr.GetDevice("0x1A2B3C4D"); err != nil || s != after {
|
||||
t.Errorf("serial route after rescan = (%v, %v), want same session", s, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_Rescan_NoSerialSingleDeviceUntouched(t *testing.T) {
|
||||
// No-SDK demo regression: empty/fake serials carry no identity, so the
|
||||
// existing session must be kept (previous "left untouched" behavior).
|
||||
for _, serials := range [][2]string{{"", ""}, {fakeSerialNumber, fakeSerialNumber}, {"", fakeSerialNumber}} {
|
||||
detected := []driver.DeviceInfo{
|
||||
{ID: "kl520-0", Type: "KL520", SerialNumber: serials[0]},
|
||||
}
|
||||
mgr := newRescanManager(&detected)
|
||||
mgr.Rescan()
|
||||
before, _ := mgr.GetDevice("kl520-0")
|
||||
|
||||
detected = []driver.DeviceInfo{
|
||||
{ID: "kl520-0", Type: "KL520", SerialNumber: serials[1]},
|
||||
}
|
||||
mgr.Rescan()
|
||||
|
||||
after, err := mgr.GetDevice("kl520-0")
|
||||
if err != nil {
|
||||
t.Fatalf("serials %v: GetDevice(kl520-0) error = %v", serials, err)
|
||||
}
|
||||
if before != after {
|
||||
t.Errorf("serials %v: session replaced although neither serial carries identity", serials)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestManager_SerialIndex_RemovedDeviceUnroutable(t *testing.T) {
|
||||
mgr := NewManager(NewRegistry(), "")
|
||||
seedDevice(mgr, driver.DeviceInfo{ID: "kl520-0", SerialNumber: "0x1A2B3C4D"})
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user