From 9e384840bab58a25842771e1a26364dae80893a5 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Wed, 13 May 2026 08:22:03 +0200 Subject: [PATCH] style(setup): un-stutter exported type names and tighten range loops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename SetupStateMachine → setup.StateMachine, SetupSessionConfig → setup.SessionConfig, SetupSession → setup.Session, and DialSetupSession → setup.DialSession. The Setup* prefix only stutters in package context (`setup.SetupSession`); the renamed forms read cleaner at every call site (revive: exported). - Iterate r.Network.Interfaces.Interfaces by index in cmd_setup.go rather than by value — each NetworkInterface is 168 bytes and the per-iteration copy was unnecessary (gocritic: rangeValCopy). Test fixtures (fakeSetupSession → fakeSession, TestSetupSession_* → TestSession_*) renamed by the same substring replacement to keep naming consistent inside the package. Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/soundtouch-cli/cmd_setup.go | 18 +++++----- pkg/service/setup/init_plan.go | 6 ++-- pkg/service/setup/init_plan_test.go | 44 +++++++++++------------ pkg/service/setup/setup.go | 10 +++--- pkg/service/setup/setup_session.go | 46 ++++++++++++------------- pkg/service/setup/setup_session_test.go | 30 ++++++++-------- 6 files changed, 78 insertions(+), 76 deletions(-) diff --git a/cmd/soundtouch-cli/cmd_setup.go b/cmd/soundtouch-cli/cmd_setup.go index a3f5db1..bab9ab0 100644 --- a/cmd/soundtouch-cli/cmd_setup.go +++ b/cmd/soundtouch-cli/cmd_setup.go @@ -129,7 +129,9 @@ func renderInspectReport(r *setup.InspectReport) { } else if r.Network != nil { fmt.Println("Network") - for _, iface := range r.Network.Interfaces.Interfaces { + for i := range r.Network.Interfaces.Interfaces { + iface := &r.Network.Interfaces.Interfaces[i] + fmt.Printf(" %s\n", iface.Type) fmt.Printf(" state : %s\n", iface.State) @@ -1034,9 +1036,9 @@ func renderPlanState(deviceIP string, inspect *setup.InspectReport, summary *set currentSSID := "" if inspect.Network != nil { - for _, iface := range inspect.Network.Interfaces.Interfaces { - if iface.SSID != "" { - currentSSID = iface.SSID + for i := range inspect.Network.Interfaces.Interfaces { + if ssid := inspect.Network.Interfaces.Interfaces[i].SSID; ssid != "" { + currentSSID = ssid break } } @@ -1259,9 +1261,9 @@ func inspectedSSID(r *setup.InspectReport) string { return "" } - for _, iface := range r.Network.Interfaces.Interfaces { - if iface.SSID != "" { - return iface.SSID + for i := range r.Network.Interfaces.Interfaces { + if ssid := r.Network.Interfaces.Interfaces[i].SSID; ssid != "" { + return ssid } } @@ -1359,7 +1361,7 @@ func runPairBare(c *cli.Context, deviceIP, accountID string) error { fmt.Printf("pre /info deviceID=%s margeAccountUUID=%q margeURL=%q\n", info.DeviceID, info.MargeAccountUUID, info.MargeURL) - session, err := setup.DialSetupSession(deviceIP, info.DeviceID, setup.SetupSessionConfig{ + session, err := setup.DialSession(deviceIP, info.DeviceID, setup.SessionConfig{ StepTimeout: c.Duration("step-timeout"), }) if err != nil { diff --git a/pkg/service/setup/init_plan.go b/pkg/service/setup/init_plan.go index 91a2561..07d0579 100644 --- a/pkg/service/setup/init_plan.go +++ b/pkg/service/setup/init_plan.go @@ -173,14 +173,14 @@ func (m *Manager) ExecuteInitPlan(ctx context.Context, plan InitPlan, progress P emit(StepDialWebSocket, "dial websocket", StatusRunning, nil) - if m.NewSetupSession == nil { - nilErr := errors.New("Manager.NewSetupSession is nil — call NewManager or set it explicitly") + if m.NewSession == nil { + nilErr := errors.New("Manager.NewSession is nil — call NewManager or set it explicitly") emit(StepDialWebSocket, "dial websocket", StatusFailed, nilErr) return plan, nilErr } - session, err := m.NewSetupSession(plan.DeviceIP, info.DeviceID, plan.StepTimeout) + session, err := m.NewSession(plan.DeviceIP, info.DeviceID, plan.StepTimeout) if err != nil { emit(StepDialWebSocket, "dial websocket", StatusFailed, err) return plan, fmt.Errorf("dial websocket: %w", err) diff --git a/pkg/service/setup/init_plan_test.go b/pkg/service/setup/init_plan_test.go index c39fefc..9566b55 100644 --- a/pkg/service/setup/init_plan_test.go +++ b/pkg/service/setup/init_plan_test.go @@ -11,15 +11,15 @@ import ( "time" ) -// fakeSetupSession is a SetupStateMachine that records the order of +// fakeSession is a StateMachine that records the order of // invocations and lets each test inject per-step errors. -type fakeSetupSession struct { +type fakeSession struct { calls []string errors map[string]error closed bool } -func (f *fakeSetupSession) record(name string) error { +func (f *fakeSession) record(name string) error { if e, ok := f.errors[name]; ok && e != nil { return e } @@ -29,34 +29,34 @@ func (f *fakeSetupSession) record(name string) error { return nil } -func (f *fakeSetupSession) Start(_ context.Context) error { return f.record("Start") } -func (f *fakeSetupSession) Enter(_ context.Context) error { return f.record("Enter") } -func (f *fakeSetupSession) Leave(_ context.Context) error { return f.record("Leave") } -func (f *fakeSetupSession) IdentifyLeave(_ context.Context) error { +func (f *fakeSession) Start(_ context.Context) error { return f.record("Start") } +func (f *fakeSession) Enter(_ context.Context) error { return f.record("Enter") } +func (f *fakeSession) Leave(_ context.Context) error { return f.record("Leave") } +func (f *fakeSession) IdentifyLeave(_ context.Context) error { return f.record("IdentifyLeave") } -func (f *fakeSetupSession) IdentifyEnter(_ context.Context, timeoutMs int) error { +func (f *fakeSession) IdentifyEnter(_ context.Context, timeoutMs int) error { return f.record(fmt.Sprintf("IdentifyEnter(%d)", timeoutMs)) } -func (f *fakeSetupSession) SetLanguage(_ context.Context, code int) error { +func (f *fakeSession) SetLanguage(_ context.Context, code int) error { return f.record(fmt.Sprintf("SetLanguage(%d)", code)) } -func (f *fakeSetupSession) SetName(_ context.Context, name string) error { +func (f *fakeSession) SetName(_ context.Context, name string) error { return f.record("SetName(" + name + ")") } -func (f *fakeSetupSession) SetMargeAccount(_ context.Context, accountID, token string) error { +func (f *fakeSession) SetMargeAccount(_ context.Context, accountID, token string) error { return f.record(fmt.Sprintf("SetMargeAccount(%s,%s)", accountID, token)) } -func (f *fakeSetupSession) PushCustomerSupportInfo(_ context.Context) error { +func (f *fakeSession) PushCustomerSupportInfo(_ context.Context) error { return f.record("PushCustomerSupportInfo") } -func (f *fakeSetupSession) Close() error { +func (f *fakeSession) Close() error { f.closed = true return nil } @@ -90,13 +90,13 @@ func (f *fakeInfoResponder) get(_ string) (*http.Response, error) { }, nil } -func newTestManagerWithFakes(t *testing.T, info *fakeInfoResponder, sess *fakeSetupSession) *Manager { +func newTestManagerWithFakes(t *testing.T, info *fakeInfoResponder, sess *fakeSession) *Manager { t.Helper() m := &Manager{ ServerURL: "http://aftertouch.local:8000", HTTPGet: info.get, - NewSetupSession: func(_, _ string, _ time.Duration) (SetupStateMachine, error) { + NewSession: func(_, _ string, _ time.Duration) (StateMachine, error) { return sess, nil }, } @@ -110,7 +110,7 @@ func TestExecuteInitPlan_FactoryReset_GeneratesAccountAndRunsAllSteps(t *testing paired: "", postInitPaired: "", // filled below after we know which ID was generated } - sess := &fakeSetupSession{} + sess := &fakeSession{} m := newTestManagerWithFakes(t, info, sess) // Intercept the generated account ID so we can prime the post-init @@ -176,7 +176,7 @@ func TestExecuteInitPlan_ReusesExistingAccountUUID(t *testing.T) { paired: "9876543", postInitPaired: "9876543", } - sess := &fakeSetupSession{} + sess := &fakeSession{} m := newTestManagerWithFakes(t, info, sess) plan := InitPlan{ @@ -202,7 +202,7 @@ func TestExecuteInitPlan_GeneratesAccountWhenDeviceUUIDInvalid(t *testing.T) { paired: "not-7-digits", postInitPaired: "", // we'll learn the generated ID from the result } - sess := &fakeSetupSession{} + sess := &fakeSession{} m := newTestManagerWithFakes(t, info, sess) // Pre-generate so the post-init /info knows what to return. @@ -231,7 +231,7 @@ func TestExecuteInitPlan_GeneratesAccountWhenDeviceUUIDInvalid(t *testing.T) { func TestExecuteInitPlan_RejectsInvalidSuppliedAccountID(t *testing.T) { info := &fakeInfoResponder{deviceID: "X", paired: ""} - sess := &fakeSetupSession{} + sess := &fakeSession{} m := newTestManagerWithFakes(t, info, sess) plan := InitPlan{ @@ -256,7 +256,7 @@ func TestExecuteInitPlan_RejectsInvalidSuppliedAccountID(t *testing.T) { func TestExecuteInitPlan_StopsAtFirstFailedStep(t *testing.T) { info := &fakeInfoResponder{deviceID: "X", paired: "", postInitPaired: "1234567"} - sess := &fakeSetupSession{ + sess := &fakeSession{ errors: map[string]error{ "Enter": errors.New("device dropped the SETUP_ENTER frame"), }, @@ -294,7 +294,7 @@ func TestExecuteInitPlan_StopsAtFirstFailedStep(t *testing.T) { func TestExecuteInitPlan_EmptyDeviceNameSkipsNameStep(t *testing.T) { info := &fakeInfoResponder{deviceID: "X", paired: "", postInitPaired: "1234567"} - sess := &fakeSetupSession{} + sess := &fakeSession{} m := newTestManagerWithFakes(t, info, sess) plan := InitPlan{ @@ -341,7 +341,7 @@ func TestExecuteInitPlan_FailsOnPostInitVerifyMismatch(t *testing.T) { paired: "", postInitPaired: "9999999", // not equal to plan.AccountID } - sess := &fakeSetupSession{} + sess := &fakeSession{} m := newTestManagerWithFakes(t, info, sess) plan := InitPlan{ diff --git a/pkg/service/setup/setup.go b/pkg/service/setup/setup.go index 828a4ea..11be03d 100644 --- a/pkg/service/setup/setup.go +++ b/pkg/service/setup/setup.go @@ -139,10 +139,10 @@ type Manager struct { NewSSH func(host string) SSHClient NewTelnet func(host string) TelnetClient - // NewSetupSession opens the WebSocket setup state-machine session used + // NewSession opens the WebSocket setup state-machine session used // by ExecuteInitPlan. Tests inject an in-memory fake; the production - // default is DialSetupSession. - NewSetupSession func(deviceIP, deviceID string, stepTimeout time.Duration) (SetupStateMachine, error) + // default is DialSession. + NewSession func(deviceIP, deviceID string, stepTimeout time.Duration) (StateMachine, error) // GetDNSRunning is an optional callback to check the actual state of the DNS server. GetDNSRunning func() (bool, string) @@ -167,8 +167,8 @@ func NewManager(serverURL string, ds *datastore.DataStore, cm *certmanager.Certi NewTelnet: func(host string) TelnetClient { return telnet.NewClient(host) }, - NewSetupSession: func(deviceIP, deviceID string, stepTimeout time.Duration) (SetupStateMachine, error) { - return DialSetupSession(deviceIP, deviceID, SetupSessionConfig{StepTimeout: stepTimeout}) + NewSession: func(deviceIP, deviceID string, stepTimeout time.Duration) (StateMachine, error) { + return DialSession(deviceIP, deviceID, SessionConfig{StepTimeout: stepTimeout}) }, HTTPGet: http.Get, MgmtUsername: "admin", diff --git a/pkg/service/setup/setup_session.go b/pkg/service/setup/setup_session.go index 6bfaaca..7289296 100644 --- a/pkg/service/setup/setup_session.go +++ b/pkg/service/setup/setup_session.go @@ -23,10 +23,10 @@ const ( LanguageEnglish = 2 ) -// SetupStateMachine is the surface the InitPlan orchestrator drives. The -// concrete WebSocket-backed implementation is *SetupSession; tests inject -// an in-memory fake via Manager.NewSetupSession. -type SetupStateMachine interface { +// StateMachine is the surface the InitPlan orchestrator drives. The +// concrete WebSocket-backed implementation is *Session; tests inject +// an in-memory fake via Manager.NewSession. +type StateMachine interface { Start(ctx context.Context) error IdentifyEnter(ctx context.Context, timeoutMs int) error SetLanguage(ctx context.Context, code int) error @@ -39,9 +39,9 @@ type SetupStateMachine interface { Close() error } -// SetupSessionConfig configures DialSetupSession. Zero values pick safe +// SessionConfig configures DialSession. Zero values pick safe // defaults; in production callers normally pass an empty struct. -type SetupSessionConfig struct { +type SessionConfig struct { // StepTimeout caps the per-message wait for an ack frame. Default 8 s. StepTimeout time.Duration // DialTimeout caps the WebSocket handshake. Default 10 s. @@ -54,25 +54,25 @@ type SetupSessionConfig struct { WSPort int } -// SetupSession is a synchronous request/response WebSocket session driving +// Session is a synchronous request/response WebSocket session driving // the speaker's setup state machine. It is deliberately separate from // pkg/client.WebSocketClient (which is event-oriented, auto-reconnecting, // and stateful) — setup is a short, linear sequence and benefits from a // purpose-built transport. -type SetupSession struct { +type Session struct { deviceID string conn *websocket.Conn reqID atomic.Int64 stepTimeout time.Duration } -// DialSetupSession opens a WebSocket to the speaker at deviceIP and +// DialSession opens a WebSocket to the speaker at deviceIP and // returns a session ready to drive the SETUP state machine. deviceID is // required because every envelope embeds it in the header; obtain // it from /info before calling. -func DialSetupSession(deviceIP, deviceID string, cfg SetupSessionConfig) (*SetupSession, error) { +func DialSession(deviceIP, deviceID string, cfg SessionConfig) (*Session, error) { if deviceID == "" { - return nil, errors.New("DialSetupSession: deviceID is required for message routing") + return nil, errors.New("DialSession: deviceID is required for message routing") } scheme := cfg.WSScheme @@ -117,11 +117,11 @@ func DialSetupSession(deviceIP, deviceID string, cfg SetupSessionConfig) (*Setup step = defaultSetupStepTimeout } - return &SetupSession{deviceID: deviceID, conn: conn, stepTimeout: step}, nil + return &Session{deviceID: deviceID, conn: conn, stepTimeout: step}, nil } // Close sends a normal-closure frame and closes the underlying socket. -func (s *SetupSession) Close() error { +func (s *Session) Close() error { if s.conn == nil { return nil } @@ -144,7 +144,7 @@ func (s *SetupSession) Close() 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 { +func (s *Session) sendStep(ctx context.Context, route, method, body string) error { if s.conn == nil { return errors.New("setup session: connection closed") } @@ -199,13 +199,13 @@ func (s *SetupSession) sendStep(ctx context.Context, route, method, body string) } // Start sends SETUP_START. -func (s *SetupSession) Start(ctx context.Context) error { +func (s *Session) Start(ctx context.Context) error { return s.sendStep(ctx, "setup", "POST", ``) } // IdentifyEnter sends SETUP_IDENTIFY_DEVICE_ENTER. timeoutMs defaults to // the value observed in captures (300 000 ms). -func (s *SetupSession) IdentifyEnter(ctx context.Context, timeoutMs int) error { +func (s *Session) IdentifyEnter(ctx context.Context, timeoutMs int) error { if timeoutMs <= 0 { timeoutMs = 300000 } @@ -216,23 +216,23 @@ func (s *SetupSession) IdentifyEnter(ctx context.Context, timeoutMs int) error { } // SetLanguage POSTs sysLanguage. Code 2 = English. -func (s *SetupSession) SetLanguage(ctx context.Context, code int) error { +func (s *Session) SetLanguage(ctx context.Context, code int) error { body := fmt.Sprintf(`%d`, code) return s.sendStep(ctx, "language", "POST", body) } // Enter sends SETUP_ENTER. -func (s *SetupSession) Enter(ctx context.Context) error { +func (s *Session) Enter(ctx context.Context) error { return s.sendStep(ctx, "setup", "POST", ``) } // IdentifyLeave sends SETUP_IDENTIFY_DEVICE_LEAVE. -func (s *SetupSession) IdentifyLeave(ctx context.Context) error { +func (s *Session) IdentifyLeave(ctx context.Context) error { return s.sendStep(ctx, "setup", "POST", ``) } // SetName POSTs a device-name change. An empty name is a no-op. -func (s *SetupSession) SetName(ctx context.Context, name string) error { +func (s *Session) SetName(ctx context.Context, name string) error { if name == "" { return nil } @@ -246,7 +246,7 @@ func (s *SetupSession) SetName(ctx context.Context, name string) error { // authToken defaults to "Bearer aftertouch" when empty — our local // service does not validate it, but a non-empty value matches the // official app's shape. -func (s *SetupSession) SetMargeAccount(ctx context.Context, accountID, authToken string) error { +func (s *Session) SetMargeAccount(ctx context.Context, accountID, authToken string) error { if accountID == "" { return errors.New("SetMargeAccount: accountID is required") } @@ -264,13 +264,13 @@ func (s *SetupSession) SetMargeAccount(ctx context.Context, accountID, authToken } // Leave sends SETUP_LEAVE. -func (s *SetupSession) Leave(ctx context.Context) error { +func (s *Session) Leave(ctx context.Context) error { 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 { +func (s *Session) PushCustomerSupportInfo(ctx context.Context) error { return s.sendStep(ctx, "pushCustomerSupportInfoToMarge", "GET", "") } diff --git a/pkg/service/setup/setup_session_test.go b/pkg/service/setup/setup_session_test.go index 589041f..0b6dc4e 100644 --- a/pkg/service/setup/setup_session_test.go +++ b/pkg/service/setup/setup_session_test.go @@ -15,7 +15,7 @@ import ( ) // fakeSpeaker is a minimal WebSocket endpoint that records frames sent by -// SetupSession and responds with canned replies. Each test wires its own +// Session and responds with canned replies. Each test wires its own // reply policy by setting reply. type fakeSpeaker struct { server *httptest.Server @@ -74,7 +74,7 @@ func newFakeSpeaker(t *testing.T) *fakeSpeaker { } // ackFor builds a minimal echo reply that carries the same requestID as -// the incoming frame, so the SetupSession's correlation logic accepts it. +// the incoming frame, so the Session's correlation logic accepts it. func ackFor(frame string) string { id := extractAttr(frame, `requestID="`, `"`) return fmt.Sprintf(`
ok`, id) @@ -106,10 +106,10 @@ func (f *fakeSpeaker) recordedFrames() []string { return out } -// dialFakeSession opens a SetupSession against the fake speaker. We turn +// dialFakeSession opens a Session against the fake speaker. We turn // the httptest server URL inside-out (http → ws, keep host:port) so the // dialer reaches our handler. -func dialFakeSession(t *testing.T, f *fakeSpeaker, deviceID string) *SetupSession { +func dialFakeSession(t *testing.T, f *fakeSpeaker, deviceID string) *Session { t.Helper() u, err := url.Parse(f.server.URL) @@ -117,13 +117,13 @@ func dialFakeSession(t *testing.T, f *fakeSpeaker, deviceID string) *SetupSessio t.Fatalf("parse server URL: %v", err) } - s, err := DialSetupSession(u.Host, deviceID, SetupSessionConfig{ + s, err := DialSession(u.Host, deviceID, SessionConfig{ StepTimeout: 2 * time.Second, DialTimeout: 2 * time.Second, WSScheme: "ws", }) if err != nil { - t.Fatalf("DialSetupSession: %v", err) + t.Fatalf("DialSession: %v", err) } t.Cleanup(func() { _ = s.Close() }) @@ -131,7 +131,7 @@ func dialFakeSession(t *testing.T, f *fakeSpeaker, deviceID string) *SetupSessio return s } -func TestSetupSession_SendsCanonicalEnvelopes(t *testing.T) { +func TestSession_SendsCanonicalEnvelopes(t *testing.T) { f := newFakeSpeaker(t) s := dialFakeSession(t, f, "AABBCCDDEEFF") @@ -189,7 +189,7 @@ func TestSetupSession_SendsCanonicalEnvelopes(t *testing.T) { mustContain(t, frames[8], `url="pushCustomerSupportInfoToMarge"`, `method="GET"`) } -func TestSetupSession_RequestIDsAreUniquePerStep(t *testing.T) { +func TestSession_RequestIDsAreUniquePerStep(t *testing.T) { f := newFakeSpeaker(t) s := dialFakeSession(t, f, "X") ctx := context.Background() @@ -215,7 +215,7 @@ func TestSetupSession_RequestIDsAreUniquePerStep(t *testing.T) { } } -func TestSetupSession_IgnoresUpdatesFramesBeforeAck(t *testing.T) { +func TestSession_IgnoresUpdatesFramesBeforeAck(t *testing.T) { f := newFakeSpeaker(t) f.reply = func(frame string) []string { id := extractAttr(frame, `requestID="`, `"`) @@ -234,7 +234,7 @@ func TestSetupSession_IgnoresUpdatesFramesBeforeAck(t *testing.T) { } } -func TestSetupSession_SurfacesDeviceErrors(t *testing.T) { +func TestSession_SurfacesDeviceErrors(t *testing.T) { f := newFakeSpeaker(t) f.reply = func(frame string) []string { return []string{ @@ -254,14 +254,14 @@ func TestSetupSession_SurfacesDeviceErrors(t *testing.T) { } } -func TestSetupSession_RejectsEmptyDeviceID(t *testing.T) { - _, err := DialSetupSession("127.0.0.1:8080", "", SetupSessionConfig{}) +func TestSession_RejectsEmptyDeviceID(t *testing.T) { + _, err := DialSession("127.0.0.1:8080", "", SessionConfig{}) if err == nil { t.Fatal("expected error for empty deviceID") } } -func TestSetupSession_RejectsEmptyAccountID(t *testing.T) { +func TestSession_RejectsEmptyAccountID(t *testing.T) { f := newFakeSpeaker(t) s := dialFakeSession(t, f, "X") @@ -271,7 +271,7 @@ func TestSetupSession_RejectsEmptyAccountID(t *testing.T) { } } -func TestSetupSession_EmptyNameIsNoOp(t *testing.T) { +func TestSession_EmptyNameIsNoOp(t *testing.T) { f := newFakeSpeaker(t) s := dialFakeSession(t, f, "X") @@ -284,7 +284,7 @@ func TestSetupSession_EmptyNameIsNoOp(t *testing.T) { } } -func TestSetupSession_XMLAttributeEscape(t *testing.T) { +func TestSession_XMLAttributeEscape(t *testing.T) { // Device names with special characters must not break the envelope. f := newFakeSpeaker(t) s := dialFakeSession(t, f, `quoted"`)