Toyota: improve platform code understanding (#30015)

* return a dict where minor version is not in keys

* limit valid chunks to 3 (max seen)

* First short version character is always 3 (we were using wrong platform code)

* docs updates

* not here

* fixes for printing new platform code format

* ecu notes

* notes

* platform code tests

* no tuple

* can visualize the whole thing now

* make it clear there's no major versions

make it clear there's no major versions

* static analysis

* two minor versions

* fix

* not using dsu

* comment

* comment

* comment

* forgot this one

selfdrive/car/toyota/tests/print_platform_codes.py

@@ -1,4 +1,5 @@
 #!/usr/bin/env python3
+from collections import defaultdict
 from cereal import car
 from openpilot.selfdrive.car.toyota.values import FW_VERSIONS, PLATFORM_CODE_ECUS, get_platform_codes
 
@@ -6,6 +7,8 @@ Ecu = car.CarParams.Ecu
 ECU_NAME = {v: k for k, v in Ecu.schema.enumerants.items()}
 
 if __name__ == "__main__":
+  parts_for_ecu: dict = defaultdict(set)
+  cars_for_code: dict = defaultdict(lambda: defaultdict(set))
   for car_model, ecus in FW_VERSIONS.items():
     print()
     print(car_model)
@@ -14,8 +17,18 @@ if __name__ == "__main__":
         continue
 
       platform_codes = get_platform_codes(ecus[ecu])
-      codes = {code for code, _ in platform_codes}
+      parts_for_ecu[ecu] |= {code.split(b'-')[0] for code in platform_codes if code.count(b'-') > 1}
-      dates = {date for _, date in platform_codes if date is not None}
+      for code in platform_codes:
+        cars_for_code[ecu][b'-'.join(code.split(b'-')[:2])] |= {car_model}
       print(f'  (Ecu.{ECU_NAME[ecu[0]]}, {hex(ecu[1])}, {ecu[2]}):')
-      print(f'    Codes: {codes}')
+      print(f'    Codes: {platform_codes}')
-      print(f' Versions: {dates}')
+
+  print('\nECU parts:')
+  for ecu, parts in parts_for_ecu.items():
+    print(f'  (Ecu.{ECU_NAME[ecu[0]]}, {hex(ecu[1])}, {ecu[2]}): {parts}')
+
+  print('\nCar models vs. platform codes (no major versions):')
+  for ecu, codes in cars_for_code.items():
+    print(f' (Ecu.{ECU_NAME[ecu[0]]}, {hex(ecu[1])}, {ecu[2]}):')
+    for code, cars in codes.items():
+      print(f'    {code!r}: {sorted(cars)}')

selfdrive/car/toyota/tests/test_toyota.py

@@ -4,7 +4,7 @@ import unittest
 
 from cereal import car
 from openpilot.selfdrive.car.toyota.values import CAR, DBC, TSS2_CAR, ANGLE_CONTROL_CAR, RADAR_ACC_CAR, FW_VERSIONS, \
-                                                  get_platform_codes
+                                                  PLATFORM_CODE_ECUS, get_platform_codes
 
 Ecu = car.CarParams.Ecu
 ECU_NAME = {v: k for k, v in Ecu.schema.enumerants.items()}
@@ -43,6 +43,8 @@ class TestToyotaInterfaces(unittest.TestCase):
 
 
 class TestToyotaFingerprint(unittest.TestCase):
+  # Tests for part numbers, platform codes, and sub-versions which Toyota will use to fuzzy
+  # fingerprint in the absence of full FW matches:
   @settings(max_examples=100)
   @given(data=st.data())
   def test_platform_codes_fuzzy_fw(self, data):
@@ -50,13 +52,73 @@ class TestToyotaFingerprint(unittest.TestCase):
     fws = data.draw(fw_strategy)
     get_platform_codes(fws)
 
-  def test_fw_pattern(self):
+  def test_platform_code_ecus_available(self):
-    """Asserts all ECUs can be parsed"""
+    # Asserts ECU keys essential for fuzzy fingerprinting are available on all platforms
-    for ecus in FW_VERSIONS.values():
+    for car_model, ecus in FW_VERSIONS.items():
-      for fws in ecus.values():
+      with self.subTest(car_model=car_model):
+        for platform_code_ecu in PLATFORM_CODE_ECUS:
+          if platform_code_ecu == Ecu.eps and car_model in (CAR.PRIUS_V, CAR.LEXUS_CTH,):
+            continue
+          if platform_code_ecu == Ecu.abs and car_model in (CAR.ALPHARD_TSS2,):
+            continue
+          self.assertIn(platform_code_ecu, [e[0] for e in ecus])
+
+  def test_fw_format(self):
+    # Asserts:
+    # - every supported ECU FW version returns one platform code
+    # - every supported ECU FW version has a part number
+    # - expected parsing of ECU sub-versions
+
+    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 PLATFORM_CODE_ECUS:
+            continue
+
+          codes = dict()
           for fw in fws:
-          ret = get_platform_codes([fw])
+            result = get_platform_codes([fw])
-          self.assertTrue(len(ret))
+            # Check only one platform code and sub-version
+            self.assertEqual(1, len(result), f"Unable to parse FW: {fw}")
+            self.assertEqual(1, len(list(result.values())[0]), f"Unable to parse FW: {fw}")
+            codes |= result
+
+          # Toyota places the ECU part number in their FW versions, assert all parsable
+          # Note that there is only one unique part number per ECU across the fleet, so this
+          # is not important for identification, just a sanity check.
+          self.assertTrue(all(code.count(b"-") > 1 for code in codes),
+                          f"FW does not have part number: {fw} {codes}")
+
+  def test_platform_codes_spot_check(self):
+    # Asserts basic platform code parsing behavior for a few cases
+    results = get_platform_codes([
+      b"F152607140\x00\x00\x00\x00\x00\x00",
+      b"F152607171\x00\x00\x00\x00\x00\x00",
+      b"F152607110\x00\x00\x00\x00\x00\x00",
+      b"F152607180\x00\x00\x00\x00\x00\x00",
+    ])
+    self.assertEqual(results, {b"F1526-07-1": {b"10", b"40", b"71", b"80"}})
+
+    results = get_platform_codes([
+      b"\x028646F4104100\x00\x00\x00\x008646G5301200\x00\x00\x00\x00",
+      b"\x028646F4104100\x00\x00\x00\x008646G3304000\x00\x00\x00\x00",
+    ])
+    self.assertEqual(results, {b"8646F-41-04": {b"100"}})
+
+    # Short version has no part number
+    results = get_platform_codes([
+      b"\x0235870000\x00\x00\x00\x00\x00\x00\x00\x00A0202000\x00\x00\x00\x00\x00\x00\x00\x00",
+      b"\x0235883000\x00\x00\x00\x00\x00\x00\x00\x00A0202000\x00\x00\x00\x00\x00\x00\x00\x00",
+    ])
+    self.assertEqual(results, {b"58-70": {b"000"}, b"58-83": {b"000"}})
+
+    results = get_platform_codes([
+      b"F152607110\x00\x00\x00\x00\x00\x00",
+      b"F152607140\x00\x00\x00\x00\x00\x00",
+      b"\x028646F4104100\x00\x00\x00\x008646G5301200\x00\x00\x00\x00",
+      b"\x0235879000\x00\x00\x00\x00\x00\x00\x00\x00A4701000\x00\x00\x00\x00\x00\x00\x00\x00",
+    ])
+    self.assertEqual(results, {b"F1526-07-1": {b"10", b"40"}, b"8646F-41-04": {b"100"}, b"58-79": {b"000"}})
 
 
 if __name__ == "__main__":

selfdrive/car/toyota/values.py

@@ -2,7 +2,7 @@ import re
 from collections import defaultdict
 from dataclasses import dataclass, field
 from enum import Enum, IntFlag
-from typing import Dict, List, Set, Tuple, Union
+from typing import Dict, List, Set, Union
 
 from cereal import car
 from openpilot.common.conversions import Conversions as CV
@@ -236,8 +236,9 @@ STATIC_DSU_MSGS = [
 ]
 
 
-def get_platform_codes(fw_versions: List[bytes]) -> Set[Tuple[bytes, bytes]]:
+def get_platform_codes(fw_versions: List[bytes]) -> Dict[bytes, Set[bytes]]:
-  codes = set()  # (Optional[part]-platform-major_version, minor_version)
+  # Returns sub versions in a dict so comparisons can be made within part-platform-major_version combos
+  codes = defaultdict(set)  # Optional[part]-platform-major_version: set of sub_version
   for fw in fw_versions:
     # FW versions returned from UDS queries can return multiple fields/chunks of data (different ECU calibrations, different data?)
     #  and are prefixed with a byte that describes how many chunks of data there are.
@@ -262,42 +263,49 @@ def get_platform_codes(fw_versions: List[bytes]) -> Set[Tuple[bytes, bytes]]:
       fw_match = SHORT_FW_PATTERN.search(first_chunk)
       if fw_match is not None:
         platform, major_version, sub_version = fw_match.groups()
-        # codes.add((platform + b'-' + major_version, sub_version))
+        codes[b'-'.join((platform, major_version))].add(sub_version)
-        codes.add((b'-'.join((platform, major_version)), sub_version))
 
     elif len(first_chunk) == 10:
       fw_match = MEDIUM_FW_PATTERN.search(first_chunk)
       if fw_match is not None:
         part, platform, major_version, sub_version = fw_match.groups()
-        codes.add((b'-'.join((part, platform, major_version)), sub_version))
+        codes[b'-'.join((part, platform, major_version))].add(sub_version)
 
     elif len(first_chunk) == 12:
       fw_match = LONG_FW_PATTERN.search(first_chunk)
       if fw_match is not None:
         part, platform, major_version, sub_version = fw_match.groups()
-        codes.add((b'-'.join((part, platform, major_version)), sub_version))
+        codes[b'-'.join((part, platform, major_version))].add(sub_version)
 
-  return codes
+  return dict(codes)
 
 
 # Regex patterns for parsing more general platform-specific identifiers from FW versions.
 # - Part number: Toyota part number (usually last character needs to be ignored to find a match).
-# - Platform: usually multiple codes per an openpilot platform, however this has the less variability and
+#    Each ECU address has just one part number.
+# - Platform: usually multiple codes per an openpilot platform, however this is the least variable and
 #    is usually shared across ECUs and model years signifying this describes something about the specific platform.
-# - Major version: second least variable part of the FW version. Seen splitting cars by model year such as RAV4 2022/2023 and Prius.
+#    This describes more generational changes (TSS-P vs TSS2), or manufacture region.
+# - Major version: second least variable part of the FW version. Seen splitting cars by model year/API such as
+#    RAV4 2022/2023 and Avalon. Used to differentiate cars where API has changed slightly, but is not a generational change.
 #    It is important to note that these aren't always consecutive, for example:
-#    Prius TSS-P has these major versions over 16 FW: 2, 3, 4, 6, 8 while Prius TSS2 has: 5
+#    Avalon 2016-18's fwdCamera has these major versions: 01, 03 while 2019 has: 02
-# - Sub version: exclusive to major version, but shared with other cars. Should only be used for further filtering,
+# - Sub version: exclusive to major version, but shared with other cars. Should only be used for further filtering.
-#    more exploration is needed.
+#    Seen bumped in TSB FW updates, and describes other minor differences.
-SHORT_FW_PATTERN = re.compile(b'(?P<platform>[A-Z0-9]{2})(?P<major_version>[A-Z0-9]{2})(?P<sub_version>[A-Z0-9]{4})')
+SHORT_FW_PATTERN = re.compile(b'[A-Z0-9](?P<platform>[A-Z0-9]{2})(?P<major_version>[A-Z0-9]{2})(?P<sub_version>[A-Z0-9]{3})')
 MEDIUM_FW_PATTERN = re.compile(b'(?P<part>[A-Z0-9]{5})(?P<platform>[A-Z0-9]{2})(?P<major_version>[A-Z0-9]{1})(?P<sub_version>[A-Z0-9]{2})')
 LONG_FW_PATTERN = re.compile(b'(?P<part>[A-Z0-9]{5})(?P<platform>[A-Z0-9]{2})(?P<major_version>[A-Z0-9]{2})(?P<sub_version>[A-Z0-9]{3})')
-FW_LEN_CODE = re.compile(b'^[\x01-\x05]')  # 5 chunks max. highest seen is 3 chunks, 16 bytes each
+FW_LEN_CODE = re.compile(b'^[\x01-\x03]')  # highest seen is 3 chunks, 16 bytes each
 FW_CHUNK_LEN = 16
 
-# List of ECUs expected to have platform codes
+# List of ECUs that are most unique across openpilot platforms
-# TODO: use hybrid ECU, splits many similar ICE and hybrid variants
+# TODO: use hybrid ECU, splits similar ICE and hybrid variants
-PLATFORM_CODE_ECUS = [Ecu.abs, Ecu.engine, Ecu.eps, Ecu.dsu, Ecu.fwdCamera, Ecu.fwdRadar]
+# - fwdCamera: describes actual features related to ADAS. For example, on the Avalon it describes
+#    when TSS-P became standard, whether the car supports stop and go, and whether it's TSS2.
+#    On the RAV4, it describes the move to the radar doing ACC, and the use of LTA for lane keeping.
+# - abs: differentiates hybrid/ICE on most cars (Corolla TSS2 is an exception)
+# - eps: describes lateral API changes for the EPS, such as using LTA for lane keeping and rejecting LKA messages
+PLATFORM_CODE_ECUS = [Ecu.fwdCamera, Ecu.abs, Ecu.eps]
 
 
 # Some ECUs that use KWP2000 have their FW versions on non-standard data identifiers.