From 47e1e72ad2aed06359b2f0fc3024fb171ffb1a87 Mon Sep 17 00:00:00 2001 From: firestar5683 <168790843+firestar5683@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:58:55 -0500 Subject: [PATCH] Preserve feature configuration contracts --- .../car/toyota/tests/test_wheel_actions.py | 2 +- openpilot/starpilot/feature_capabilities.py | 20 +++++++- .../test_native_feature_configuration.py | 47 +++++++++++++++++++ .../tests/test_vehicle_preferences_api.py | 1 + .../starpilot/ui/gm_cancel_remap_feature.py | 2 +- openpilot/starpilot/vehicle_preferences.py | 2 +- 6 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 openpilot/starpilot/tests/test_native_feature_configuration.py diff --git a/openpilot/starpilot/car/toyota/tests/test_wheel_actions.py b/openpilot/starpilot/car/toyota/tests/test_wheel_actions.py index 88c63ae7ed..2b6ba35d80 100644 --- a/openpilot/starpilot/car/toyota/tests/test_wheel_actions.py +++ b/openpilot/starpilot/car/toyota/tests/test_wheel_actions.py @@ -223,7 +223,7 @@ class FactoryToyotaInterface: self.complete = complete def sample(self, now_ns, lda=0): - values = {'PCM_CRUISE': {'GAS_RELEASED': 1}, 'PCM_CRUISE_2': {'MAIN_ON': 1}, + values: dict[str, dict[str, float]] = {'PCM_CRUISE': {'GAS_RELEASED': 1}, 'PCM_CRUISE_2': {'MAIN_ON': 1}, 'GEAR_PACKET': {'GEAR': 0}, 'EPS_STATUS': {'LKA_STATE': 1}, 'BODY_CONTROL_STATE': {}, 'LKAS_HUD': {'LDA_ON_MESSAGE': lda}} frames = [self.packer.make_can_msg(name, 0 if bus == Bus.pt else 2, values.get(name, {})) diff --git a/openpilot/starpilot/feature_capabilities.py b/openpilot/starpilot/feature_capabilities.py index 4e744020c2..89dcbbcdb3 100644 --- a/openpilot/starpilot/feature_capabilities.py +++ b/openpilot/starpilot/feature_capabilities.py @@ -5,12 +5,28 @@ PCM cruise describes engagement/set-speed integration, not who controls accelera """ +def _native_configuration_valid(cp, *, longitudinal: bool) -> bool: + """Keep native configuration checks separate from global feature availability. + + GM's finalized owners encode the physical topology and longitudinal owner in + correlated safety words, flags and vehicle identity. A control boolean alone + cannot validate that contract. Reuse its native validators instead of a + feature launch allowlist or a second, weaker copy of those safety rules. + """ + if getattr(cp, 'brand', None) == 'gm': + from opendbc.car.gm.feature_capabilities import display_supported, longitudinal_supported + return longitudinal_supported(cp) if longitudinal else display_supported(cp) + return True + + def display_supported(cp) -> bool: return bool(cp is not None and getattr(cp, 'carFingerprint', '') and getattr(cp, 'passive', None) is False and getattr(cp, 'dashcamOnly', None) is False and - getattr(cp, 'notCar', None) is False) + getattr(cp, 'notCar', None) is False and + _native_configuration_valid(cp, longitudinal=False)) def longitudinal_supported(cp) -> bool: - return display_supported(cp) and getattr(cp, 'openpilotLongitudinalControl', None) is True + return bool(display_supported(cp) and getattr(cp, 'openpilotLongitudinalControl', None) is True and + _native_configuration_valid(cp, longitudinal=True)) diff --git a/openpilot/starpilot/tests/test_native_feature_configuration.py b/openpilot/starpilot/tests/test_native_feature_configuration.py new file mode 100644 index 0000000000..c43e53c34c --- /dev/null +++ b/openpilot/starpilot/tests/test_native_feature_configuration.py @@ -0,0 +1,47 @@ +"""Global feature availability never validates a malformed native CarParams.""" +import pytest + +from opendbc.car.gm.tests.test_silverado_cc_pedal import params as silverado_params +from opendbc.car.gm.values import CAR +from openpilot.common.params import Params +from openpilot.starpilot.feature_capabilities import display_supported, longitudinal_supported +from openpilot.starpilot.feature_runtime import enabled, slc_runtime_settings, slc_transport_capable, vision_control_enabled +from openpilot.starpilot.speed_limits.vision_gate import diagnostic_choice_enabled + + +@pytest.mark.parametrize('removed', [False, True]) +@pytest.mark.parametrize('disabled', [False, True]) +def test_native_silverado_configuration_is_required_for_global_features(tmp_path, removed, disabled): + params = Params(str(tmp_path)) + params.put_bool('CurveSpeedController', True, block=True) + params.put_bool('SpeedLimitController', True, block=True) + cp = silverado_params(removed=removed, disabled=disabled) + assert display_supported(cp) + assert longitudinal_supported(cp) is not disabled + for feature in ('conditional', 'curve', 'profile'): + assert enabled(params, cp, feature, {}) is not disabled + for feature in ('slc', 'vision'): + assert enabled(params, cp, feature, {}) + settings = slc_runtime_settings(params, cp, {}) + assert settings.enabled is not disabled + assert vision_control_enabled(params, cp) is not disabled + if disabled: + assert settings.display and settings.acceptance.display_only + for mutation in ('word', 'identity', 'dashcam', 'flags'): + bad = cp.as_reader().as_builder() + if mutation == 'word': + bad.safetyConfigs[0].safetyParam ^= 8 + elif mutation == 'identity': + bad.carFingerprint = CAR.CADILLAC_CT6_CC + elif mutation == 'dashcam': + bad.dashcamOnly = True + else: + bad.flags |= 1 << 30 + assert not display_supported(bad), mutation + assert not longitudinal_supported(bad), mutation + for feature in ('conditional', 'curve', 'profile', 'slc', 'vision'): + assert not enabled(params, bad, feature, {}), (mutation, feature) + assert not slc_transport_capable(bad, {}), mutation + assert not diagnostic_choice_enabled({}, bad), mutation + assert not vision_control_enabled(params, bad), mutation + assert not slc_runtime_settings(params, bad, {}).enabled, mutation diff --git a/openpilot/starpilot/tests/test_vehicle_preferences_api.py b/openpilot/starpilot/tests/test_vehicle_preferences_api.py index fa5fff7860..a8852b4e0c 100644 --- a/openpilot/starpilot/tests/test_vehicle_preferences_api.py +++ b/openpilot/starpilot/tests/test_vehicle_preferences_api.py @@ -25,6 +25,7 @@ def test_original_positional_preferences_do_not_enable_door_features(): ) assert tuple(getattr(preferences, name) for name in names) == values assert not preferences.toyota_lock_doors and not preferences.toyota_unlock_doors + assert not preferences.gm_cancel_remap assert VehicleStartupPreferences(True, True).turn_assist diff --git a/openpilot/starpilot/ui/gm_cancel_remap_feature.py b/openpilot/starpilot/ui/gm_cancel_remap_feature.py index 336263c9db..6d5c393744 100644 --- a/openpilot/starpilot/ui/gm_cancel_remap_feature.py +++ b/openpilot/starpilot/ui/gm_cancel_remap_feature.py @@ -21,7 +21,7 @@ class GmCancelRemapFeature: if capability is None: return [] row = self.owner._bool_row(KEY, "Use Cancel as Secondary Button", parked and self.owner.authority("parked_preferences"), - "Use the Cancel button assignments in Wheel Controls instead of cancelling cruise. Applies next drive.") + "Use the Cancel button assignments in Wheel Controls instead of canceling cruise. Applies next drive.") return [replace(row, capability=capability, available=row.available and self.owner._readable(KEY))] def apply(self, request): diff --git a/openpilot/starpilot/vehicle_preferences.py b/openpilot/starpilot/vehicle_preferences.py index ff2f283b4f..54fc2ba396 100644 --- a/openpilot/starpilot/vehicle_preferences.py +++ b/openpilot/starpilot/vehicle_preferences.py @@ -24,7 +24,6 @@ class VehicleStartupPreferences: tesla_preap_stock: bool = True volt_sng: bool = False gm_auto_hold: bool = False - gm_cancel_remap: bool = False volt_one_pedal: bool = False tesla_screen: bool = False tesla_screen_brake: bool = False @@ -35,6 +34,7 @@ class VehicleStartupPreferences: toyota_lock_doors: bool = False toyota_unlock_doors: bool = False toyota_radar_mode: int = 0 + gm_cancel_remap: bool = False @classmethod def read(cls, params, *, enabled: bool):