diff --git a/board/safety/safety_hyundai.h b/board/safety/safety_hyundai.h index 8cf8f669..d1a9e689 100644 --- a/board/safety/safety_hyundai.h +++ b/board/safety/safety_hyundai.h @@ -69,6 +69,11 @@ enum { HYUNDAI_BTN_CANCEL = 4, }; +// some newer HKG models can re-enable after spamming cancel button, +// so keep track of user button presses to deny engagement if no interaction +const uint8_t HYUNDAI_PREV_BUTTON_SAMPLES = 4; // roughly 80 ms +uint8_t hyundai_last_button_interaction; // button messages since the user pressed an enable button + bool hyundai_legacy = false; bool hyundai_ev_gas_signal = false; bool hyundai_hybrid_gas_signal = false; @@ -165,38 +170,46 @@ static int hyundai_rx_hook(CANPacket_t *to_push) { update_sample(&torque_driver, torque_driver_new); } - if (hyundai_longitudinal) { - // ACC steering wheel buttons - if (addr == 1265) { - int button = GET_BYTE(to_push, 0) & 0x7U; + // ACC steering wheel buttons + if (addr == 1265) { + int cruise_button = GET_BYTE(to_push, 0) & 0x7U; + int main_button = GET_BIT(to_push, 3U); + if ((cruise_button == HYUNDAI_BTN_RESUME) || (cruise_button == HYUNDAI_BTN_SET) || (cruise_button == HYUNDAI_BTN_CANCEL) || (main_button != 0)) { + hyundai_last_button_interaction = 0U; + } else { + hyundai_last_button_interaction = MIN(hyundai_last_button_interaction + 1U, HYUNDAI_PREV_BUTTON_SAMPLES); + } + + if (hyundai_longitudinal) { // exit controls on cancel press - if (button == HYUNDAI_BTN_CANCEL) { + if (cruise_button == HYUNDAI_BTN_CANCEL) { controls_allowed = 0; } // enter controls on falling edge of resume or set - bool set = (button == HYUNDAI_BTN_NONE) && (cruise_button_prev == HYUNDAI_BTN_SET); - bool res = (button == HYUNDAI_BTN_NONE) && (cruise_button_prev == HYUNDAI_BTN_RESUME); + bool set = (cruise_button == HYUNDAI_BTN_NONE) && (cruise_button_prev == HYUNDAI_BTN_SET); + bool res = (cruise_button == HYUNDAI_BTN_NONE) && (cruise_button_prev == HYUNDAI_BTN_RESUME); if (set || res) { controls_allowed = 1; } - cruise_button_prev = button; + cruise_button_prev = cruise_button; } - } else { - // enter controls on rising edge of ACC, exit controls on ACC off - if (addr == 1057) { - // 2 bits: 13-14 - int cruise_engaged = (GET_BYTES_04(to_push) >> 13) & 0x3U; - if (cruise_engaged && !cruise_engaged_prev) { - controls_allowed = 1; - } - if (!cruise_engaged) { - controls_allowed = 0; - } - cruise_engaged_prev = cruise_engaged; + } + + // enter controls on rising edge of ACC and user button press, exit controls when ACC off + if (!hyundai_longitudinal && (addr == 1057)) { + // 2 bits: 13-14 + int cruise_engaged = (GET_BYTES_04(to_push) >> 13) & 0x3U; + if (cruise_engaged && !cruise_engaged_prev && (hyundai_last_button_interaction < HYUNDAI_PREV_BUTTON_SAMPLES)) { + controls_allowed = 1; } + + if (!cruise_engaged) { + controls_allowed = 0; + } + cruise_engaged_prev = cruise_engaged; } // read gas pressed signal @@ -371,6 +384,7 @@ static const addr_checks* hyundai_init(uint32_t param) { hyundai_legacy = false; hyundai_ev_gas_signal = GET_FLAG(param, HYUNDAI_PARAM_EV_GAS); hyundai_hybrid_gas_signal = !hyundai_ev_gas_signal && GET_FLAG(param, HYUNDAI_PARAM_HYBRID_GAS); + hyundai_last_button_interaction = HYUNDAI_PREV_BUTTON_SAMPLES; #ifdef ALLOW_DEBUG hyundai_longitudinal = GET_FLAG(param, HYUNDAI_PARAM_LONGITUDINAL); @@ -392,6 +406,7 @@ static const addr_checks* hyundai_legacy_init(uint32_t param) { hyundai_longitudinal = false; hyundai_ev_gas_signal = GET_FLAG(param, HYUNDAI_PARAM_EV_GAS); hyundai_hybrid_gas_signal = !hyundai_ev_gas_signal && GET_FLAG(param, HYUNDAI_PARAM_HYBRID_GAS); + hyundai_last_button_interaction = HYUNDAI_PREV_BUTTON_SAMPLES; hyundai_rx_checks = (addr_checks){hyundai_legacy_addr_checks, HYUNDAI_LEGACY_ADDR_CHECK_LEN}; return &hyundai_rx_checks; } diff --git a/tests/safety/test_hyundai.py b/tests/safety/test_hyundai.py index 8996ccc3..270ea8ac 100755 --- a/tests/safety/test_hyundai.py +++ b/tests/safety/test_hyundai.py @@ -25,6 +25,9 @@ class Buttons: SET = 2 CANCEL = 4 +PREV_BUTTON_SAMPLES = 4 +ENABLE_BUTTONS = (Buttons.RESUME, Buttons.SET, Buttons.CANCEL) + # 4 bit checkusm used in some hyundai messages # lives outside the can packer because we never send this msg def checksum(msg): @@ -71,6 +74,7 @@ class TestHyundaiSafety(common.PandaSafetyTest): cnt_speed = 0 cnt_brake = 0 cnt_cruise = 0 + cnt_button = 0 def setUp(self): self.packer = CANPackerPanda("hyundai_kia_generic") @@ -78,8 +82,9 @@ class TestHyundaiSafety(common.PandaSafetyTest): self.safety.set_safety_hooks(Panda.SAFETY_HYUNDAI, 0) self.safety.init_tests() - def _button_msg(self, buttons): - values = {"CF_Clu_CruiseSwState": buttons} + def _button_msg(self, buttons, main_button=0): + values = {"CF_Clu_CruiseSwState": buttons, "CF_Clu_CruiseSwMain": main_button, "CF_Clu_AliveCnt1": self.cnt_button} + self.__class__.cnt_button += 1 return self.packer.make_can_msg_panda("CLU11", 0, values) def _user_gas_msg(self, gas): @@ -222,6 +227,39 @@ class TestHyundaiSafety(common.PandaSafetyTest): self._rx(self._pcm_status_msg(enabled)) self.assertEqual(enabled, self._tx(self._button_msg(Buttons.CANCEL))) + def test_enable_control_allowed_from_cruise(self): + """ + Hyundai non-longitudinal only enables on PCM rising edge and recent button press. Tests PCM enabling with: + - disallowed: No buttons + - disallowed: Buttons that don't enable cruise + - allowed: Buttons that do enable cruise + - allowed: Main button with all above combinations + """ + for main_button in [0, 1]: + for btn in range(8): + for _ in range(PREV_BUTTON_SAMPLES): # reset + self._rx(self._button_msg(Buttons.NONE)) + + self._rx(self._pcm_status_msg(False)) + self.assertFalse(self.safety.get_controls_allowed()) + self._rx(self._button_msg(btn, main_button=main_button)) + self._rx(self._pcm_status_msg(True)) + controls_allowed = btn in ENABLE_BUTTONS or main_button + self.assertEqual(controls_allowed, self.safety.get_controls_allowed()) + + def test_sampling_cruise_buttons(self): + """ + Test that we allow controls on recent button press, but not as button leaves sliding window + """ + self._rx(self._button_msg(Buttons.SET)) + for i in range(2 * PREV_BUTTON_SAMPLES): + self._rx(self._pcm_status_msg(False)) + self.assertFalse(self.safety.get_controls_allowed()) + self._rx(self._pcm_status_msg(True)) + controls_allowed = i < PREV_BUTTON_SAMPLES + self.assertEqual(controls_allowed, self.safety.get_controls_allowed()) + self._rx(self._button_msg(Buttons.NONE)) + class TestHyundaiLegacySafety(TestHyundaiSafety): def setUp(self): @@ -256,7 +294,6 @@ class TestHyundaiLegacySafetyHEV(TestHyundaiSafety): class TestHyundaiLongitudinalSafety(TestHyundaiSafety): TX_MSGS = [[832, 0], [1265, 0], [1157, 0], [1056, 0], [1057, 0], [1290, 0], [905, 0], [1186, 0], [909, 0], [1155, 0], [2000, 0]] - cnt_button = 0 def setUp(self): self.packer = CANPackerPanda("hyundai_kia_generic") @@ -271,6 +308,9 @@ class TestHyundaiLongitudinalSafety(TestHyundaiSafety): def test_enable_control_allowed_from_cruise(self): pass + def test_sampling_cruise_buttons(self): + pass + def test_cruise_engaged_prev(self): pass @@ -280,11 +320,6 @@ class TestHyundaiLongitudinalSafety(TestHyundaiSafety): def _pcm_status_msg(self, enable): raise NotImplementedError - def _button_msg(self, buttons): - values = {"CF_Clu_CruiseSwState": buttons, "CF_Clu_AliveCnt1": self.cnt_button} - self.__class__.cnt_button += 1 - return self.packer.make_can_msg_panda("CLU11", 0, values) - def _send_accel_msg(self, accel, aeb_req=False, aeb_decel=0): values = { "aReqRaw": accel, @@ -357,6 +392,5 @@ class TestHyundaiLongitudinalSafety(TestHyundaiSafety): self.assertTrue(self.safety.get_relay_malfunction()) - if __name__ == "__main__": unittest.main()