HKG: only allow engagement on user button press (commaai#24140)

* use deque for prev_cruise_buttons

* stash

* disallow engagement if resume or decel isn't pressed

* fix buttons

* reduce chances of causing 30-frame fault

Fix clu11

drive down cancel times

* Revert "reduce chances of causing 30-frame fault"

This reverts commit 0f54c05.

* consider pause/resume button

* stash

* Revert "stash"

This reverts commit 551ca7b.

* sadly some cars need 8 op frames (identical to 4 updates from CLU11)

* add main button

* 6 should be safe

* use max

* clean up

* change to 4 samples and process all messages received, like panda

* bump panda

* test: replay segments with known re-engagements on-device

* Revert "test: replay segments with known re-engagements on-device"

This reverts commit 9730c3c.

* need prev_cruise_buttons as we don't run every CAN message through carstate

* more generic

* extend

* Update selfdrive/car/interfaces.py

Co-authored-by: Adeeb Shihadeh <adeebshihadeh@gmail.com>

* 3% faster at 1000000 loops

* update refs

Co-authored-by: Adeeb Shihadeh <adeebshihadeh@gmail.com>

selfdrive/car/hyundai/carstate.py

@@ -1,25 +1,31 @@
+from collections import deque
 import copy
+
 from cereal import car
 from common.conversions import Conversions as CV
 from opendbc.can.parser import CANParser
 from opendbc.can.can_define import CANDefine
-from selfdrive.car.hyundai.values import DBC, STEER_THRESHOLD, FEATURES, EV_CAR, HYBRID_CAR
+from selfdrive.car.hyundai.values import DBC, STEER_THRESHOLD, FEATURES, EV_CAR, HYBRID_CAR, Buttons
 from selfdrive.car.interfaces import CarStateBase
 
+PREV_BUTTON_SAMPLES = 4
+
 
 class CarState(CarStateBase):
   def __init__(self, CP):
     super().__init__(CP)
     can_define = CANDefine(DBC[CP.carFingerprint]["pt"])
 
+    self.cruise_buttons = deque([Buttons.NONE] * PREV_BUTTON_SAMPLES, maxlen=PREV_BUTTON_SAMPLES)
+    self.main_buttons = deque([Buttons.NONE] * PREV_BUTTON_SAMPLES, maxlen=PREV_BUTTON_SAMPLES)
+
     if self.CP.carFingerprint in FEATURES["use_cluster_gears"]:
       self.shifter_values = can_define.dv["CLU15"]["CF_Clu_Gear"]
     elif self.CP.carFingerprint in FEATURES["use_tcu_gears"]:
       self.shifter_values = can_define.dv["TCU12"]["CUR_GR"]
     else:  # preferred and elect gear methods use same definition
       self.shifter_values = can_define.dv["LVR12"]["CF_Lvr_Gear"]
 
-
   def update(self, cp, cp_cam):
     ret = car.CarState.new_message()
 
@@ -107,9 +113,10 @@ def update(self, cp, cp_cam):
     self.lkas11 = copy.copy(cp_cam.vl["LKAS11"])
     self.clu11 = copy.copy(cp.vl["CLU11"])
     self.steer_state = cp.vl["MDPS12"]["CF_Mdps_ToiActive"]  # 0 NOT ACTIVE, 1 ACTIVE
-    self.brake_error = cp.vl["TCS13"]["ACCEnable"] != 0 # 0 ACC CONTROL ENABLED, 1-3 ACC CONTROL DISABLED
-    self.prev_cruise_buttons = self.cruise_buttons
-    self.cruise_buttons = cp.vl["CLU11"]["CF_Clu_CruiseSwState"]
+    self.brake_error = cp.vl["TCS13"]["ACCEnable"] != 0  # 0 ACC CONTROL ENABLED, 1-3 ACC CONTROL DISABLED
+    self.prev_cruise_buttons = self.cruise_buttons[-1]
+    self.cruise_buttons.extend(cp.vl_all["CLU11"]["CF_Clu_CruiseSwState"])
+    self.main_buttons.extend(cp.vl_all["CLU11"]["CF_Clu_CruiseSwMain"])
 
     return ret
 

selfdrive/car/hyundai/interface.py

@@ -10,6 +10,8 @@
 
 ButtonType = car.CarState.ButtonEvent.Type
 EventName = car.CarEvent.EventName
+ENABLE_BUTTONS = (Buttons.RES_ACCEL, Buttons.SET_DECEL, Buttons.CANCEL)
+
 
 class CarInterface(CarInterfaceBase):
   @staticmethod
@@ -315,20 +317,24 @@ def _update(self, c):
     ret = self.CS.update(self.cp, self.cp_cam)
     ret.steeringRateLimited = self.CC.steer_rate_limited if self.CC is not None else False
 
-    events = self.create_common_events(ret, pcm_enable=self.CS.CP.pcmCruise)
+    # On some newer model years, the CANCEL button acts as a pause/resume button based on the PCM state
+    # To avoid re-engaging when openpilot cancels, check user engagement intention via buttons
+    # Main button also can trigger an engagement on these cars
+    allow_enable = any(btn in ENABLE_BUTTONS for btn in self.CS.cruise_buttons) or any(self.CS.main_buttons)
+    events = self.create_common_events(ret, pcm_enable=self.CS.CP.pcmCruise, allow_enable=allow_enable)
 
     if self.CS.brake_error:
       events.add(EventName.brakeUnavailable)
 
     if self.CS.CP.openpilotLongitudinalControl:
       buttonEvents = []
 
-      if self.CS.cruise_buttons != self.CS.prev_cruise_buttons:
+      if self.CS.cruise_buttons[-1] != self.CS.prev_cruise_buttons:
         be = car.CarState.ButtonEvent.new_message()
         be.type = ButtonType.unknown
-        if self.CS.cruise_buttons != 0:
+        if self.CS.cruise_buttons[-1] != 0:
           be.pressed = True
-          but = self.CS.cruise_buttons
+          but = self.CS.cruise_buttons[-1]
         else:
           be.pressed = False
           but = self.CS.prev_cruise_buttons

selfdrive/car/interfaces.py

@@ -131,7 +131,7 @@ def update(self, c: car.CarControl, can_strings: List[bytes]) -> car.CarState:
   def apply(self, c: car.CarControl) -> Tuple[car.CarControl.Actuators, List[bytes]]:
     pass
 
-  def create_common_events(self, cs_out, extra_gears=None, pcm_enable=True):
+  def create_common_events(self, cs_out, extra_gears=None, pcm_enable=True, allow_enable=True):
     events = Events()
 
     if cs_out.doorOpen:
@@ -175,8 +175,9 @@ def create_common_events(self, cs_out, extra_gears=None, pcm_enable=True):
       events.add(EventName.steerUnavailable)
 
     # we engage when pcm is active (rising edge)
+    # enabling can optionally be blocked by the car interface
     if pcm_enable:
-      if cs_out.cruiseState.enabled and not self.CS.out.cruiseState.enabled:
+      if cs_out.cruiseState.enabled and not self.CS.out.cruiseState.enabled and allow_enable:
         events.add(EventName.pcmEnable)
       elif not cs_out.cruiseState.enabled:
         events.add(EventName.pcmDisable)

selfdrive/test/process_replay/ref_commit

@@ -1 +1 @@
-fe7b24c1914046a3a7be19b6e812984e7e14301d
+17141a8240bd8ec5defa1de2da937544c52ef0fe