From cf62057a26abbba9440bcf37079451b0202f96af Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Thu, 14 May 2026 16:16:05 +0200 Subject: [PATCH] fix(cli): omit senderIPAddress on master's /addGroup payload The speaker's GroupService state machine uses the presence of in the addGroup payload to decide whether it should form the group as master or join as slave: "SenderIp is provided, I am the slave". Sending the same XML to both speakers (with senderIP set to the master's IP) made the master also conclude it was the slave, enter AddingSlave, time out after 5 s waiting for a master that never confirmed, and revert. The slave briefly showed GROUP_OK before following the master back to NoGroup -- the "stereo pair appears for a few seconds, then disappears" symptom reported in #252. Send two distinct payloads from propagateAddGroup: the master receives the base request with no senderIPAddress, the slave receives a copy with senderIPAddress set to the master's IP. The base request built by createGroup no longer carries senderIPAddress; the per-role injection is contained inside propagateAddGroup where the master/slave roles are unambiguous. Update TestPropagateAddGroup_BothSucceed to assert the master's body has no while the slave's body does, so any future regression on either side fails the test. Refs #252 Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/soundtouch-cli/cmd_group.go | 21 ++++++++++++++++++--- cmd/soundtouch-cli/cmd_group_test.go | 24 ++++++++++++++++++------ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/cmd/soundtouch-cli/cmd_group.go b/cmd/soundtouch-cli/cmd_group.go index 3649819..ec971ff 100644 --- a/cmd/soundtouch-cli/cmd_group.go +++ b/cmd/soundtouch-cli/cmd_group.go @@ -84,7 +84,8 @@ func createGroup(c *cli.Context) error { {DeviceID: rightInfo.DeviceID, Role: "RIGHT", IPAddress: rightIP}, }, }, - SenderIPAddress: leftIP, + // SenderIPAddress is intentionally omitted on the base request. + // propagateAddGroup adds it to the slave's copy only — see comment there. } leftClient, err := clientForHost(c, leftIP) @@ -139,7 +140,21 @@ type addGroupOutcome struct { // propagateAddGroup POSTs /addGroup to both speakers concurrently and returns // the (LEFT, RIGHT) outcomes. A non-GROUP_OK Status in the response is // reported as an error so callers don't have to re-inspect the body. +// +// The two POSTs carry different payloads: the master (LEFT) receives the base +// request with no senderIPAddress so its state machine forms the group as the +// master, while the slave (RIGHT) receives a copy with senderIPAddress set to +// the master's IP so its state machine joins as the slave. Sending the same +// payload to both makes both speakers think they're the slave — they enter +// AddingSlave, wait for a master that never confirms, time out after 5 s, and +// revert (issue #252). func propagateAddGroup(left, right *client.Client, leftIP, rightIP string, req *models.Group) (addGroupOutcome, addGroupOutcome) { + masterReq := *req + masterReq.SenderIPAddress = "" + + slaveReq := *req + slaveReq.SenderIPAddress = leftIP + var ( wg sync.WaitGroup leftOut, rightOut addGroupOutcome @@ -150,13 +165,13 @@ func propagateAddGroup(left, right *client.Client, leftIP, rightIP string, req * go func() { defer wg.Done() - leftOut = postAddGroup(left, leftIP, req) + leftOut = postAddGroup(left, leftIP, &masterReq) }() go func() { defer wg.Done() - rightOut = postAddGroup(right, rightIP, req) + rightOut = postAddGroup(right, rightIP, &slaveReq) }() wg.Wait() diff --git a/cmd/soundtouch-cli/cmd_group_test.go b/cmd/soundtouch-cli/cmd_group_test.go index 9e13245..5477e39 100644 --- a/cmd/soundtouch-cli/cmd_group_test.go +++ b/cmd/soundtouch-cli/cmd_group_test.go @@ -61,7 +61,8 @@ func sampleGroupRequest(leftIP, rightIP string) *models.Group { {DeviceID: "F45EAB3115DA", Role: "RIGHT", IPAddress: rightIP}, }, }, - SenderIPAddress: leftIP, + // senderIPAddress is intentionally not set here; propagateAddGroup + // adds it to the slave's copy only. } } @@ -95,19 +96,30 @@ func TestPropagateAddGroup_BothSucceed(t *testing.T) { t.Errorf("RIGHT group = %+v, want status=GROUP_OK", rightOut.group) } - // Both speakers must have received the same payload, including senderIPAddress. - for _, bodies := range []*[]string{leftBodies, rightBodies} { + // Both speakers must have received the roles, but only the slave's payload + // carries senderIPAddress — see propagateAddGroup for the why. + for label, bodies := range map[string]*[]string{"LEFT": leftBodies, "RIGHT": rightBodies} { if len(*bodies) != 1 { - t.Fatalf("expected exactly one POST, got %d", len(*bodies)) + t.Fatalf("%s: expected exactly one POST, got %d", label, len(*bodies)) } body := (*bodies)[0] - for _, want := range []string{"LEFT", "RIGHT", "192.168.1.131"} { + for _, want := range []string{"LEFT", "RIGHT"} { if !strings.Contains(body, want) { - t.Errorf("body missing %q\nbody:\n%s", want, body) + t.Errorf("%s body missing %q\nbody:\n%s", label, want, body) } } } + + leftBody := (*leftBodies)[0] + if strings.Contains(leftBody, "") { + t.Errorf("LEFT (master) body must NOT carry , otherwise the master flips into slave mode (issue #252)\nbody:\n%s", leftBody) + } + + rightBody := (*rightBodies)[0] + if !strings.Contains(rightBody, "192.168.1.131") { + t.Errorf("RIGHT (slave) body must carry 192.168.1.131\nbody:\n%s", rightBody) + } } func TestPropagateAddGroup_RightFails(t *testing.T) {