From a1ae10650f0579cda2fd2ec6a46e581e1bd7465c Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Wed, 13 May 2026 08:17:45 +0200 Subject: [PATCH] style(setup): address actionable golangci-lint findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the lint hits that pointed at real bugs or dead code; leaves the remaining style-only suggestions (rangeValCopy micro-copies, gocyclo informational, intentional name choices like SetupStateMachine) alone. - pkg/models/clockdisplay.go: restore XMLName tag on both ClockDisplay and ClockDisplayRequest. The earlier `xml:"-"` clashed with ClockDisplayUpdatedEvent.ClockDisplay's `xml:"clockDisplay"` tag (SA5008). Custom MarshalXML/UnmarshalXML still own the wire format. - pkg/service/setup/setup.go: drop the now-unused checkRemoteServices helper (replaced by applyProbeToSummary) and rename the unused deviceIP parameter of populatePlannedNetworkConfig to _. - pkg/service/setup/setup_session.go: collapse sendStep's (string, error) return to plain error — every caller already discarded the string. - pkg/service/setup/init_plan.go: rename shadowed err variables to rwErr / genErr / invalidErr / nilErr / stepErr. - cmd/soundtouch-cli/cmd_setup.go: drop redundant int(syscall.Stdin) conversion (already int) and rename a shadowed err to pairErr. go build ./..., go vet ./..., and tests for the touched packages all green. Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/soundtouch-cli/cmd_setup.go | 8 +++--- pkg/models/clockdisplay.go | 4 +-- pkg/service/setup/init_plan.go | 32 +++++++++++----------- pkg/service/setup/setup.go | 23 +--------------- pkg/service/setup/setup_session.go | 44 ++++++++++++------------------ 5 files changed, 41 insertions(+), 70 deletions(-) diff --git a/cmd/soundtouch-cli/cmd_setup.go b/cmd/soundtouch-cli/cmd_setup.go index a4e2e8c..a3f5db1 100644 --- a/cmd/soundtouch-cli/cmd_setup.go +++ b/cmd/soundtouch-cli/cmd_setup.go @@ -626,7 +626,7 @@ func promptBasicAuth() (string, string, error) { fmt.Fprint(os.Stderr, "Password: ") - pass, err := term.ReadPassword(int(syscall.Stdin)) + pass, err := term.ReadPassword(syscall.Stdin) fmt.Fprintln(os.Stderr) @@ -1373,9 +1373,9 @@ func runPairBare(c *cli.Context, deviceIP, accountID string) error { fmt.Printf("→ setMargeAccount accountID=%s (no SETUP bracket)\n", accountID) - if err := session.SetMargeAccount(ctx, accountID, ""); err != nil { - PrintError(fmt.Sprintf("setMargeAccount: %v", err)) - return err + if pairErr := session.SetMargeAccount(ctx, accountID, ""); pairErr != nil { + PrintError(fmt.Sprintf("setMargeAccount: %v", pairErr)) + return pairErr } time.Sleep(2 * time.Second) diff --git a/pkg/models/clockdisplay.go b/pkg/models/clockdisplay.go index fe67160..26472ea 100644 --- a/pkg/models/clockdisplay.go +++ b/pkg/models/clockdisplay.go @@ -28,7 +28,7 @@ import ( // other callers don't have to be rewritten; custom MarshalXML / // UnmarshalXML methods bridge to the nested format on the wire. type ClockDisplay struct { - XMLName xml.Name `xml:"-"` + XMLName xml.Name `xml:"clockDisplay"` DeviceID string Enabled bool Format string // public-facing values: "12", "24", "auto" @@ -234,7 +234,7 @@ func (c *ClockDisplay) IsEmpty() bool { // settings. Fields use the same public names as the response struct; // MarshalXML produces the nested wire format the device requires. type ClockDisplayRequest struct { - XMLName xml.Name `xml:"-"` + XMLName xml.Name `xml:"clockDisplay"` Enabled *bool Format string Brightness *int diff --git a/pkg/service/setup/init_plan.go b/pkg/service/setup/init_plan.go index d207cd6..91a2561 100644 --- a/pkg/service/setup/init_plan.go +++ b/pkg/service/setup/init_plan.go @@ -137,9 +137,9 @@ func (m *Manager) ExecuteInitPlan(ctx context.Context, plan InitPlan, progress P emit(StepURLRewrite, "telnet URL rewrite", StatusRunning, nil) urls := defaultTelnetURLs(plan.ServiceURL) - if _, err := m.migrateViaTelnet(plan.DeviceIP, plan.ServiceURL, urls); err != nil { - emit(StepURLRewrite, "telnet URL rewrite", StatusFailed, err) - return plan, fmt.Errorf("URL rewrite: %w", err) + if _, rwErr := m.migrateViaTelnet(plan.DeviceIP, plan.ServiceURL, urls); rwErr != nil { + emit(StepURLRewrite, "telnet URL rewrite", StatusFailed, rwErr) + return plan, fmt.Errorf("URL rewrite: %w", rwErr) } emit(StepURLRewrite, "telnet URL rewrite", StatusOK, nil) @@ -154,10 +154,10 @@ func (m *Manager) ExecuteInitPlan(ctx context.Context, plan InitPlan, progress P known := listKnownAccountIDs(m) - id, err := GenerateAccountID(known) - if err != nil { - emit(StepGenerateAccountID, "generate account ID", StatusFailed, err) - return plan, fmt.Errorf("generate account ID: %w", err) + id, genErr := GenerateAccountID(known) + if genErr != nil { + emit(StepGenerateAccountID, "generate account ID", StatusFailed, genErr) + return plan, fmt.Errorf("generate account ID: %w", genErr) } plan.AccountID = id @@ -165,19 +165,19 @@ func (m *Manager) ExecuteInitPlan(ctx context.Context, plan InitPlan, progress P emit(StepGenerateAccountID, "generate account ID="+id, StatusOK, nil) } } else if !IsValidAccountID(plan.AccountID) { - err := fmt.Errorf("invalid AccountID %q: must be exactly 7 digits", plan.AccountID) - emit(StepGenerateAccountID, "validate account ID", StatusFailed, err) + invalidErr := fmt.Errorf("invalid AccountID %q: must be exactly 7 digits", plan.AccountID) + emit(StepGenerateAccountID, "validate account ID", StatusFailed, invalidErr) - return plan, err + return plan, invalidErr } emit(StepDialWebSocket, "dial websocket", StatusRunning, nil) if m.NewSetupSession == nil { - err := errors.New("Manager.NewSetupSession is nil — call NewManager or set it explicitly") - emit(StepDialWebSocket, "dial websocket", StatusFailed, err) + nilErr := errors.New("Manager.NewSetupSession is nil — call NewManager or set it explicitly") + emit(StepDialWebSocket, "dial websocket", StatusFailed, nilErr) - return plan, err + return plan, nilErr } session, err := m.NewSetupSession(plan.DeviceIP, info.DeviceID, plan.StepTimeout) @@ -229,9 +229,9 @@ func (m *Manager) ExecuteInitPlan(ctx context.Context, plan InitPlan, progress P emit(st.kind, st.name, StatusRunning, nil) - if err := st.fn(ctx); err != nil { - emit(st.kind, st.name, StatusFailed, err) - return plan, fmt.Errorf("%s: %w", st.name, err) + if stepErr := st.fn(ctx); stepErr != nil { + emit(st.kind, st.name, StatusFailed, stepErr) + return plan, fmt.Errorf("%s: %w", st.name, stepErr) } emit(st.kind, st.name, StatusOK, nil) diff --git a/pkg/service/setup/setup.go b/pkg/service/setup/setup.go index eea9e2c..828a4ea 100644 --- a/pkg/service/setup/setup.go +++ b/pkg/service/setup/setup.go @@ -357,7 +357,7 @@ func (m *Manager) GetMigrationSummary(deviceIP, targetURL, proxyURL string, opti return summary, nil } -func (m *Manager) populatePlannedNetworkConfig(summary *MigrationSummary, deviceIP, targetURL string) { +func (m *Manager) populatePlannedNetworkConfig(summary *MigrationSummary, _, targetURL string) { parsedURL, err := url.Parse(targetURL) if err != nil { return @@ -744,27 +744,6 @@ func applyURLOverrides(cfg *PrivateCfg, options map[string]string) { } } -// checkRemoteServices checks for remote services files on the device -func (m *Manager) checkRemoteServices(summary *MigrationSummary, deviceIP string) { - client := m.NewSSH(deviceIP) - locations := []string{ - "/etc/remote_services", - "/mnt/nv/remote_services", - "/tmp/remote_services", - } - - for _, loc := range locations { - if _, err := client.Run(fmt.Sprintf("[ -e %s ]", loc)); err == nil { - summary.RemoteServicesFound = append(summary.RemoteServicesFound, loc) - - summary.RemoteServicesEnabled = true - if loc != "/tmp/remote_services" { - summary.RemoteServicesPersistent = true - } - } - } -} - // checkCACertTrusted checks if the local CA certificate is already in // the device's trust store. The CALabel grep works regardless of whether // Manager.Crypto is configured — only the secondary "match cert payload" diff --git a/pkg/service/setup/setup_session.go b/pkg/service/setup/setup_session.go index dc245ec..6bfaaca 100644 --- a/pkg/service/setup/setup_session.go +++ b/pkg/service/setup/setup_session.go @@ -141,10 +141,12 @@ func (s *SetupSession) Close() error { // sendStep wraps body in the canonical
… // envelope, sends it, and drains incoming frames until one references the // same requestID, status path, or url attribute — that frame is the ack. -// Pushed and frames are ignored. -func (s *SetupSession) sendStep(ctx context.Context, route, method, body string) (string, error) { +// Pushed and frames are ignored. The ack +// payload is consumed for error detection () only and never +// returned — every caller discards it. +func (s *SetupSession) sendStep(ctx context.Context, route, method, body string) error { if s.conn == nil { - return "", errors.New("setup session: connection closed") + return errors.New("setup session: connection closed") } id := s.reqID.Add(1) @@ -162,7 +164,7 @@ func (s *SetupSession) sendStep(ctx context.Context, route, method, body string) _ = s.conn.SetWriteDeadline(deadline) if err := s.conn.WriteMessage(websocket.TextMessage, []byte(envelope)); err != nil { - return "", fmt.Errorf("send %s: %w", route, err) + return fmt.Errorf("send %s: %w", route, err) } idNeedle := fmt.Sprintf(`requestID="%d"`, id) @@ -174,7 +176,7 @@ func (s *SetupSession) sendStep(ctx context.Context, route, method, body string) _, data, err := s.conn.ReadMessage() if err != nil { - return "", fmt.Errorf("await ack for %s: %w", route, err) + return fmt.Errorf("await ack for %s: %w", route, err) } text := string(data) @@ -187,19 +189,18 @@ func (s *SetupSession) sendStep(ctx context.Context, route, method, body string) // Device-side errors surface as in the body. if strings.Contains(strings.ToLower(text), "`) - return err + return s.sendStep(ctx, "setup", "POST", ``) } // IdentifyEnter sends SETUP_IDENTIFY_DEVICE_ENTER. timeoutMs defaults to @@ -210,29 +211,24 @@ func (s *SetupSession) IdentifyEnter(ctx context.Context, timeoutMs int) error { } body := fmt.Sprintf(``, timeoutMs) - _, err := s.sendStep(ctx, "setup", "POST", body) - return err + return s.sendStep(ctx, "setup", "POST", body) } // SetLanguage POSTs sysLanguage. Code 2 = English. func (s *SetupSession) SetLanguage(ctx context.Context, code int) error { body := fmt.Sprintf(`%d`, code) - _, err := s.sendStep(ctx, "language", "POST", body) - - return err + return s.sendStep(ctx, "language", "POST", body) } // Enter sends SETUP_ENTER. func (s *SetupSession) Enter(ctx context.Context) error { - _, err := s.sendStep(ctx, "setup", "POST", ``) - return err + return s.sendStep(ctx, "setup", "POST", ``) } // IdentifyLeave sends SETUP_IDENTIFY_DEVICE_LEAVE. func (s *SetupSession) IdentifyLeave(ctx context.Context) error { - _, err := s.sendStep(ctx, "setup", "POST", ``) - return err + return s.sendStep(ctx, "setup", "POST", ``) } // SetName POSTs a device-name change. An empty name is a no-op. @@ -242,9 +238,8 @@ func (s *SetupSession) SetName(ctx context.Context, name string) error { } body := fmt.Sprintf(`%s`, xmlBodyEscape(name)) - _, err := s.sendStep(ctx, "name", "POST", body) - return err + return s.sendStep(ctx, "name", "POST", body) } // SetMargeAccount sends the canonical PairDeviceWithAccount envelope. @@ -264,22 +259,19 @@ func (s *SetupSession) SetMargeAccount(ctx context.Context, accountID, authToken `%s%s`, xmlBodyEscape(accountID), xmlBodyEscape(authToken), ) - _, err := s.sendStep(ctx, "setMargeAccount", "POST", body) - return err + return s.sendStep(ctx, "setMargeAccount", "POST", body) } // Leave sends SETUP_LEAVE. func (s *SetupSession) Leave(ctx context.Context) error { - _, err := s.sendStep(ctx, "setup", "POST", ``) - return err + return s.sendStep(ctx, "setup", "POST", ``) } // PushCustomerSupportInfo triggers the post-setup telemetry sync. Harmless // on our local service. func (s *SetupSession) PushCustomerSupportInfo(ctx context.Context) error { - _, err := s.sendStep(ctx, "pushCustomerSupportInfoToMarge", "GET", "") - return err + return s.sendStep(ctx, "pushCustomerSupportInfoToMarge", "GET", "") } // xmlAttrEscape escapes the small set of characters that would break an