From 4e7a20f7ec4d2d671799d2c81efdbb743b0a39b1 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sun, 17 May 2026 11:09:26 +0200 Subject: [PATCH] refactor(soundtouch-web): make DeviceConnection.Status atomically swappable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Status was a value-typed DeviceStatus field on DeviceConnection, written from the periodic poller (UpdateDeviceStatus) and from four WebSocket event handlers (OnNowPlaying, OnVolumeUpdated, OnConnectionState, OnPresetUpdated) while being read from every HTTP handler and the WebSocket broadcaster. The struct was 8+ words wide with time.Time and string members, so concurrent readers could observe torn fields or mixed-update snapshots. The map-level race was fixed in the previous commit; this one closes the per-connection struct race. Hide the field behind atomic.Pointer[DeviceStatus]: Status() // returns current snapshot SetStatus(*DeviceStatus) // wholesale replace UpdateStatus(func(*DeviceStatus)) // CAS retry loop NewDeviceConnection constructs a connection with the atomic pointer pre-initialised, so Status() never returns nil for callers that go through the constructor (the old struct-literal pattern is no longer possible because the status field is now private). UpdateDeviceStatus runs network fetches into local vars first, then batches them into a single UpdateStatus call so the CAS loop only retries the merge — not the slow IO. WebSocket event handlers and the connect/disconnect transitions each use UpdateStatus, so any ordering of poller + event delivery converges to a consistent status. The UpdateStatus docstring is explicit about the shallow-copy contract: nested pointer fields (NowPlaying, Volume, Bass, Presets, Sources) MUST be replaced, not mutated through, because the copy mut receives shares those pointers with the prior snapshot. All production callers already follow this pattern (every value comes fresh from the device API). Tests: - types_test.go: migrated literal struct to NewDeviceConnection + SetStatus, switched reads to Status(). - status_test.go (new): six tests covering constructor init, SetStatus replacement semantics, UpdateStatus mutator application, field preservation across UpdateStatus, snapshot isolation (old snapshot stable under later writes), and a concurrent stress test (16 writers + 32 readers x 200 ops) that runs under -race. - handlers_test.go, registry_test.go, spa_test.go: migrated to constructor. Not addressed by this commit: - DeviceConnection.WebSocket (set once in ConnectDeviceWebSocket, read elsewhere). Word-sized pointer, atomic at the hardware level on amd64/arm64; race detector may still flag. - DeviceConnection.LastSeen (written under devicesMu by the registry, read outside that lock via DeviceSnapshot consumers). time.Time is non-atomic but the read is cosmetic. Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/soundtouch-web/handlers/handlers.go | 6 +- cmd/soundtouch-web/handlers/handlers_test.go | 26 +-- cmd/soundtouch-web/handlers/registry_test.go | 8 +- cmd/soundtouch-web/handlers/websocket.go | 135 ++++++++----- cmd/soundtouch-web/main.go | 10 +- cmd/soundtouch-web/spa_test.go | 9 +- cmd/soundtouch-web/webtypes/status_test.go | 197 +++++++++++++++++++ cmd/soundtouch-web/webtypes/types.go | 71 ++++++- cmd/soundtouch-web/webtypes/types_test.go | 29 ++- 9 files changed, 382 insertions(+), 109 deletions(-) create mode 100644 cmd/soundtouch-web/webtypes/status_test.go diff --git a/cmd/soundtouch-web/handlers/handlers.go b/cmd/soundtouch-web/handlers/handlers.go index d41d2f9..a5e4325 100644 --- a/cmd/soundtouch-web/handlers/handlers.go +++ b/cmd/soundtouch-web/handlers/handlers.go @@ -134,7 +134,7 @@ func (app *WebApp) HandleAPIDevices(w http.ResponseWriter, _ *http.Request) { for _, entry := range snapshot { devices[entry.ID] = map[string]interface{}{ "info": entry.Device.DeviceInfo, - "status": entry.Device.Status, + "status": entry.Device.Status(), "lastSeen": entry.Device.LastSeen, } } @@ -177,7 +177,7 @@ func (app *WebApp) HandleAPIDevice(w http.ResponseWriter, r *http.Request) { Success: true, Data: map[string]interface{}{ "info": device.DeviceInfo, - "status": device.Status, + "status": device.Status(), }, } @@ -539,7 +539,7 @@ func (app *WebApp) BroadcastDeviceList() { for _, entry := range snapshot { devices[entry.ID] = map[string]interface{}{ "info": entry.Device.DeviceInfo, - "status": entry.Device.Status, + "status": entry.Device.Status(), "lastSeen": entry.Device.LastSeen, } } diff --git a/cmd/soundtouch-web/handlers/handlers_test.go b/cmd/soundtouch-web/handlers/handlers_test.go index b0c6fcb..5945ae2 100644 --- a/cmd/soundtouch-web/handlers/handlers_test.go +++ b/cmd/soundtouch-web/handlers/handlers_test.go @@ -28,17 +28,13 @@ func createTestApp() *WebApp { }, } - device := &webtypes.DeviceConnection{ - Client: nil, // No real client for unit tests - DeviceInfo: deviceInfo, - LastSeen: time.Now(), - Status: webtypes.DeviceStatus{ - Volume: &models.Volume{ActualVolume: 50, MuteEnabled: false}, - Bass: &models.Bass{ActualBass: 0}, - IsConnected: true, - LastActivity: time.Now(), - }, - } + device := webtypes.NewDeviceConnection(nil, deviceInfo) + device.SetStatus(&webtypes.DeviceStatus{ + Volume: &models.Volume{ActualVolume: 50, MuteEnabled: false}, + Bass: &models.Bass{ActualBass: 0}, + IsConnected: true, + LastActivity: time.Now(), + }) app.AddDevice("test-device", device) return app @@ -545,11 +541,9 @@ func BenchmarkHandleAPIDevices(b *testing.B) { // Add more devices for realistic benchmarking for i := 0; i < 10; i++ { deviceID := "device-" + string(rune('0'+i)) - app.AddDevice(deviceID, &webtypes.DeviceConnection{ - Client: &client.Client{}, - DeviceInfo: &models.DeviceInfo{Name: "Test Device " + deviceID}, - Status: webtypes.DeviceStatus{IsConnected: true}, - }) + conn := webtypes.NewDeviceConnection(&client.Client{}, &models.DeviceInfo{Name: "Test Device " + deviceID}) + conn.SetStatus(&webtypes.DeviceStatus{IsConnected: true}) + app.AddDevice(deviceID, conn) } req := httptest.NewRequest("GET", "/api/devices", nil) diff --git a/cmd/soundtouch-web/handlers/registry_test.go b/cmd/soundtouch-web/handlers/registry_test.go index 20d87b4..75a67c4 100644 --- a/cmd/soundtouch-web/handlers/registry_test.go +++ b/cmd/soundtouch-web/handlers/registry_test.go @@ -13,10 +13,10 @@ import ( ) func newRegistryDevice(name string) *webtypes.DeviceConnection { - return &webtypes.DeviceConnection{ - DeviceInfo: &models.DeviceInfo{Name: name}, - Status: webtypes.DeviceStatus{IsConnected: true}, - } + conn := webtypes.NewDeviceConnection(nil, &models.DeviceInfo{Name: name}) + conn.SetStatus(&webtypes.DeviceStatus{IsConnected: true}) + + return conn } func TestAddDevice_Inserts(t *testing.T) { diff --git a/cmd/soundtouch-web/handlers/websocket.go b/cmd/soundtouch-web/handlers/websocket.go index b03ca3e..ceac73e 100644 --- a/cmd/soundtouch-web/handlers/websocket.go +++ b/cmd/soundtouch-web/handlers/websocket.go @@ -41,7 +41,7 @@ func (app *WebApp) HandleWebSocket(w http.ResponseWriter, r *http.Request) { for _, entry := range snapshot { devices[entry.ID] = map[string]interface{}{ "info": entry.Device.DeviceInfo, - "status": entry.Device.Status, + "status": entry.Device.Status(), "lastSeen": entry.Device.LastSeen, } } @@ -91,11 +91,12 @@ func (app *WebApp) HandleWebSocket(w http.ResponseWriter, r *http.Request) { // Send periodic status updates for _, entry := range app.DeviceSnapshot() { - if entry.Device.Status.IsConnected { + status := entry.Device.Status() + if status.IsConnected { statusMessage := webtypes.WebSocketMessage{ Type: "status_update", DeviceID: entry.ID, - Data: entry.Device.Status, + Data: status, } if err := conn.WriteJSON(statusMessage); err != nil { @@ -136,25 +137,35 @@ func (app *WebApp) ConnectDeviceWebSocket(deviceID string, conn *webtypes.Device wsClient := conn.Client.NewWebSocketClient(nil) - // Setup event handlers + // Setup event handlers. Each handler funnels its change through + // UpdateStatus so concurrent events and the periodic poller + // (UpdateDeviceStatus) cannot lose each other's writes. wsClient.OnNowPlaying(func(event *models.NowPlayingUpdatedEvent) { - conn.Status.NowPlaying = &event.NowPlaying - conn.Status.LastActivity = time.Now() + conn.UpdateStatus(func(s *webtypes.DeviceStatus) { + s.NowPlaying = &event.NowPlaying + s.LastActivity = time.Now() + }) }) wsClient.OnVolumeUpdated(func(event *models.VolumeUpdatedEvent) { - conn.Status.Volume = &event.Volume - conn.Status.LastActivity = time.Now() + conn.UpdateStatus(func(s *webtypes.DeviceStatus) { + s.Volume = &event.Volume + s.LastActivity = time.Now() + }) }) wsClient.OnConnectionState(func(event *models.ConnectionStateUpdatedEvent) { - conn.Status.IsConnected = event.ConnectionState.IsConnected() - conn.Status.LastActivity = time.Now() + conn.UpdateStatus(func(s *webtypes.DeviceStatus) { + s.IsConnected = event.ConnectionState.IsConnected() + s.LastActivity = time.Now() + }) }) wsClient.OnPresetUpdated(func(event *models.PresetUpdatedEvent) { - conn.Status.Presets = &event.Presets - conn.Status.LastActivity = time.Now() + conn.UpdateStatus(func(s *webtypes.DeviceStatus) { + s.Presets = &event.Presets + s.LastActivity = time.Now() + }) }) // Connect WebSocket @@ -164,64 +175,81 @@ func (app *WebApp) ConnectDeviceWebSocket(deviceID string, conn *webtypes.Device } conn.WebSocket = wsClient - conn.Status.IsConnected = true + + conn.UpdateStatus(func(s *webtypes.DeviceStatus) { + s.IsConnected = true + }) log.Printf("WebSocket connected for device %s", deviceID) // Wait for disconnection wsClient.Wait() - conn.Status.IsConnected = false + conn.UpdateStatus(func(s *webtypes.DeviceStatus) { + s.IsConnected = false + }) log.Printf("WebSocket disconnected for device %s", deviceID) } -// UpdateDeviceStatus fetches current status from device +// UpdateDeviceStatus fetches current status from the device. +// +// Network calls run outside the atomic merge so the CAS loop in +// UpdateStatus stays fast and doesn't retry slow IO. WebSocket event +// handlers running concurrently are not lost: their UpdateStatus +// runs against whichever snapshot they observe, and the merge below +// sees their changes when it CAS-loops onto the latest status. func (app *WebApp) UpdateDeviceStatus(_ string, conn *webtypes.DeviceConnection) { // Skip status update if client is not available (e.g., in tests) if conn.Client == nil { return } - statusUpdated := false + // Phase 1: slow network fetches. Local vars only, no shared state + // is touched yet. Errors are recorded so the merge below can tell + // "field N stayed unchanged" apart from "field N got refreshed". + nowPlaying, nowPlayingErr := conn.Client.GetNowPlaying() + volume, volumeErr := conn.Client.GetVolume() + presets, presetsErr := conn.Client.GetPresets() + sources, sourcesErr := conn.Client.GetSources() + bass, bassErr := conn.Client.GetBass() - // Get current now playing - if nowPlaying, err := conn.Client.GetNowPlaying(); err == nil { - conn.Status.NowPlaying = nowPlaying - statusUpdated = true - } + // Phase 2: fast merge. Only fields we successfully fetched + // overwrite; everything else keeps the value other goroutines may + // have just written. + conn.UpdateStatus(func(s *webtypes.DeviceStatus) { + statusUpdated := false - // Get current volume - if volume, err := conn.Client.GetVolume(); err == nil { - conn.Status.Volume = volume - statusUpdated = true - } + if nowPlayingErr == nil { + s.NowPlaying = nowPlaying + statusUpdated = true + } - // Get presets - if presets, err := conn.Client.GetPresets(); err == nil { - conn.Status.Presets = presets - statusUpdated = true - } + if volumeErr == nil { + s.Volume = volume + statusUpdated = true + } - // Update last activity if any status was updated - if statusUpdated { - conn.Status.LastActivity = time.Now() - } - // Get sources - if sources, err := conn.Client.GetSources(); err == nil { - conn.Status.Sources = sources - statusUpdated = true - } + if presetsErr == nil { + s.Presets = presets + statusUpdated = true + } - // Get bass (if available) - if bass, err := conn.Client.GetBass(); err == nil { - conn.Status.Bass = bass - statusUpdated = true - } + if sourcesErr == nil { + s.Sources = sources + statusUpdated = true + } - // Mark as connected if we successfully got at least one status - conn.Status.IsConnected = statusUpdated - conn.Status.LastActivity = time.Now() + if bassErr == nil { + s.Bass = bass + statusUpdated = true + } + + // Mark as connected if we successfully got at least one + // status from this round. Mirrors prior behaviour. + s.IsConnected = statusUpdated + s.LastActivity = time.Now() + }) } // HandleDeviceWebSocket handles individual device WebSocket connections for real-time device-specific updates @@ -253,7 +281,7 @@ func (app *WebApp) HandleDeviceWebSocket(w http.ResponseWriter, r *http.Request) DeviceID: deviceID, Data: map[string]interface{}{ "info": device.DeviceInfo, - "status": device.Status, + "status": device.Status(), }, } @@ -295,12 +323,13 @@ func (app *WebApp) HandleDeviceWebSocket(w http.ResponseWriter, r *http.Request) } // Send device status update + status := device.Status() statusMessage := webtypes.WebSocketMessage{ Type: "device_status", DeviceID: deviceID, Data: map[string]interface{}{ "info": device.DeviceInfo, - "status": device.Status, + "status": status, }, } @@ -311,13 +340,13 @@ func (app *WebApp) HandleDeviceWebSocket(w http.ResponseWriter, r *http.Request) // If device has active WebSocket connection to SoundTouch device, // also send any real-time updates from that connection - if device.WebSocket != nil && device.Status.IsConnected { + if device.WebSocket != nil && status.IsConnected { realtimeMessage := webtypes.WebSocketMessage{ Type: "device_realtime", DeviceID: deviceID, Data: map[string]interface{}{ - "nowPlaying": device.Status.NowPlaying, - "volume": device.Status.Volume, + "nowPlaying": status.NowPlaying, + "volume": status.Volume, "timestamp": time.Now(), }, } diff --git a/cmd/soundtouch-web/main.go b/cmd/soundtouch-web/main.go index 1606fac..4ca8356 100644 --- a/cmd/soundtouch-web/main.go +++ b/cmd/soundtouch-web/main.go @@ -234,15 +234,7 @@ func addDevice(app *handlers.WebApp, host string, port int, source string) { return } - conn := &webtypes.DeviceConnection{ - Client: c, - DeviceInfo: info, - LastSeen: time.Now(), - Status: webtypes.DeviceStatus{ - IsConnected: false, - LastActivity: time.Now(), - }, - } + conn := webtypes.NewDeviceConnection(c, info) if !app.AddDevice(host, conn) { // Lost a race — another goroutine inserted the same host // between TouchDevice and AddDevice. AddDevice bumped LastSeen diff --git a/cmd/soundtouch-web/spa_test.go b/cmd/soundtouch-web/spa_test.go index 867d24d..51645c5 100644 --- a/cmd/soundtouch-web/spa_test.go +++ b/cmd/soundtouch-web/spa_test.go @@ -7,7 +7,6 @@ import ( "net/http/httptest" "strings" "testing" - "time" "github.com/gesellix/bose-soundtouch/cmd/soundtouch-web/handlers" "github.com/gesellix/bose-soundtouch/cmd/soundtouch-web/webtypes" @@ -250,12 +249,8 @@ func TestControlAPIValidation(t *testing.T) { } // Add a mock device for testing unknown action validation - mockDevice := &webtypes.DeviceConnection{ - Client: nil, - DeviceInfo: &models.DeviceInfo{Name: "Test Device"}, - LastSeen: time.Now(), - Status: webtypes.DeviceStatus{IsConnected: true}, - } + mockDevice := webtypes.NewDeviceConnection(nil, &models.DeviceInfo{Name: "Test Device"}) + mockDevice.SetStatus(&webtypes.DeviceStatus{IsConnected: true}) app.AddDevice("testdevice", mockDevice) for _, tt := range tests { diff --git a/cmd/soundtouch-web/webtypes/status_test.go b/cmd/soundtouch-web/webtypes/status_test.go new file mode 100644 index 0000000..a7fb6ed --- /dev/null +++ b/cmd/soundtouch-web/webtypes/status_test.go @@ -0,0 +1,197 @@ +// Package webtypes tests for the atomic Status API on DeviceConnection +// (Status, SetStatus, UpdateStatus, NewDeviceConnection). +package webtypes + +import ( + "fmt" + "sync" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/models" +) + +func TestNewDeviceConnection_InitialStatus(t *testing.T) { + conn := NewDeviceConnection(nil, &models.DeviceInfo{Name: "test"}) + + status := conn.Status() + if status == nil { + t.Fatal("Status() returned nil from a NewDeviceConnection") + } + + if status.IsConnected { + t.Error("IsConnected should default to false") + } + + if status.LastActivity.IsZero() { + t.Error("LastActivity should be initialised, got zero time") + } +} + +func TestSetStatus_ReplacesEntireStatus(t *testing.T) { + conn := NewDeviceConnection(nil, &models.DeviceInfo{Name: "test"}) + conn.SetStatus(&DeviceStatus{ + Volume: &models.Volume{ActualVolume: 42}, + IsConnected: true, + }) + + got := conn.Status() + if got.Volume == nil || got.Volume.ActualVolume != 42 { + t.Errorf("Volume not stored: got %+v", got.Volume) + } + + // Setting a sparser status should wipe previously-set fields. + conn.SetStatus(&DeviceStatus{IsConnected: false}) + + got = conn.Status() + if got.Volume != nil { + t.Error("SetStatus did not wipe previously-set Volume") + } + + if got.IsConnected { + t.Error("SetStatus did not wipe IsConnected") + } +} + +func TestUpdateStatus_AppliesMutator(t *testing.T) { + conn := NewDeviceConnection(nil, &models.DeviceInfo{Name: "test"}) + + conn.UpdateStatus(func(s *DeviceStatus) { + s.IsConnected = true + s.Volume = &models.Volume{ActualVolume: 30} + }) + + got := conn.Status() + if !got.IsConnected { + t.Error("UpdateStatus did not set IsConnected") + } + + if got.Volume == nil || got.Volume.ActualVolume != 30 { + t.Errorf("UpdateStatus did not set Volume: %+v", got.Volume) + } +} + +func TestUpdateStatus_PreservesUnchangedFields(t *testing.T) { + conn := NewDeviceConnection(nil, &models.DeviceInfo{Name: "test"}) + conn.SetStatus(&DeviceStatus{ + Volume: &models.Volume{ActualVolume: 10}, + Bass: &models.Bass{ActualBass: 3}, + IsConnected: true, + }) + + // Only touch Volume; Bass and IsConnected must survive. + conn.UpdateStatus(func(s *DeviceStatus) { + s.Volume = &models.Volume{ActualVolume: 99} + }) + + got := conn.Status() + if got.Volume.ActualVolume != 99 { + t.Errorf("Volume = %d, want 99", got.Volume.ActualVolume) + } + + if got.Bass == nil || got.Bass.ActualBass != 3 { + t.Errorf("Bass not preserved: %+v", got.Bass) + } + + if !got.IsConnected { + t.Error("IsConnected not preserved") + } +} + +func TestStatusSnapshotIsolation(t *testing.T) { + // A snapshot returned by Status() must NOT change when a later + // UpdateStatus replaces a pointer field. This proves the atomic + // store gives readers a stable view (so long as the writer + // follows the docstring contract of replacing nested pointers). + conn := NewDeviceConnection(nil, &models.DeviceInfo{Name: "test"}) + conn.SetStatus(&DeviceStatus{Volume: &models.Volume{ActualVolume: 1}}) + + first := conn.Status() + + conn.UpdateStatus(func(s *DeviceStatus) { + s.Volume = &models.Volume{ActualVolume: 2} + }) + + if first.Volume.ActualVolume != 1 { + t.Errorf("Snapshot mutated after later UpdateStatus: got %d, want 1", + first.Volume.ActualVolume) + } + + if conn.Status().Volume.ActualVolume != 2 { + t.Errorf("Current status not updated: got %d, want 2", + conn.Status().Volume.ActualVolume) + } +} + +// TestStatusConcurrent runs many UpdateStatus writers alongside many +// Status() readers. Before atomic.Pointer[DeviceStatus] this pattern +// would be flagged by the race detector (writers mutate +// conn.Status.X while readers copy conn.Status). With the atomic +// pointer it must run clean under `go test -race`. +func TestStatusConcurrent(t *testing.T) { + conn := NewDeviceConnection(nil, &models.DeviceInfo{Name: "concurrent"}) + + const writers = 16 + + const readersPerKind = 16 + + const opsPerGoroutine = 200 + + var wg sync.WaitGroup + wg.Add(writers + 2*readersPerKind) + + // Writers: each goroutine replaces NowPlaying with a fresh struct + // carrying its worker id. Replacement (not in-place mutation) + // is what the UpdateStatus contract requires for nested + // pointers. + for w := 0; w < writers; w++ { + go func(worker int) { + defer wg.Done() + + for i := 0; i < opsPerGoroutine; i++ { + conn.UpdateStatus(func(s *DeviceStatus) { + s.NowPlaying = &models.NowPlaying{ + Track: fmt.Sprintf("w%d-%d", worker, i), + } + s.IsConnected = true + }) + } + }(w) + } + + // Readers via Status() — full snapshot. + for r := 0; r < readersPerKind; r++ { + go func() { + defer wg.Done() + + for i := 0; i < opsPerGoroutine; i++ { + _ = conn.Status() + } + }() + } + + // Readers that deref a single field. Tests the common + // "device.Status().IsConnected" pattern. + for r := 0; r < readersPerKind; r++ { + go func() { + defer wg.Done() + + for i := 0; i < opsPerGoroutine; i++ { + _ = conn.Status().IsConnected + } + }() + } + + wg.Wait() + + // After all writers finish, IsConnected should be true (every + // writer sets it). The exact NowPlaying value is whichever + // writer landed last, but it must be a valid non-nil pointer. + final := conn.Status() + if !final.IsConnected { + t.Error("IsConnected should be true after writers ran") + } + + if final.NowPlaying == nil { + t.Error("NowPlaying should be non-nil after writers ran") + } +} diff --git a/cmd/soundtouch-web/webtypes/types.go b/cmd/soundtouch-web/webtypes/types.go index 69eceac..b47f1b0 100644 --- a/cmd/soundtouch-web/webtypes/types.go +++ b/cmd/soundtouch-web/webtypes/types.go @@ -2,6 +2,7 @@ package webtypes import ( + "sync/atomic" "time" "github.com/gesellix/bose-soundtouch/pkg/client" @@ -29,13 +30,21 @@ type SoundTouchClient interface { NewWebSocketClient(config interface{}) *client.WebSocketClient } -// DeviceConnection wraps a SoundTouch client with WebSocket connection +// DeviceConnection wraps a SoundTouch client with WebSocket connection. +// +// The Status field is stored behind atomic.Pointer so concurrent +// readers (HTTP handlers, WebSocket broadcasters) never observe a +// torn struct while a writer (UpdateDeviceStatus, WebSocket event +// handlers) is mid-update. Access status through Status / SetStatus +// / UpdateStatus rather than the private field; construct connections +// via NewDeviceConnection to guarantee the status is initialised. type DeviceConnection struct { Client *client.Client WebSocket *client.WebSocketClient DeviceInfo *models.DeviceInfo LastSeen time.Time - Status DeviceStatus + + status atomic.Pointer[DeviceStatus] } // DeviceStatus represents the current device state @@ -49,6 +58,64 @@ type DeviceStatus struct { LastActivity time.Time `json:"lastActivity"` } +// NewDeviceConnection creates a fully-initialised connection. The +// status starts with IsConnected=false and LastActivity set to now; +// real values arrive via UpdateStatus once the device responds. +func NewDeviceConnection(c *client.Client, info *models.DeviceInfo) *DeviceConnection { + conn := &DeviceConnection{ + Client: c, + DeviceInfo: info, + LastSeen: time.Now(), + } + conn.status.Store(&DeviceStatus{ + IsConnected: false, + LastActivity: time.Now(), + }) + + return conn +} + +// Status returns a snapshot of the current device status. The returned +// pointer is read-only from the caller's perspective; mutating the +// pointed-to struct has no effect on the stored status. Use +// UpdateStatus or SetStatus to apply changes. Never returns nil for +// connections built via NewDeviceConnection. +func (c *DeviceConnection) Status() *DeviceStatus { + return c.status.Load() +} + +// SetStatus atomically replaces the entire status. Use sparingly — +// UpdateStatus is the preferred entry point because it preserves +// concurrent changes from other goroutines. +func (c *DeviceConnection) SetStatus(s *DeviceStatus) { + c.status.Store(s) +} + +// UpdateStatus atomically applies mut to a copy of the current status +// and stores the result. If another goroutine updates the status while +// mut runs, UpdateStatus retries with the newer status — so concurrent +// writers cannot silently lose each other's changes. +// +// The copy mut receives is a shallow value copy of the previous status. +// Nested pointer fields (NowPlaying, Volume, Presets, Sources, Bass) +// share their backing struct with the previous version: callers MUST +// REPLACE these pointers (s.Volume = &models.Volume{...}) rather than +// mutate through them (s.Volume.ActualVolume++ would race with any +// reader still holding the previous snapshot). Production callers +// receive these values fresh from the device API, so this is the +// natural shape. +func (c *DeviceConnection) UpdateStatus(mut func(*DeviceStatus)) { + for { + old := c.status.Load() + next := *old + mut(&next) + + if c.status.CompareAndSwap(old, &next) { + return + } + } +} + // APIResponse is a standard JSON response wrapper type APIResponse struct { Success bool `json:"success"` diff --git a/cmd/soundtouch-web/webtypes/types_test.go b/cmd/soundtouch-web/webtypes/types_test.go index 7cd6b87..e93c3a1 100644 --- a/cmd/soundtouch-web/webtypes/types_test.go +++ b/cmd/soundtouch-web/webtypes/types_test.go @@ -145,31 +145,30 @@ func TestDeviceConnection(t *testing.T) { MuteEnabled: false, } - conn := &DeviceConnection{ - DeviceInfo: deviceInfo, - LastSeen: time.Now(), - Status: DeviceStatus{ - NowPlaying: nowPlaying, - Volume: volume, - IsConnected: true, - LastActivity: time.Now(), - }, - } + conn := NewDeviceConnection(nil, deviceInfo) + conn.SetStatus(&DeviceStatus{ + NowPlaying: nowPlaying, + Volume: volume, + IsConnected: true, + LastActivity: time.Now(), + }) t.Run("device connection fields", func(t *testing.T) { if conn.DeviceInfo.Name != "Test Speaker" { t.Errorf("Expected device name 'Test Speaker', got '%s'", conn.DeviceInfo.Name) } - if conn.Status.NowPlaying.Track != "Test Track" { - t.Errorf("Expected track 'Test Track', got '%s'", conn.Status.NowPlaying.Track) + status := conn.Status() + + if status.NowPlaying.Track != "Test Track" { + t.Errorf("Expected track 'Test Track', got '%s'", status.NowPlaying.Track) } - if conn.Status.Volume.ActualVolume != 50 { - t.Errorf("Expected volume 50, got %d", conn.Status.Volume.ActualVolume) + if status.Volume.ActualVolume != 50 { + t.Errorf("Expected volume 50, got %d", status.Volume.ActualVolume) } - if !conn.Status.IsConnected { + if !status.IsConnected { t.Error("Expected device to be connected") } })