From 6bb0f8b0c2e129dbd3c86e7e513da0bc7a5eb31a Mon Sep 17 00:00:00 2001 From: firestar5683 <168790843+firestar5683@users.noreply.github.com> Date: Thu, 8 Oct 2026 09:31:09 -0500 Subject: [PATCH] Experimental driving mode --- openpilot/selfdrive/selfdrived/selfdrived.py | 26 ++-- .../starpilot/conditional_mode/actions.py | 89 ++++++++++++ .../conditional_mode/runtime_settings.py | 67 +++++++-- .../tests/test_runtime_settings.py | 37 ++++- .../tests/test_status_consumer.py | 105 +++++++++++++- openpilot/starpilot/ui/conditional_feature.py | 64 +++++++-- .../ui/tests/test_conditional_feature.py | 130 +++++++++++++++++- 7 files changed, 483 insertions(+), 35 deletions(-) diff --git a/openpilot/selfdrive/selfdrived/selfdrived.py b/openpilot/selfdrive/selfdrived/selfdrived.py index 22ebbe5387..c54a28270b 100755 --- a/openpilot/selfdrive/selfdrived/selfdrived.py +++ b/openpilot/selfdrive/selfdrived/selfdrived.py @@ -38,7 +38,7 @@ from openpilot.starpilot.conditional_mode.policy import ModeChoice from openpilot.starpilot.conditional_mode.manual import ioniq6_media_eligible from openpilot.starpilot.controllers.mode_actions import SwitchbackStatusOwner, SwitchbackCooldown, ModeActionOwner from openpilot.starpilot.conditional_mode.projection import paired_clocks_ns -from openpilot.starpilot.conditional_mode.runtime_settings import ConditionalSettingsOwner +from openpilot.starpilot.conditional_mode.runtime_settings import (ConditionalSettingsOwner, experimental_requested, read_mode_sources) from openpilot.starpilot.conditional_mode.status import settings_fingerprint from openpilot.starpilot.feature_runtime import enabled as feature_enabled from openpilot.starpilot.nostalgia import aol_no_entry, paddle_cancel, physical_cancel, saved_enabled as nostalgia_saved_enabled @@ -795,9 +795,9 @@ class SelfdriveD: self.events_prev = self.events.names.copy() def update_conditional_mode(self, CS): - # The Params thread owns the stock request; only this thread publishes the - # effective choice. Conditional proposals cannot race a Params assignment. - self.experimental_mode = self.requested_experimental_mode + # Conditional document and baseline come from one accepted owner snapshot; + # only this thread publishes their effective choice. + self.experimental_mode = self.requested_experimental_mode if not self.conditional_replay else False self.conditional_result = ConsumerResult(self.experimental_mode, False, 'unavailable') if not self.conditional_replay: return @@ -813,6 +813,10 @@ class SelfdriveD: verdict = self.conditional_settings.verdict(snapshot, now_mono_ns=now_ns, drive_id=drive_id) configured = (verdict.status == 'ready' and verdict.selection is not None and snapshot is not None and verdict.revision == snapshot.revision) + baseline = bool(configured and experimental_requested(snapshot.experimental_raw, snapshot.experimental_readable, + snapshot.safe_mode_raw, True, self.CP)) + self.requested_experimental_mode = baseline + self.experimental_mode = baseline required = ('deviceState', 'modelV2', 'carControl') fresh = all(self.sm.seen[service] and self.sm.valid[service] and self.sm.alive[service] and 0 < self.sm.logMonoTime[service] <= now_ns and @@ -827,7 +831,7 @@ class SelfdriveD: self.sm['slcState'], now_ns=now_ns, now_boot_ns=boot_ns, sample_skew_ns=skew_ns, message_ns=int(self.sm.logMonoTime['slcState']), receipt_ns=int(self.sm.recv_time['slcState'] * 1e9), drive_id=drive_id, model_ns=int(self.sm.logMonoTime['modelV2']), car_state_ns=self.aol_car_state_log_ns, - authority=authority, stock_experimental=self.requested_experimental_mode, + authority=authority, stock_experimental=baseline, choice=verdict.selection.choice if configured else ModeChoice.STOCK, settings_fingerprint=settings_fingerprint(snapshot) if configured else None) self.experimental_mode = result.experimental @@ -891,6 +895,14 @@ class SelfdriveD: self.CS_prev = CS + def refresh_saved_driving_mode(self, now_ns: int): + if self.conditional_replay: + assert self.conditional_settings is not None + self.conditional_settings.refresh(now_ns, force=True, wait=True) + else: + _, (baseline, readable), (safe, safe_readable) = read_mode_sources(self.params, wait=True) + self.requested_experimental_mode = experimental_requested(baseline, readable, safe, safe_readable, self.CP) + def params_thread(self, evt): while not evt.is_set(): self.is_metric = self.params.get_bool("IsMetric") @@ -900,9 +912,7 @@ class SelfdriveD: self.disengage_on_accelerator = self.params.get_bool("DisengageOnAccelerator") self.radar_recovery_enabled = self.params.get_bool("RadarRecoveryAlert") self.nostalgia_enabled = nostalgia_saved_enabled(self.params) - self.requested_experimental_mode = self.params.get_bool("ExperimentalMode") and self.CP.openpilotLongitudinalControl - if self.conditional_settings is not None: - self.conditional_settings.refresh(time.monotonic_ns()) + self.refresh_saved_driving_mode(time.monotonic_ns()) self.personality = self.params.get("LongitudinalPersonality", return_default=True) time.sleep(0.1) diff --git a/openpilot/starpilot/conditional_mode/actions.py b/openpilot/starpilot/conditional_mode/actions.py index 1c30231a24..60aa951f53 100644 --- a/openpilot/starpilot/conditional_mode/actions.py +++ b/openpilot/starpilot/conditional_mode/actions.py @@ -113,6 +113,7 @@ class CommitResult: committed: bool verified: bool manual_cleared: bool = False + rollback_failed: bool = False def commit(params, raw: bytes, expected: bytes | None, expected_units: bytes | None, @@ -181,6 +182,94 @@ def commit(params, raw: bytes, expected: bytes | None, expected_units: bytes | N pass + +def commit_mode(params, raw: bytes, baseline: bytes, *, expected: bytes | None, expected_baseline: bytes | None, + expected_units: bytes | None, expected_safe: bytes | None, authorized: Callable[[], bool]) -> CommitResult: + """Commit the selector's document and baseline together for locked readers. + + Params.put and ordinary document commits use this same lock. If either + replacement fails, restore both original sources before releasing it. + """ + if baseline not in (b"0", b"1") or expected_baseline not in (None, b"0", b"1") or expected_safe not in (None, b"0", b"1"): + return CommitResult(False, False) + try: + if encode_preferences(decode_preferences(raw)) != raw or not authorized(): + return CommitResult(False, False) + except PreferenceError: + return CommitResult(False, False) + + def sources_match() -> bool: + return all(read_saved(params, key, limit) == (source, True) for key, limit, source in ( + (DOCUMENT_KEY, MAX_DOCUMENT_BYTES, expected), ("ExperimentalMode", 8, expected_baseline), + ("IsMetric", 8, expected_units), ("SafeMode", 8, expected_safe))) + + temporary: list[str] = [] + lock_fd = None + replaced = False + try: + destinations = (Path(params.get_param_path(DOCUMENT_KEY)), Path(params.get_param_path("ExperimentalMode"))) + root = destinations[0].parent.parent + + def stage(value: bytes | None) -> str | None: + if value is None: + return None + with tempfile.NamedTemporaryFile(prefix=".tmp_conditional_mode_", dir=root, delete=False) as source: + temporary.append(source.name) + source.write(value) + source.flush() + os.fsync(source.fileno()) + return source.name + + staged = (stage(raw), stage(baseline)) + rollback = (stage(expected), stage(expected_baseline)) + lock_fd = os.open(root / ".lock", os.O_CREAT | os.O_RDONLY, 0o775) + fcntl.flock(lock_fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + if not authorized() or not sources_match(): + return CommitResult(False, False) + directory_fd = os.open(destinations[0].parent, os.O_RDONLY) + try: + try: + for source, destination in zip(staged, destinations, strict=True): + assert source is not None + os.replace(source, destination) + replaced = True + os.fsync(directory_fd) + if not authorized() or read_saved(params, DOCUMENT_KEY, MAX_DOCUMENT_BYTES) != (raw, True) or \ + read_saved(params, "ExperimentalMode", 8) != (baseline, True): + raise OSError("Mode commit verification failed") + return CommitResult(True, True) + except (OSError, ValueError, TypeError): + if not replaced: + return CommitResult(False, False) + restored = True + for source, destination in zip(rollback, destinations, strict=True): + try: + if source is None: + destination.unlink(missing_ok=True) + else: + os.replace(source, destination) + except OSError: + restored = False + try: + os.fsync(directory_fd) + restored = restored and sources_match() + except OSError: + restored = False + return CommitResult(False, False, rollback_failed=not restored) + finally: + os.close(directory_fd) + except (OSError, ValueError, TypeError): + return CommitResult(False, False) + finally: + if lock_fd is not None: + os.close(lock_fd) + for source in temporary: + try: + os.unlink(source) + except OSError: + pass + + def commit_manual(params, *, expected_config: bytes | None, expected_units: bytes | None, expected_safe: bytes | None, expected_manual: bytes | None, authorized: Callable[[], bool], choice: ModeChoice | None, diff --git a/openpilot/starpilot/conditional_mode/runtime_settings.py b/openpilot/starpilot/conditional_mode/runtime_settings.py index 9e9b12b2a0..fb0697c052 100644 --- a/openpilot/starpilot/conditional_mode/runtime_settings.py +++ b/openpilot/starpilot/conditional_mode/runtime_settings.py @@ -9,6 +9,9 @@ from __future__ import annotations from dataclasses import dataclass from enum import StrEnum +import fcntl +import os +from pathlib import Path import secrets from typing import Any @@ -21,6 +24,7 @@ from openpilot.starpilot.saved_source import read_saved DOCUMENT_KEY = 'ConditionalModeConfig' SAFE_MODE_KEY = 'SafeMode' +EXPERIMENTAL_KEY = 'ExperimentalMode' REFRESH_NS = 1_000_000_000 REVERIFY_NS = REFRESH_NS // 2 @@ -51,6 +55,8 @@ class SettingsSnapshot: document_raw: bytes | None safe_mode_raw: bytes | None preferences: SavedPreferences | None + experimental_raw: bytes | None = None + experimental_readable: bool = True @dataclass(frozen=True) @@ -71,6 +77,37 @@ def _read(params: Any, key: str, limit: int) -> tuple[bytes | None, bool]: return b'', False + +def read_mode_sources(params: Any, *, wait: bool = False, defer_busy: bool = False) -> tuple[ + tuple[bytes | None, bool], tuple[bytes | None, bool], tuple[bytes | None, bool] +]: + """Read one coherent source pair. Only background callers may wait for a writer.""" + lock_fd = None + try: + root = Path(params.get_param_path(DOCUMENT_KEY)).parent.parent + lock_fd = os.open(root / '.lock', os.O_CREAT | os.O_RDONLY, 0o775) + fcntl.flock(lock_fd, fcntl.LOCK_SH if wait else fcntl.LOCK_SH | fcntl.LOCK_NB) + return (_read(params, DOCUMENT_KEY, MAX_DOCUMENT_BYTES), + _read(params, EXPERIMENTAL_KEY, 8), _read(params, SAFE_MODE_KEY, 8)) + except BlockingIOError: + if defer_busy: + raise + return (b'', False), (b'', False), (b'', False) + except (AttributeError, OSError, TypeError, ValueError): + return (b'', False), (b'', False), (b'', False) + finally: + if lock_fd is not None: + os.close(lock_fd) + + +def experimental_available(cp: Any) -> bool: + return bool(cp is not None and getattr(cp, 'openpilotLongitudinalControl', None) is True and + not getattr(cp, 'passive', False) and not getattr(cp, 'dashcamOnly', False) and not getattr(cp, 'notCar', False)) + + +def experimental_requested(raw: bytes | None, readable: bool, safe_raw: bytes | None, safe_readable: bool, cp: Any) -> bool: + return bool(readable and raw == b'1' and safe_readable and safe_raw in (None, b'0') and experimental_available(cp)) + def _document(raw: bytes | None, readable: bool) -> tuple[DocumentState, SavedPreferences | None]: if not readable: return DocumentState.READ_ERROR, None @@ -108,35 +145,41 @@ class ConditionalSettingsOwner: snapshot = SettingsSnapshot( self.owner_token, 1 if previous is None else previous.revision + 1, now_mono_ns, now_mono_ns, DocumentState.READ_ERROR, SafeModeState.READ_ERROR, - None, None, None, + None, None, None, None, False, ) self.current = snapshot self.last_refresh_ns = now_mono_ns return snapshot - def refresh(self, now_mono_ns: int) -> SettingsSnapshot: + def refresh(self, now_mono_ns: int, *, force: bool = False, wait: bool = False) -> SettingsSnapshot: """Perform a bounded disk read when due; errors invalidate prior authority.""" if type(now_mono_ns) is not int or now_mono_ns <= 0: return self._unavailable(0) if self.last_refresh_ns is not None and now_mono_ns < self.last_refresh_ns: return self._unavailable(now_mono_ns) - if self.current is not None and self.last_refresh_ns is not None and now_mono_ns - self.last_refresh_ns < REVERIFY_NS: + if not force and self.current is not None and self.last_refresh_ns is not None and now_mono_ns - self.last_refresh_ns < REVERIFY_NS: return self.current - document_raw, document_readable = _read(self.params, DOCUMENT_KEY, MAX_DOCUMENT_BYTES) - safe_raw, safe_readable = _read(self.params, SAFE_MODE_KEY, 8) + try: + sources = read_mode_sources(self.params, wait=wait, defer_busy=True) + except BlockingIOError: + # A paired write is not corrupt data. Keep the accepted revision while its + # original authority is live; neither read unlocked bytes nor renew its age. + return self.current if self.current is not None else self._unavailable(now_mono_ns) + (document_raw, document_readable), (experimental_raw, experimental_readable), (safe_raw, safe_readable) = sources document_state, preferences = _document(document_raw, document_readable) safe_state = _safe_mode(safe_raw, safe_readable) previous = self.current same_source = bool(previous is not None and previous.document_raw == document_raw and previous.safe_mode_raw == safe_raw and previous.document_state is document_state and - previous.safe_mode_state is safe_state and previous.preferences == preferences) + previous.safe_mode_state is safe_state and previous.preferences == preferences and + previous.experimental_raw == experimental_raw and previous.experimental_readable == experimental_readable) snapshot = SettingsSnapshot( self.owner_token, previous.revision if same_source and previous is not None else (1 if previous is None else previous.revision + 1), previous.observed_mono_ns if same_source and previous is not None else now_mono_ns, now_mono_ns, - document_state, safe_state, document_raw, safe_raw, preferences, + document_state, safe_state, document_raw, safe_raw, preferences, experimental_raw, experimental_readable, ) self.current = snapshot self.last_refresh_ns = now_mono_ns @@ -169,8 +212,8 @@ class ConditionalSettingsOwner: return SettingsVerdict(status, current.revision if current is not None else None, current.observed_mono_ns if current is not None else None, None, None, None, safe) - assert current is not None and current.preferences is not None - selection = saved_selection_for_drive(current.preferences, drive_id) - return SettingsVerdict('ready' if selection is not None else 'invalid_drive', current.revision, - current.observed_mono_ns, selection, current.preferences.cem, - current.preferences.ccm, safe) + assert snapshot is not None and snapshot.preferences is not None + selection = saved_selection_for_drive(snapshot.preferences, drive_id) + return SettingsVerdict('ready' if selection is not None else 'invalid_drive', snapshot.revision, + snapshot.observed_mono_ns, selection, snapshot.preferences.cem, + snapshot.preferences.ccm, False) diff --git a/openpilot/starpilot/conditional_mode/tests/test_runtime_settings.py b/openpilot/starpilot/conditional_mode/tests/test_runtime_settings.py index da1ebedcfe..9621ba10d1 100644 --- a/openpilot/starpilot/conditional_mode/tests/test_runtime_settings.py +++ b/openpilot/starpilot/conditional_mode/tests/test_runtime_settings.py @@ -1,6 +1,9 @@ """Actual temporary-Params receipts for the read-only conditional owner.""" from dataclasses import replace +import fcntl +import os +from types import SimpleNamespace import json from pathlib import Path import tempfile @@ -12,7 +15,7 @@ from openpilot.starpilot.conditional_mode.policy import ModeChoice from openpilot.starpilot.conditional_mode.preferences import CEMOptions, SavedPreferences, encode_preferences from openpilot.starpilot.conditional_mode.runtime_settings import ( DOCUMENT_KEY, REFRESH_NS, SAFE_MODE_KEY, ConditionalSettingsOwner, - DocumentState, SafeModeState, + DocumentState, SafeModeState, experimental_requested, read_mode_sources, ) @@ -29,6 +32,38 @@ class ConditionalRuntimeSettingsTest(unittest.TestCase): def saved(self, mode: ModeChoice = ModeChoice.CEM) -> SavedPreferences: return SavedPreferences(mode=mode, cem=replace(CEMOptions(), speed_mps=10.0)) + def test_mode_reader_excludes_inflight_pair_and_rejects_invalid_baseline(self): + self.params.put_bool("ExperimentalMode", True, block=True) + self.params.put_bool("SafeMode", False, block=True) + cp = SimpleNamespace(openpilotLongitudinalControl=True, passive=False) + first = self.owner.refresh(1_000_000_000) + self.assertTrue(experimental_requested(first.experimental_raw, first.experimental_readable, + first.safe_mode_raw, True, cp)) + lock = os.open(self.path(DOCUMENT_KEY).parent.parent / ".lock", os.O_RDONLY) + try: + fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) + self.assertEqual(read_mode_sources(self.params), ((b"", False), (b"", False), (b"", False))) + deferred = self.owner.refresh(1_100_000_000, force=True) + self.assertIs(deferred, first) + self.assertEqual(deferred.verified_mono_ns, 1_000_000_000) + self.assertTrue(self.owner.affirm(deferred, now_mono_ns=1_100_000_000)) + expired = self.owner.refresh(2_000_000_001, force=True) + self.assertIs(expired, first) + self.assertFalse(self.owner.affirm(expired, now_mono_ns=2_000_000_001)) + finally: + os.close(lock) + recovered = self.owner.refresh(1_200_000_000, force=True) + self.assertTrue(recovered.experimental_readable) + self.path("ExperimentalMode").write_bytes(b"01") + invalid = self.owner.refresh(1_300_000_000, force=True) + self.assertGreater(invalid.revision, recovered.revision) + self.assertFalse(experimental_requested(invalid.experimental_raw, invalid.experimental_readable, + invalid.safe_mode_raw, True, cp)) + self.assertEqual(self.path("ExperimentalMode").read_bytes(), b"01") + + def test_legacy_params_without_path_api_fail_closed(self): + self.assertEqual(read_mode_sources(SimpleNamespace()), ((b"", False), (b"", False), (b"", False))) + def test_absent_is_known_factory_cem_and_safe_mode_false(self): snapshot = self.owner.refresh(1_000_000_000) self.assertIs(snapshot.document_state, DocumentState.ABSENT) diff --git a/openpilot/starpilot/conditional_mode/tests/test_status_consumer.py b/openpilot/starpilot/conditional_mode/tests/test_status_consumer.py index c13a1db178..e7748d7308 100644 --- a/openpilot/starpilot/conditional_mode/tests/test_status_consumer.py +++ b/openpilot/starpilot/conditional_mode/tests/test_status_consumer.py @@ -6,6 +6,8 @@ from unittest.mock import patch import json import tempfile import unittest +import os +from pathlib import Path from openpilot.cereal import messaging from openpilot.common.params import Params @@ -16,7 +18,11 @@ from openpilot.starpilot.conditional_mode.policy import ManualIntent, ModeChoice from openpilot.starpilot.conditional_mode.preferences import SavedPreferences, encode_preferences, manual_for_drive from openpilot.starpilot.conditional_mode.runtime_settings import ConditionalSettingsOwner, DOCUMENT_KEY from openpilot.starpilot.conditional_mode.status import StatusPublisher, observation, settings_fingerprint -from openpilot.starpilot.conditional_mode.tests.test_projection import BOOT, CP, MONO, FakeSubMaster, serialized_scene +from openpilot.starpilot.conditional_mode.tests.test_projection import BOOT, CP, MONO, FakeSubMaster, serialized_scene, owner_context +from opendbc.car.honda.interface import CarInterface +from opendbc.car.honda.values import CAR +from openpilot.starpilot.ui.feature_settings_owner import FeatureSettingsOwner +from openpilot.starpilot.ui.feature_settings_state import FeaturePage, row_change class TestModeTransport(unittest.TestCase): @@ -32,8 +38,8 @@ class TestModeTransport(unittest.TestCase): self.produce(MONO) self.consume(None, MONO) - def produce(self, now, *, intent=ManualIntent.FORCE_EXPERIMENTAL, lat=False, long=True, outer=None): - sm = FakeSubMaster(serialized_scene(stamp=BOOT + now - MONO - 2_000_000), now - 2_000_000) + def produce(self, now, *, intent=ManualIntent.FORCE_EXPERIMENTAL, lat=False, long=True, outer=None, scene=None, context=None): + sm = FakeSubMaster(serialized_scene(stamp=BOOT + now - MONO - 2_000_000, **(scene or {})), now - 2_000_000) control = messaging.new_message('carControl') control.carControl.longActive = long control.carControl.latActive = lat @@ -47,6 +53,7 @@ class TestModeTransport(unittest.TestCase): drive_id=self.drive_id, settings_owner=self.owner, manual=manual_for_drive(intent, self.drive_id, self.drive_id + 1), + owner_context=context, selected_t_follow_s=1.45, selected_t_follow_observed_mono_ns=now - 2_000_000, ) @@ -74,6 +81,96 @@ class TestModeTransport(unittest.TestCase): values.update(changes) return self.consumer.sample(payload, **values) + def test_shared_chill_selector_preserves_committed_stop_in_actual_selfdrived(self): + cp = CarInterface.get_non_essential_params(CAR.HONDA_CIVIC_BOSCH) + cp.openpilotLongitudinalControl = True + ui = FeatureSettingsOwner(self.params, lambda group: group == 'preferences', + vehicle_fingerprint=lambda: cp.carFingerprint, vehicle_params=lambda: cp) + drive = SelfdriveD.__new__(SelfdriveD) + drive.params, drive.CP = self.params, cp + drive.conditional_replay = True + drive.conditional_settings, drive.conditional_consumer = self.owner, self.consumer + drive.conditional_car_state_valid = drive.initialized = drive.enabled = True + drive.aol_replay = False + scene = {'speed': 10.0, 'horizon': 40.0, 'lead_present': False} + for tick in range(1, 31): + now = MONO + tick * 50_000_000 + event, _, _ = self.produce(now, intent=ManualIntent.NONE, scene=scene, context=owner_context(now - 2_000_000)) + self.consume(event, now) + self.assertTrue(self.host.projector.stop_detector.committed) + row = next(row for row in ui.snapshot(FeaturePage.CONDITIONAL, parked=True, system_long=True, + lateral_context=False, metric=False).rows if row.key == 'conditional:mode') + request = row_change(row) + assert request is not None + accepted = self.owner.refresh(MONO + 30 * 50_000_000, force=True) + + def model_tick(tick, choice, after_verdict=None): + now = MONO + tick * 50_000_000 + event, proposal, sm = self.produce(now, intent=ManualIntent.FORCE_CHILL, scene=scene, + context=owner_context(now - 2_000_000)) + self.assertIs(proposal.choice, choice) + self.assertEqual(proposal.decision.reason.value, 'cem_stop') + self.assertTrue(event.slcState.conditionalMode.hasOverride) + device = messaging.new_message('deviceState', valid=True) + device.deviceState.started = True + device.deviceState.startedMonoTime = self.drive_id + sm.payloads['deviceState'] = messaging.log_from_bytes(device.to_bytes()).deviceState + sm.payloads['slcState'] = messaging.log_from_bytes(event.as_reader().as_builder().to_bytes()).slcState + for service in ('deviceState', 'slcState'): + sm.logMonoTime[service] = now + sm.recv_time[service] = now / 1e9 + sm.seen[service] = sm.alive[service] = sm.valid[service] = True + drive.sm, drive.aol_car_state_log_ns = sm, now - 2_000_000 + original_verdict = self.owner.verdict + + def interleaved_verdict(snapshot, **kwargs): + verdict = original_verdict(snapshot, **kwargs) + if after_verdict is not None: + after_verdict() + return verdict + + with patch('openpilot.selfdrive.selfdrived.selfdrived.paired_clocks_ns', return_value=(now, BOOT + now - MONO, 1000)), \ + patch.object(self.owner, 'verdict', side_effect=interleaved_verdict): + drive.update_conditional_mode(sm['carState']) + self.assertTrue(drive.conditional_result.accepted) + self.assertTrue(drive.experimental_mode) + self.assertTrue(self.host.projector.stop_detector.committed) + + original_replace = os.replace + sampled = False + + def sample_between_pair_replacements(source, destination): + nonlocal sampled + if Path(destination).name == 'ExperimentalMode': + sampled = True + # commit_mode holds the actual writer lock with its document already + # replaced. A model tick must keep the prior accepted pair and stop. + now = MONO + 31 * 50_000_000 + self.assertIs(self.owner.refresh(now, force=True), accepted) + self.assertEqual(self.owner.current.verified_mono_ns, accepted.verified_mono_ns) + model_tick(31, ModeChoice.CEM) + return original_replace(source, destination) + + with patch('openpilot.starpilot.conditional_mode.actions.os.replace', side_effect=sample_between_pair_replacements): + self.assertTrue(ui.apply(replace(request, value='Chill'))) + self.assertTrue(sampled) + self.assertFalse(self.params.get_bool('ExperimentalMode')) + drive.refresh_saved_driving_mode(MONO + 32 * 50_000_000) + def refresh_experimental_between_main_snapshot_and_baseline(): + row = next(row for row in ui.snapshot(FeaturePage.CONDITIONAL, parked=True, system_long=True, + lateral_context=False, metric=False).rows if row.key == 'conditional:mode') + change = row_change(row) + assert change is not None + self.assertTrue(ui.apply(replace(change, value='Experimental'))) + drive.refresh_saved_driving_mode(MONO + 32 * 50_000_000) + self.assertEqual(self.owner.current.experimental_raw, b'1') + + model_tick(32, ModeChoice.STOCK, after_verdict=refresh_experimental_between_main_snapshot_and_baseline) + self.assertFalse(drive.requested_experimental_mode) # Main consumed its captured Chill pair. + drive.refresh_saved_driving_mode(MONO + 33 * 50_000_000) + model_tick(33, ModeChoice.STOCK) + self.assertTrue(drive.requested_experimental_mode) + def test_both_directions_and_independent_longitudinal_authority(self): now = MONO + 50_000_000 event, proposal, _ = self.produce(now) @@ -156,6 +253,8 @@ class TestModeTransport(unittest.TestCase): drive.requested_experimental_mode = True drive.update_conditional_mode(None) self.assertTrue(drive.experimental_mode) + self.params.put_bool('ExperimentalMode', True, block=True) + self.owner.refresh(MONO, force=True) drive.conditional_replay = True drive.conditional_settings = self.owner drive.conditional_consumer = self.consumer diff --git a/openpilot/starpilot/ui/conditional_feature.py b/openpilot/starpilot/ui/conditional_feature.py index 00fc0506fd..22e0d3f55d 100644 --- a/openpilot/starpilot/ui/conditional_feature.py +++ b/openpilot/starpilot/ui/conditional_feature.py @@ -3,6 +3,7 @@ from __future__ import annotations from dataclasses import fields, replace +import hashlib from openpilot.starpilot.conditional_mode.button_actions import ( BUTTON_PREFIX, MEDIA_KEYS, DISTANCE_KEYS, capture_sources, commit_assignment, display_action, media_capability, @@ -10,7 +11,7 @@ from openpilot.starpilot.conditional_mode.button_actions import ( ) from openpilot.starpilot.conditional_mode.actions import ( - BOOLEAN_FIELDS, DOCUMENT_KEY, MANUAL_RESET, MODE, NUMBER_FIELDS, PREFIX, RESET, commit, commit_manual, + BOOLEAN_FIELDS, DOCUMENT_KEY, MANUAL_RESET, MODE, NUMBER_FIELDS, PREFIX, RESET, commit, commit_manual, commit_mode, display_number, edit, field_limit, stock_document, ) from openpilot.starpilot.conditional_mode.preferences import ( @@ -19,6 +20,7 @@ from openpilot.starpilot.conditional_mode.preferences import ( from openpilot.starpilot.saved_source import read_saved from openpilot.starpilot.conditional_mode.manual_saved import KEY as MANUAL_KEY, read_codes from openpilot.starpilot.conditional_mode.preferences import encode_preferences +from openpilot.starpilot.conditional_mode.runtime_settings import experimental_available, read_mode_sources from openpilot.starpilot.conditional_mode.policy import ModeChoice from openpilot.starpilot.ui.feature_settings_state import FeatureRow, FeatureSettingsRequest @@ -31,7 +33,7 @@ LABELS = { "signal_lane_detection": "Signal lane detection", "signal_lane_width_m": "Signal lane width", "set_speed_margin_mps": "Set speed margin", "launch_assist": "Launch assist", } -MODES = ("Chill", "Conditional Experimental", "Conditional Chill") +MODES = ("Chill", "Experimental", "Conditional Experimental", "Conditional Chill") SIGNAL_LANE_FIELDS = frozenset(("signal_lane_detection", "signal_lane_width_m")) SIGNAL_LANE_HELP = { "signal_lane_detection": "Distinguish a turn from a lane change using the space beside your car. With this on, a turn signal requests " + @@ -43,6 +45,17 @@ MEDIA_LABELS = ("MODE press", "MODE long press", "MODE very long press", "Star button", "Star button long press", "Star button very long press") + +def _mode_capability(cp) -> tuple | None: + if cp is None: + return None + try: + reader = cp.as_reader() if hasattr(cp, "as_reader") else cp + encoded = reader.as_builder().to_bytes() + return (str(cp.carFingerprint), hashlib.sha256(encoded).hexdigest()) + except (AttributeError, TypeError, ValueError, OverflowError): + return None + def _display(number: float) -> str: return f"{number:.2f}".rstrip("0").rstrip(".") if number else "0" @@ -147,12 +160,19 @@ class ConditionalFeature: manual_dependencies = dependencies + ((MANUAL_KEY, manual.raw), ("SafeMode", safe)) manual_available = repair_allowed and safe_readable and safe in (None, b"0", b"1") if page == "conditional": - choice = {"stock": MODES[0], "conditional_experimental": MODES[1], - "conditional_chill": MODES[2]}[preferences.mode.value] - rows.append(FeatureRow(MODE, "Saved driving mode", choice, document, MODES, default_value=MODES[0], - available=allowed, reason="Chill uses the normal driving mode; conditional modes switch automatically when their conditions match." - if allowed else unavailable, - dependencies=dependencies)) + (mode_document, mode_readable), (baseline, baseline_readable), (mode_safe, mode_safe_readable) = read_mode_sources(self.owner.params) + mode_valid = (mode_readable and mode_document == document and baseline_readable and baseline in (None, b"0", b"1") and + mode_safe_readable and mode_safe in (None, b"0", b"1")) + choice = {"stock": "Experimental" if baseline == b"1" else "Chill", + "conditional_experimental": "Conditional Experimental", "conditional_chill": "Conditional Chill"}[preferences.mode.value] + cp = self.owner.vehicle_params() + choices = MODES if choice == "Experimental" or experimental_available(cp) and mode_safe in (None, b"0") else tuple( + mode for mode in MODES if mode != "Experimental") + rows.append(FeatureRow(MODE, "Saved driving mode", choice if mode_valid else "Unavailable", document, + choices if mode_valid else (), default_value=MODES[0], available=allowed and mode_valid, + reason="Choose Chill, Experimental, or automatic switching from your saved conditions." if allowed and mode_valid else unavailable, + vehicle_fingerprint=self.owner.vehicle_fingerprint(), capability=_mode_capability(cp), + dependencies=dependencies + (("ExperimentalMode", baseline), ("SafeMode", mode_safe)))) rows.append(FeatureRow("", "Experimental conditions", "Saved options", page="conditional/cem", available=True)) rows.append(FeatureRow("", "Chill conditions", "Saved options", page="conditional/ccm", available=True)) if manual.status in ("invalid", "read_error"): @@ -218,6 +238,34 @@ class ConditionalFeature: expected=request.expected, dependencies=request.dependencies, authorized=authorized_button) return result.committed and result.verified + if request.key == MODE: + (document, readable), (baseline, baseline_readable), (safe, safe_readable) = read_mode_sources(self.owner.params) + units, units_readable = read_saved(self.owner.params, "IsMetric", 8) + dependencies = (("IsMetric", units), ("ExperimentalMode", baseline), ("SafeMode", safe)) + if (not readable or not baseline_readable or not safe_readable or not units_readable or + document != request.expected or request.dependencies != dependencies or + baseline not in (None, b"0", b"1") or safe not in (None, b"0", b"1") or request.value not in MODES): + return False + + def authorized_mode() -> bool: + cp = self.owner.vehicle_params() + return bool(self.owner.authority("preferences") and + (request.value != "Experimental" or request.capability is not None and + _mode_capability(cp) == request.capability and self.owner.vehicle_fingerprint() == request.capability[0] and + experimental_available(cp) and safe in (None, b"0"))) + + if not authorized_mode(): + return False + try: + preferences = SavedPreferences() if document is None else decode_preferences(document) + value = "Stock" if request.value in ("Chill", "Experimental") else request.value + raw = edit(preferences, MODE, value, metric=units == b"1") + except PreferenceError: + return False + next_baseline = b"1" if request.value == "Experimental" else b"0" if request.value == "Chill" else baseline or b"0" + result = commit_mode(self.owner.params, raw, next_baseline, expected=document, expected_baseline=baseline, + expected_units=units, expected_safe=safe, authorized=authorized_mode) + return result.committed and result.verified if (request.key in (RESET, MANUAL_RESET) and not request.confirmation) or \ request.capability is not None or request.vehicle_fingerprint is not None: return False diff --git a/openpilot/starpilot/ui/tests/test_conditional_feature.py b/openpilot/starpilot/ui/tests/test_conditional_feature.py index cf62ed9f51..4ed34a55d7 100644 --- a/openpilot/starpilot/ui/tests/test_conditional_feature.py +++ b/openpilot/starpilot/ui/tests/test_conditional_feature.py @@ -4,6 +4,7 @@ from openpilot.starpilot.ui.presentation import BitmapFonts from pathlib import Path +import json from dataclasses import replace from types import SimpleNamespace import subprocess @@ -11,6 +12,7 @@ import sys import os import tempfile import unittest +import warnings from unittest.mock import patch from openpilot.common.params import Params @@ -19,9 +21,11 @@ from opendbc.car.hyundai.values import CAR, HyundaiFlags from openpilot.starpilot.conditional_mode.button_actions import MEDIA_KEYS from openpilot.starpilot.ui.wheel_feature import PREFIX as BUTTON_PREFIX from openpilot.starpilot.conditional_mode.preferences import MPH_TO_MPS, SavedPreferences, decode_preferences, encode_preferences -from openpilot.starpilot.conditional_mode.actions import commit, commit_manual, stock_document +from openpilot.starpilot.conditional_mode.actions import commit, commit_manual, commit_mode, stock_document +from openpilot.starpilot.conditional_mode.runtime_settings import ConditionalSettingsOwner, read_mode_sources +from openpilot.selfdrive.selfdrived.selfdrived import SelfdriveD from openpilot.starpilot.conditional_mode.policy import ModeChoice -from openpilot.starpilot.conditional_mode.manual_saved import SavedCodes, decode as decode_manual, encode as encode_manual +from openpilot.starpilot.conditional_mode.manual_saved import KEY as MANUAL_KEY, SavedCodes, decode as decode_manual, encode as encode_manual from openpilot.starpilot.ui.feature_settings_owner import FeatureSettingsOwner from openpilot.starpilot.ui.feature_settings_state import FeatureInput, FeaturePage, FeatureRow, FeatureSettingsRequest, row_change from openpilot.starpilot.ui import feature_settings_compact as compact @@ -50,6 +54,126 @@ class ConditionalFeatureTests(unittest.TestCase): vehicle_fingerprint=lambda: getattr(self.cp, "carFingerprint", None), vehicle_params=lambda: self.cp) + def test_four_saved_modes_round_trip_through_actual_baseline_owner(self): + original = SavedPreferences(cem=replace(SavedPreferences().cem, speed_mps=13.0)) + self.params.put("ConditionalModeConfig", json.loads(encode_preferences(original)), block=True) + manual = encode_manual(SavedCodes()) + Path(self.params.get_param_path(MANUAL_KEY)).write_bytes(manual) + drive = SelfdriveD.__new__(SelfdriveD) + drive.params, drive.CP = self.params, self.cp + drive.conditional_settings = ConditionalSettingsOwner(self.params) + drive.conditional_replay = False + now = 1_000_000_000 + for value, choice, baseline in (("Experimental", ModeChoice.STOCK, True), ("Chill", ModeChoice.STOCK, False), + ("Conditional Experimental", ModeChoice.CEM, False), ("Conditional Chill", ModeChoice.CCM, False)): + row = self.row("conditional:mode") + self.assertEqual(row.choices, ("Chill", "Experimental", "Conditional Experimental", "Conditional Chill")) + self.assertTrue(self.owner.apply(replace(required_change(row), value=value))) + self.assertEqual(self.row("conditional:mode").value, value) + saved = decode_preferences(Path(self.params.get_param_path("ConditionalModeConfig")).read_bytes()) + self.assertEqual(saved.mode, choice) + self.assertEqual(saved.cem, original.cem) + self.assertEqual(saved.ccm, original.ccm) + self.assertEqual(Path(self.params.get_param_path(MANUAL_KEY)).read_bytes(), manual) + drive.refresh_saved_driving_mode(now) + drive.update_conditional_mode(None) + self.assertEqual(drive.experimental_mode, baseline) + now += 100_000_000 + self.assertTrue(self.owner.apply(replace(required_change(self.row("conditional:mode")), value="Experimental"))) + self.params.put_bool("SafeMode", True, block=True) + drive.refresh_saved_driving_mode(now) + drive.update_conditional_mode(None) + self.assertFalse(drive.experimental_mode) + + def test_actual_firmware_data_round_trip_and_changed_firmware_rejects_request(self): + from opendbc.car.honda.fingerprints import FW_VERSIONS + from opendbc.car.honda.values import CAR as HONDA + (ecu, address, subaddress), versions = next(iter(FW_VERSIONS[HONDA.HONDA_CIVIC_BOSCH].items())) + firmware = self.cp.init("carFw", 1)[0] + firmware.ecu, firmware.address, firmware.subAddress = ecu, address, subaddress or 0 + firmware.fwVersion, firmware.brand = versions[0], "honda" + self.assertIsInstance(self.cp.to_dict()["carFw"][0]["fwVersion"], bytes) + row = self.row("conditional:mode") + self.assertIsNotNone(row.capability) + for malformed in ((), (self.cp.carFingerprint,), (self.cp.carFingerprint, "invalid")): + self.assertFalse(self.owner.apply(replace(required_change(row), value="Experimental", capability=malformed))) + self.assertTrue(self.owner.apply(replace(required_change(row), value="Experimental"))) + self.assertEqual(self.row("conditional:mode").value, "Experimental") + self.assertTrue(self.owner.apply(replace(required_change(self.row("conditional:mode")), value="Chill"))) + request = replace(required_change(self.row("conditional:mode")), value="Experimental") + firmware.fwVersion = versions[0] + b"changed" + self.assertFalse(self.owner.apply(request)) + self.assertFalse(self.params.get_bool("ExperimentalMode")) + # Hashing copies the full CP instead of marking the live builder serialized. + with warnings.catch_warnings(): + warnings.simplefilter("error", UserWarning) + self.cp.to_bytes() + + def test_mode_request_rejects_changed_baseline_safe_mode_and_final_cp(self): + for change in ("baseline", "safe", "cp"): + self.params.put_bool("ExperimentalMode", False, block=True) + self.params.put_bool("SafeMode", False, block=True) + self.cp.carVin = "VIN1" + request = replace(required_change(self.row("conditional:mode")), value="Experimental") + if change == "baseline": + self.params.put_bool("ExperimentalMode", True, block=True) + elif change == "safe": + self.params.put_bool("SafeMode", True, block=True) + else: + self.cp.carVin = "VIN2" + before = self.params.get("ConditionalModeConfig") + self.assertFalse(self.owner.apply(request), change) + self.assertEqual(self.params.get("ConditionalModeConfig"), before) + self.params.put_bool("SafeMode", False, block=True) + self.cp.openpilotLongitudinalControl = False + row = self.row("conditional:mode") + self.assertNotIn("Experimental", row.choices) + self.assertFalse(self.owner.apply(replace(required_change(row), value="Experimental"))) + Path(self.params.get_param_path("ExperimentalMode")).write_bytes(b"01") + self.assertFalse(self.row("conditional:mode").available) + self.assertEqual(Path(self.params.get_param_path("ExperimentalMode")).read_bytes(), b"01") + + def test_mode_second_write_failure_restores_both_sources_and_reports_rollback_failure(self): + self.params.put_bool("ExperimentalMode", False, block=True) + old = self.params.get("ConditionalModeConfig") + baseline_path = Path(self.params.get_param_path("ExperimentalMode")) + original_replace = os.replace + attempts = 0 + + def fail_second(source, destination): + nonlocal attempts + attempts += 1 + if attempts == 2: + self.assertEqual(read_mode_sources(self.params), ((b"", False), (b"", False), (b"", False))) + raise OSError("second replacement failed") + return original_replace(source, destination) + + with patch("openpilot.starpilot.conditional_mode.actions.os.replace", side_effect=fail_second): + result = commit_mode(self.params, stock_document(), b"1", expected=old, expected_baseline=b"0", + expected_units=None, expected_safe=None, authorized=lambda: True) + self.assertFalse(result.committed) + self.assertFalse(result.verified) + self.assertFalse(result.rollback_failed) + self.assertEqual(self.params.get("ConditionalModeConfig"), old) + self.assertEqual(baseline_path.read_bytes(), b"0") + attempts = 0 + + def fail_write_and_rollback(source, destination): + nonlocal attempts + attempts += 1 + if attempts >= 2: + raise OSError("replacement and restoration failed") + return original_replace(source, destination) + + self.params.put("ConditionalModeConfig", json.loads(encode_preferences(SavedPreferences())), block=True) + old = Path(self.params.get_param_path("ConditionalModeConfig")).read_bytes() + with patch("openpilot.starpilot.conditional_mode.actions.os.replace", side_effect=fail_write_and_rollback): + result = commit_mode(self.params, stock_document(), b"1", expected=old, expected_baseline=b"0", + expected_units=None, expected_safe=None, authorized=lambda: True) + self.assertFalse(result.committed) + self.assertFalse(result.verified) + self.assertTrue(result.rollback_failed) + def test_saved_mode_and_conditions_can_change_during_drive_with_exact_source(self): self.parked = False self.owner.authority = lambda group: group == "preferences" @@ -128,7 +252,7 @@ class ConditionalFeatureTests(unittest.TestCase): self.assertEqual(saved.mode, ModeChoice.STOCK) self.assertNotEqual(saved.cem.signal_lane_detection, decode_preferences(original).cem.signal_lane_detection) mode = self.row("conditional:mode") - self.assertTrue(self.owner.apply(FeatureSettingsRequest(mode.key, mode.source, "Chill", dependencies=mode.dependencies))) + self.assertTrue(self.owner.apply(replace(required_change(mode), value="Chill"))) self.assertEqual(decode_preferences(path.read_bytes()).mode, ModeChoice.STOCK) def test_core_configuration_without_car_params_keeps_media_vehicle_bound(self):