github/Bose-SoundTouch
1
0
Fork 0
mirror of https://github.com/gesellix/Bose-SoundTouch.git synced 2026-09-07 15:07:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(player): never confirm a source the speaker reports as not playing

Browse Source 
PROVIDER_SOURCES covers the sources known to produce the stub now-playing.
It cannot cover the ones we have not tested, ALEXA among them, and a source
list is whatever the speaker chooses to advertise.

So the readback now refuses to confirm the stub itself, wherever it comes
from: a now-playing that names the source we asked for but reports no
location, no play status, and an item name echoing the source is reported as
a failure rather than a success. That shape is what a speaker returns for a
select it accepted but cannot act on.

All three conditions are required together. A physical input reports no
location and no item name of its own yet is genuinely playing, so any one
condition alone would reject real selections; a test covers exactly that
case, confirming with PlayStatus set and no location.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-09-05 20:36:58 +02:00
co-authored by Claude Opus 5
parent 1e7dae35aa
commit 2c1ac79ddd
2 changed files with 103 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/service/soundtouchweb/browser_compatibility_test.go
+65
View File
@@ -702,6 +702,71 @@ func TestProviderSourceNavigatesWhenRecentsFail(t *testing.T) {
}
}
// TestStubNowPlayingIsNotReportedAsSuccess: PROVIDER_SOURCES only covers the
// sources known to produce the stub. For any other, the readback must not
// confirm a now-playing that names the source but reports nothing playing:
// no location, no play status, item name echoing the source.
func TestStubNowPlayingIsNotReportedAsSuccess(t *testing.T) {
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) {
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",` +
`"PlayStatus":"","ContentItem":{"Source":"AUX","Type":"","Location":"",` +
`"ItemName":"AUX","IsPresetable":false}}}}}`))
})
})
ctx := newHeadlessChromeContext(t)
var statusText string
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-btn:nth-child(1)').classList.contains('failed')`, nil),
chromedp.Text(`.source-command-status`, &statusText, chromedp.ByQuery),
); err != nil {
t.Fatalf("exercise stub now-playing readback: %v", err)
}
if !strings.Contains(statusText, "nothing playing") {
t.Errorf("status = %q, want it to report that nothing is playing", statusText)
}
}
// TestPlayingSourceWithoutLocationStillConfirms guards the backstop's own
// blast radius: a physical input reports no location, and must still confirm
// as long as the speaker says it is playing.
func TestPlayingSourceWithoutLocationStillConfirms(t *testing.T) {
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) {
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",` +
`"PlayStatus":"PLAY_STATE","ContentItem":{"Source":"AUX","Type":"","Location":"",` +
`"ItemName":"AUX","IsPresetable":false}}}}}`))
})
})
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),
); err != nil {
t.Fatalf("exercise playing source with no location: %v", err)
}
}
func TestSourceSelectionStopsReadbacksOnceTheEventStreamConfirms(t *testing.T) {
var mu sync.Mutex
reads := 0
pkg/service/soundtouchweb/static/js/components/Sources.js
+38 -2
View File
@@ -44,6 +44,26 @@ function isErrorSource(source) {
return source === 'INVALID_SOURCE' || source?.endsWith('_ERROR');
}
// A speaker given a select it cannot act on parks on a stub now-playing
// rather than refusing: the source name echoed back as the item name, no
// location, and no play status, while whatever was already playing carries
// on. Confirmed on real hardware for RADIO_BROWSER and LOCAL_INTERNET_RADIO.
//
// PROVIDER_SOURCES prevents the selects we know produce this. This check is
// the backstop for the ones we do not know about, ALEXA among them: reporting
// such a readback as success is worse than reporting nothing, because the
// source list then shows a source the speaker is demonstrably not playing.
//
// All three conditions are required. A genuinely playing source reports a
// play status even when it has no location to report, a physical input for
// instance, so no single condition would be safe on its own.
function isStubNowPlaying(nowPlaying) {
const item = nowPlaying?.ContentItem;
if (!item) return false;
return !item.Location && !nowPlaying.PlayStatus && item.ItemName === nowPlaying.Source;
}
function sourceAccountIdentity(source, account) {
return account && account !== source ? account : '';
}
@@ -101,6 +121,7 @@ export function Sources({
const matches = currentSource === command.source &&
sourceAccountsMatch(command.source, currentAccount, command.account);
const stub = currentSource === command.source && isStubNowPlaying(status?.nowPlaying);
if (command.outcome === 'final-confirmed') {
if (nowPlayingRevision > command.confirmedRevision && !matches) {
setCommand(previous => previous?.generation === command.generation
@@ -108,11 +129,15 @@ export function Sources({
}
return;
}
if (isErrorSource(currentSource)) {
if (isErrorSource(currentSource) || stub) {
clearReadbacks();
commandRef.current.active = null;
setCommand(previous => previous?.generation === command.generation
? { ...previous, outcome: 'failed', error: currentSource }
? {
...previous,
outcome: 'failed',
error: stub ? 'speaker reported nothing playing' : currentSource,
}
: previous);
} else if (matches && command.outcome === 'pending') {
setCommand(previous => previous?.generation === command.generation
@@ -250,6 +275,17 @@ export function Sources({
error: nowPlaying.Source,
startNowPlayingRevision: nowPlayingRevision,
});
} else if (revisionIsNewer && nowPlaying?.Source === target.source &&
isStubNowPlaying(nowPlaying)) {
clearReadbacks();
commandRef.current.active = null;
setCommand({
...target,
generation,
outcome: 'failed',
error: 'speaker reported nothing playing',
startNowPlayingRevision: nowPlayingRevision,
});
} 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