VIN: query on bus 0 to log data (#31165)

* query on bus 0

* fix

* log vin_rx_bus

* fix!
old-commit-hash: 289868297e

selfdrive/car/car_helpers.py

@@ -127,9 +127,6 @@ def fingerprint(logcan, sendcan, num_pandas):
 
   start_time = time.monotonic()
   if not skip_fw_query:
-    # Vin query only reliably works through OBDII
-    bus = 1
-
     cached_params = params.get("CarParamsCache")
     if cached_params is not None:
       with car.CarParams.from_bytes(cached_params) as cached_params:
@@ -139,20 +136,21 @@ def fingerprint(logcan, sendcan, num_pandas):
     if cached_params is not None and len(cached_params.carFw) > 0 and \
        cached_params.carVin is not VIN_UNKNOWN and not disable_fw_cache:
       cloudlog.warning("Using cached CarParams")
-      vin, vin_rx_addr = cached_params.carVin, 0
+      vin_rx_addr, vin_rx_bus, vin = -1, -1, cached_params.carVin
       car_fw = list(cached_params.carFw)
       cached = True
     else:
       cloudlog.warning("Getting VIN & FW versions")
       set_obd_multiplexing(params, True)
-      vin_rx_addr, vin = get_vin(logcan, sendcan, bus)
+      # Vin query only reliably works through OBDII
+      vin_rx_addr, vin_rx_bus, vin = get_vin(logcan, sendcan, (1, 0))
       ecu_rx_addrs = get_present_ecus(logcan, sendcan, num_pandas=num_pandas)
       car_fw = get_fw_versions_ordered(logcan, sendcan, ecu_rx_addrs, num_pandas=num_pandas)
       cached = False
 
     exact_fw_match, fw_candidates = match_fw_to_car(car_fw)
   else:
-    vin, vin_rx_addr = VIN_UNKNOWN, 0
+    vin_rx_addr, vin_rx_bus, vin = -1, -1, VIN_UNKNOWN
     exact_fw_match, fw_candidates, car_fw = True, set(), []
     cached = False
 
@@ -187,8 +185,8 @@ def fingerprint(logcan, sendcan, num_pandas):
     source = car.CarParams.FingerprintSource.fixed
 
   cloudlog.event("fingerprinted", car_fingerprint=car_fingerprint, source=source, fuzzy=not exact_match, cached=cached,
-                 fw_count=len(car_fw), ecu_responses=list(ecu_rx_addrs), vin_rx_addr=vin_rx_addr, fingerprints=finger,
-                 fw_query_time=fw_query_time, error=True)
+                 fw_count=len(car_fw), ecu_responses=list(ecu_rx_addrs), vin_rx_addr=vin_rx_addr, vin_rx_bus=vin_rx_bus,
+                 fingerprints=finger, fw_query_time=fw_query_time, error=True)
   return car_fingerprint, finger, vin, car_fw, source, exact_match
 
 

selfdrive/car/fw_versions.py

@@ -370,8 +370,8 @@ if __name__ == "__main__":
 
   t = time.time()
   print("Getting vin...")
-  vin_rx_addr, vin = get_vin(logcan, sendcan, 1, retry=10, debug=args.debug)
-  print(f'RX: {hex(vin_rx_addr)}, VIN: {vin}')
+  vin_rx_addr, vin_rx_bus, vin = get_vin(logcan, sendcan, (1, 0), retry=10, debug=args.debug)
+  print(f'RX: {hex(vin_rx_addr)}, BUS: {vin_rx_bus}, VIN: {vin}')
   print(f"Getting VIN took {time.time() - t:.3f} s")
   print()
 

selfdrive/car/tests/test_fw_fingerprint.py

@@ -212,7 +212,7 @@ class TestFwFingerprintTiming(unittest.TestCase):
 
   def test_startup_timing(self):
     # Tests worse-case VIN query time and typical present ECU query time
-    vin_ref_time = 1.0
+    vin_ref_time = 2.0
     present_ecu_ref_time = 0.75
 
     def fake_get_ecu_addrs(*_, timeout):
@@ -232,7 +232,7 @@ class TestFwFingerprintTiming(unittest.TestCase):
     self.total_time = 0.0
     with (mock.patch("openpilot.selfdrive.car.isotp_parallel_query.IsoTpParallelQuery.get_data", self.fake_get_data)):
       for _ in range(self.N):
-        get_vin(fake_socket, fake_socket, 1)
+        get_vin(fake_socket, fake_socket, (0, 1))
     self._assert_timing(self.total_time / self.N, vin_ref_time)
     print(f'get_vin, query time={self.total_time / self.N} seconds')
 

selfdrive/car/vin.py

@@ -15,33 +15,34 @@ def is_valid_vin(vin: str):
   return re.fullmatch(VIN_RE, vin) is not None
 
 
-def get_vin(logcan, sendcan, bus, timeout=0.1, retry=5, debug=False):
+def get_vin(logcan, sendcan, buses, timeout=0.1, retry=5, debug=False):
   addrs = list(range(0x7e0, 0x7e8)) + list(range(0x18DA00F1, 0x18DB00F1, 0x100))  # addrs to process/wait for
   valid_vin_addrs = [0x7e0, 0x7e2, 0x18da10f1, 0x18da0ef1]  # engine, VMCU, 29-bit engine, PGM-FI
   for i in range(retry):
-    for request, response in ((StdQueries.UDS_VIN_REQUEST, StdQueries.UDS_VIN_RESPONSE), (StdQueries.OBD_VIN_REQUEST, StdQueries.OBD_VIN_RESPONSE)):
-      try:
-        query = IsoTpParallelQuery(sendcan, logcan, bus, addrs, [request, ], [response, ], functional_addrs=FUNCTIONAL_ADDRS, debug=debug)
-        results = query.get_data(timeout)
+    for bus in buses:
+      for request, response in ((StdQueries.UDS_VIN_REQUEST, StdQueries.UDS_VIN_RESPONSE), (StdQueries.OBD_VIN_REQUEST, StdQueries.OBD_VIN_RESPONSE)):
+        try:
+          query = IsoTpParallelQuery(sendcan, logcan, bus, addrs, [request, ], [response, ], functional_addrs=FUNCTIONAL_ADDRS, debug=debug)
+          results = query.get_data(timeout)
 
-        for addr in valid_vin_addrs:
-          vin = results.get((addr, None))
-          if vin is not None:
-            # Ford pads with null bytes
-            if len(vin) == 24:
-              vin = re.sub(b'\x00*$', b'', vin)
+          for addr in valid_vin_addrs:
+            vin = results.get((addr, None))
+            if vin is not None:
+              # Ford pads with null bytes
+              if len(vin) == 24:
+                vin = re.sub(b'\x00*$', b'', vin)
 
-            # Honda Bosch response starts with a length, trim to correct length
-            if vin.startswith(b'\x11'):
-              vin = vin[1:18]
+              # Honda Bosch response starts with a length, trim to correct length
+              if vin.startswith(b'\x11'):
+                vin = vin[1:18]
 
-            return get_rx_addr_for_tx_addr(addr), vin.decode()
+              return get_rx_addr_for_tx_addr(addr), bus, vin.decode()
 
-        cloudlog.error(f"vin query retry ({i+1}) ...")
-      except Exception:
-        cloudlog.exception("VIN query exception")
+          cloudlog.error(f"vin query retry ({i+1}) ...")
+        except Exception:
+          cloudlog.exception("VIN query exception")
 
-  return 0, VIN_UNKNOWN
+  return -1, -1, VIN_UNKNOWN
 
 
 if __name__ == "__main__":
@@ -59,5 +60,5 @@ if __name__ == "__main__":
   logcan = messaging.sub_sock('can')
   time.sleep(1)
 
-  vin_rx_addr, vin = get_vin(logcan, sendcan, args.bus, args.timeout, args.retry, debug=args.debug)
-  print(f'RX: {hex(vin_rx_addr)}, VIN: {vin}')
+  vin_rx_addr, vin_rx_bus, vin = get_vin(logcan, sendcan, (args.bus,), args.timeout, args.retry, debug=args.debug)
+  print(f'RX: {hex(vin_rx_addr)}, BUS: {vin_rx_bus}, VIN: {vin}')