From 6f8de353d4ea4aca644c8a61b6d2400877a6d99f Mon Sep 17 00:00:00 2001 From: Jake Thomas Date: Fri, 31 Jul 2026 06:23:06 -0400 Subject: [PATCH 1/2] chore: upgrade to Go 1.26 and modernize Move the go directive and CI to 1.26, then adopt the standard-library features the code predated. Go 1.26's only language change is new(expr), which has no site here: no pointer-helper generics exist, and the &cloned returns in event.go mutate the local first. errors.AsType likewise does not apply, since this module uses errors.Is exclusively. The reductions therefore come from slices, maps, cmp and the min/max builtins, which the code had not yet taken up. - slices/maps replace hand-rolled clone, contains, equal, sort and dedup loops - slices.Sorted(maps.Keys(...)) replaces collect-then-sort for deterministic map iteration - min/max replace clamp blocks; Duration.Abs replaces a manual negate - sync.WaitGroup.Go, range-over-int and strings.SplitSeq in tests - a generic asEvents helper collapses six copies of the same widen-to-interface loop in gesture and action - ControlOf extracts the stick and trigger mapping it repeated four times across its value and pointer cases Two idioms are deliberately kept. The append([]byte(nil), x...) clones in audit/recorder.go and merkle.go collapse empty to nil, which HMACKey depends on: it is tested with != nil to select the HMAC chain, and WithHMAC builds a non-nil empty key that is rejected later. And the omitempty tags on nested struct fields, though genuine no-ops, are hashed into the audit record chain, so changing them to omitzero would break verification of logs already on disk. Verified across darwin, linux and windows, with and without cgo, under the race detector. --- .github/workflows/ci.yml | 2 +- README.md | 2 +- action/action.go | 52 ++++++--------- audit/integrity_test.go | 2 +- audit/provenance.go | 17 +---- clock.go | 5 +- cmd/teleop-monitor/tui.go | 5 +- cmd/teleop-monitor/tui_test.go | 6 +- command.go | 3 +- control.go | 4 +- controller.go | 14 ++-- controller_remediation_test.go | 10 +-- device.go | 20 ++---- event.go | 71 ++++++++++---------- gesture/gesture.go | 114 +++++++++++---------------------- go.mod | 2 +- normalize.go | 8 +-- registry.go | 12 ++-- state.go | 12 ++-- testkit/fake.go | 3 +- tracker.go | 31 +++------ xbox/provider_darwin.go | 2 +- xbox/provider_darwin_test.go | 1 - xbox/provider_linux.go | 12 ++-- 24 files changed, 153 insertions(+), 257 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9951111..407598a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,7 +17,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: "1.25.x" + go-version: "1.26.x" - run: go test ./... - run: go test ./... env: diff --git a/README.md b/README.md index dc628b3..9b2882e 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ and Lip Gloss. ## Install -`teleop` currently requires Go 1.25 or newer. +`teleop` currently requires Go 1.26 or newer. ```sh go get github.com/open-ships/teleop diff --git a/action/action.go b/action/action.go index c03330f..08dec4d 100644 --- a/action/action.go +++ b/action/action.go @@ -5,7 +5,7 @@ package action import ( "context" "fmt" - "sort" + "slices" "sync" "sync/atomic" @@ -45,7 +45,7 @@ func (Event) Kind() teleop.EventKind { return EventKind } // CloneEvent implements teleop.EventCloner. func (e Event) CloneEvent() teleop.Event { e.Meta = e.Meta.Clone() - e.Controls = append([]teleop.ControlID(nil), e.Controls...) + e.Controls = slices.Clone(e.Controls) if e.Value.Vector != nil { vector := *e.Value.Vector e.Value.Vector = &vector @@ -148,7 +148,7 @@ type Mapper struct { // New validates and copies bindings. Invalid bindings panic instead of silently // matching a different control. func New(bindings ...Binding) *Mapper { - copied := append([]Binding(nil), bindings...) + copied := slices.Clone(bindings) for index := range copied { copied[index].Controls = canonicalControls(copied[index].Controls) if err := validateBinding(copied[index]); err != nil { @@ -203,11 +203,7 @@ func validateBinding(binding Binding) error { // Process implements teleop.Processor. func (m *Mapper) Process(input teleop.Event) []teleop.Event { mapped := m.Map(input) - result := make([]teleop.Event, len(mapped)) - for index := range mapped { - result[index] = mapped[index] - } - return result + return asEvents(mapped) } // ProcessContext implements teleop.ContextProcessor. @@ -217,11 +213,7 @@ func (m *Mapper) ProcessContext( input teleop.Event, ) ([]teleop.Event, error) { mapped := m.mapInput(input, processing) - result := make([]teleop.Event, len(mapped)) - for index := range mapped { - result[index] = mapped[index] - } - return result, nil + return asEvents(mapped), nil } // Map returns strongly typed application actions using an instance-unique @@ -285,7 +277,7 @@ func (m *Mapper) mapInput( Action: binding.Action, Phase: values.phase, Control: values.control, - Controls: append([]teleop.ControlID(nil), values.controls...), + Controls: slices.Clone(values.controls), Value: values.value, }) } @@ -387,28 +379,20 @@ func triggerControl(trigger teleop.TriggerID) (teleop.ControlID, bool) { } } -func canonicalControls(values []teleop.ControlID) []teleop.ControlID { - result := append([]teleop.ControlID(nil), values...) - sort.Slice(result, func(i, j int) bool { return result[i] < result[j] }) - write := 0 - for _, value := range result { - if value == "" || write > 0 && result[write-1] == value { - continue - } - result[write] = value - write++ +// asEvents widens mapped actions to the open teleop.Event interface. +func asEvents[T teleop.Event](values []T) []teleop.Event { + result := make([]teleop.Event, len(values)) + for index, value := range values { + result[index] = value } - return result[:write] + return result +} + +func canonicalControls(values []teleop.ControlID) []teleop.ControlID { + sorted := slices.Compact(slices.Sorted(slices.Values(values))) + return slices.DeleteFunc(sorted, func(value teleop.ControlID) bool { return value == "" }) } func equalControls(left, right []teleop.ControlID) bool { - if len(left) != len(right) { - return false - } - for index := range left { - if left[index] != right[index] { - return false - } - } - return true + return slices.Equal(left, right) } diff --git a/audit/integrity_test.go b/audit/integrity_test.go index 871dede..9cc38c7 100644 --- a/audit/integrity_test.go +++ b/audit/integrity_test.go @@ -863,7 +863,7 @@ func TestManifestBindsSession(t *testing.T) { func splitLines(t *testing.T, raw []byte) [][]byte { t.Helper() var lines [][]byte - for _, line := range strings.Split(strings.TrimRight(string(raw), "\n"), "\n") { + for line := range strings.SplitSeq(strings.TrimRight(string(raw), "\n"), "\n") { lines = append(lines, []byte(line)) } return lines diff --git a/audit/provenance.go b/audit/provenance.go index 8e0ba6b..0f2cf35 100644 --- a/audit/provenance.go +++ b/audit/provenance.go @@ -1,6 +1,7 @@ package audit import ( + "maps" "os" "runtime" "runtime/debug" @@ -88,19 +89,7 @@ func CaptureProvenance() Provenance { // Clone returns a deep copy so a recorder cannot observe later mutation of a // caller's maps. func (p Provenance) Clone() Provenance { - if p.Config != nil { - config := make(map[string]any, len(p.Config)) - for key, value := range p.Config { - config[key] = value - } - p.Config = config - } - if p.Platform != nil { - platform := make(map[string]string, len(p.Platform)) - for key, value := range p.Platform { - platform[key] = value - } - p.Platform = platform - } + p.Config = maps.Clone(p.Config) + p.Platform = maps.Clone(p.Platform) return p } diff --git a/clock.go b/clock.go index 076962d..9aa97bf 100644 --- a/clock.go +++ b/clock.go @@ -65,10 +65,7 @@ func (m *clockMonitor) observe(now time.Time) clockSample { wallDelta := now.Round(0).Sub(m.lastWall.Round(0)) monoDelta := monotonic - m.lastMono divergence := wallDelta - monoDelta - magnitude := divergence - if magnitude < 0 { - magnitude = -magnitude - } + magnitude := divergence.Abs() if magnitude >= m.threshold { m.steps++ sample.Step = divergence diff --git a/cmd/teleop-monitor/tui.go b/cmd/teleop-monitor/tui.go index 23ac36b..f6276b3 100644 --- a/cmd/teleop-monitor/tui.go +++ b/cmd/teleop-monitor/tui.go @@ -922,10 +922,7 @@ func keyHint(key, action string) string { } func padBetween(left, right string, width int) string { - spaces := width - lipgloss.Width(left) - lipgloss.Width(right) - if spaces < 1 { - spaces = 1 - } + spaces := max(width-lipgloss.Width(left)-lipgloss.Width(right), 1) return left + strings.Repeat(" ", spaces) + right } diff --git a/cmd/teleop-monitor/tui_test.go b/cmd/teleop-monitor/tui_test.go index 9bd7149..58d8a56 100644 --- a/cmd/teleop-monitor/tui_test.go +++ b/cmd/teleop-monitor/tui_test.go @@ -354,7 +354,7 @@ func TestMonitorModelTogglesRumble(t *testing.T) { if repeated != nil { t.Fatal("repeated rumble key started a second request") } - for _, line := range strings.Split(model.renderHaptics(30), "\n") { + for line := range strings.SplitSeq(model.renderHaptics(30), "\n") { if lipgloss.Width(line) > 30 { t.Fatalf( "compact haptic line is %d cells wide, want at most 30", @@ -523,7 +523,7 @@ func TestMonitorModelAlternatesRumbleAtConfiguredInterval(t *testing.T) { t.Fatalf("alternating haptic controls do not contain %q", expected) } } - for _, line := range strings.Split(model.renderHaptics(30), "\n") { + for line := range strings.SplitSeq(model.renderHaptics(30), "\n") { if lipgloss.Width(line) > 30 { t.Fatalf( "compact alternating haptic line is %d cells wide, want at most 30", @@ -756,7 +756,7 @@ func TestMonitorModelReportsUnavailableAndFailedRumble(t *testing.T) { t.Fatalf("failed rumble view does not contain %q", expected) } } - for _, line := range strings.Split(model.renderHaptics(30), "\n") { + for line := range strings.SplitSeq(model.renderHaptics(30), "\n") { if lipgloss.Width(line) > 30 { t.Fatalf( "compact haptic error line is %d cells wide, want at most 30", diff --git a/command.go b/command.go index b255110..1ce7b43 100644 --- a/command.go +++ b/command.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "slices" "time" ) @@ -69,7 +70,7 @@ func (c *Controller) RecordCommand(ctx context.Context, command Command) error { payload: payload, issuedAt: c.clock.Now(), } - request.command.Causes = append([]EventID(nil), command.Causes...) + request.command.Causes = slices.Clone(command.Causes) request.command.Payload = nil // A caller may use a deferred controller solely for audit-backed command diff --git a/control.go b/control.go index ccdda72..9ed8c8b 100644 --- a/control.go +++ b/control.go @@ -1,5 +1,7 @@ package teleop +import "slices" + // ControllerType identifies a family of game controllers. It is string-backed // so third-party providers can add controller types without changing teleop. type ControllerType string @@ -89,7 +91,7 @@ var standardButtons = []ControlID{ // StandardButtonIDs returns the standard physical buttons in a stable order. func StandardButtonIDs() []ControlID { - return append([]ControlID(nil), standardButtons...) + return slices.Clone(standardButtons) } // StickID selects one of the two standard analog sticks. diff --git a/controller.go b/controller.go index 4d514df..f09eedd 100644 --- a/controller.go +++ b/controller.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "math" + "slices" "sync" "time" ) @@ -522,10 +523,7 @@ func (c *Controller) onTick(now time.Time) error { if meta.ReceivedAt.IsZero() { meta.ReceivedAt = c.startedAt } - age := now.Sub(meta.ReceivedAt) - if age < 0 { - age = 0 - } + age := max(now.Sub(meta.ReceivedAt), 0) stale := c.options.staleAfter > 0 && age >= c.options.staleAfter if stale && !meta.Stale && c.options.neutralizeOnStale { if err := c.neutralize(now, "input stale"); err != nil { @@ -755,7 +753,7 @@ func (c *Controller) nextHeaderAt( Monotonic: c.clocks.since(publishedAt), ReceivedMonotonic: c.clocks.since(receivedAt), DeviceTimestamp: deviceTimestamp, - Causes: append([]EventID(nil), causes...), + Causes: slices.Clone(causes), Synthetic: synthetic, } } @@ -826,7 +824,7 @@ func (c *Controller) publishTerminal(event Event) { if c.processorDisabled[index] { continue } - stageInputs := append([]Event(nil), visible...) + stageInputs := slices.Clone(visible) var derived []Event failed := false for _, input := range stageInputs { @@ -865,13 +863,13 @@ func (c *Controller) dispatchTerminal(event Event) (completed bool) { } func (c *Controller) processStages(inputs []Event, start int) error { - visible := append([]Event(nil), inputs...) + visible := slices.Clone(inputs) for index := start; index < len(c.options.processors); index++ { if c.processorDisabled[index] { continue } processor := c.options.processors[index] - stageInputs := append([]Event(nil), visible...) + stageInputs := slices.Clone(visible) derived := make([]Event, 0, len(stageInputs)) for _, input := range stageInputs { output, err := c.callProcessor(processor, input) diff --git a/controller_remediation_test.go b/controller_remediation_test.go index 28dcfe6..319586a 100644 --- a/controller_remediation_test.go +++ b/controller_remediation_test.go @@ -240,7 +240,7 @@ func TestSubscriptionCloseBroadcastsToAllBlockedReaders(t *testing.T) { } ctx, cancel := context.WithTimeout(context.Background(), time.Second) defer cancel() - for index := 0; index < 2; index++ { + for range 2 { if _, err := subscription.Next(ctx); err != nil { t.Fatal(err) } @@ -248,21 +248,21 @@ func TestSubscriptionCloseBroadcastsToAllBlockedReaders(t *testing.T) { started := make(chan struct{}, 4) finished := make(chan error, 4) - for index := 0; index < 4; index++ { + for range 4 { go func() { started <- struct{}{} _, err := subscription.Next(context.Background()) finished <- err }() } - for index := 0; index < 4; index++ { + for range 4 { <-started } time.Sleep(10 * time.Millisecond) if err := subscription.Close(); err != nil { t.Fatal(err) } - for index := 0; index < 4; index++ { + for index := range 4 { select { case err := <-finished: if !errors.Is(err, teleop.ErrClosed) { @@ -642,7 +642,7 @@ func TestSlowSinkDoesNotDelayCanonicalSnapshot(t *testing.T) { waitForSnapshot(t, controller, func(_ teleop.State, meta teleop.StateMeta) bool { return meta.Connected }) - for index := 0; index < 8; index++ { + for index := range 8 { if err := source.Push(context.Background(), teleop.State{ LeftTrigger: float32(index) / 7, }); err != nil { diff --git a/device.go b/device.go index f517e92..3588132 100644 --- a/device.go +++ b/device.go @@ -2,6 +2,8 @@ package teleop import ( "context" + "maps" + "slices" "time" ) @@ -69,18 +71,15 @@ func (c Capabilities) Clone() Capabilities { if !c.AuditGrade.Valid() { c.AuditGrade = AuditUnavailable } - c.Controls = append([]ControlDescriptor(nil), c.Controls...) + c.Controls = slices.Clone(c.Controls) return c } // Supports reports whether id appears in the discovered control list. func (c Capabilities) Supports(id ControlID) bool { - for _, control := range c.Controls { - if control.ID == id { - return true - } - } - return false + return slices.ContainsFunc(c.Controls, func(control ControlDescriptor) bool { + return control.ID == id + }) } // Descriptor contains the stable metadata and capabilities of one discovered @@ -100,12 +99,7 @@ type Descriptor struct { // Clone returns an isolated copy of the device descriptor. func (d Descriptor) Clone() Descriptor { d.Capability = d.Capability.Clone() - if d.Properties != nil { - d.Properties = make(map[string]string, len(d.Properties)) - for key, value := range d.Properties { - d.Properties[key] = value - } - } + d.Properties = maps.Clone(d.Properties) return d } diff --git a/event.go b/event.go index 66a3d40..745ec3d 100644 --- a/event.go +++ b/event.go @@ -4,6 +4,8 @@ import ( "encoding/hex" "encoding/json" "fmt" + "maps" + "slices" "time" ) @@ -93,7 +95,7 @@ type Header struct { // Clone returns an isolated copy of the header. func (h Header) Clone() Header { - h.Causes = append([]EventID(nil), h.Causes...) + h.Causes = slices.Clone(h.Causes) return h } @@ -118,13 +120,8 @@ type NativeInput struct { } func (n NativeInput) clone() NativeInput { - n.Data = append([]byte(nil), n.Data...) - if n.Fields != nil { - n.Fields = make(map[string]int64, len(n.Fields)) - for key, value := range n.Fields { - n.Fields[key] = value - } - } + n.Data = slices.Clone(n.Data) + n.Fields = maps.Clone(n.Fields) return n } @@ -418,6 +415,10 @@ func cloneEvent(event Event) Event { // ControlOf returns the physical control associated with a standard input // event. Not every event has one. +// +// Each case appears in both its value and pointer form because an event may be +// published either way; field access auto-dereferences, so the two arms of a +// pair are the same expression. func ControlOf(event Event) (ControlID, bool) { switch value := event.(type) { case ButtonEvent: @@ -425,37 +426,37 @@ func ControlOf(event Event) (ControlID, bool) { case *ButtonEvent: return value.Button, true case StickEvent: - if value.Stick == LeftStick { - return StickLeft, true - } - if value.Stick == RightStick { - return StickRight, true - } - return "", false + return stickControl(value.Stick) case *StickEvent: - if value.Stick == LeftStick { - return StickLeft, true - } - if value.Stick == RightStick { - return StickRight, true - } - return "", false + return stickControl(value.Stick) case TriggerEvent: - if value.Trigger == LeftTrigger { - return TriggerLeft, true - } - if value.Trigger == RightTrigger { - return TriggerRight, true - } - return "", false + return triggerControl(value.Trigger) case *TriggerEvent: - if value.Trigger == LeftTrigger { - return TriggerLeft, true - } - if value.Trigger == RightTrigger { - return TriggerRight, true - } + return triggerControl(value.Trigger) + default: return "", false + } +} + +// stickControl maps a stick to its control identity. +func stickControl(stick StickID) (ControlID, bool) { + switch stick { + case LeftStick: + return StickLeft, true + case RightStick: + return StickRight, true + default: + return "", false + } +} + +// triggerControl maps a trigger to its control identity. +func triggerControl(trigger TriggerID) (ControlID, bool) { + switch trigger { + case LeftTrigger: + return TriggerLeft, true + case RightTrigger: + return TriggerRight, true default: return "", false } diff --git a/gesture/gesture.go b/gesture/gesture.go index a2be50e..f21311c 100644 --- a/gesture/gesture.go +++ b/gesture/gesture.go @@ -3,10 +3,13 @@ package gesture import ( + "bytes" + "cmp" "context" "fmt" + "maps" "math" - "sort" + "slices" "sync" "sync/atomic" "time" @@ -55,7 +58,7 @@ func (Event) Kind() teleop.EventKind { return EventKind } // CloneEvent implements teleop.EventCloner. func (e Event) CloneEvent() teleop.Event { e.Meta = e.Meta.Clone() - e.Controls = append([]teleop.ControlID(nil), e.Controls...) + e.Controls = slices.Clone(e.Controls) return e } @@ -155,7 +158,7 @@ func New(config Config) *Recognizer { if config.TriggerHysteresis == 0 { config.TriggerHysteresis = defaults.TriggerHysteresis } - config.Chords = append([]ChordSpec(nil), config.Chords...) + config.Chords = slices.Clone(config.Chords) for index := range config.Chords { config.Chords[index].Buttons = canonicalControls(config.Chords[index].Buttons) } @@ -211,11 +214,7 @@ func validateConfig(config Config) error { // Process implements teleop.Processor. func (r *Recognizer) Process(input teleop.Event) []teleop.Event { recognized := r.Recognize(input) - result := make([]teleop.Event, len(recognized)) - for index := range recognized { - result[index] = recognized[index] - } - return result + return asEvents(recognized) } // ProcessContext implements teleop.ContextProcessor. @@ -229,21 +228,13 @@ func (r *Recognizer) ProcessContext( r.processing = processing defer func() { r.processing = nil }() recognized := r.recognizeLocked(input) - result := make([]teleop.Event, len(recognized)) - for index := range recognized { - result[index] = recognized[index] - } - return result, nil + return asEvents(recognized), nil } // Advance implements teleop.AdvancingProcessor. func (r *Recognizer) Advance(now time.Time) []teleop.Event { recognized := r.AdvanceGestures(now) - result := make([]teleop.Event, len(recognized)) - for index := range recognized { - result[index] = recognized[index] - } - return result + return asEvents(recognized) } // AdvanceContext implements teleop.ContextAdvancingProcessor. @@ -257,11 +248,7 @@ func (r *Recognizer) AdvanceContext( r.processing = processing defer func() { r.processing = nil }() recognized := r.advanceLocked(now) - result := make([]teleop.Event, len(recognized)) - for index := range recognized { - result[index] = recognized[index] - } - return result, nil + return asEvents(recognized), nil } // Recognize consumes one canonical input event. @@ -309,14 +296,11 @@ func (r *Recognizer) AdvanceGestures(now time.Time) []Event { } func (r *Recognizer) advanceLocked(now time.Time) []Event { - keys := make([]streamKey, 0, len(r.states)) - for key := range r.states { - keys = append(keys, key) - } - sort.Slice(keys, func(i, j int) bool { - left := keys[i].session.String() + string(keys[i].device) - right := keys[j].session.String() + string(keys[j].device) - return left < right + keys := slices.SortedFunc(maps.Keys(r.states), func(left, right streamKey) int { + return cmp.Or( + bytes.Compare(left.session[:], right.session[:]), + cmp.Compare(left.device, right.device), + ) }) var result []Event for _, key := range keys { @@ -325,7 +309,7 @@ func (r *Recognizer) advanceLocked(now time.Time) []Event { buttons := sortedPressed(state.pressed) for _, button := range buttons { pressState := state.pressed[button] - duration := nonNegative(now.Sub(pressState.at)) + duration := max(now.Sub(pressState.at), 0) if pressState.consumed || pressState.holdSent || duration < r.config.HoldMinimum { continue } @@ -480,7 +464,7 @@ func (r *Recognizer) startedChords( event.Meta.ObservedAt, Chord, teleop.PhaseStarted, - append([]teleop.ControlID(nil), chord.Buttons...), + slices.Clone(chord.Buttons), 0, chord.Name, 0, @@ -514,7 +498,7 @@ func (r *Recognizer) endedChords( event.Meta.ObservedAt, Chord, teleop.PhaseEnded, - append([]teleop.ControlID(nil), chord.Buttons...), + slices.Clone(chord.Buttons), 0, chord.Name, 0, @@ -594,7 +578,7 @@ func (r *Recognizer) reset( } ended := r.event( cause, cause.ObservedAt, Chord, teleop.PhaseEnded, - append([]teleop.ControlID(nil), chord.Buttons...), 0, chord.Name, 0, + slices.Clone(chord.Buttons), 0, chord.Name, 0, ) ended.Meta.Causes = []teleop.EventID{startedID, cause.ID} result = append(result, ended) @@ -604,7 +588,7 @@ func (r *Recognizer) reset( if value.consumed || !value.holdSent { continue } - duration := nonNegative(cause.ObservedAt.Sub(value.at)) + duration := max(cause.ObservedAt.Sub(value.at), 0) ended := r.event( cause, cause.ObservedAt, Hold, teleop.PhaseEnded, []teleop.ControlID{button}, duration, "", 0, @@ -618,7 +602,7 @@ func (r *Recognizer) reset( sticks = append(sticks, string(stick)) } } - sort.Strings(sticks) + slices.Sort(sticks) for _, raw := range sticks { stick := teleop.StickID(raw) control, ok := stickControl(stick) @@ -635,7 +619,7 @@ func (r *Recognizer) reset( triggers = append(triggers, string(trigger)) } } - sort.Strings(triggers) + slices.Sort(triggers) for _, raw := range triggers { trigger := teleop.TriggerID(raw) control, ok := triggerControl(trigger) @@ -690,7 +674,7 @@ func (r *Recognizer) event( Meta: header, Type: kind, Phase: phase, - Controls: append([]teleop.ControlID(nil), controls...), + Controls: slices.Clone(controls), Duration: duration, Region: region, Value: value, @@ -723,47 +707,32 @@ func (r *Recognizer) expireTaps(state *streamState, now time.Time) { } } -func sortedPressed(values map[teleop.ControlID]press) []teleop.ControlID { - result := make([]teleop.ControlID, 0, len(values)) - for value := range values { - result = append(result, value) +// asEvents widens recognized gestures to the open teleop.Event interface. +func asEvents[T teleop.Event](values []T) []teleop.Event { + result := make([]teleop.Event, len(values)) + for index, value := range values { + result[index] = value } - sort.Slice(result, func(i, j int) bool { return result[i] < result[j] }) return result } +func sortedPressed(values map[teleop.ControlID]press) []teleop.ControlID { + return slices.Sorted(maps.Keys(values)) +} + func canonicalControls(values []teleop.ControlID) []teleop.ControlID { - seen := make(map[teleop.ControlID]struct{}, len(values)) - result := make([]teleop.ControlID, 0, len(values)) - for _, value := range values { - if value == "" { - continue - } - if _, ok := seen[value]; ok { - continue - } - seen[value] = struct{}{} - result = append(result, value) - } - sort.Slice(result, func(i, j int) bool { return result[i] < result[j] }) - return result + sorted := slices.Compact(slices.Sorted(slices.Values(values))) + return slices.DeleteFunc(sorted, func(value teleop.ControlID) bool { return value == "" }) } func equalControls(left, right []teleop.ControlID) bool { - if len(left) != len(right) { - return false - } - for index := range left { - if left[index] != right[index] { - return false - } - } - return true + return slices.Equal(left, right) } +// contains reports membership in a canonical (sorted, deduplicated) control list. func contains(values []teleop.ControlID, value teleop.ControlID) bool { - index := sort.Search(len(values), func(index int) bool { return values[index] >= value }) - return index < len(values) && values[index] == value + _, found := slices.BinarySearch(values, value) + return found } func appendChordCauses( @@ -806,13 +775,6 @@ func triggerControl(trigger teleop.TriggerID) (teleop.ControlID, bool) { } } -func nonNegative(duration time.Duration) time.Duration { - if duration < 0 { - return 0 - } - return duration -} - func stickRegion(stick teleop.Stick, threshold float32) string { magnitude := math.Hypot(float64(stick.X), float64(stick.Y)) if magnitude < float64(threshold) { diff --git a/go.mod b/go.mod index b531ce8..a585444 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/open-ships/teleop -go 1.25.0 +go 1.26.0 require ( charm.land/bubbletea/v2 v2.0.8 diff --git a/normalize.go b/normalize.go index 7c389cf..89bcc95 100644 --- a/normalize.go +++ b/normalize.go @@ -10,13 +10,7 @@ func Clamp(value, minimum, maximum float32) float32 { } return minimum } - if value < minimum { - return minimum - } - if value > maximum { - return maximum - } - return value + return min(max(value, minimum), maximum) } // NormalizeAxis converts an integer absolute axis into [-1,+1]. diff --git a/registry.go b/registry.go index 6fd5f36..e54e78b 100644 --- a/registry.go +++ b/registry.go @@ -4,7 +4,8 @@ import ( "context" "errors" "fmt" - "sort" + "maps" + "slices" "sync" ) @@ -43,13 +44,8 @@ func (r *Registry) Register(provider Provider) { // provider-specific errors. func (r *Registry) Discover(ctx context.Context) ([]Descriptor, error) { r.mu.RLock() - types := make([]ControllerType, 0, len(r.providers)) - for controllerType := range r.providers { - types = append(types, controllerType) - } - sort.Slice(types, func(i, j int) bool { return types[i] < types[j] }) - providers := make([]Provider, 0, len(types)) - for _, controllerType := range types { + providers := make([]Provider, 0, len(r.providers)) + for _, controllerType := range slices.Sorted(maps.Keys(r.providers)) { providers = append(providers, r.providers[controllerType]) } r.mu.RUnlock() diff --git a/state.go b/state.go index 9f53bd4..9b9fc82 100644 --- a/state.go +++ b/state.go @@ -1,5 +1,7 @@ package teleop +import "maps" + // Stick is a normalized two-dimensional stick position. Both axes are in // [-1,+1]. Positive X is right and positive Y is up. type Stick struct { @@ -130,14 +132,8 @@ func (b *Buttons) Set(id ControlID, pressed bool) { } func (b Buttons) clone() Buttons { - result := b - if b.Extensions != nil { - result.Extensions = make(map[ControlID]bool, len(b.Extensions)) - for id, pressed := range b.Extensions { - result.Extensions[id] = pressed - } - } - return result + b.Extensions = maps.Clone(b.Extensions) + return b } // State is a canonical, transport-independent controller snapshot. diff --git a/testkit/fake.go b/testkit/fake.go index c26a9e1..82b569a 100644 --- a/testkit/fake.go +++ b/testkit/fake.go @@ -5,6 +5,7 @@ package testkit import ( "context" "io" + "slices" "sync" "time" @@ -143,7 +144,7 @@ type ReplaySource struct { func NewReplaySource(descriptor teleop.Descriptor, observations []teleop.Observation) *ReplaySource { return &ReplaySource{ descriptor: descriptor.Clone(), - observations: append([]teleop.Observation(nil), observations...), + observations: slices.Clone(observations), } } diff --git a/tracker.go b/tracker.go index 02bc525..c7c2402 100644 --- a/tracker.go +++ b/tracker.go @@ -1,6 +1,6 @@ package teleop -import "sort" +import "slices" func diffEvents(previous, current State, header func() Header) []Event { var events []Event @@ -26,24 +26,16 @@ func diffEvents(previous, current State, header func() Header) []Event { } var extensions []ControlID - for id := range previous.Buttons.Extensions { - if !isStandardButton(id) { - extensions = append(extensions, id) + for _, set := range []map[ControlID]bool{previous.Buttons.Extensions, current.Buttons.Extensions} { + for id := range set { + if !isStandardButton(id) { + extensions = append(extensions, id) + } } } - for id := range current.Buttons.Extensions { - if !isStandardButton(id) { - extensions = append(extensions, id) - } - } - sort.Slice(extensions, func(i, j int) bool { return extensions[i] < extensions[j] }) - var previousID ControlID - for index, id := range extensions { - if index > 0 && id == previousID { - continue - } + slices.Sort(extensions) + for _, id := range slices.Compact(extensions) { appendButton(id) - previousID = id } if previous.LeftStick != current.LeftStick { @@ -88,10 +80,5 @@ func diffEvents(previous, current State, header func() Header) []Event { } func isStandardButton(id ControlID) bool { - for _, standard := range standardButtons { - if id == standard { - return true - } - } - return false + return slices.Contains(standardButtons, id) } diff --git a/xbox/provider_darwin.go b/xbox/provider_darwin.go index dfb8681..32ce55f 100644 --- a/xbox/provider_darwin.go +++ b/xbox/provider_darwin.go @@ -27,7 +27,7 @@ import ( func discoverPlatform(ctx context.Context) ([]teleop.Descriptor, error) { count := int(C.teleop_gc_count()) var devices []teleop.Descriptor - for index := 0; index < count; index++ { + for index := range count { if err := ctx.Err(); err != nil { return nil, err } diff --git a/xbox/provider_darwin_test.go b/xbox/provider_darwin_test.go index 3b41346..29c90b6 100644 --- a/xbox/provider_darwin_test.go +++ b/xbox/provider_darwin_test.go @@ -104,7 +104,6 @@ func TestIsXboxIdentity(t *testing.T) { }, } for _, test := range tests { - test := test t.Run(test.name, func(t *testing.T) { t.Parallel() if got := isXboxIdentity(test.vendorName, test.productCategory); got != test.want { diff --git a/xbox/provider_linux.go b/xbox/provider_linux.go index f1464b0..e74da05 100644 --- a/xbox/provider_linux.go +++ b/xbox/provider_linux.go @@ -11,7 +11,7 @@ import ( "io" "os" "path/filepath" - "sort" + "slices" "strings" "sync" "syscall" @@ -178,7 +178,7 @@ func linuxInputPaths() ([]string, error) { seen[realPath] = true paths = append(paths, path) } - sort.Strings(paths) + slices.Sort(paths) return paths, nil } @@ -349,7 +349,7 @@ func (s *linuxSource) Read(ctx context.Context) (teleop.Observation, error) { if err := s.resync(); err != nil { return teleop.Observation{}, s.normalizeReadError(err) } - raw := append([]byte(nil), s.raw.Bytes()...) + raw := slices.Clone(s.raw.Bytes()) s.raw.Reset() s.dropped = false return teleop.Observation{ @@ -371,7 +371,7 @@ func (s *linuxSource) Read(ctx context.Context) (teleop.Observation, error) { s.apply(event) if event.Type == evSyn && event.Code == synReport { - raw := append([]byte(nil), s.raw.Bytes()...) + raw := slices.Clone(s.raw.Bytes()) s.raw.Reset() return teleop.Observation{ State: s.state.Clone(), @@ -693,9 +693,7 @@ func (s *linuxSource) waitReadable(ctx context.Context) error { if remaining <= 0 { return ctx.Err() } - if remaining < timeout { - timeout = remaining - } + timeout = min(timeout, remaining) } timeoutMillis := int((timeout + time.Millisecond - 1) / time.Millisecond) pollFDs := []unix.PollFd{{ From b5db09855628a2f14efeb43077ea73a7e5513c10 Mon Sep 17 00:00:00 2001 From: Jake Thomas Date: Fri, 31 Jul 2026 06:23:41 -0400 Subject: [PATCH 2/2] refactor: model the safety lifecycle as a state machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Guard tracked the operator's standing intent in three booleans — armed, latched and estop — mutated by hand in five places. The invariants between them were implicit: an emergency stop had to also force armed to false, and every assignment restated that by convention rather than by construction. Replace them with a compiled transition table from github.com/open-ships/statemachine. The lifecycle is now one field with three states (idle, armed, stopped) moved only by five commands: arm, disarm, trip, stop and reset. Arm's two refusals — no bound controller, and a latched emergency stop that only Reset may clear — become Guards on table rows rather than conditionals buried in the method. Mapping the booleans onto the table showed that latched was written in four places and read in none, so it is removed. The library holds no state, so the current lifecycle stays on the Guard under the Guard's own mutex, which already protects everything a transition touches. Arm still reports ErrUnbound and its own emergency-stop error rather than the machine's refusal. Fire wraps statemachine.ErrNotPermitted, and letting that sentinel into this package's public error contract would break callers comparing with ==. Guard.Evaluate's published State (safe, armed, live) is unchanged: it is derived per call from live conditions, which is a separate concept from the lifecycle this table models. Behavior was checked against a verbatim transcription of the replaced boolean logic over every command sequence up to depth six, bound and unbound, comparing Arm's error and both predicates Evaluate reads. --- go.mod | 1 + go.sum | 2 + safety/event.go | 10 ++-- safety/guard.go | 77 ++++++++++++---------------- safety/guard_test.go | 6 +-- safety/lifecycle.go | 116 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 156 insertions(+), 56 deletions(-) create mode 100644 safety/lifecycle.go diff --git a/go.mod b/go.mod index a585444..8451989 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.26.0 require ( charm.land/bubbletea/v2 v2.0.8 charm.land/lipgloss/v2 v2.0.5 + github.com/open-ships/statemachine v1.0.0 golang.org/x/sys v0.47.0 golang.org/x/term v0.45.0 ) diff --git a/go.sum b/go.sum index ebabde3..78ab5f2 100644 --- a/go.sum +++ b/go.sum @@ -28,6 +28,8 @@ github.com/mattn/go-runewidth v0.0.23 h1:7ykA0T0jkPpzSvMS5i9uoNn2Xy3R383f9HDx3Ry github.com/mattn/go-runewidth v0.0.23/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs= github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA= github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo= +github.com/open-ships/statemachine v1.0.0 h1:F1baKU3/Rf3iA2lGb+qmdp2Km50MfY/Z/E2pl0Oowlk= +github.com/open-ships/statemachine v1.0.0/go.mod h1:xvscz8a/Imnv8LzI9sJ1hc/Gw5RsszhllQxVeiCJRuk= github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= diff --git a/safety/event.go b/safety/event.go index a025c0e..cfa75bf 100644 --- a/safety/event.go +++ b/safety/event.go @@ -1,6 +1,7 @@ package safety import ( + "slices" "time" "github.com/open-ships/teleop" @@ -91,7 +92,7 @@ func (Event) Kind() teleop.EventKind { return EventDecision } // CloneEvent implements teleop.EventCloner. func (e Event) CloneEvent() teleop.Event { e.Meta = e.Meta.Clone() - e.Reasons = append([]Reason(nil), e.Reasons...) + e.Reasons = slices.Clone(e.Reasons) return e } @@ -116,10 +117,5 @@ type Decision struct { // Has reports whether the decision carries a specific reason. func (d Decision) Has(reason Reason) bool { - for _, candidate := range d.Reasons { - if candidate == reason { - return true - } - } - return false + return slices.Contains(d.Reasons, reason) } diff --git a/safety/guard.go b/safety/guard.go index d8f9471..112249a 100644 --- a/safety/guard.go +++ b/safety/guard.go @@ -37,6 +37,7 @@ package safety import ( "context" "errors" + "slices" "sync" "time" @@ -134,9 +135,8 @@ type Guard struct { mu sync.Mutex source Source - armed bool - latched bool - estop bool + // lifecycle is the operator's standing intent, moved only through gate. + lifecycle lifecycle // heartbeat and deadManSince are monotonic readings from the bound // controller's session clock. Zero is a legitimate reading at the start of @@ -170,9 +170,10 @@ func New(opts ...Option) *Guard { } } return &Guard{ - options: configured, - state: StateSafe, - reasons: []Reason{ReasonUnbound}, + options: configured, + lifecycle: lifecycleIdle, + state: StateSafe, + reasons: []Reason{ReasonUnbound}, } } @@ -190,14 +191,17 @@ func (g *Guard) Bind(source Source) { func (g *Guard) Arm() error { g.mu.Lock() defer g.mu.Unlock() - if g.source == nil { - return ErrUnbound - } - if g.estop { - return errors.New("teleop/safety: cannot arm during an emergency stop") + next, err := gate.Fire(context.Background(), g.lifecycle, commandArm, g) + if err != nil { + // Report this package's own errors rather than the machine's refusal, + // so a caller comparing against ErrUnbound keeps working and safety's + // error contract stays independent of the machine. + if errors.Is(err, ErrUnbound) { + return ErrUnbound + } + return errArmDuringStop } - g.armed = true - g.latched = false + g.lifecycle = next g.detail = "" return nil } @@ -206,8 +210,7 @@ func (g *Guard) Arm() error { func (g *Guard) Disarm(detail string) { g.mu.Lock() defer g.mu.Unlock() - g.armed = false - g.latched = true + g.fire(commandDisarm) g.detail = detail } @@ -215,9 +218,7 @@ func (g *Guard) Disarm(detail string) { func (g *Guard) EmergencyStop(detail string) { g.mu.Lock() defer g.mu.Unlock() - g.estop = true - g.armed = false - g.latched = true + g.fire(commandStop) g.detail = detail } @@ -225,7 +226,7 @@ func (g *Guard) EmergencyStop(detail string) { func (g *Guard) Reset(detail string) { g.mu.Lock() defer g.mu.Unlock() - g.estop = false + g.fire(commandReset) g.detail = detail } @@ -286,10 +287,7 @@ func (g *Guard) evaluateLocked() Decision { reasons = append(reasons, ReasonNoInput) age = now default: - age = now - meta.ReceivedMonotonic - if age < 0 { - age = 0 - } + age = max(now-meta.ReceivedMonotonic, 0) if age >= g.options.commandTimeout { reasons = append(reasons, ReasonCommandTimeout) } @@ -322,19 +320,18 @@ func (g *Guard) evaluateLocked() Decision { } } - if g.estop { + if g.lifecycle == lifecycleStopped { reasons = append(reasons, ReasonEmergencyStop) } - if !g.armed { + if g.lifecycle != lifecycleArmed { reasons = append(reasons, ReasonNotArmed) } // Any unmet condition latches, so a transient fault cannot silently // restore authority when it clears. - if len(reasons) > 0 && g.options.latching && g.armed { + if len(reasons) > 0 && g.options.latching && g.lifecycle == lifecycleArmed { if !onlyDeadManEngagement(reasons) { - g.armed = false - g.latched = true + g.fire(commandTrip) reasons = appendUnique(reasons, ReasonNotArmed) } } @@ -357,7 +354,7 @@ func (g *Guard) evaluateLocked() Decision { } g.state = decision.State - g.reasons = append([]Reason(nil), reasons...) + g.reasons = slices.Clone(reasons) return decision } @@ -379,16 +376,14 @@ func onlyDeadManEngagement(reasons []Reason) bool { } func appendUnique(reasons []Reason, reason Reason) []Reason { - for _, candidate := range reasons { - if candidate == reason { - return reasons - } + if slices.Contains(reasons, reason) { + return reasons } return append(reasons, reason) } func (g *Guard) copyReasons() []Reason { - return append([]Reason(nil), g.reasons...) + return slices.Clone(g.reasons) } // Process implements teleop.Processor for callers that cannot supply a @@ -487,22 +482,14 @@ func (g *Guard) transitionLocked( Meta: header, State: decision.State, Permit: decision.Permit, - Reasons: append([]Reason(nil), decision.Reasons...), + Reasons: slices.Clone(decision.Reasons), InputAge: decision.InputAge, Detail: g.detail, }} } func (g *Guard) changedLocked(decision Decision) bool { - if decision.State != g.lastPublished.State || + return decision.State != g.lastPublished.State || decision.Permit != g.lastPublished.Permit || - len(decision.Reasons) != len(g.lastPublished.Reasons) { - return true - } - for index, reason := range decision.Reasons { - if g.lastPublished.Reasons[index] != reason { - return true - } - } - return false + !slices.Equal(decision.Reasons, g.lastPublished.Reasons) } diff --git a/safety/guard_test.go b/safety/guard_test.go index 8a32ce5..bf3ae0f 100644 --- a/safety/guard_test.go +++ b/safety/guard_test.go @@ -461,15 +461,13 @@ func TestConcurrentEvaluationIsRaceFree(t *testing.T) { var waiting sync.WaitGroup for range 8 { - waiting.Add(1) - go func() { - defer waiting.Done() + waiting.Go(func() { for range 200 { guard.Heartbeat() guard.Evaluate() source.observe(heldState()) } - }() + }) } waiting.Wait() } diff --git a/safety/lifecycle.go b/safety/lifecycle.go new file mode 100644 index 0000000..b57516d --- /dev/null +++ b/safety/lifecycle.go @@ -0,0 +1,116 @@ +package safety + +import ( + "context" + "errors" + + "github.com/open-ships/statemachine" +) + +// lifecycle is the operator's standing intent for the gate, independent of +// whether the automatic conditions currently hold. Evaluate derives the +// published State from this together with the live conditions, so this type +// records only what a command established: nothing an input observation does +// can change it, and nothing but a command can restore authority once it is +// lost. +type lifecycle string + +const ( + // lifecycleIdle inhibits output. It is the state of a gate that has never + // been armed, that an operator disarmed, or that a fault tripped. + lifecycleIdle lifecycle = "idle" + // lifecycleArmed means an operator authorized output, subject to every + // configured condition still being provable at the instant of each + // Evaluate. + lifecycleArmed lifecycle = "armed" + // lifecycleStopped is a latched emergency stop. Only Reset clears it, and + // clearing it does not restore authority. + lifecycleStopped lifecycle = "stopped" +) + +// command is an operator action, or the internal trip, that moves the +// lifecycle. Input events are not commands: they are conditions Evaluate +// re-derives, not transitions. +type command string + +const ( + commandArm command = "arm" + commandDisarm command = "disarm" + commandTrip command = "trip" + commandStop command = "stop" + commandReset command = "reset" +) + +// errArmDuringStop is the refusal reason carried by the arm row that a latched +// emergency stop declines. +var errArmDuringStop = errors.New("teleop/safety: cannot arm during an emergency stop") + +// transition is an alias, not a defined type, so MustCompile can infer the +// machine's type parameters from the table. +type transition = statemachine.Transition[lifecycle, command, *Guard] + +// gate is the operator-command state machine. It is the whole answer to which +// commands are legal in which state; the booleans this table replaced encoded +// the same rules implicitly, and had to be kept mutually consistent by hand at +// every assignment. +// +// The machine holds no state. The current lifecycle lives on the Guard, under +// the Guard's own mutex, which also protects everything else a transition +// touches. +var gate = statemachine.MustCompile([]transition{ + // Arming requires a bound controller, and is refused outright while a stop + // is latched: clearing one has to be a deliberate Reset rather than a + // reflexive re-arm. The stopped row exists to carry that reason; its Guard + // never applies, so it is never selected. + {From: lifecycleIdle, Event: commandArm, To: lifecycleArmed, Guard: requireBound}, + {From: lifecycleArmed, Event: commandArm, To: lifecycleArmed, Guard: requireBound}, + {From: lifecycleStopped, Event: commandArm, To: lifecycleArmed, Guard: refuseArmDuringStop}, + + // Disarming and tripping both drop authority. Neither clears a stop, so + // both are self-transitions once one is latched. + {From: lifecycleIdle, Event: commandDisarm, To: lifecycleIdle}, + {From: lifecycleArmed, Event: commandDisarm, To: lifecycleIdle}, + {From: lifecycleStopped, Event: commandDisarm, To: lifecycleStopped}, + + // Only an armed gate can trip; Evaluate fires this when a condition it + // cannot prove latches the gate. + {From: lifecycleArmed, Event: commandTrip, To: lifecycleIdle}, + + // A stop latches from every state, including itself. + {From: lifecycleIdle, Event: commandStop, To: lifecycleStopped}, + {From: lifecycleArmed, Event: commandStop, To: lifecycleStopped}, + {From: lifecycleStopped, Event: commandStop, To: lifecycleStopped}, + + // Reset clears a stop and leaves output inhibited until the next Arm. On a + // gate with no stop latched it changes nothing. + {From: lifecycleStopped, Event: commandReset, To: lifecycleIdle}, + {From: lifecycleIdle, Event: commandReset, To: lifecycleIdle}, + {From: lifecycleArmed, Event: commandReset, To: lifecycleArmed}, +}) + +// requireBound declines while the Guard has no controller. Binding is checked +// before any other arming condition so an unbound gate reports that first. +func requireBound(_ context.Context, g *Guard) error { + if g.source == nil { + return ErrUnbound + } + return nil +} + +// refuseArmDuringStop declines every arm attempt made while a stop is latched, +// reporting the unbound gate first so the reason does not depend on which +// condition is checked first. +func refuseArmDuringStop(ctx context.Context, g *Guard) error { + if err := requireBound(ctx, g); err != nil { + return err + } + return errArmDuringStop +} + +// fire applies a command that every lifecycle state accepts. Such a command +// cannot be refused, and assigning Fire's result is correct even if one ever +// were: the machine reports the state it was given whenever it reports an +// error. The caller must hold g.mu. +func (g *Guard) fire(event command) { + g.lifecycle, _ = gate.Fire(context.Background(), g.lifecycle, event, g) +}