mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-09-07 15:07:17 +00:00
fix(player): make Zone.js source zone candidates independently of Group projection
Zone.js derived its "add speaker" candidate list from the `devices`
prop, which is app.js's projected/collapsed device list. Once the
stereo-pair projection (device_projection.go) started hiding a pair's
non-master member from that list, it silently became impossible to
add that physical device to an unrelated multiroom zone, even though
the backend's HandleZoneAdd/HandleZoneRemove already operate on the
raw device registry directly and never cared about pairing at all.
Zone.js's own file wasn't touched by that change; its effective input
just changed underneath it.
Added GET /api/control/devices/{id}/zone/candidates, deliberately
bypassing deviceViewSnapshot's projection and deliberately not
excluding {id} itself -- which candidates to exclude is a caller
concern (Zone.js already does this via the existing zoneIps set,
which includes the zone master's own IP even when standalone, per
models.ZoneInfo.IsStandalone). Zone and Group are separate, unrelated
groupings and should stay that way.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
522c6b8cb6
commit
78c6c1bfca
@@ -1099,6 +1099,54 @@ func (app *WebApp) HandleZoneLeave(w http.ResponseWriter, r *http.Request) {
|
||||
"Left zone")
|
||||
}
|
||||
|
||||
// HandleGetZoneCandidates returns every registered physical device, for the
|
||||
// "add to zone" picker. Unlike HandleAPIDevices, this deliberately bypasses
|
||||
// the stereo-pair projection (deviceViewSnapshot): Zone and Group are
|
||||
// separate, unrelated groupings, and a device that's currently hidden as a
|
||||
// stereo-pair member (see device_projection.go) must still be an
|
||||
// independently addressable zone target, exactly as the backend
|
||||
// HandleZoneAdd/HandleZoneRemove already treat it (both look devices up via
|
||||
// the raw registry, unaffected by projection).
|
||||
//
|
||||
// Deliberately does not exclude the {id} device itself: which candidates to
|
||||
// exclude (the page's own device, current zone members, ...) is a caller
|
||||
// concern, not an inherent property of "what devices exist" -- excluding it
|
||||
// here would make this endpoint silently unusable for any future caller
|
||||
// that isn't Zone.js's current "add to my own zone" flow.
|
||||
func (app *WebApp) HandleGetZoneCandidates(w http.ResponseWriter, r *http.Request) {
|
||||
deviceID := chi.URLParam(r, "id")
|
||||
|
||||
if _, exists := app.GetDevice(deviceID); !exists {
|
||||
app.sendError(w, "Device not found", http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
|
||||
type zoneCandidate struct {
|
||||
Info *models.DeviceInfo `json:"info,omitempty"`
|
||||
}
|
||||
|
||||
candidates := make(map[string]zoneCandidate)
|
||||
|
||||
for _, entry := range app.DeviceSnapshot() {
|
||||
if entry.Device == nil || entry.Device.DeviceInfo == nil {
|
||||
continue
|
||||
}
|
||||
|
||||
candidates[entry.ID] = zoneCandidate{Info: entry.Device.DeviceInfo}
|
||||
}
|
||||
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
|
||||
response := webtypes.APIResponse{
|
||||
Success: true,
|
||||
Data: candidates,
|
||||
}
|
||||
|
||||
if err := json.NewEncoder(w).Encode(response); err != nil {
|
||||
http.Error(w, "Failed to encode response", http.StatusInternalServerError)
|
||||
}
|
||||
}
|
||||
|
||||
// HandleDeviceRecents returns recently played items for a device.
|
||||
func (app *WebApp) HandleDeviceRecents(w http.ResponseWriter, r *http.Request) {
|
||||
deviceID := chi.URLParam(r, "id")
|
||||
|
||||
@@ -893,3 +893,78 @@ func TestHandleZoneLeave_UsesRemoveZoneSlave(t *testing.T) {
|
||||
t.Errorf("removeZoneSlave body should name the master, got: %s", masterBody)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleGetZoneCandidates_IncludesHiddenPairMemberAndSelf guards two
|
||||
// things the stereo-pair projection (device_projection.go) must not affect:
|
||||
// Zone and Group are separate, unrelated groupings, so (a) a stereo pair's
|
||||
// hidden non-master member -- absent from the collapsed "devices" list --
|
||||
// must still be a valid zone-add candidate, matching how HandleZoneAdd
|
||||
// already treats it (raw registry lookup, unaffected by projection); and
|
||||
// (b) the endpoint itself does not exclude the requesting {id} device,
|
||||
// since deciding what to exclude (this page's own device, current zone
|
||||
// members, ...) is the caller's concern -- Zone.js already does this via
|
||||
// the existing zoneIps set, which includes the master's own IP even for a
|
||||
// standalone zone (see models.ZoneInfo.IsStandalone).
|
||||
func TestHandleGetZoneCandidates_IncludesHiddenPairMemberAndSelf(t *testing.T) {
|
||||
app := NewWebApp()
|
||||
|
||||
group := testStereoGroup()
|
||||
master := webtypes.NewDeviceConnection(nil, &models.DeviceInfo{DeviceID: "left-id", Name: "Living Room", IPAddress: "192.0.2.10"})
|
||||
master.SetStatus(&webtypes.DeviceStatus{IsConnected: true, Group: group})
|
||||
app.AddDevice("192.0.2.10", master)
|
||||
|
||||
hiddenMember := webtypes.NewDeviceConnection(nil, &models.DeviceInfo{DeviceID: "right-id", Name: "Living Room", IPAddress: "192.0.2.11"})
|
||||
hiddenMember.SetStatus(&webtypes.DeviceStatus{IsConnected: true, Group: group})
|
||||
app.AddDevice("192.0.2.11", hiddenMember)
|
||||
|
||||
standalone := webtypes.NewDeviceConnection(nil, &models.DeviceInfo{DeviceID: "kitchen-id", Name: "Kitchen", IPAddress: "192.0.2.12"})
|
||||
standalone.SetStatus(&webtypes.DeviceStatus{IsConnected: true})
|
||||
app.AddDevice("192.0.2.12", standalone)
|
||||
|
||||
// Sanity check: the hidden member really is absent from the projected
|
||||
// device list this test is guarding against leaking into.
|
||||
projected := app.deviceViewSnapshot()
|
||||
if _, visible := projected["192.0.2.11"]; visible {
|
||||
t.Fatal("test setup: expected 192.0.2.11 to be hidden by the stereo-pair projection")
|
||||
}
|
||||
|
||||
req := httptest.NewRequest("GET", "/api/control/devices/192.0.2.10/zone/candidates", nil)
|
||||
req = withChiParams(req, map[string]string{"id": "192.0.2.10"})
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
app.HandleGetZoneCandidates(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
|
||||
var response webtypes.APIResponse
|
||||
if err := json.NewDecoder(w.Body).Decode(&response); err != nil {
|
||||
t.Fatalf("failed to decode response: %v", err)
|
||||
}
|
||||
|
||||
data, ok := response.Data.(map[string]interface{})
|
||||
if !ok {
|
||||
t.Fatalf("expected data to be a map, got %T", response.Data)
|
||||
}
|
||||
|
||||
for _, ip := range []string{"192.0.2.10", "192.0.2.11", "192.0.2.12"} {
|
||||
if _, ok := data[ip]; !ok {
|
||||
t.Errorf("expected %s in zone candidates, got: %+v", ip, data)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleGetZoneCandidates_UnknownDeviceNotFound(t *testing.T) {
|
||||
app := NewWebApp()
|
||||
|
||||
req := httptest.NewRequest("GET", "/api/control/devices/192.0.2.99/zone/candidates", nil)
|
||||
req = withChiParams(req, map[string]string{"id": "192.0.2.99"})
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
app.HandleGetZoneCandidates(w, req)
|
||||
|
||||
if w.Code != http.StatusNotFound {
|
||||
t.Fatalf("expected 404 for an unknown device, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -86,6 +86,7 @@ func (app *WebApp) MountWeb(r chi.Router, discoveryService *discovery.UnifiedDis
|
||||
|
||||
r.Route("/zone", func(r chi.Router) {
|
||||
r.Get("/", app.HandleGetZone)
|
||||
r.Get("/candidates", app.HandleGetZoneCandidates)
|
||||
r.Post("/add/{slaveId}", app.HandleZoneAdd)
|
||||
r.Post("/remove/{slaveId}", app.HandleZoneRemove)
|
||||
r.Post("/dissolve", app.HandleZoneDissolve)
|
||||
|
||||
@@ -20,6 +20,7 @@ export const api = {
|
||||
power: (id) => req(`/api/control/devices/${id}/power`, { method: 'POST' }),
|
||||
recents: (id) => req(`/api/control/devices/${id}/recents`),
|
||||
zone: (id) => req(`/api/control/devices/${id}/zone`),
|
||||
zoneCandidates: (id) => req(`/api/control/devices/${id}/zone/candidates`),
|
||||
zoneAdd: (masterId, slaveId) => req(`/api/control/devices/${masterId}/zone/add/${slaveId}`, { method: 'POST' }),
|
||||
zoneRemove: (masterId, slaveId) => req(`/api/control/devices/${masterId}/zone/remove/${slaveId}`, { method: 'POST' }),
|
||||
zoneDissolve: (id) => req(`/api/control/devices/${id}/zone/dissolve`, { method: 'POST' }),
|
||||
|
||||
@@ -7,13 +7,17 @@ const html = htm.bind(h);
|
||||
|
||||
export function Zone({ deviceId, devices }) {
|
||||
const [zone, setZone] = useState(null);
|
||||
const [candidates, setCandidates] = useState({});
|
||||
const [loading, setLoading] = useState(true);
|
||||
const [showPicker, setShowPicker] = useState(false);
|
||||
|
||||
function refresh() {
|
||||
api.zone(deviceId).then(resp => {
|
||||
if (resp.success) setZone(resp.data);
|
||||
}).finally(() => setLoading(false));
|
||||
Promise.all([api.zone(deviceId), api.zoneCandidates(deviceId)])
|
||||
.then(([zoneResp, candidatesResp]) => {
|
||||
if (zoneResp.success) setZone(zoneResp.data);
|
||||
if (candidatesResp.success) setCandidates(candidatesResp.data || {});
|
||||
})
|
||||
.finally(() => setLoading(false));
|
||||
}
|
||||
|
||||
useEffect(() => { refresh(); }, [deviceId]);
|
||||
@@ -48,11 +52,15 @@ export function Zone({ deviceId, devices }) {
|
||||
|
||||
if (!zone) return null;
|
||||
|
||||
// Devices not already in the zone are available to add
|
||||
// Devices not already in the zone are available to add. Sourced from a
|
||||
// dedicated candidates endpoint rather than the `devices` prop: Zone and
|
||||
// Group are separate, unrelated groupings, so a stereo pair's hidden
|
||||
// member -- absent from the collapsed device list -- is still a valid,
|
||||
// independent zone-add target.
|
||||
const zoneIps = new Set([zone.masterIp, ...(zone.members || []).map(m => m.ip)].filter(Boolean));
|
||||
const available = Object.entries(devices || {}).filter(([ip]) => !zoneIps.has(ip));
|
||||
const available = Object.entries(candidates).filter(([ip]) => !zoneIps.has(ip));
|
||||
|
||||
const deviceName = (ip) => devices[ip]?.info?.name || ip;
|
||||
const deviceName = (ip) => devices[ip]?.info?.name || candidates[ip]?.info?.name || ip;
|
||||
|
||||
return html`
|
||||
<div class="zone-section">
|
||||
|
||||
Reference in New Issue
Block a user