diff --git a/docs/guides/TROUBLESHOOTING.md b/docs/guides/TROUBLESHOOTING.md index 9e73733..e99b97a 100644 --- a/docs/guides/TROUBLESHOOTING.md +++ b/docs/guides/TROUBLESHOOTING.md @@ -130,6 +130,37 @@ Expected when the misconfiguration is present: `443=000` plus a `curl: (7) Faile **Fix:** route `:443` to AfterTouch's HTTPS listener — see [HTTPS-SETUP.md → Binding to port 443](HTTPS-SETUP.md#binding-to-port-443). The AfterTouch settings page shows a ✅ / ❌ indicator for `:443` reachability once the routing is in place. +### ❌ Presets flash then revert to "Select a preset" after a factory reset + +**Symptoms:** + +- You factory-reset a SoundTouch (Wave / 10 / 20 / 30 / …) that was previously migrated. +- After reconnecting it to Wi-Fi, AfterTouch sees the speaker again, but pressing a preset on the device or in the app makes the display briefly show the preset name and then revert to *"Select a preset or explore music in the SoundTouch App"*. +- Spotify presets show the same revert unless Spotify Connect is started from the mobile app first. +- The speaker's `/sources` is missing TUNEIN / LOCAL_INTERNET_RADIO / DEEZER / your linked Spotify account — only AUX, BLUETOOTH, AIRPLAY, the SpotifyConnectUserName placeholder, NOTIFICATION, and QPLAY appear. + +**Cause:** + +A factory reset wipes `/mnt/nv/BoseApp-Persistence/1/Marge.xml` — the file that carries the speaker's auth token for the AfterTouch (or Bose) cloud service. The migrated URL configuration is preserved (it lives in `envswitch`), so the speaker keeps talking to AfterTouch, but with no token it can't authenticate for preset playback. Separately, the device's `/sources` cache is reduced until it receives a `` notification. + +**Fix:** + +1. **Re-open the Migration tab** in the AfterTouch UI. The wizard reads `/info`, sees `margeAccountUUID` is empty, and renders: + + > **Current: ❌ Not paired (factory-reset or never paired) — set an ID to pair as part of Apply** + + The devices list now also shows a `⚠ Not paired — re-pair` badge next to such speakers, so you don't have to remember to open the Migration tab cold. + +2. **Pick the previously-used account ID** from the "pick from datastore" dropdown (if AfterTouch remembers it), or click **Generate** for a fresh one. + +3. **Click Apply.** The wizard runs `pair-account` along with the rest, recreating `Marge.xml` on the device with the chosen ID. + +4. **Click Data Sync** (Tab 3). AfterTouch persists the speaker's presets/recents/sources and posts a `` notification to the device — the missing TUNEIN / LOCAL_INTERNET_RADIO / DEEZER / linked Spotify entries reappear in `/sources` automatically. + +5. Press a preset. It should play normally. + +If presets still won't play after step 5, capture `logread -f | grep -v '127.0.0.1:'` on the speaker (see [DEVICE-LOGGING.md](../DEVICE-LOGGING.md#1-accessing-system-logs-requires-root)) while pressing the preset and file an issue with the snippet — the lines around the failed playback name the deeper cause. + ### ❌ "Connection refused" **Symptoms:** diff --git a/docs/images/ui-devices.png b/docs/images/ui-devices.png index a67b8c1..555a0d6 100644 Binary files a/docs/images/ui-devices.png and b/docs/images/ui-devices.png differ diff --git a/docs/images/ui-migration.png b/docs/images/ui-migration.png index 85f4703..0b16bc3 100644 Binary files a/docs/images/ui-migration.png and b/docs/images/ui-migration.png differ diff --git a/docs/images/ui-settings.png b/docs/images/ui-settings.png index dc2ecfd..cf06a9b 100644 Binary files a/docs/images/ui-settings.png and b/docs/images/ui-settings.png differ diff --git a/docs/images/ui-sync.png b/docs/images/ui-sync.png index b703c95..e368b22 100644 Binary files a/docs/images/ui-sync.png and b/docs/images/ui-sync.png differ diff --git a/pkg/service/handlers/web/js/script.js b/pkg/service/handlers/web/js/script.js index 728e20e..5e5a55e 100644 --- a/pkg/service/handlers/web/js/script.js +++ b/pkg/service/handlers/web/js/script.js @@ -1809,7 +1809,31 @@ async function updateDeviceInfo(deviceId, ip) { if (deviceIdEl && info.deviceID) deviceIdEl.innerText = info.deviceID; const accountIdEl = row.querySelector(".col-accountid"); - if (accountIdEl && info.margeAccountUUID) accountIdEl.innerText = info.margeAccountUUID; + if (accountIdEl) { + if (info.margeAccountUUID) { + accountIdEl.innerText = info.margeAccountUUID; + accountIdEl.style.color = "#666"; + } else { + // Empty in /info → speaker is + // either factory-reset or never paired. The + // Migration tab's wizard already detects this + // state and prompts for re-pairing; this badge + // surfaces the affordance from the devices list + // so users don't have to know to open the + // Migration tab cold. See issue #234. + accountIdEl.replaceChildren(); + const badge = document.createElement("a"); + badge.href = "#"; + badge.onclick = (e) => { + e.preventDefault(); + prepareMigration(deviceId); + }; + badge.innerText = "⚠ Not paired — re-pair"; + badge.style.color = "#c62828"; + badge.title = "Open the Migration tab to re-pair this speaker (factory-reset or never paired)."; + accountIdEl.appendChild(badge); + } + } } } catch (error) { console.warn("Failed to fetch live info for " + ip, error); diff --git a/pkg/service/setup/issue234_regression_test.go b/pkg/service/setup/issue234_regression_test.go index 2fce2a6..1e92cef 100644 --- a/pkg/service/setup/issue234_regression_test.go +++ b/pkg/service/setup/issue234_regression_test.go @@ -115,12 +115,22 @@ func TestIssue234_FactoryResetSpeakerSyncsReducedSources(t *testing.T) { t.Errorf("DeviceID = %q, want %q", info.DeviceID, "DEADBEEFCAFE") } - // 2. Source round-trip: reduced list survives sync verbatim. - const accountID = "issue234" + // 2. End-to-end sync. Driving SyncDeviceData rather than + // syncSources directly exercises the wiring between + // syncSources and notifySpeakerSourcesUpdated — the source + // list still lands on disk (assertions below) AND the + // sourcesUpdated notification fires against the device. + // SyncDeviceData derives accountID/deviceID from /info; with + // an empty margeAccountUUID the account falls through to + // "default". + if err := m.SyncDeviceData(deviceIP); err != nil { + t.Fatalf("SyncDeviceData: %v", err) + } - const deviceID = "DEADBEEFCAFE" - - m.syncSources(deviceIP, accountID, deviceID) + const ( + accountID = "default" + deviceID = "DEADBEEFCAFE" + ) sourcesPath := filepath.Join(tempDir, "accounts", accountID, "devices", deviceID, "Sources.xml") @@ -144,14 +154,46 @@ func TestIssue234_FactoryResetSpeakerSyncsReducedSources(t *testing.T) { } // Casualties: TUNEIN / LOCAL_INTERNET_RADIO are the symptom of - // #234 — they should remain absent until auto-recovery lands. + // #234 — they should remain absent on the persisted side + // because fakespeaker is stateless (the next /sources read + // returns the same reduced fixture even after the + // notification). On a real speaker the device would react to + // the notification, re-expose the missing sources, and the + // next Data Sync would persist them — that second-sync step + // is the runbook user-facing flow, not something we model + // here. for _, missingKey := range []string{ `) is up to encoding/xml and + // not protocol-meaningful — assert on the load-bearing pieces + // instead of the byte-identical body. + body := string(notifs[0].Body) + if !strings.Contains(body, `deviceID="DEADBEEFCAFE"`) { + t.Errorf("notification body missing deviceID; got: %q", body) + } + + if !strings.Contains(body, "sourcesUpdated") { + t.Errorf("notification body missing sourcesUpdated; got: %q", body) + } + + if !strings.Contains(notifs[0].ContentType, "xml") { + t.Errorf("notification Content-Type = %q, want something xml-shaped", notifs[0].ContentType) + } } diff --git a/pkg/service/setup/setup.go b/pkg/service/setup/setup.go index 4a275f1..32ccaa2 100644 --- a/pkg/service/setup/setup.go +++ b/pkg/service/setup/setup.go @@ -16,6 +16,7 @@ import ( "strings" "time" + "github.com/gesellix/bose-soundtouch/pkg/client" "github.com/gesellix/bose-soundtouch/pkg/models" "github.com/gesellix/bose-soundtouch/pkg/service/certmanager" @@ -2490,7 +2491,16 @@ func (m *Manager) SyncDeviceData(deviceIP string) error { // 4. Fetch Sources m.syncSources(deviceIP, accountID, deviceID) - // 5. Create off-device backup of system configuration + // 5. Nudge the device to re-render its source list. After a factory + // reset (issue #234) the speaker's /sources only lists the always-on + // local entries until it receives a notification; + // the reporter's workaround was to POST this by hand. Wiring it into + // the sync flow means the user gets the visible recovery for free + // after they click Data Sync — re-pairing (which the wizard already + // detects + prompts for) is the orthogonal half of the fix. + m.notifySpeakerSourcesUpdated(deviceIP, deviceID) + + // 6. Create off-device backup of system configuration _ = m.BackupConfigOffDevice(deviceIP) return nil @@ -2666,3 +2676,33 @@ func (m *Manager) syncSources(deviceIP, accountID, deviceID string) { _ = m.DataStore.SaveConfiguredSources(accountID, deviceID, configuredSources) } } + +// notifySpeakerSourcesUpdated POSTs the notification +// to /notification on the device, mirroring the manual workaround +// documented in issue #234. The device responds by re-evaluating its +// /sources catalogue — after a factory reset that's what makes TUNEIN / +// LOCAL_INTERNET_RADIO / DEEZER / linked Spotify accounts reappear in +// the list. The wizard's pair-account flow restores playback (it +// recreates the Marge.xml token); this nudge restores the *visible* +// source list. Both are needed for a full #234 recovery; this is the +// half AfterTouch can automate without user input. +// +// Delegates the HTTP plumbing to pkg/client.Client.NotifySourcesUpdated, +// which is the same path handlers_mgmt.go uses after music-service +// account changes — keeping the wire-shape definition in one place +// (pkg/models.NewSourcesUpdatedNotification). +// +// Fire-and-forget: a network failure (or the device returning an +// unexpected response) doesn't fail the surrounding sync. The sync's +// persisted state is already on disk by the time we fire the +// notification; whether the device acts on it is observable on the +// next sync. +func (m *Manager) notifySpeakerSourcesUpdated(deviceIP, deviceID string) { + c := client.NewClientFromHost(deviceIP) + if err := c.NotifySourcesUpdated(deviceID); err != nil { + log.Printf("[SYNC] notify %s: %v", deviceIP, err) + return + } + + log.Printf("[SYNC] notify %s sourcesUpdated -> ok", deviceIP) +} diff --git a/pkg/service/testing/fakespeaker/fakespeaker.go b/pkg/service/testing/fakespeaker/fakespeaker.go index 896da03..9944aaf 100644 --- a/pkg/service/testing/fakespeaker/fakespeaker.go +++ b/pkg/service/testing/fakespeaker/fakespeaker.go @@ -17,6 +17,7 @@ import ( "io" "net" "net/http" + "sync" "time" ) @@ -57,6 +58,34 @@ type Server struct { srv *http.Server httpAddr string telnet *telnetServer + + mu sync.Mutex + notifications []NotificationCall +} + +// NotificationCall records a single POST /notification request the +// fake received. Tests use it to assert that AfterTouch (or any +// other component under test) fired the expected speaker-side +// notification. +type NotificationCall struct { + // Body is the request body verbatim. + Body []byte + // ContentType is the value of the Content-Type header. + ContentType string +} + +// Notifications returns a snapshot of every POST /notification call +// the fake has received, in arrival order. The slice is independent +// of the server's internal state — callers can keep it for assertions +// without holding a lock. +func (s *Server) Notifications() []NotificationCall { + s.mu.Lock() + defer s.mu.Unlock() + + out := make([]NotificationCall, len(s.notifications)) + copy(out, s.notifications) + + return out } // Start binds the configured listeners and serves them in background @@ -73,17 +102,18 @@ func Start(cfg Config) (*Server, error) { return nil, fmt.Errorf("fakespeaker: listen %s: %w", httpListen, err) } - mux := http.NewServeMux() - registerRoutes(mux, cfg.FixtureOverrides) - s := &Server{ - srv: &http.Server{ - Handler: mux, - ReadHeaderTimeout: 5 * time.Second, - }, httpAddr: ln.Addr().String(), } + mux := http.NewServeMux() + registerRoutes(mux, cfg.FixtureOverrides, s) + + s.srv = &http.Server{ + Handler: mux, + ReadHeaderTimeout: 5 * time.Second, + } + go func() { _ = s.srv.Serve(ln) }() @@ -130,7 +160,7 @@ func (s *Server) Stop(ctx context.Context) error { return nil } -func registerRoutes(mux *http.ServeMux, overrides map[string][]byte) { +func registerRoutes(mux *http.ServeMux, overrides map[string][]byte, s *Server) { fixture := func(route, embedPath string) { mux.HandleFunc(route, serveFixtureOr(embedPath, overrides[route])) } @@ -147,6 +177,36 @@ func registerRoutes(mux *http.ServeMux, overrides map[string][]byte) { mux.HandleFunc("/addGroup", handleAddGroup) mux.HandleFunc("/updateGroup", handleUpdateGroup) mux.HandleFunc("/removeGroup", handleRemoveGroup) + mux.HandleFunc("/notification", s.handleNotification) +} + +// handleNotification records a POST /notification call so tests can +// assert that AfterTouch fired the expected speaker-side nudge (e.g. +// the notification that recovers the source list +// after a factory reset, per issue #234). GET returns 405 — real +// speakers expose /notification as POST-only. +func (s *Server) handleNotification(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + w.Header().Set("Allow", "POST") + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + + return + } + + body, _ := io.ReadAll(http.MaxBytesReader(w, r.Body, 64*1024)) + + s.mu.Lock() + s.notifications = append(s.notifications, NotificationCall{ + Body: body, + ContentType: r.Header.Get("Content-Type"), + }) + s.mu.Unlock() + + // Real speakers respond with /notification; the + // pkg/client.Client.NotifySourcesUpdated path validates that + // shape, so the fake has to match it too. + w.Header().Set("Content-Type", "application/xml; charset=utf-8") + _, _ = w.Write([]byte(`` + "\n/notification\n")) } // serveFixtureOr returns a handler that writes override (when non-nil) diff --git a/pkg/service/testing/fakespeaker/fakespeaker_test.go b/pkg/service/testing/fakespeaker/fakespeaker_test.go index b8e5b79..10b351a 100644 --- a/pkg/service/testing/fakespeaker/fakespeaker_test.go +++ b/pkg/service/testing/fakespeaker/fakespeaker_test.go @@ -228,6 +228,73 @@ func TestFakeSpeakerFixtureOverride_ReplacesEmbeddedBody(t *testing.T) { } } +func TestFakeSpeakerNotificationRecorder(t *testing.T) { + s, err := Start(Config{}) + if err != nil { + t.Fatalf("start: %v", err) + } + + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + _ = s.Stop(ctx) + }) + + body := `` + + req, err := http.NewRequest(http.MethodPost, + "http://"+s.HTTPAddr()+"/notification", + strings.NewReader(body)) + if err != nil { + t.Fatalf("build request: %v", err) + } + + req.Header.Set("Content-Type", "application/xml") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("post: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + t.Errorf("status = %d, want 200", resp.StatusCode) + } + + got := s.Notifications() + if len(got) != 1 { + t.Fatalf("Notifications() returned %d entries, want 1", len(got)) + } + + // The test POSTs the request body verbatim; the recorder must + // return it byte-identical. (Wire-shape variation — self-closing + // vs long-form sourcesUpdated — happens upstream in + // pkg/client.NotifySourcesUpdated, not here.) + if string(got[0].Body) != body { + t.Errorf("body = %q, want %q", got[0].Body, body) + } + + if got[0].ContentType != "application/xml" { + t.Errorf("ContentType = %q, want application/xml", got[0].ContentType) + } + + // GET on the same path is a 405 — real speakers don't expose it. + getResp, err := http.Get("http://" + s.HTTPAddr() + "/notification") //nolint:noctx + if err != nil { + t.Fatalf("get: %v", err) + } + defer func() { _ = getResp.Body.Close() }() + + if getResp.StatusCode != http.StatusMethodNotAllowed { + t.Errorf("GET status = %d, want 405", getResp.StatusCode) + } + + if got := getResp.Header.Get("Allow"); got != "POST" { + t.Errorf("Allow header = %q, want POST", got) + } +} + func TestFakeSpeakerRemoveGroupRejectsNonGET(t *testing.T) { s, err := Start(Config{}) if err != nil { diff --git a/pkg/service/testing/fakespeaker/testdata/info.xml b/pkg/service/testing/fakespeaker/testdata/info.xml index f2595d6..7ebd2d0 100644 --- a/pkg/service/testing/fakespeaker/testdata/info.xml +++ b/pkg/service/testing/fakespeaker/testdata/info.xml @@ -2,7 +2,7 @@ Demo SoundTouch SoundTouch 10 - 0000000 + 1234567 SCM