From ccdc2bd6a44873ecc56c4101762674fed4f810fc Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Wed, 20 May 2026 20:36:48 +0200 Subject: [PATCH] fix(datastore): preserve speaker's isPresetable verdict in SavePresets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SavePresets hard-coded isPresetable="true" on every persisted preset, overwriting the speaker firmware's verdict. The speaker sets isPresetable="false" for content it can't independently recall later (notably Spotify Connect pushes from a phone — see GH-235); masking that flag made the on-disk XML look valid while pressing the preset on the speaker still did nothing, leaving users debugging a phantom "stored but won't play" state. Now preserve the caller's value and default to "true" only when it's empty. A non-recallable preset is logged at info level so users can tell from the service log why a stored preset isn't playing. Co-Authored-By: Claude Sonnet 4.6 --- pkg/service/datastore/datastore.go | 21 +++++- .../datastore/presets_ispresetable_test.go | 74 +++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 pkg/service/datastore/presets_ispresetable_test.go diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go index 72d1c33..0ab41e6 100644 --- a/pkg/service/datastore/datastore.go +++ b/pkg/service/datastore/datastore.go @@ -11,6 +11,7 @@ import ( "errors" "fmt" "io" + "log" "math/rand" "os" "path/filepath" @@ -948,7 +949,25 @@ func (ds *DataStore) SavePresets(account, device string, presets []models.Servic pxml.ContentItem.Type = p.Type pxml.ContentItem.Location = p.Location pxml.ContentItem.SourceAccount = p.SourceAccount - pxml.ContentItem.IsPresetable = "true" + + // Preserve the speaker's IsPresetable verdict instead of forcing + // "true". The speaker firmware sets isPresetable="false" for + // content it can't independently recall later (e.g. Spotify Connect + // pushes from a phone — see GH-235). Hard-coding "true" makes the + // on-disk XML look valid while the speaker still refuses to play + // the preset, which leaves users debugging a phantom "stored but + // won't play" state. Default to "true" only when the caller + // supplied nothing. + pxml.ContentItem.IsPresetable = p.IsPresetable + if pxml.ContentItem.IsPresetable == "" { + pxml.ContentItem.IsPresetable = "true" + } + + if pxml.ContentItem.IsPresetable == "false" { + log.Printf("[Datastore] SavePresets: storing preset %s as isPresetable=false (account=%s device=%s source=%s) — speaker firmware marked this content non-recallable; preset will appear on the speaker but pressing it will not play", + pxml.ID, account, device, p.Source) + } + pxml.ContentItem.ItemName = p.Name pxml.ContentItem.ContainerArt = p.ContainerArt pxml.SourceID = p.SourceID diff --git a/pkg/service/datastore/presets_ispresetable_test.go b/pkg/service/datastore/presets_ispresetable_test.go new file mode 100644 index 0000000..bbb9706 --- /dev/null +++ b/pkg/service/datastore/presets_ispresetable_test.go @@ -0,0 +1,74 @@ +package datastore + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/models" +) + +// TestSavePresets_PreservesIsPresetable is a regression test for GH-235: +// SavePresets used to hard-code isPresetable="true", masking the speaker's +// own verdict that Spotify-Connect content isn't recallable. Storing the +// preset looked like it succeeded but pressing it on the speaker did +// nothing. The fix preserves whatever IsPresetable the speaker provided, +// defaulting to "true" only when the caller supplied an empty string. +func TestSavePresets_PreservesIsPresetable(t *testing.T) { + tempDir, err := os.MkdirTemp("", "datastore-ispresetable-*") + if err != nil { + t.Fatalf("tempdir: %v", err) + } + defer func() { _ = os.RemoveAll(tempDir) }() + + ds := NewDataStore(tempDir) + account := "1234567" + device := "AABBCCDDEEFF" + + presets := []models.ServicePreset{ + { + ServiceContentItem: models.ServiceContentItem{ + Name: "Connect Playlist", + Source: "SPOTIFY", + IsPresetable: "false", + }, + ButtonNumber: "1", + }, + { + ServiceContentItem: models.ServiceContentItem{ + Name: "Default Truth", + Source: "TUNEIN", + IsPresetable: "true", + }, + ButtonNumber: "2", + }, + { + ServiceContentItem: models.ServiceContentItem{ + Name: "Caller Left Empty", + Source: "INTERNET_RADIO", + // IsPresetable intentionally unset + }, + ButtonNumber: "3", + }, + } + + if err := ds.SavePresets(account, device, presets); err != nil { + t.Fatalf("SavePresets: %v", err) + } + + body, err := os.ReadFile(filepath.Join(ds.AccountDeviceDir(account, device), "Presets.xml")) + if err != nil { + t.Fatalf("read Presets.xml: %v", err) + } + + got := string(body) + + if !strings.Contains(got, `id="1"`) || !strings.Contains(got, `isPresetable="false"`) { + t.Errorf("preset 1: expected isPresetable=\"false\" preserved; Presets.xml:\n%s", got) + } + + if !strings.Contains(got, `id="2"`) || strings.Count(got, `isPresetable="true"`) < 2 { + t.Errorf("preset 2/3: expected isPresetable=\"true\" (preset 2 from caller, preset 3 from empty-string default); Presets.xml:\n%s", got) + } +}