diff --git a/selfdrive/ui/tests/test_obdyssey_ui.py b/selfdrive/ui/tests/test_obdyssey_ui.py index 311317fda..2bbf0e19b 100644 --- a/selfdrive/ui/tests/test_obdyssey_ui.py +++ b/selfdrive/ui/tests/test_obdyssey_ui.py @@ -10,6 +10,7 @@ from openpilot.starpilot.system.bluetooth.tests.test_bluetooth import FakeParams from openpilot.starpilot.system.obdyssey.protocol import OBDysseyStatus from openpilot.system.ui.lib.application import gui_app from openpilot.system.ui.widgets.bluetooth import device_status_text +from openpilot.system.ui.widgets import obdyssey as obdyssey_widget from openpilot.system.ui.widgets.obdyssey import OBDysseyScreen from openpilot.system.ui.widgets.obdyssey import DTC_STATE_CLEAN, DTC_STATE_FAULTS, DTC_STATE_IN_PROGRESS, DTC_STATE_UNAVAILABLE @@ -73,6 +74,8 @@ def make_obdyssey_screen(client: FakeOBDysseyClient, params: FakeParams) -> OBDy screen._clear_in_progress = False screen._retry_in_progress = False screen._last_error = "" + screen._smoke_test_retry_at = None + screen._smoke_test_retry_used = False return screen @@ -172,6 +175,92 @@ def test_obdyssey_ready_refresh_preserves_telemetry_and_dtcs(): assert screen._dtc_state == DTC_STATE_FAULTS +def test_obdyssey_failed_sample_retries_once_and_recovers(monkeypatch): + class SequencedClient(FakeOBDysseyClient): + def __init__(self): + super().__init__(connected=True) + self.read_calls = 0 + + def read_signals(self, _ids: list[str]) -> dict: + self.read_calls += 1 + if self.read_calls == 1: + return { + "signals": {}, + "errors": {"BOLT_HVBAT_SOC": {"type": "ElmNoDataError", "message": "ELM returned NO DATA"}}, + } + return {"signals": {"BOLT_HVBAT_SOC": 78.5}, "errors": {}} + + class StopAfterTwoRefreshes: + def __init__(self): + self.refreshes = 0 + self.stopped = False + + def clear(self): + self.refreshes = 0 + self.stopped = False + + def is_set(self): + return self.stopped + + def wait(self, _timeout): + self.refreshes += 1 + self.stopped = self.refreshes >= 2 + return self.stopped + + monkeypatch.setattr(obdyssey_widget, "SMOKE_TEST_RETRY_DELAY", 0.0) + client = SequencedClient() + screen = make_obdyssey_screen(client, FakeParams(IsOffroad=True)) + screen._stop_event = StopAfterTwoRefreshes() + + screen._worker_loop() + + assert client.read_calls == 2 + assert screen._live_telemetry == {"BOLT_HVBAT_SOC": 78.5} + assert screen._last_error == "" + + +def test_obdyssey_failed_sample_does_not_retry_indefinitely(monkeypatch): + class ErrorClient(FakeOBDysseyClient): + def __init__(self): + super().__init__(connected=True) + self.read_calls = 0 + + def read_signals(self, _ids: list[str]) -> dict: + self.read_calls += 1 + return { + "signals": {}, + "errors": {"BOLT_HVBAT_SOC": {"type": "ElmNoDataError", "message": "ELM returned NO DATA"}}, + } + + class StopAfterThreeRefreshes: + def __init__(self): + self.refreshes = 0 + self.stopped = False + + def clear(self): + self.refreshes = 0 + self.stopped = False + + def is_set(self): + return self.stopped + + def wait(self, _timeout): + self.refreshes += 1 + self.stopped = self.refreshes >= 3 + return self.stopped + + monkeypatch.setattr(obdyssey_widget, "SMOKE_TEST_RETRY_DELAY", 0.0) + client = ErrorClient() + screen = make_obdyssey_screen(client, FakeParams(IsOffroad=True)) + screen._stop_event = StopAfterThreeRefreshes() + + screen._worker_loop() + + assert client.read_calls == 2 + assert screen._live_telemetry == {} + assert screen._last_error == "ELM returned NO DATA" + + def test_obdyssey_screen_clear_codes_safety_gating(monkeypatch): fake_client = FakeOBDysseyClient(connected=True) params = FakeParams(IsOffroad=False) diff --git a/system/ui/widgets/obdyssey.py b/system/ui/widgets/obdyssey.py index de02f0aaf..0c4f98c9f 100644 --- a/system/ui/widgets/obdyssey.py +++ b/system/ui/widgets/obdyssey.py @@ -80,6 +80,7 @@ SMOKE_TEST_SIGNAL_IDS = ( "BOLT_HVBAT_SOC", "BOLT_HVBAT_VOLTAGE", ) +SMOKE_TEST_RETRY_DELAY = 2.0 class OBDysseyScreen(Widget): @@ -109,6 +110,8 @@ class OBDysseyScreen(Widget): self._clear_in_progress = False self._retry_in_progress = False self._last_error = "" + self._smoke_test_retry_at: float | None = None + self._smoke_test_retry_used = False def _go_back(self): gui_app.pop_widget() @@ -150,14 +153,35 @@ class OBDysseyScreen(Widget): # API v2 separates successful values from per-signal errors. Keep a # small compatibility path for developer fakes implementing the old # flat dictionary contract. + error_message = "" if isinstance(readings, dict) and "signals" in readings: self._live_telemetry = dict(readings.get("signals", {})) + errors = readings.get("errors", {}) + if isinstance(errors, dict) and errors: + first_error = next(iter(errors.values())) + if isinstance(first_error, dict): + error_message = str(first_error.get("message") or first_error.get("type") or "") + else: + error_message = str(first_error) else: self._live_telemetry = dict(readings or {}) + if error_message: + self._last_error = error_message + elif self._live_telemetry: + self._last_error = "" + + def _reset_smoke_test_retry(self): + self._smoke_test_retry_at = None + self._smoke_test_retry_used = False + + def _schedule_smoke_test_retry(self): + if self._available_signals and not self._live_telemetry and not self._smoke_test_retry_used: + self._smoke_test_retry_at = time.monotonic() + SMOKE_TEST_RETRY_DELAY def show_event(self): super().show_event() self._stop_event.clear() + self._reset_smoke_test_retry() self._poller_thread = threading.Thread(target=self._worker_loop, daemon=True) self._poller_thread.start() @@ -241,11 +265,13 @@ class OBDysseyScreen(Widget): self._connect_adapter() self._status = self._client.status() self._read_smoke_test_signals() + self._schedule_smoke_test_retry() except Exception as err: self._last_error = str(err) # Main status loop. Vehicle reads remain demand-driven; this screen takes - # one small sample after connection/recovery rather than polling all PIDs. + # one small sample after connection/recovery, with one delayed retry when + # the sample returns no values rather than polling all PIDs. while not self._stop_event.is_set(): try: was_diagnostic_ready = self._diagnostic_ready() @@ -259,15 +285,23 @@ class OBDysseyScreen(Widget): if self._diagnostic_ready(): if not was_diagnostic_ready: + self._reset_smoke_test_retry() self._read_smoke_test_signals() - # A ready → ready status refresh is not a lifecycle transition. - # Preserve the last successful telemetry and DTC results. + self._schedule_smoke_test_retry() + elif (self._smoke_test_retry_at is not None + and time.monotonic() >= self._smoke_test_retry_at): + self._smoke_test_retry_at = None + self._smoke_test_retry_used = True + self._read_smoke_test_signals() + # A ready → ready status refresh is not a lifecycle transition, + # except for the single bounded retry above. elif was_diagnostic_ready: # Only clear stale readings when leaving a usable backend state; # don't repeatedly erase an already-empty/error state on every poll. self._live_telemetry.clear() self._dtcs.clear() self._dtc_state = DTC_STATE_UNAVAILABLE + self._reset_smoke_test_retry() except Exception as err: self._last_error = str(err)