mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-08-18 08:36:13 +00:00
fix(zone): remove a member via /removeZoneSlave instead of a /setZone rebuild (refs #511)
Removing one member from a multi-member zone did nothing. The remove paths rebuilt the zone with /setZone and the remaining members, but /setZone is additive: it never drops a member that is simply absent from the list. It only "removed" when the resulting set was empty (equivalent to dissolve), which is why removing the last member worked but removing one of several did not. Switch all three remove paths to the dedicated /removeZoneSlave endpoint (already implemented as client.RemoveZoneSlave): - HandleZoneRemove (web UI "remove member") - HandleZoneLeave (web UI slave "leave zone") - RemoveFromZone (client lib, used by CLI `zone remove`) DissolveZone (setZone master-only) and HandleZoneAdd (additive setZone) are correct and unchanged. Adds handler regression tests for remove/leave and rewrites TestClient_RemoveFromZone to assert /removeZoneSlave (the old test removed one of two members but only checked that setZone was called, never that the member was dropped). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
462b4179f1
commit
720d2abc5c
+21
-5
@@ -1338,7 +1338,13 @@ func (c *Client) AddToZone(deviceID, ipAddress string) error {
|
||||
return c.SetZone(zoneRequest)
|
||||
}
|
||||
|
||||
// RemoveFromZone removes a device from the current zone
|
||||
// RemoveFromZone removes a device from the current zone.
|
||||
//
|
||||
// It uses the dedicated /removeZoneSlave endpoint rather than rebuilding the
|
||||
// zone with /setZone and the remaining members: /setZone does not drop a member
|
||||
// from a multi-member zone (the speaker only goes standalone when the resulting
|
||||
// member set is empty), so a setZone rebuild silently fails to remove one of
|
||||
// several members. See #511.
|
||||
func (c *Client) RemoveFromZone(deviceID string) error {
|
||||
// Get current zone configuration
|
||||
currentZone, err := c.GetZone()
|
||||
@@ -1346,11 +1352,21 @@ func (c *Client) RemoveFromZone(deviceID string) error {
|
||||
return fmt.Errorf("failed to get current zone: %w", err)
|
||||
}
|
||||
|
||||
// Convert to zone request and remove member
|
||||
zoneRequest := currentZone.ToZoneRequest()
|
||||
zoneRequest.RemoveMember(deviceID)
|
||||
if currentZone.IsStandalone() {
|
||||
return nil // nothing to remove
|
||||
}
|
||||
|
||||
return c.SetZone(zoneRequest)
|
||||
// Carry the member's IP (as the speaker expects) when we know it.
|
||||
slaveIP := ""
|
||||
|
||||
for i := range currentZone.Members {
|
||||
if currentZone.Members[i].DeviceID == deviceID {
|
||||
slaveIP = currentZone.Members[i].IP
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
return c.RemoveZoneSlave(currentZone.Master, deviceID, slaveIP)
|
||||
}
|
||||
|
||||
// DissolveZone dissolves the current zone, making all devices standalone
|
||||
|
||||
+30
-6
@@ -1,6 +1,7 @@
|
||||
package client
|
||||
|
||||
import (
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
@@ -295,12 +296,15 @@ func TestClient_RemoveFromZone(t *testing.T) {
|
||||
getZoneCalled := false
|
||||
setZoneCalled := false
|
||||
|
||||
var removeBody string
|
||||
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/xml")
|
||||
|
||||
if r.URL.Path == "/getZone" && r.Method == http.MethodGet {
|
||||
switch {
|
||||
case r.URL.Path == "/getZone" && r.Method == http.MethodGet:
|
||||
getZoneCalled = true
|
||||
// Return existing zone with members
|
||||
// Return existing zone with two members.
|
||||
response := `<?xml version="1.0" encoding="UTF-8" ?>
|
||||
<zone master="ABCD1234EFGH">
|
||||
<member ipaddress="192.0.2.11">EFGH5678IJKL</member>
|
||||
@@ -309,11 +313,16 @@ func TestClient_RemoveFromZone(t *testing.T) {
|
||||
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = w.Write([]byte(response))
|
||||
} else if r.URL.Path == "/setZone" && r.Method == http.MethodPost {
|
||||
case r.URL.Path == "/removeZoneSlave" && r.Method == http.MethodPost:
|
||||
b, _ := io.ReadAll(r.Body)
|
||||
removeBody = string(b)
|
||||
|
||||
w.WriteHeader(http.StatusOK)
|
||||
case r.URL.Path == "/setZone" && r.Method == http.MethodPost:
|
||||
setZoneCalled = true
|
||||
|
||||
w.WriteHeader(http.StatusOK)
|
||||
} else {
|
||||
default:
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
}))
|
||||
@@ -321,6 +330,9 @@ func TestClient_RemoveFromZone(t *testing.T) {
|
||||
|
||||
client := createTestClient(server.URL)
|
||||
|
||||
// Removing one of two members must target that member via /removeZoneSlave,
|
||||
// not rebuild the zone via /setZone (which does not drop a member from a
|
||||
// multi-member zone). Regression for #511.
|
||||
err := client.RemoveFromZone("EFGH5678IJKL")
|
||||
if err != nil {
|
||||
t.Errorf("Expected no error, but got: %v", err)
|
||||
@@ -330,8 +342,20 @@ func TestClient_RemoveFromZone(t *testing.T) {
|
||||
t.Error("Expected GetZone to be called")
|
||||
}
|
||||
|
||||
if !setZoneCalled {
|
||||
t.Error("Expected SetZone to be called")
|
||||
if setZoneCalled {
|
||||
t.Error("RemoveFromZone must not use /setZone to drop a member from a multi-member zone")
|
||||
}
|
||||
|
||||
if !strings.Contains(removeBody, "EFGH5678IJKL") {
|
||||
t.Errorf("removeZoneSlave body should target the member, got: %s", removeBody)
|
||||
}
|
||||
|
||||
if !strings.Contains(removeBody, `master="ABCD1234EFGH"`) {
|
||||
t.Errorf("removeZoneSlave body should name the master, got: %s", removeBody)
|
||||
}
|
||||
|
||||
if !strings.Contains(removeBody, `ipaddress="192.0.2.11"`) {
|
||||
t.Errorf("removeZoneSlave body should carry the member IP from the zone, got: %s", removeBody)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user