From a5f5bdb9165c492163e569cfad2ccdd79509e268 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Wed, 27 May 2026 20:22:24 +0200 Subject: [PATCH] fix(group): propagate removeGroup to all members; handle DELETE /group/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs prevented clean stereo-pair teardown: 1. removeGroup (CLI) only contacted the --host speaker (master). The slave never received /removeGroup and stayed stuck in GroupSlave state indefinitely, blocking direct playback. Fix: fetch the current group first, then send /removeGroup to every member in parallel — mirrors the same symmetry as createGroup (issue #252). 2. Speakers send DELETE /streaming/account/{id}/group/ (trailing slash, no group ID) during teardown. Master and slave live in different accounts, so each deletes its own copy independently. AfterTouch had no route for this form → 405. Fix: add DeleteAllGroupsForAccount to the datastore (scans Group_*.xml, idempotent if none found) and wire DELETE /group and DELETE /group/ to a new HandleMargeDeleteAccountGroups handler in both routing blocks. Confirmed: after the fix both DELETE calls return 200 and the slave exits GroupSlave state cleanly. Co-Authored-By: Claude Sonnet 4.6 --- cmd/soundtouch-cli/cmd_group.go | 74 ++++++++++++++++++- cmd/soundtouch-service/main.go | 7 ++ .../testdata/router_routes.txt | 4 + pkg/service/datastore/datastore.go | 31 ++++++++ pkg/service/handlers/handlers_marge.go | 24 +++++- 5 files changed, 136 insertions(+), 4 deletions(-) diff --git a/cmd/soundtouch-cli/cmd_group.go b/cmd/soundtouch-cli/cmd_group.go index ec971ff..507ef1e 100644 --- a/cmd/soundtouch-cli/cmd_group.go +++ b/cmd/soundtouch-cli/cmd_group.go @@ -244,7 +244,10 @@ func renameGroup(c *cli.Context) error { return nil } -// removeGroup tears down the device's stereo pair. +// removeGroup tears down the device's stereo pair by sending /removeGroup to +// every member in parallel. Sending it only to the master (as the old code +// did) leaves the slave stuck in GroupSlave state indefinitely — mirrors the +// same symmetry as createGroup (see issue #252 comment there). func removeGroup(c *cli.Context) error { clientConfig := GetClientConfig(c) PrintDeviceHeader("Removing stereo pair", clientConfig.Host, clientConfig.Port) @@ -255,11 +258,76 @@ func removeGroup(c *cli.Context) error { return err } - if err := stClient.RemoveGroup(); err != nil { - PrintError(fmt.Sprintf("Failed to remove group: %v", err)) + // Fetch current group to learn every member's IP before tearing down. + group, err := stClient.GetGroup() + if err != nil { + PrintError(fmt.Sprintf("Failed to read current group: %v", err)) return err } + if group.IsEmpty() { + fmt.Println("Device is not in a stereo pair — nothing to remove") + return nil + } + + // Collect the unique set of member IPs. The master is always reachable + // via clientConfig.Host; the roles carry all members including slaves. + type memberResult struct { + ip string + err error + } + + members := make([]string, 0, len(group.Roles.Roles)) + seen := map[string]bool{} + + for _, role := range group.Roles.Roles { + if role.IPAddress != "" && !seen[role.IPAddress] { + seen[role.IPAddress] = true + members = append(members, role.IPAddress) + } + } + + // Always include the addressed host even if the group response omitted IPs. + if !seen[clientConfig.Host] { + members = append(members, clientConfig.Host) + } + + results := make([]memberResult, len(members)) + + var wg sync.WaitGroup + + for i, ip := range members { + wg.Add(1) + + go func(idx int, host string) { + defer wg.Done() + + mc, mcErr := clientForHost(c, host) + if mcErr != nil { + results[idx] = memberResult{ip: host, err: mcErr} + return + } + + results[idx] = memberResult{ip: host, err: mc.RemoveGroup()} + }(i, ip) + } + + wg.Wait() + + anyErr := false + + for _, r := range results { + if r.err != nil { + PrintError(fmt.Sprintf("%s /removeGroup failed: %v", r.ip, r.err)) + + anyErr = true + } + } + + if anyErr { + return fmt.Errorf("/removeGroup propagation failed") + } + PrintSuccess("Stereo pair removed") return nil diff --git a/cmd/soundtouch-service/main.go b/cmd/soundtouch-service/main.go index 83b7ac8..3415747 100644 --- a/cmd/soundtouch-service/main.go +++ b/cmd/soundtouch-service/main.go @@ -1097,6 +1097,11 @@ func setupRouter(server *handlers.Server, stockholmHandler *stockholm.Handler) * r.Post("/group/", server.HandleMargeAddGroup) r.Post("/group/{groupId}", server.HandleMargeModifyGroup) r.Delete("/group/{groupId}", server.HandleMargeDeleteGroup) + // Speakers send DELETE /group/ (no group ID, trailing slash) during + // stereo-pair teardown; master and slave use their own account IDs + // so each deletes its own copy. + r.Delete("/group", server.HandleMargeDeleteAccountGroups) + r.Delete("/group/", server.HandleMargeDeleteAccountGroups) }) r.Get("/device/{device}/streaming_token", server.HandleMargeStreamingToken) @@ -1144,6 +1149,8 @@ func setupRouter(server *handlers.Server, stockholmHandler *stockholm.Handler) * r.Post("/group/", server.HandleMargeAddGroup) r.Post("/group/{groupId}", server.HandleMargeModifyGroup) r.Delete("/group/{groupId}", server.HandleMargeDeleteGroup) + r.Delete("/group", server.HandleMargeDeleteAccountGroups) + r.Delete("/group/", server.HandleMargeDeleteAccountGroups) r.Get("/devices/{device}/presets", server.HandleMargePresets) r.Get("/devices/{device}/recents", server.HandleMargeRecents) diff --git a/cmd/soundtouch-service/testdata/router_routes.txt b/cmd/soundtouch-service/testdata/router_routes.txt index ed33ac3..adb9082 100644 --- a/cmd/soundtouch-service/testdata/router_routes.txt +++ b/cmd/soundtouch-service/testdata/router_routes.txt @@ -1,6 +1,8 @@ CONNECT /core02/svc-bmx-adapter-siriusxm-everest-eco1/prod/live-adapter handlers.(*Server).HandleSiriusXMLiveAdapter-fm CONNECT /core02/svc-bmx-adapter-siriusxm-everest-eco1/prod/live-adapter/* handlers.(*Server).HandleSiriusXMLiveAdapterSubpath-fm DELETE /accounts/{account}/devices/{device} handlers.(*Server).HandleMargeRemoveDevice-fm +DELETE /accounts/{account}/group handlers.(*Server).HandleMargeDeleteAccountGroups-fm +DELETE /accounts/{account}/group/ handlers.(*Server).HandleMargeDeleteAccountGroups-fm DELETE /accounts/{account}/group/{groupId} handlers.(*Server).HandleMargeDeleteGroup-fm DELETE /bmx/tunein/v1/favorite/{stationID} handlers.(*Server).HandleTuneInDeleteFavorite-fm DELETE /core02/svc-bmx-adapter-siriusxm-everest-eco1/prod/live-adapter handlers.(*Server).HandleSiriusXMLiveAdapter-fm @@ -12,6 +14,8 @@ DELETE /setup/interactions/sessions/{session} handlers.( DELETE /setup/sources/{account}/{device}/{sourceID} handlers.(*Server).HandleDeleteSource-fm DELETE /streaming/account/{account}/device/{device} handlers.(*Server).HandleMargeRemoveDevice-fm DELETE /streaming/account/{account}/device/{device}/preset/{presetNumber} handlers.(*Server).HandleMargeRemovePreset-fm +DELETE /streaming/account/{account}/group handlers.(*Server).HandleMargeDeleteAccountGroups-fm +DELETE /streaming/account/{account}/group/ handlers.(*Server).HandleMargeDeleteAccountGroups-fm DELETE /streaming/account/{account}/group/{groupId} handlers.(*Server).HandleMargeDeleteGroup-fm GET / handlers.(*Server).HandleRoot-fm GET /accounts/{account}/devices handlers.(*Server).HandleMargeAccountDevices-fm diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go index 2769728..b1cf4e8 100644 --- a/pkg/service/datastore/datastore.go +++ b/pkg/service/datastore/datastore.go @@ -2816,6 +2816,37 @@ func (ds *DataStore) DeleteGroup(account, groupID string) error { return err } +// DeleteAllGroupsForAccount removes every Group_*.xml file stored under +// account. Speakers send DELETE /streaming/account/{id}/group/ (no group +// ID) during stereo-pair teardown; since master and slave may live in +// different accounts each speaker deletes its own copy. Returns nil if no +// group files are found — idempotent by design. +func (ds *DataStore) DeleteAllGroupsForAccount(account string) error { + ds.fileMutex.Lock() + defer ds.fileMutex.Unlock() + + dir := ds.AccountDevicesDir(account) + + entries, err := ds.rootReadDir(dir) + if err != nil { + if os.IsNotExist(err) { + return nil // nothing to delete + } + + return err + } + + for _, e := range entries { + if e.IsDir() || !strings.HasPrefix(e.Name(), "Group_") || !strings.HasSuffix(e.Name(), ".xml") { + continue + } + + _ = ds.rootRemove(filepath.Join(dir, e.Name())) + } + + return nil +} + // SaveTuneInFavorite records a TuneIn station as favorited by creating a marker file. // File presence indicates the station is a favorite; no content is stored. func (ds *DataStore) SaveTuneInFavorite(stationID string) error { diff --git a/pkg/service/handlers/handlers_marge.go b/pkg/service/handlers/handlers_marge.go index 015b3a1..fa42240 100644 --- a/pkg/service/handlers/handlers_marge.go +++ b/pkg/service/handlers/handlers_marge.go @@ -901,7 +901,7 @@ func (s *Server) HandleMargeModifyGroup(w http.ResponseWriter, r *http.Request) _, _ = w.Write(data) } -// HandleMargeDeleteGroup removes a stereo group. +// HandleMargeDeleteGroup removes a stereo group identified by {groupId}. func (s *Server) HandleMargeDeleteGroup(w http.ResponseWriter, r *http.Request) { account := chi.URLParam(r, "account") groupID := chi.URLParam(r, "groupId") @@ -921,6 +921,28 @@ func (s *Server) HandleMargeDeleteGroup(w http.ResponseWriter, r *http.Request) _, _ = w.Write([]byte(constants.XMLHeader + `Group deleted successfully`)) } +// HandleMargeDeleteAccountGroups removes all stereo groups stored for an +// account. Speakers send DELETE /streaming/account/{id}/group/ (trailing +// slash, no group ID) during stereo-pair teardown. Master and slave often +// live in different accounts, so each speaker deletes its own copy here. +func (s *Server) HandleMargeDeleteAccountGroups(w http.ResponseWriter, r *http.Request) { + account := chi.URLParam(r, "account") + + if !validatePathID(account) { + http.Error(w, "Invalid account ID", http.StatusBadRequest) + return + } + + if err := s.ds.DeleteAllGroupsForAccount(account); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", "application/vnd.bose.streaming-v1.2+xml") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(constants.XMLHeader + `Group deleted successfully`)) +} + // HandleMusicProviderIsEligible returns the music provider eligibility. func (s *Server) HandleMusicProviderIsEligible(w http.ResponseWriter, _ *http.Request) { // For now, we return false as seen in the interaction sample.