Hyundai: enforce recent button press to allow controls with PCM (#917)

* add safety for hyundai stock long button-only enable

* add full logic, use 4 frames of button samples

* comment above and in openpilot is enough explanation

* make misra happy

* add tests

* add comments and better variable names

* only check button values on rising edge of PCM ACC enable

* common function

* Revert "common function"

This reverts commit 81e642ca98.

* simplify: keep track of last interaction instead

* revert changes to sample_t

* make unsigned

* clip to max and use uint8

* start at 4

* consistent

* apply suggestions

* Update board/safety/safety_hyundai.h

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

* unify function

Co-authored-by: Adeeb Shihadeh <adeebshihadeh@gmail.com>
This commit is contained in:
Shane Smiskol 2022-04-25 18:57:02 -07:00 committed by GitHub
parent 5eefc3ad62
commit 9a91323c04
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 78 additions and 29 deletions

View File

@ -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;
}

View File

@ -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()