From 7caba241d05306fcc48016ae4a0a423899f91470 Mon Sep 17 00:00:00 2001 From: Ted Slesinski Date: Wed, 6 Jun 2018 17:20:07 -0400 Subject: [PATCH] Addition to Bosch safety to support Hatchback (#111) * Addition to Bosch safety to support Hatchback The computer brake is shown on 0x17c sensor on Accord and CR-V. We assumed all Bosch Hondas had the new 0x1be message which reports manual brake, but Civic Hatchback is not like this- It doesn't have this message and 0x17c works like the other Hondas so we are are passing a parameter from Openpilot for this. * Renamed variable * Make comment more descriptive * Added safety check for cancel-only spamming * Add regression test for brake on accord and crv Init with bosch safety variables Some more testing changes (still broken) Make second test work * Adds one more ttest * Cannot implicitly convert type 'int' to 'bool ' * ok to spam resume if controls_allowed==true * need to use current_controls_allowed. Still need to fix the message blocking * checking for bus 0 on button spam * better to use the car name in front of global vars * even better name and fixed safety tests --- board/safety/safety_honda.h | 25 ++++++++++++++---- tests/safety/libpandasafety_py.py | 2 ++ tests/safety/test.c | 8 ++++++ tests/safety/test_honda.py | 42 +++++++++++++++++++++++++++---- 4 files changed, 67 insertions(+), 10 deletions(-) diff --git a/board/safety/safety_honda.h b/board/safety/safety_honda.h index b9264a50af1d62..b64e45ce6d9653 100644 --- a/board/safety/safety_honda.h +++ b/board/safety/safety_honda.h @@ -16,6 +16,7 @@ int gas_interceptor_prev = 0; int ego_speed = 0; // TODO: auto-detect bosch hardware based on CAN messages? bool bosch_hardware = false; +bool honda_alt_brake_msg = false; static void honda_rx_hook(CAN_FIFOMailBox_TypeDef *to_push) { @@ -36,11 +37,14 @@ static void honda_rx_hook(CAN_FIFOMailBox_TypeDef *to_push) { } } - // user brake signal is different for nidec vs bosch hardware - // nidec hardware: 0x17C bit 53 - // bosch hardware: 0x1BE bit 4 - #define IS_USER_BRAKE_MSG(to_push) (!bosch_hardware ? to_push->RIR>>21 == 0x17C : to_push->RIR>>21 == 0x1BE) - #define USER_BRAKE_VALUE(to_push) (!bosch_hardware ? to_push->RDHR & 0x200000 : to_push->RDLR & 0x10) + // user brake signal on 0x17C reports applied brake from computer brake on accord + // and crv, which prevents the usual brake safety from working correctly. these + // cars have a signal on 0x1BE which only detects user's brake being applied so + // in these cases, this is used instead. + // most hondas: 0x17C bit 53 + // accord, crv: 0x1BE bit 4 + #define IS_USER_BRAKE_MSG(to_push) (!honda_alt_brake_msg ? to_push->RIR>>21 == 0x17C : to_push->RIR>>21 == 0x1BE) + #define USER_BRAKE_VALUE(to_push) (!honda_alt_brake_msg ? to_push->RDHR & 0x200000 : to_push->RDLR & 0x10) // exit controls on rising edge of brake press or on brake press when // speed > 0 if (IS_USER_BRAKE_MSG(to_push)) { @@ -115,6 +119,14 @@ static int honda_tx_hook(CAN_FIFOMailBox_TypeDef *to_send) { if ((to_send->RDLR & 0xFFFF0000) != to_send->RDLR) return 0; } } + + // FORCE CANCEL: safety check only relevant when spamming the cancel button in Bosch HW + // ensuring that only the cancel button press is sent (VAL 2) when controls are off. + // This avoids unintended engagements while still allowing resume spam + if (((to_send->RIR>>21) == 0x296) && bosch_hardware && + !current_controls_allowed && ((to_send->RDTR >> 4) & 0xFF) == 0) { + if (((to_send->RDLR >> 5) & 0x7) != 2) return 0; + } // 1 allows the message through return true; @@ -128,6 +140,7 @@ static int honda_tx_lin_hook(int lin_num, uint8_t *data, int len) { static void honda_init(int16_t param) { controls_allowed = 0; bosch_hardware = false; + honda_alt_brake_msg = false; } static int honda_fwd_hook(int bus_num, CAN_FIFOMailBox_TypeDef *to_fwd) { @@ -146,6 +159,8 @@ const safety_hooks honda_hooks = { static void honda_bosch_init(int16_t param) { controls_allowed = 0; bosch_hardware = true; + // Checking for alternate brake override from safety parameter + honda_alt_brake_msg = param == 1 ? true : false; } static int honda_bosch_fwd_hook(int bus_num, CAN_FIFOMailBox_TypeDef *to_fwd) { diff --git a/tests/safety/libpandasafety_py.py b/tests/safety/libpandasafety_py.py index bfb1f890751686..a8065255ed8ed7 100644 --- a/tests/safety/libpandasafety_py.py +++ b/tests/safety/libpandasafety_py.py @@ -52,6 +52,8 @@ int honda_tx_hook(CAN_FIFOMailBox_TypeDef *to_send); int get_brake_prev(void); int get_gas_prev(void); +void set_honda_alt_brake_msg(bool); +void set_bosch_hardware(bool); void init_tests_cadillac(void); void cadillac_init(int16_t param); diff --git a/tests/safety/test.c b/tests/safety/test.c index 6d387de14d7cad..66fc8ba8a9f2f7 100644 --- a/tests/safety/test.c +++ b/tests/safety/test.c @@ -105,6 +105,14 @@ int get_gas_prev(void){ return gas_prev; } +void set_honda_alt_brake_msg(bool c){ + honda_alt_brake_msg = c; +} + +void set_bosch_hardware(bool c){ + bosch_hardware = c; +} + void init_tests_toyota(void){ torque_meas.min = 0; torque_meas.max = 0; diff --git a/tests/safety/test_honda.py b/tests/safety/test_honda.py index fb169af7e3bf8c..b7e0612a799119 100755 --- a/tests/safety/test_honda.py +++ b/tests/safety/test_honda.py @@ -18,9 +18,9 @@ def _speed_msg(self, speed): return to_send - def _button_msg(self, buttons): + def _button_msg(self, buttons, msg): to_send = libpandasafety_py.ffi.new('CAN_FIFOMailBox_TypeDef *') - to_send[0].RIR = 0x1A6 << 21 + to_send[0].RIR = msg << 21 to_send[0].RDLR = buttons << 5 return to_send @@ -32,6 +32,13 @@ def _brake_msg(self, brake): return to_send + def _alt_brake_msg(self, brake): + to_send = libpandasafety_py.ffi.new('CAN_FIFOMailBox_TypeDef *') + to_send[0].RIR = 0x1BE << 21 + to_send[0].RDLR = 0x10 if brake else 0 + + return to_send + def _gas_msg(self, gas): to_send = libpandasafety_py.ffi.new('CAN_FIFOMailBox_TypeDef *') to_send[0].RIR = 0x17C << 21 @@ -65,18 +72,18 @@ def test_default_controls_not_allowed(self): def test_resume_button(self): RESUME_BTN = 4 - self.safety.honda_rx_hook(self._button_msg(RESUME_BTN)) + self.safety.honda_rx_hook(self._button_msg(RESUME_BTN, 0x1A6)) self.assertTrue(self.safety.get_controls_allowed()) def test_set_button(self): SET_BTN = 3 - self.safety.honda_rx_hook(self._button_msg(SET_BTN)) + self.safety.honda_rx_hook(self._button_msg(SET_BTN, 0x1A6)) self.assertTrue(self.safety.get_controls_allowed()) def test_cancel_button(self): CANCEL_BTN = 2 self.safety.set_controls_allowed(1) - self.safety.honda_rx_hook(self._button_msg(CANCEL_BTN)) + self.safety.honda_rx_hook(self._button_msg(CANCEL_BTN, 0x1A6)) self.assertFalse(self.safety.get_controls_allowed()) def test_sample_speed(self): @@ -94,6 +101,17 @@ def test_disengage_on_brake(self): self.safety.honda_rx_hook(self._brake_msg(1)) self.assertFalse(self.safety.get_controls_allowed()) + def test_alt_disengage_on_brake(self): + self.safety.set_honda_alt_brake_msg(1) + self.safety.set_controls_allowed(1) + self.safety.honda_rx_hook(self._alt_brake_msg(1)) + self.assertFalse(self.safety.get_controls_allowed()) + + self.safety.set_honda_alt_brake_msg(0) + self.safety.set_controls_allowed(1) + self.safety.honda_rx_hook(self._alt_brake_msg(1)) + self.assertTrue(self.safety.get_controls_allowed()) + def test_allow_brake_at_zero_speed(self): # Brake was already pressed self.safety.honda_rx_hook(self._brake_msg(True)) @@ -143,6 +161,20 @@ def test_steer_safety_check(self): self.assertTrue(self.safety.honda_tx_hook(self._send_steer_msg(0x0000))) self.assertFalse(self.safety.honda_tx_hook(self._send_steer_msg(0x1000))) + def test_spam_cancel_safety_check(self): + RESUME_BTN = 4 + SET_BTN = 3 + CANCEL_BTN = 2 + BUTTON_MSG = 0x296 + self.safety.set_bosch_hardware(1) + self.safety.set_controls_allowed(0) + self.assertTrue(self.safety.honda_tx_hook(self._button_msg(CANCEL_BTN, BUTTON_MSG))) + self.assertFalse(self.safety.honda_tx_hook(self._button_msg(RESUME_BTN, BUTTON_MSG))) + self.assertFalse(self.safety.honda_tx_hook(self._button_msg(SET_BTN, BUTTON_MSG))) + # do not block resume if we are engaged already + self.safety.set_controls_allowed(1) + self.assertTrue(self.safety.honda_tx_hook(self._button_msg(RESUME_BTN, BUTTON_MSG))) + if __name__ == "__main__": unittest.main()