From 0d15bce96fbae67639c2a2d299e06c76baf0cba1 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: Tue, 25 Aug 2026 18:26:17 +0200 Subject: [PATCH] fix(service): retry persisted player devices after startup --- cmd/soundtouch-service/main.go | 17 ++- pkg/service/soundtouchweb/discovery.go | 133 ++++++++++++++++++-- pkg/service/soundtouchweb/discovery_test.go | 84 +++++++++++++ pkg/service/soundtouchweb/handler.go | 22 ++++ 4 files changed, 245 insertions(+), 11 deletions(-) diff --git a/cmd/soundtouch-service/main.go b/cmd/soundtouch-service/main.go index 849593d5..091fa62d 100644 --- a/cmd/soundtouch-service/main.go +++ b/cmd/soundtouch-service/main.go @@ -46,6 +46,11 @@ var ( repoURL = "https://github.com/gesellix/bose-soundtouch" ) +const ( + embeddedDeviceSeedRetryInterval = 30 * time.Second + embeddedDeviceSeedRetryWindow = 10 * time.Minute +) + func updateBuildInfo() { if info, ok := debug.ReadBuildInfo(); ok { if info.Main.Path != "" { @@ -1478,10 +1483,14 @@ func newEmbeddedWebApp(server *handlers.Server, serverURL, internalURL string, d }) go func() { - // Project the current device set into the UI; the devices-changed hook - // and the service's periodic discovery keep it current from here on. - webApp.SeedExtraDevices() - webApp.BroadcastDeviceList() + // Project the current device set into the UI. During gateway boot the + // service can start before persisted speaker addresses are routable, so + // retry only those known addresses for a bounded startup window. The + // devices-changed hook and explicit discovery keep it current afterwards. + ctx, cancel := context.WithTimeout(context.Background(), embeddedDeviceSeedRetryWindow) + defer cancel() + + webApp.SeedExtraDevicesUntilReady(ctx, embeddedDeviceSeedRetryInterval) }() return webApp diff --git a/pkg/service/soundtouchweb/discovery.go b/pkg/service/soundtouchweb/discovery.go index 5da69cbf..71e5681a 100644 --- a/pkg/service/soundtouchweb/discovery.go +++ b/pkg/service/soundtouchweb/discovery.go @@ -64,9 +64,13 @@ func NewDiscoveryService(discoveryInterface string, configuredHosts ...string) * // mDNS/UPnP. If the host is already known, the existing entry's // LastSeen is bumped and the function returns without re-fetching. func (app *WebApp) AddDeviceByHost(host string, port int, source string) { + app.addDeviceByHost(host, port, source) +} + +func (app *WebApp) addDeviceByHost(host string, port int, source string) *webtypes.DeviceConnection { // Fast path: skip the network call if we already know this host. if app.TouchDevice(host) { - return + return nil } c := client.NewClient(&client.Config{ @@ -78,7 +82,7 @@ func (app *WebApp) AddDeviceByHost(host string, port int, source string) { info, err := c.GetDeviceInfo() if err != nil { log.Printf("Failed to fetch device info from %s (%s): %v", sanitizeLog(host), sanitizeLog(source), err) - return + return nil } // Ensure IPAddress is set for the web UI @@ -91,7 +95,7 @@ func (app *WebApp) AddDeviceByHost(host string, port int, source string) { // Lost a race — another goroutine inserted the same host // between TouchDevice and AddDevice. AddDevice bumped LastSeen // on the existing entry; discard our conn. - return + return nil } go app.UpdateDeviceStatus(host, conn) @@ -114,6 +118,8 @@ func (app *WebApp) AddDeviceByHost(host string, port int, source string) { }() log.Printf("Added %s device %s (%s) at %s:%d", sanitizeLog(source), sanitizeLog(info.Name), sanitizeLog(info.Type), sanitizeLog(host), port) + + return conn } // SeedExtraDevices registers any devices reported by the ExtraDeviceHosts hook @@ -127,14 +133,26 @@ func (app *WebApp) AddDeviceByHost(host string, port int, source string) { // out bounds the cost to roughly a single timeout regardless of how many // devices are offline. AddDeviceByHost is registry-safe under concurrency. func (app *WebApp) SeedExtraDevices() { + app.seedExtraDevices() +} + +type seededExtraDevice struct { + host string + conn *webtypes.DeviceConnection +} + +func (app *WebApp) seedExtraDevices() []seededExtraDevice { if app.ExtraDeviceHosts == nil { - return + return nil } + hosts := app.extraDeviceHosts() + inserted := make(chan seededExtraDevice, len(hosts)) + var wg sync.WaitGroup - for _, host := range app.ExtraDeviceHosts() { - if host == "" { + for _, host := range hosts { + if _, ok := app.GetDevice(host); ok { continue } @@ -143,11 +161,112 @@ func (app *WebApp) SeedExtraDevices() { go func(h string) { defer wg.Done() - app.AddDeviceByHost(h, 8090, "service-store") + if conn := app.addDeviceByHost(h, 8090, "service-store"); conn != nil { + inserted <- seededExtraDevice{host: h, conn: conn} + } }(host) } wg.Wait() + close(inserted) + + added := make([]seededExtraDevice, 0, len(inserted)) + for device := range inserted { + added = append(added, device) + } + + return added +} + +// SeedExtraDevicesUntilReady retries only the hosts returned by +// ExtraDeviceHosts until all of them have been registered or ctx expires. It +// does not run mDNS or UPnP discovery. This gives embedded deployments a +// bounded way to recover when their persisted speakers are not yet routable +// while the service is starting. +func (app *WebApp) SeedExtraDevicesUntilReady(ctx context.Context, retryInterval time.Duration) { + retryUntilReady(ctx, retryInterval, func() bool { + inserted := app.seedExtraDevices() + desired := app.extraDeviceHostSet() + + for _, device := range inserted { + if _, ok := desired[device.host]; !ok { + app.removeDeviceIfMatch(device.host, device.conn) + } + } + + if len(inserted) > 0 { + app.BroadcastDeviceList() + } + + return app.extraDeviceHostsPresent(app.extraDeviceHostSet()) + }) +} + +func (app *WebApp) extraDeviceHosts() []string { + if app.ExtraDeviceHosts == nil { + return nil + } + + hosts := make([]string, 0) + seen := make(map[string]struct{}) + + for _, host := range app.ExtraDeviceHosts() { + if host == "" { + continue + } + + if _, ok := seen[host]; ok { + continue + } + + seen[host] = struct{}{} + hosts = append(hosts, host) + } + + return hosts +} + +func (app *WebApp) extraDeviceHostSet() map[string]struct{} { + hosts := app.extraDeviceHosts() + desired := make(map[string]struct{}, len(hosts)) + + for _, host := range hosts { + desired[host] = struct{}{} + } + + return desired +} + +func (app *WebApp) extraDeviceHostsPresent(desired map[string]struct{}) bool { + for host := range desired { + if _, ok := app.GetDevice(host); !ok { + return false + } + } + + return true +} + +func retryUntilReady(ctx context.Context, retryInterval time.Duration, attempt func() bool) { + for { + select { + case <-ctx.Done(): + return + default: + } + + if attempt() { + return + } + + timer := time.NewTimer(retryInterval) + select { + case <-ctx.Done(): + timer.Stop() + return + case <-timer.C: + } + } } // DiscoverDevices refreshes the device registry. When TriggerDiscovery is set diff --git a/pkg/service/soundtouchweb/discovery_test.go b/pkg/service/soundtouchweb/discovery_test.go index e63976be..9492e7c0 100644 --- a/pkg/service/soundtouchweb/discovery_test.go +++ b/pkg/service/soundtouchweb/discovery_test.go @@ -8,6 +8,9 @@ import ( "sync/atomic" "testing" "time" + + "github.com/gesellix/bose-soundtouch/pkg/models" + "github.com/gesellix/bose-soundtouch/pkg/service/soundtouchweb/webtypes" ) func TestDiscoverDevicesRetriesConfiguredHosts(t *testing.T) { @@ -97,3 +100,84 @@ func TestClassifySource(t *testing.T) { } } } + +func TestRetryUntilReadyStopsAfterSuccess(t *testing.T) { + attempts := 0 + retryUntilReady(context.Background(), time.Millisecond, func() bool { + attempts++ + return attempts == 3 + }) + + if attempts != 3 { + t.Fatalf("attempt count = %d, want 3", attempts) + } +} + +func TestRetryUntilReadyStopsAfterContextCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + attempts := 0 + + retryUntilReady(ctx, time.Hour, func() bool { + attempts++ + cancel() + + return false + }) + + if attempts != 1 { + t.Fatalf("attempt count = %d, want 1", attempts) + } +} + +func TestExtraDeviceHostsPresent(t *testing.T) { + app := NewWebApp() + app.ExtraDeviceHosts = func() []string { return []string{"known", "", "known", "missing"} } + app.AddDevice("known", &webtypes.DeviceConnection{}) + desired := app.extraDeviceHostSet() + + if len(desired) != 2 { + t.Fatalf("desired host count = %d, want 2", len(desired)) + } + if app.extraDeviceHostsPresent(desired) { + t.Fatal("extraDeviceHostsPresent = true with a missing host") + } + + app.AddDevice("missing", &webtypes.DeviceConnection{}) + if !app.extraDeviceHostsPresent(desired) { + t.Fatal("extraDeviceHostsPresent = false with all hosts registered") + } +} + +func TestSeedExtraDevicesSkipsKnownHosts(t *testing.T) { + app := NewWebApp() + app.ExtraDeviceHosts = func() []string { return []string{"known"} } + lastSeen := time.Unix(123, 0) + conn := &webtypes.DeviceConnection{LastSeen: lastSeen} + app.AddDevice("known", conn) + + app.SeedExtraDevices() + + if !conn.LastSeen.Equal(lastSeen) { + t.Fatalf("known host LastSeen changed from %s to %s", lastSeen, conn.LastSeen) + } +} + +func TestRemoveDeviceIfMatchKeepsReplacement(t *testing.T) { + app := NewWebApp() + original := webtypes.NewDeviceConnection(nil, &models.DeviceInfo{}) + replacement := webtypes.NewDeviceConnection(nil, &models.DeviceInfo{}) + app.AddDevice("speaker", original) + app.RemoveDevice("speaker") + app.AddDevice("speaker", replacement) + + if app.removeDeviceIfMatch("speaker", original) { + t.Fatal("removeDeviceIfMatch removed a replacement connection") + } + + got, ok := app.GetDevice("speaker") + if !ok || got != replacement { + t.Fatal("replacement connection was not preserved") + } + + app.RemoveDevice("speaker") +} diff --git a/pkg/service/soundtouchweb/handler.go b/pkg/service/soundtouchweb/handler.go index 0868f0b3..6964de72 100644 --- a/pkg/service/soundtouchweb/handler.go +++ b/pkg/service/soundtouchweb/handler.go @@ -214,6 +214,28 @@ func (app *WebApp) RemoveDevice(id string) bool { return ok } +// removeDeviceIfMatch removes id only when it still points at expected. It is +// used when an asynchronous probe must not delete a newer replacement that was +// registered under the same host. +func (app *WebApp) removeDeviceIfMatch(id string, expected *webtypes.DeviceConnection) bool { + app.devicesMu.Lock() + + current, ok := app.devices[id] + if ok && current == expected { + delete(app.devices, id) + } else { + ok = false + } + + app.devicesMu.Unlock() + + if ok { + expected.Close() + } + + return ok +} + // HandleAPIDevices returns all devices as JSON func (app *WebApp) HandleAPIDevices(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json")