Refactor: Introduce ButtonHoldTracker to manage button hold durations (#593)

Add ButtonHoldTracker for button hold logic and tests

Introduce a new `ButtonHoldTracker` class to manage button hold durations, replacing manual timer handling in `ExperimentalSwitcher`. Updated `ExperimentalSwitcher` to leverage this implementation for cleaner and more modular code. Added comprehensive unit tests for both `ButtonHoldTracker` and `ExperimentalSwitcher` to ensure functionality and edge case coverage.

sunnypilot/selfdrive/car/button_hold_tracker.py

@@ -0,0 +1,32 @@
+"""
+Copyright (c) 2021-, Haibin Wen, sunnypilot, and a number of other contributors.
+
+This file is part of sunnypilot and is licensed under the MIT License.
+See the LICENSE.md file in the root directory for more details.
+"""
+
+from cereal import car, log
+ButtonType = car.CarState.ButtonEvent.Type
+EventName = log.OnroadEvent.EventName
+
+
+class ButtonHoldTracker:
+  def __init__(self):
+    self.button_frame_counts = {}
+
+  def update(self, CS) -> None:
+    if CS.cruiseState.available:
+      self.update_button_timers(CS)
+
+  def update_button_timers(self, CS) -> None:
+    for button_type, held_frames in self.button_frame_counts.items():
+      if held_frames > 0:
+        self.button_frame_counts[button_type] += 1
+
+    for event in CS.buttonEvents:
+      event_type = event.type.raw
+      if not (event.pressed and self.button_frame_counts.get(event_type, 0) > 0):  # No need to reset if the button is "pulsing" always "pressed"
+        self.button_frame_counts[event_type] = int(event.pressed)
+
+  def get_hold_duration(self, button_type):
+    return self.button_frame_counts.get(button_type, 0)

sunnypilot/selfdrive/car/experimental_switcher.py

@@ -5,37 +5,25 @@ This file is part of sunnypilot and is licensed under the MIT License.
 See the LICENSE.md file in the root directory for more details.
 """
 
-from cereal import car, log
-
-ButtonType = car.CarState.ButtonEvent.Type
-EventName = log.OnroadEvent.EventName
+from openpilot.sunnypilot.selfdrive.car.button_hold_tracker import ButtonHoldTracker, ButtonType, EventName
 
 DISTANCE_LONG_PRESS = 50
 
 
 class ExperimentalSwitcher:
-  def __init__(self, params):
+  def __init__(self, params, button_type=ButtonType.gapAdjustCruise):
     self.params = params
-
-    self.button_timers = {ButtonType.gapAdjustCruise: 0}
-    self.experimental_mode_switched = False
+    self.button_type = button_type
+    self._button_tracker = ButtonHoldTracker()
+    self.experimental_mode_switched = False  # Meant to be toggled off by this class's children.
 
   def update(self, CS, events, experimental_mode):
+    self._button_tracker.update(CS)
     if CS.cruiseState.available:
-      self.update_button_timers(CS)
       self.update_mode(events, experimental_mode)
 
-  def update_button_timers(self, CS) -> None:
-    for k in self.button_timers:
-      if self.button_timers[k] > 0:
-        self.button_timers[k] += 1
-
-    for b in CS.buttonEvents:
-      if b.type.raw in self.button_timers:
-        self.button_timers[b.type.raw] = 1 if b.pressed else 0
-
   def update_mode(self, events, experimental_mode) -> None:
-    if self.button_timers[ButtonType.gapAdjustCruise] >= DISTANCE_LONG_PRESS and not self.experimental_mode_switched:
+    if self._button_tracker.get_hold_duration(self.button_type) >= DISTANCE_LONG_PRESS and not self.experimental_mode_switched:
       experimental_mode = not experimental_mode
       self.params.put_bool_nonblocking("ExperimentalMode", experimental_mode)
       events.add(EventName.experimentalModeSwitched)

sunnypilot/selfdrive/car/tests/test_button_hold_tracker.py

@@ -0,0 +1,81 @@
+"""
+Copyright (c) 2021-, Haibin Wen, sunnypilot, and a number of other contributors.
+
+This file is part of sunnypilot and is licensed under the MIT License.
+See the LICENSE.md file in the root directory for more details.
+"""
+
+import pytest
+
+from cereal import car
+from sunnypilot.selfdrive.car.button_hold_tracker import ButtonHoldTracker, ButtonType
+
+
+@pytest.fixture
+def setup_button_tracker():
+  return ButtonHoldTracker()
+
+
+class TestButtonHoldTracker:
+  @pytest.mark.parametrize(
+    "button_type, button_events, initial_timer, expected_timer",
+    [
+      # Basic button press scenarios
+      (ButtonType.gapAdjustCruise, [car.CarState.ButtonEvent(type=ButtonType.gapAdjustCruise, pressed=True)], 0, 1),
+      (ButtonType.gapAdjustCruise, [car.CarState.ButtonEvent(type=ButtonType.gapAdjustCruise, pressed=False)], 1, 0),
+
+      # Auto-increment when already held
+      (ButtonType.gapAdjustCruise, [], 5, 6),
+      (ButtonType.cancel, [], 10, 11),
+      (ButtonType.leftBlinker, [], 1, 2),
+
+      # Multiple button events in same frame
+      (ButtonType.gapAdjustCruise,
+       [
+         car.CarState.ButtonEvent(type=ButtonType.cancel, pressed=True),
+         car.CarState.ButtonEvent(type=ButtonType.gapAdjustCruise, pressed=False)
+       ], 0, 0),
+
+      # Events for different buttons shouldn't affect each other
+      (ButtonType.gapAdjustCruise, [car.CarState.ButtonEvent(type=ButtonType.cancel, pressed=True)], 5, 6),
+
+      # Press while already held (like if it was pulsing the event for some reason)
+      (ButtonType.cancel, [car.CarState.ButtonEvent(type=ButtonType.cancel, pressed=True)], 5, 6),
+
+      # Edge cases
+      (ButtonType.leftBlinker, [], 0, 0),
+      (ButtonType.cancel, [car.CarState.ButtonEvent(type=ButtonType.cancel, pressed=False)], 0, 0),
+    ],
+    ids=[
+      "initial_button_press",
+      "button_release",
+      "increment_held_gap_adjust",
+      "increment_held_cancel",
+      "increment_held_blinker",
+      "multiple_events_same_frame",
+      "different_button_no_effect",
+      "press_while_held",
+      "zero_state_no_events",
+      "release_when_not_held"
+    ]
+  )
+  def test_update_button_timers(self, mocker, setup_button_tracker, button_type, button_events, initial_timer, expected_timer):
+    tracker = setup_button_tracker
+    mock_CS = mocker.Mock(buttonEvents=button_events, cruiseState=mocker.Mock(available=True))
+
+    if initial_timer > 0:
+      tracker.button_frame_counts[button_type] = initial_timer
+
+    tracker.update(mock_CS)
+    timer_value = tracker.get_hold_duration(button_type)
+    assert timer_value == expected_timer, f"Expected timer for {button_type} to be {expected_timer}, but got {timer_value}"
+
+  def test_cruise_state_unavailable(self, mocker, setup_button_tracker):
+    tracker = setup_button_tracker
+    mock_CS = mocker.Mock(
+      buttonEvents=[car.CarState.ButtonEvent(type=ButtonType.gapAdjustCruise, pressed=True)],
+      cruiseState=mocker.Mock(available=False)
+    )
+
+    tracker.update(mock_CS)
+    assert tracker.get_hold_duration(ButtonType.gapAdjustCruise) == 0

sunnypilot/selfdrive/car/tests/test_experimental_switcher.py

@@ -0,0 +1,55 @@
+"""
+Copyright (c) 2021-, Haibin Wen, sunnypilot, and a number of other contributors.
+
+This file is part of sunnypilot and is licensed under the MIT License.
+See the LICENSE.md file in the root directory for more details.
+"""
+
+import pytest
+
+from cereal import car
+from sunnypilot.selfdrive.car.experimental_switcher import ExperimentalSwitcher, ButtonType, EventName, DISTANCE_LONG_PRESS
+
+
+@pytest.fixture
+def setup_switcher(mocker):
+  mock_params = mocker.Mock()
+  return ExperimentalSwitcher(mock_params), mock_params
+
+
+class TestExperimentalSwitcher:
+  @pytest.mark.parametrize(
+    "use_experimental_mode, long_press, expected_mode, expected_experimental_mode",
+    [
+      (False, True, True, True),  # Normal -> Experimental via long press
+      (True, True, False, True),  # Experimental -> Normal via long press
+      (False, False, False, False)  # No change without long press
+    ],
+    ids=[
+      "enable_experimental_mode",
+      "disable_experimental_mode",
+      "no_mode_change_without_long_press"
+    ]
+  )
+  def test_update_mode_switches(self, mocker, setup_switcher, use_experimental_mode, long_press, expected_mode, expected_experimental_mode):
+    switcher, params = setup_switcher
+    mock_events = mocker.Mock()
+    mock_CS = mocker.Mock(
+      buttonEvents=[car.CarState.ButtonEvent(type=ButtonType.gapAdjustCruise, pressed=True)],
+      cruiseState=mocker.Mock(available=True)
+    )
+
+    if long_press:
+      for _ in range(DISTANCE_LONG_PRESS + 1):
+        switcher.update(mock_CS, mock_events, use_experimental_mode)
+
+    switcher.update_mode(mock_events, use_experimental_mode)
+
+    if expected_experimental_mode:
+      params.put_bool_nonblocking.assert_called_once_with("ExperimentalMode", expected_mode)
+      mock_events.add.assert_called_once_with(EventName.experimentalModeSwitched)
+      assert switcher.experimental_mode_switched is True
+    else:
+      params.put_bool_nonblocking.assert_not_called()
+      mock_events.add.assert_not_called()
+      assert switcher.experimental_mode_switched is False