From 69a21cdda7100257b19dabdd87d7404d015a4ed7 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sun, 9 Aug 2026 11:04:56 +0200 Subject: [PATCH] feat(cli): configurable pause between enable-ssh --full-config commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per #515 comment 5228449448: on a real Lifestyle console, the same 6 commands (5 sys configuration/envswitch + reboot) sent back-to-back left sshd down after reboot, but succeeded sent one at a time with ~7s gaps — same commands, same order, same device, minutes apart. Sending fast may not let the device fully process one command before the next arrives. Adds --command-delay (setup.DefaultTelnetCommandDelay, 3s), threaded through EnableSSHViaTelnetFullConfig/runTelnetInjection (pause after each of the 5 commands) and runEnableSSHInjection (one more pause before the reboot). 0 restores the old back-to-back behavior. The reporter didn't try to find the true minimum, just confirmed ~7s works and speculated "a second or two may well be enough" — 3s is a middle ground, tunable via the flag if a specific device needs more. Also prints an approximate total for the injection phase up front (6 steps x delay, ~18s at the default) so the command doesn't look hung — separate from the existing --wait message for sshd coming up after reboot, which can take much longer. Refs #515 --- cmd/soundtouch-cli/cmd_setup.go | 29 ++++++++--- pkg/service/setup/enable_ssh.go | 39 +++++++++++---- pkg/service/setup/enable_ssh_test.go | 75 +++++++++++++++++++++++++++- 3 files changed, 126 insertions(+), 17 deletions(-) diff --git a/cmd/soundtouch-cli/cmd_setup.go b/cmd/soundtouch-cli/cmd_setup.go index e0a4420..f89e3d4 100644 --- a/cmd/soundtouch-cli/cmd_setup.go +++ b/cmd/soundtouch-cli/cmd_setup.go @@ -546,17 +546,23 @@ func setupSSHCheckCmd() *cli.Command { // runEnableSSHInjection runs the port-17000 SSH-enable injection over telnet, // printing the device transcript as it goes. With fullConfig it sends the // #515 sequence (all four config URLs with the injection on margeServerUrl, not -// just envswitch) and reboots afterwards; otherwise it sends the single- -// envswitch default that fires on the speaker's next boseurls check. -func runEnableSSHInjection(m *setup.Manager, host, serviceURL string, fullConfig bool) error { +// just envswitch), pausing commandDelay between each of the 6 steps (5 +// commands + reboot) — see setup.DefaultTelnetCommandDelay for why the pause +// exists — then reboots; otherwise it sends the single-envswitch default that +// fires on the speaker's next boseurls check (no pause needed, it's one +// command). +func runEnableSSHInjection(m *setup.Manager, host, serviceURL string, fullConfig bool, commandDelay time.Duration) error { var ( logs string err error ) if fullConfig { - fmt.Printf("Enabling SSH on %s via telnet :17000 (full #515 sequence: all four config URLs with the injection on margeServerUrl, then reboot)...\n", host) - logs, err = m.EnableSSHViaTelnetFullConfig(host, serviceURL) + // 6 steps total (5 commands + reboot), so 6 gaps between/around them. + fmt.Printf("Enabling SSH on %s via telnet :17000 (full #515 sequence: all four config URLs with "+ + "the injection on margeServerUrl, %s between each of 6 steps — about %s before the reboot fires "+ + "— then reboot)...\n", host, commandDelay, 6*commandDelay) + logs, err = m.EnableSSHViaTelnetFullConfig(host, serviceURL, commandDelay) } else { fmt.Printf("Enabling SSH on %s via telnet :17000 (runs on the speaker's next boseurls check, up to ~60s)...\n", host) logs, err = m.EnableSSHViaTelnet(host, serviceURL) @@ -575,6 +581,10 @@ func runEnableSSHInjection(m *setup.Manager, host, serviceURL string, fullConfig return nil } + if commandDelay > 0 { + time.Sleep(commandDelay) + } + fmt.Println("Rebooting the speaker to apply the new configuration...") rlogs, rerr := m.Reboot(host, setup.RebootMethodTelnet) @@ -613,6 +623,13 @@ func setupEnableSSHCmd() *cli.Command { Usage: "For stubborn devices (ST Portable, CineMate 520) where the default single-envswitch injection is accepted but sshd never starts: " + "replicate the #515 manual sequence — write all four sys configuration URL keys with the SSH-enable injection on margeServerUrl (not just envswitch), then reboot", }, + &cli.DurationFlag{ + Name: "command-delay", + Value: setup.DefaultTelnetCommandDelay, + Usage: "Only affects --full-config: pause between each of its 6 steps (5 commands + reboot). Confirmed necessary on a real device (#515) — " + + "the same commands sent back-to-back left sshd down after reboot, but succeeded sent one at a time with ~7s gaps. Raise this if the " + + "default doesn't work on your device; 0 sends everything back-to-back (the old behavior)", + }, &cli.BoolFlag{ Name: "no-reset-urls", Usage: "Skip restoring clean boseurls after SSH is up (leaves the injected marge URL in place)", @@ -646,7 +663,7 @@ func setupEnableSSHCmd() *cli.Command { serviceURL = "https://aftertouch.invalid" } - if err := runEnableSSHInjection(m, cfg.Host, serviceURL, c.Bool("full-config")); err != nil { + if err := runEnableSSHInjection(m, cfg.Host, serviceURL, c.Bool("full-config"), c.Duration("command-delay")); err != nil { return err } diff --git a/pkg/service/setup/enable_ssh.go b/pkg/service/setup/enable_ssh.go index 89a198f..18379a1 100644 --- a/pkg/service/setup/enable_ssh.go +++ b/pkg/service/setup/enable_ssh.go @@ -36,6 +36,17 @@ func (m *Manager) ResetBoseURLs(deviceIP, serviceURL string) (string, error) { return m.setBoseURLsViaTelnet(deviceIP, serviceURL, serviceURL+"/update") } +// DefaultTelnetCommandDelay is the pause between successive commands in +// EnableSSHViaTelnetFullConfig's sequence. Confirmed necessary on a real +// device (#515, issue comment 5228449448): the same six commands sent +// back-to-back left sshd down after reboot, but succeeded when sent one at a +// time with ~7s gaps — sending fast enough may not let the device fully +// process one command before the next arrives. 3s is a reasonable middle +// ground (the reporter didn't try to find the true minimum); the caller +// exposes it as a flag so a specific device can be tuned without a code +// change. +const DefaultTelnetCommandDelay = 3 * time.Second + // EnableSSHViaTelnetFullConfig is the #515 variant of EnableSSHViaTelnet for // devices where the single-envswitch injection is accepted and persisted but // sshd never starts (ST Portable, CineMate 520; see also memory note #471). It @@ -43,13 +54,15 @@ func (m *Manager) ResetBoseURLs(deviceIP, serviceURL string) (string, error) { // writes all four `sys configuration` URL keys with the remote_services // injection on margeServerUrl (the runtime layer, not just the envswitch // persistence layer), mirrors the injection into `envswitch boseurls set`, and -// verifies with getpdo. The caller should reboot afterwards (the injection -// fires on the speaker's next full config re-parse at boot) and then -// WaitForSSHPort. +// verifies with getpdo. The caller should pause commandDelay again, reboot +// (the injection fires on the speaker's next full config re-parse at boot), +// and then WaitForSSHPort. // // serviceURL is the AfterTouch service base the speaker should point at // (e.g. https://192.0.2.10:8443). It must not contain a double quote. -func (m *Manager) EnableSSHViaTelnetFullConfig(deviceIP, serviceURL string) (string, error) { +// commandDelay is the pause between each command (see +// DefaultTelnetCommandDelay); 0 sends them back-to-back. +func (m *Manager) EnableSSHViaTelnetFullConfig(deviceIP, serviceURL string, commandDelay time.Duration) (string, error) { u := defaultTelnetURLs(serviceURL) margeInjected := serviceURL + remoteServicesInjection @@ -65,16 +78,18 @@ func (m *Manager) EnableSSHViaTelnetFullConfig(deviceIP, serviceURL string) (str `envswitch boseurls set "` + margeInjected + `" "` + u.SwUpdate + `"`, } - return m.runTelnetInjection(deviceIP, []string{serviceURL, u.SwUpdate}, cmds) + return m.runTelnetInjection(deviceIP, []string{serviceURL, u.SwUpdate}, cmds, commandDelay) } // runTelnetInjection opens the port-17000 shell, runs an ordered list of // commands (aborting on the first transport error or "command not found" -// rejection), then logs a getpdo verification. forbidQuote values are checked -// for an embedded double quote, which would break the command parsing. -// Verification is best-effort (logged, never fatal) to match enable-ssh's -// forgiving philosophy and tolerate the aftertouch.invalid placeholder. -func (m *Manager) runTelnetInjection(deviceIP string, forbidQuote, cmds []string) (string, error) { +// rejection), pausing commandDelay after each one (see +// DefaultTelnetCommandDelay), then logs a getpdo verification. forbidQuote +// values are checked for an embedded double quote, which would break the +// command parsing. Verification is best-effort (logged, never fatal) to +// match enable-ssh's forgiving philosophy and tolerate the +// aftertouch.invalid placeholder. +func (m *Manager) runTelnetInjection(deviceIP string, forbidQuote, cmds []string, commandDelay time.Duration) (string, error) { if m.NewTelnet == nil { return "", errors.New("telnet not configured: Manager.NewTelnet is nil") } @@ -109,6 +124,10 @@ func (m *Manager) runTelnetInjection(deviceIP string, forbidQuote, cmds []string if isCommandNotFound(resp) { return logs.String(), fmt.Errorf("device rejected %q (firmware does not expose this command)", cmd) } + + if commandDelay > 0 { + time.Sleep(commandDelay) + } } if verify, err := t.SendCommand("getpdo CurrentSystemConfiguration"); err == nil { diff --git a/pkg/service/setup/enable_ssh_test.go b/pkg/service/setup/enable_ssh_test.go index 257aa16..edb921a 100644 --- a/pkg/service/setup/enable_ssh_test.go +++ b/pkg/service/setup/enable_ssh_test.go @@ -3,6 +3,7 @@ package setup import ( "strings" "testing" + "time" ) func TestEnableSSHViaTelnet_BuildsInjectedCommand(t *testing.T) { @@ -44,7 +45,7 @@ func TestEnableSSHViaTelnetFullConfig_BuildsInjectedSequence(t *testing.T) { f := &fakeTelnet{responses: resp} m := newFakeTelnetManager(f) - if _, err := m.EnableSSHViaTelnetFullConfig("192.0.2.10", svc); err != nil { + if _, err := m.EnableSSHViaTelnetFullConfig("192.0.2.10", svc, 0); err != nil { t.Fatalf("EnableSSHViaTelnetFullConfig: %v", err) } @@ -59,6 +60,78 @@ func TestEnableSSHViaTelnetFullConfig_BuildsInjectedSequence(t *testing.T) { } } +// fullConfigResponses builds the {command: "OK"} map for +// EnableSSHViaTelnetFullConfig's fixed 6-step sequence (5 commands + the +// getpdo verification) against svc, matching +// TestEnableSSHViaTelnetFullConfig_BuildsInjectedSequence's command list. +func fullConfigResponses(svc string) map[string]string { + injected := svc + `;touch /tmp/remote_services;/etc/init.d/sshd start` + + cmds := []string{ + `sys configuration bmxRegistryUrl "` + svc + `/bmx/registry/v1/services"`, + `sys configuration statsServerUrl "` + svc + `"`, + `sys configuration margeServerUrl "` + injected + `"`, + `sys configuration swUpdateUrl "` + svc + `/updates/soundtouch"`, + `envswitch boseurls set "` + injected + `" "` + svc + `/updates/soundtouch"`, + `getpdo CurrentSystemConfiguration`, + } + + resp := make(map[string]string, len(cmds)) + for _, c := range cmds { + resp[c] = "OK\n" + } + + return resp +} + +// TestEnableSSHViaTelnetFullConfig_PausesBetweenCommands is the regression +// test for #515 comment 5228449448: the same commands sent back-to-back +// left sshd down on a real device, but succeeded sent one at a time with +// gaps. Uses a small real duration rather than a fake clock/injectable +// sleeper — simplest thing that actually proves time.Sleep is in the loop, +// and small enough (5 gaps x 5ms) not to slow the suite down. +func TestEnableSSHViaTelnetFullConfig_PausesBetweenCommands(t *testing.T) { + const svc = "https://192.0.2.10:8443" + const delay = 5 * time.Millisecond + + f := &fakeTelnet{responses: fullConfigResponses(svc)} + m := newFakeTelnetManager(f) + + start := time.Now() + + if _, err := m.EnableSSHViaTelnetFullConfig("192.0.2.10", svc, delay); err != nil { + t.Fatalf("EnableSSHViaTelnetFullConfig: %v", err) + } + + elapsed := time.Since(start) + // 5 real commands = 5 gaps (see runTelnetInjection: delay after each + // command in the loop, including before the getpdo verification). + wantMin := 5 * delay + + if elapsed < wantMin { + t.Errorf("elapsed %v, want at least %v (delay not applied between commands)", elapsed, wantMin) + } +} + +// TestEnableSSHViaTelnetFullConfig_ZeroDelayIsInstant verifies 0 keeps the +// old back-to-back behavior — no accidental minimum sleep. +func TestEnableSSHViaTelnetFullConfig_ZeroDelayIsInstant(t *testing.T) { + const svc = "https://192.0.2.10:8443" + + f := &fakeTelnet{responses: fullConfigResponses(svc)} + m := newFakeTelnetManager(f) + + start := time.Now() + + if _, err := m.EnableSSHViaTelnetFullConfig("192.0.2.10", svc, 0); err != nil { + t.Fatalf("EnableSSHViaTelnetFullConfig: %v", err) + } + + if elapsed := time.Since(start); elapsed > 50*time.Millisecond { + t.Errorf("elapsed %v with a 0 delay, expected near-instant", elapsed) + } +} + func TestResetBoseURLs_BuildsCleanCommand(t *testing.T) { const svc = "https://192.0.2.10:8443"