diff --git a/cmd/soundtouch-service/testdata/router_routes.txt b/cmd/soundtouch-service/testdata/router_routes.txt index 1b0260a3..33b2ee3c 100644 --- a/cmd/soundtouch-service/testdata/router_routes.txt +++ b/cmd/soundtouch-service/testdata/router_routes.txt @@ -43,6 +43,7 @@ GET /api/control/devices/{id}/ soundtouch GET /api/control/devices/{id}/action/{action} soundtouchweb.(*WebApp).HandleAPIControl-fm GET /api/control/devices/{id}/library/browse soundtouchweb.(*WebApp).HandleLibraryBrowse-fm GET /api/control/devices/{id}/library/servers soundtouchweb.(*WebApp).HandleDeviceLibraryServers-fm +GET /api/control/devices/{id}/now-playing soundtouchweb.(*WebApp).HandleDeviceNowPlaying-fm GET /api/control/devices/{id}/power-status soundtouchweb.(*WebApp).HandleDevicePowerStatus-fm GET /api/control/devices/{id}/recents soundtouchweb.(*WebApp).HandleDeviceRecents-fm GET /api/control/devices/{id}/stereo-pair/ soundtouchweb.(*WebApp).HandleGetStereoPair-fm diff --git a/pkg/service/soundtouchweb/browser_compatibility_test.go b/pkg/service/soundtouchweb/browser_compatibility_test.go index f4b410ab..6bcf8a04 100644 --- a/pkg/service/soundtouchweb/browser_compatibility_test.go +++ b/pkg/service/soundtouchweb/browser_compatibility_test.go @@ -427,7 +427,7 @@ func TestSourceSelectionUsesOneWriteAndAbsoluteReadbacks(t *testing.T) { } _ = json.NewEncoder(w).Encode(webtypes.APIResponse{Success: true}) }) - r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { mu.Lock() run := &runs[len(runs)-1] run.readTimes = append(run.readTimes, time.Since(run.startedAt)) @@ -504,6 +504,88 @@ func TestSourceSelectionUsesOneWriteAndAbsoluteReadbacks(t *testing.T) { } } +// TestSourceSelectionStopsReadbacksOnceTheEventStreamConfirms: when the +// speaker's event stream is live it will report a late rejection on its own, +// so a confirmed selection must not keep polling. This is the difference +// between one readback per source tap and three. +func TestSourceSelectionStopsReadbacksOnceTheEventStreamConfirms(t *testing.T) { + var mu sync.Mutex + reads := 0 + server := newPlayerFixtureServer(t, sourceFixtureScript, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + reads++ + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true,"data":{"status":{"revision":9,"nowPlayingRevision":9,` + + `"webSocketConnected":true,"nowPlaying":{"Source":"AUX","SourceAccount":"AUX1"}}}}`)) + }) + }) + + ctx := newHeadlessChromeContext(t) + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn:nth-child(1)`, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selected'`, nil), + // Outlast the remaining readback deadlines (250ms and 500ms here). + chromedp.Sleep(900*time.Millisecond), + ); err != nil { + t.Fatalf("exercise event-stream-confirmed selection: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if reads != 1 { + t.Errorf("readbacks with a live event stream = %d, want 1", reads) + } +} + +// TestSourceSelectionKeepsReadbacksWithoutAnEventStream is the other half: +// with no event stream to watch for a late rejection, the readbacks are the +// only watcher and must run to the end of their window. +func TestSourceSelectionKeepsReadbacksWithoutAnEventStream(t *testing.T) { + var mu sync.Mutex + reads := 0 + server := newPlayerFixtureServer(t, sourceFixtureScript, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + reads++ + readCount := reads + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprintf(w, `{"success":true,"data":{"status":{"revision":%d,"nowPlayingRevision":%d,`+ + `"webSocketConnected":false,"nowPlaying":{"Source":"AUX","SourceAccount":"AUX1"}}}}`, + readCount+8, readCount+8) + }) + }) + + ctx := newHeadlessChromeContext(t) + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn:nth-child(1)`, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selected'`, nil), + chromedp.Sleep(300*time.Millisecond), + ); err != nil { + t.Fatalf("exercise selection without an event stream: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if reads != len([]int{100, 250, 500}) { + t.Errorf("readbacks without an event stream = %d, want 3", reads) + } +} + func TestSourceSelectionTreatsSelfAccountAsOmitted(t *testing.T) { const fixture = ` import { h, render } from 'preact'; @@ -533,7 +615,7 @@ render(h(Sources, { w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"success":true}`)) }) - r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"success":true,"data":{"status":{"revision":2,"nowPlayingRevision":2,"nowPlaying":{"Source":"AUX","SourceAccount":""}}}}`)) }) @@ -574,7 +656,7 @@ func TestSourceSelectionReadbacksDoNotWaitForSlowWriteResponse(t *testing.T) { w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"success":true}`)) }) - r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { mu.Lock() readTimes = append(readTimes, time.Since(startedAt)) readCount := len(readTimes) @@ -619,7 +701,7 @@ func TestSourceSelectionDefinitiveRefusalFailsImmediately(t *testing.T) { w.WriteHeader(http.StatusNotFound) _, _ = w.Write([]byte(`{"success":false,"error":"Device not found"}`)) }) - r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { mu.Lock() reads++ mu.Unlock() @@ -663,7 +745,7 @@ func TestSourceSelectionLaterFirmwareErrorOverridesProvisionalConfirmation(t *te w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"success":true}`)) }) - r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { mu.Lock() reads++ read := reads @@ -714,7 +796,7 @@ func TestSourceSelectionKeepsPushConfirmationWhenReadbacksFail(t *testing.T) { _, _ = w.Write([]byte(`{"success":true}`)) }) // Every readback fails, so only the pushed status can confirm anything. - r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { http.Error(w, "unavailable", http.StatusServiceUnavailable) }) }) @@ -748,7 +830,7 @@ func TestSourceSelectionRejectsReadbackWithOnlyNewerAggregateRevision(t *testing w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"success":true}`)) }) - r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { mu.Lock() reads++ mu.Unlock() @@ -782,7 +864,7 @@ func TestSourceSelectionTreatsFirmwareErrorSourceAsFailed(t *testing.T) { w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"success":true}`)) }) - r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { mu.Lock() reads++ read := reads @@ -829,7 +911,7 @@ func TestSourceSelectionLaterAuthoritativeSourceClearsFinalProjection(t *testing w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"success":true}`)) }) - r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { mu.Lock() reads++ revision := reads + 1 @@ -899,7 +981,7 @@ render(h(Fixture), document.getElementById('fixture')); w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"success":true}`)) }) - r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { mu.Lock() reads++ revision := reads + 1 @@ -948,7 +1030,7 @@ func TestSourceSelectionResetsWhenDeviceChanges(t *testing.T) { w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"success":true}`)) }) - r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"success":true}`)) }) @@ -990,7 +1072,7 @@ func TestSourceSelectionFencesOlderReadbackAndStatus(t *testing.T) { w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(webtypes.APIResponse{Success: true}) }) - r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { mu.Lock() source := currentSource readRevision++ diff --git a/pkg/service/soundtouchweb/handler.go b/pkg/service/soundtouchweb/handler.go index 53ceae77..eec19d0c 100644 --- a/pkg/service/soundtouchweb/handler.go +++ b/pkg/service/soundtouchweb/handler.go @@ -411,6 +411,59 @@ func (app *WebApp) HandleAPIDevices(w http.ResponseWriter, _ *http.Request) { } } +// HandleDeviceNowPlaying refreshes ONLY /now_playing and returns the device in +// the same shape as HandleAPIDevice. +// +// It exists for the player's source-selection readback, which needs one +// question answered ("what is the speaker playing now?") and nothing else. +// Going through HandleAPIDevice for that costs a full UpdateDeviceStatus, six +// sequential speaker calls plus /getGroup on a stereo-capable model, against a +// device the readback may well be checking on precisely because it is slow. +// +// The refresh runs under FieldNowPlaying's generation, so it orders against +// push events and concurrent polls exactly like any other now-playing write, +// and it reports its outcome to the health tracker like any other HTTP round. +func (app *WebApp) HandleDeviceNowPlaying(w http.ResponseWriter, r *http.Request) { + deviceID := chi.URLParam(r, "id") + if deviceID == "" { + app.sendError(w, "Device ID required", http.StatusBadRequest) + return + } + + device, exists := app.GetDevice(deviceID) + if !exists { + app.sendError(w, "Device not found", http.StatusNotFound) + return + } + + if device.Client != nil { + generation := device.BeginFieldPoll(webtypes.FieldNowPlaying) + pollGeneration := device.BeginHTTPPoll() + + nowPlaying, err := device.Client.GetNowPlaying() + if err == nil { + device.CompleteFieldPoll(webtypes.FieldNowPlaying, generation, func(status *webtypes.DeviceStatus) { + status.NowPlaying = nowPlaying + status.LastActivity = time.Now() + }) + } + + device.CompleteHTTPPoll(pollGeneration, err == nil, time.Now(), nil) + } + + view, visible := app.deviceViewForID(deviceID) + if !visible { + app.sendError(w, "Device not found", http.StatusNotFound) + return + } + + w.Header().Set("Content-Type", "application/json") + + if err := json.NewEncoder(w).Encode(webtypes.APIResponse{Success: true, Data: view}); err != nil { + http.Error(w, "Failed to encode response", http.StatusInternalServerError) + } +} + // HandleAPIDevice returns a specific device as JSON func (app *WebApp) HandleAPIDevice(w http.ResponseWriter, r *http.Request) { deviceID := chi.URLParam(r, "id") diff --git a/pkg/service/soundtouchweb/mount.go b/pkg/service/soundtouchweb/mount.go index 7cc185f2..a50c73b7 100644 --- a/pkg/service/soundtouchweb/mount.go +++ b/pkg/service/soundtouchweb/mount.go @@ -69,6 +69,10 @@ func (app *WebApp) MountWeb(r chi.Router, discoveryService *discovery.UnifiedDis r.Post("/power", app.HandleDevicePower) r.Get("/power-status", app.HandleDevicePowerStatus) r.Get("/recents", app.HandleDeviceRecents) + // Now-playing-only refresh. The player's source-selection + // readback uses this instead of the full device fetch, which + // would poll every field to answer one question. + r.Get("/now-playing", app.HandleDeviceNowPlaying) // Low-level "play this ContentItem" primitive (not a provider). r.Post("/play", app.HandleDevicePlay) // Generic key / preset / source / bass actions. Source selection is diff --git a/pkg/service/soundtouchweb/source_cache_test.go b/pkg/service/soundtouchweb/source_cache_test.go index 49240b8d..ab6a90ec 100644 --- a/pkg/service/soundtouchweb/source_cache_test.go +++ b/pkg/service/soundtouchweb/source_cache_test.go @@ -4,6 +4,8 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "reflect" + "sync" "testing" "github.com/gesellix/bose-soundtouch/pkg/client" @@ -169,3 +171,73 @@ func TestHandleAPIDevicePublishesCanonicalReadback(t *testing.T) { t.Fatalf("response did not publish canonical status: response=%+v status=%+v", response.Data.Status, canonical) } } + +// TestHandleDeviceNowPlayingPollsOnlyNowPlaying is the point of the endpoint: +// the source-selection readback needs one question answered, and going +// through the full device fetch would poll every field to answer it. +func TestHandleDeviceNowPlayingPollsOnlyNowPlaying(t *testing.T) { + var mu sync.Mutex + var paths []string + speaker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + paths = append(paths, r.URL.Path) + mu.Unlock() + + if r.URL.Path != "/now_playing" { + http.NotFound(w, r) + + return + } + + w.Header().Set("Content-Type", "application/xml") + _, _ = w.Write([]byte(`PLAY_STATE`)) + })) + defer speaker.Close() + + app := NewWebApp() + conn := webtypes.NewDeviceConnection( + client.NewClient(&client.Config{Host: speaker.URL}), + &models.DeviceInfo{Name: "Speaker"}, + ) + conn.SetStatus(&webtypes.DeviceStatus{NowPlaying: &models.NowPlaying{Source: "STANDBY"}}) + baseline := conn.Status().NowPlayingRevision + app.AddDevice("speaker", conn) + + req := httptest.NewRequest(http.MethodGet, "/api/control/devices/speaker/now-playing", nil) + req = withChiParams(req, map[string]string{"id": "speaker"}) + w := httptest.NewRecorder() + app.HandleDeviceNowPlaying(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("now-playing readback = %d: %s", w.Code, w.Body.String()) + } + + mu.Lock() + defer mu.Unlock() + if want := []string{"/now_playing"}; !reflect.DeepEqual(paths, want) { + t.Fatalf("speaker requests = %v, want only %v", paths, want) + } + + var response struct { + Success bool `json:"success"` + Data struct { + Status webtypes.DeviceStatus `json:"status"` + } `json:"data"` + } + if err := json.NewDecoder(w.Body).Decode(&response); err != nil { + t.Fatalf("decode now-playing readback: %v", err) + } + + canonical := conn.Status() + if !response.Success || canonical.NowPlaying.Source != "AUX" { + t.Fatalf("now-playing was not merged: response=%+v status=%+v", response, canonical) + } + // The readback confirms a source by comparing this revision, so it has to + // carry the same values the connection now holds. + if response.Data.Status.NowPlayingRevision != canonical.NowPlayingRevision || + response.Data.Status.Revision != canonical.Revision || + canonical.NowPlayingRevision <= baseline { + t.Fatalf("response did not publish canonical revisions: response=%+v status=%+v", + response.Data.Status, canonical) + } +} diff --git a/pkg/service/soundtouchweb/static/js/api.js b/pkg/service/soundtouchweb/static/js/api.js index 59a8bd82..a9c5d896 100644 --- a/pkg/service/soundtouchweb/static/js/api.js +++ b/pkg/service/soundtouchweb/static/js/api.js @@ -39,6 +39,9 @@ async function checkedReq(url, opts = {}) { export const api = { devices: () => req('/api/control/devices'), device: (id) => req(`/api/control/devices/${id}`), + // Refreshes only /now_playing. Used by the source-selection readback, + // which would otherwise poll every field to answer one question. + deviceNowPlaying: (id) => req(`/api/control/devices/${id}/now-playing`), removeDevice: (id) => req(`/api/control/devices/${id}`, { method: 'DELETE' }), discover: () => req('/api/control/discover', { method: 'POST' }), key: (id, key) => req(`/api/control/devices/${id}/key/${key}`, { method: 'POST' }), diff --git a/pkg/service/soundtouchweb/static/js/components/Sources.js b/pkg/service/soundtouchweb/static/js/components/Sources.js index 23ac8c7d..eb305889 100644 --- a/pkg/service/soundtouchweb/static/js/components/Sources.js +++ b/pkg/service/soundtouchweb/static/js/components/Sources.js @@ -152,7 +152,7 @@ export function Sources({ active.latestReadback = index; try { - const response = await api.device(deviceId); + const response = await api.deviceNowPlaying(deviceId); if (commandRef.current.active !== active || active.latestReadback !== index) return; const readbackStatus = response?.data?.status; @@ -174,15 +174,29 @@ export function Sources({ }); } else if (revisionIsNewer && nowPlaying?.Source === target.source && sourceAccountsMatch(target.source, nowPlaying?.SourceAccount, target.account)) { + // A match is not the end of the story: /select answers + // 200 even for a source the speaker goes on to reject a + // few seconds later, which surfaces as a transition to + // an error source. Something has to keep watching. + // + // The event stream is the better watcher when it is + // live: nowPlayingUpdated reports that transition as it + // happens, and the effect above already turns it into a + // failure. Polling on top of that only re-asks a + // question we are already subscribed to the answer of. + // So keep the remaining readbacks only as a fallback + // for a device whose events we are not receiving. const isFinalReadback = index === readbackDelays.length - 1; + const eventStreamWatching = readbackStatus?.webSocketConnected === true; + const settled = isFinalReadback || eventStreamWatching; setCommand({ ...target, generation, - outcome: isFinalReadback ? 'final-confirmed' : 'provisional-confirmed', + outcome: settled ? 'final-confirmed' : 'provisional-confirmed', startNowPlayingRevision: nowPlayingRevision, confirmedRevision: readbackRevision, }); - if (isFinalReadback) { + if (settled) { clearReadbacks(); commandRef.current.active = null; }