From b01ab1e1bb6562d57a9fda0731d5bb0cc86ab86a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lipinsk=C3=BD?= <6032558+Mr-Tao@users.noreply.github.com> Date: Wed, 26 Aug 2026 03:09:01 +0200 Subject: [PATCH] fix(player): project stereo pairs as logical devices --- .../soundtouchweb/device_projection.go | 260 ++++++++++++++++++ .../soundtouchweb/device_projection_test.go | 181 ++++++++++++ pkg/service/soundtouchweb/handler.go | 26 +- pkg/service/soundtouchweb/static/css/app.css | 2 + pkg/service/soundtouchweb/static/js/app.js | 7 + .../static/js/components/DeviceList.js | 10 +- .../static/js/components/Library.js | 6 +- pkg/service/soundtouchweb/websocket.go | 59 ++-- pkg/service/soundtouchweb/websocket_test.go | 141 ++++++++++ .../soundtouchweb/webtypes/status_test.go | 79 ++++++ pkg/service/soundtouchweb/webtypes/types.go | 73 ++++- 11 files changed, 780 insertions(+), 64 deletions(-) create mode 100644 pkg/service/soundtouchweb/device_projection.go create mode 100644 pkg/service/soundtouchweb/device_projection_test.go create mode 100644 pkg/service/soundtouchweb/websocket_test.go diff --git a/pkg/service/soundtouchweb/device_projection.go b/pkg/service/soundtouchweb/device_projection.go new file mode 100644 index 00000000..213a3b42 --- /dev/null +++ b/pkg/service/soundtouchweb/device_projection.go @@ -0,0 +1,260 @@ +package soundtouchweb + +import ( + "strings" + "time" + + "github.com/gesellix/bose-soundtouch/pkg/models" + "github.com/gesellix/bose-soundtouch/pkg/service/soundtouchweb/webtypes" +) + +// deviceView is the player-facing representation of one control target. +// A stereo pair is projected as one target keyed by its master speaker's host; +// the underlying registry continues to track both physical speakers. +type deviceView struct { + Info *models.DeviceInfo `json:"info"` + Status *webtypes.DeviceStatus `json:"status"` + LastSeen time.Time `json:"lastSeen"` + StereoPair *stereoPairView `json:"stereoPair,omitempty"` +} + +// stereoPairView describes the physical members represented by a logical +// player target. Controls are always sent to MasterDeviceID via the map key. +type stereoPairView struct { + ID string `json:"id"` + Name string `json:"name,omitempty"` + MasterDeviceID string `json:"masterDeviceId"` + Status string `json:"status,omitempty"` + MemberCount int `json:"memberCount"` + AvailableMemberCount int `json:"availableMemberCount"` + Degraded bool `json:"degraded"` + Members []stereoPairMemberView `json:"members"` +} + +// stereoPairMemberView is the player-facing role and availability of one +// physical speaker in a stereo pair. +type stereoPairMemberView struct { + DeviceID string `json:"deviceId"` + Role string `json:"role"` + IPAddress string `json:"ipAddress,omitempty"` + Name string `json:"name,omitempty"` + Available bool `json:"available"` +} + +// deviceViewSnapshot projects the physical registry into logical control +// targets for the HTTP API and the global player WebSocket. +func (app *WebApp) deviceViewSnapshot() map[string]deviceView { + return projectDeviceEntries(app.DeviceSnapshot()) +} + +func projectDeviceEntries(snapshot []DeviceEntry) map[string]deviceView { + byDeviceID := make(map[string][]DeviceEntry, len(snapshot)) + for _, entry := range snapshot { + if entry.Device == nil || entry.Device.DeviceInfo == nil { + continue + } + + deviceID := strings.TrimSpace(entry.Device.DeviceInfo.DeviceID) + if deviceID != "" { + byDeviceID[deviceID] = append(byDeviceID[deviceID], entry) + } + } + + masters := make(map[string]*stereoPairView) + hidden := make(map[string]bool) + + for _, entry := range snapshot { + if entry.Device == nil || entry.Device.DeviceInfo == nil { + continue + } + + status := entry.Device.Status() + if status == nil || !validMasterGroup(entry.Device.DeviceInfo.DeviceID, status.Group) { + continue + } + + master, unique := uniqueDeviceEntry(byDeviceID, status.Group.MasterDeviceID) + if !unique || master.ID != entry.ID || !registeredMembersAgree(status.Group, byDeviceID) { + continue + } + + pair := newStereoPairView(status.Group, byDeviceID) + masters[entry.ID] = pair + + for _, role := range status.Group.Roles.Roles { + member, ok := uniqueDeviceEntry(byDeviceID, role.DeviceID) + if ok && member.ID != entry.ID { + hidden[member.ID] = true + } + } + } + + devices := make(map[string]deviceView, len(snapshot)) + for _, entry := range snapshot { + if entry.Device == nil || hidden[entry.ID] { + continue + } + + pair := masters[entry.ID] + devices[entry.ID] = deviceView{ + Info: projectedDeviceInfo(entry.Device.DeviceInfo, pair), + Status: entry.Device.Status(), + LastSeen: entry.Device.LastSeen, + StereoPair: pair, + } + } + + return devices +} + +func validMasterGroup(deviceID string, group *models.Group) bool { + if group == nil || group.IsEmpty() || strings.TrimSpace(group.ID) == "" || + strings.TrimSpace(group.MasterDeviceID) == "" || len(group.Roles.Roles) != 2 || + strings.TrimSpace(deviceID) != strings.TrimSpace(group.MasterDeviceID) { + return false + } + + seenDevices := make(map[string]bool, len(group.Roles.Roles)) + seenRoles := make(map[string]bool, len(group.Roles.Roles)) + masterPresent := false + + for _, role := range group.Roles.Roles { + memberID := strings.TrimSpace(role.DeviceID) + memberRole := strings.ToUpper(strings.TrimSpace(role.Role)) + if memberID == "" || seenDevices[memberID] || (memberRole != "LEFT" && memberRole != "RIGHT") || seenRoles[memberRole] { + return false + } + + seenDevices[memberID] = true + seenRoles[memberRole] = true + masterPresent = masterPresent || memberID == strings.TrimSpace(group.MasterDeviceID) + } + + return masterPresent && seenRoles["LEFT"] && seenRoles["RIGHT"] +} + +func uniqueDeviceEntry(byDeviceID map[string][]DeviceEntry, deviceID string) (DeviceEntry, bool) { + entries := byDeviceID[strings.TrimSpace(deviceID)] + if len(entries) != 1 { + return DeviceEntry{}, false + } + + return entries[0], true +} + +func registeredMembersAgree(group *models.Group, byDeviceID map[string][]DeviceEntry) bool { + for _, role := range group.Roles.Roles { + entries := byDeviceID[strings.TrimSpace(role.DeviceID)] + if len(entries) > 1 { + return false + } + + if len(entries) == 0 { + continue + } + + status := entries[0].Device.Status() + if status == nil || !sameGroupClaim(group, status.Group) { + return false + } + } + + return true +} + +func sameGroupClaim(left, right *models.Group) bool { + if left == nil || right == nil || left.ID != right.ID || left.MasterDeviceID != right.MasterDeviceID || + len(left.Roles.Roles) != len(right.Roles.Roles) { + return false + } + + rightRoles := make(map[string]string, len(right.Roles.Roles)) + for _, role := range right.Roles.Roles { + rightRoles[strings.TrimSpace(role.DeviceID)] = strings.ToUpper(strings.TrimSpace(role.Role)) + } + + for _, role := range left.Roles.Roles { + if rightRoles[strings.TrimSpace(role.DeviceID)] != strings.ToUpper(strings.TrimSpace(role.Role)) { + return false + } + } + + return true +} + +func newStereoPairView(group *models.Group, byDeviceID map[string][]DeviceEntry) *stereoPairView { + members := make([]stereoPairMemberView, 0, len(group.Roles.Roles)) + available := 0 + + for _, role := range group.Roles.Roles { + member := stereoPairMemberView{ + DeviceID: role.DeviceID, + Role: role.Role, + IPAddress: role.IPAddress, + } + + if entry, ok := uniqueDeviceEntry(byDeviceID, role.DeviceID); ok && entry.Device != nil { + if entry.Device.DeviceInfo != nil { + member.Name = entry.Device.DeviceInfo.Name + if entry.Device.DeviceInfo.IPAddress != "" { + member.IPAddress = entry.Device.DeviceInfo.IPAddress + } + } + + status := entry.Device.Status() + member.Available = status != nil && status.IsConnected + if member.Available { + available++ + } + } + + members = append(members, member) + } + + return &stereoPairView{ + ID: group.ID, + Name: logicalPairName(group.Name, members), + MasterDeviceID: group.MasterDeviceID, + Status: group.Status, + MemberCount: len(members), + AvailableMemberCount: available, + Degraded: available != len(members) || (group.Status != "" && group.Status != "GROUP_OK"), + Members: members, + } +} + +func projectedDeviceInfo(info *models.DeviceInfo, pair *stereoPairView) *models.DeviceInfo { + if info == nil || pair == nil || pair.Name == "" || pair.Name == info.Name { + return info + } + + projected := *info + projected.Name = pair.Name + + return &projected +} + +func logicalPairName(groupName string, members []stereoPairMemberView) string { + commonName := "" + for _, member := range members { + name := strings.TrimSpace(member.Name) + if name == "" { + return groupName + } + + if commonName == "" { + commonName = name + continue + } + + if !strings.EqualFold(commonName, name) { + return groupName + } + } + + if commonName != "" { + return commonName + } + + return groupName +} diff --git a/pkg/service/soundtouchweb/device_projection_test.go b/pkg/service/soundtouchweb/device_projection_test.go new file mode 100644 index 00000000..39920a8d --- /dev/null +++ b/pkg/service/soundtouchweb/device_projection_test.go @@ -0,0 +1,181 @@ +package soundtouchweb + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/models" + "github.com/gesellix/bose-soundtouch/pkg/service/soundtouchweb/webtypes" +) + +func projectionDevice(host, deviceID, name string, connected bool, group *models.Group) DeviceEntry { + conn := webtypes.NewDeviceConnection(nil, &models.DeviceInfo{ + DeviceID: deviceID, + Name: name, + IPAddress: host, + }) + conn.SetStatus(&webtypes.DeviceStatus{IsConnected: connected, Group: group}) + + return DeviceEntry{ID: host, Device: conn} +} + +func testStereoGroup() *models.Group { + return &models.Group{ + ID: "pair-1", + Name: "Living Room + Living Room", + MasterDeviceID: "left-id", + Status: "GROUP_OK", + Roles: models.GroupRoles{Roles: []models.GroupRole{ + {DeviceID: "left-id", Role: "LEFT", IPAddress: "192.0.2.10"}, + {DeviceID: "right-id", Role: "RIGHT", IPAddress: "192.0.2.11"}, + }}, + } +} + +func TestProjectDeviceEntriesCollapsesStereoPairUnderMaster(t *testing.T) { + group := testStereoGroup() + got := projectDeviceEntries([]DeviceEntry{ + projectionDevice("192.0.2.10", "left-id", "Living Room", true, group), + projectionDevice("192.0.2.11", "right-id", "Living Room", true, group), + }) + + if len(got) != 1 { + t.Fatalf("projected devices = %d, want one logical stereo target: %+v", len(got), got) + } + + master, ok := got["192.0.2.10"] + if !ok { + t.Fatalf("master control target missing: %+v", got) + } + + if master.StereoPair == nil { + t.Fatal("master is missing stereo-pair metadata") + } + + if master.StereoPair.MemberCount != 2 || master.StereoPair.AvailableMemberCount != 2 || master.StereoPair.Degraded { + t.Errorf("unexpected pair availability: %+v", master.StereoPair) + } + + if master.Info.Name != "Living Room" || master.StereoPair.Name != "Living Room" { + t.Errorf("logical pair name was not projected consistently: %+v", master) + } + + if _, ok := got["192.0.2.11"]; ok { + t.Error("physical right member must not be a second control target") + } +} + +func TestProjectDeviceEntriesShowsDegradedPairWhenMemberIsMissing(t *testing.T) { + got := projectDeviceEntries([]DeviceEntry{ + projectionDevice("192.0.2.10", "left-id", "Living Room", true, testStereoGroup()), + }) + + pair := got["192.0.2.10"].StereoPair + if pair == nil { + t.Fatal("connected master should remain a logical pair when its member is unavailable") + } + + if pair.AvailableMemberCount != 1 || !pair.Degraded { + t.Errorf("missing member not reflected as degraded: %+v", pair) + } +} + +func TestProjectDeviceEntriesKeepsStablePairWhenMasterIsDisconnected(t *testing.T) { + group := testStereoGroup() + got := projectDeviceEntries([]DeviceEntry{ + projectionDevice("192.0.2.10", "left-id", "Living Room", false, group), + projectionDevice("192.0.2.11", "right-id", "Living Room", true, group), + }) + + if len(got) != 1 { + t.Fatalf("projected devices = %d, want a stable logical pair while its master is registered", len(got)) + } + + pair := got["192.0.2.10"].StereoPair + if pair == nil || !pair.Degraded || pair.AvailableMemberCount != 1 { + t.Errorf("disconnected master should produce a degraded logical pair: %+v", got) + } +} + +func TestProjectDeviceEntriesLeavesMemberPhysicalWhenMasterIsAbsent(t *testing.T) { + got := projectDeviceEntries([]DeviceEntry{ + projectionDevice("192.0.2.11", "right-id", "Living Room", true, testStereoGroup()), + }) + + if len(got) != 1 || got["192.0.2.11"].StereoPair != nil { + t.Fatalf("member without a registered master must remain a physical target: %+v", got) + } +} + +func TestProjectDeviceEntriesRequiresMasterReportedGroup(t *testing.T) { + group := testStereoGroup() + got := projectDeviceEntries([]DeviceEntry{ + projectionDevice("192.0.2.10", "left-id", "Living Room", true, nil), + projectionDevice("192.0.2.11", "right-id", "Living Room", true, group), + }) + + if len(got) != 2 { + t.Fatalf("slave-only group data must not collapse the registry: %+v", got) + } +} + +func TestProjectDeviceEntriesRejectsMalformedGroup(t *testing.T) { + group := testStereoGroup() + group.Roles.Roles[1].DeviceID = group.Roles.Roles[0].DeviceID + + got := projectDeviceEntries([]DeviceEntry{ + projectionDevice("192.0.2.10", "left-id", "Living Room", true, group), + projectionDevice("192.0.2.11", "right-id", "Living Room", true, group), + }) + + if len(got) != 2 { + t.Fatalf("malformed pair must not hide a physical device: %+v", got) + } +} + +func TestProjectDeviceEntriesRejectsConflictingMemberClaim(t *testing.T) { + masterGroup := testStereoGroup() + memberGroup := testStereoGroup() + memberGroup.ID = "different-pair" + + got := projectDeviceEntries([]DeviceEntry{ + projectionDevice("192.0.2.10", "left-id", "Living Room", true, masterGroup), + projectionDevice("192.0.2.11", "right-id", "Living Room", true, memberGroup), + }) + + if len(got) != 2 { + t.Fatalf("conflicting pair claims must fail open: %+v", got) + } +} + +func TestHandleAPIDevicesUsesLogicalStereoProjection(t *testing.T) { + app := NewWebApp() + group := testStereoGroup() + for _, entry := range []DeviceEntry{ + projectionDevice("192.0.2.10", "left-id", "Living Room", true, group), + projectionDevice("192.0.2.11", "right-id", "Living Room", true, group), + } { + app.AddDevice(entry.ID, entry.Device) + } + + response := httptest.NewRecorder() + app.HandleAPIDevices(response, httptest.NewRequest("GET", "/api/control/devices", nil)) + + var payload struct { + Success bool `json:"success"` + Data map[string]deviceView `json:"data"` + } + if err := json.NewDecoder(response.Body).Decode(&payload); err != nil { + t.Fatalf("decode devices response: %v", err) + } + + if response.Code != http.StatusOK || !payload.Success || len(payload.Data) != 1 { + t.Fatalf("unexpected devices response: status=%d payload=%+v", response.Code, payload) + } + + if pair := payload.Data["192.0.2.10"].StereoPair; pair == nil || pair.ID != "pair-1" || pair.MemberCount != 2 { + t.Fatalf("logical stereo metadata missing from devices API: %+v", payload.Data) + } +} diff --git a/pkg/service/soundtouchweb/handler.go b/pkg/service/soundtouchweb/handler.go index 3591ef66..34401063 100644 --- a/pkg/service/soundtouchweb/handler.go +++ b/pkg/service/soundtouchweb/handler.go @@ -251,20 +251,9 @@ func (app *WebApp) HandleAPIDevices(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") // Return all devices as JSON - snapshot := app.DeviceSnapshot() - devices := make(map[string]interface{}, len(snapshot)) - - for _, entry := range snapshot { - devices[entry.ID] = map[string]interface{}{ - "info": entry.Device.DeviceInfo, - "status": entry.Device.Status(), - "lastSeen": entry.Device.LastSeen, - } - } - response := webtypes.APIResponse{ Success: true, - Data: devices, + Data: app.deviceViewSnapshot(), } if err := json.NewEncoder(w).Encode(response); err != nil { @@ -734,20 +723,9 @@ func (app *WebApp) BroadcastDeviceList() { app.WSMutex.RLock() defer app.WSMutex.RUnlock() - snapshot := app.DeviceSnapshot() - devices := make(map[string]interface{}, len(snapshot)) - - for _, entry := range snapshot { - devices[entry.ID] = map[string]interface{}{ - "info": entry.Device.DeviceInfo, - "status": entry.Device.Status(), - "lastSeen": entry.Device.LastSeen, - } - } - message := webtypes.WebSocketMessage{ Type: "devices", - Data: devices, + Data: app.deviceViewSnapshot(), } // Send to all connected clients diff --git a/pkg/service/soundtouchweb/static/css/app.css b/pkg/service/soundtouchweb/static/css/app.css index dec52dc7..dc179a2e 100644 --- a/pkg/service/soundtouchweb/static/css/app.css +++ b/pkg/service/soundtouchweb/static/css/app.css @@ -399,6 +399,8 @@ img { display: block; max-width: 100%; } } .device-type { font-size: .8rem; color: var(--text-dim); margin-bottom: .5rem; display: flex; gap: .4rem; flex-wrap: wrap; } .device-ip { color: var(--text); font-family: monospace; font-weight: 500; } +.stereo-pair-state { color: var(--accent); font-weight: 600; } +.stereo-pair-state.degraded { color: var(--offline); } .device-indicator { width: 8px; height: 8px; border-radius: 50%; diff --git a/pkg/service/soundtouchweb/static/js/app.js b/pkg/service/soundtouchweb/static/js/app.js index 5eb871c5..2e31cd6f 100644 --- a/pkg/service/soundtouchweb/static/js/app.js +++ b/pkg/service/soundtouchweb/static/js/app.js @@ -138,6 +138,13 @@ function App() { }; }, []); + useEffect(() => { + if (selectedId && !devices[selectedId]) { + setSelectedId(null); + if (page === 'device') setPage('devices'); + } + }, [devices, selectedId, page]); + function showToast(msg) { setToast(null); setTimeout(() => setToast(msg), 10); diff --git a/pkg/service/soundtouchweb/static/js/components/DeviceList.js b/pkg/service/soundtouchweb/static/js/components/DeviceList.js index 4785300f..3cc7c40e 100644 --- a/pkg/service/soundtouchweb/static/js/components/DeviceList.js +++ b/pkg/service/soundtouchweb/static/js/components/DeviceList.js @@ -23,6 +23,7 @@ function sortEntries(entries, mode) { function DeviceCard({ id, device, onSelect, onRemove }) { const { info, status } = device; + const stereoPair = device.stereoPair; const np = status?.nowPlaying; const isPlaying = np?.PlayStatus === 'PLAY_STATE'; const isStandby = !np || np.Source === 'STANDBY'; @@ -33,14 +34,19 @@ function DeviceCard({ id, device, onSelect, onRemove }) { ${info?.name || id} - + onClick=${(e) => { e.stopPropagation(); onRemove(id); }}>✕` : null}
${info?.type || ''} ${info?.ip_address ? html`(${info.ip_address})` : null} + ${stereoPair ? html` + + Stereo pair ${stereoPair.availableMemberCount}/${stereoPair.memberCount} + + ` : null}
${!isStandby ? html`
diff --git a/pkg/service/soundtouchweb/static/js/components/Library.js b/pkg/service/soundtouchweb/static/js/components/Library.js index d8d42229..239e491f 100644 --- a/pkg/service/soundtouchweb/static/js/components/Library.js +++ b/pkg/service/soundtouchweb/static/js/components/Library.js @@ -23,10 +23,12 @@ export function Library({ devices }) { // invalidate the current selection. useEffect(() => { const entries = Object.entries(devices); - if (!deviceId && entries.length > 0) { + if ((!deviceId || !devices[deviceId]) && entries.length > 0) { setDeviceId(entries[0][0]); + } else if (deviceId && entries.length === 0) { + setDeviceId(null); } - }, [devices]); + }, [devices, deviceId]); // Reload registered servers whenever deviceId changes. useEffect(() => { diff --git a/pkg/service/soundtouchweb/websocket.go b/pkg/service/soundtouchweb/websocket.go index cff60460..72c1ee39 100644 --- a/pkg/service/soundtouchweb/websocket.go +++ b/pkg/service/soundtouchweb/websocket.go @@ -46,23 +46,10 @@ func (app *WebApp) HandleWebSocket(w http.ResponseWriter, r *http.Request) { } // Send initial device list - snapshot := app.DeviceSnapshot() - devices := make(map[string]interface{}, len(snapshot)) - - for _, entry := range snapshot { - devices[entry.ID] = map[string]interface{}{ - "info": entry.Device.DeviceInfo, - "status": entry.Device.Status(), - "lastSeen": entry.Device.LastSeen, - } - } - - initialMessage := webtypes.WebSocketMessage{ + if err := conn.WriteJSON(webtypes.WebSocketMessage{ Type: "devices", - Data: devices, - } - - if err := conn.WriteJSON(initialMessage); err != nil { + Data: app.deviceViewSnapshot(), + }); err != nil { log.Printf("Failed to send initial data: %v", err) return } @@ -100,21 +87,14 @@ func (app *WebApp) HandleWebSocket(w http.ResponseWriter, r *http.Request) { return } - // Send periodic status updates - for _, entry := range app.DeviceSnapshot() { - status := entry.Device.Status() - if status.IsConnected { - statusMessage := webtypes.WebSocketMessage{ - Type: "status_update", - DeviceID: entry.ID, - Data: status, - } - - if err := conn.WriteJSON(statusMessage); err != nil { - log.Printf("Failed to send status update: %v", err) - return - } - } + // A full projected list keeps pair topology and availability current + // without event handlers writing to this browser connection. + if err := conn.WriteJSON(webtypes.WebSocketMessage{ + Type: "devices", + Data: app.deviceViewSnapshot(), + }); err != nil { + log.Printf("Failed to send device update: %v", err) + return } } } @@ -218,6 +198,10 @@ func (app *WebApp) ConnectDeviceWebSocket(deviceID string, conn *webtypes.Device }) }) + wsClient.OnGroupUpdated(func(event *models.GroupUpdatedEvent) { + applyGroupUpdatedEvent(conn, event) + }) + if err := wsClient.Connect(); err != nil { log.Printf("Failed to connect WebSocket for device %s: %v (retrying in %s)", sanitizeLog(deviceID), err, backoff) @@ -298,6 +282,8 @@ func (app *WebApp) UpdateDeviceStatus(_ string, conn *webtypes.DeviceConnection) return } + groupGeneration := conn.BeginGroupRefresh() + // 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". @@ -306,6 +292,7 @@ func (app *WebApp) UpdateDeviceStatus(_ string, conn *webtypes.DeviceConnection) presets, presetsErr := conn.Client.GetPresets() sources, sourcesErr := conn.Client.GetSources() bass, bassErr := conn.Client.GetBass() + group, groupErr := conn.Client.GetGroup() // Phase 2: fast merge. Only fields we successfully fetched // overwrite; everything else keeps the value other goroutines may @@ -338,11 +325,21 @@ func (app *WebApp) UpdateDeviceStatus(_ string, conn *webtypes.DeviceConnection) statusUpdated = true } + statusUpdated = statusUpdated || groupErr == nil + // Mark as connected if we successfully got at least one // status from this round. Mirrors prior behaviour. s.IsConnected = statusUpdated s.LastActivity = time.Now() }) + + if groupErr == nil { + conn.ApplyPolledGroup(groupGeneration, group) + } +} + +func applyGroupUpdatedEvent(conn *webtypes.DeviceConnection, event *models.GroupUpdatedEvent) { + conn.ApplyGroupEvent(&event.Group, time.Now()) } // HandleDeviceWebSocket handles individual device WebSocket connections for real-time device-specific updates diff --git a/pkg/service/soundtouchweb/websocket_test.go b/pkg/service/soundtouchweb/websocket_test.go new file mode 100644 index 00000000..56682fa4 --- /dev/null +++ b/pkg/service/soundtouchweb/websocket_test.go @@ -0,0 +1,141 @@ +package soundtouchweb + +import ( + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gesellix/bose-soundtouch/pkg/client" + "github.com/gesellix/bose-soundtouch/pkg/models" + "github.com/gesellix/bose-soundtouch/pkg/service/soundtouchweb/webtypes" +) + +func TestUpdateDeviceStatusRefreshesGroup(t *testing.T) { + server := newStatusTestServer(t, http.StatusOK, ` + Living Room + master-1 + + master-1LEFT + member-1RIGHT + + GROUP_OK + `) + defer server.Close() + + conn := webtypes.NewDeviceConnection(client.NewClientFromHost(server.URL), nil) + NewWebApp().UpdateDeviceStatus("device-1", conn) + + status := conn.Status() + if status.Group == nil { + t.Fatal("Group was not populated by UpdateDeviceStatus") + } + + if status.Group.ID != "pair-1" || status.Group.MasterDeviceID != "master-1" { + t.Errorf("Group = %+v, want refreshed stereo pair", status.Group) + } + + if len(status.Group.Roles.Roles) != 2 { + t.Errorf("group roles = %d, want 2", len(status.Group.Roles.Roles)) + } + + if !status.IsConnected { + t.Error("successful status refresh should mark the device connected") + } +} + +func TestUpdateDeviceStatusPreservesGroupOnError(t *testing.T) { + server := newStatusTestServer(t, http.StatusInternalServerError, "group unavailable") + defer server.Close() + + conn := webtypes.NewDeviceConnection(client.NewClientFromHost(server.URL), nil) + existing := &models.Group{ID: "pair-old", Name: "Existing Pair"} + conn.SetStatus(&webtypes.DeviceStatus{Group: existing}) + + NewWebApp().UpdateDeviceStatus("device-1", conn) + + status := conn.Status() + if status.Group != existing { + t.Errorf("Group = %+v, want previous group preserved on refresh error", status.Group) + } + + if !status.IsConnected { + t.Error("other successful status fetches should keep the device connected") + } +} + +func TestApplyGroupUpdatedEventReplacesGroup(t *testing.T) { + conn := webtypes.NewDeviceConnection(nil, nil) + previousActivity := time.Unix(1, 0) + conn.SetStatus(&webtypes.DeviceStatus{ + Group: &models.Group{ID: "pair-old"}, + Volume: &models.Volume{ActualVolume: 25}, + IsConnected: true, + LastActivity: previousActivity, + }) + + event := &models.GroupUpdatedEvent{ + Group: models.Group{ID: "pair-new", Name: "Renamed Pair"}, + } + applyGroupUpdatedEvent(conn, event) + + status := conn.Status() + if status.Group != &event.Group || status.Group.ID != "pair-new" { + t.Errorf("Group = %+v, want event group", status.Group) + } + + if status.Volume == nil || status.Volume.ActualVolume != 25 || !status.IsConnected { + t.Errorf("unrelated status fields were not preserved: %+v", status) + } + + if !status.LastActivity.After(previousActivity) { + t.Errorf("LastActivity = %s, want after %s", status.LastActivity, previousActivity) + } + + teardown := &models.GroupUpdatedEvent{Group: models.Group{}} + applyGroupUpdatedEvent(conn, teardown) + + if conn.Status().Group != nil { + t.Errorf("teardown event did not clear the group: %+v", conn.Status().Group) + } +} + +func newStatusTestServer(t *testing.T, groupStatus int, groupBody string) *httptest.Server { + t.Helper() + + responses := map[string]string{ + "/now_playing": `STOP_STATE`, + "/volume": `1010false`, + "/presets": ``, + "/sources": ``, + "/bass": `00`, + } + + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + t.Errorf("method for %s = %s, want GET", r.URL.Path, r.Method) + w.WriteHeader(http.StatusMethodNotAllowed) + + return + } + + w.Header().Set("Content-Type", "application/xml") + + if r.URL.Path == "/getGroup" { + w.WriteHeader(groupStatus) + _, _ = w.Write([]byte(groupBody)) + + return + } + + body, ok := responses[r.URL.Path] + if !ok { + t.Errorf("unexpected status endpoint %q", r.URL.Path) + w.WriteHeader(http.StatusNotFound) + + return + } + + _, _ = w.Write([]byte(body)) + })) +} diff --git a/pkg/service/soundtouchweb/webtypes/status_test.go b/pkg/service/soundtouchweb/webtypes/status_test.go index a7fb6ede..fbeae144 100644 --- a/pkg/service/soundtouchweb/webtypes/status_test.go +++ b/pkg/service/soundtouchweb/webtypes/status_test.go @@ -3,9 +3,11 @@ package webtypes import ( + "encoding/json" "fmt" "sync" "testing" + "time" "github.com/gesellix/bose-soundtouch/pkg/models" ) @@ -75,6 +77,7 @@ func TestUpdateStatus_PreservesUnchangedFields(t *testing.T) { conn.SetStatus(&DeviceStatus{ Volume: &models.Volume{ActualVolume: 10}, Bass: &models.Bass{ActualBass: 3}, + Group: &models.Group{ID: "pair-1", Name: "Living Room"}, IsConnected: true, }) @@ -92,11 +95,87 @@ func TestUpdateStatus_PreservesUnchangedFields(t *testing.T) { t.Errorf("Bass not preserved: %+v", got.Bass) } + if got.Group == nil || got.Group.ID != "pair-1" { + t.Errorf("Group not preserved: %+v", got.Group) + } + if !got.IsConnected { t.Error("IsConnected not preserved") } } +func TestDeviceStatusGroupJSON(t *testing.T) { + status := DeviceStatus{ + Group: &models.Group{ + ID: "pair-1", + Name: "Living Room", + MasterDeviceID: "master-1", + }, + } + + payload, err := json.Marshal(status) + if err != nil { + t.Fatalf("Marshal DeviceStatus: %v", err) + } + + var decoded struct { + Group *models.Group `json:"group"` + } + + if err := json.Unmarshal(payload, &decoded); err != nil { + t.Fatalf("Unmarshal DeviceStatus: %v", err) + } + + if decoded.Group == nil || decoded.Group.ID != "pair-1" || decoded.Group.MasterDeviceID != "master-1" { + t.Fatalf("group did not round-trip in status JSON: %+v", decoded.Group) + } + + emptyPayload, err := json.Marshal(DeviceStatus{}) + if err != nil { + t.Fatalf("Marshal empty DeviceStatus: %v", err) + } + + var emptyDecoded map[string]json.RawMessage + if err := json.Unmarshal(emptyPayload, &emptyDecoded); err != nil { + t.Fatalf("Unmarshal empty DeviceStatus: %v", err) + } + + if _, ok := emptyDecoded["group"]; ok { + t.Errorf("nil group should be omitted, JSON = %s", emptyPayload) + } +} + +func TestGroupEventSupersedesInFlightPoll(t *testing.T) { + conn := NewDeviceConnection(nil, &models.DeviceInfo{Name: "test"}) + generation := conn.BeginGroupRefresh() + + eventGroup := &models.Group{ID: "new-pair", MasterDeviceID: "master"} + if !conn.ApplyGroupEvent(eventGroup, time.Now()) { + t.Fatal("new group event should change group state") + } + + if conn.ApplyPolledGroup(generation, &models.Group{ID: "stale-pair"}) { + t.Fatal("stale poll must not replace a newer group event") + } + + if got := conn.Status().Group; got == nil || got.ID != "new-pair" { + t.Fatalf("Group = %+v, want newer event state", got) + } +} + +func TestEmptyGroupClearsCurrentClaim(t *testing.T) { + conn := NewDeviceConnection(nil, &models.DeviceInfo{Name: "test"}) + conn.SetStatus(&DeviceStatus{Group: &models.Group{ID: "pair-1"}}) + + if !conn.ApplyGroupEvent(&models.Group{}, time.Now()) { + t.Fatal("empty teardown event should change group state") + } + + if got := conn.Status().Group; got != nil { + t.Fatalf("Group = %+v, want nil after teardown", got) + } +} + 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 diff --git a/pkg/service/soundtouchweb/webtypes/types.go b/pkg/service/soundtouchweb/webtypes/types.go index 37b42e3e..cd0f1caa 100644 --- a/pkg/service/soundtouchweb/webtypes/types.go +++ b/pkg/service/soundtouchweb/webtypes/types.go @@ -2,6 +2,7 @@ package webtypes import ( + "reflect" "sync" "sync/atomic" "time" @@ -28,6 +29,7 @@ type SoundTouchClient interface { GetPresets() (*models.Presets, error) GetSources() (*models.Sources, error) GetBass() (*models.Bass, error) + GetGroup() (*models.Group, error) NewWebSocketClient(config interface{}) *client.WebSocketClient } @@ -47,6 +49,12 @@ type DeviceConnection struct { status atomic.Pointer[DeviceStatus] + // groupMu orders polled /getGroup responses against real-time + // groupUpdated events. Starting a newer refresh or receiving an event + // invalidates any older in-flight poll. + groupMu sync.Mutex + groupGeneration uint64 + // done is closed by Close when the device is removed from the // registry, signalling its background goroutines (the status poller // and the WebSocket reconnect loop) to exit. closeOnce keeps Close @@ -62,6 +70,7 @@ type DeviceStatus struct { Presets *models.Presets `json:"presets,omitempty"` Sources *models.Sources `json:"sources,omitempty"` Bass *models.Bass `json:"bass,omitempty"` + Group *models.Group `json:"group,omitempty"` IsConnected bool `json:"isConnected"` LastActivity time.Time `json:"lastActivity"` } @@ -85,10 +94,9 @@ func NewDeviceConnection(c *client.Client, info *models.DeviceInfo) *DeviceConne } // 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. +// pointer is read-only from the caller's perspective and must not be +// mutated. Use UpdateStatus or SetStatus to apply changes. Never returns +// nil for connections built via NewDeviceConnection. func (c *DeviceConnection) Status() *DeviceStatus { return c.status.Load() } @@ -128,7 +136,7 @@ func (c *DeviceConnection) SetStatus(s *DeviceStatus) { // 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) +// Nested pointer fields (NowPlaying, Volume, Presets, Sources, Bass, Group) // 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 @@ -147,6 +155,61 @@ func (c *DeviceConnection) UpdateStatus(mut func(*DeviceStatus)) { } } +// BeginGroupRefresh starts a new generation for an asynchronous /getGroup +// request. Only the latest started request may later update Group. +func (c *DeviceConnection) BeginGroupRefresh() uint64 { + c.groupMu.Lock() + defer c.groupMu.Unlock() + + c.groupGeneration++ + + return c.groupGeneration +} + +// ApplyPolledGroup stores a /getGroup result only when no newer poll or +// groupUpdated event superseded it. Empty groups clear the current claim. +func (c *DeviceConnection) ApplyPolledGroup(generation uint64, group *models.Group) bool { + c.groupMu.Lock() + defer c.groupMu.Unlock() + + if generation != c.groupGeneration { + return false + } + + return c.replaceGroup(normalizeGroup(group), time.Time{}) +} + +// ApplyGroupEvent stores the newest groupUpdated event and invalidates all +// in-flight /getGroup requests. Empty teardown events clear the current claim. +func (c *DeviceConnection) ApplyGroupEvent(group *models.Group, activity time.Time) bool { + c.groupMu.Lock() + defer c.groupMu.Unlock() + + c.groupGeneration++ + + return c.replaceGroup(normalizeGroup(group), activity) +} + +func (c *DeviceConnection) replaceGroup(group *models.Group, activity time.Time) bool { + changed := !reflect.DeepEqual(c.Status().Group, group) + c.UpdateStatus(func(s *DeviceStatus) { + s.Group = group + if !activity.IsZero() { + s.LastActivity = activity + } + }) + + return changed +} + +func normalizeGroup(group *models.Group) *models.Group { + if group == nil || group.IsEmpty() { + return nil + } + + return group +} + // APIResponse is a standard JSON response wrapper type APIResponse struct { Success bool `json:"success"`