From ea4d8bacacc873768d25bfcb704e7c54efdfdca5 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Thu, 7 May 2026 08:43:34 +0200 Subject: [PATCH] revert(setup): revert OverrideSdkPrivateCfg.xml migration approach (#220) The OverrideSdkPrivateCfg.xml override path introduced in #209 does not work on SoundTouch 10 (and likely other models): the firmware ignores the override file, leaving the device pointing at the original Bose cloud URLs. Revert to editing SoundTouchSdkPrivateCfg.xml directly with a .original backup, which is the approach known to work. Relates to #214 --------- Co-authored-by: Claude Sonnet 4.6 --- pkg/service/setup/setup.go | 100 ++++++++++++++------------------ pkg/service/setup/setup_test.go | 18 +----- 2 files changed, 47 insertions(+), 71 deletions(-) diff --git a/pkg/service/setup/setup.go b/pkg/service/setup/setup.go index 324d637..af7d44a 100644 --- a/pkg/service/setup/setup.go +++ b/pkg/service/setup/setup.go @@ -37,12 +37,6 @@ const ( // SoundTouchSdkPrivateCfgPath is the path to the speaker's private configuration file on device. const SoundTouchSdkPrivateCfgPath = "/opt/Bose/etc/SoundTouchSdkPrivateCfg.xml" -// SoundTouchSdkPrivateCfgOverridePath is the path to the speaker's override configuration file on device. -// The firmware reads this file in preference to SoundTouchSdkPrivateCfgPath when it exists. -// Writing here is safer than editing the original: a malformed override cannot cause a reboot loop -// because the device falls back to the untouched original. (Credit: Ueberbose team via soundcork.) -const SoundTouchSdkPrivateCfgOverridePath = "/mnt/nv/OverrideSdkPrivateCfg.xml" - // PrivateCfg represents the SoundTouchSdkPrivateCfg XML structure. type PrivateCfg struct { XMLName xml.Name `xml:"SoundTouchSdkPrivateCfg" json:"-"` @@ -505,26 +499,10 @@ func (m *Manager) populateDeviceInfo(summary *MigrationSummary, deviceIP string) // checkCurrentConfig reads and validates the current speaker configuration func (m *Manager) checkCurrentConfig(summary *MigrationSummary, deviceIP string) (string, error) { + path := SoundTouchSdkPrivateCfgPath client := m.NewSSH(deviceIP) - // Check for override config (new-style XML migration) — the device prefers this over the original. - // Test existence first: client.Run uses CombinedOutput, so a missing file's stderr would otherwise - // be returned as a non-empty config and surfaced to the UI. - if _, checkErr := client.Run(fmt.Sprintf("[ -f %s ]", SoundTouchSdkPrivateCfgOverridePath)); checkErr == nil { - if overrideCfg, _ := client.Run(fmt.Sprintf("cat %s", SoundTouchSdkPrivateCfgOverridePath)); overrideCfg != "" { - summary.SSHSuccess = true - // The untouched factory config at the original path is the OriginalConfig. - if origCfg, _ := client.Run(fmt.Sprintf("cat %s", SoundTouchSdkPrivateCfgPath)); origCfg != "" { - summary.OriginalConfig = origCfg - } - - return overrideCfg, nil - } - } - - path := SoundTouchSdkPrivateCfgPath - - // Check if .original exists (legacy migration: original file was edited directly) + // Check if .original exists if _, checkErr := client.Run(fmt.Sprintf("[ -f %s.original ]", path)); checkErr == nil { if originalConfig, _ := client.Run(fmt.Sprintf("cat %s.original", path)); originalConfig != "" { summary.OriginalConfig = originalConfig @@ -749,7 +727,7 @@ func (m *Manager) checkDNSPreFlight() error { return nil } -func (m *Manager) migrateViaXML(deviceIP, targetURL, proxyURL string, options map[string]string, client SSHClient, _ string) (string, error) { +func (m *Manager) migrateViaXML(deviceIP, targetURL, proxyURL string, options map[string]string, client SSHClient, rwCmd string) (string, error) { var logs string out, err := m.EnsureRemoteServices(deviceIP) @@ -800,17 +778,45 @@ func (m *Manager) migrateViaXML(deviceIP, targetURL, proxyURL string, options ma // Add XML header xmlContent = append([]byte("\n"), xmlContent...) - // Write to the override path; the original at SoundTouchSdkPrivateCfgPath is left untouched. - // /mnt/nv is always writable so no rw remount is needed here. - remotePath := SoundTouchSdkPrivateCfgOverridePath + // 0. Backup original config if it doesn't exist + remotePath := SoundTouchSdkPrivateCfgPath + if backupOut, err := client.Run(fmt.Sprintf("[ -f %s.original ]", remotePath)); err != nil { + logs += fmt.Sprintf("Backing up original config to %s.original (check: %s)\n", remotePath, backupOut) + fmt.Printf("Backing up original config to %s.original\n", remotePath) + + if output, err := client.Run(fmt.Sprintf("%s && cp %s %s.original", rwCmd, remotePath, remotePath)); err != nil { + logs += fmt.Sprintf("cp backup failed: %v (output: %s)\n", err, output) + fmt.Printf("cp backup failed: %v (output: %s)\n", err, output) + + if config, err := client.Run(fmt.Sprintf("cat %s", remotePath)); err == nil && config != "" { + if err := client.UploadContent([]byte(config), remotePath+".original"); err != nil { + logs += "failed to upload backup config: " + err.Error() + "\n" + return logs, fmt.Errorf("cannot create backup of %s before migration: %w", remotePath, err) + } + + logs += "Uploaded backup config via fallback\n" + } else { + return logs, fmt.Errorf("cannot create backup of %s before migration: failed to read original config", remotePath) + } + } else { + logs += "Copied backup config to .original\n" + } + } else { + logs += "Backup .original already exists\n" + } + + // 1. Upload the configuration + out, _ = client.Run(rwCmd) + + logs += rwCmd + ": " + out + "\n" if err := client.UploadContent(xmlContent, remotePath); err != nil { return logs, fmt.Errorf("failed to upload config: %w", err) } logs += "Uploaded new configuration to " + remotePath + "\n" - // Verify the configuration on device + // 2. Verify the configuration on device if verification, err := client.Run(fmt.Sprintf("cat %s", remotePath)); err == nil { if !strings.Contains(verification, cfg.MargeServerUrl) { return logs, fmt.Errorf("verification failed: uploaded config on %s does not contain expected margeServerUrl", deviceIP) @@ -1499,36 +1505,18 @@ func (m *Manager) RevertMigration(deviceIP string) (string, error) { func (m *Manager) revertXMLConfig(client SSHClient, rwCmd string) (string, error) { var logs string - reverted := false + remotePath := SoundTouchSdkPrivateCfgPath + if _, err := client.Run(fmt.Sprintf("[ -f %s.original ]", remotePath)); err == nil { + logs += fmt.Sprintf("Reverting %s from backup\n", remotePath) + fmt.Printf("Reverting %s from backup\n", remotePath) + out, err := client.Run(fmt.Sprintf("%s && cp %s.original %s", rwCmd, remotePath, remotePath)) - // Remove override file if it exists (new-style XML migration). - if _, err := client.Run(fmt.Sprintf("[ -f %s ]", SoundTouchSdkPrivateCfgOverridePath)); err == nil { - logs += fmt.Sprintf("Removing override config %s\n", SoundTouchSdkPrivateCfgOverridePath) - out, err := client.Run(fmt.Sprintf("rm -f %s", SoundTouchSdkPrivateCfgOverridePath)) - - logs += fmt.Sprintf("rm %s: %s\n", SoundTouchSdkPrivateCfgOverridePath, out) + logs += fmt.Sprintf("cp %s.original %s: %s\n", remotePath, remotePath, out) if err != nil { - return logs, fmt.Errorf("failed to remove override config: %w", err) + return logs, fmt.Errorf("failed to revert %s: %w", remotePath, err) } - - reverted = true - } - - // Restore from .original backup if present (legacy migration: original file was edited directly). - if _, err := client.Run(fmt.Sprintf("[ -f %s.original ]", SoundTouchSdkPrivateCfgPath)); err == nil { - logs += fmt.Sprintf("Reverting %s from legacy backup\n", SoundTouchSdkPrivateCfgPath) - out, err := client.Run(fmt.Sprintf("%s && cp %s.original %s", rwCmd, SoundTouchSdkPrivateCfgPath, SoundTouchSdkPrivateCfgPath)) - - logs += fmt.Sprintf("cp %s.original %s: %s\n", SoundTouchSdkPrivateCfgPath, SoundTouchSdkPrivateCfgPath, out) - if err != nil { - return logs, fmt.Errorf("failed to revert %s: %w", SoundTouchSdkPrivateCfgPath, err) - } - - reverted = true - } - - if !reverted { - return logs, fmt.Errorf("nothing to revert: no override config at %s or backup at %s.original", SoundTouchSdkPrivateCfgOverridePath, SoundTouchSdkPrivateCfgPath) + } else { + return logs, fmt.Errorf("backup %s.original not found, cannot revert", remotePath) } return logs, nil diff --git a/pkg/service/setup/setup_test.go b/pkg/service/setup/setup_test.go index 61fd078..f7d61bb 100644 --- a/pkg/service/setup/setup_test.go +++ b/pkg/service/setup/setup_test.go @@ -1535,11 +1535,9 @@ func TestCheckIsMigrated(t *testing.T) { }) } -// TestCheckCurrentConfig_OverrideMissing reproduces issue #214: when the override -// file does not exist, client.Run returns the cat stderr ("cat: can't open ...") -// via CombinedOutput. The previous implementation treated that non-empty stderr -// as a valid config and surfaced it to the UI. Existence must be tested first. -func TestCheckCurrentConfig_OverrideMissing(t *testing.T) { +// TestCheckCurrentConfig_ReadsOriginalPath verifies that checkCurrentConfig reads +// from SoundTouchSdkPrivateCfgPath on an unmigrated device (issue #214 regression test). +func TestCheckCurrentConfig_ReadsOriginalPath(t *testing.T) { m := NewManager("http://aftertouch:8000", nil, nil) originalCfg := "http://streaming.bose.com" @@ -1547,13 +1545,6 @@ func TestCheckCurrentConfig_OverrideMissing(t *testing.T) { m.NewSSH = func(host string) SSHClient { return &mockSSH{ runFunc: func(command string) (string, error) { - if command == fmt.Sprintf("[ -f %s ]", SoundTouchSdkPrivateCfgOverridePath) { - return "", fmt.Errorf("exit status 1") - } - if command == fmt.Sprintf("cat %s", SoundTouchSdkPrivateCfgOverridePath) { - return fmt.Sprintf("cat: can't open '%s': No such file or directory\n", SoundTouchSdkPrivateCfgOverridePath), - fmt.Errorf("exit status 1") - } if strings.HasPrefix(command, "[ -f ") && strings.Contains(command, ".original") { return "", fmt.Errorf("exit status 1") } @@ -1573,9 +1564,6 @@ func TestCheckCurrentConfig_OverrideMissing(t *testing.T) { if cfg != originalCfg { t.Errorf("Expected current config to be the original SoundTouchSdkPrivateCfg.xml, got %q", cfg) } - if strings.Contains(cfg, "No such file or directory") { - t.Errorf("Current config must not contain cat stderr from missing override file: %q", cfg) - } if !summary.SSHSuccess { t.Errorf("Expected SSHSuccess to be true when original config is readable") }