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) {