From b302c2ef8f464c72c77481a046fd34cacf0df1b0 Mon Sep 17 00:00:00 2001 From: firestar5683 <168790843+firestar5683@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:53:35 -0600 Subject: [PATCH] Avoid repeated CCNC longitudinal configuration clones --- .../opendbc/car/hyundai/ev9_longitudinal.py | 2 +- .../car/hyundai/ioniq5pe_long_profile.py | 17 +++-- .../tests/test_ccnc_long_validation.py | 72 +++++++++++++++++++ .../hyundai/tests/test_ev9_long_controller.py | 8 +-- prebuilt.json | 2 +- tools/ci/run_vehicle_tests.py | 1 + 6 files changed, 90 insertions(+), 12 deletions(-) create mode 100644 opendbc_repo/opendbc/car/hyundai/tests/test_ccnc_long_validation.py diff --git a/opendbc_repo/opendbc/car/hyundai/ev9_longitudinal.py b/opendbc_repo/opendbc/car/hyundai/ev9_longitudinal.py index b55f89cf1f..856ff36d16 100644 --- a/opendbc_repo/opendbc/car/hyundai/ev9_longitudinal.py +++ b/opendbc_repo/opendbc/car/hyundai/ev9_longitudinal.py @@ -314,7 +314,7 @@ def qualified(cp): cp.alternativeExperience not in (0, 32) or not cp.alphaLongitudinalAvailable or len(cp.safetyConfigs) != 1 or cp.safetyConfigs[0].safetyParam != LONG_PARAM): return False - if type(cp) is _DynamicStructBuilder and cp.schema == structs.CarParams.schema: + if type(cp) in (_DynamicStructBuilder, _DynamicStructReader) and cp.schema == structs.CarParams.schema: return metadata_qualified(cp) stock = stock_copy(cp) stock.alternativeExperience = 0 diff --git a/opendbc_repo/opendbc/car/hyundai/ioniq5pe_long_profile.py b/opendbc_repo/opendbc/car/hyundai/ioniq5pe_long_profile.py index 72ceb8cf98..0f72b22ff2 100644 --- a/opendbc_repo/opendbc/car/hyundai/ioniq5pe_long_profile.py +++ b/opendbc_repo/opendbc/car/hyundai/ioniq5pe_long_profile.py @@ -1,8 +1,9 @@ from math import isfinite +from capnp.lib.capnp import _DynamicStructBuilder, _DynamicStructReader from opendbc.car import structs from opendbc.car.hyundai.values import CAR -from opendbc.car.hyundai.ccnc_ev_stock import qualified as stock_qualified +from opendbc.car.hyundai.ccnc_ev_stock import qualified as stock_qualified, metadata_qualified from opendbc.car.hyundai.ev9_longitudinal import copy_cp STOCK_PARAM = 0x5491 @@ -36,13 +37,17 @@ def qualified(cp): if (not required(cp) or cp.pcmCruise or not cp.alphaLongitudinalAvailable or cp.alternativeExperience not in (0, 32) or len(cp.safetyConfigs) != 1 or cp.safetyConfigs[0].safetyParam != LONG_PARAM or - cp.steerControlType != structs.CarParams.SteerControlType.angle or not cp.deprecated.startingState or - not all(isfinite(value) for value in (cp.deprecated.startAccel, cp.deprecated.vEgoStarting, cp.deprecated.vEgoStopping, - cp.deprecated.stoppingDecelRate, cp.longitudinalActuatorDelay)) or - abs(cp.deprecated.startAccel - 1.) > 1e-6 or abs(cp.deprecated.vEgoStarting - .4) > 1e-6 or - abs(cp.deprecated.vEgoStopping - .3) > 1e-6 or abs(cp.deprecated.stoppingDecelRate - .4) > 1e-6 or + cp.steerControlType != structs.CarParams.SteerControlType.angle): + return False + deprecated = cp.deprecated + if (not deprecated.startingState or not all(isfinite(value) for value in (deprecated.startAccel, deprecated.vEgoStarting, deprecated.vEgoStopping, + deprecated.stoppingDecelRate, cp.longitudinalActuatorDelay)) or + abs(deprecated.startAccel - 1.) > 1e-6 or abs(deprecated.vEgoStarting - .4) > 1e-6 or + abs(deprecated.vEgoStopping - .3) > 1e-6 or abs(deprecated.stoppingDecelRate - .4) > 1e-6 or abs(cp.longitudinalActuatorDelay - .35) > 1e-6): return False + if type(cp) in (_DynamicStructBuilder, _DynamicStructReader) and cp.schema == structs.CarParams.schema: + return metadata_qualified(cp) stock = copy_cp(cp) stock.alternativeExperience = 0 stock.openpilotLongitudinalControl = False diff --git a/opendbc_repo/opendbc/car/hyundai/tests/test_ccnc_long_validation.py b/opendbc_repo/opendbc/car/hyundai/tests/test_ccnc_long_validation.py new file mode 100644 index 0000000000..90ab6c34dd --- /dev/null +++ b/opendbc_repo/opendbc/car/hyundai/tests/test_ccnc_long_validation.py @@ -0,0 +1,72 @@ +from unittest.mock import patch + +import pytest + +from opendbc.car.hyundai import ev9_longitudinal, ioniq5pe_long_profile +from opendbc.car.hyundai.tests.test_stock_angle_aol import source_profile +from opendbc.car.hyundai.values import CAR, HyundaiFlags + + +def profile(identity): + module = ioniq5pe_long_profile if identity == CAR.HYUNDAI_IONIQ_5_PE else ev9_longitudinal + cp = module.candidate(source_profile(identity), enabled=True, is_release=False) + return module, cp + + +@pytest.mark.parametrize('identity', (CAR.HYUNDAI_IONIQ_5_PE, CAR.KIA_EV9)) +@pytest.mark.parametrize('reader', (False, True)) +def test_canonical_long_validation_does_not_clone_carparams(identity, reader): + module, cp = profile(identity) + cp.alternativeExperience = 32 + value = cp.as_reader() if reader else cp + before = cp.to_dict() + with patch.object(module, 'copy_cp', side_effect=AssertionError('CYCLIC_LONG_QUALIFICATION_CLONED_CP')): + assert module.qualified(value) + cp.passive = True + assert not module.qualified(value) + cp.passive = False + assert module.qualified(value) + assert cp.to_dict() == before + + +@pytest.mark.parametrize('identity', (CAR.HYUNDAI_IONIQ_5_PE, CAR.KIA_EV9)) +@pytest.mark.parametrize('mutation', ('passive','dashcam','notcar','brand','fuel','long','pcm','experience','available','word','model','slots')) +def test_every_admission_field_is_still_checked_on_every_call(identity, mutation): + module, cp = profile(identity) + assert module.qualified(cp) + if mutation == 'passive': + cp.passive = True + elif mutation == 'dashcam': + cp.dashcamOnly = True + elif mutation == 'notcar': + cp.notCar = True + elif mutation == 'brand': + cp.brand = 'toyota' + elif mutation == 'fuel': + cp.flags = int(cp.flags | HyundaiFlags.HYBRID) + elif mutation == 'long': + cp.openpilotLongitudinalControl = False + elif mutation == 'pcm': + cp.pcmCruise = True + elif mutation == 'experience': + cp.alternativeExperience = 33 + elif mutation == 'available': + cp.alphaLongitudinalAvailable = False + elif mutation == 'word': + cp.safetyConfigs[0].safetyParam ^= 4 + elif mutation == 'model': + cp.safetyConfigs[0].safetyModel = 'noOutput' + else: + cp.safetyConfigs = [cp.safetyConfigs[0].to_dict(), cp.safetyConfigs[0].to_dict()] + assert not module.qualified(cp) + assert not module.qualified(cp.as_reader()) + + +@pytest.mark.parametrize('field,value', [('startingState',False),('startAccel',.5),('vEgoStarting',.5),('vEgoStopping',.4), + ('stoppingDecelRate',.5),('startAccel',float('nan')),('startAccel',float('inf'))]) +def test_pe_frozen_longitudinal_values_still_refuse_mutation(field, value): + module, cp = profile(CAR.HYUNDAI_IONIQ_5_PE) + assert module.qualified(cp) + setattr(cp.deprecated, field, value) + assert not module.qualified(cp) + assert not module.qualified(cp.as_reader()) diff --git a/opendbc_repo/opendbc/car/hyundai/tests/test_ev9_long_controller.py b/opendbc_repo/opendbc/car/hyundai/tests/test_ev9_long_controller.py index d2ce05367a..f8c0960a68 100644 --- a/opendbc_repo/opendbc/car/hyundai/tests/test_ev9_long_controller.py +++ b/opendbc_repo/opendbc/car/hyundai/tests/test_ev9_long_controller.py @@ -271,16 +271,16 @@ class TestEV9FreshMetadataQualification(unittest.TestCase): cp.safetyConfigs[0].safetyParam ^= 1 self.assertTrue(qualified(cp)) - def test_valid_builder_avoids_copy_and_reader_keeps_copy_contract(self): + def test_typed_builder_and_reader_avoid_copy_with_original_semantics(self): from unittest.mock import patch from opendbc.car.hyundai import ev9_longitudinal cp = self.base() with patch.object(ev9_longitudinal, 'stock_copy', side_effect=AssertionError('unexpected whole CP copy')): self.assertTrue(qualified(cp)) with structs.CarParams.from_bytes(cp.to_bytes()) as reader: - with patch.object(ev9_longitudinal, 'stock_copy', wraps=ev9_longitudinal.stock_copy) as copied: - self.assertTrue(qualified(reader)) - self.assertEqual(copied.call_count, 1) + expected = self.original(reader) + with patch.object(ev9_longitudinal, 'stock_copy', side_effect=AssertionError('unexpected whole CP copy')): + self.assertEqual(qualified(reader), expected) def test_custom_owner_keeps_original_access_and_copy_behavior(self): cp = self.base() diff --git a/prebuilt.json b/prebuilt.json index b4e27263df..ed4e1ae659 100644 --- a/prebuilt.json +++ b/prebuilt.json @@ -1 +1 @@ -{"protected_inventory_sha256": "9d10e892ff4106a821c4c67568318d46598503096446efdb729bad3b2462d39c", "schema_version": 1} +{"protected_inventory_sha256": "16b87cb64ab51930429e6251dad19d486b669b0c86e4f005f97fa7a5f9045661", "schema_version": 1} diff --git a/tools/ci/run_vehicle_tests.py b/tools/ci/run_vehicle_tests.py index cdcc968295..cb95e500dd 100644 --- a/tools/ci/run_vehicle_tests.py +++ b/tools/ci/run_vehicle_tests.py @@ -45,6 +45,7 @@ AOL_LIFECYCLE_TARGETS = ( 'openpilot/starpilot/aol/tests/test_hkg_early_long_withdrawal.py', 'openpilot/starpilot/aol/tests/test_hkg_angle_eps_retention.py', 'opendbc_repo/opendbc/car/hyundai/tests/test_angle_fault_recovery.py', + 'opendbc_repo/opendbc/car/hyundai/tests/test_ccnc_long_validation.py', 'opendbc_repo/opendbc/car/hyundai/tests/test_daw.py', 'openpilot/starpilot/aol/tests/test_hkg_angle_limit_warning.py', 'openpilot/starpilot/car/toyota/tests/test_prius_retrofit_stock.py',