safety: clean up passing in longitudinal_allowed (commaai#1176)

* clean up * clean up * and fix that * interceptor func * fix * draft * fix that
budney · Dec 1, 2022 · b238d41 · b238d41
1 parent 9c50939
commit b238d41
Show file tree

Hide file tree

Showing 19 changed files with 58 additions and 67 deletions.
diff --git a/board/safety.h b/board/safety.h
@@ -70,7 +70,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, get_longitudinal_allowed()));
+  return (relay_malfunction ? -1 : current_hooks->tx(to_send));
 }
 
 int safety_tx_lin_hook(int lin_num, uint8_t *data, int len) {
@@ -484,39 +484,39 @@ float interpolate(struct lookup_t xy, float x) {
 }
 
 // Safety checks for longitudinal actuation
-bool longitudinal_accel_checks(int desired_accel, const LongitudinalLimits limits, const bool longitudinal_allowed) {
+bool longitudinal_accel_checks(int desired_accel, const LongitudinalLimits limits) {
   bool violation = false;
-  if (!longitudinal_allowed) {
+  if (!get_longitudinal_allowed()) {
     violation |= desired_accel != limits.inactive_accel;
   } else {
     violation |= max_limit_check(desired_accel, limits.max_accel, limits.min_accel);
   }
   return violation;
 }
 
-bool longitudinal_speed_checks(int desired_speed, const LongitudinalLimits limits, const bool longitudinal_allowed) {
-  return !longitudinal_allowed && (desired_speed != limits.inactive_speed);
+bool longitudinal_speed_checks(int desired_speed, const LongitudinalLimits limits) {
+  return !get_longitudinal_allowed() && (desired_speed != limits.inactive_speed);
 }
 
-bool longitudinal_gas_checks(int desired_gas, const LongitudinalLimits limits, const bool longitudinal_allowed) {
+bool longitudinal_gas_checks(int desired_gas, const LongitudinalLimits limits) {
   bool violation = false;
-  if (!longitudinal_allowed) {
+  if (!get_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 longitudinal_brake_checks(int desired_brake, const LongitudinalLimits limits) {
   bool violation = false;
-  violation |= !longitudinal_allowed && (desired_brake != 0);
+  violation |= !get_longitudinal_allowed() && (desired_brake != 0);
   violation |= desired_brake > limits.max_brake;
   return violation;
 }
 
-bool longitudinal_interceptor_checks(CANPacket_t *to_send, bool longitudinal_allowed) {
-  return !longitudinal_allowed && (GET_BYTE(to_send, 0) || GET_BYTE(to_send, 1));
+bool longitudinal_interceptor_checks(CANPacket_t *to_send) {
+  return !get_longitudinal_allowed() && (GET_BYTE(to_send, 0) || GET_BYTE(to_send, 1));
 }
 
 // Safety checks for torque-based steering commands

diff --git a/board/safety/safety_body.h b/board/safety/safety_body.h
@@ -16,8 +16,7 @@ static int body_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int body_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
-  UNUSED(longitudinal_allowed);
+static int body_tx_hook(CANPacket_t *to_send) {
 
   int tx = 0;
   int addr = GET_ADDR(to_send);
@@ -30,8 +29,8 @@ static int body_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
   if (msg_allowed(to_send, BODY_TX_MSGS, sizeof(BODY_TX_MSGS)/sizeof(BODY_TX_MSGS[0])) && controls_allowed) {
     tx = 1;
   }
-  
-  // Allow going into CAN flashing mode even if controls are not allowed 
+
+  // Allow going into CAN flashing mode even if controls are not allowed
   if (!controls_allowed && ((uint32_t)GET_BYTES_04(to_send) == 0xdeadfaceU) && ((uint32_t)GET_BYTES_48(to_send) == 0x0ab00b1eU)) {
     tx = 1;
   }

diff --git a/board/safety/safety_chrysler.h b/board/safety/safety_chrysler.h
@@ -226,8 +226,7 @@ static int chrysler_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int chrysler_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
-  UNUSED(longitudinal_allowed);
+static int chrysler_tx_hook(CANPacket_t *to_send) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);

diff --git a/board/safety/safety_defaults.h b/board/safety/safety_defaults.h
@@ -15,9 +15,8 @@ static const addr_checks* nooutput_init(uint16_t param) {
   return &default_rx_checks;
 }
 
-static int nooutput_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+static int nooutput_tx_hook(CANPacket_t *to_send) {
   UNUSED(to_send);
-  UNUSED(longitudinal_allowed);
   return false;
 }
 
@@ -54,9 +53,8 @@ static const addr_checks* alloutput_init(uint16_t param) {
   return &default_rx_checks;
 }
 
-static int alloutput_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+static int alloutput_tx_hook(CANPacket_t *to_send) {
   UNUSED(to_send);
-  UNUSED(longitudinal_allowed);
   return true;
 }
 

diff --git a/board/safety/safety_elm327.h b/board/safety/safety_elm327.h
@@ -1,5 +1,4 @@
-static int elm327_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
-  UNUSED(longitudinal_allowed);
+static int elm327_tx_hook(CANPacket_t *to_send) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);

diff --git a/board/safety/safety_ford.h b/board/safety/safety_ford.h
@@ -76,8 +76,7 @@ static int ford_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int ford_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
-  UNUSED(longitudinal_allowed);
+static int ford_tx_hook(CANPacket_t *to_send) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);

diff --git a/board/safety/safety_gm.h b/board/safety/safety_gm.h
@@ -147,7 +147,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, bool longitudinal_allowed) {
+static int gm_tx_hook(CANPacket_t *to_send) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);
@@ -166,7 +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_brake_checks(brake, *gm_long_limits, longitudinal_allowed)) {
+    if (longitudinal_brake_checks(brake, *gm_long_limits)) {
       tx = 0;
     }
   }
@@ -189,7 +189,7 @@ static int gm_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
     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);
+    violation |= longitudinal_gas_checks(gas_regen, *gm_long_limits);
 
     if (violation) {
       tx = 0;

diff --git a/board/safety/safety_honda.h b/board/safety/safety_honda.h
@@ -260,7 +260,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, bool longitudinal_allowed) {
+static int honda_tx_hook(CANPacket_t *to_send) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);
@@ -285,8 +285,8 @@ static int honda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
     int pcm_gas = GET_BYTE(to_send, 2);
 
     bool violation = false;
-    violation |= longitudinal_speed_checks(pcm_speed, HONDA_NIDEC_LONG_LIMITS, longitudinal_allowed);
-    violation |= longitudinal_gas_checks(pcm_gas, HONDA_NIDEC_LONG_LIMITS, longitudinal_allowed);
+    violation |= longitudinal_speed_checks(pcm_speed, HONDA_NIDEC_LONG_LIMITS);
+    violation |= longitudinal_gas_checks(pcm_gas, HONDA_NIDEC_LONG_LIMITS);
     if (violation) {
       tx = 0;
     }
@@ -295,7 +295,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_brake_checks(honda_brake, HONDA_NIDEC_LONG_LIMITS, longitudinal_allowed)) {
+    if (longitudinal_brake_checks(honda_brake, HONDA_NIDEC_LONG_LIMITS)) {
       tx = 0;
     }
     if (honda_fwd_brake) {
@@ -312,8 +312,8 @@ static int honda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
     gas = to_signed(gas, 16);
 
     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);
+    violation |= longitudinal_accel_checks(accel, HONDA_BOSCH_LONG_LIMITS);
+    violation |= longitudinal_gas_checks(gas, HONDA_BOSCH_LONG_LIMITS);
     if (violation) {
       tx = 0;
     }
@@ -338,7 +338,7 @@ static int honda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
 
   // GAS: safety check (interceptor)
   if (addr == 0x200) {
-    if (longitudinal_interceptor_checks(to_send, longitudinal_allowed)) {
+    if (longitudinal_interceptor_checks(to_send)) {
       tx = 0;
     }
   }

diff --git a/board/safety/safety_hyundai.h b/board/safety/safety_hyundai.h
@@ -227,7 +227,7 @@ static int hyundai_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int hyundai_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+static int hyundai_tx_hook(CANPacket_t *to_send) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);
@@ -261,8 +261,8 @@ static int hyundai_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
 
     bool violation = false;
 
-    violation |= longitudinal_accel_checks(desired_accel_raw, HYUNDAI_LONG_LIMITS, longitudinal_allowed);
-    violation |= longitudinal_accel_checks(desired_accel_val, HYUNDAI_LONG_LIMITS, longitudinal_allowed);
+    violation |= longitudinal_accel_checks(desired_accel_raw, HYUNDAI_LONG_LIMITS);
+    violation |= longitudinal_accel_checks(desired_accel_val, HYUNDAI_LONG_LIMITS);
     violation |= (aeb_decel_cmd != 0);
     violation |= (aeb_req != 0);
 

diff --git a/board/safety/safety_hyundai_canfd.h b/board/safety/safety_hyundai_canfd.h
@@ -244,8 +244,7 @@ static int hyundai_canfd_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int hyundai_canfd_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
-  UNUSED(longitudinal_allowed);
+static int hyundai_canfd_tx_hook(CANPacket_t *to_send) {
 
   int tx = 0;
   int addr = GET_ADDR(to_send);
@@ -296,8 +295,8 @@ static int hyundai_canfd_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed
     bool violation = false;
 
     if (hyundai_longitudinal) {
-      violation |= longitudinal_accel_checks(desired_accel_raw, HYUNDAI_LONG_LIMITS, longitudinal_allowed);
-      violation |= longitudinal_accel_checks(desired_accel_val, HYUNDAI_LONG_LIMITS, longitudinal_allowed);
+      violation |= longitudinal_accel_checks(desired_accel_raw, HYUNDAI_LONG_LIMITS);
+      violation |= longitudinal_accel_checks(desired_accel_val, HYUNDAI_LONG_LIMITS);
     } else {
       // only used to cancel on here
       if ((desired_accel_raw != 0) || (desired_accel_val != 0)) {

diff --git a/board/safety/safety_mazda.h b/board/safety/safety_mazda.h
@@ -72,8 +72,7 @@ static int mazda_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int mazda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
-  UNUSED(longitudinal_allowed);
+static int mazda_tx_hook(CANPacket_t *to_send) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);

diff --git a/board/safety/safety_nissan.h b/board/safety/safety_nissan.h
@@ -100,8 +100,7 @@ static int nissan_rx_hook(CANPacket_t *to_push) {
 }
 
 
-static int nissan_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
-  UNUSED(longitudinal_allowed);
+static int nissan_tx_hook(CANPacket_t *to_send) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);

diff --git a/board/safety/safety_subaru.h b/board/safety/safety_subaru.h
@@ -120,8 +120,7 @@ static int subaru_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int subaru_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
-  UNUSED(longitudinal_allowed);
+static int subaru_tx_hook(CANPacket_t *to_send) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);

diff --git a/board/safety/safety_subaru_legacy.h b/board/safety/safety_subaru_legacy.h
@@ -61,8 +61,7 @@ static int subaru_legacy_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int subaru_legacy_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
-  UNUSED(longitudinal_allowed);
+static int subaru_legacy_tx_hook(CANPacket_t *to_send) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);

diff --git a/board/safety/safety_tesla.h b/board/safety/safety_tesla.h
@@ -115,7 +115,7 @@ static int tesla_rx_hook(CANPacket_t *to_push) {
 }
 
 
-static int tesla_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+static int tesla_tx_hook(CANPacket_t *to_send) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);
@@ -188,8 +188,8 @@ static int tesla_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
       // Don't allow any acceleration limits above the safety limits
       int raw_accel_max = ((GET_BYTE(to_send, 6) & 0x1FU) << 4) | (GET_BYTE(to_send, 5) >> 4);
       int raw_accel_min = ((GET_BYTE(to_send, 5) & 0x0FU) << 5) | (GET_BYTE(to_send, 4) >> 3);
-      violation |= longitudinal_accel_checks(raw_accel_max, TESLA_LONG_LIMITS, longitudinal_allowed);
-      violation |= longitudinal_accel_checks(raw_accel_min, TESLA_LONG_LIMITS, longitudinal_allowed);
+      violation |= longitudinal_accel_checks(raw_accel_max, TESLA_LONG_LIMITS);
+      violation |= longitudinal_accel_checks(raw_accel_min, TESLA_LONG_LIMITS);
     } else {
       violation = true;
     }

diff --git a/board/safety/safety_toyota.h b/board/safety/safety_toyota.h
@@ -132,7 +132,7 @@ static int toyota_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int toyota_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+static int toyota_tx_hook(CANPacket_t *to_send) {
 
   int tx = 1;
   int addr = GET_ADDR(to_send);
@@ -147,7 +147,7 @@ static int toyota_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
 
     // GAS PEDAL: safety check
     if (addr == 0x200) {
-      if (longitudinal_interceptor_checks(to_send, longitudinal_allowed)) {
+      if (longitudinal_interceptor_checks(to_send)) {
         tx = 0;
       }
     }
@@ -158,15 +158,17 @@ static int toyota_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
       desired_accel = to_signed(desired_accel, 16);
 
       bool violation = false;
-      violation |= longitudinal_accel_checks(desired_accel, TOYOTA_LONG_LIMITS, longitudinal_allowed);
-      violation |= longitudinal_accel_checks(desired_accel, TOYOTA_LONG_LIMITS, !toyota_stock_longitudinal);
+      violation |= longitudinal_accel_checks(desired_accel, TOYOTA_LONG_LIMITS);
 
       // only ACC messages that cancel are allowed when openpilot is not controlling longitudinal
       if (toyota_stock_longitudinal) {
         bool cancel_req = GET_BIT(to_send, 24U) != 0U;
         if (!cancel_req) {
           violation = true;
         }
+        if (desired_accel != TOYOTA_LONG_LIMITS.inactive_accel) {
+          violation = true;
+        }
       }
 
       if (violation) {

diff --git a/board/safety/safety_volkswagen_mqb.h b/board/safety/safety_volkswagen_mqb.h
@@ -202,7 +202,7 @@ static int volkswagen_mqb_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int volkswagen_mqb_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+static int volkswagen_mqb_tx_hook(CANPacket_t *to_send) {
   int addr = GET_ADDR(to_send);
   int tx = 1;
 
@@ -244,7 +244,7 @@ static int volkswagen_mqb_tx_hook(CANPacket_t *to_send, bool longitudinal_allowe
       desired_accel = (((GET_BYTE(to_send, 7) << 3) | ((GET_BYTE(to_send, 6) & 0xE0U) >> 5)) * 5U) - 7220U;
     }
 
-    violation |= longitudinal_accel_checks(desired_accel, VOLKSWAGEN_MQB_LONG_LIMITS, longitudinal_allowed);
+    violation |= longitudinal_accel_checks(desired_accel, VOLKSWAGEN_MQB_LONG_LIMITS);
 
     if (violation) {
       tx = 0;

diff --git a/board/safety/safety_volkswagen_pq.h b/board/safety/safety_volkswagen_pq.h
@@ -177,7 +177,7 @@ static int volkswagen_pq_rx_hook(CANPacket_t *to_push) {
   return valid;
 }
 
-static int volkswagen_pq_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
+static int volkswagen_pq_tx_hook(CANPacket_t *to_send) {
   int addr = GET_ADDR(to_send);
   int tx = 1;
 
@@ -209,7 +209,7 @@ static int volkswagen_pq_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed
     // Signal: ACC_System.ACS_Sollbeschl (acceleration in m/s2, scale 0.005, offset -7.22)
     int desired_accel = ((((GET_BYTE(to_send, 4) & 0x7U) << 8) | GET_BYTE(to_send, 3)) * 5U) - 7220U;
 
-    if (longitudinal_accel_checks(desired_accel, VOLKSWAGEN_PQ_LONG_LIMITS, longitudinal_allowed)) {
+    if (longitudinal_accel_checks(desired_accel, VOLKSWAGEN_PQ_LONG_LIMITS)) {
       tx = 0;
     }
   }