safety: common gas & brake safety checks (commaai#1168)

* do toyota, hyundai, gm * comments * honda draft * Revert "honda draft" This reverts commit a1f466a. * do tesla * vw draft * finish vw * fix safety * clean up * fix function names * do honda and fix safety test not checking pos accel for bosch * clean that up
budney · Nov 30, 2022 · e8bd1df · e8bd1df
1 parent e6f62a6
commit e8bd1df
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 66 deletions.
diff --git a/board/safety.h b/board/safety.h
@@ -494,6 +494,23 @@ bool longitudinal_accel_checks(int desired_accel, const LongitudinalLimits limit
   return violation;
 }
 
+bool longitudinal_gas_checks(int desired_gas, const LongitudinalLimits limits, const bool longitudinal_allowed) {
+  bool violation = false;
+  if (!longitudinal_allowed) {
+    violation |= desired_gas != limits.inactive_gas;
+  } else {
+    violation |= max_limit_check(desired_gas, limits.max_gas, limits.min_gas);
+  }
+  return violation;
+}
+
+bool longitudinal_brake_checks(int desired_brake, const LongitudinalLimits limits, const bool longitudinal_allowed) {
+  bool violation = false;
+  violation |= !longitudinal_allowed && (desired_brake != 0);
+  violation |= desired_brake > limits.max_brake;
+  return violation;
+}
+
 // Safety checks for torque-based steering commands
 bool steer_torque_cmd_checks(int desired_torque, int steer_req, const SteeringLimits limits) {
   bool violation = false;

diff --git a/board/safety/safety_gm.h b/board/safety/safety_gm.h
@@ -9,28 +9,21 @@ const SteeringLimits GM_STEERING_LIMITS = {
   .type = TorqueDriverLimited,
 };
 
-typedef struct {
-  const int max_gas;
-  const int max_regen;
-  const int inactive_regen;
-  const int max_brake;
-} GmLongLimits;
-
-const GmLongLimits GM_ASCM_LONG_LIMITS = {
+const LongitudinalLimits GM_ASCM_LONG_LIMITS = {
   .max_gas = 3072,
-  .max_regen = 1404,
-  .inactive_regen = 1404,
+  .min_gas = 1404,
+  .inactive_gas = 1404,
   .max_brake = 400,
 };
 
-const GmLongLimits GM_CAM_LONG_LIMITS = {
+const LongitudinalLimits GM_CAM_LONG_LIMITS = {
   .max_gas = 3400,
-  .max_regen = 1514,
-  .inactive_regen = 1554,
+  .min_gas = 1514,
+  .inactive_gas = 1554,
   .max_brake = 400,
 };
 
-const GmLongLimits *gm_long_limits;
+const LongitudinalLimits *gm_long_limits;
 
 const int GM_STANDSTILL_THRSLD = 10;  // 0.311kph
 
@@ -173,12 +166,7 @@ static int gm_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
   if (addr == 789) {
     int brake = ((GET_BYTE(to_send, 0) & 0xFU) << 8) + GET_BYTE(to_send, 1);
     brake = (0x1000 - brake) & 0xFFF;
-    if (!longitudinal_allowed) {
-      if (brake != 0) {
-        tx = 0;
-      }
-    }
-    if (brake > gm_long_limits->max_brake) {
+    if (longitudinal_brake_checks(brake, *gm_long_limits, longitudinal_allowed)) {
       tx = 0;
     }
   }
@@ -195,23 +183,15 @@ static int gm_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
 
   // GAS/REGEN: safety check
   if (addr == 715) {
+    bool apply = GET_BIT(to_send, 0U) != 0U;
     int gas_regen = ((GET_BYTE(to_send, 2) & 0x7FU) << 5) + ((GET_BYTE(to_send, 3) & 0xF8U) >> 3);
-    // Disabled message is !engaged with gas
-    // value that corresponds to inactive regen.
-    if (!longitudinal_allowed) {
-      if (gas_regen != gm_long_limits->inactive_regen) {
-        tx = 0;
-      }
-    }
-    // Need to allow apply bit in pre-enabled and overriding states
-    if (!controls_allowed) {
-      bool apply = GET_BIT(to_send, 0U) != 0U;
-      if (apply) {
-        tx = 0;
-      }
-    }
-    // Enforce gas/regen actuation limits (max_regen <= gas_regen <= max_gas)
-    if ((gas_regen < gm_long_limits->max_regen) || (gas_regen > gm_long_limits->max_gas)) {
+
+    bool violation = false;
+    // Allow apply bit in pre-enabled and overriding states
+    violation |= !controls_allowed && apply;
+    violation |= longitudinal_gas_checks(gas_regen, *gm_long_limits, longitudinal_allowed);
+
+    if (violation) {
       tx = 0;
     }
   }

diff --git a/board/safety/safety_honda.h b/board/safety/safety_honda.h
@@ -16,9 +16,19 @@ const CanMsg HONDA_BOSCH_LONG_TX_MSGS[] = {{0xE4, 1, 5}, {0x1DF, 1, 8}, {0x1EF,
 // Threshold calculated from DBC gains: round(((83.3 / 0.253984064) + (83.3 / 0.126992032)) / 2) = 492
 const int HONDA_GAS_INTERCEPTOR_THRESHOLD = 492;
 #define HONDA_GET_INTERCEPTOR(msg) (((GET_BYTE((msg), 0) << 8) + GET_BYTE((msg), 1) + (GET_BYTE((msg), 2) << 8) + GET_BYTE((msg), 3)) / 2U)  // avg between 2 tracks
-const int HONDA_BOSCH_NO_GAS_VALUE = -30000; // value sent when not requesting gas
-const int HONDA_BOSCH_GAS_MAX = 2000;
-const int HONDA_BOSCH_ACCEL_MIN = -350; // max braking == -3.5m/s2
+
+const LongitudinalLimits HONDA_BOSCH_LONG_LIMITS = {
+  .max_accel = 200,   // accel is used for brakes
+  .min_accel = -350,
+
+  .max_gas = 2000,
+  .min_gas = -30000,
+  .inactive_gas = -30000,
+};
+
+const LongitudinalLimits HONDA_NIDEC_LONG_LIMITS = {
+  .max_brake = 255,
+};
 
 // Nidec and bosch radarless has the powertrain bus on bus 0
 AddrCheckStruct honda_common_addr_checks[] = {
@@ -280,12 +290,7 @@ static int honda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
   // BRAKE: safety check (nidec)
   if ((addr == 0x1FA) && (bus == bus_pt)) {
     honda_brake = (GET_BYTE(to_send, 0) << 2) + ((GET_BYTE(to_send, 1) >> 6) & 0x3U);
-    if (!longitudinal_allowed) {
-      if (honda_brake != 0) {
-        tx = 0;
-      }
-    }
-    if (honda_brake > 255) {
+    if (longitudinal_brake_checks(honda_brake, HONDA_NIDEC_LONG_LIMITS, longitudinal_allowed)) {
       tx = 0;
     }
     if (honda_fwd_brake) {
@@ -297,23 +302,14 @@ static int honda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
   if ((addr == 0x1DF) && (bus == bus_pt)) {
     int accel = (GET_BYTE(to_send, 3) << 3) | ((GET_BYTE(to_send, 4) >> 5) & 0x7U);
     accel = to_signed(accel, 11);
-    if (!longitudinal_allowed) {
-      if (accel != 0) {
-        tx = 0;
-      }
-    }
-    if (accel < HONDA_BOSCH_ACCEL_MIN) {
-      tx = 0;
-    }
 
     int gas = (GET_BYTE(to_send, 0) << 8) | GET_BYTE(to_send, 1);
     gas = to_signed(gas, 16);
-    if (!longitudinal_allowed) {
-      if (gas != HONDA_BOSCH_NO_GAS_VALUE) {
-        tx = 0;
-      }
-    }
-    if (gas > HONDA_BOSCH_GAS_MAX) {
+
+    bool violation = false;
+    violation |= longitudinal_accel_checks(accel, HONDA_BOSCH_LONG_LIMITS, longitudinal_allowed);
+    violation |= longitudinal_gas_checks(gas, HONDA_BOSCH_LONG_LIMITS, longitudinal_allowed);
+    if (violation) {
       tx = 0;
     }
   }

diff --git a/board/safety/safety_volkswagen_mqb.h b/board/safety/safety_volkswagen_mqb.h
@@ -203,8 +203,6 @@ static int volkswagen_mqb_rx_hook(CANPacket_t *to_push) {
 }
 
 static int volkswagen_mqb_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
-  UNUSED(longitudinal_allowed);
-
   int addr = GET_ADDR(to_send);
   int tx = 1;
 

diff --git a/board/safety_declarations.h b/board/safety_declarations.h
@@ -59,6 +59,13 @@ typedef struct {
   const int max_accel;
   const int min_accel;
   const int inactive_accel;
+
+  // gas & brake cmd limits
+  // inactive and min gas are 0 on most safety modes
+  const int max_gas;
+  const int min_gas;
+  const int inactive_gas;
+  const int max_brake;
 } LongitudinalLimits;
 
 typedef struct {
@@ -121,6 +128,8 @@ void relay_malfunction_set(void);
 void relay_malfunction_reset(void);
 bool steer_torque_cmd_checks(int desired_torque, int steer_req, const SteeringLimits limits);
 bool longitudinal_accel_checks(int desired_accel, const LongitudinalLimits limits, const bool longitudinal_allowed);
+bool longitudinal_gas_checks(int desired_gas, const LongitudinalLimits limits, const bool longitudinal_allowed);
+bool longitudinal_brake_checks(int desired_brake, const LongitudinalLimits limits, const bool longitudinal_allowed);
 void pcm_cruise_check(bool cruise_engaged);
 
 typedef const addr_checks* (*safety_hook_init)(uint16_t param);

diff --git a/tests/safety/test_honda.py b/tests/safety/test_honda.py
@@ -467,7 +467,8 @@ class TestHondaBoschLongSafety(HondaButtonEnableBase, TestHondaBoschSafetyBase):
   """
   NO_GAS = -30000
   MAX_GAS = 2000
-  MAX_BRAKE = -3.5
+  MAX_ACCEL = 2.0  # accel is used for brakes, but openpilot can set positive values
+  MIN_ACCEL = -3.5
 
   STEER_BUS = 1
   TX_MSGS = [[0xE4, 1], [0x1DF, 1], [0x1EF, 1], [0x1FA, 1], [0x30C, 1], [0x33D, 1], [0x33DA, 1], [0x33DB, 1], [0x39F, 1], [0x18DAB0F1, 1]]
@@ -509,14 +510,14 @@ def test_gas_safety_check(self):
         accel = 0 if gas < 0 else gas / 1000
         self.safety.set_controls_allowed(controls_allowed)
         send = gas <= self.MAX_GAS if controls_allowed else gas == self.NO_GAS
-        self.assertEqual(send, self.safety.safety_tx_hook(self._send_gas_brake_msg(gas, accel)), gas)
+        self.assertEqual(send, self.safety.safety_tx_hook(self._send_gas_brake_msg(gas, accel)), (controls_allowed, gas, accel))
 
   def test_brake_safety_check(self):
     for controls_allowed in [True, False]:
-      for accel in np.arange(0, self.MAX_BRAKE - 1, -0.01):
-        accel = round(accel, 2) # floats might not hit exact boundary conditions without rounding
+      for accel in np.arange(self.MIN_ACCEL - 1, self.MAX_ACCEL + 1, 0.01):
+        accel = round(accel, 2)  # floats might not hit exact boundary conditions without rounding
         self.safety.set_controls_allowed(controls_allowed)
-        send = self.MAX_BRAKE <= accel <= 0 if controls_allowed else accel == 0
+        send = self.MIN_ACCEL <= accel <= self.MAX_ACCEL if controls_allowed else accel == 0
         self.assertEqual(send, self._tx(self._send_gas_brake_msg(self.NO_GAS, accel)), (controls_allowed, accel))