From b94bf3adca652d577184e4809aa9be2e5192e6fa Mon Sep 17 00:00:00 2001 From: firestarsdog <229254897+firestarsdog@users.noreply.github.com> Date: Wed, 14 Oct 2026 14:00:05 -0400 Subject: [PATCH] Test cleanup --- .gitattributes | 1 + .../galaxy/tests/test_http_server.py | 5 ++- openpilot/starpilot/replay/frames.py | 21 +++++----- openpilot/starpilot/replay/media.py | 23 ++++++++--- openpilot/starpilot/replay/owner.py | 13 +++++-- openpilot/starpilot/replay/render.py | 38 ++++++++++++------- .../starpilot/replay/tests/test_catalog.py | 1 + .../starpilot/replay/tests/test_frames.py | 2 + .../starpilot/replay/tests/test_media.py | 11 +++++- .../starpilot/replay/tests/test_owner.py | 12 +++++- .../starpilot/replay/tests/test_render.py | 37 ++++++++++++------ openpilot/starpilot/ui/pip_render.py | 6 ++- openpilot/starpilot/ui/replay_settings.py | 20 +++++----- .../ui/tests/test_replay_settings.py | 25 ++++++++---- .../ui/tests/test_ui_started_authority.py | 4 +- openpilot/tools/replay/tests/test_machine.py | 5 ++- scripts/lint/check_added_large_files.py | 3 ++ 17 files changed, 160 insertions(+), 67 deletions(-) diff --git a/.gitattributes b/.gitattributes index a7a632ca1b..75e8a8a133 100644 --- a/.gitattributes +++ b/.gitattributes @@ -34,6 +34,7 @@ tinygrad_repo/** !filter !diff !merge -text *.so.* -text openpilot/cereal/messaging/bridge -text openpilot/selfdrive/pandad/pandad -text +openpilot/tools/replay/replay -text openpilot/system/camerad/camerad -text openpilot/system/loggerd/bootlog -text openpilot/system/loggerd/encoderd -text diff --git a/openpilot/starpilot/galaxy/tests/test_http_server.py b/openpilot/starpilot/galaxy/tests/test_http_server.py index 7b398ad891..f64a90942f 100644 --- a/openpilot/starpilot/galaxy/tests/test_http_server.py +++ b/openpilot/starpilot/galaxy/tests/test_http_server.py @@ -6,6 +6,7 @@ import os import socket import threading from types import SimpleNamespace +from typing import Any from urllib.parse import urlsplit import pytest @@ -22,8 +23,8 @@ def server(tmp_path): directory.mkdir(parents=True) media = directory / 'init.mp4' media.write_bytes(b'x' * (65536 + 5)) - state = {'requests': [], 'chunks': 0, 'revoke_after': None, 'expire_session': False, 'expired': False, - 'recording_checks': 0, 'change_recording': False} + state: dict[str, Any] = {'requests': [], 'chunks': 0, 'revoke_after': None, 'expire_session': False, 'expired': False, + 'recording_checks': 0, 'change_recording': False} def owner(payload): if payload['op'] == 'media': diff --git a/openpilot/starpilot/replay/frames.py b/openpilot/starpilot/replay/frames.py index 2fee61cb93..e750b9f598 100644 --- a/openpilot/starpilot/replay/frames.py +++ b/openpilot/starpilot/replay/frames.py @@ -105,34 +105,35 @@ class ReplayFrameReader: self.fd = fd def latest(self) -> ReplayFrame | None: - if self.mm is None: + mm, fd = self.mm, self.fd + if mm is None or fd is None: return None try: - fcntl.flock(self.fd, fcntl.LOCK_SH | fcntl.LOCK_NB) + fcntl.flock(fd, fcntl.LOCK_SH | fcntl.LOCK_NB) except BlockingIOError: return None try: - return self._latest() + return self._latest(mm) finally: - fcntl.flock(self.fd, fcntl.LOCK_UN) + fcntl.flock(fd, fcntl.LOCK_UN) - def _latest(self): + def _latest(self, mm: mmap.mmap): for _ in range(3): - seq = SEQ.unpack_from(self.mm, 8)[0] + seq = SEQ.unpack_from(mm, 8)[0] if not seq or seq & 1: continue - magic, stamp, version, width, height, profile, epoch, frame_id, captured_ns, position, session = HEADER.unpack_from(self.mm) + magic, stamp, version, width, height, profile, epoch, frame_id, captured_ns, position, session = HEADER.unpack_from(mm) if (magic != MAGIC or stamp != seq or version != VERSION or profile not in PROFILES.values() or self.profile is not None and profile != PROFILES[self.profile] or not 0 < width <= MAX_WIDTH or not 0 < height <= MAX_HEIGHT or - len(self.mm) != HEADER_SIZE + width * height * 4 or session.rstrip(b'\0') != self.session_id.encode() or + len(mm) != HEADER_SIZE + width * height * 4 or session.rstrip(b'\0') != self.session_id.encode() or epoch <= 0 or frame_id <= 0 or captured_ns <= 0 or not math.isfinite(position) or position < 0): return None key = epoch, frame_id if key == self.last_key: return None - data = self.mm[HEADER_SIZE:] - if SEQ.unpack_from(self.mm, 8)[0] != seq: + data = mm[HEADER_SIZE:] + if SEQ.unpack_from(mm, 8)[0] != seq: continue self.last_key = key return ReplayFrame(data, width, height, self.session_id, epoch, position, frame_id, captured_ns) diff --git a/openpilot/starpilot/replay/media.py b/openpilot/starpilot/replay/media.py index 65d2c8be78..c35094ec3d 100644 --- a/openpilot/starpilot/replay/media.py +++ b/openpilot/starpilot/replay/media.py @@ -6,6 +6,7 @@ import math import os from pathlib import Path import re +from typing import TypedDict MAX_FRAGMENT_BYTES = 1024 * 1024 MAX_MEDIA_BYTES = 8 * 1024 * 1024 @@ -100,6 +101,15 @@ class MediaMux: self.lib.replay_mux_close(handle) +class MediaFragment(TypedDict): + sequence: int + mediaPath: str + length: int + firstPosition: float | None + lastPosition: float | None + keyframe: bool + + class MediaStore: def __init__(self, session_dir: Path, session_id: str, width: int, height: int): self.root = Path(session_dir) / 'media' @@ -108,8 +118,8 @@ class MediaStore: self.session_id, self.width, self.height = session_id, width, height self.epoch = 0 self.codec = '' - self.init = None - self.fragments = deque() + self.init: MediaFragment | None = None + self.fragments: deque[MediaFragment] = deque() self.sequence = 0 def _write(self, name: str, data: bytes): @@ -139,14 +149,17 @@ class MediaStore: Path(previous['mediaPath']).parent.rmdir() def publish(self, data: bytes, *, position: float, keyframe: bool): + init = self.init + if init is None: + raise ValueError('Replay media has not been initialized') if not self.fragments and not keyframe: raise ValueError('Replay media must begin with a keyframe') self.sequence += 1 - item = {'sequence': self.sequence, 'mediaPath': self._write(f'{self.sequence}.m4s', data), 'length': len(data), - 'firstPosition': position, 'lastPosition': position, 'keyframe': bool(keyframe)} + item: MediaFragment = {'sequence': self.sequence, 'mediaPath': self._write(f'{self.sequence}.m4s', data), 'length': len(data), + 'firstPosition': position, 'lastPosition': position, 'keyframe': bool(keyframe)} self.fragments.append(item) while (len(self.fragments) > MAX_FRAGMENTS or - sum(row['length'] for row in self.fragments) + self.init['length'] > MAX_MEDIA_BYTES): + sum(row['length'] for row in self.fragments) + init['length'] > MAX_MEDIA_BYTES): following = next((i for i, row in enumerate(self.fragments) if i and row['keyframe']), None) if following is None: raise RuntimeError('Replay GOP exceeds the media memory budget') diff --git a/openpilot/starpilot/replay/owner.py b/openpilot/starpilot/replay/owner.py index e3f9a276b3..2d8565ef70 100644 --- a/openpilot/starpilot/replay/owner.py +++ b/openpilot/starpilot/replay/owner.py @@ -21,6 +21,7 @@ import subprocess import sys import threading import time +from typing import cast import uuid from openpilot.starpilot.replay.client import MAX_REQUEST, MAX_RESPONSE, encode, parse, receive, replay_root @@ -346,7 +347,10 @@ class ReplayOwner: if self.status.get('target') != 'web' or request['epoch'] != self.status['epoch'] or not self.status.get('mediaReady'): raise Rejected('epoch_changed') from openpilot.starpilot.replay.media import media_descriptor - return media_descriptor(self.session_dir, request['epoch'], request['sequence'], count) + directory = self.session_dir + if directory is None: + raise Rejected('session_changed') + return media_descriptor(directory, request['epoch'], request['sequence'], count) def _stop(self, reason): if self.worker is not None: @@ -396,8 +400,10 @@ class ReplayOwner: ranges = value.get('seekRanges', []) if (type(ranges) is not list or len(ranges) > 512 or any(type(item) is not list or len(item) != 2 or not all(_finite(number) for number in item) or - not value['start'] <= item[0] <= item[1] <= value['end'] for item in ranges) or - any(before[1] > after[0] for before, after in zip(ranges, ranges[1:], strict=False))): + not value['start'] <= item[0] <= item[1] <= value['end'] for item in ranges)): + raise Rejected('worker_failed') + ranges = cast(list[list[float]], ranges) + if any(before[1] > after[0] for before, after in zip(ranges, ranges[1:], strict=False)): raise Rejected('worker_failed') self.status['seekRanges'] = ranges warnings = value.get('sourceWarnings', []) @@ -532,6 +538,7 @@ class ReplayOwner: with self.lock: self.child, self.child_birth = child, _process_birth(child.pid) self._record_session(directory, child) + assert child.stdout is not None and child.stderr is not None and child.stdin is not None for file in (child.stdout, child.stderr, child.stdin): os.set_blocking(file.fileno(), False) stdout = bytearray() diff --git a/openpilot/starpilot/replay/render.py b/openpilot/starpilot/replay/render.py index 80e968e013..0270b70833 100644 --- a/openpilot/starpilot/replay/render.py +++ b/openpilot/starpilot/replay/render.py @@ -13,10 +13,14 @@ import subprocess import sys import threading import time +from typing import TYPE_CHECKING, cast from openpilot.starpilot.replay.frames import ReplayFramePublisher, SESSION from openpilot.starpilot.replay.media import MediaMux, MediaStore +if TYPE_CHECKING: + from openpilot.selfdrive.ui.mici.onroad.augmented_road_view import AugmentedRoadView as CompactRoadView + SIZES = {'native': {'large': (1600, 800), 'compact': (536, 240)}, 'web': {'large': (1280, 640), 'compact': (536, 240)}} FRAME_INTERVAL = 0.05 @@ -51,7 +55,7 @@ def frame_matches(client, frame, state, stamp: int, now_ns: int | None) -> bool: def recorded_wall_clock(sample, wall_minus_mono_ns: int | None): - if sample is None or not sample.valid or type(wall_minus_mono_ns) is not int: + if sample is None or not sample.valid or sample.now_ns is None or type(wall_minus_mono_ns) is not int: return None try: return datetime.fromtimestamp((sample.now_ns + wall_minus_mono_ns) / 1e9).astimezone() @@ -170,6 +174,7 @@ class Engine: environment = dict(os.environ, SP_REPLAY_PARENT_PID=str(os.getpid())) self.process = subprocess.Popen(command, env=environment, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=sys.stderr, text=True, bufsize=1, close_fds=True) + assert self.process.stdin is not None and self.process.stdout is not None self.status = None self.error = None self.write_lock = threading.Lock() @@ -299,7 +304,7 @@ class Compositor: self.resources.callback(rl.unload_render_texture, self.output) self.readback = FrameReadback(width * height * (3 if nv12 else 8) // 2, asynchronous=True) self.resources.callback(self.readback.close) - self.meta = None + self.meta: tuple[int, float, int] | None = None self.epoch = None self.last_route_frame = None self.primary_ready = False @@ -343,17 +348,19 @@ class Compositor: for name, receipt in receipts.items()} sample = self.ui.replay_sample updates['extrinsicsCalibration'] = bool(updates['extrinsicsCalibration'] and sample and sample.valid and + sample.now_ns is not None and sm.valid['extrinsicsCalibration'] and 0 < sm.logMonoTime['extrinsicsCalibration'] <= sample.now_ns) flags = DrawUpdates(updates) sm.updated = flags try: if self.profile == 'compact': - self.camera.render_camera_model_layer(rect, camera_view=state.appearance.camera_view, - reverse_driver=state.reverse_driver_camera, - lead_indicator=state.appearance.show_lead_indicator and state.lead_indicator_source_fresh, - lead_info_mode=state.appearance.lead_info_mode, lead_info_metric=state.appearance.lead_info_metric, - lateral_active=state.lateral_active, road_style=state.customization['roadColors']['compact']) + camera = cast('CompactRoadView', self.camera) + camera.render_camera_model_layer(rect, camera_view=state.appearance.camera_view, + reverse_driver=state.reverse_driver_camera, + lead_indicator=state.appearance.show_lead_indicator and state.lead_indicator_source_fresh, + lead_info_mode=state.appearance.lead_info_mode, lead_info_metric=state.appearance.lead_info_metric, + lateral_active=state.lateral_active, road_style=state.customization['roadColors']['compact']) else: self.camera.render_camera_model_layer(rect, road_style=state.customization['roadColors']['large']) finally: @@ -384,7 +391,7 @@ class Compositor: name = names.get(self.camera.stream_type) sm, sample = self.ui.sm, self.ui.replay_sample state = sm[name] if name is not None and sm.seen[name] and sm.valid[name] else None - return bool(sample and sample.valid and state is not None and frame_matches( + return bool(sample and sample.valid and name is not None and state is not None and frame_matches( self.camera.client, self.camera.frame, state, sm.logMonoTime[name], sample.boot_ns)) def _receive_primary(self): @@ -400,7 +407,8 @@ class Compositor: from openpilot.starpilot.ui.runtime_snapshot import display_message from openpilot.starpilot.ui.presentation import FontRole saved = self.pip_saved - if saved.enabled is not True or saved.mask is None or None in (saved.on_blinker, saved.on_bsm, saved.invert): + if (saved.enabled is not True or saved.mask is None or + saved.on_blinker is None or saved.on_bsm is None or saved.invert is None): self.pip.deactivate() return now = state.observed_ns or 0 @@ -501,6 +509,8 @@ class Compositor: def draw(self, epoch: int, position: float, now_ns: int): sample = self.ui.replay_sample + if sample is None or sample.now_ns is None: + return False key = epoch, sample.now_ns if key == self.last_route_frame: return False @@ -547,7 +557,8 @@ class Compositor: try: pixels = self.readback.finish() sample = self.ui.replay_clock.sample() - yield (pixels, self.meta) if sample.valid and sample.epoch == self.meta[0] else None + meta = self.meta + yield (pixels, meta) if meta is not None and sample.valid and sample.epoch == meta[0] else None finally: self.readback.release() @@ -631,7 +642,7 @@ def run(session_dir: Path, route_id: str, data_dir: Path, target: str, profile: core.wall_minus_mono_ns = parse_wall_offset(status.get('wallMinusMonoNs')) sample = core.update() now = time.monotonic() - valid = bool(status and sample is not None and sample.valid and sample.epoch == status.get('epoch')) + valid = bool(status and sample is not None and sample.valid and sample.now_ns is not None and sample.epoch == status.get('epoch')) ready_frames = [] if pending_frame is not None and (not status or pending_frame[2] != status.get('epoch')): pending_frame = None @@ -679,12 +690,13 @@ def run(session_dir: Path, route_id: str, data_dir: Path, target: str, profile: if engine.command({'op': 'seek', 'position': status['position'], 'requestId': status.get('lastRequestId') or '0' * 32}, reprime=True): primed_clients.update(unprimed) valid = False - if valid and getattr(core.ui, 'replay_needs_reprime', False): + if valid and status is not None and getattr(core.ui, 'replay_needs_reprime', False): if engine.command({'op': 'seek', 'position': status['position'], 'requestId': status.get('lastRequestId') or '0' * 32}, reprime=True): core.ui.replay_needs_reprime = False valid = False interval = FRAME_INTERVAL - if valid and not core.readback.pending and now - last_draw >= interval: + if (valid and status is not None and sample is not None and sample.now_ns is not None and + not core.readback.pending and now - last_draw >= interval): origin = status.get('routeStartNs') if type(origin) is not int: raise RuntimeError('Replay engine did not expose its route clock origin') diff --git a/openpilot/starpilot/replay/tests/test_catalog.py b/openpilot/starpilot/replay/tests/test_catalog.py index efa1eaa0b2..a2bee48fb2 100644 --- a/openpilot/starpilot/replay/tests/test_catalog.py +++ b/openpilot/starpilot/replay/tests/test_catalog.py @@ -229,6 +229,7 @@ class ReplayCatalogTest(unittest.TestCase): uploader.listdir_by_creation = lambda root: sorted(os.listdir(root)) path = Path(__file__).resolve().parents[3] / 'system/loggerd/deleter.py' spec = importlib.util.spec_from_file_location('replay_fixture_deleter', path) + assert spec is not None and spec.loader is not None deleter = importlib.util.module_from_spec(spec) with patch.dict(sys.modules, {'openpilot.system.loggerd.uploader': uploader}): spec.loader.exec_module(deleter) diff --git a/openpilot/starpilot/replay/tests/test_frames.py b/openpilot/starpilot/replay/tests/test_frames.py index 73df499955..ed2fa09790 100644 --- a/openpilot/starpilot/replay/tests/test_frames.py +++ b/openpilot/starpilot/replay/tests/test_frames.py @@ -18,6 +18,7 @@ def test_coherent_epoch_and_viewer_close_preserves_owned_file(tmp_path): publisher.publish(b'\x22' * 32, epoch=2, position=2, captured_ns=456) assert reader.latest().epoch == 2 reader.close() + assert reader.latest() is None assert path.exists() finally: reader.close() @@ -75,6 +76,7 @@ def test_kernel_lock_prevents_copying_pixels_while_writer_is_active(tmp_path): assert reader.latest() is None fcntl.flock(publisher.fd, fcntl.LOCK_UN) assert reader.latest().data == b'a' * 32 + assert reader.fd is not None fcntl.flock(reader.fd, fcntl.LOCK_SH | fcntl.LOCK_NB) with pytest.raises(RuntimeError, match='consumer stalled'): publisher.publish(b'b' * 32, epoch=2, position=1, captured_ns=2) diff --git a/openpilot/starpilot/replay/tests/test_media.py b/openpilot/starpilot/replay/tests/test_media.py index 31a3277027..b0b630a381 100644 --- a/openpilot/starpilot/replay/tests/test_media.py +++ b/openpilot/starpilot/replay/tests/test_media.py @@ -14,6 +14,14 @@ def store(tmp_path): return MediaStore(session, session.name, 1280, 640) +def test_publish_requires_initialization_without_creating_fragments(tmp_path): + media = store(tmp_path) + with pytest.raises(ValueError, match='initialized'): + media.publish(b'frame', position=0., keyframe=True) + assert media.sequence == 0 and not media.fragments + assert not media.manifest.exists() and not list(media.root.iterdir()) + + def test_bounded_fragments_evict_whole_gop_and_keep_init(tmp_path): media = store(tmp_path) media.begin(1, 'avc1.42c01f', b'initialization') @@ -96,7 +104,8 @@ def test_mux_copy_has_explicit_pts_and_independent_initialization(tmp_path): ffmpeg = root / '.venv/lib/python3.12/site-packages/ffmpeg/install/bin/ffmpeg' ffprobe = ffmpeg.with_name('ffprobe') if not library.exists() or not ffmpeg.exists() or not ffprobe.exists(): - pytest.skip('Host replay muxer and bundled FFmpeg must be built') + # shortcut: ty drops pytest's reason argument; remove the ignore when its callable binding is fixed. + pytest.skip('Host replay muxer and bundled FFmpeg must be built') # ty: ignore[too-many-positional-arguments] source = tmp_path / 'fixture.ts' subprocess.run([str(ffmpeg), '-nostdin', '-hide_banner', '-loglevel', 'error', '-f', 'rawvideo', '-pixel_format', 'yuv420p', '-video_size', '32x18', '-framerate', '20', '-i', 'pipe:0', diff --git a/openpilot/starpilot/replay/tests/test_owner.py b/openpilot/starpilot/replay/tests/test_owner.py index b46a2c322b..5ceb361da5 100644 --- a/openpilot/starpilot/replay/tests/test_owner.py +++ b/openpilot/starpilot/replay/tests/test_owner.py @@ -371,6 +371,15 @@ def test_strict_revision_numbers_and_request_identity(owner): value.dispatch(dict(control(value, 'pause', 'paused', False, '7' * 32), expectedRevision=accepted['revision'] - 1)) +@pytest.mark.parametrize('ranges', [None, [[0]], [[0, True]], [[0, float('nan')]], [[20, 10]], + [[0, 101]], [[0, 20], [10, 30]]]) +def test_worker_seek_ranges_reject_malformed_or_overlapping_bounds(owner, tmp_path, ranges): + value, _, _, _ = owner + status = {**value._idle(), 'event': 'status', 'state': 'paused', 'end': 100., 'seekRanges': ranges} + with pytest.raises(Rejected, match='worker failed'): + value._observe(status, tmp_path) + + def test_gap_seek_is_rejected_without_losing_owned_viewer(owner): value, _, _, children = owner start(value) @@ -393,7 +402,8 @@ def test_real_machine_uses_owner_created_clock_and_cleans_it(tmp_path, monkeypat binary = Path(BASEDIR) / 'openpilot/tools/replay/replay' if not binary.is_file(): - pytest.skip('Build the isolated replay engine first') + # shortcut: ty drops pytest's reason argument; remove the ignore when its callable binding is fixed. + pytest.skip('Build the isolated replay engine first') # ty: ignore[too-many-positional-arguments] routes = tmp_path / 'routes' routes.mkdir() segment(routes, 0, [.1, .2, 10], compressed=True) diff --git a/openpilot/starpilot/replay/tests/test_render.py b/openpilot/starpilot/replay/tests/test_render.py index 31185f2d7a..1bb402988c 100644 --- a/openpilot/starpilot/replay/tests/test_render.py +++ b/openpilot/starpilot/replay/tests/test_render.py @@ -1,12 +1,14 @@ from contextlib import nullcontext from dataclasses import dataclass from types import SimpleNamespace +from typing import Any from unittest.mock import Mock import pytest from openpilot.starpilot.replay import render from openpilot.starpilot.replay.render import Compositor, Engine, camera_options, frame_matches, parse_wall_offset +from openpilot.starpilot.ui.presentation import Profile def test_camera_selection_uses_selected_sources_and_current_preferences(): @@ -49,7 +51,7 @@ def test_camera_matches_recorded_identity_across_distinct_clock_domains(): def test_paused_same_route_clock_never_advances_filters_or_gpu(): - core = object.__new__(Compositor) + core: Any = object.__new__(Compositor) core.ui = SimpleNamespace(replay_sample=SimpleNamespace(now_ns=100)) core.last_route_frame = (2, 100) core.context = Mock() @@ -57,6 +59,15 @@ def test_paused_same_route_clock_never_advances_filters_or_gpu(): core.context.begin_frame.assert_not_called() +@pytest.mark.parametrize('sample', [None, SimpleNamespace(now_ns=None)]) +def test_missing_route_clock_never_submits_a_gpu_frame(sample): + core: Any = object.__new__(Compositor) + core.ui = SimpleNamespace(replay_sample=sample) + core.context = Mock() + assert core.draw(1, 0., 100) is False + core.context.begin_frame.assert_not_called() + + @pytest.mark.parametrize('frame_ready', (False, True)) def test_recorded_camera_preview_does_not_require_initial_device_state(frame_ready): @dataclass @@ -64,7 +75,7 @@ def test_recorded_camera_preview_does_not_require_initial_device_state(frame_rea camera_available: bool = False viewport_width: int = 0 - core = object.__new__(Compositor) + core: Any = object.__new__(Compositor) core.ui = SimpleNamespace(replay_sample=SimpleNamespace(now_ns=100), sm=SimpleNamespace(seen={'extrinsicsCalibration': False})) core.last_route_frame = None @@ -77,7 +88,7 @@ def test_recorded_camera_preview_does_not_require_initial_device_state(frame_rea core.gui = SimpleNamespace(_render_prepare=[], _frame=0) core.content = Mock() core.scale = 1 - core.profile = SimpleNamespace(size=(1600, 800)) + core.profile = Profile.LARGE core.view = Mock() core.view.render.side_effect = lambda state: setattr(core, 'primary_ready', state.camera_available) core.output = None @@ -225,7 +236,7 @@ def test_published_readiness_survives_delivery_jitter_but_expires_and_rejects_ol def test_epoch_reset_preserves_native_camera_stream_and_eligible_paused_frame(monkeypatch, matching_frame): monkeypatch.setattr('openpilot.starpilot.ui.runtime_snapshot.RuntimeSnapshotAdapter', Mock()) monkeypatch.setattr('openpilot.starpilot.ui.pip_render.PiPRenderer', Mock()) - core = object.__new__(Compositor) + core: Any = object.__new__(Compositor) streams = {0, 2} client = Mock(frame_id=5 if matching_frame else 4, timestamp_sof=9_000_000_000, timestamp_eof=9_010_000_000) client.is_connected.return_value = True @@ -316,6 +327,7 @@ def test_recorded_wall_clock_uses_only_verified_route_pair(): sample = SimpleNamespace(valid=True, now_ns=10_000_000_000) assert recorded_wall_clock(sample, None) is None assert recorded_wall_clock(SimpleNamespace(valid=False), 1_000_000_000) is None + assert recorded_wall_clock(SimpleNamespace(valid=True, now_ns=None), 1_000_000_000) is None instant = recorded_wall_clock(sample, 1_000_000_000_000_000_000) assert instant.timestamp() == 1_000_000_010 @@ -333,7 +345,7 @@ def test_camera_reprime_cannot_overwrite_an_unacknowledged_user_action(): import io import json import threading - engine = object.__new__(Engine) + engine: Any = object.__new__(Engine) engine.write_lock = threading.Lock() engine.process = SimpleNamespace(stdin=io.StringIO()) engine.last_request_id = engine.seek_request = None @@ -384,9 +396,9 @@ def test_replay_draw_retains_updates_until_stock_consumers_read_them(monkeypatch camera._target_stream_type, camera._switching = None, False camera.available_streams, camera.frame = {camera.stream_type}, object() camera.model_renderer = camera._model_renderer = renderer - core = object.__new__(Compositor) + core: Any = object.__new__(Compositor) core.ui, core.camera = ui, camera - core.profile = 'compact' if profile == 'mici.onroad' else 'large' + core.profile = Profile.COMPACT if profile == 'mici.onroad' else Profile.LARGE core.cabin = core.wide = False core.render_receipts = {} core._primary_matches = Mock(return_value=True) @@ -474,19 +486,20 @@ def test_replay_draw_retains_updates_until_stock_consumers_read_them(monkeypatch assert sm.updated is original and core.render_receipts == before -@pytest.mark.parametrize('valid,epoch', [(True, 1), (False, 1), (True, 2)]) -def test_finished_frame_is_borrowed_until_delivery_finishes(valid, epoch): - core = object.__new__(Compositor) +@pytest.mark.parametrize('valid,epoch,meta', [(True, 1, (1, 0., 100)), (False, 1, (1, 0., 100)), + (True, 2, (1, 0., 100)), (True, 1, None)]) +def test_finished_frame_is_borrowed_until_delivery_finishes(valid, epoch, meta): + core: Any = object.__new__(Compositor) pixels = memoryview(bytearray(b'pixels')) core.readback = Mock(pending=True) core.readback.finish.return_value = pixels core.ui = SimpleNamespace(replay_clock=Mock()) core.ui.replay_clock.sample.return_value = SimpleNamespace(valid=valid, epoch=epoch) - core.meta = 1, 0., 100 + core.meta = meta with pytest.raises(RuntimeError, match='delivery failed'): with core.finish() as finished: core.readback.release.assert_not_called() - assert finished == ((pixels, core.meta) if valid and epoch == 1 else None) + assert finished == ((pixels, meta) if meta is not None and valid and epoch == 1 else None) if finished: assert finished[0] is pixels raise RuntimeError('delivery failed') diff --git a/openpilot/starpilot/ui/pip_render.py b/openpilot/starpilot/ui/pip_render.py index f03a59a57f..a6a9b1a6af 100644 --- a/openpilot/starpilot/ui/pip_render.py +++ b/openpilot/starpilot/ui/pip_render.py @@ -3,6 +3,7 @@ from __future__ import annotations import time +from typing import TYPE_CHECKING import pyray as rl @@ -12,6 +13,9 @@ from openpilot.starpilot.ui.pip_shaders import PIP_CURVED_FRAGMENT_SHADER, PIP_F from openpilot.starpilot.ui.pip_sidecam import Mask, PiPStream, Rect, Signals, bubble_rect, selected_sides from openpilot.system.ui.lib.egl import EGLImage, bind_egl_image_to_texture, create_egl_image, destroy_egl_image, init_egl +if TYPE_CHECKING: + from openpilot.starpilot.replay.render import ReplayCameraStream + def _external_shader(fragment: str) -> str: """Retain the frozen mask/crop, sampling CameraView's AGNOS EGL RGB image.""" @@ -29,7 +33,7 @@ def _external_shader(fragment: str) -> str: class PiPRenderer: """One renderer; close it on onroad exit or when the owning shell closes.""" - def __init__(self, shape: str, stream: PiPStream | None = None, *, frame_availability=None): + def __init__(self, shape: str, stream: PiPStream | ReplayCameraStream | None = None, *, frame_availability=None): if shape not in ("bubble", "curved"): raise ValueError("unknown PiP shape") self.frame_availability = frame_availability if frame_availability is not None else CameraAvailability() diff --git a/openpilot/starpilot/ui/replay_settings.py b/openpilot/starpilot/ui/replay_settings.py index 97ed601e90..0283128276 100644 --- a/openpilot/starpilot/ui/replay_settings.py +++ b/openpilot/starpilot/ui/replay_settings.py @@ -152,7 +152,7 @@ class ReplayController: if type(position) not in (int, float) or not math.isfinite(position): return False start, end = self.status.get('start'), self.status.get('end') - if type(start) not in (int, float) or type(end) not in (int, float) or end <= start: + if start is None or end is None or type(start) not in (int, float) or type(end) not in (int, float) or end <= start: return False position = max(start, min(end, position)) ranges = self.status.get('seekRanges') @@ -305,7 +305,7 @@ class ReplayPage(Widget): self.last_touch = time.monotonic() self.stopping = False self._epoch = None - self.stack_widget = self + self.stack_widget: Widget = self def show_event(self): super().show_event() @@ -676,12 +676,12 @@ class ReplayPage(Widget): start, end = self.controller.status.get('start') or 0, self.controller.status.get('end') or 0 self.seek_preview = start + max(0, min(1, (x - left) / width)) * (end - start) - def _handle_mouse_event(self, event): - if event.left_released and not rl.check_collision_point_rec(event.pos, self._hit_rect): + def _handle_mouse_event(self, mouse_event): + if mouse_event.left_released and not rl.check_collision_point_rec(mouse_event.pos, self._hit_rect): self.press = self.seek_preview = None self.input.cancel() - if event.left_down: - x, y = event.pos.x - self.rect.x, event.pos.y - self.rect.y + if mouse_event.left_down: + x, y = mouse_event.pos.x - self.rect.x, mouse_event.pos.y - self.rect.y if self.press: px, py, target, epoch, _ = self.press tolerance = 36 if self.profile == Profile.LARGE else 5 @@ -690,7 +690,7 @@ class ReplayPage(Widget): else: self._move_seek(x) elif self.view: - self.input.move(x, y, self._state(), event.t) + self.input.move(x, y, self._state(), mouse_event.t) def _handle_mouse_release(self, pos): x, y = pos.x - self.rect.x, pos.y - self.rect.y @@ -738,10 +738,10 @@ class ReplayCompactPage(NavWidget): def _back_enabled(self): return self.content.page != 'viewer' - def _handle_mouse_event(self, event): + def _handle_mouse_event(self, mouse_event): was_dismissing = self._playing_dismiss_animation - super()._handle_mouse_event(event) - if event.left_released and not was_dismissing and self._playing_dismiss_animation and self.content.page == 'detail': + super()._handle_mouse_event(mouse_event) + if mouse_event.left_released and not was_dismissing and self._playing_dismiss_animation and self.content.page == 'detail': self._playing_dismiss_animation = False self.content._back() diff --git a/openpilot/starpilot/ui/tests/test_replay_settings.py b/openpilot/starpilot/ui/tests/test_replay_settings.py index 8dfeb331d8..1c924d4127 100644 --- a/openpilot/starpilot/ui/tests/test_replay_settings.py +++ b/openpilot/starpilot/ui/tests/test_replay_settings.py @@ -3,6 +3,7 @@ from types import SimpleNamespace as NS import threading import time +from typing import Any from unittest.mock import Mock import pyray as rl @@ -99,10 +100,10 @@ def test_details_show_source_timezone_and_full_id_without_reformatting_each_fram class Backend: def __init__(self): self.calls = [] - self.status = {'schemaVersion': 1, 'ownerId': 'a' * 32, 'sessionId': None, 'state': 'idle', - 'routeId': None, 'profile': 'large', 'revision': 0, 'epoch': 0, - 'position': 0.0, 'start': 0.0, 'end': 120.0, 'rate': 1.0, 'paused': True, - 'frameReady': False, 'errorCode': None, 'owned': False, 'offroad': True} + self.status: dict[str, Any] = {'schemaVersion': 1, 'ownerId': 'a' * 32, 'sessionId': None, 'state': 'idle', + 'routeId': None, 'profile': 'large', 'revision': 0, 'epoch': 0, + 'position': 0.0, 'start': 0.0, 'end': 120.0, 'rate': 1.0, 'paused': True, + 'frameReady': False, 'errorCode': None, 'owned': False, 'offroad': True} self.start_gate = None def __call__(self, payload): @@ -142,7 +143,8 @@ def pump(owner, predicate): if predicate(): return time.sleep(0.001) - pytest.fail('fake replay operation did not finish') + # shortcut: ty drops pytest's reason argument; remove the ignore when its callable binding is fixed. + pytest.fail('fake replay operation did not finish') # ty: ignore[invalid-argument-type] @pytest.fixture @@ -175,6 +177,15 @@ def test_owned_start_and_controls_bind_revision_and_client(playback): assert not owner.seek(float('nan')) +@pytest.mark.parametrize('bounds', [(None, 120), (0, None), (False, 120), (0, True), ('0', 120), (120, 0)]) +def test_seek_rejects_missing_or_invalid_bounds_without_sending_controls(playback, bounds): + owner, backend, _, _ = playback + owner.status['start'], owner.status['end'] = bounds + count = len(backend.calls) + assert not owner.seek(30) + assert len(backend.calls) == count + + @pytest.fixture(params=[Profile.LARGE, Profile.COMPACT]) def prepared_playback(request): backend = Backend() @@ -335,7 +346,7 @@ def page_for_input(): def test_timeline_commits_only_on_release_and_epoch_change_cancels(): page, controller = page_for_input() page._handle_mouse_press(NS(x=100, y=940)) - page._handle_mouse_event(NS(left_down=True, left_released=False, pos=NS(x=1570, y=940), t=1)) + page._handle_mouse_event(mouse_event=NS(left_down=True, left_released=False, pos=NS(x=1570, y=940), t=1)) controller.seek.assert_not_called() page._handle_mouse_release(NS(x=1570, y=940)) controller.seek.assert_called_once_with(90) @@ -753,7 +764,7 @@ def test_compact_swipe_down_goes_back_one_level_and_is_blocked_during_playback(m page.content.page = page_name page.content.press = (200, 40, 'held button', 1, True) for y, pressed, down, released in [(40, True, True, False), (160, False, True, False), (160, False, False, True)]: - page._handle_mouse_event(NS(pos=NS(x=200, y=y), left_pressed=pressed, left_down=down, left_released=released)) + page._handle_mouse_event(mouse_event=NS(pos=NS(x=200, y=y), left_pressed=pressed, left_down=down, left_released=released)) assert page._playing_dismiss_animation is dismisses assert page.content.page == ('routes' if returns_to_routes else page_name) if returns_to_routes: diff --git a/openpilot/starpilot/ui/tests/test_ui_started_authority.py b/openpilot/starpilot/ui/tests/test_ui_started_authority.py index 7e81bcae16..3fd08fb880 100644 --- a/openpilot/starpilot/ui/tests/test_ui_started_authority.py +++ b/openpilot/starpilot/ui/tests/test_ui_started_authority.py @@ -3,6 +3,7 @@ from unittest.mock import MagicMock from openpilot.cereal.messaging import SubMaster from types import SimpleNamespace as NS +from typing import Any from unittest.mock import patch import pytest @@ -119,7 +120,7 @@ def test_started_presentation_does_not_replace_existing_message_freshness(): def replay_owner(sample): from openpilot.starpilot.ui.tests.test_runtime_snapshot import SubMasterFake - ui = object.__new__(UIState) + ui: Any = object.__new__(UIState) ui.sm = SubMasterFake() ui.sm.sock = {name: object() for name in ('carControl', 'carState', 'selfdriveState')} ui.sm.static_freq_services = set(ui.sm.sock) @@ -240,6 +241,7 @@ def test_replay_boot_time_camera_primer_retention_and_freshness(): from openpilot.starpilot.ui.tests.test_runtime_snapshot import NOW from openpilot.starpilot.ui.runtime_snapshot import display_message sample = ClockSample(NOW, NOW + 2_000_000_000, NOW, epoch=2, paused=True, valid=True) + assert sample.boot_ns is not None ui = replay_owner(sample) ui.sm.sock['cabinCameraState'] = object() ui.sm.static_freq_services.add('cabinCameraState') diff --git a/openpilot/tools/replay/tests/test_machine.py b/openpilot/tools/replay/tests/test_machine.py index 880b58a97b..d502fd7ac1 100644 --- a/openpilot/tools/replay/tests/test_machine.py +++ b/openpilot/tools/replay/tests/test_machine.py @@ -97,7 +97,9 @@ class Machine: self.process = subprocess.Popen(self.command, env=self.env, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=self.stderr, text=True) def reader(): - for line in self.process.stdout: + stdout = self.process.stdout + assert stdout is not None + for line in stdout: try: self.statuses.put(json.loads(line)) except ValueError as error: @@ -275,6 +277,7 @@ class TestMachine(unittest.TestCase): @unittest.skipUnless(FFMPEG, "installed system ffmpeg fixture encoder required") def test_shared_decoder_returns_correct_pixels(self): + assert FFMPEG is not None native_test = BINARY.parent / "tests/test_route" self.assertTrue(native_test.is_file()) with tempfile.TemporaryDirectory(prefix="replay-decode-") as temporary: diff --git a/scripts/lint/check_added_large_files.py b/scripts/lint/check_added_large_files.py index 667122fd40..e5c6ea792d 100755 --- a/scripts/lint/check_added_large_files.py +++ b/scripts/lint/check_added_large_files.py @@ -6,6 +6,9 @@ import subprocess LARGE_TEXT_FILES = frozenset(( + # shortcut: these existing modules bypass 120 KiB; split when their responsibilities warrant it. + 'openpilot/selfdrive/selfdrived/selfdrived.py', + 'openpilot/starpilot/galaxy/server.py', 'openpilot/cereal/gen/cpp/car.capnp.c++', 'openpilot/cereal/gen/cpp/car.capnp.h', 'openpilot/cereal/gen/cpp/custom.capnp.c++',