safety: disallow longitudinal actuation while gas is pressed (#884)

* test implemented for Toyota

* fix Toyota interceptor

* use hasattr

* do Honda (still need Nidec ACC_HUD safety)

* add longitudinal_allowed to ACC_HUD (Nidec w/ no pedal

* rx not needed

* add base test for longitudinal

* update name

* stash

* do GM

* use gas_pressed_prev

* do tesla safety

* add full tests

* add get_longitudinal_allowed for simpler tests

* remove unnecessary tests and revert honda/hyundai

* fix honda after revert

* make sure releasing gas allows longitudinal again

* clean up

* clean up

* Just check longitudinal allowed

* don't need to reset detection

* use prev and add brake detection

* only on gas

* make sure we don't change current_controls_allowed braking behavior

* clean up tests

* use current_controls_allowed for honda & GM

* this is pretty much tested in common

* fix up tests and move common tests

* revert

* better place

* add for body

board/safety.h

@@ -54,7 +54,7 @@ int safety_rx_hook(CANPacket_t *to_push) {
 }
 
 int safety_tx_hook(CANPacket_t *to_send) {
-  return (relay_malfunction ? -1 : current_hooks->tx(to_send));
+  return (relay_malfunction ? -1 : current_hooks->tx(to_send, get_longitudinal_allowed()));
 }
 
 int safety_tx_lin_hook(int lin_num, uint8_t *data, int len) {
@@ -65,6 +65,10 @@ int safety_fwd_hook(int bus_num, CANPacket_t *to_fwd) {
   return (relay_malfunction ? -1 : current_hooks->fwd(bus_num, to_fwd));
 }
 
+bool get_longitudinal_allowed(void) {
+  return controls_allowed && !gas_pressed_prev;
+}
+
 // Given a CRC-8 poly, generate a static lookup table to use with a fast CRC-8
 // algorithm. Called at init time for safety modes using CRC-8.
 void gen_crc_lookup_table(uint8_t poly, uint8_t crc_lut[]) {

board/safety/safety_body.h

@@ -11,7 +11,8 @@ static int body_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int body_tx_hook(CANPacket_t *to_send) {
+static int body_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+  UNUSED(longitudinal_allowed);
 
   int tx = 0;
   int addr = GET_ADDR(to_send);

board/safety/safety_chrysler.h

@@ -112,7 +112,8 @@ static int chrysler_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int chrysler_tx_hook(CANPacket_t *to_send) {
+static int chrysler_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+  UNUSED(longitudinal_allowed);
 
   int tx = 1;
   int addr = GET_ADDR(to_send);

board/safety/safety_defaults.h

@@ -17,8 +17,9 @@ static const addr_checks* nooutput_init(int16_t param) {
   return &default_rx_checks;
 }
 
-static int nooutput_tx_hook(CANPacket_t *to_send) {
+static int nooutput_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
   UNUSED(to_send);
+  UNUSED(longitudinal_allowed);
   return false;
 }
 
@@ -57,8 +58,9 @@ static const addr_checks* alloutput_init(int16_t param) {
   return &default_rx_checks;
 }
 
-static int alloutput_tx_hook(CANPacket_t *to_send) {
+static int alloutput_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
   UNUSED(to_send);
+  UNUSED(longitudinal_allowed);
   return true;
 }
 

board/safety/safety_elm327.h

@@ -1,4 +1,5 @@
-static int elm327_tx_hook(CANPacket_t *to_send) {
+static int elm327_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+  UNUSED(longitudinal_allowed);
 
   int tx = 1;
   int addr = GET_ADDR(to_send);

board/safety/safety_ford.h

@@ -65,7 +65,8 @@ static int ford_rx_hook(CANPacket_t *to_push) {
 // else
 //     block all commands that produce actuation
 
-static int ford_tx_hook(CANPacket_t *to_send) {
+static int ford_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+  UNUSED(longitudinal_allowed);
 
   int tx = 1;
   int addr = GET_ADDR(to_send);

board/safety/safety_gm.h

@@ -112,7 +112,7 @@ static int gm_rx_hook(CANPacket_t *to_push) {
 // else
 //     block all commands that produce actuation
 
-static int gm_tx_hook(CANPacket_t *to_send) {
+static int gm_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);
@@ -134,7 +134,7 @@ static int gm_tx_hook(CANPacket_t *to_send) {
   if (addr == 789) {
     int brake = ((GET_BYTE(to_send, 0) & 0xFU) << 8) + GET_BYTE(to_send, 1);
     brake = (0x1000 - brake) & 0xFFF;
-    if (!current_controls_allowed) {
+    if (!current_controls_allowed || !longitudinal_allowed) {
       if (brake != 0) {
         tx = 0;
       }
@@ -197,8 +197,9 @@ static int gm_tx_hook(CANPacket_t *to_send) {
     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 max regen.
-    if (!current_controls_allowed) {
+    if (!current_controls_allowed || !longitudinal_allowed) {
       bool apply = GET_BYTE(to_send, 0) & 1U;
+      // Stock ECU sends max regen when not enabled
       if (apply || (gas_regen != GM_MAX_REGEN)) {
         tx = 0;
       }

board/safety/safety_honda.h

@@ -243,7 +243,7 @@ static int honda_rx_hook(CANPacket_t *to_push) {
 // else
 //     block all commands that produce actuation
 
-static int honda_tx_hook(CANPacket_t *to_send) {
+static int honda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);
@@ -271,7 +271,7 @@ static int honda_tx_hook(CANPacket_t *to_send) {
   if ((addr == 0x30C) && (bus == bus_pt)) {
     int pcm_speed = (GET_BYTE(to_send, 0) << 8) | GET_BYTE(to_send, 1);
     int pcm_gas = GET_BYTE(to_send, 2);
-    if (!current_controls_allowed) {
+    if (!current_controls_allowed || !longitudinal_allowed) {
       if ((pcm_speed != 0) || (pcm_gas != 0)) {
         tx = 0;
       }
@@ -281,7 +281,7 @@ static int honda_tx_hook(CANPacket_t *to_send) {
   // 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 (!current_controls_allowed) {
+    if (!current_controls_allowed || !longitudinal_allowed) {
       if (honda_brake != 0) {
         tx = 0;
       }
@@ -298,7 +298,7 @@ static int honda_tx_hook(CANPacket_t *to_send) {
   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 (!current_controls_allowed) {
+    if (!current_controls_allowed || !longitudinal_allowed) {
       if (accel != 0) {
         tx = 0;
       }
@@ -309,7 +309,7 @@ static int honda_tx_hook(CANPacket_t *to_send) {
 
     int gas = (GET_BYTE(to_send, 0) << 8) | GET_BYTE(to_send, 1);
     gas = to_signed(gas, 16);
-    if (!current_controls_allowed) {
+    if (!current_controls_allowed || !longitudinal_allowed) {
       if (gas != HONDA_BOSCH_NO_GAS_VALUE) {
         tx = 0;
       }
@@ -338,7 +338,7 @@ static int honda_tx_hook(CANPacket_t *to_send) {
 
   // GAS: safety check (interceptor)
   if (addr == 0x200) {
-    if (!current_controls_allowed) {
+    if (!current_controls_allowed || !longitudinal_allowed) {
       if (GET_BYTE(to_send, 0) || GET_BYTE(to_send, 1)) {
         tx = 0;
       }
@@ -401,7 +401,7 @@ static const addr_checks* honda_bosch_init(int16_t param) {
 static int honda_nidec_fwd_hook(int bus_num, CANPacket_t *to_fwd) {
   // fwd from car to camera. also fwd certain msgs from camera to car
   // 0xE4 is steering on all cars except CRV and RDX, 0x194 for CRV and RDX,
-  // 0x1FA is brake control, 0x30C is acc hud, 0x33D is lkas hud,
+  // 0x1FA is brake control, 0x30C is acc hud, 0x33D is lkas hud
   int bus_fwd = -1;
 
   if (bus_num == 0) {

board/safety/safety_hyundai.h

@@ -232,7 +232,7 @@ static int hyundai_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int hyundai_tx_hook(CANPacket_t *to_send) {
+static int hyundai_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);
@@ -264,7 +264,7 @@ static int hyundai_tx_hook(CANPacket_t *to_send) {
 
     bool violation = 0;
 
-    if (!controls_allowed) {
+    if (!longitudinal_allowed) {
       if ((desired_accel_raw != 0) || (desired_accel_val != 0)) {
         violation = 1;
       }

board/safety/safety_mazda.h

@@ -80,7 +80,9 @@ static int mazda_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int mazda_tx_hook(CANPacket_t *to_send) {
+static int mazda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+  UNUSED(longitudinal_allowed);
+
   int tx = 1;
   int addr = GET_ADDR(to_send);
   int bus = GET_BUS(to_send);

board/safety/safety_nissan.h

@@ -106,7 +106,9 @@ static int nissan_rx_hook(CANPacket_t *to_push) {
 }
 
 
-static int nissan_tx_hook(CANPacket_t *to_send) {
+static int nissan_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+  UNUSED(longitudinal_allowed);
+
   int tx = 1;
   int addr = GET_ADDR(to_send);
   bool violation = 0;

board/safety/safety_subaru.h

@@ -146,7 +146,9 @@ static int subaru_legacy_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int subaru_tx_hook(CANPacket_t *to_send) {
+static int subaru_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+  UNUSED(longitudinal_allowed);
+
   int tx = 1;
   int addr = GET_ADDR(to_send);
 
@@ -206,7 +208,9 @@ static int subaru_tx_hook(CANPacket_t *to_send) {
   return tx;
 }
 
-static int subaru_legacy_tx_hook(CANPacket_t *to_send) {
+static int subaru_legacy_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+  UNUSED(longitudinal_allowed);
+
   int tx = 1;
   int addr = GET_ADDR(to_send);
 

board/safety/safety_tesla.h

@@ -117,7 +117,8 @@ static int tesla_rx_hook(CANPacket_t *to_push) {
 }
 
 
-static int tesla_tx_hook(CANPacket_t *to_send) {
+static int tesla_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+
   int tx = 1;
   int addr = GET_ADDR(to_send);
   bool violation = false;
@@ -196,7 +197,7 @@ static int tesla_tx_hook(CANPacket_t *to_send) {
       }
 
       // Don't allow longitudinal actuation if controls aren't allowed
-      if (!controls_allowed) {
+      if (!longitudinal_allowed) {
         if ((raw_accel_max != TESLA_NO_ACCEL_VALUE) || (raw_accel_min != TESLA_NO_ACCEL_VALUE)) {
           violation = true;
         }

board/safety/safety_toyota.h

@@ -134,7 +134,7 @@ static int toyota_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int toyota_tx_hook(CANPacket_t *to_send) {
+static int toyota_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);
@@ -149,7 +149,7 @@ static int toyota_tx_hook(CANPacket_t *to_send) {
 
     // GAS PEDAL: safety check
     if (addr == 0x200) {
-      if (!controls_allowed) {
+      if (!longitudinal_allowed) {
         if (GET_BYTE(to_send, 0) || GET_BYTE(to_send, 1)) {
           tx = 0;
         }
@@ -160,7 +160,7 @@ static int toyota_tx_hook(CANPacket_t *to_send) {
     if (addr == 0x343) {
       int desired_accel = (GET_BYTE(to_send, 0) << 8) | GET_BYTE(to_send, 1);
       desired_accel = to_signed(desired_accel, 16);
-      if (!controls_allowed) {
+      if (!longitudinal_allowed) {
         if (desired_accel != 0) {
           tx = 0;
         }

board/safety/safety_volkswagen_mqb.h

@@ -145,7 +145,9 @@ static int volkswagen_mqb_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int volkswagen_mqb_tx_hook(CANPacket_t *to_send) {
+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;
 

board/safety/safety_volkswagen_pq.h

@@ -114,7 +114,9 @@ static int volkswagen_pq_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int volkswagen_pq_tx_hook(CANPacket_t *to_send) {
+static int volkswagen_pq_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+  UNUSED(longitudinal_allowed);
+
   int addr = GET_ADDR(to_send);
   int tx = 1;
 

board/safety_declarations.h

@@ -66,6 +66,7 @@ bool dist_to_meas_check(int val, int val_last, struct sample_t *val_meas,
 bool driver_limit_check(int val, int val_last, struct sample_t *val_driver,
   const int MAX, const int MAX_RATE_UP, const int MAX_RATE_DOWN,
   const int MAX_ALLOWANCE, const int DRIVER_FACTOR);
+bool get_longitudinal_allowed(void);
 bool rt_rate_limit_check(int val, int val_last, const int MAX_RT_DELTA);
 float interpolate(struct lookup_t xy, float x);
 void gen_crc_lookup_table(uint8_t poly, uint8_t crc_lut[]);
@@ -85,7 +86,7 @@ void relay_malfunction_reset(void);
 
 typedef const addr_checks* (*safety_hook_init)(int16_t param);
 typedef int (*rx_hook)(CANPacket_t *to_push);
-typedef int (*tx_hook)(CANPacket_t *to_send);
+typedef int (*tx_hook)(CANPacket_t *to_send, bool longitudinal_allowed);
 typedef int (*tx_lin_hook)(int lin_num, uint8_t *data, int len);
 typedef int (*fwd_hook)(int bus_num, CANPacket_t *to_fwd);
 

tests/safety/common.py

@@ -91,10 +91,12 @@ class InterceptorSafetyTest(PandaSafetyTestBase):
     self.safety.set_alternative_experience(ALTERNATIVE_EXPERIENCE.DISABLE_DISENGAGE_ON_GAS)
     for g in range(0, 0x1000):
       self._rx(self._interceptor_user_gas(g))
+      # Test we allow lateral, but not longitudinal
       self.assertTrue(self.safety.get_controls_allowed())
+      self.assertEqual(g <= self.INTERCEPTOR_THRESHOLD, self.safety.get_longitudinal_allowed())
+      # Make sure we can re-gain longitudinal actuation
       self._rx(self._interceptor_user_gas(0))
-      self.safety.set_gas_interceptor_detected(False)
-    self.safety.set_alternative_experience(ALTERNATIVE_EXPERIENCE.DEFAULT)
+      self.assertTrue(self.safety.get_longitudinal_allowed())
 
   def test_allow_engage_with_gas_interceptor_pressed(self):
     self._rx(self._interceptor_user_gas(0x1000))
@@ -358,7 +360,12 @@ class PandaSafetyTest(PandaSafetyTestBase):
     self.safety.set_controls_allowed(True)
     self.safety.set_alternative_experience(ALTERNATIVE_EXPERIENCE.DISABLE_DISENGAGE_ON_GAS)
     self._rx(self._user_gas_msg(self.GAS_PRESSED_THRESHOLD + 1))
+    # Test we allow lateral, but not longitudinal
     self.assertTrue(self.safety.get_controls_allowed())
+    self.assertFalse(self.safety.get_longitudinal_allowed())
+    # Make sure we can re-gain longitudinal actuation
+    self._rx(self._user_gas_msg(0))
+    self.assertTrue(self.safety.get_longitudinal_allowed())
 
   def test_prev_brake(self):
     self.assertFalse(self.safety.get_brake_pressed_prev())
@@ -393,11 +400,14 @@ class PandaSafetyTest(PandaSafetyTestBase):
     self.safety.set_controls_allowed(1)
     self._rx(self._user_brake_msg(1))
     self.assertTrue(self.safety.get_controls_allowed())
+    self.assertTrue(self.safety.get_longitudinal_allowed())
     self._rx(self._user_brake_msg(0))
     self.assertTrue(self.safety.get_controls_allowed())
+    self.assertTrue(self.safety.get_longitudinal_allowed())
     # rising edge of brake should disengage
     self._rx(self._user_brake_msg(1))
     self.assertFalse(self.safety.get_controls_allowed())
+    self.assertFalse(self.safety.get_longitudinal_allowed())
     self._rx(self._user_brake_msg(0))  # reset no brakes
 
   def test_not_allow_brake_when_moving(self):
@@ -407,9 +417,11 @@ class PandaSafetyTest(PandaSafetyTestBase):
     self._rx(self._speed_msg(self.STANDSTILL_THRESHOLD))
     self._rx(self._user_brake_msg(1))
     self.assertTrue(self.safety.get_controls_allowed())
+    self.assertTrue(self.safety.get_longitudinal_allowed())
     self._rx(self._speed_msg(self.STANDSTILL_THRESHOLD + 1))
     self._rx(self._user_brake_msg(1))
     self.assertFalse(self.safety.get_controls_allowed())
+    self.assertFalse(self.safety.get_longitudinal_allowed())
     self._rx(self._speed_msg(0))
 
   def test_sample_speed(self):

tests/safety/libpandasafety_py.py

@@ -27,6 +27,7 @@ typedef struct
 
 void set_controls_allowed(bool c);
 bool get_controls_allowed(void);
+bool get_longitudinal_allowed(void);
 void set_alternative_experience(int mode);
 int get_alternative_experience(void);
 void set_relay_malfunction(bool c);

tests/safety/test_gm.py

@@ -255,16 +255,13 @@ class TestGmSafety(common.PandaSafetyTest):
         self._rx(self._user_gas_msg(MAX_GAS))
         allow_ctrl = True
 
+      # Test we allow lateral on gas press, but never longitudinal
       self.safety.set_controls_allowed(1)
-      self.assertEqual(allow_ctrl, self._tx(self._send_brake_msg(MAX_BRAKE)))
       self.assertEqual(allow_ctrl, self._tx(self._torque_msg(MAX_RATE_UP)))
-      self.assertEqual(allow_ctrl, self._tx(self._send_gas_msg(MAX_GAS)))
+      self.assertFalse(self._tx(self._send_brake_msg(MAX_BRAKE)))
+      self.assertFalse(self._tx(self._send_gas_msg(MAX_GAS)))
 
       # reset status
-      self.safety.set_controls_allowed(0)
-      self.safety.set_alternative_experience(ALTERNATIVE_EXPERIENCE.DEFAULT)
-      self._tx(self._send_brake_msg(0))
-      self._tx(self._torque_msg(0))
       if pedal == 'brake':
         self._rx(self._speed_msg(0))
         self._rx(self._user_brake_msg(0))

tests/safety/test_honda.py

@@ -370,19 +370,12 @@ class TestHondaNidecSafetyBase(HondaBase):
 
       self.safety.set_controls_allowed(1)
       self.safety.set_honda_fwd_brake(False)
-      self.assertEqual(allow_ctrl, self._tx(self._send_brake_msg(self.MAX_BRAKE)))
-      self.assertEqual(allow_ctrl, self._tx(self._interceptor_gas_cmd(self.INTERCEPTOR_THRESHOLD)))
+
+      # Test we allow lateral, but never longitudinal
+      self.assertFalse(self._tx(self._interceptor_gas_cmd(self.INTERCEPTOR_THRESHOLD)))
+      self.assertFalse(self._tx(self._send_brake_msg(self.MAX_BRAKE)))
       self.assertEqual(allow_ctrl, self._tx(self._send_steer_msg(0x1000)))
 
-      # reset status
-      self.safety.set_controls_allowed(0)
-      self.safety.set_alternative_experience(ALTERNATIVE_EXPERIENCE.DEFAULT)
-      self._tx(self._send_brake_msg(0))
-      self._tx(self._send_steer_msg(0))
-      self._tx(self._interceptor_gas_cmd(0))
-      self.safety.set_gas_interceptor_detected(False)
-
-
 
 class TestHondaNidecSafety(HondaPcmEnableBase, TestHondaNidecSafetyBase):
   """