From 7ee7e73ce708f9b52aabed23e398ccf94b1834cf Mon Sep 17 00:00:00 2001 From: DevTekVE Date: Sun, 19 Jan 2025 13:59:55 +0100 Subject: [PATCH] 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. --- .../selfdrive/car/button_hold_tracker.py | 32 ++++++++ .../selfdrive/car/experimental_switcher.py | 26 ++---- sunnypilot/selfdrive/car/tests/__init__.py | 0 .../car/tests/test_button_hold_tracker.py | 81 +++++++++++++++++++ .../car/tests/test_experimental_switcher.py | 55 +++++++++++++ 5 files changed, 175 insertions(+), 19 deletions(-) create mode 100644 sunnypilot/selfdrive/car/button_hold_tracker.py create mode 100644 sunnypilot/selfdrive/car/tests/__init__.py create mode 100644 sunnypilot/selfdrive/car/tests/test_button_hold_tracker.py create mode 100644 sunnypilot/selfdrive/car/tests/test_experimental_switcher.py diff --git a/sunnypilot/selfdrive/car/button_hold_tracker.py b/sunnypilot/selfdrive/car/button_hold_tracker.py new file mode 100644 index 0000000000..d7b20d938b --- /dev/null +++ b/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) diff --git a/sunnypilot/selfdrive/car/experimental_switcher.py b/sunnypilot/selfdrive/car/experimental_switcher.py index 3dd3db1478..9708d570ee 100644 --- a/sunnypilot/selfdrive/car/experimental_switcher.py +++ b/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) diff --git a/sunnypilot/selfdrive/car/tests/__init__.py b/sunnypilot/selfdrive/car/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/sunnypilot/selfdrive/car/tests/test_button_hold_tracker.py b/sunnypilot/selfdrive/car/tests/test_button_hold_tracker.py new file mode 100644 index 0000000000..c51af4c51d --- /dev/null +++ b/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 diff --git a/sunnypilot/selfdrive/car/tests/test_experimental_switcher.py b/sunnypilot/selfdrive/car/tests/test_experimental_switcher.py new file mode 100644 index 0000000000..1e794cd31b --- /dev/null +++ b/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