From 572a221a86d8048baba3fe50d6a950e4775f6f9f Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Tue, 13 Jun 2023 20:41:03 -0700 Subject: [PATCH] FwQueryConfig: remove platform codes (#28538) * bring platform codes out of config for now (can re-introduce later) * clean that up * flake8 * comment * comments and better test names * typo * Update selfdrive/car/hyundai/tests/test_hyundai.py * Update selfdrive/car/hyundai/tests/test_hyundai.py --- selfdrive/car/fw_query_definitions.py | 8 +--- selfdrive/car/hyundai/tests/test_hyundai.py | 46 +++++++++++++-------- selfdrive/car/hyundai/values.py | 8 ++-- selfdrive/car/tests/test_fw_fingerprint.py | 15 ------- 4 files changed, 34 insertions(+), 43 deletions(-) diff --git a/selfdrive/car/fw_query_definitions.py b/selfdrive/car/fw_query_definitions.py index b3fb8476e5..7ae9bee404 100755 --- a/selfdrive/car/fw_query_definitions.py +++ b/selfdrive/car/fw_query_definitions.py @@ -3,7 +3,7 @@ import capnp import copy from dataclasses import dataclass, field import struct -from typing import Callable, Dict, List, Optional, Set, Tuple +from typing import Dict, List, Optional, Tuple import panda.python.uds as uds @@ -75,12 +75,6 @@ class FwQueryConfig: # Ecus added for data collection, not to be fingerprinted on extra_ecus: List[Tuple[capnp.lib.capnp._EnumModule, int, Optional[int]]] = field(default_factory=list) - # Brand-specific fuzzy fingerprinting config options: - # A function to get unique, platform-specific identification codes for a set of versions - fuzzy_get_platform_codes: Optional[Callable[[List[bytes]], Set[bytes]]] = None - # List of ECUs expected to have platform codes - platform_code_ecus: List[capnp.lib.capnp._EnumModule] = field(default_factory=list) - def __post_init__(self): for i in range(len(self.requests)): if self.requests[i].auxiliary: diff --git a/selfdrive/car/hyundai/tests/test_hyundai.py b/selfdrive/car/hyundai/tests/test_hyundai.py index 8bac256935..3b82b59d03 100755 --- a/selfdrive/car/hyundai/tests/test_hyundai.py +++ b/selfdrive/car/hyundai/tests/test_hyundai.py @@ -3,7 +3,8 @@ import unittest from cereal import car from selfdrive.car.hyundai.values import CAMERA_SCC_CAR, CANFD_CAR, CAN_GEARS, CAR, CHECKSUM, FW_QUERY_CONFIG, \ - FW_VERSIONS, LEGACY_SAFETY_MODE_CAR, PART_NUMBER_FW_PATTERN + FW_VERSIONS, LEGACY_SAFETY_MODE_CAR, PART_NUMBER_FW_PATTERN, PLATFORM_CODE_ECUS, \ + get_platform_codes Ecu = car.CarParams.Ecu ECU_NAME = {v: k for k, v in Ecu.schema.enumerants.items()} @@ -25,21 +26,24 @@ class TestHyundaiFingerprint(unittest.TestCase): ecu_strings = ", ".join([f'Ecu.{ECU_NAME[ecu]}' for ecu in ecus_not_in_whitelist]) self.assertEqual(len(ecus_not_in_whitelist), 0, f'{car_model}: Car model has ECUs not in auxiliary request whitelists: {ecu_strings}') + # Tests for platform codes, part numbers, and FW dates which Hyundai will use to fuzzy + # fingerprint in the absence of full FW matches: def test_platform_code_ecus_available(self): + # TODO: add queries for these non-CAN FD cars to get EPS no_eps_platforms = CANFD_CAR | {CAR.KIA_SORENTO, CAR.KIA_OPTIMA_G4, CAR.KIA_OPTIMA_G4_FL, CAR.SONATA_LF, CAR.TUCSON, CAR.GENESIS_G90, CAR.GENESIS_G80} # Asserts ECU keys essential for fuzzy fingerprinting are available on all platforms for car_model, ecus in FW_VERSIONS.items(): with self.subTest(car_model=car_model): - for fuzzy_ecu in FW_QUERY_CONFIG.platform_code_ecus: - if fuzzy_ecu in (Ecu.fwdRadar, Ecu.eps) and car_model == CAR.HYUNDAI_GENESIS: + for platform_code_ecu in PLATFORM_CODE_ECUS: + if platform_code_ecu in (Ecu.fwdRadar, Ecu.eps) and car_model == CAR.HYUNDAI_GENESIS: continue - if fuzzy_ecu == Ecu.eps and car_model in no_eps_platforms: + if platform_code_ecu == Ecu.eps and car_model in no_eps_platforms: continue - self.assertIn(fuzzy_ecu, [e[0] for e in ecus]) + self.assertIn(platform_code_ecu, [e[0] for e in ecus]) - def test_fw_part_number(self): + def test_fw_part_numbers(self): # Hyundai places the ECU part number in their FW versions, assert all parsable # Some examples of valid formats: '56310-L0010', '56310L0010', '56310/M6300' for car_model, ecus in FW_VERSIONS.items(): @@ -48,42 +52,50 @@ class TestHyundaiFingerprint(unittest.TestCase): raise unittest.SkipTest("No part numbers for car model") for ecu, fws in ecus.items(): - if ecu[0] not in FW_QUERY_CONFIG.platform_code_ecus: + if ecu[0] not in PLATFORM_CODE_ECUS: continue for fw in fws: match = PART_NUMBER_FW_PATTERN.search(fw) self.assertIsNotNone(match, fw) - def test_fuzzy_fw_dates(self): + def test_fw_dates(self): # Some newer platforms have date codes in a different format we don't yet parse, # for now assert date format is consistent for all FW across each platform for car_model, ecus in FW_VERSIONS.items(): with self.subTest(car_model=car_model): for ecu, fws in ecus.items(): - if ecu[0] not in FW_QUERY_CONFIG.platform_code_ecus: + if ecu[0] not in PLATFORM_CODE_ECUS: continue codes = set() for fw in fws: - codes |= FW_QUERY_CONFIG.fuzzy_get_platform_codes([fw]) + codes |= get_platform_codes([fw]) # Either no dates should be parsed or all dates should be parsed self.assertEqual(len({b'-' in code for code in codes}), 1) - def test_fuzzy_platform_codes(self): - # Asserts basic platform code parsing behavior - codes = FW_QUERY_CONFIG.fuzzy_get_platform_codes([b'\xf1\x00DH LKAS 1.1 -150210']) + def test_platform_codes_all_parsable(self): + # Assert every supported ECU FW version returns one platform code + for fw_by_addr in FW_VERSIONS.values(): + for addr, fws in fw_by_addr.items(): + if addr[0] in PLATFORM_CODE_ECUS: + for f in fws: + self.assertEqual(1, len(get_platform_codes([f])), f"Unable to parse FW: {f}") + + def test_platform_codes_spot_check(self): + # Asserts basic platform code parsing behavior for a few cases + codes = get_platform_codes([b'\xf1\x00DH LKAS 1.1 -150210']) self.assertEqual(codes, {b"DH-1502"}) # Some cameras and all radars do not have dates - codes = FW_QUERY_CONFIG.fuzzy_get_platform_codes([b'\xf1\x00AEhe SCC H-CUP 1.01 1.01 96400-G2000 ']) + codes = get_platform_codes([b'\xf1\x00AEhe SCC H-CUP 1.01 1.01 96400-G2000 ']) self.assertEqual(codes, {b"AEhe"}) - codes = FW_QUERY_CONFIG.fuzzy_get_platform_codes([b'\xf1\x00CV1_ RDR ----- 1.00 1.01 99110-CV000 ']) + codes = get_platform_codes([b'\xf1\x00CV1_ RDR ----- 1.00 1.01 99110-CV000 ']) self.assertEqual(codes, {b"CV1"}) - codes = FW_QUERY_CONFIG.fuzzy_get_platform_codes([ + codes = get_platform_codes([ b'\xf1\x00DH LKAS 1.1 -150210', b'\xf1\x00AEhe SCC H-CUP 1.01 1.01 96400-G2000 ', b'\xf1\x00CV1_ RDR ----- 1.00 1.01 99110-CV000 ', @@ -91,7 +103,7 @@ class TestHyundaiFingerprint(unittest.TestCase): self.assertEqual(codes, {b"DH-1502", b"AEhe", b"CV1"}) # Returned platform codes must inclusively contain start/end dates - codes = FW_QUERY_CONFIG.fuzzy_get_platform_codes([ + codes = get_platform_codes([ b'\xf1\x00LX2 MFC AT USA LHD 1.00 1.07 99211-S8100 220222', b'\xf1\x00LX2 MFC AT USA LHD 1.00 1.08 99211-S8100 211103', b'\xf1\x00ON MFC AT USA LHD 1.00 1.01 99211-S9100 190405', diff --git a/selfdrive/car/hyundai/values.py b/selfdrive/car/hyundai/values.py index 76284ce664..c0a8f8fae6 100644 --- a/selfdrive/car/hyundai/values.py +++ b/selfdrive/car/hyundai/values.py @@ -398,6 +398,10 @@ PLATFORM_CODE_FW_PATTERN = re.compile(b'((?<=' + HYUNDAI_VERSION_REQUEST_LONG[1: DATE_FW_PATTERN = re.compile(b'(?<=[ -])([0-9]{6}$)') PART_NUMBER_FW_PATTERN = re.compile(b'(?<=[0-9][.,][0-9]{2} )([0-9]{5}[-/]?[A-Z][A-Z0-9]{3}[0-9])') +# List of ECUs expected to have platform codes, camera and radar should exist on all cars +# TODO: use abs, it has the platform code and part number on many platforms +PLATFORM_CODE_ECUS = [Ecu.fwdRadar, Ecu.fwdCamera, Ecu.eps] + FW_QUERY_CONFIG = FwQueryConfig( requests=[ # TODO: minimize shared whitelists for CAN and cornerRadar for CAN-FD @@ -454,10 +458,6 @@ FW_QUERY_CONFIG = FwQueryConfig( (Ecu.hvac, 0x7b3, None), # HVAC Control Assembly (Ecu.cornerRadar, 0x7b7, None), ], - fuzzy_get_platform_codes=get_platform_codes, - # Camera and radar should exist on all cars - # TODO: use abs, it has the platform code and part number on many platforms - platform_code_ecus=[Ecu.fwdRadar, Ecu.fwdCamera, Ecu.eps], ) FW_VERSIONS = { diff --git a/selfdrive/car/tests/test_fw_fingerprint.py b/selfdrive/car/tests/test_fw_fingerprint.py index 97441f89af..b90120a1e2 100755 --- a/selfdrive/car/tests/test_fw_fingerprint.py +++ b/selfdrive/car/tests/test_fw_fingerprint.py @@ -123,21 +123,6 @@ class TestFwFingerprint(unittest.TestCase): with self.subTest(): self.fail(f"Brands do not implement FW_QUERY_CONFIG: {brand_versions - brand_configs}") - def test_fuzzy_fingerprint_config(self): - for brand, config in FW_QUERY_CONFIGS.items(): - with self.subTest(brand=brand): - if config.fuzzy_get_platform_codes is None: - self.assertEqual(len(config.platform_code_ecus), 0, "Cannot specify platform code ECUs without full config") - else: - self.assertGreater(len(config.platform_code_ecus), 0, "Need to specify platform code ECUs") - - # Assert every supported ECU FW version returns one platform code - for fw_by_addr in VERSIONS[brand].values(): - for addr, fws in fw_by_addr.items(): - if addr[0] in config.platform_code_ecus: - for f in fws: - self.assertEqual(1, len(config.fuzzy_get_platform_codes([f])), f"Unable to parse FW: {f}") - def test_fw_request_ecu_whitelist(self): for brand, config in FW_QUERY_CONFIGS.items(): with self.subTest(brand=brand):