Keep Hyundai startup testers until controller publication succeeds

opendbc_repo/opendbc/car/hyundai/ecu_continuity.py

@@ -5,6 +5,18 @@ from opendbc.car.hyundai.ecu_startup import HyundaiECUStartup, Outcome
 from opendbc.car.hyundai.ev9_keeper import EV9Keeper as StartupKeeper
 
 
+def commit_controller_tester(owner, frames, *, valid):
+  keeper = getattr(owner, "keeper", None)
+  if keeper is None or owner.handed_off or not valid or owner.closed or not owner.published:
+    return
+  tester = make_tester_present_msg(owner.address, owner.bus, suppress_response=True)
+  if tester not in frames:
+    return
+  keeper.stop()
+  owner.check()
+  owner.handed_off = True
+
+
 class ContinuousECUStartup(HyundaiECUStartup):
   def __init__(self, cp, callbacks, *, address, bus, label, clock=time.monotonic, keeper_factory=StartupKeeper):
     super().__init__(cp, callbacks, address=address, bus=bus, label=label)
@@ -16,8 +28,7 @@ class ContinuousECUStartup(HyundaiECUStartup):
   def prepare(self, *, admission):
     result = super().prepare(admission=admission)
     if self.continuity_required() and self.outcome is Outcome.SENT_UNCONFIRMED:
-      self.keeper = self.keeper_factory(
-        lambda: self.callbacks[1]([make_tester_present_msg(self.address, self.bus, suppress_response=True)]), clock=self.clock)
+      self.keeper = self.keeper_factory(lambda: self.callbacks[1]([make_tester_present_msg(self.address, self.bus, suppress_response=True)]), clock=self.clock)
       try:
         self.keeper.start()
       except Exception:
@@ -30,28 +41,20 @@ class ContinuousECUStartup(HyundaiECUStartup):
 
   def check(self):
     super().check()
-    keeper = getattr(self, 'keeper', None)
+    keeper = getattr(self, "keeper", None)
     if keeper is not None and keeper.abort_reason is not None:
       keeper.stop()
       self.outcome = Outcome.ABORT_UNCERTAIN
-      raise RuntimeError(f'{self.label} startup diagnostic continuity failed')
+      raise RuntimeError(f"{self.label} startup diagnostic continuity failed")
 
   def maintain(self, *, configured):
     self.check()
 
   def sent(self, frames, *, valid):
-    keeper = getattr(self, 'keeper', None)
-    if keeper is None or self.handed_off or not valid or self.closed or not self.published:
-      return
-    tester = make_tester_present_msg(self.address, self.bus, suppress_response=True)
-    if tester not in frames:
-      return
-    keeper.stop()
-    self.check()
-    self.handed_off = True
+    commit_controller_tester(self, frames, valid=valid)
 
   def close(self):
-    keeper = getattr(self, 'keeper', None)
+    keeper = getattr(self, "keeper", None)
     if keeper is not None:
       keeper.stop()
     super().close()

opendbc_repo/opendbc/car/hyundai/ev9_startup.py

@@ -1,10 +1,12 @@
 """EV9 source-selected communication-control startup, resolved before CarParams publication."""
+
 from copy import deepcopy
 import time
 
 from opendbc.car import Bus, make_tester_present_msg
 from opendbc.can.parser import MAX_BAD_COUNTER
 from opendbc.car.hyundai.ecu_startup import HyundaiECUStartup, Outcome
+from opendbc.car.hyundai.ecu_continuity import commit_controller_tester
 from opendbc.car.hyundai.ev9_longitudinal import qualified, copy_cp
 from opendbc.car.hyundai.ccnc_ev_stock import qualified as stock_qualified
 from opendbc.car.hyundai.values import CAR
@@ -20,9 +22,9 @@ def required(cp):
 
 class EV9Startup(HyundaiECUStartup):
   def __init__(self, cp, callbacks, *, stock_cp, clock=time.monotonic, keeper_factory=EV9Keeper):
-    super().__init__(cp, callbacks, address=0x730, bus=1, label='EV9')
+    super().__init__(cp, callbacks, address=0x730, bus=1, label="EV9")
     if stock_cp.carFingerprint != CAR.KIA_EV9 or not stock_qualified(stock_cp):
-      raise ValueError('EV9 startup requires exact original stock CP')
+      raise ValueError("EV9 startup requires exact original stock CP")
     self.clock = clock
     self.boot_clock = clock_boot_ns
     self.stock_cp = copy_cp(stock_cp)
@@ -31,7 +33,6 @@ class EV9Startup(HyundaiECUStartup):
     self.handed_off = False
     self.disable_response = None
 
-
   def _send(self, frames):
     # Source-selected EV9 request is unsuppressed enable-RX/disable-TX.
     for address, data, bus in frames:
@@ -45,23 +46,23 @@ class EV9Startup(HyundaiECUStartup):
     for _ in range(10):
       try:
         session = self._query(can_send, can_recv, self.bus, [(self.address, None)], [b"\x10\x03"], [b"\x50\x03"])
-        if not session.get_data(.1):
-          time.sleep(.1)
+        if not session.get_data(0.1):
+          time.sleep(0.1)
           continue
-        time.sleep(.05)
+        time.sleep(0.05)
         command = self._query(can_send, can_recv, self.bus, [(self.address, None)], [b"\x28\x01\x01"], [b""])
-        replies = command.get_data(.1)
+        replies = command.get_data(0.1)
         if not replies:
-          self.disable_response = 'no_reply_sent_unconfirmed'
+          self.disable_response = "no_reply_sent_unconfirmed"
           return True
         for payload in replies.values():
           if payload == b"\x68\x01":
-            self.disable_response = 'positive_communication_control'
+            self.disable_response = "positive_communication_control"
           elif len(payload) == 3 and payload[:2] == b"\x7f\x28":
-            self.disable_response = 'negative_communication_control'
+            self.disable_response = "negative_communication_control"
             return False
           else:
-            self.disable_response = 'unexpected_or_malformed_reply'
+            self.disable_response = "unexpected_or_malformed_reply"
             return False
         return True
       except Exception:
@@ -69,7 +70,7 @@ class EV9Startup(HyundaiECUStartup):
         # The shared owner restores before attempting stock publication.
         if self.disable_attempted:
           raise
-        time.sleep(.1)
+        time.sleep(0.1)
     return False
 
   def _ready(self):
@@ -78,15 +79,15 @@ class EV9Startup(HyundaiECUStartup):
     deadline = self.clock() + 0.5
     while self.clock() < deadline:
       packets = self.callbacks[0](wait_for_one=False)
-      observer.update([(int(getattr(packet, 'log_mono_time_ns', 0)), list(packet)) for packet in packets], self.boot_clock())
+      observer.update([(int(getattr(packet, "log_mono_time_ns", 0)), list(packet)) for packet in packets], self.boot_clock())
       if observer.current(self.boot_clock(), floor, 100_000_000) is True:
         return True
-      time.sleep(.005)
+      time.sleep(0.005)
     return observer.current(self.boot_clock(), floor, 100_000_000)
 
   def prepare(self, *, admission):
     if not qualified(self.cp):
-      raise ValueError('Unqualified EV9 LONG startup profile')
+      raise ValueError("Unqualified EV9 LONG startup profile")
     # A READY vehicle retains stock SCC and never receives communication control.
     skip = not admission() or self._ready() is not False
     super().prepare(admission=(lambda: False) if skip else admission)
@@ -94,8 +95,7 @@ class EV9Startup(HyundaiECUStartup):
       self.cp = copy_cp(self.stock_cp)
       self.prepared_cp = deepcopy(self.cp.to_dict())
     elif self.outcome is Outcome.SENT_UNCONFIRMED:
-      self.keeper = self.keeper_factory(
-        lambda: self.callbacks[1]([make_tester_present_msg(self.address, self.bus, suppress_response=True)]), clock=self.clock)
+      self.keeper = self.keeper_factory(lambda: self.callbacks[1]([make_tester_present_msg(self.address, self.bus, suppress_response=True)]), clock=self.clock)
       self.keeper.start()
     return self.cp
 
@@ -110,11 +110,11 @@ class EV9Startup(HyundaiECUStartup):
         except Exception:
           pass
       self.outcome = Outcome.ABORT_UNCERTAIN
-      raise RuntimeError('EV9 startup keeper aborted; session must stop')
+      raise RuntimeError("EV9 startup keeper aborted; session must stop")
 
   def sources_current(self, ci, now_ns):
     parser = ci.can_parsers[Bus.pt]
-    for name in ('MDPS', 'ACCELERATOR', 'WHEEL_SPEEDS', 'TCS'):
+    for name in ("MDPS", "ACCELERATOR", "WHEEL_SPEEDS", "TCS"):
       source = parser.message_states.get(parser.dbc.name_to_msg[name].address)
       if source is None or not source.timestamps or source.counter_fail >= MAX_BAD_COUNTER:
         return False
@@ -129,12 +129,11 @@ class EV9Startup(HyundaiECUStartup):
       return True
     if not (configured and sources_current and control_current):
       return False
-    if not self.handed_off:
-      self.keeper.stop()  # No sender lock held; join before controller handoff.
-      self.check()
-      self.handed_off = True
     return True
 
+  def sent(self, frames, *, valid):
+    commit_controller_tester(self, frames, valid=valid)
+
   def close(self):
     if self.keeper is not None:
       self.keeper.stop()
@@ -146,17 +145,17 @@ class EV9Startup(HyundaiECUStartup):
     super().close()
 
   def _warm_stock(self, ci):
-    deadline = self.clock() + 3.
+    deadline = self.clock() + 3.0
     while self.clock() < deadline:
       packets = self.callbacks[0]()
-      stamped = [(int(getattr(packet, 'log_mono_time_ns', 0)), list(packet)) for packet in packets]
+      stamped = [(int(getattr(packet, "log_mono_time_ns", 0)), list(packet)) for packet in packets]
       if any(stamp <= 0 for stamp, _ in stamped):
-        raise RuntimeError('EV9 stock warmup requires timestamped CAN')
+        raise RuntimeError("EV9 stock warmup requires timestamped CAN")
       state = ci.update(stamped)
       now = time.clock_gettime_ns(time.CLOCK_BOOTTIME)
       parser = ci.can_parsers[Bus.pt]
       ready = state.canValid and not state.canTimeout and parser.bus == 1
-      for name in ('SCC_CONTROL', 'MDPS', 'ACCELERATOR', 'WHEEL_SPEEDS', 'TCS'):
+      for name in ("SCC_CONTROL", "MDPS", "ACCELERATOR", "WHEEL_SPEEDS", "TCS"):
         source = parser.message_states.get(parser.dbc.name_to_msg[name].address)
         if source is None or not source.timestamps or source.counter_fail >= MAX_BAD_COUNTER:
           ready = False
@@ -167,6 +166,6 @@ class EV9Startup(HyundaiECUStartup):
       if ready:
         self.ready = True
         return
-      time.sleep(.005)
+      time.sleep(0.005)
     self.outcome = Outcome.ABORT_UNCERTAIN
-    raise RuntimeError('EV9 stock sources did not resume before publication')
+    raise RuntimeError("EV9 stock sources did not resume before publication")

opendbc_repo/opendbc/car/hyundai/tests/test_ev9_startup.py

@@ -1,11 +1,12 @@
 """Actual EV9 candidate and startup owner at the publication boundary."""
+
 import time
 import unittest
 from types import SimpleNamespace
 from unittest.mock import patch
 
 from opendbc.can import CANPacker
-from opendbc.car import Bus
+from opendbc.car import Bus, make_tester_present_msg
 from opendbc.car.can_definitions import CanData
 from opendbc.car.hyundai.ecu_startup import Outcome
 from opendbc.car.hyundai.ev9_longitudinal import candidate, qualified
@@ -22,80 +23,89 @@ class TestEV9Startup(unittest.TestCase):
     sleeper.start()
     self.addCleanup(sleeper.stop)
 
-  def owner(self, mode='sent'):
+  def owner(self, mode="sent"):
     stock = params(candidate=CAR.KIA_EV9)
     long = candidate(stock, enabled=True, is_release=False)
     sent, requests = [], []
+
     class Keeper:
       abort_reason = None
       stopped = False
+
       def __init__(self, send, **kwargs):
         self.send = send
+
       def start(self):
         pass
+
       def stop(self):
         self.stopped = True
+
     owner = EV9Startup(long, (lambda **kwargs: [], sent.extend), stock_cp=stock, keeper_factory=Keeper)
+
     def query(send, recv, bus, addresses, requests_arg, responses):
       request = requests_arg[0]
-      expected = {b'\x10\x03': b'\x50\x03', b'\x28\x01\x01': b'', b'\x28\x00\x01': b'\x68\x00', b'\x10\x01': b'\x50\x01'}
+      expected = {b"\x10\x03": b"\x50\x03", b"\x28\x01\x01": b"", b"\x28\x00\x01": b"\x68\x00", b"\x10\x01": b"\x50\x01"}
       self.assertEqual((bus, addresses, responses), (1, [(0x730, None)], [expected[request]]))
       requests.append(request)
+
       def data(timeout):
         send([CanData(0x730, bytes((len(request),)) + request + bytes(7 - len(request)), 1)])
-        if request == b'\x10\x03':
-          return {} if mode == 'untouched' else {(0x730, None): b''}
-        if request == b'\x10\x01':
-          return {(0x730, None): b''} if mode in ('untouched', 'restored') else {}
-        if request == b'\x28\x01\x01':
-          if mode in ('restored', 'uncertain'):
-            raise OSError('after actual command emission')
+        if request == b"\x10\x03":
+          return {} if mode == "untouched" else {(0x730, None): b""}
+        if request == b"\x10\x01":
+          return {(0x730, None): b""} if mode in ("untouched", "restored") else {}
+        if request == b"\x28\x01\x01":
+          if mode in ("restored", "uncertain"):
+            raise OSError("after actual command emission")
           return {}  # Original helper treats absence as sent, not confirmed.
-        return {(0x730, None): b''} if mode == 'restored' else {}
+        return {(0x730, None): b""} if mode == "restored" else {}
+
       return SimpleNamespace(get_data=data)
+
     return stock, owner, sent, requests, query
 
-  @patch.object(time, 'CLOCK_BOOTTIME', getattr(time, 'CLOCK_BOOTTIME', time.CLOCK_MONOTONIC), create=True)
+  @patch.object(time, "CLOCK_BOOTTIME", getattr(time, "CLOCK_BOOTTIME", time.CLOCK_MONOTONIC), create=True)
   def test_source_selected_request_silence_is_sent_unconfirmed(self):
     stock, owner, sent, requests, query = self.owner()
-    with patch.object(owner, '_ready', return_value=False), patch.object(owner, '_query', side_effect=query):
+    with patch.object(owner, "_ready", return_value=False), patch.object(owner, "_query", side_effect=query):
       cp = owner.prepare(admission=lambda: True)
     self.assertIs(owner.outcome, Outcome.SENT_UNCONFIRMED)
     self.assertTrue(qualified(cp))
-    self.assertEqual(requests, [b'\x10\x03', b'\x28\x01\x01'])
+    self.assertEqual(requests, [b"\x10\x03", b"\x28\x01\x01"])
     self.assertTrue(owner.disable_attempted)
-    self.assertEqual(stock.safetyConfigs[0].safetyParam, 0x5c91)
+    self.assertEqual(stock.safetyConfigs[0].safetyParam, 0x5C91)
     self.assertAlmostEqual(cp.longitudinalActuatorDelay, 0.3)
 
-  @patch.object(time, 'CLOCK_BOOTTIME', getattr(time, 'CLOCK_BOOTTIME', time.CLOCK_MONOTONIC), create=True)
+  @patch.object(time, "CLOCK_BOOTTIME", getattr(time, "CLOCK_BOOTTIME", time.CLOCK_MONOTONIC), create=True)
   def test_session_failure_requires_default_ack_before_exact_stock(self):
-    stock, owner, sent, requests, query = self.owner('untouched')
-    with patch.object(owner, '_ready', return_value=False), patch.object(owner, '_query', side_effect=query):
+    stock, owner, sent, requests, query = self.owner("untouched")
+    with patch.object(owner, "_ready", return_value=False), patch.object(owner, "_query", side_effect=query):
       cp = owner.prepare(admission=lambda: True)
     self.assertIs(owner.outcome, Outcome.STOCK_RESTORED)
     self.assertTrue(owner.session_attempted)
     self.assertTrue(owner.restored)
-    self.assertIn(b'\x10\x01', requests)
-    self.assertNotIn(b'\x28\x00\x01', requests)
+    self.assertIn(b"\x10\x01", requests)
+    self.assertNotIn(b"\x28\x00\x01", requests)
     self.assertFalse(owner.disable_attempted)
     self.assertEqual(cp.to_dict(), stock.to_dict())
 
-  @patch.object(time, 'CLOCK_BOOTTIME', getattr(time, 'CLOCK_BOOTTIME', time.CLOCK_MONOTONIC), create=True)
+  @patch.object(time, "CLOCK_BOOTTIME", getattr(time, "CLOCK_BOOTTIME", time.CLOCK_MONOTONIC), create=True)
   def test_original_ready_bit_skips_diagnostic_mutation(self):
     stock, owner, sent, requests, query = self.owner()
-    owner.callbacks = (lambda **kwargs: [[CanData(0x35, b'\0\0\0\x40' + bytes(28), 1)]], sent.extend)
-    with patch.object(owner, '_query', side_effect=AssertionError('READY must not query')):
+    owner.callbacks = (lambda **kwargs: [[CanData(0x35, b"\0\0\0\x40" + bytes(28), 1)]], sent.extend)
+    with patch.object(owner, "_query", side_effect=AssertionError("READY must not query")):
       cp = owner.prepare(admission=lambda: True)
     self.assertEqual(cp.to_dict(), stock.to_dict())
     self.assertIs(owner.outcome, Outcome.STOCK_UNTOUCHED)
     self.assertEqual(sent, [])
 
-  @patch.object(time, 'CLOCK_BOOTTIME', getattr(time, 'CLOCK_BOOTTIME', time.CLOCK_MONOTONIC), create=True)
+  @patch.object(time, "CLOCK_BOOTTIME", getattr(time, "CLOCK_BOOTTIME", time.CLOCK_MONOTONIC), create=True)
   def test_attempted_failure_requires_verified_restore(self):
-    for mode in ('restored', 'uncertain'):
+    for mode in ("restored", "uncertain"):
       stock, owner, sent, requests, query = self.owner(mode)
-      with patch.object(owner, '_ready', return_value=False), patch.object(owner, '_query', side_effect=query):
-        if mode == 'uncertain':
+      with patch.object(owner, "_ready", return_value=False), patch.object(owner, "_query", side_effect=query):
+        if mode == "uncertain":
           with self.assertRaises(RuntimeError):
             owner.prepare(admission=lambda: True)
           self.assertIs(owner.outcome, Outcome.ABORT_UNCERTAIN)
@@ -104,26 +114,27 @@ class TestEV9Startup(unittest.TestCase):
           self.assertEqual(cp.to_dict(), stock.to_dict())
           self.assertIs(owner.outcome, Outcome.STOCK_RESTORED)
       self.assertTrue(owner.disable_attempted)
-      self.assertIn(b'\x28\x00\x01', requests)
+      self.assertIn(b"\x28\x00\x01", requests)
 
-  @patch.object(time, 'CLOCK_BOOTTIME', getattr(time, 'CLOCK_BOOTTIME', time.CLOCK_MONOTONIC), create=True)
+  @patch.object(time, "CLOCK_BOOTTIME", getattr(time, "CLOCK_BOOTTIME", time.CLOCK_MONOTONIC), create=True)
   def test_publication_seal_rejects_changed_cp(self):
     stock, owner, sent, requests, query = self.owner()
-    with patch.object(owner, '_ready', return_value=False), patch.object(owner, '_query', side_effect=query):
+    with patch.object(owner, "_ready", return_value=False), patch.object(owner, "_query", side_effect=query):
       cp = owner.prepare(admission=lambda: True)
     ci = CarInterface(cp)
     owner.configure(ci)
-    ci.CP.safetyConfigs[0].safetyParam = 0x5c91
+    ci.CP.safetyConfigs[0].safetyParam = 0x5C91
     with self.assertRaises(RuntimeError):
       owner.seal_publication()
 
-  @patch.object(time, 'CLOCK_BOOTTIME', getattr(time, 'CLOCK_BOOTTIME', time.CLOCK_MONOTONIC), create=True)
+  @patch.object(time, "CLOCK_BOOTTIME", getattr(time, "CLOCK_BOOTTIME", time.CLOCK_MONOTONIC), create=True)
   def test_restored_stock_requires_real_parser_warmup(self):
-    stock, owner, sent, requests, query = self.owner('restored')
-    with patch.object(owner, '_ready', return_value=False), patch.object(owner, '_query', side_effect=query):
+    stock, owner, sent, requests, query = self.owner("restored")
+    with patch.object(owner, "_ready", return_value=False), patch.object(owner, "_query", side_effect=query):
       cp = owner.prepare(admission=lambda: True)
     ci = CarInterface(cp)
     packer = CANPacker(DBC[cp.carFingerprint][Bus.pt])
+
     def recv(**kwargs):
       frames = []
       for parser in ci.can_parsers.values():
@@ -131,6 +142,7 @@ class TestEV9Startup(unittest.TestCase):
           message = parser.dbc.addr_to_msg[address]
           frames.append(CanData(*packer.make_can_msg(message.name, parser.bus, {})))
       return [TimestampedCanPacket(frames, time.clock_gettime_ns(time.CLOCK_BOOTTIME))]
+
     owner.callbacks = (recv, sent.extend)
     owner.configure(ci)
     owner.seal_publication()
@@ -138,50 +150,62 @@ class TestEV9Startup(unittest.TestCase):
     self.assertTrue(owner.published)
     self.assertEqual(ci.CP.to_dict(), stock.to_dict())
 
-  @patch.object(time, 'CLOCK_BOOTTIME', getattr(time, 'CLOCK_BOOTTIME', time.CLOCK_MONOTONIC), create=True)
+  @patch.object(time, "CLOCK_BOOTTIME", getattr(time, "CLOCK_BOOTTIME", time.CLOCK_MONOTONIC), create=True)
   def test_keeper_handoff_requires_configured_current_sources_and_control(self):
     stock, owner, sent, requests, query = self.owner()
-    with patch.object(owner, '_ready', return_value=False), patch.object(owner, '_query', side_effect=query):
+    with patch.object(owner, "_ready", return_value=False), patch.object(owner, "_query", side_effect=query):
       owner.prepare(admission=lambda: True)
     for values in ((False, True, True), (True, False, True), (True, True, False)):
       self.assertFalse(owner.before_control(configured=values[0], sources_current=values[1], control_current=values[2]))
       self.assertFalse(owner.keeper.stopped)
     self.assertTrue(owner.before_control(configured=True, sources_current=True, control_current=True))
+    self.assertFalse(owner.keeper.stopped)
+    self.assertFalse(owner.handed_off)
+    owner.configure(CarInterface(owner.cp))
+    owner.seal_publication()
+    tester = make_tester_present_msg(0x730, 1, suppress_response=True)
+    owner.sent([tester], valid=False)
+    self.assertFalse(owner.handed_off)
+    owner.sent([tester], valid=True)
     self.assertTrue(owner.keeper.stopped)
     self.assertTrue(owner.handed_off)
 
-  @patch.object(time, 'CLOCK_BOOTTIME', getattr(time, 'CLOCK_BOOTTIME', time.CLOCK_MONOTONIC), create=True)
+  @patch.object(time, "CLOCK_BOOTTIME", getattr(time, "CLOCK_BOOTTIME", time.CLOCK_MONOTONIC), create=True)
   def test_keeper_abort_restores_on_main_thread_and_blocks_control(self):
     stock, owner, sent, requests, query = self.owner()
-    with patch.object(owner, '_ready', return_value=False), patch.object(owner, '_query', side_effect=query):
+    with patch.object(owner, "_ready", return_value=False), patch.object(owner, "_query", side_effect=query):
       owner.prepare(admission=lambda: True)
-    owner.keeper.abort_reason = 'software deadline'
-    with patch.object(owner, '_restore', return_value=True) as restore:
+    owner.keeper.abort_reason = "software deadline"
+    with patch.object(owner, "_restore", return_value=True) as restore:
       with self.assertRaises(RuntimeError):
         owner.before_control(configured=True, sources_current=True, control_current=True)
     restore.assert_called_once()
     self.assertTrue(owner.keeper.stopped)
     self.assertIs(owner.outcome, Outcome.ABORT_UNCERTAIN)
 
-  @patch.object(time, 'CLOCK_BOOTTIME', getattr(time, 'CLOCK_BOOTTIME', time.CLOCK_MONOTONIC), create=True)
+  @patch.object(time, "CLOCK_BOOTTIME", getattr(time, "CLOCK_BOOTTIME", time.CLOCK_MONOTONIC), create=True)
   def test_factory_and_required_constructor_boundary_use_prepared_cp(self):
     from openpilot.starpilot.vehicle_startup import VehicleStartupOwner
+
     stock, owner, sent, requests, query = self.owner()
     self.assertIsNone(CarInterface.startup_owner(stock, owner.callbacks, requested=False))
     self.assertTrue(CarInterface.startup_required(owner.cp))
     with self.assertRaises(RuntimeError):
       VehicleStartupOwner().configure(CarInterface(owner.cp))
     holder = VehicleStartupOwner()
-    with patch.object(CarInterface, 'startup_owner', return_value=owner), \
-         patch.object(owner, '_ready', return_value=False), patch.object(owner, '_query', side_effect=query):
+    with (
+      patch.object(CarInterface, "startup_owner", return_value=owner),
+      patch.object(owner, "_ready", return_value=False),
+      patch.object(owner, "_query", side_effect=query),
+    ):
       prepared = holder.prepare(stock, CarInterface, owner.callbacks, requested=True, admission=lambda: True)
     ci = CarInterface(prepared)
     holder.configure(ci)
     holder.seal_publication()
     self.assertEqual(ci.CP.to_dict(), prepared.to_dict())
-    self.assertEqual(ci.CP.safetyConfigs[0].safetyParam, 0x5c95)
+    self.assertEqual(ci.CP.safetyConfigs[0].safetyParam, 0x5C95)
 
-  @patch.object(time, 'CLOCK_BOOTTIME', getattr(time, 'CLOCK_BOOTTIME', time.CLOCK_MONOTONIC), create=True)
+  @patch.object(time, "CLOCK_BOOTTIME", getattr(time, "CLOCK_BOOTTIME", time.CLOCK_MONOTONIC), create=True)
   def test_actual_factory_promotes_only_exact_stock_cp(self):
     stock = params(candidate=CAR.KIA_EV9)
     actual = CarInterface.startup_owner(stock, (lambda **kwargs: [], lambda frames: None), requested=True)
@@ -193,6 +217,7 @@ class TestEV9Startup(unittest.TestCase):
     sibling = params(candidate=CAR.HYUNDAI_IONIQ_5_PE)
     from opendbc.car.hyundai.ioniq5pe_startup import PEPendingStartup, Phase
     from opendbc.car.hyundai.ioniq5pe_long_profile import qualified as pe_qualified
+
     pe = CarInterface.startup_owner(sibling, actual.callbacks, requested=True)
     self.assertIsInstance(pe, PEPendingStartup)
     self.assertNotIsInstance(pe, EV9Startup)
@@ -206,60 +231,71 @@ class TestEV9Startup(unittest.TestCase):
     stock, owner, sent, requests, query = self.owner()
     now, polls = [0.0], []
     owner.clock = lambda: now[0]
+
     def recv(wait_for_one=False):
       self.assertFalse(wait_for_one)
       polls.append(now[0])
-      now[0] += .1
+      now[0] += 0.1
       return []
+
     owner.callbacks = (recv, sent.extend)
-    with patch('opendbc.car.hyundai.ev9_startup.time.sleep'):
+    with patch("opendbc.car.hyundai.ev9_startup.time.sleep"):
       self.assertIsNone(owner._ready())
     self.assertEqual(len(polls), 5)
 
   def test_unsuppressed_reply_classification(self):
-    for payload, expected in ((b'\x68\x01', True), (b'\x7f\x28\x22', False),
-                              (b'', False), (b'\x68\x03', False), (b'garbage', False)):
+    for payload, expected in ((b"\x68\x01", True), (b"\x7f\x28\x22", False), (b"", False), (b"\x68\x03", False), (b"garbage", False)):
       stock, owner, sent, requests, query = self.owner()
       calls = [0]
-      def replies(*args, **kwargs):
+
+      def replies(*args, calls=calls, payload=payload, **kwargs):
         calls[0] += 1
         if calls[0] == 1:
-          return SimpleNamespace(get_data=lambda timeout: {(0x730, None): b''})
+          return SimpleNamespace(get_data=lambda timeout: {(0x730, None): b""})
         return SimpleNamespace(get_data=lambda timeout: {(0x730, None): payload})
-      with patch.object(owner, '_query', side_effect=replies), patch('opendbc.car.hyundai.ev9_startup.time.sleep'):
+
+      with patch.object(owner, "_query", side_effect=replies), patch("opendbc.car.hyundai.ev9_startup.time.sleep"):
         self.assertEqual(owner._disable(owner.callbacks[0], owner.callbacks[1]), expected)
 
-  @patch.object(time, 'CLOCK_BOOTTIME', getattr(time, 'CLOCK_BOOTTIME', time.CLOCK_MONOTONIC), create=True)
+  @patch.object(time, "CLOCK_BOOTTIME", getattr(time, "CLOCK_BOOTTIME", time.CLOCK_MONOTONIC), create=True)
   def test_actual_get_car_finalizes_long_before_interface_construction(self):
     from opendbc.car import car_helpers, gen_empty_fingerprint
     from openpilot.starpilot.vehicle_startup import VehicleStartupOwner
+
     stock, owner, sent, requests, query = self.owner()
     fingerprint = gen_empty_fingerprint()
     fingerprint[1].update({0x35: 32, 0x175: 24, 0xA0: 24, 0xEA: 24, 0x1CF: 8, 0x1A0: 32, 0x130: 16})
     fingerprint[2].update({0x110: 32, 0x362: 32})
-    found = (CAR.KIA_EV9, fingerprint, '0' * 17, [], stock.fingerprintSource, True)
+    found = (CAR.KIA_EV9, fingerprint, "0" * 17, [], stock.fingerprintSource, True)
     holder = VehicleStartupOwner()
+
     def hook(cp, candidate_id, fingerprints, firmware):
       return holder.prepare(cp, CarInterface, owner.callbacks, requested=True, admission=lambda: True)
-    with patch.object(car_helpers, 'fingerprint', return_value=found), \
-         patch.object(EV9Startup, '_ready', return_value=False), patch.object(EV9Startup, '_query', side_effect=query):
+
+    with (
+      patch.object(car_helpers, "fingerprint", return_value=found),
+      patch.object(EV9Startup, "_ready", return_value=False),
+      patch.object(EV9Startup, "_query", side_effect=query),
+    ):
       # Inject a non-thread keeper at the actual factory boundary.
       original_factory = CarInterface.startup_owner
+
       def factory(cp, callbacks, *, requested):
         selected = original_factory(cp, callbacks, requested=requested)
         selected.keeper_factory = owner.keeper_factory
         return selected
-      with patch.object(CarInterface, 'startup_owner', side_effect=factory):
-        ci = car_helpers.get_car(owner.callbacks[0], owner.callbacks[1], lambda *args: None,
-                                 True, False, pre_create_hook=hook)
+
+      with patch.object(CarInterface, "startup_owner", side_effect=factory):
+        ci = car_helpers.get_car(owner.callbacks[0], owner.callbacks[1], lambda *args: None, True, False, pre_create_hook=hook)
     holder.configure(ci)
     holder.seal_publication()
     self.assertTrue(qualified(ci.CP))
-    self.assertEqual(ci.CP.safetyConfigs[0].safetyParam, 0x5c95)
+    self.assertEqual(ci.CP.safetyConfigs[0].safetyParam, 0x5C95)
     self.assertIs(holder.owner.outcome, Outcome.SENT_UNCONFIRMED)
 
   def test_malformed_long_cp_cannot_bypass_required_startup_owner(self):
     from openpilot.starpilot.vehicle_startup import VehicleStartupOwner
+
     for field, value in (("brand", "other"), ("pcmCruise", True), ("alternativeExperience", 32)):
       stock, owner, sent, requests, query = self.owner()
       setattr(owner.cp, field, value)
@@ -267,7 +303,7 @@ class TestEV9Startup(unittest.TestCase):
       self.assertTrue(CarInterface.startup_required(owner.cp))
       with self.assertRaises(RuntimeError):
         VehicleStartupOwner().configure(CarInterface(owner.cp))
-    for word in (0x5c91, 0x5495, 0x5c97):
+    for word in (0x5C91, 0x5495, 0x5C97):
       stock, owner, sent, requests, query = self.owner()
       owner.cp.safetyConfigs[0].safetyParam = word
       self.assertFalse(qualified(owner.cp))

opendbc_repo/opendbc/car/hyundai/tests/test_fd_keeper_continuity.py

@@ -0,0 +1,121 @@
+from contextlib import nullcontext
+from types import SimpleNamespace
+from unittest.mock import patch
+
+import pytest
+
+from opendbc.can import CANPacker
+from opendbc.car import Bus, make_tester_present_msg, structs
+from opendbc.car.hyundai.ecu_startup import Outcome
+from opendbc.car.hyundai.interface import CarInterface
+from opendbc.car.hyundai.tests.test_ioniq5pe_stock import params
+from opendbc.car.hyundai.tests.test_torque_ev_startup import physical_frames, stock as torque_stock
+from opendbc.car.hyundai.tests.test_ev9_long_controller import packets as ev9_frames
+from opendbc.car.hyundai.values import CAR, DBC
+from opendbc.safety.tests.libsafety import libsafety_py
+from openpilot.selfdrive.car.card import Car
+from openpilot.starpilot.vehicle_startup import VehicleStartupOwner
+
+PROFILES = [(CAR.KIA_EV9, True), *[(car, alt) for car in (CAR.HYUNDAI_IONIQ_5, CAR.HYUNDAI_KONA_EV_2ND_GEN) for alt in (False, True)]]
+
+
+class Keeper:
+  def __init__(self, send, **kwargs):
+    self.send = send
+    self.abort_reason = None
+    self.started = False
+
+  def start(self):
+    self.started = True
+
+  def stop(self):
+    self.started = False
+
+
+@pytest.mark.parametrize("profile", PROFILES)
+@pytest.mark.parametrize("publication", ["success", "invalid", "exception"])
+def test_actual_controller_tester_publish_commits_fd_startup_handoff(profile, publication, monkeypatch):
+  car_id, _ = profile
+  stock = params(candidate=car_id) if car_id == CAR.KIA_EV9 else torque_stock.__wrapped__(SimpleNamespace(param=profile))
+  requests = []
+  owner = CarInterface.startup_owner(stock, (lambda **kwargs: [], lambda frames: None), requested=True)
+  assert owner is not None
+  owner.keeper_factory = Keeper
+
+  def query(send, recv, bus, addresses, request, response):
+    payload = request[0]
+    requests.append(payload)
+
+    def data(timeout):
+      send([(0x730, bytes([len(payload)]) + payload + bytes(7 - len(payload)), 1)])
+      return {} if payload[0] == 0x28 else {(0x730, None): b""}
+
+    return SimpleNamespace(get_data=data)
+
+  ready = patch.object(owner, "_ready", return_value=False) if car_id == CAR.KIA_EV9 else nullcontext()
+  with ready, patch.object(owner, "_query", side_effect=query), patch("time.sleep"):
+    cp = owner.prepare(admission=lambda: True)
+  assert owner.outcome is Outcome.SENT_UNCONFIRMED
+  assert requests == [b"\x10\x03", b"\x28\x01\x01" if car_id == CAR.KIA_EV9 else b"\x28\x83\x01"]
+  ci = CarInterface(cp)
+  ci.update([])
+  owner.configure(ci)
+  owner.seal_publication()
+  holder = VehicleStartupOwner(owner)
+  packer = CANPacker(DBC[car_id][Bus.pt])
+  stamp = owner.floor_ns
+  for tick in range(1, 121):
+    frames = ev9_frames(packer, tick, advance_counter=True) if car_id == CAR.KIA_EV9 else physical_frames(cp, packer, tick)
+    stamp = owner.floor_ns + tick * 10_000_000
+    state = ci.update([(stamp, frames)])
+  assert state.canValid and not state.canTimeout and owner.sources_current(ci, stamp)
+  for configured, sources, current in ((False, True, True), (True, False, True), (True, True, False)):
+    assert not owner.before_control(configured=configured, sources_current=sources, control_current=current)
+    assert owner.keeper.started and not owner.handed_off
+  assert owner.before_control(configured=True, sources_current=True, control_current=True)
+  assert owner.keeper.started and not owner.handed_off
+  published = []
+  should_fail = [False]
+
+  def publish(service, packet):
+    assert service == "sendcan" and not holder.send_lock.acquire(blocking=False)
+    if should_fail[0]:
+      raise OSError("publisher failure before successful send")
+    published.append(packet)
+
+  card = object.__new__(Car)
+  card.CP, card.CI, card.vehicle_startup = cp, ci, holder
+  monkeypatch.setattr(card, "pm", SimpleNamespace(send=publish), raising=False)
+  command = structs.CarControl().as_reader()
+  tester = make_tester_present_msg(0x730, 1, suppress_response=True)
+  config = cp.safetyConfigs[0]
+  native = libsafety_py.libsafety
+  native.set_alternative_experience(cp.alternativeExperience)
+  assert native.set_safety_hooks(int(config.safetyModel.raw), int(config.safetyParam)) == 0
+  native.init_tests()
+  assert native.safety_tx_hook(libsafety_py.make_CANPacket(tester[0], tester[2], tester[1]))
+  try:
+    ci.CC.frame = 1
+    _, frames = ci.apply(command, stamp)
+    assert tester not in frames
+    card.publish_sendcan(frames, valid=True)
+    assert owner.keeper.started and not owner.handed_off
+    ci.CC.frame = 100
+    _, frames = ci.apply(command, stamp + 10_000_000)
+    assert tester in frames
+    if publication == "exception":
+      should_fail[0] = True
+      with pytest.raises(OSError, match="publisher failure"):
+        card.publish_sendcan(frames, valid=True)
+      should_fail[0] = False
+    elif publication == "invalid":
+      card.publish_sendcan(frames, valid=False)
+    if publication != "success":
+      assert owner.keeper.started and not owner.handed_off
+    card.publish_sendcan(frames, valid=True)
+    assert published and not owner.keeper.started and owner.handed_off
+    assert holder.send_lock.acquire(blocking=False)
+    holder.send_lock.release()
+    assert not native.get_controls_allowed() and native.aol_get_permission_mask() == 0
+  finally:
+    owner.close()

opendbc_repo/opendbc/car/hyundai/tests/test_torque_ev_startup.py

@@ -5,7 +5,7 @@ from types import SimpleNamespace
 from unittest.mock import patch
 import pytest
 
-from opendbc.car import gen_empty_fingerprint
+from opendbc.car import gen_empty_fingerprint, make_tester_present_msg
 from opendbc.car.hyundai.interface import CarInterface
 from opendbc.car.hyundai.values import CAR, HyundaiFlags
 from opendbc.car.hyundai.ecu_startup import Outcome
@@ -104,10 +104,12 @@ def test_suppressed_silence_is_sent_unconfirmed_not_positive_ack(stock):
   assert startup.keeper.started
   assert not startup.before_control(configured=False, sources_current=True, control_current=True)
   assert startup.before_control(configured=True, sources_current=True, control_current=True)
-  assert not startup.keeper.started
+  assert startup.keeper.started
   startup.configure(SimpleNamespace(CP=final))
   startup.seal_publication()
   assert startup.published
+  startup.sent([make_tester_present_msg(0x730, 1, suppress_response=True)], valid=True)
+  assert not startup.keeper.started and startup.handed_off
   startup.close()
 
 
@@ -239,7 +241,7 @@ def physical_frames(cp, packer, tick):
   sources = (
     ("ACCELERATOR", 1, {"GEAR": 5, "ACCELERATOR_PEDAL": 0}),
     ("TCS", 1, {"ACCEnable": 0, "ACC_REQ": 0, "DriverBraking": 0}),
-    ("WHEEL_SPEEDS", 1, {name: 30 for name in ("WHL_SpdFLVal", "WHL_SpdFRVal", "WHL_SpdRLVal", "WHL_SpdRRVal")}),
+    ("WHEEL_SPEEDS", 1, dict.fromkeys(("WHL_SpdFLVal", "WHL_SpdFRVal", "WHL_SpdRLVal", "WHL_SpdRRVal"), 30)),
     ("MDPS", 1, {"MDPS_StrTqSnsrVal": 0, "MDPS_OutTqVal": 0}),
     ("STEERING_SENSORS", 1, {}),
     ("DOORS_SEATBELTS", 1, {"DRIVER_SEATBELT": 1}),
@@ -372,6 +374,8 @@ def test_actual_card_publishes_only_sealed_cp_then_controls(stock, mode, aol, mo
         assert holder.owner.sources_current(ci, stamp)
         assert not holder.owner.before_control(configured=False, sources_current=True, control_current=True)
         assert holder.owner.before_control(configured=True, sources_current=holder.owner.sources_current(ci, stamp), control_current=True)
+        assert not holder.owner.handed_off
+        card.publish_sendcan([make_tester_present_msg(0x730, 1, suppress_response=True)], valid=True)
         assert holder.owner.handed_off
       with structs.CarParams.from_bytes(saved.get("CarParams")) as reader:
         assert reader.to_dict() == expected

opendbc_repo/opendbc/car/hyundai/torque_ev_startup.py

@@ -10,6 +10,7 @@ import time
 from opendbc.car import Bus, make_tester_present_msg
 from opendbc.car.hyundai.canfd_stock_aol import qualified as stock_qualified
 from opendbc.car.hyundai.ecu_startup import HyundaiECUStartup, Outcome
+from opendbc.car.hyundai.ecu_continuity import commit_controller_tester
 from opendbc.car.hyundai.ev9_keeper import EV9Keeper as StartupKeeper
 from opendbc.car.hyundai.values import CAR, HyundaiFlags, HyundaiSafetyFlags
 from opendbc.car.hyundai.hyundaicanfd import CanBus
@@ -179,12 +180,11 @@ class TorqueEVStartup(HyundaiECUStartup):
       return True
     if not (configured and sources_current and control_current):
       return False
-    if not self.handed_off:
-      self.keeper.stop()  # Never join while holding sender callback lock.
-      self.check()
-      self.handed_off = True
     return True
 
+  def sent(self, frames, *, valid):
+    commit_controller_tester(self, frames, valid=valid)
+
   def _warm_stock(self, ci):
     deadline = self.clock() + 3.0
     while self.clock() < deadline:

prebuilt.json

@@ -1 +1 @@
-{"protected_inventory_sha256": "70eae3b6ac51ac3b1a3f51881aa11dee1d8e9ddd02490be71338b2c9c1a2b9de", "schema_version": 1}
+{"protected_inventory_sha256": "1d903fa124d7981fe0d4803ca3a087fe967cf9e91eaa1953d66517181b12148c", "schema_version": 1}