From efa98d2a0694b9a33a2fda3bebfae50fb4ba33cc Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Tue, 26 Apr 2022 13:27:28 -0700 Subject: [PATCH] HKG: only allow engagement on user button press (#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 0f54c051bf5a22b1f584718ec46e78bc3a82a0dd. * consider pause/resume button * stash * Revert "stash" This reverts commit 551ca7be6c45729f0747abc81cd81894cd621c32. * 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 9730c3c14f942b82b6ed5ef2e81b8ae0126f3006. * 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 * 3% faster at 1000000 loops * update refs Co-authored-by: Adeeb Shihadeh --- panda | 2 +- selfdrive/car/hyundai/carstate.py | 17 ++++++++++++----- selfdrive/car/hyundai/interface.py | 14 ++++++++++---- selfdrive/car/interfaces.py | 5 +++-- selfdrive/test/process_replay/ref_commit | 2 +- 5 files changed, 27 insertions(+), 13 deletions(-) diff --git a/panda b/panda index 5eefc3ad62422d..bb2aea1ac6bd9a 160000 --- a/panda +++ b/panda @@ -1 +1 @@ -Subproject commit 5eefc3ad62422dea3fc356f82267af58f9b1518a +Subproject commit bb2aea1ac6bd9ab198ec5192268a6ea6eec6bbe5 diff --git a/selfdrive/car/hyundai/carstate.py b/selfdrive/car/hyundai/carstate.py index a244d9e8fb92d4..7752bf82cddafe 100644 --- a/selfdrive/car/hyundai/carstate.py +++ b/selfdrive/car/hyundai/carstate.py @@ -1,17 +1,24 @@ +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"]: @@ -19,7 +26,6 @@ def __init__(self, CP): 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 diff --git a/selfdrive/car/hyundai/interface.py b/selfdrive/car/hyundai/interface.py index 00cd4507f9ddfa..999910eff13cdd 100644 --- a/selfdrive/car/hyundai/interface.py +++ b/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,7 +317,11 @@ 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) @@ -323,12 +329,12 @@ def _update(self, c): 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 diff --git a/selfdrive/car/interfaces.py b/selfdrive/car/interfaces.py index 2d4b3416a4a84c..ac15b2e1f81d7c 100644 --- a/selfdrive/car/interfaces.py +++ b/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) diff --git a/selfdrive/test/process_replay/ref_commit b/selfdrive/test/process_replay/ref_commit index 51103797460a4d..47d1a862350d02 100644 --- a/selfdrive/test/process_replay/ref_commit +++ b/selfdrive/test/process_replay/ref_commit @@ -1 +1 @@ -fe7b24c1914046a3a7be19b6e812984e7e14301d \ No newline at end of file +17141a8240bd8ec5defa1de2da937544c52ef0fe \ No newline at end of file