mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-09-07 15:07:17 +00:00
fix(player): don't strand the speaker on a bare provider-source select
A speaker advertises RADIO_BROWSER and TUNEIN in /sources with
status="READY", so the player rendered them as ordinary source buttons and
issued the same bare /select it uses for AUX or SPOTIFY: source and account,
no ContentItem.
They are not selectable inputs. Playing one needs a station ContentItem
carrying a Location, which is what stations.ResolveContentItem builds and
what HandlePlayRadioBrowser sends. Given a bare select the speaker accepts
the command and parks on a stub now-playing instead, observed on real
hardware:
<nowPlaying source="RADIO_BROWSER" sourceAccount="">
<ContentItem source="RADIO_BROWSER" type="" location="" isPresetable="false">
<itemName>RADIO_BROWSER</itemName>
</ContentItem>
</nowPlaying>
Empty type, empty location, itemName echoing the source name, and no
playStatus, while the previous audio keeps playing. The speaker then reports
that stub indefinitely, so the player shows RadioBrowser while Spotify is
audible. Worse, the readback sees the source it asked for and confirms
"Source selected" for a command that produced a dead state.
Clicking such a source now resumes its most recent station, using the
Recents entry's own ContentItem, which is the real item the speaker was
given and carries the Location a bare select cannot supply. With nothing to
resume, or if the lookup fails, the click navigates to that provider's
browser rather than issuing a select known to strand the speaker.
Only RADIO_BROWSER and TUNEIN are treated this way. LOCAL_INTERNET_RADIO and
ALEXA are advertised READY too, but whether a bare select resumes anything
for them is unverified, so they keep today's behaviour.
api.playChecked mirrors api.selectSource: the command path needs a write
whose failure it can see, while api.play keeps its response-level behaviour
for the callers that already rely on it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
7bd2a3d9f5
commit
21af353472
@@ -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
|
||||
|
||||
@@ -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)}`),
|
||||
|
||||
@@ -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} />
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user