diff --git a/cmd/soundtouch-cli/cmd_setup.go b/cmd/soundtouch-cli/cmd_setup.go index 9d38e00..c4b8f9a 100644 --- a/cmd/soundtouch-cli/cmd_setup.go +++ b/cmd/soundtouch-cli/cmd_setup.go @@ -2081,6 +2081,17 @@ func runPairBare(c *cli.Context, deviceIP, accountID string) error { func runPairFull(c *cli.Context, deviceIP, accountID string) error { m := setup.NewManager(c.String("service-url"), nil, nil) + needed, status, err := m.PreflightInitPlan(deviceIP) + if err != nil { + PrintError(fmt.Sprintf("preflight: %v", err)) + return err + } + + if !needed { + PrintSuccess(fmt.Sprintf("Device already configured (status=%s) — nothing to do.", status)) + return nil + } + plan := setup.InitPlan{ DeviceIP: deviceIP, ServiceURL: c.String("service-url"), @@ -2094,7 +2105,7 @@ func runPairFull(c *cli.Context, deviceIP, accountID string) error { ctx, cancel := context.WithTimeout(c.Context, 60*time.Second) defer cancel() - _, err := m.ExecuteInitPlan(ctx, plan, func(e setup.StepEvent) { + _, err = m.ExecuteInitPlan(ctx, plan, func(e setup.StepEvent) { switch e.Status { case setup.StatusOK: fmt.Printf("[%d] %s — ok\n", e.Kind, e.Name) diff --git a/docs/content/docs/analysis/SETUP-WEBSOCKET-EXPERIMENT.md b/docs/content/docs/analysis/SETUP-WEBSOCKET-EXPERIMENT.md index 9c51517..2bb3fb1 100644 --- a/docs/content/docs/analysis/SETUP-WEBSOCKET-EXPERIMENT.md +++ b/docs/content/docs/analysis/SETUP-WEBSOCKET-EXPERIMENT.md @@ -112,6 +112,14 @@ Factory-reset the same speaker again and run the full state machine — the same This drives `setup.Manager.ExecuteInitPlan` with `SkipURLRewrite=true`, which runs: +> **Update (#615):** `--mode=full` now preflights via `Manager.PreflightInitPlan` +> before opening the WebSocket — it checks `/supportedURLs` for +> `/setMargeAccount` and requires `/soundTouchConfigurationStatus` to read +> `SOUNDTOUCH_NOT_CONFIGURED`, and no-ops on an already-configured device. +> A freshly factory-reset speaker (as in this experiment) reports +> `SOUNDTOUCH_NOT_CONFIGURED`, so the preflight passes through unchanged; +> see `docs/content/docs/reference/DEVICE-PAIRING-FLOW.md`. + ``` SETUP_START SETUP_IDENTIFY_DEVICE_ENTER diff --git a/docs/content/docs/guides/CLI-REFERENCE.md b/docs/content/docs/guides/CLI-REFERENCE.md index 980b7f9..6d1ea63 100644 --- a/docs/content/docs/guides/CLI-REFERENCE.md +++ b/docs/content/docs/guides/CLI-REFERENCE.md @@ -1427,6 +1427,15 @@ name during pairing (empty keeps current). `--language` defaults to `2` (English). `--token` defaults to a built-in placeholder matching the Bose app's token shape. +`--mode=full` first reads `/supportedURLs` and `/soundTouchConfigurationStatus` +and only runs the state machine when the device reports +`SOUNDTOUCH_NOT_CONFIGURED` (see [#615](https://github.com/gesellix/Bose-SoundTouch/issues/615): +a speaker can be reachable, named, and already account-paired yet still +report `SOUNDTOUCH_NOT_CONFIGURED`, leaving the "install the Bose app" +prompt on screen — only a full pass through the state machine clears it). +An already-configured device is a no-op; an unsupported route or an +unrecognised status value fails the command instead of guessing. + #### `setup sync` Pulls presets, recents, and sources from the speaker into AfterTouch's diff --git a/docs/content/docs/reference/DEVICE-PAIRING-FLOW.md b/docs/content/docs/reference/DEVICE-PAIRING-FLOW.md index 46a080a..7bb7c1e 100644 --- a/docs/content/docs/reference/DEVICE-PAIRING-FLOW.md +++ b/docs/content/docs/reference/DEVICE-PAIRING-FLOW.md @@ -100,6 +100,20 @@ All subsequent messages (except `selectLastWiFiSource`, see below) use this enve ## Phase 2 — Pairing a New Speaker +> **Preflight (AfterTouch's `setup pair --mode=full`).** Before opening the +> WebSocket, AfterTouch reads `GET /supportedURLs` (must list +> `/setMargeAccount`) and `GET /soundTouchConfigurationStatus`, and only +> runs the state machine below when the status is exactly +> `SOUNDTOUCH_NOT_CONFIGURED`. This matters because a speaker can be +> reachable, named, and already have a `margeAccountUUID` set, yet still +> report `SOUNDTOUCH_NOT_CONFIGURED` — the firmware keeps prompting to +> install the Bose app until a full acknowledged pass through this state +> machine runs, not just `setMargeAccount` on its own. Already-configured +> devices are a no-op; an unsupported route or an unrecognised status value +> aborts without writing anything. See +> [#615](https://github.com/gesellix/Bose-SoundTouch/issues/615) and +> `Manager.PreflightInitPlan` (`pkg/service/setup/marge_pairing.go`). + ### 2.1 Setup State Machine The pairing flow uses a setup state machine on the device. States must be sent in order. diff --git a/pkg/service/setup/marge_pairing.go b/pkg/service/setup/marge_pairing.go index 1ff33fd..e635360 100644 --- a/pkg/service/setup/marge_pairing.go +++ b/pkg/service/setup/marge_pairing.go @@ -225,6 +225,82 @@ func (m *Manager) postSetMargeAccount(deviceIP, accountID string) error { return nil } +// ConfigurationStatus values reported by GET /soundTouchConfigurationStatus. +// See issue #615: a speaker can be reachable, named, and already +// account-paired yet still report SOUNDTOUCH_NOT_CONFIGURED, which leaves +// the firmware nagging the owner to install the Bose app. Only a full pass +// through the WebSocket setup state machine (ExecuteInitPlan) clears it. +const ( + ConfigurationStatusConfigured = "SOUNDTOUCH_CONFIGURED" + ConfigurationStatusNotConfigured = "SOUNDTOUCH_NOT_CONFIGURED" +) + +// ReadConfigurationStatus fetches /soundTouchConfigurationStatus and returns +// its raw status attribute (e.g. "SOUNDTOUCH_CONFIGURED"). +func (m *Manager) ReadConfigurationStatus(deviceIP string) (string, error) { + url := buildDeviceURL(deviceIP, "/soundTouchConfigurationStatus") + + client := &http.Client{Timeout: supportedURLsTimeout} + + resp, err := client.Get(url) + if err != nil { + return "", fmt.Errorf("GET %s: %w", url, err) + } + + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("GET %s returned %d", url, resp.StatusCode) + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + return "", fmt.Errorf("read %s: %w", url, err) + } + + var doc struct { + Status string `xml:"status,attr"` + } + + if err := xml.Unmarshal(body, &doc); err != nil { + return "", fmt.Errorf("parse %s: %w", url, err) + } + + return doc.Status, nil +} + +// PreflightInitPlan reports whether ExecuteInitPlan should be run against +// deviceIP, gated on the two conditions from issue #615: /setMargeAccount +// must be listed in /supportedURLs, and the device's current +// /soundTouchConfigurationStatus must be exactly SOUNDTOUCH_NOT_CONFIGURED. +// needed=false with a nil error means "already configured, nothing to do." +// Any other outcome (unsupported route, unrecognised status value) is +// treated as unknown and returned as an error rather than guessed at. +func (m *Manager) PreflightInitPlan(deviceIP string) (needed bool, status string, err error) { + supported, probeErr := m.probeSetMargeAccount(deviceIP) + if probeErr != nil { + return false, "", fmt.Errorf("supportedURLs probe: %w", probeErr) + } + + if !supported { + return false, "", errors.New("/setMargeAccount is not listed in /supportedURLs — device does not support this pairing path") + } + + status, err = m.ReadConfigurationStatus(deviceIP) + if err != nil { + return false, "", fmt.Errorf("read /soundTouchConfigurationStatus: %w", err) + } + + switch status { + case ConfigurationStatusConfigured: + return false, status, nil + case ConfigurationStatusNotConfigured: + return true, status, nil + default: + return false, status, fmt.Errorf("unexpected /soundTouchConfigurationStatus value %q", status) + } +} + // buildDeviceURL builds a URL for a SoundTouch device's HTTP API. If // deviceIP already includes a port (test scenarios using httptest) it is // reused as-is; otherwise the canonical port 8090 is appended. diff --git a/pkg/service/setup/marge_pairing_test.go b/pkg/service/setup/marge_pairing_test.go index 1d6c05a..1f737e8 100644 --- a/pkg/service/setup/marge_pairing_test.go +++ b/pkg/service/setup/marge_pairing_test.go @@ -16,13 +16,14 @@ import ( // device's :8090 HTTP API. It records POSTs to /setMargeAccount so tests // can assert on the body. type fakeDevice struct { - srv *httptest.Server - addr string // "host:port" usable as deviceIP - supportsSetMarge bool - postStatus int // status code returned for POST /setMargeAccount - postDelay time.Duration - gotPostBody string - margeAccountUUID string // served by /info; empty means "unpaired" + srv *httptest.Server + addr string // "host:port" usable as deviceIP + supportsSetMarge bool + postStatus int // status code returned for POST /setMargeAccount + postDelay time.Duration + gotPostBody string + margeAccountUUID string // served by /info; empty means "unpaired" + configurationStatus string // served by /soundTouchConfigurationStatus; empty = route not served (404) } func newFakeDevice(t *testing.T) *fakeDevice { @@ -62,6 +63,16 @@ func newFakeDevice(t *testing.T) *fakeDevice { fmt.Fprintf(w, `%s`, d.margeAccountUUID) }) + mux.HandleFunc("/soundTouchConfigurationStatus", func(w http.ResponseWriter, _ *http.Request) { + if d.configurationStatus == "" { + w.WriteHeader(http.StatusNotFound) + return + } + + w.Header().Set("Content-Type", "application/xml") + fmt.Fprintf(w, ``, d.configurationStatus) + }) + d.srv = httptest.NewServer(mux) u := d.srv.URL[len("http://"):] @@ -360,6 +371,110 @@ func TestEnsureMargeAccountPaired_PropagatesPairingFailure(t *testing.T) { } } +func TestReadConfigurationStatus_ReturnsRawStatus(t *testing.T) { + d := newFakeDevice(t) + d.configurationStatus = ConfigurationStatusConfigured + + m := &Manager{} + + status, err := m.ReadConfigurationStatus(d.addr) + if err != nil { + t.Fatalf("ReadConfigurationStatus: %v", err) + } + + if status != ConfigurationStatusConfigured { + t.Errorf("status = %q, want %q", status, ConfigurationStatusConfigured) + } +} + +func TestReadConfigurationStatus_ErrorsWhenRouteUnsupported(t *testing.T) { + d := newFakeDevice(t) + d.configurationStatus = "" + + m := &Manager{} + + if _, err := m.ReadConfigurationStatus(d.addr); err == nil { + t.Fatal("expected an error when the route is unsupported (404)") + } +} + +func TestPreflightInitPlan_NotConfiguredNeedsRepair(t *testing.T) { + d := newFakeDevice(t) + d.configurationStatus = ConfigurationStatusNotConfigured + + m := &Manager{} + + needed, status, err := m.PreflightInitPlan(d.addr) + if err != nil { + t.Fatalf("PreflightInitPlan: %v", err) + } + + if !needed { + t.Error("needed should be true for SOUNDTOUCH_NOT_CONFIGURED") + } + + if status != ConfigurationStatusNotConfigured { + t.Errorf("status = %q, want %q", status, ConfigurationStatusNotConfigured) + } +} + +func TestPreflightInitPlan_AlreadyConfiguredIsNoOp(t *testing.T) { + d := newFakeDevice(t) + d.configurationStatus = ConfigurationStatusConfigured + + m := &Manager{} + + needed, status, err := m.PreflightInitPlan(d.addr) + if err != nil { + t.Fatalf("PreflightInitPlan: %v", err) + } + + if needed { + t.Error("needed should be false for SOUNDTOUCH_CONFIGURED") + } + + if status != ConfigurationStatusConfigured { + t.Errorf("status = %q, want %q", status, ConfigurationStatusConfigured) + } +} + +func TestPreflightInitPlan_UnsupportedSetMargeAccountFailsClosed(t *testing.T) { + d := newFakeDevice(t) + d.supportsSetMarge = false + d.configurationStatus = ConfigurationStatusNotConfigured + + m := &Manager{} + + needed, _, err := m.PreflightInitPlan(d.addr) + if err == nil { + t.Fatal("expected an error when /setMargeAccount is not listed in /supportedURLs") + } + + if needed { + t.Error("needed should be false when preflight fails") + } +} + +func TestPreflightInitPlan_UnrecognisedStatusFailsClosed(t *testing.T) { + d := newFakeDevice(t) + d.configurationStatus = "SOMETHING_UNEXPECTED" + + m := &Manager{} + + needed, status, err := m.PreflightInitPlan(d.addr) + if err == nil { + t.Fatal("expected an error for an unrecognised status value") + } + + if needed { + t.Error("needed should be false when the status is unrecognised") + } + + if status != "SOMETHING_UNEXPECTED" { + t.Errorf("status = %q, want the raw unrecognised value returned alongside the error", status) + } +} + func TestIsValidAccountID(t *testing.T) { cases := []struct { in string