From ce66b103b408ab06282ee1c8ad355e3dc19425ab 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 11:32:13 +0200 Subject: [PATCH] fix(player): keep stereo updates coherent and compatible --- .../soundtouchweb/device_projection.go | 86 +++++++++++++------ .../soundtouchweb/device_projection_test.go | 23 +++++ pkg/service/soundtouchweb/websocket.go | 37 ++++++-- pkg/service/soundtouchweb/websocket_test.go | 36 ++++++++ pkg/service/soundtouchweb/webtypes/types.go | 1 - 5 files changed, 147 insertions(+), 36 deletions(-) diff --git a/pkg/service/soundtouchweb/device_projection.go b/pkg/service/soundtouchweb/device_projection.go index 213a3b42..6e453c8c 100644 --- a/pkg/service/soundtouchweb/device_projection.go +++ b/pkg/service/soundtouchweb/device_projection.go @@ -18,6 +18,17 @@ type deviceView struct { StereoPair *stereoPairView `json:"stereoPair,omitempty"` } +// deviceProjectionEntry captures one immutable status pointer per physical +// device. Projection must not re-read live connection state midway through +// building a response, otherwise group membership and the emitted status can +// describe different moments. +type deviceProjectionEntry struct { + ID string + Info *models.DeviceInfo + Status *webtypes.DeviceStatus + LastSeen time.Time +} + // stereoPairView describes the physical members represented by a logical // player target. Controls are always sent to MasterDeviceID via the map key. type stereoPairView struct { @@ -48,13 +59,35 @@ func (app *WebApp) deviceViewSnapshot() map[string]deviceView { } func projectDeviceEntries(snapshot []DeviceEntry) map[string]deviceView { - byDeviceID := make(map[string][]DeviceEntry, len(snapshot)) + return projectCapturedDeviceEntries(captureDeviceProjectionEntries(snapshot)) +} + +func captureDeviceProjectionEntries(snapshot []DeviceEntry) []deviceProjectionEntry { + captured := make([]deviceProjectionEntry, 0, len(snapshot)) for _, entry := range snapshot { - if entry.Device == nil || entry.Device.DeviceInfo == nil { + if entry.Device == nil { continue } - deviceID := strings.TrimSpace(entry.Device.DeviceInfo.DeviceID) + captured = append(captured, deviceProjectionEntry{ + ID: entry.ID, + Info: entry.Device.DeviceInfo, + Status: entry.Device.Status(), + LastSeen: entry.Device.LastSeen, + }) + } + + return captured +} + +func projectCapturedDeviceEntries(snapshot []deviceProjectionEntry) map[string]deviceView { + byDeviceID := make(map[string][]deviceProjectionEntry, len(snapshot)) + for _, entry := range snapshot { + if entry.Info == nil { + continue + } + + deviceID := strings.TrimSpace(entry.Info.DeviceID) if deviceID != "" { byDeviceID[deviceID] = append(byDeviceID[deviceID], entry) } @@ -64,24 +97,23 @@ func projectDeviceEntries(snapshot []DeviceEntry) map[string]deviceView { hidden := make(map[string]bool) for _, entry := range snapshot { - if entry.Device == nil || entry.Device.DeviceInfo == nil { + if entry.Info == nil { continue } - status := entry.Device.Status() - if status == nil || !validMasterGroup(entry.Device.DeviceInfo.DeviceID, status.Group) { + if entry.Status == nil || !validMasterGroup(entry.Info.DeviceID, entry.Status.Group) { continue } - master, unique := uniqueDeviceEntry(byDeviceID, status.Group.MasterDeviceID) - if !unique || master.ID != entry.ID || !registeredMembersAgree(status.Group, byDeviceID) { + master, unique := uniqueDeviceEntry(byDeviceID, entry.Status.Group.MasterDeviceID) + if !unique || master.ID != entry.ID || !registeredMembersAgree(entry.Status.Group, byDeviceID) { continue } - pair := newStereoPairView(status.Group, byDeviceID) + pair := newStereoPairView(entry.Status.Group, byDeviceID) masters[entry.ID] = pair - for _, role := range status.Group.Roles.Roles { + for _, role := range entry.Status.Group.Roles.Roles { member, ok := uniqueDeviceEntry(byDeviceID, role.DeviceID) if ok && member.ID != entry.ID { hidden[member.ID] = true @@ -91,15 +123,15 @@ func projectDeviceEntries(snapshot []DeviceEntry) map[string]deviceView { devices := make(map[string]deviceView, len(snapshot)) for _, entry := range snapshot { - if entry.Device == nil || hidden[entry.ID] { + if 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, + Info: projectedDeviceInfo(entry.Info, pair), + Status: entry.Status, + LastSeen: entry.LastSeen, StereoPair: pair, } } @@ -120,6 +152,7 @@ func validMasterGroup(deviceID string, group *models.Group) bool { 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 @@ -133,16 +166,16 @@ func validMasterGroup(deviceID string, group *models.Group) bool { return masterPresent && seenRoles["LEFT"] && seenRoles["RIGHT"] } -func uniqueDeviceEntry(byDeviceID map[string][]DeviceEntry, deviceID string) (DeviceEntry, bool) { +func uniqueDeviceEntry(byDeviceID map[string][]deviceProjectionEntry, deviceID string) (deviceProjectionEntry, bool) { entries := byDeviceID[strings.TrimSpace(deviceID)] if len(entries) != 1 { - return DeviceEntry{}, false + return deviceProjectionEntry{}, false } return entries[0], true } -func registeredMembersAgree(group *models.Group, byDeviceID map[string][]DeviceEntry) bool { +func registeredMembersAgree(group *models.Group, byDeviceID map[string][]deviceProjectionEntry) bool { for _, role := range group.Roles.Roles { entries := byDeviceID[strings.TrimSpace(role.DeviceID)] if len(entries) > 1 { @@ -153,8 +186,7 @@ func registeredMembersAgree(group *models.Group, byDeviceID map[string][]DeviceE continue } - status := entries[0].Device.Status() - if status == nil || !sameGroupClaim(group, status.Group) { + if entries[0].Status == nil || !sameGroupClaim(group, entries[0].Status.Group) { return false } } @@ -182,7 +214,7 @@ func sameGroupClaim(left, right *models.Group) bool { return true } -func newStereoPairView(group *models.Group, byDeviceID map[string][]DeviceEntry) *stereoPairView { +func newStereoPairView(group *models.Group, byDeviceID map[string][]deviceProjectionEntry) *stereoPairView { members := make([]stereoPairMemberView, 0, len(group.Roles.Roles)) available := 0 @@ -193,16 +225,15 @@ func newStereoPairView(group *models.Group, byDeviceID map[string][]DeviceEntry) 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 + if entry, ok := uniqueDeviceEntry(byDeviceID, role.DeviceID); ok { + if entry.Info != nil { + member.Name = entry.Info.Name + if entry.Info.IPAddress != "" { + member.IPAddress = entry.Info.IPAddress } } - status := entry.Device.Status() - member.Available = status != nil && status.IsConnected + member.Available = entry.Status != nil && entry.Status.IsConnected if member.Available { available++ } @@ -236,6 +267,7 @@ func projectedDeviceInfo(info *models.DeviceInfo, pair *stereoPairView) *models. func logicalPairName(groupName string, members []stereoPairMemberView) string { commonName := "" + for _, member := range members { name := strings.TrimSpace(member.Name) if name == "" { diff --git a/pkg/service/soundtouchweb/device_projection_test.go b/pkg/service/soundtouchweb/device_projection_test.go index 39920a8d..2989db30 100644 --- a/pkg/service/soundtouchweb/device_projection_test.go +++ b/pkg/service/soundtouchweb/device_projection_test.go @@ -5,6 +5,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/gesellix/bose-soundtouch/pkg/models" "github.com/gesellix/bose-soundtouch/pkg/service/soundtouchweb/webtypes" @@ -150,6 +151,28 @@ func TestProjectDeviceEntriesRejectsConflictingMemberClaim(t *testing.T) { } } +func TestProjectCapturedDeviceEntriesUsesOneCoherentStatusPerDevice(t *testing.T) { + group := testStereoGroup() + entries := []DeviceEntry{ + projectionDevice("192.0.2.10", "left-id", "Living Room", true, group), + projectionDevice("192.0.2.11", "right-id", "Living Room", true, group), + } + captured := captureDeviceProjectionEntries(entries) + + entries[0].Device.ApplyGroupEvent(&models.Group{}, time.Now()) + entries[1].Device.ApplyGroupEvent(&models.Group{}, time.Now()) + + got := projectCapturedDeviceEntries(captured) + master := got["192.0.2.10"] + if master.StereoPair == nil || master.Status == nil || master.Status.Group == nil || master.Status.Group.ID != "pair-1" { + t.Fatalf("captured projection mixed newer connection state into its response: %+v", got) + } + + if fresh := projectDeviceEntries(entries); len(fresh) != 2 { + t.Fatalf("fresh projection did not observe the cleared group: %+v", fresh) + } +} + func TestHandleAPIDevicesUsesLogicalStereoProjection(t *testing.T) { app := NewWebApp() group := testStereoGroup() diff --git a/pkg/service/soundtouchweb/websocket.go b/pkg/service/soundtouchweb/websocket.go index 72c1ee39..4ae50099 100644 --- a/pkg/service/soundtouchweb/websocket.go +++ b/pkg/service/soundtouchweb/websocket.go @@ -87,18 +87,39 @@ func (app *WebApp) HandleWebSocket(w http.ResponseWriter, r *http.Request) { 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 + for _, message := range app.periodicPlayerMessages() { + if err := conn.WriteJSON(message); err != nil { + log.Printf("Failed to send device update: %v", err) + return + } } } } +// periodicPlayerMessages refreshes the projected inventory while retaining +// the established per-device status_update stream for API clients. +func (app *WebApp) periodicPlayerMessages() []webtypes.WebSocketMessage { + snapshot := captureDeviceProjectionEntries(app.DeviceSnapshot()) + messages := []webtypes.WebSocketMessage{{ + Type: "devices", + Data: projectCapturedDeviceEntries(snapshot), + }} + + for _, entry := range snapshot { + if entry.Status == nil || !entry.Status.IsConnected { + continue + } + + messages = append(messages, webtypes.WebSocketMessage{ + Type: "status_update", + DeviceID: entry.ID, + Data: entry.Status, + }) + } + + return messages +} + // HandleAPIDiscover triggers device discovery func (app *WebApp) HandleAPIDiscover(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPost { diff --git a/pkg/service/soundtouchweb/websocket_test.go b/pkg/service/soundtouchweb/websocket_test.go index 56682fa4..405ed00e 100644 --- a/pkg/service/soundtouchweb/websocket_test.go +++ b/pkg/service/soundtouchweb/websocket_test.go @@ -100,6 +100,42 @@ func TestApplyGroupUpdatedEventReplacesGroup(t *testing.T) { } } +func TestPeriodicPlayerMessagesPreserveStatusUpdateStream(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), + projectionDevice("192.0.2.12", "standalone-id", "Kitchen", false, nil), + } { + app.AddDevice(entry.ID, entry.Device) + } + + messages := app.periodicPlayerMessages() + if len(messages) != 3 { + t.Fatalf("periodic messages = %d, want one devices frame and two connected status updates: %+v", len(messages), messages) + } + + if messages[0].Type != "devices" { + t.Fatalf("first periodic message type = %q, want devices", messages[0].Type) + } + devices, ok := messages[0].Data.(map[string]deviceView) + if !ok || len(devices) != 2 || devices["192.0.2.10"].StereoPair == nil { + t.Fatalf("periodic devices frame is not the logical projection: %#v", messages[0].Data) + } + + statusUpdates := make(map[string]bool) + for _, message := range messages[1:] { + if message.Type != "status_update" { + t.Fatalf("periodic message type = %q, want status_update", message.Type) + } + statusUpdates[message.DeviceID] = true + } + if !statusUpdates["192.0.2.10"] || !statusUpdates["192.0.2.11"] || statusUpdates["192.0.2.12"] { + t.Fatalf("unexpected status_update device IDs: %+v", statusUpdates) + } +} + func newStatusTestServer(t *testing.T, groupStatus int, groupBody string) *httptest.Server { t.Helper() diff --git a/pkg/service/soundtouchweb/webtypes/types.go b/pkg/service/soundtouchweb/webtypes/types.go index cd0f1caa..5f2bbba7 100644 --- a/pkg/service/soundtouchweb/webtypes/types.go +++ b/pkg/service/soundtouchweb/webtypes/types.go @@ -29,7 +29,6 @@ type SoundTouchClient interface { GetPresets() (*models.Presets, error) GetSources() (*models.Sources, error) GetBass() (*models.Bass, error) - GetGroup() (*models.Group, error) NewWebSocketClient(config interface{}) *client.WebSocketClient }