From 718a2c3ddd3c526fb3feaa04f7dd62cbc278cfb7 Mon Sep 17 00:00:00 2001 From: firestar5683 <168790843+firestar5683@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:17:20 -0500 Subject: [PATCH] Keep SpacePilot open across cold setup and interrupted clock reads Discard a preempted clock pair and try up to three fresh pairs while retaining the 1 ms accepted span and all existing freshness/resume gates. Yield after receive-only decoder construction so normal UI collection refreshes before full physical admission, Offroad request or input. Recorded d5 clock failure; reproduced timing regressions; 250 focused native cases, lint/type checks and independent source/evidence review. No vehicle commands, device deployment or physical cure claim. --- openpilot/starpilot/ui/spacepilot/README.md | 5 + openpilot/starpilot/ui/spacepilot/guard.py | 14 ++ openpilot/starpilot/ui/spacepilot/ignition.py | 9 +- openpilot/starpilot/ui/spacepilot/native.py | 8 +- openpilot/starpilot/ui/spacepilot/session.py | 12 +- .../ui/tests/test_spacepilot_entry.py | 4 + .../ui/tests/test_spacepilot_timing.py | 164 ++++++++++++++++++ prebuilt.json | 2 +- 8 files changed, 199 insertions(+), 19 deletions(-) create mode 100644 openpilot/starpilot/ui/tests/test_spacepilot_timing.py diff --git a/openpilot/starpilot/ui/spacepilot/README.md b/openpilot/starpilot/ui/spacepilot/README.md index c6f825dccc..b07394a9cf 100644 --- a/openpilot/starpilot/ui/spacepilot/README.md +++ b/openpilot/starpilot/ui/spacepilot/README.md @@ -82,3 +82,8 @@ and cleanup. Native GL previews and host CPU/RSS/frame-time measurements use isolated synthetic telemetry. They do not establish physical C4/C3 performance, actual wheel/controller hardware behavior, full-system headroom or deployment. No road-device installation, restart, or driving test is part of host validation. + +Clock reads interrupted by scheduling are retried up to three times, with the +same 1 ms sampling bound and resume/freshness checks on the accepted sample. +Cold receive-only decoder construction yields to the next normal UI update +before admission; it never grants gameplay or Offroad from an aged UI snapshot. diff --git a/openpilot/starpilot/ui/spacepilot/guard.py b/openpilot/starpilot/ui/spacepilot/guard.py index 63eee17dc0..ae0e880d48 100644 --- a/openpilot/starpilot/ui/spacepilot/guard.py +++ b/openpilot/starpilot/ui/spacepilot/guard.py @@ -2,6 +2,7 @@ from dataclasses import dataclass import math +import time from typing import Any from openpilot.starpilot.parked_evidence import PANDA_TTL_NS, RESUME_SKEW_NS @@ -12,6 +13,19 @@ DWELL_NS = 1_000_000_000 SERVICES = ('carState', 'selfdriveState', 'carControl', 'deviceState', 'pandaStates') +def sample_clocks() -> tuple[int, int]: + """Discard a preempted clock read, never widen evidence or resume tolerances.""" + for _ in range(3): + before = time.monotonic_ns() + boot = time.clock_gettime_ns(time.CLOCK_BOOTTIME) + after = time.monotonic_ns() + if before <= 0 or boot <= 0 or after < before: + raise ValueError('Device clock moved backwards or is unavailable') + if after - before <= 1_000_000: + return after, boot + raise ValueError('Waiting for a stable device clock') + + @dataclass(frozen=True) class Witness: mono_ns: int diff --git a/openpilot/starpilot/ui/spacepilot/ignition.py b/openpilot/starpilot/ui/spacepilot/ignition.py index d325ec5186..27abe97882 100644 --- a/openpilot/starpilot/ui/spacepilot/ignition.py +++ b/openpilot/starpilot/ui/spacepilot/ignition.py @@ -2,10 +2,9 @@ from dataclasses import dataclass import math -import time from openpilot.starpilot.parked_evidence import PANDA_TTL_NS, RESUME_SKEW_NS -from openpilot.starpilot.ui.spacepilot.guard import DWELL_NS, FAST_AGE_NS, SLOW_AGE_NS, read_offroad_witness +from openpilot.starpilot.ui.spacepilot.guard import DWELL_NS, FAST_AGE_NS, SLOW_AGE_NS, read_offroad_witness, sample_clocks @dataclass(frozen=True) @@ -21,10 +20,8 @@ class IgnitionSample: def sample_ignition(ui, *, native_device: bool) -> IgnitionSample | None: try: - before = time.monotonic_ns() - boot = time.clock_gettime_ns(time.CLOCK_BOOTTIME) - mono = time.monotonic_ns() - if (not native_device or not 0 <= mono - before <= 1_000_000 or ui.projection_read_only or + mono, boot = sample_clocks() + if (not native_device or ui.projection_read_only or ui.replay_clock is not None or getattr(ui.sm, 'replay_sample', None) is not None): return None sm = ui.sm diff --git a/openpilot/starpilot/ui/spacepilot/native.py b/openpilot/starpilot/ui/spacepilot/native.py index 90541f0eb8..e21b0e2269 100644 --- a/openpilot/starpilot/ui/spacepilot/native.py +++ b/openpilot/starpilot/ui/spacepilot/native.py @@ -10,7 +10,7 @@ from openpilot.common.hardware import HARDWARE from openpilot.common.swaglog import cloudlog from openpilot.selfdrive.ui.mici.widgets.button import BigButton from openpilot.selfdrive.ui.ui_state import device, ui_state -from openpilot.starpilot.ui.spacepilot.guard import ParkGate, Witness, read_offroad_witness, read_witness +from openpilot.starpilot.ui.spacepilot.guard import ParkGate, Witness, read_offroad_witness, read_witness, sample_clocks from openpilot.starpilot.ui.spacepilot.session import GameSession, Stage from openpilot.starpilot.ui.spacepilot.ignition import IgnitionReturn from openpilot.starpilot.drive_state.owner import DriveStateOwner @@ -50,11 +50,7 @@ def poll_game_mode() -> None: def live_witness() -> Witness | None: try: - before = time.monotonic_ns() - boot = time.clock_gettime_ns(time.CLOCK_BOOTTIME) - after = time.monotonic_ns() - if not 0 <= after - before <= 1_000_000: - return None + after, boot = sample_clocks() native_device = HARDWARE.get_device_type() in ('mici', 'tici', 'tizi') if ui_state.started: return read_witness(ui_state, after, boot, c4=native_device) diff --git a/openpilot/starpilot/ui/spacepilot/session.py b/openpilot/starpilot/ui/spacepilot/session.py index f0b7230059..1ae70bedcd 100644 --- a/openpilot/starpilot/ui/spacepilot/session.py +++ b/openpilot/starpilot/ui/spacepilot/session.py @@ -9,7 +9,7 @@ from enum import StrEnum import time from openpilot.starpilot.parked_vehicle import ParkedVehicleReader -from openpilot.starpilot.ui.spacepilot.guard import ParkGate, read_offroad_witness, read_witness +from openpilot.starpilot.ui.spacepilot.guard import ParkGate, read_offroad_witness, read_witness, sample_clocks from openpilot.starpilot.ui.spacepilot.state import WheelInput from openpilot.starpilot.ui.spacepilot.ignition import IgnitionReturn @@ -40,11 +40,7 @@ class GameSession: self._clock_offset = None def _clocks(self): - before = time.monotonic_ns() - boot = time.clock_gettime_ns(time.CLOCK_BOOTTIME) - mono = time.monotonic_ns() - if not 0 <= mono - before <= 1_000_000: - raise ValueError('Waiting for a stable device clock') + mono, boot = sample_clocks() offset = boot - mono if self._clock_offset is not None and abs(offset - self._clock_offset) > 5_000_000: raise ValueError('Device resumed; re-enter SpacePilot') @@ -78,6 +74,10 @@ class GameSession: not 0 < int(sm.logMonoTime['carParams']) <= mono): raise ValueError('Current vehicle identity is unavailable') self.vehicle = self._reader_factory(sm['carParams']) + # Cold imports/parser construction can outlast this frame's UI sample. + # Let the normal collector refresh next frame before any admission, + # request or input; all original dwell and freshness checks still run. + return False samples = self.vehicle.poll() if self.vehicle is not None else () mono, boot = self._clocks() vehicle_ready = bool(self.vehicle is not None and self.vehicle.ready(mono, boot)) diff --git a/openpilot/starpilot/ui/tests/test_spacepilot_entry.py b/openpilot/starpilot/ui/tests/test_spacepilot_entry.py index cdf2dbc168..879fd03205 100644 --- a/openpilot/starpilot/ui/tests/test_spacepilot_entry.py +++ b/openpilot/starpilot/ui/tests/test_spacepilot_entry.py @@ -113,6 +113,10 @@ def test_fresh_park_page_prepares_after_dwell_reset_and_keeps_actual_owner_gates page._tick() assert not page.closed and game.stage == native.Stage.VERIFYING assert owner.snapshot().mode == 'auto' and not game.playable() + assert game.gate.since is None # Reader setup yields before using this frame's snapshot. + clock.refresh(ui) + page._tick() + assert owner.snapshot().mode == 'auto' and not game.playable() assert game.gate.since == clock.now for _ in range(9): clock.refresh(ui) diff --git a/openpilot/starpilot/ui/tests/test_spacepilot_timing.py b/openpilot/starpilot/ui/tests/test_spacepilot_timing.py new file mode 100644 index 0000000000..dcbc6f612d --- /dev/null +++ b/openpilot/starpilot/ui/tests/test_spacepilot_timing.py @@ -0,0 +1,164 @@ +"""Cold native entry and preempted clock reads from the recorded d5 exit paths.""" + +from types import SimpleNamespace as NS +from unittest.mock import Mock + +import pytest + +from openpilot.starpilot.ui.spacepilot import ignition, native, session +from openpilot.starpilot.ui.tests.test_spacepilot_session import Clock, Vehicle, game_ui, offroad, setup as setup +from openpilot.starpilot.ui.tests.test_spacepilot_state import mature_gate, read +from openpilot.system.ui.lib.application import gui_app + + +def clock_reads(monkeypatch, *, delayed=1, backwards=False, offset=2_000_000_000): + now = 10_000_000_000 + monos, boots = [], [] + for index in range(4): + delta = -1 if backwards else 5_000_000 if index < delayed else 1_000 + monos.extend((now, now + delta)) + boots.append(now + offset) + now += max(delta, 0) + mono = Mock(side_effect=monos) + monkeypatch.setattr(session.time, 'monotonic_ns', mono) + monkeypatch.setattr(session.time, 'clock_gettime_ns', Mock(side_effect=boots)) + return mono + + +def witness_ui(monkeypatch): + ui = game_ui() + Clock().refresh(ui, 0) + monkeypatch.setattr(native, 'ui_state', ui) + monkeypatch.setattr(native.HARDWARE, 'get_device_type', lambda: 'mici') + return ui + + +@pytest.mark.parametrize('caller', ['session', 'entry', 'ignition']) +@pytest.mark.parametrize('delayed', [1, 2]) +def test_preempted_read_is_discarded_then_fresh_pair_used(monkeypatch, caller, delayed): + ui = witness_ui(monkeypatch) + game = session.GameSession(ui, Mock(), native_device=True) + mono = clock_reads(monkeypatch, delayed=delayed) + if caller == 'session': + result = game._clocks() + assert result[0] == 10_000_000_000 + delayed * 5_000_000 + 1_000 + assert result[1] - result[0] == 1_999_999_000 + elif caller == 'entry': + witness = native.live_witness() + assert witness is not None and witness.mono_ns == 10_000_000_000 + delayed * 5_000_000 + 1_000 + else: + sample = ignition.sample_ignition(ui, native_device=True) + assert sample is not None and sample.mono == 10_000_000_000 + delayed * 5_000_000 + 1_000 + assert mono.call_count == 2 * (delayed + 1) + + +@pytest.mark.parametrize('caller', ['session', 'entry', 'ignition']) +def test_three_preemptions_are_bounded_and_still_refuse(monkeypatch, caller): + ui = witness_ui(monkeypatch) + game = session.GameSession(ui, Mock(), native_device=True) + mono = clock_reads(monkeypatch, delayed=3) + if caller == 'session': + with pytest.raises(ValueError, match='stable device clock'): + game._clocks() + elif caller == 'entry': + assert native.live_witness() is None + else: + assert ignition.sample_ignition(ui, native_device=True) is None + assert mono.call_count <= 6 + + +def test_resumed_clock_is_not_hidden_by_successful_resample(monkeypatch): + game = session.GameSession(NS(), Mock(), native_device=True) + game._clock_offset = 2_000_000_000 + clock_reads(monkeypatch, offset=2_006_000_000) + with pytest.raises(ValueError): + game._clocks() + assert game._clock_offset == 2_000_000_000 + + +@pytest.mark.parametrize('caller', ['session', 'entry', 'ignition']) +def test_backwards_clock_never_retries_into_admission(monkeypatch, caller): + ui = witness_ui(monkeypatch) + game = session.GameSession(ui, Mock(), native_device=True) + mono = clock_reads(monkeypatch, backwards=True) + if caller == 'session': + with pytest.raises(ValueError): + game._clocks() + elif caller == 'entry': + assert native.live_witness() is None + else: + assert ignition.sample_ignition(ui, native_device=True) is None + assert mono.call_count == 2 + + +@pytest.mark.parametrize('change', ['none', 'drive', 'engaged', 'stale', 'timeout']) +def test_cold_reader_yields_to_next_ui_sample_without_request_or_input(setup, monkeypatch, change): + game, ui, owner, clock = setup + monkeypatch.setattr(native, 'ui_state', ui) + monkeypatch.setattr(native, 'live_witness', lambda: read(ui, clock.now)) + monkeypatch.setattr(native, 'GameSession', lambda *_a, **_k: game) + monkeypatch.setattr(native, 'device', Mock()) + monkeypatch.setattr(gui_app, '_nav_stack', [Mock()]) + monkeypatch.setattr(gui_app, '_nav_stack_ticks', []) + monkeypatch.setattr(native, '_ignition_return', None) + def cold_reader(cp): + clock.now += 400_000_000 # Recorded first launch spent396ms in a UI frame. + return Vehicle(cp) + game._reader_factory = cold_reader + page = native.SpacePilotPage(mature_gate(), Mock()) + gui_app.push_widget(page) + try: + page._tick() + assert not page.closed and game.stage == session.Stage.VERIFYING + assert owner.snapshot().mode == 'auto' and not game.playable() + assert game.gate.since is None and not game.wheel.armed + if change == 'timeout': + clock.refresh(ui, session.TRANSITION_NS) + elif change != 'stale': + clock.refresh(ui) + if change == 'drive': + ui.sm['carState'].gearShifter = 'drive' + elif change == 'engaged': + ui.sm['selfdriveState'].enabled = True + page._tick() + if change != 'none': + assert page.closed and game.stage == session.Stage.CLOSED and game.vehicle is None + assert owner.snapshot().mode == 'auto' + return + assert game.gate.since == clock.now and game.stage == session.Stage.VERIFYING + for _ in range(9): + clock.refresh(ui) + page._tick() + assert owner.snapshot().mode == 'auto' and not game.playable() + clock.refresh(ui) + page._tick() + assert game.stage == session.Stage.STOPPING and owner.snapshot().mode == 'offroad' + assert not game.playable() + offroad(ui) + for _ in range(12): + clock.refresh(ui) + page._tick() + assert game.stage == session.Stage.PLAYING and not page.closed and game.playable() + finally: + page.hide_event() + + +def test_playing_offroad_game_survives_preempted_read_without_bypassing_witness(monkeypatch): + from openpilot.starpilot.ui.spacepilot.guard import read_offroad_witness + ui = witness_ui(monkeypatch) + offroad(ui) + ui.sm['pandaStates'][0].ignitionLine = False + game = session.GameSession(ui, Mock(), native_device=True) + game.stage = session.Stage.PLAYING + game._was_ignition = False + game._clock_offset = 2_000_000_000 + game.gate.since = 9_000_000_000 + game.gate.previous = read_offroad_witness(ui, 10_000_000_000, 12_000_000_000, native_device=True) + assert game.gate.previous is not None + game.gate.ready = True + mono = clock_reads(monkeypatch) + assert not game.step() + assert game.stage == session.Stage.PLAYING and game.gate.ready + assert mono.call_count == 6 + assert not game.requested_offroad and game.vehicle is None + game.close() diff --git a/prebuilt.json b/prebuilt.json index ca001f89a5..45b29287a4 100644 --- a/prebuilt.json +++ b/prebuilt.json @@ -1 +1 @@ -{"protected_inventory_sha256": "0c603130945897676c958ed5c1fd8f5332f7069a9d4b082541a7baa4302d01aa", "schema_version": 1} +{"protected_inventory_sha256": "9742943ea33307b48b47dbdcba8a6390aeba9077c821c543e398374cd193e6bb", "schema_version": 1}