VW: explicitly test for inactive accel (commaai#1167)

* explicitely test for inactive accel * fix the safety * and catch the mistake * revert
budney · Nov 29, 2022 · 8accca1 · 8accca1
1 parent ba6b371
commit 8accca1
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 11 deletions.
diff --git a/board/safety/safety_volkswagen_pq.h b/board/safety/safety_volkswagen_pq.h
@@ -209,17 +209,13 @@ 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)
     desired_accel = ((((GET_BYTE(to_send, 4) & 0x7U) << 8) | GET_BYTE(to_send, 3)) * 5U) - 7220U;
 
-    // VW send one increment above the max range when inactive
-    if (desired_accel == 3010) {
-      desired_accel = 0;
-    }
-
-    if (!longitudinal_allowed && (desired_accel != 0)) {
-      violation = 1;
+    // VW sends one increment above the max range when inactive
+    if (!longitudinal_allowed) {
+      violation |= desired_accel != 3010;
+    } else {
+      violation |= max_limit_check(desired_accel, VOLKSWAGEN_PQ_MAX_ACCEL, VOLKSWAGEN_PQ_MIN_ACCEL);
     }
 
-    violation |= max_limit_check(desired_accel, VOLKSWAGEN_PQ_MAX_ACCEL, VOLKSWAGEN_PQ_MIN_ACCEL);
-
     if (violation) {
       tx = 0;
     }

diff --git a/tests/safety/test_volkswagen_pq.py b/tests/safety/test_volkswagen_pq.py
@@ -146,6 +146,7 @@ class TestVolkswagenPqLongSafety(TestVolkswagenPqSafety):
   TX_MSGS = [[MSG_HCA_1, 0], [MSG_LDW_1, 0], [MSG_ACC_SYSTEM, 0], [MSG_ACC_GRA_ANZIEGE, 0]]
   FWD_BLACKLISTED_ADDRS = {2: [MSG_HCA_1, MSG_LDW_1, MSG_ACC_SYSTEM, MSG_ACC_GRA_ANZIEGE]}
   FWD_BUS_LOOKUP = {0: 2, 2: 0}
+  INACTIVE_ACCEL = 3.01
 
   def setUp(self):
     self.packer = CANPackerPanda("vw_golf_mk4")
@@ -193,9 +194,9 @@ def test_main_switch(self):
 
   def test_accel_safety_check(self):
     for controls_allowed in [True, False]:
-      for accel in np.arange(MIN_ACCEL - 1, MAX_ACCEL + 1, 0.01):
+      for accel in np.arange(MIN_ACCEL - 2, MAX_ACCEL + 2, 0.01):
         accel = round(accel, 2)  # floats might not hit exact boundary conditions without rounding
-        send = MIN_ACCEL <= accel <= MAX_ACCEL if controls_allowed else accel == 0
+        send = MIN_ACCEL <= accel <= MAX_ACCEL if controls_allowed else accel == self.INACTIVE_ACCEL
         self.safety.set_controls_allowed(controls_allowed)
         # primary accel request used by ECU
         self.assertEqual(send, self._tx(self._accel_msg(accel)), (controls_allowed, accel))