From 1b1989f794a241eaa361c6a4814dcf10c8149db6 Mon Sep 17 00:00:00 2001 From: AngusBell97 <124716116+AngusBell97@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:55:06 -0500 Subject: [PATCH] Galaxy: tighten shared action picker integration Clean up imported picker commentary and update Galaxy tests for the unified favourites/controller catalogue. --- .../onroad/starpilot/favorite_radial_menu.py | 2 -- .../tools/controller_action_picker.js | 10 ------- .../test_controller_action_categories.mjs | 1 - .../the_galaxy/tests/test_dashboard_stats.py | 3 +- .../tests/test_navigation_params.py | 28 ++++--------------- .../system/wheel_controls/wheel_controlsd.py | 7 ----- 6 files changed, 7 insertions(+), 44 deletions(-) diff --git a/selfdrive/ui/onroad/starpilot/favorite_radial_menu.py b/selfdrive/ui/onroad/starpilot/favorite_radial_menu.py index f56af6d147..75ae767bab 100644 --- a/selfdrive/ui/onroad/starpilot/favorite_radial_menu.py +++ b/selfdrive/ui/onroad/starpilot/favorite_radial_menu.py @@ -594,8 +594,6 @@ class FavoriteRadialMenu: def _open_picker(self, slot_index: int) -> None: options = self._refresh_option_catalog() - # This on-road picker has no numeric editor. Keep speed actions in the - # catalogue for activating saved slots, but configure their value in Galaxy. self._picker_options = [option for option in (options or []) if option.get("value_type") != "speed"] self._selected_slot = slot_index self._editing_slot = None diff --git a/starpilot/system/the_galaxy/assets/components/tools/controller_action_picker.js b/starpilot/system/the_galaxy/assets/components/tools/controller_action_picker.js index bd04f3e2e5..7988fbcc5d 100644 --- a/starpilot/system/the_galaxy/assets/components/tools/controller_action_picker.js +++ b/starpilot/system/the_galaxy/assets/components/tools/controller_action_picker.js @@ -1,9 +1,7 @@ import { longitudinalModeLayout, LONGITUDINAL_MODE_KEY } from "./longitudinal_mode.mjs" -// Presentation-only: the caller owns eligibility, persistence and speed values. const stylesheet = new URL("./controller_action_picker.css", import.meta.url).href -// Use the same catalogue as both Toggles menus, not the API's alphabetic order. const layoutUrl = "/assets/components/tools/device_settings_layout.json?v=settings-tier-1" function settingsAnchor(key) { @@ -75,8 +73,6 @@ export function filterActions(options, query = "", category = "", layout = []) { option.picker_description, actionCategory(option, layout), ...actionHierarchy(option, layout).map(part => part.label)].join(" ").toLocaleLowerCase().includes(word))) } -// Native modal supplies focus containment, inert background and Escape handling. -// Read options/guard/selection live: polling may change availability while open. export function openControllerActionPicker({ theme = "classic", index, trigger, getOptions, getSlot, isDisabled, onSelect, title = `Controller Action #${index + 1}`, slotAttribute = "data-controller-action-slot", noun = "actions" }) { if (isDisabled()) return () => {} if (!document.querySelector('link[data-controller-picker]')) { @@ -116,7 +112,6 @@ export function openControllerActionPicker({ theme = "classic", index, trigger, let closed = false let timer const fitViewport = () => { - // CSS zoom scales vh units too. Bound the modal in unzoomed layout pixels. let zoom = 1 for (let node = dialog; node; node = node.parentElement) zoom *= Number.parseFloat(getComputedStyle(node).zoom) || 1 const viewport = window.visualViewport @@ -131,7 +126,6 @@ export function openControllerActionPicker({ theme = "classic", index, trigger, window.visualViewport?.removeEventListener("resize", fitViewport) dialog.close() dialog.remove() - // Classic polling can replace the original card while the modal is open. const target = trigger?.isConnected ? trigger : document.querySelector(`[${slotAttribute}="${index}"]`) target?.focus({ preventScroll: true }) } @@ -186,7 +180,6 @@ export function openControllerActionPicker({ theme = "classic", index, trigger, container.append(button) if (focusedKey === option.key) button.focus({ preventScroll: true }) } - // The original empty option remains available independently of filters. add({ key: "", label: "Not configured", description: "Clear this assignment" }, "") if (grouped) { for (const name of categories) { @@ -235,7 +228,6 @@ export function openControllerActionPicker({ theme = "classic", index, trigger, } } input.addEventListener("input", () => { - // Search always covers the complete eligible catalogue, not the last category. category.value = "" render() results.scrollTop = 0 @@ -248,8 +240,6 @@ export function openControllerActionPicker({ theme = "classic", index, trigger, render() dialog.showModal() input.focus() - // A missing layout must never hide eligible actions or block assignment. - // Retry on the next open; don't cache a transient failure or stale catalogue. fetch(layoutUrl, { cache: "no-store" }).then(response => { if (!response.ok) throw new Error("Settings catalogue unavailable") return response.json() diff --git a/starpilot/system/the_galaxy/tests/test_controller_action_categories.mjs b/starpilot/system/the_galaxy/tests/test_controller_action_categories.mjs index 7a9cd9a8fb..9dee7b4bec 100644 --- a/starpilot/system/the_galaxy/tests/test_controller_action_categories.mjs +++ b/starpilot/system/the_galaxy/tests/test_controller_action_categories.mjs @@ -1,4 +1,3 @@ -// Run with node; optional source-derived full option fixture from browser test. import assert from 'node:assert/strict' import fs from 'node:fs' import { actionCategory, actionCategories, filterActions } from '../assets/components/tools/controller_action_picker.js' diff --git a/starpilot/system/the_galaxy/tests/test_dashboard_stats.py b/starpilot/system/the_galaxy/tests/test_dashboard_stats.py index 01ff4dba53..b03d1809f8 100644 --- a/starpilot/system/the_galaxy/tests/test_dashboard_stats.py +++ b/starpilot/system/the_galaxy/tests/test_dashboard_stats.py @@ -287,7 +287,7 @@ def _install_server_import_stubs(): sync_persist_chill_state=lambda *args, **kwargs: None, sync_persist_experimental_state=lambda *args, **kwargs: None, ) - def _trigger_stub_favorite_action(key, params_memory=None): + def _trigger_stub_favorite_action(key, params_memory=None, **_kwargs): if params_memory is None: return False counter_key = ( @@ -406,6 +406,7 @@ def _install_server_import_stubs(): sys.modules["openpilot.starpilot.system.wheel_controls"] = _simple_module( "openpilot.starpilot.system.wheel_controls", CONTROLLER_ACTION_OPTIONS=( + {"key": "__starpilot_controller_action__:cycle_driving_personality", "label": "Cycle Driving Personality", "section": "Controller Actions"}, {"key": "__starpilot_controller_action__:set_speed", "label": "Set Speed To", "section": "Controller Actions", "value_type": "speed"}, {"key": "__starpilot_controller_action__:selfie", "label": "Take Comma Selfie", "section": "Controller Actions"}, {"key": "__starpilot_controller_action__:bookmark", "label": "Bookmark", "section": "Controller Actions"}, diff --git a/starpilot/system/the_galaxy/tests/test_navigation_params.py b/starpilot/system/the_galaxy/tests/test_navigation_params.py index d6af895f65..64eb872d97 100644 --- a/starpilot/system/the_galaxy/tests/test_navigation_params.py +++ b/starpilot/system/the_galaxy/tests/test_navigation_params.py @@ -219,17 +219,7 @@ def test_wheel_controls_status_includes_favorite_slots(monkeypatch): assert len(response.get_json()["slots"]) == 3 assert len(response.get_json()["controller_slots"]) == 10 option_keys = {option["key"] for option in response.get_json()["controller_options"]} - assert option_keys == { - "ForceOffroad", - "__starpilot_controller_action__:set_speed", - "__starpilot_controller_action__:selfie", - "__starpilot_controller_action__:bookmark", - "__starpilot_controller_action__:pulse_and_glide", - "__starpilot_controller_action__:force_coast", - "__starpilot_controller_action__:toggle_aol", - "__starpilot_controller_action__:engage_openpilot", - "__starpilot_controller_action__:disengage_openpilot", - } + assert option_keys == {"ForceOffroad"} assert response.get_json()["speed_unit"] == "mph" assert response.get_json()["disconnect_controllers_offroad"] is False @@ -261,24 +251,16 @@ def test_wheel_controls_configures_a_controller_only_action(monkeypatch): response = client.post("/api/wheel-controls/action", json={"slot": 9, "key": "ForceOffroad"}) assert response.status_code == 200 - expected_keys = { - "ForceOffroad", - "__starpilot_controller_action__:set_speed", - "__starpilot_controller_action__:selfie", - "__starpilot_controller_action__:bookmark", - "__starpilot_controller_action__:pulse_and_glide", - "__starpilot_controller_action__:force_coast", - "__starpilot_controller_action__:toggle_aol", - "__starpilot_controller_action__:engage_openpilot", - "__starpilot_controller_action__:disengage_openpilot", - } + expected_keys = {"ForceOffroad"} assert calls == [((9, "ForceOffroad", "Force Offroad", the_galaxy.params), {"value": None, "eligible_keys": expected_keys})] def test_wheel_controls_configures_set_speed_in_current_units(monkeypatch): client, _ = _params_client(monkeypatch, {"IsOffroad": True, "IsMetric": False}, "mici") calls = [] - monkeypatch.setattr(the_galaxy, "_get_available_favorite_slot_options", list) + monkeypatch.setattr(the_galaxy, "_get_available_controller_action_options", lambda: [ + {"key": "__starpilot_controller_action__:set_speed", "label": "Set Speed To", "value_type": "speed"}, + ]) monkeypatch.setattr(the_galaxy, "set_controller_action_slot", lambda *args, **kwargs: calls.append((args, kwargs))) response = client.post("/api/wheel-controls/action", json={ diff --git a/starpilot/system/wheel_controls/wheel_controlsd.py b/starpilot/system/wheel_controls/wheel_controlsd.py index 89fc86f38e..2e8746574f 100644 --- a/starpilot/system/wheel_controls/wheel_controlsd.py +++ b/starpilot/system/wheel_controls/wheel_controlsd.py @@ -438,7 +438,6 @@ def cycle_driving_personality(params: Params) -> bool: if params.get_bool("SafeMode"): return False try: - # Match native UI capability selection; never use parked CP while on-road. cp_bytes = params.get("CarParams" if params.get_bool("IsOnroad") else "CarParamsPersistent") if not cp_bytes: return False @@ -447,8 +446,6 @@ def cycle_driving_personality(params: Params) -> bool: if not available: return False - # Typed Params decoding truncates fractional ints and defaults corrupt text. - # Inspect the stored token so malformed selections cannot become Aggressive. current = Path(params.get_param_path("LongitudinalPersonality")).read_bytes() profiles = tuple(int(profile) for profile in ( log.LongitudinalPersonality.aggressive, @@ -460,14 +457,10 @@ def cycle_driving_personality(params: Params) -> bool: return False next_personality = profiles[(tokens.index(current) + 1) % len(profiles)] except Exception: - # Missing/malformed CP or selection: do not change effective driving state. return False if params.get_bool("SafeMode"): return False - # Synchronous persistence lets successive HID events see the preceding change. - # selfdrived's existing Params reader publishes this selection; Traffic retains - # its independent override. No toggle refresh or virtual speed input is needed. params.put_int("LongitudinalPersonality", next_personality) return True