GM: handle run-away set speed (commaai#1156)

* Revert "GM: enable on rising edge of resume (commaai#1155)" This reverts commit 5aa33e1. * allow set/resume to any other button (except cancel) * explicit test * This is a test of both PRs combined * Revert "This is a test of both PRs combined" This reverts commit c7fdc3b. * fix test
budney · Nov 12, 2022 · 0096d0c · 0096d0c
1 parent ff48ecc
commit 0096d0c
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 16 deletions.
diff --git a/board/safety/safety_gm.h b/board/safety/safety_gm.h
@@ -105,18 +105,18 @@ static int gm_rx_hook(CANPacket_t *to_push) {
     if ((addr == 481) && !gm_pcm_cruise) {
       int button = (GET_BYTE(to_push, 5) & 0x70U) >> 4;
 
-      // exit controls on cancel press
-      if (button == GM_BTN_CANCEL) {
-        controls_allowed = 0;
-      }
-
       // enter controls on falling edge of set or resume
-      bool set = (button == GM_BTN_UNPRESS) && (cruise_button_prev == GM_BTN_SET);
-      bool res = (button == GM_BTN_UNPRESS) && (cruise_button_prev == GM_BTN_RESUME);
+      bool set = (button != GM_BTN_SET) && (cruise_button_prev == GM_BTN_SET);
+      bool res = (button != GM_BTN_RESUME) && (cruise_button_prev == GM_BTN_RESUME);
       if (set || res) {
         controls_allowed = 1;
       }
 
+      // exit controls on cancel press
+      if (button == GM_BTN_CANCEL) {
+        controls_allowed = 0;
+      }
+
       cruise_button_prev = button;
     }
 

diff --git a/tests/safety/test_gm.py b/tests/safety/test_gm.py
@@ -23,16 +23,21 @@ def test_set_resume_buttons(self):
     """
       SET and RESUME enter controls allowed on their falling edge.
     """
-    for btn in range(8):
-      self.safety.set_controls_allowed(0)
-      for _ in range(10):
-        self._rx(self._button_msg(btn))
-        self.assertFalse(self.safety.get_controls_allowed())
-
-      # should enter controls allowed on falling edge
-      if btn in (Buttons.RES_ACCEL, Buttons.DECEL_SET):
+    for btn_prev in range(8):
+      for btn_cur in range(8):
         self._rx(self._button_msg(Buttons.UNPRESS))
-        self.assertTrue(self.safety.get_controls_allowed())
+        self.safety.set_controls_allowed(0)
+        for _ in range(10):
+          self._rx(self._button_msg(btn_prev))
+          self.assertFalse(self.safety.get_controls_allowed())
+
+        # should enter controls allowed on falling edge and not transitioning to cancel
+        should_enable = btn_cur != btn_prev and \
+                        btn_cur != Buttons.CANCEL and \
+                        btn_prev in (Buttons.RES_ACCEL, Buttons.DECEL_SET)
+
+        self._rx(self._button_msg(btn_cur))
+        self.assertEqual(should_enable, self.safety.get_controls_allowed())
 
   def test_cancel_button(self):
     self.safety.set_controls_allowed(1)