diff --git a/pkg/service/soundtouchweb/browser_compatibility_test.go b/pkg/service/soundtouchweb/browser_compatibility_test.go index 6bcf8a04..1fff35a4 100644 --- a/pkg/service/soundtouchweb/browser_compatibility_test.go +++ b/pkg/service/soundtouchweb/browser_compatibility_test.go @@ -54,6 +54,27 @@ func newPlayerFixtureServer(t *testing.T, moduleScript string, configure func(ch return server } +// providerFixtureScript renders a RADIO_BROWSER source, which the speaker +// advertises READY but cannot act on without a station ContentItem. +const providerFixtureScript = ` +import { h, render } from 'preact'; +import { Sources } from '/app/static/js/components/Sources.js'; +window.navigated = []; +render(h(Sources, { + deviceId: 'speaker', + status: { + revision: 1, + nowPlayingRevision: 1, + sources: { SourceItem: [ + { Source: 'RADIO_BROWSER', SourceAccount: '', DisplayName: 'RadioBrowser', Status: 'READY' }, + ] }, + nowPlaying: { Source: 'SPOTIFY', SourceAccount: 'someone' }, + }, + onNavigate: page => window.navigated.push(page), + readbackDelays: [100, 250, 500], +}), document.getElementById('fixture')); +` + const sourceFixtureScript = ` import { h, render } from 'preact'; import { Sources } from '/app/static/js/components/Sources.js'; @@ -508,6 +529,163 @@ func TestSourceSelectionUsesOneWriteAndAbsoluteReadbacks(t *testing.T) { // 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. +// TestProviderSourceResumesMostRecentStation: RADIO_BROWSER is advertised +// READY but is not a selectable input. A bare /select strands the speaker on a +// stub now-playing (empty type and location, no playStatus) while the previous +// audio keeps playing, and the speaker then reports that stub indefinitely. +// Playing the most recent station for the source sends a real ContentItem. +func TestProviderSourceResumesMostRecentStation(t *testing.T) { + var mu sync.Mutex + var played map[string]any + selects := 0 + server := newPlayerFixtureServer(t, providerFixtureScript, func(r chi.Router) { + r.Get("/api/control/devices/speaker/recents", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true,"data":{"Items":[ + {"ID":1,"ContentItem":{"Source":"SPOTIFY","Location":"spotify:track:x"}}, + {"ID":2,"ContentItem":{"Source":"RADIO_BROWSER","Type":"stationurl", + "Location":"/station/abc","ItemName":"Some Station","IsPresetable":true}} + ]}}`)) + }) + r.Post("/api/control/devices/speaker/play", func(w http.ResponseWriter, req *http.Request) { + mu.Lock() + _ = json.NewDecoder(req.Body).Decode(&played) + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + selects++ + mu.Unlock() + 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) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true,"data":{"status":{"revision":9,"nowPlayingRevision":9,` + + `"webSocketConnected":true,"nowPlaying":{"Source":"RADIO_BROWSER","SourceAccount":""}}}}`)) + }) + }) + + ctx := newHeadlessChromeContext(t) + var navigated []string + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn`, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selected'`, nil), + chromedp.Evaluate(`window.navigated`, &navigated), + ); err != nil { + t.Fatalf("exercise provider source with a recent station: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if selects != 0 { + t.Errorf("provider source issued %d bare selects, want 0", selects) + } + if len(navigated) != 0 { + t.Errorf("navigated to %v, want to stay and play the recent station", navigated) + } + // A Location is the whole point: without it the speaker gets the same stub. + if played["location"] != "/station/abc" || played["source"] != "RADIO_BROWSER" || + played["type"] != "stationurl" || played["itemName"] != "Some Station" { + t.Errorf("played ContentItem = %+v, want the recent RADIO_BROWSER station", played) + } +} + +// TestProviderSourceWithoutRecentsNavigatesInstead: with nothing to resume, the +// only options are stranding the speaker or sending the user somewhere useful. +func TestProviderSourceWithoutRecentsNavigatesInstead(t *testing.T) { + var mu sync.Mutex + writes := 0 + server := newPlayerFixtureServer(t, providerFixtureScript, func(r chi.Router) { + r.Get("/api/control/devices/speaker/recents", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true,"data":{"Items":[ + {"ID":1,"ContentItem":{"Source":"SPOTIFY","Location":"spotify:track:x"}} + ]}}`)) + }) + r.Post("/api/control/devices/speaker/play", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + writes++ + mu.Unlock() + }) + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + writes++ + mu.Unlock() + }) + }) + + ctx := newHeadlessChromeContext(t) + var navigated []string + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn`, chromedp.ByQuery), + chromedp.Poll(`window.navigated.length === 1`, nil), + chromedp.Evaluate(`window.navigated`, &navigated), + ); err != nil { + t.Fatalf("exercise provider source without recents: %v", err) + } + + if len(navigated) != 1 || navigated[0] != "radiobrowser" { + t.Errorf("navigated = %v, want [radiobrowser]", navigated) + } + + mu.Lock() + defer mu.Unlock() + if writes != 0 { + t.Errorf("issued %d writes for a provider with nothing to resume, want 0", writes) + } +} + +// TestProviderSourceNavigatesWhenRecentsFail: a recents lookup that errors must +// not fall through to the bare select this whole path exists to avoid. +func TestProviderSourceNavigatesWhenRecentsFail(t *testing.T) { + var mu sync.Mutex + writes := 0 + server := newPlayerFixtureServer(t, providerFixtureScript, func(r chi.Router) { + r.Get("/api/control/devices/speaker/recents", func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, "unavailable", http.StatusServiceUnavailable) + }) + r.Post("/api/control/devices/speaker/play", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + writes++ + mu.Unlock() + }) + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + writes++ + mu.Unlock() + }) + }) + + ctx := newHeadlessChromeContext(t) + var navigated []string + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn`, chromedp.ByQuery), + chromedp.Poll(`window.navigated.length === 1`, nil), + chromedp.Evaluate(`window.navigated`, &navigated), + ); err != nil { + t.Fatalf("exercise provider source with failing recents: %v", err) + } + + if len(navigated) != 1 || navigated[0] != "radiobrowser" { + t.Errorf("navigated = %v, want [radiobrowser]", navigated) + } + + mu.Lock() + defer mu.Unlock() + if writes != 0 { + t.Errorf("issued %d writes after a failed recents lookup, want 0", writes) + } +} + func TestSourceSelectionStopsReadbacksOnceTheEventStreamConfirms(t *testing.T) { var mu sync.Mutex reads := 0 diff --git a/pkg/service/soundtouchweb/static/js/api.js b/pkg/service/soundtouchweb/static/js/api.js index a9c5d896..b2a11b7b 100644 --- a/pkg/service/soundtouchweb/static/js/api.js +++ b/pkg/service/soundtouchweb/static/js/api.js @@ -80,6 +80,15 @@ export const api = { headers: JSON_HEADERS, body: JSON.stringify(item), }), + // Same request as play, but surfaces failures. Used by the source-selection + // command path, which reports an outcome and so must be able to tell a + // rejected write from an accepted one. play() keeps its response-level + // behaviour for the callers that already rely on it. + playChecked: (id, item) => checkedReq(`/api/control/devices/${id}/play`, { + method: 'POST', + headers: JSON_HEADERS, + body: JSON.stringify(item), + }), tuneInBrowse: (path) => req(path ? `/api/control/providers/tunein/navigate/${path}` : '/api/control/providers/tunein/navigate'), tuneInSearch: (q) => req(`/api/control/providers/tunein/search?q=${encodeURIComponent(q)}`), tuneInSearchNext: (cursor) => req(`/api/control/providers/tunein/search/next?cursor=${encodeURIComponent(cursor)}`), diff --git a/pkg/service/soundtouchweb/static/js/app.js b/pkg/service/soundtouchweb/static/js/app.js index ef23002e..11fd7022 100644 --- a/pkg/service/soundtouchweb/static/js/app.js +++ b/pkg/service/soundtouchweb/static/js/app.js @@ -94,7 +94,7 @@ export function mergeStatusUpdate(previous, deviceId, status) { }); } -function DeviceDetail({ deviceId, devices, onBack, onDevicesChanged, notify, onRemove, onStatusReadback }) { +function DeviceDetail({ deviceId, devices, onBack, onDevicesChanged, notify, onRemove, onStatusReadback, onNavigate }) { const device = devices[deviceId]; if (!device) { @@ -124,6 +124,7 @@ function DeviceDetail({ deviceId, devices, onBack, onDevicesChanged, notify, onR deviceId=${deviceId} status=${device.status} onStatusReadback=${status => onStatusReadback(deviceId, status)} + onNavigate=${onNavigate} /> <${StereoPair} deviceId=${deviceId} @@ -376,6 +377,7 @@ function App() { notify=${showToast} onRemove=${removeDevice} onStatusReadback=${mergeDeviceReadback} + onNavigate=${navigate} /> ` : page === 'tunein' ? html` <${TuneInBrowser} key="tunein-browser" devices=${devices} /> diff --git a/pkg/service/soundtouchweb/static/js/components/Sources.js b/pkg/service/soundtouchweb/static/js/components/Sources.js index eb305889..ddcfd530 100644 --- a/pkg/service/soundtouchweb/static/js/components/Sources.js +++ b/pkg/service/soundtouchweb/static/js/components/Sources.js @@ -14,6 +14,26 @@ const SOURCE_ICONS = { const SOURCE_READBACK_DELAYS_MS = [2000, 5000, 10000]; +// Sources the speaker advertises as READY but which are NOT selectable inputs. +// They are providers: playing one needs a station ContentItem carrying a +// Location (see stations.ResolveContentItem, which sets type="stationurl"). +// +// A bare /select with just source and account parks the speaker on a stub +// now-playing -- source="RADIO_BROWSER", empty type and location, itemName +// echoing the source name, no playStatus -- while the previous audio keeps +// playing. The speaker then reports that stub indefinitely, so the player +// shows a source the speaker is not actually playing. +// +// Verified against real hardware for RADIO_BROWSER. TUNEIN is listed here +// because ResolveContentItem treats it identically (both need a Location). +// LOCAL_INTERNET_RADIO and ALEXA are also advertised READY but are NOT listed: +// whether a bare select resumes something for them is unverified, and leaving +// them alone preserves today's behaviour. +const PROVIDER_SOURCES = { + RADIO_BROWSER: { page: 'radiobrowser', label: 'RadioBrowser' }, + TUNEIN: { page: 'tunein', label: 'TuneIn' }, +}; + function isErrorSource(source) { return source === 'INVALID_SOURCE' || source?.endsWith('_ERROR'); } @@ -30,6 +50,7 @@ export function Sources({ deviceId, status, onStatusReadback, + onNavigate, readbackDelays = SOURCE_READBACK_DELAYS_MS, }) { const [command, setCommand] = useState(null); @@ -109,7 +130,58 @@ export function Sources({ : ''; const availabilityId = sourcesStale ? 'source-stale-status' : null; + // Finds the most recent playable item for a source: the newest Recents + // entry for it that carries a Location. Recents entries hold the real + // ContentItem the speaker was given, which is exactly what a provider + // source needs and what a bare select cannot supply. + async function mostRecentPlayableFor(src) { + const response = await api.recents(deviceId); + const account = src.SourceAccount ?? ''; + const match = (response?.data?.Items ?? []).find(item => { + const ci = item?.ContentItem; + return ci?.Source === src.Source && ci?.Location && + sourceAccountsMatch(src.Source, ci.SourceAccount ?? '', account); + }); + + return match?.ContentItem ?? null; + } + async function select(src) { + const provider = PROVIDER_SOURCES[src.Source]; + if (!provider) { + await runSourceCommand(src, + () => api.selectSource(deviceId, src.Source, src.SourceAccount ?? '')); + + return; + } + + let item = null; + try { + item = await mostRecentPlayableFor(src); + } catch (_) { + item = null; + } + + // Nothing to resume, and a bare select would strand the speaker on a + // stub: send the user to the browser to pick a station instead. + if (!item) { + onNavigate?.(provider.page); + + return; + } + + await runSourceCommand(src, () => api.playChecked(deviceId, { + source: item.Source, + type: item.Type, + location: item.Location, + sourceAccount: item.SourceAccount, + itemName: item.ItemName, + containerArt: item.ContainerArt, + isPresetable: item.IsPresetable, + })); + } + + async function runSourceCommand(src, write) { clearReadbacks(); const generation = commandRef.current.generation + 1; const target = { source: src.Source, account: src.SourceAccount ?? '' }; @@ -216,7 +288,7 @@ export function Sources({ }); try { - await api.selectSource(deviceId, target.source, target.account); + await write(); } catch (error) { if (commandRef.current.active !== active) return; // A definitive refusal (4xx) means the speaker never saw the