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

perf(player): confirm a source with one light readback, not three heavy ones

Browse Source 
Selecting a source cost up to ~21 speaker requests. Two multipliers, both
removed here.

A match on the first readback only marked the command provisional and left
the later deadlines running, so the happy path always spent all three. The
reason to keep watching is real: /select answers 200 even for a source the
speaker rejects seconds later, surfacing as a transition to an error source.
But the event stream already reports that transition as it happens, and the
effect watching nowPlayingUpdated already turns it into a failure. Polling
on top is re-asking a question we are subscribed to the answer of. The
remaining readbacks are now kept only when the readback itself reports no
live event stream, which is the case they are actually needed for.

Each readback also fetched the whole device, and HandleAPIDevice runs a full
UpdateDeviceStatus: six sequential speaker calls plus /getGroup on a
stereo-capable model, to answer one question, against a device the readback
may be checking on precisely because it is slow. GET
/devices/{id}/now-playing refreshes only /now_playing and returns the same
shape, under FieldNowPlaying's generation so it still orders against push
events and concurrent polls, and reporting to the health tracker like any
other HTTP round.

A confirmed selection on a speaker with a live event stream now costs one
speaker request instead of about twenty-one. A speaker whose events are not
arriving keeps the full three-readback window, at one request each.

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 43d085b9e6
commit 7bd2a3d9f5
7 changed files with 244 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/soundtouch-service/testdata/router_routes.txt
Vendored
+1
View File
@@ -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
pkg/service/soundtouchweb/browser_compatibility_test.go
+94 -12
View File
@@ -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++
pkg/service/soundtouchweb/handler.go
+53
View File
@@ -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")
pkg/service/soundtouchweb/mount.go
+4
View File
@@ -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
pkg/service/soundtouchweb/source_cache_test.go
+72
View File
@@ -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(`<nowPlaying source="AUX" sourceAccount="AUX1"><playStatus>PLAY_STATE</playStatus></nowPlaying>`))
}))
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)
}
}
pkg/service/soundtouchweb/static/js/api.js
+3
View File
@@ -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' }),
pkg/service/soundtouchweb/static/js/components/Sources.js
+17 -3
View File
@@ -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;
}