From 5ce64a49a85a208f3c61a5d999fe00966bde091b Mon Sep 17 00:00:00 2001 From: firestar5683 <168790843+firestar5683@users.noreply.github.com> Date: Tue, 22 Sep 2026 14:34:44 -0500 Subject: [PATCH] Reduce first settings open cost and repeated vehicle catalog parsing --- selfdrive/ui/lib/fingerprint_catalog.py | 30 +++-- .../ui/mici/layouts/settings/settings.py | 57 ++++++--- .../ui/tests/test_fingerprint_catalog.py | 83 +++++++++++++ selfdrive/ui/tests/test_mici_settings_lazy.py | 116 ++++++++++++++++++ 4 files changed, 254 insertions(+), 32 deletions(-) create mode 100644 selfdrive/ui/tests/test_mici_settings_lazy.py diff --git a/selfdrive/ui/lib/fingerprint_catalog.py b/selfdrive/ui/lib/fingerprint_catalog.py index 8c96970bf9..527792139f 100644 --- a/selfdrive/ui/lib/fingerprint_catalog.py +++ b/selfdrive/ui/lib/fingerprint_catalog.py @@ -91,26 +91,24 @@ def _get_openpilot_root() -> Path: return Path(__file__).resolve().parents[5] -def _extract_fingerprint_models_for_make(make_key: str) -> list[tuple[str, str, str]]: - source_make = FINGERPRINT_MAKE_TO_VALUES_DIR.get(make_key, make_key) - root = _get_openpilot_root() +def _extract_fingerprint_models_for_source(source_make: str, root: Path) -> dict[str, list[tuple[str, str, str]]]: values_candidates = ( root / "opendbc" / "car" / source_make / "values.py", root / "selfdrive" / "car" / source_make / "values.py", ) values_path = next((path for path in values_candidates if path.is_file()), None) if values_path is None: - return [] + return {} try: content = values_path.read_text(encoding="utf-8", errors="replace") except Exception: - return [] + return {} content = re.sub(r'#[^\n]*', "", content) content = re.sub(r'footnotes=\[[^\]]*\],\s*', "", content) - models: list[tuple[str, str, str]] = [] + models: dict[str, list[tuple[str, str, str]]] = {} seen: set[tuple[str, str]] = set() for platform_match in _FINGERPRINT_PLATFORM_RE.finditer(content): @@ -125,20 +123,23 @@ def _extract_fingerprint_models_for_make(make_key: str) -> list[tuple[str, str, continue make_label = car_name.split(" ", 1)[0] - if make_label.lower() != make_key: - continue - dedupe_key = (car_name, platform_name) if dedupe_key in seen: continue seen.add(dedupe_key) - models.append((platform_name, car_name, make_label)) + models.setdefault(make_label.lower(), []).append((platform_name, car_name, make_label)) - models.sort(key=lambda entry: entry[1].lower()) + for entries in models.values(): + entries.sort(key=lambda entry: entry[1].lower()) return models +def _extract_fingerprint_models_for_make(make_key: str) -> list[tuple[str, str, str]]: + source_make = FINGERPRINT_MAKE_TO_VALUES_DIR.get(make_key, make_key) + return _extract_fingerprint_models_for_source(source_make, _get_openpilot_root()).get(make_key, []) + + @lru_cache(maxsize=1) def get_fingerprint_catalog() -> tuple[ tuple[str, ...], @@ -151,8 +152,13 @@ def get_fingerprint_catalog() -> tuple[ model_by_value: dict[str, FingerprintModelOption] = {} make_by_model: dict[str, str] = {} + root = _get_openpilot_root() + sources: dict[str, dict[str, list[tuple[str, str, str]]]] = {} for make_key in sorted(FINGERPRINT_MAKE_TO_VALUES_DIR): - entries = _extract_fingerprint_models_for_make(make_key) + source_make = FINGERPRINT_MAKE_TO_VALUES_DIR[make_key] + if source_make not in sources: + sources[source_make] = _extract_fingerprint_models_for_source(source_make, root) + entries = sources[source_make].get(make_key, []) if not entries: continue diff --git a/selfdrive/ui/mici/layouts/settings/settings.py b/selfdrive/ui/mici/layouts/settings/settings.py index e153dc88e0..5ee1c141a9 100644 --- a/selfdrive/ui/mici/layouts/settings/settings.py +++ b/selfdrive/ui/mici/layouts/settings/settings.py @@ -1,16 +1,14 @@ +from functools import cached_property + from openpilot.common.params import Params from openpilot.system.ui.widgets.scroller import NavScroller from openpilot.selfdrive.ui.mici.widgets.button import BigButton, BigMultiToggle from openpilot.selfdrive.ui.mici.layouts.settings.toggles import TogglesLayoutMici from openpilot.selfdrive.ui.mici.layouts.settings.network.network_layout import NetworkLayoutMici -from openpilot.selfdrive.ui.mici.layouts.settings.bluetooth import BluetoothLayoutMici -from openpilot.selfdrive.ui.mici.layouts.settings.vehicle import VehicleLayoutMici from openpilot.selfdrive.ui.mici.layouts.settings.device import DeviceLayoutMici, PairBigButton from openpilot.selfdrive.ui.mici.layouts.settings.developer import DeveloperLayoutMici -from openpilot.selfdrive.ui.mici.layouts.settings.software import SoftwareLayoutMici from openpilot.selfdrive.ui.mici.layouts.settings.driving_model import DrivingModelBigButton from openpilot.selfdrive.ui.mici.layouts.settings.galaxy import GalaxyBigButton -from openpilot.selfdrive.ui.mici.layouts.settings.visuals import VisualsLayoutMici from openpilot.system.ui.lib.application import gui_app, FontWeight from openpilot.system.ui.lib.wifi_manager import WifiManager @@ -61,37 +59,32 @@ class SettingsLayout(NavScroller): super().__init__() self._params = Params() - toggles_panel = TogglesLayoutMici() + self._toggles_panel = TogglesLayoutMici() toggles_btn = SettingsBigButton("toggles", "", gui_app.texture("icons_mici/settings.png", 64, 64)) - toggles_btn.set_click_callback(lambda: gui_app.push_widget(toggles_panel)) + toggles_btn.set_click_callback(lambda: gui_app.push_widget(self._toggles_panel)) - network_panel = NetworkLayoutMici(wifi_manager) + self._network_panel = NetworkLayoutMici(wifi_manager) network_btn = SettingsBigButton("network", "", gui_app.texture("icons_mici/settings/network/wifi_strength_full.png", 76, 56)) - network_btn.set_click_callback(lambda: gui_app.push_widget(network_panel)) + network_btn.set_click_callback(lambda: gui_app.push_widget(self._network_panel)) - bluetooth_panel = BluetoothLayoutMici() bluetooth_btn = SettingsBigButton("bluetooth", "", gui_app.texture("icons_mici/settings/bluetooth.png", 64, 64)) - bluetooth_btn.set_click_callback(lambda: gui_app.push_widget(bluetooth_panel)) + bluetooth_btn.set_click_callback(lambda: gui_app.push_widget(self._bluetooth_panel)) - vehicle_panel = VehicleLayoutMici() vehicle_btn = SettingsBigButton("vehicle", "", gui_app.texture("icons_mici/settings/vehicle.png", 64, 57)) - vehicle_btn.set_click_callback(lambda: gui_app.push_widget(vehicle_panel)) + vehicle_btn.set_click_callback(lambda: gui_app.push_widget(self._vehicle_panel)) - visuals_panel = VisualsLayoutMici() visuals_btn = SettingsBigButton("visuals", "", gui_app.texture("icons_mici/settings/device/cameras.png", 64, 64)) - visuals_btn.set_click_callback(lambda: gui_app.push_widget(visuals_panel)) + visuals_btn.set_click_callback(lambda: gui_app.push_widget(self._visuals_panel)) - device_panel = DeviceLayoutMici() device_btn = SettingsBigButton("device", "", gui_app.texture("icons_mici/settings/device_icon.png", 72, 58)) - device_btn.set_click_callback(lambda: gui_app.push_widget(device_panel)) + device_btn.set_click_callback(lambda: gui_app.push_widget(self._device_panel)) - software_panel = SoftwareLayoutMici() software_btn = SettingsBigButton("software", "", gui_app.texture("icons_mici/settings/device/update.png", 64, 75)) - software_btn.set_click_callback(lambda: gui_app.push_widget(software_panel)) + software_btn.set_click_callback(lambda: gui_app.push_widget(self._software_panel)) - developer_panel = DeveloperLayoutMici() + self._developer_panel = DeveloperLayoutMici() developer_btn = SettingsBigButton("developer", "", gui_app.texture("icons_mici/settings/developer_icon.png", 64, 60)) - developer_btn.set_click_callback(lambda: gui_app.push_widget(developer_panel)) + developer_btn.set_click_callback(lambda: gui_app.push_widget(self._developer_panel)) self._force_drive_state_btn = ForceDriveStateBigButton() self._driving_model_btn = DrivingModelBigButton() @@ -115,6 +108,30 @@ class SettingsLayout(NavScroller): self._font_medium = gui_app.font(FontWeight.MEDIUM) + @cached_property + def _bluetooth_panel(self): + from openpilot.selfdrive.ui.mici.layouts.settings.bluetooth import BluetoothLayoutMici + return BluetoothLayoutMici() + + @cached_property + def _vehicle_panel(self): + from openpilot.selfdrive.ui.mici.layouts.settings.vehicle import VehicleLayoutMici + return VehicleLayoutMici() + + @cached_property + def _visuals_panel(self): + from openpilot.selfdrive.ui.mici.layouts.settings.visuals import VisualsLayoutMici + return VisualsLayoutMici() + + @cached_property + def _device_panel(self): + return DeviceLayoutMici() + + @cached_property + def _software_panel(self): + from openpilot.selfdrive.ui.mici.layouts.settings.software import SoftwareLayoutMici + return SoftwareLayoutMici() + def show_event(self): super().show_event() self._force_drive_state_btn.refresh() diff --git a/selfdrive/ui/tests/test_fingerprint_catalog.py b/selfdrive/ui/tests/test_fingerprint_catalog.py index 2854e91bb2..e6c3adb3a6 100644 --- a/selfdrive/ui/tests/test_fingerprint_catalog.py +++ b/selfdrive/ui/tests/test_fingerprint_catalog.py @@ -18,3 +18,86 @@ def test_tesla_model_3_hardware_variants_remain_distinct_menu_options(): "Tesla Model 3 (with HW3) 2019-23", "Tesla Model 3 (with HW4) 2024-26", ] + + +def test_catalog_parses_shared_sources_once_and_preserves_make_order(tmp_path, monkeypatch): + from pathlib import Path + from openpilot.selfdrive.ui.lib import fingerprint_catalog as catalog + + values = tmp_path / 'opendbc' / 'car' / 'honda' / 'values.py' + values.parent.mkdir(parents=True) + values.write_text(''' +SHARED = PlatformConfig([ + HondaCarDocs("Honda Accord 2018"), + HondaCarDocs("Acura ILX 2019"), + HondaCarDocs("Honda Accord 2018"), +], specs=CarSpecs()) +SECOND = PlatformConfig([ + HondaCarDocs("Honda Accord 2018"), + HondaCarDocs("Honda Civic 2020", footnotes=[Footnote.SAMPLE]), +], specs=CarSpecs()) +''') + monkeypatch.setattr(catalog, 'FINGERPRINT_MAKE_TO_VALUES_DIR', {'honda': 'honda', 'acura': 'honda', 'missing': 'missing'}) + roots = [] + monkeypatch.setattr(catalog, '_get_openpilot_root', lambda: roots.append(tmp_path) or tmp_path) + reads = [] + read_text = Path.read_text + + def tracked_read(path, *args, **kwargs): + reads.append(path) + return read_text(path, *args, **kwargs) + + monkeypatch.setattr(Path, 'read_text', tracked_read) + catalog.get_fingerprint_catalog.cache_clear() + try: + result = catalog.get_fingerprint_catalog() + makes, by_make, by_value, make_by_model = result + assert makes == ('Acura', 'Honda') + assert [option.value for option in by_make['Honda']] == ['SHARED', 'SECOND', 'SECOND'] + assert [option.option_label for option in by_make['Honda']] == ['Honda Accord 2018', 'Honda Accord 2018 (SECOND)', 'Civic 2020'] + assert by_value['SHARED'].label == 'Acura ILX 2019' + assert make_by_model['SHARED'] == 'Acura' + assert reads == [values] + assert roots == [tmp_path] + assert catalog.get_fingerprint_catalog() is result + assert reads == [values] + + catalog.get_fingerprint_catalog.cache_clear() + assert catalog.get_fingerprint_catalog() == result + assert reads == [values, values] + finally: + catalog.get_fingerprint_catalog.cache_clear() + + +def test_extract_uses_legacy_source_when_opendbc_source_is_missing(tmp_path, monkeypatch): + from openpilot.selfdrive.ui.lib import fingerprint_catalog as catalog + + values = tmp_path / 'selfdrive' / 'car' / 'honda' / 'values.py' + values.parent.mkdir(parents=True) + values.write_text('CAR = PlatformConfig([CarDocs("Acura ILX 2019")], specs=CarSpecs())') + monkeypatch.setattr(catalog, '_get_openpilot_root', lambda: tmp_path) + + assert catalog._extract_fingerprint_models_for_make('acura') == [('CAR', 'Acura ILX 2019', 'Acura')] + assert catalog._extract_fingerprint_models_for_make('honda') == [] + assert catalog._extract_fingerprint_models_for_make('missing') == [] + + +def test_extract_keeps_empty_result_when_selected_source_cannot_be_read(tmp_path, monkeypatch): + from pathlib import Path + from openpilot.selfdrive.ui.lib import fingerprint_catalog as catalog + + for directory in ('opendbc', 'selfdrive'): + values = tmp_path / directory / 'car' / 'honda' / 'values.py' + values.parent.mkdir(parents=True) + values.write_text('CAR = PlatformConfig([CarDocs("Honda Civic 2020")], specs=CarSpecs())') + monkeypatch.setattr(catalog, '_get_openpilot_root', lambda: tmp_path) + reads = [] + + def failed_read(path, **kwargs): + reads.append(path) + raise OSError('unreadable source') + + monkeypatch.setattr(Path, 'read_text', failed_read) + + assert catalog._extract_fingerprint_models_for_make('honda') == [] + assert reads == [tmp_path / 'opendbc' / 'car' / 'honda' / 'values.py'] diff --git a/selfdrive/ui/tests/test_mici_settings_lazy.py b/selfdrive/ui/tests/test_mici_settings_lazy.py new file mode 100644 index 0000000000..35b9ea4f16 --- /dev/null +++ b/selfdrive/ui/tests/test_mici_settings_lazy.py @@ -0,0 +1,116 @@ +import importlib +from types import SimpleNamespace + +import pytest + +from openpilot.selfdrive.ui.mici.layouts.settings import settings + + +PANELS = { + "toggles": ("settings", "TogglesLayoutMici"), + "network": ("settings", "NetworkLayoutMici"), + "bluetooth": ("bluetooth", "BluetoothLayoutMici"), + "vehicle": ("vehicle", "VehicleLayoutMici"), + "visuals": ("visuals", "VisualsLayoutMici"), + "device": ("settings", "DeviceLayoutMici"), + "software": ("software", "SoftwareLayoutMici"), + "developer": ("settings", "DeveloperLayoutMici"), +} +EAGER_PANELS = ("toggles", "network", "developer") + + +@pytest.fixture +def settings_menu(monkeypatch): + created, pushed, buttons = [], [], [] + + class Button: + def __init__(self, label="", *_args): + self.label = label + self.callback = None + self.refreshes = 0 + + def set_click_callback(self, callback): + self.callback = callback + + def refresh(self): + self.refreshes += 1 + + def initialize_scroller(layout): + layout._scroller = SimpleNamespace(add_widgets=buttons.extend) + + for name, (module_name, class_name) in PANELS.items(): + module = importlib.import_module(f"openpilot.selfdrive.ui.mici.layouts.settings.{module_name}") + + def create_panel(*args, name=name): + panel = SimpleNamespace(name=name, args=args) + created.append(panel) + return panel + + monkeypatch.setattr(module, class_name, create_panel) + + monkeypatch.setattr(settings.NavScroller, "__init__", initialize_scroller) + monkeypatch.setattr(settings.NavScroller, "show_event", lambda _layout: None) + monkeypatch.setattr(settings, "Params", lambda: None) + monkeypatch.setattr(settings, "SettingsBigButton", Button) + for class_name in ("ForceDriveStateBigButton", "DrivingModelBigButton", "GalaxyBigButton", "PairBigButton"): + monkeypatch.setattr(settings, class_name, Button) + monkeypatch.setattr(settings.gui_app, "texture", lambda *_args: None) + monkeypatch.setattr(settings.gui_app, "font", lambda *_args: None) + monkeypatch.setattr(settings.gui_app, "push_widget", pushed.append) + + wifi_manager = object() + layout = settings.SettingsLayout(wifi_manager) + return SimpleNamespace(layout=layout, created=created, pushed=pushed, wifi_manager=wifi_manager, + buttons={button.label: button for button in buttons if button.label}) + + +def test_opening_settings_preserves_runtime_initializers(settings_menu): + settings_menu.layout.show_event() + assert [panel.name for panel in settings_menu.created] == list(EAGER_PANELS) + assert settings_menu.created[1].args == (settings_menu.wifi_manager,) + assert not settings_menu.pushed + assert settings_menu.layout._force_drive_state_btn.refreshes == 1 + assert settings_menu.layout._driving_model_btn.refreshes == 1 + + +@pytest.mark.parametrize("name", PANELS) +def test_only_selected_subpanel_is_constructed_and_reused(settings_menu, name): + button = settings_menu.buttons[name] + button.callback() + expected_names = [*EAGER_PANELS, *([] if name in EAGER_PANELS else [name])] + assert [panel.name for panel in settings_menu.created] == expected_names + panel = settings_menu.pushed[-1] + assert panel.name == name + assert panel.args == ((settings_menu.wifi_manager,) if name == "network" else ()) + assert settings_menu.pushed == [panel] + + settings_menu.layout.show_event() + button.callback() + assert [panel.name for panel in settings_menu.created] == expected_names + assert settings_menu.pushed == [panel, panel] + + +def test_visiting_one_subpanel_does_not_construct_another(settings_menu): + settings_menu.buttons["vehicle"].callback() + settings_menu.buttons["visuals"].callback() + settings_menu.buttons["vehicle"].callback() + assert [panel.name for panel in settings_menu.created] == [*EAGER_PANELS, "vehicle", "visuals"] + assert settings_menu.pushed[0] is settings_menu.pushed[2] + + +def test_failed_subpanel_construction_can_be_retried(settings_menu, monkeypatch): + module = importlib.import_module("openpilot.selfdrive.ui.mici.layouts.settings.vehicle") + original_constructor = module.VehicleLayoutMici + + def fail(): + raise RuntimeError("constructor failed") + + monkeypatch.setattr(module, "VehicleLayoutMici", fail) + with pytest.raises(RuntimeError, match="constructor failed"): + settings_menu.buttons["vehicle"].callback() + assert not settings_menu.pushed + + monkeypatch.setattr(module, "VehicleLayoutMici", original_constructor) + settings_menu.buttons["vehicle"].callback() + assert [panel.name for panel in settings_menu.created] == [*EAGER_PANELS, "vehicle"] + assert settings_menu.pushed == [settings_menu.created[-1]]