diff --git a/board/safety.h b/board/safety.h index 087273698..9898fb1f7 100644 --- a/board/safety.h +++ b/board/safety.h @@ -81,14 +81,14 @@ int safety_fwd_hook(int bus_num, int addr) { return (relay_malfunction ? -1 : current_hooks->fwd(bus_num, addr)); } -bool get_longitudinal_allowed(void) { +bool get_decel_allowed(void) { // No longitudinal control when overriding with gas. Only brake is allowed when pre-enabling at a standstill return controls_allowed && !gas_pressed_prev; } bool get_accel_allowed(void) { // No positive acceleration/gas command while pre-enabled at a stop with brake - return get_longitudinal_allowed() && !brake_pressed_prev; + return get_decel_allowed() && !brake_pressed_prev && !regen_braking_prev; } // Given a CRC-8 poly, generate a static lookup table to use with a fast CRC-8 @@ -510,27 +510,15 @@ int ROUND(float val) { // Safety checks for longitudinal actuation bool longitudinal_accel_checks(int desired_accel, const LongitudinalLimits limits) { -// bool braking = desired_accel < 0; - bool accel_valid = get_accel_allowed() && !max_limit_check(desired_accel, limits.max_accel, 0); - bool decel_valid = get_longitudinal_allowed() && !max_limit_check(desired_accel, 0, limits.min_accel); - -// if (get_longitudinal_allowed()) { -// if (!get_accel_allowed()) -// } -// -// int max_accel = -// if (get_accel_allowed()) { -// bool accel_valid -// } - -// bool accel_valid = get_accel_allowed() && !max_limit_check(desired_accel, limits.max_accel, limits.min_accel); + bool decel_valid = get_decel_allowed() && !max_limit_check(desired_accel, 0, limits.min_accel); bool accel_inactive = desired_accel == limits.inactive_accel; + print("here: "); puth(accel_valid); print(" "); puth(accel_valid); print(" "); puth(decel_valid); print(" "); puth(accel_inactive); print("\n"); return !(accel_valid || decel_valid || accel_inactive); } bool longitudinal_speed_checks(int desired_speed, const LongitudinalLimits limits) { - return !get_longitudinal_allowed() && (desired_speed != limits.inactive_speed); + return !get_decel_allowed() && (desired_speed != limits.inactive_speed); } bool longitudinal_transmission_rpm_checks(int desired_transmission_rpm, const LongitudinalLimits limits) { @@ -547,7 +535,7 @@ bool longitudinal_gas_checks(int desired_gas, const LongitudinalLimits limits) { bool longitudinal_brake_checks(int desired_brake, const LongitudinalLimits limits) { bool violation = false; - violation |= !get_longitudinal_allowed() && (desired_brake != 0); + violation |= !get_decel_allowed() && (desired_brake != 0); violation |= desired_brake > limits.max_brake; return violation; } diff --git a/board/safety_declarations.h b/board/safety_declarations.h index 4eef6549d..93b13e9ea 100644 --- a/board/safety_declarations.h +++ b/board/safety_declarations.h @@ -147,7 +147,7 @@ bool dist_to_meas_check(int val, int val_last, struct sample_t *val_meas, bool driver_limit_check(int val, int val_last, struct sample_t *val_driver, const int MAX, const int MAX_RATE_UP, const int MAX_RATE_DOWN, const int MAX_ALLOWANCE, const int DRIVER_FACTOR); -bool get_longitudinal_allowed(void); +bool get_decel_allowed(void); bool get_accel_allowed(void); bool rt_rate_limit_check(int val, int val_last, const int MAX_RT_DELTA); float interpolate(struct lookup_t xy, float x); diff --git a/tests/libpanda/safety_helpers.py b/tests/libpanda/safety_helpers.py index d10a82b00..1a42e47cf 100644 --- a/tests/libpanda/safety_helpers.py +++ b/tests/libpanda/safety_helpers.py @@ -5,7 +5,8 @@ def setup_safety_helpers(ffi): ffi.cdef(""" void set_controls_allowed(bool c); bool get_controls_allowed(void); - bool get_longitudinal_allowed(void); + bool get_decel_allowed(void); + bool get_accel_allowed(void); void set_alternative_experience(int mode); int get_alternative_experience(void); void set_relay_malfunction(bool c); @@ -55,7 +56,8 @@ def setup_safety_helpers(ffi): class PandaSafety(Protocol): def set_controls_allowed(self, c: bool) -> None: ... def get_controls_allowed(self) -> bool: ... - def get_longitudinal_allowed(self) -> bool: ... + def get_decel_allowed(self) -> bool: ... + def get_accel_allowed(self) -> bool: ... def set_alternative_experience(self, mode: int) -> None: ... def get_alternative_experience(self) -> int: ... def set_relay_malfunction(self, c: bool) -> None: ... diff --git a/tests/safety/common.py b/tests/safety/common.py index f95487593..ecf902884 100644 --- a/tests/safety/common.py +++ b/tests/safety/common.py @@ -143,10 +143,12 @@ class InterceptorSafetyTest(PandaSafetyTestBase): self._rx(self._interceptor_user_gas(g)) # Test we allow lateral, but not longitudinal self.assertTrue(self.safety.get_controls_allowed()) - self.assertEqual(g <= self.INTERCEPTOR_THRESHOLD, self.safety.get_longitudinal_allowed()) + self.assertEqual(g <= self.INTERCEPTOR_THRESHOLD, self.safety.get_decel_allowed()) + self.assertEqual(g <= self.INTERCEPTOR_THRESHOLD, self.safety.get_accel_allowed()) # Make sure we can re-gain longitudinal actuation self._rx(self._interceptor_user_gas(0)) - self.assertTrue(self.safety.get_longitudinal_allowed()) + self.assertTrue(self.safety.get_decel_allowed()) + self.assertTrue(self.safety.get_accel_allowed()) def test_allow_engage_with_gas_interceptor_pressed(self): self._rx(self._interceptor_user_gas(0x1000)) @@ -201,9 +203,29 @@ class LongitudinalAccelSafetyTest(PandaSafetyTestBase, abc.ABC): should_tx = False else: should_tx = controls_allowed and min_accel <= accel <= max_accel + should_tx = should_tx or accel == self.INACTIVE_ACCEL self.assertEqual(should_tx, self._tx(self._accel_msg(accel))) + def test_brake_preenable(self): + """Asserts safety for enabling at a standstill with brake depressed""" + + # Brake was already pressed + self._rx(self._speed_msg(0)) + self._rx(self._user_brake_msg(1)) + self.safety.set_controls_allowed(1) + self._rx(self._user_brake_msg(1)) + + # Test we allow brake but not positive accel + for valid_accel in (0, self.INACTIVE_ACCEL, self.MIN_ACCEL): + self.assertTrue(self._tx(self._accel_msg(valid_accel))) + self.assertFalse(self._tx(self._accel_msg(self.MAX_ACCEL))) + + # Make sure we can re-gain longitudinal actuation + self._rx(self._user_brake_msg(0)) + for valid_accel in (0, self.INACTIVE_ACCEL, self.MIN_ACCEL, self.MAX_ACCEL): + self.assertTrue(self._tx(self._accel_msg(valid_accel))) + class LongitudinalGasBrakeSafetyTest(PandaSafetyTestBase, abc.ABC): @@ -237,6 +259,33 @@ class LongitudinalGasBrakeSafetyTest(PandaSafetyTestBase, abc.ABC): def test_gas_safety_check(self): self._generic_limit_safety_check(self._send_gas_msg, self.MIN_GAS, self.MAX_GAS, 0, self.MAX_POSSIBLE_GAS, 1, self.INACTIVE_GAS) + def test_brake_preenable(self): + """Asserts safety for enabling at a standstill with brake depressed""" + + # Brake was already pressed + self._rx(self._speed_msg(0)) + self._rx(self._user_brake_msg(1)) + self.safety.set_controls_allowed(1) + self._rx(self._user_brake_msg(1)) + + # Test we allow brake but not gas + self.assertFalse(self._tx(self._send_gas_msg(0))) + self.assertTrue(self._tx(self._send_gas_msg(self.INACTIVE_GAS))) + if self.INACTIVE_GAS != self.MIN_GAS: + self.assertFalse(self._tx(self._send_gas_msg(self.MIN_GAS))) + + self.assertTrue(self._tx(self._send_brake_msg(0))) + self.assertTrue(self._tx(self._send_brake_msg(self.MIN_BRAKE))) + self.assertTrue(self._tx(self._send_brake_msg(self.MAX_BRAKE))) + + # for valid_accel in (0, self.INACTIVE_ACCEL, self.MIN_ACCEL): + # self.assertTrue(self._tx(self._accel_msg(valid_accel))) + # self.assertFalse(self._tx(self._accel_msg(self.MAX_ACCEL))) + # + # # Make sure we can re-gain longitudinal actuation + # self._rx(self._user_brake_msg(0)) + # for valid_accel in (0, self.INACTIVE_ACCEL, self.MIN_ACCEL, self.MAX_ACCEL): + # self.assertTrue(self._tx(self._accel_msg(valid_accel))) class TorqueSteeringSafetyTestBase(PandaSafetyTestBase, abc.ABC): @@ -868,10 +917,12 @@ class PandaSafetyTest(PandaSafetyTestBase): self._rx(self._user_gas_msg(self.GAS_PRESSED_THRESHOLD + 1)) # Test we allow lateral, but not longitudinal self.assertTrue(self.safety.get_controls_allowed()) - self.assertFalse(self.safety.get_longitudinal_allowed()) + self.assertFalse(self.safety.get_decel_allowed()) + self.assertFalse(self.safety.get_accel_allowed()) # Make sure we can re-gain longitudinal actuation self._rx(self._user_gas_msg(0)) - self.assertTrue(self.safety.get_longitudinal_allowed()) + self.assertTrue(self.safety.get_decel_allowed()) + self.assertTrue(self.safety.get_accel_allowed()) def test_prev_user_brake(self, _user_brake_msg=None, get_brake_pressed_prev=None): if _user_brake_msg is None: @@ -913,14 +964,17 @@ class PandaSafetyTest(PandaSafetyTestBase): self.safety.set_controls_allowed(1) self._rx(_user_brake_msg(1)) self.assertTrue(self.safety.get_controls_allowed()) - self.assertTrue(self.safety.get_longitudinal_allowed()) + self.assertTrue(self.safety.get_decel_allowed()) + self.assertFalse(self.safety.get_accel_allowed()) self._rx(_user_brake_msg(0)) self.assertTrue(self.safety.get_controls_allowed()) - self.assertTrue(self.safety.get_longitudinal_allowed()) + self.assertTrue(self.safety.get_decel_allowed()) + self.assertTrue(self.safety.get_accel_allowed()) # rising edge of brake should disengage self._rx(_user_brake_msg(1)) self.assertFalse(self.safety.get_controls_allowed()) - self.assertFalse(self.safety.get_longitudinal_allowed()) + self.assertFalse(self.safety.get_decel_allowed()) + self.assertFalse(self.safety.get_accel_allowed()) self._rx(_user_brake_msg(0)) # reset no brakes def test_not_allow_user_brake_when_moving(self, _user_brake_msg=None, get_brake_pressed_prev=None): @@ -933,11 +987,13 @@ class PandaSafetyTest(PandaSafetyTestBase): self._rx(self._vehicle_moving_msg(self.STANDSTILL_THRESHOLD)) self._rx(_user_brake_msg(1)) self.assertTrue(self.safety.get_controls_allowed()) - self.assertTrue(self.safety.get_longitudinal_allowed()) + self.assertTrue(self.safety.get_decel_allowed()) + self.assertFalse(self.safety.get_accel_allowed()) self._rx(self._vehicle_moving_msg(self.STANDSTILL_THRESHOLD + 1)) self._rx(_user_brake_msg(1)) self.assertFalse(self.safety.get_controls_allowed()) - self.assertFalse(self.safety.get_longitudinal_allowed()) + self.assertFalse(self.safety.get_decel_allowed()) + self.assertFalse(self.safety.get_accel_allowed()) self._rx(self._vehicle_moving_msg(0)) def test_vehicle_moving(self): diff --git a/tests/safety/test_tesla.py b/tests/safety/test_tesla.py index 83e58352b..5551a837e 100755 --- a/tests/safety/test_tesla.py +++ b/tests/safety/test_tesla.py @@ -144,7 +144,7 @@ class TestTeslaLongitudinalSafety(TestTeslaSafety): send = (MIN_ACCEL <= min_accel <= MAX_ACCEL) and (MIN_ACCEL <= max_accel <= MAX_ACCEL) else: send = np.all(np.isclose([min_accel, max_accel], 0, atol=0.0001)) - self.assertEqual(send, self._tx(self._long_control_msg(10, acc_val=4, accel_limits=[min_accel, max_accel]))) + self.assertEqual(send, self._tx(self._long_control_msg(10, acc_val=4, accel_limits=[min_accel, max_accel])), (min_accel, max_accel)) class TestTeslaChassisLongitudinalSafety(TestTeslaLongitudinalSafety): diff --git a/tests/safety/test_toyota.py b/tests/safety/test_toyota.py index abe724e84..c0b8fce6c 100755 --- a/tests/safety/test_toyota.py +++ b/tests/safety/test_toyota.py @@ -186,6 +186,9 @@ class TestToyotaStockLongitudinalBase(TestToyotaSafetyBase): def test_accel_actuation_limits(self, stock_longitudinal=True): super().test_accel_actuation_limits(stock_longitudinal=stock_longitudinal) + def test_brake_preenable(self): + raise unittest.SkipTest + def test_acc_cancel(self): """ Regardless of controls allowed, never allow ACC_CONTROL if cancel bit isn't set