fuzzy fingerprint: track number of matching ECUs, not FW versions (#2…

…8423)

* first attempt trying to do it implicitly bad

* back to lists and explicitly do this

* continue checking if candidate switches on these, that's fine to check

* debugging

* clean up

* more clean up

* spot free

* only need to create/update on change

* fix test

* draft

* Revert "draft"

This reverts commit dbe32f5.

* Revert "fix test"

This reverts commit 1d34269.

* fix test for real

* first draft of test

* this should be good

* can replace existing fuzzy test

* rm

* matching

* test non-empty fws

* TEST DOWN HERE

* no test

* unique + fix skip message + comment + clean up

* less confusing

* ing

* better

selfdrive/car/fw_versions.py

@@ -75,24 +75,25 @@ def match_fw_to_car_fuzzy(fw_versions_dict, log=True, exclude=None):
       for f in fws:
         all_fw_versions[(addr[1], addr[2], f)].append(candidate)
 
-  match_count = 0
+  matched_ecus = set()
   candidate = None
   for addr, versions in fw_versions_dict.items():
+    ecu_key = (addr[0], addr[1])
     for version in versions:
       # All cars that have this FW response on the specified address
-      candidates = all_fw_versions[(addr[0], addr[1], version)]
+      candidates = all_fw_versions[(*ecu_key, version)]
 
       if len(candidates) == 1:
-        match_count += 1
+        matched_ecus.add(ecu_key)
         if candidate is None:
           candidate = candidates[0]
         # We uniquely matched two different cars. No fuzzy match possible
         elif candidate != candidates[0]:
           return set()
 
-  if match_count >= 2:
+  if len(matched_ecus) >= 2:
     if log:
-      cloudlog.error(f"Fingerprinted {candidate} using fuzzy match. {match_count} matching ECUs")
+      cloudlog.error(f"Fingerprinted {candidate} using fuzzy match. {len(matched_ecus)} matching ECUs")
     return {candidate}
   else:
     return set()

selfdrive/car/tests/test_fw_fingerprint.py

@@ -48,24 +48,26 @@ def test_exact_match(self, brand, car_model, ecus):
       self.assertFingerprints(matches, car_model)
 
   @parameterized.expand([(b, c, e[c]) for b, e in VERSIONS.items() for c in e])
-  def test_fuzzy_match(self, brand, car_model, ecus):
-    # TODO: speed up fuzzy matching and test more
-    CP = car.CarParams.new_message()
-    for _ in range(5):
-      fw = []
-      for ecu, fw_versions in ecus.items():
-        if not len(fw_versions):
-          raise unittest.SkipTest("Car model has no FW versions")
-        ecu_name, addr, sub_addr = ecu
-        fw.append({"ecu": ecu_name, "fwVersion": random.choice(fw_versions), 'brand': brand,
+  def test_fuzzy_match_ecu_count(self, brand, car_model, ecus):
+    # Asserts that fuzzy matching does not count matching FW, but ECU address keys
+    valid_ecus = [e for e in ecus if e[0] not in FUZZY_EXCLUDE_ECUS and len(ecus[e])]
+    if not len(valid_ecus):
+      raise unittest.SkipTest("Car model has no compatible ECUs for fuzzy matching")
+
+    fw = []
+    for ecu in valid_ecus:
+      ecu_name, addr, sub_addr = ecu
+      for _ in range(5):
+        # Add multiple FW versions to simulate ECU returning to multiple queries in a brand
+        fw.append({"ecu": ecu_name, "fwVersion": random.choice(ecus[ecu]), 'brand': brand,
                    "address": addr, "subAddress": 0 if sub_addr is None else sub_addr})
-      CP.carFw = fw
+      CP = car.CarParams.new_message(carFw=fw)
       _, matches = match_fw_to_car(CP.carFw, allow_exact=False, log=False)
 
-      # Assert no match if there are not enough valid ECUs
-      valid_ecus = [f['ecu'] for f in fw if f['ecu'] not in FUZZY_EXCLUDE_ECUS]
-      if len(valid_ecus) < 2:
-        self.assertEqual(len(matches), 0, valid_ecus)
+      # Assert no match if there are not enough unique ECUs
+      unique_ecus = {(f['address'], f['subAddress']) for f in fw}
+      if len(unique_ecus) < 2:
+        self.assertEqual(len(matches), 0)
       # There won't always be a match due to shared FW, but if there is it should be correct
       elif len(matches):
         self.assertFingerprints(matches, car_model)