Pr suggestions

Author: chrispypatt

opendbc/safety/modes/toyota.h

@@ -216,7 +216,7 @@ static bool toyota_tx_hook(const CANPacket_t *msg) {
       if (toyota_secoc) {
         // SecOC cars move accel to 0x183. Only allow inactive accel on 0x343 to match stock behavior
         violation = desired_accel != TOYOTA_LONG_LIMITS.inactive_accel;
-     }
+      }
       violation |= longitudinal_accel_checks(desired_accel, TOYOTA_LONG_LIMITS);
 
       // only ACC messages that cancel are allowed when openpilot is not controlling longitudinal

opendbc/safety/tests/common.py

@@ -150,20 +150,6 @@ def setUpClass(cls):
   def _accel_msg(self, accel: float):
     pass
 
-  def _accel_msg_2(self, accel: float):
-    return None
-
-  def _should_tx_1(self, controls_allowed: bool, accel: float, min_accel: float, max_accel: float):
-    if self.LONGITUDINAL:
-      should_tx = controls_allowed and min_accel <= accel <= max_accel
-      should_tx = should_tx or accel == self.INACTIVE_ACCEL
-    else:
-      should_tx = False
-    return should_tx
-
-  def _should_tx_2(self, controls_allowed: bool, accel: float, min_accel: float, max_accel: float):
-    return False
-
   def test_accel_limits_correct(self):
     self.assertGreater(self.MAX_ACCEL, 0)
     self.assertLess(self.MIN_ACCEL, 0)
@@ -179,12 +165,12 @@ def test_accel_actuation_limits(self):
         for controls_allowed in [True, False]:
           self.safety.set_controls_allowed(controls_allowed)
           self.safety.set_alternative_experience(alternative_experience)
-          should_tx = self._should_tx_1(controls_allowed, accel, min_accel, max_accel)
+          if self.LONGITUDINAL:
+            should_tx = controls_allowed and min_accel <= accel <= max_accel
+            should_tx = should_tx or accel == self.INACTIVE_ACCEL
+          else:
+            should_tx = False
           self.assertEqual(should_tx, self._tx(self._accel_msg(accel)))
-          accel_msg_2 = self._accel_msg_2(accel)
-          if accel_msg_2 is not None:
-            should_tx_2 = self._should_tx_2(controls_allowed, accel, min_accel, max_accel)
-            self.assertEqual(should_tx_2, self._tx(self._accel_msg_2(accel)))
 
 
 class LongitudinalGasBrakeSafetyTest(PandaSafetyTestBase, abc.ABC):

opendbc/safety/tests/test_toyota.py

@@ -52,10 +52,17 @@ def _lta_msg(self, req, req2, angle_cmd, torque_wind_down=100):
     values = {"STEER_REQUEST": req, "STEER_REQUEST_2": req2, "STEER_ANGLE_CMD": angle_cmd, "TORQUE_WIND_DOWN": torque_wind_down}
     return self.packer.make_can_msg_panda("STEERING_LTA", 0, values)
 
-  def _accel_msg(self, accel, cancel_req=0):
+  def _accel_msg_343(self, accel, cancel_req=0):
     values = {"ACCEL_CMD": accel, "CANCEL_REQ": cancel_req}
     return self.packer.make_can_msg_panda("ACC_CONTROL", 0, values)
 
+  def _accel_msg_183(self, accel):
+    values = {"ACCEL_CMD": accel}
+    return self.packer.make_can_msg_panda("ACC_CONTROL_2", 0, values)
+
+  def _accel_msg(self, accel, cancel_req=0):
+    return self._accel_msg_343(accel, cancel_req)
+
   def _speed_msg(self, speed):
     values = {("WHEEL_SPEED_%s" % n): speed * 3.6 for n in ["FR", "FL", "RR", "RL"]}
     return self.packer.make_can_msg_panda("WHEEL_SPEEDS", 0, values)
@@ -285,9 +292,9 @@ def test_acc_cancel(self):
     for controls_allowed in [True, False]:
       self.safety.set_controls_allowed(controls_allowed)
       for accel in np.arange(self.MIN_ACCEL - 1, self.MAX_ACCEL + 1, 0.1):
-        self.assertFalse(self._tx(self._accel_msg(accel)))
-        should_tx = np.isclose(accel, 0, atol=0.0001)
-        self.assertEqual(should_tx, self._tx(self._accel_msg(accel, cancel_req=1)))
+        self.assertFalse(self._tx(self._accel_msg_343(accel)))
+        should_tx = np.isclose(accel, self.INACTIVE_ACCEL, atol=0.0001)
+        self.assertEqual(should_tx, self._tx(self._accel_msg_343(accel, cancel_req=1)))
 
 
 class TestToyotaStockLongitudinalTorque(TestToyotaStockLongitudinalBase, TestToyotaSafetyTorque):
@@ -345,6 +352,9 @@ def test_lta_2_steer_cmd(self):
       should_tx = not req and not req2 and angle == 0
       self.assertEqual(should_tx, self._tx(self._lta_2_msg(req, req2, angle)), f"{req=} {req2=} {angle=}")
 
+  def _accel_msg(self, accel, cancel_req=0):
+    return self._accel_msg_183(accel)
+
 
 class TestToyotaSecOcSafetyStockLongitudinal(TestToyotaSecOcSafetyBase, TestToyotaStockLongitudinalBase):
 
@@ -371,17 +381,17 @@ def setUp(self):
   def test_block_aeb(self, stock_longitudinal: bool = False):
     pass
 
-  def _accel_msg_2(self, accel):
-    values = {"ACCEL_CMD": accel}
-    return self.packer.make_can_msg_panda("ACC_CONTROL_2", 0, values)
-
-  # On a SecOC vehicle, we still transmit ACC_CONTROL but the accel value moves to ACC_CONTROL_2
-  # Verify that all non-idle accel values in ACC_CONTROL are rejected, verify ACC_CONTROL_2 accel normally
-  def _should_tx_1(self, controls_allowed: bool, accel: float, min_accel: float, max_accel: float):
-    return accel == self.INACTIVE_ACCEL
-
-  def _should_tx_2(self, controls_allowed: bool, accel: float, min_accel: float, max_accel: float):
-    return (controls_allowed and min_accel <= accel <= max_accel) or accel == self.INACTIVE_ACCEL
+  def test_343_actuation_blocked(self):
+    """
+    For SecOC cars, longitudinal acceleration must be sent in ACC_CONTROL_2, but all other ACC
+    data remains in ACC_CONTROL. Verify no actuation is sent via ACC_CONTROL.
+    """
+    for controls_allowed in [True, False]:
+      self.safety.set_controls_allowed(controls_allowed)
+      for accel in np.arange(self.MIN_ACCEL - 1, self.MAX_ACCEL + 1, 0.1):
+        should_tx = np.isclose(accel, self.INACTIVE_ACCEL, atol=0.0001)
+        self.assertEqual(should_tx, self._tx(self._accel_msg_343(accel)), f"accel: {accel=}, {controls_allowed=}, {should_tx=}")
+        self.assertEqual(should_tx, self._tx(self._accel_msg_343(accel, cancel_req=1)))
 
 
 if __name__ == "__main__":