Skip to content

Commit

Permalink
safety: remove pre-enable state (commaai#1121)
Browse files Browse the repository at this point in the history
* current_controls_allowed is now unused

* reduce diff

* remove from honda

* these test nothing

* fix
  • Loading branch information
sshane authored Oct 28, 2022
1 parent 609a8e0 commit 187fdee
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 131 deletions.
19 changes: 5 additions & 14 deletions board/safety/safety_gm.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,20 +144,11 @@ static int gm_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
tx = msg_allowed(to_send, GM_ASCM_TX_MSGS, sizeof(GM_ASCM_TX_MSGS)/sizeof(GM_ASCM_TX_MSGS[0]));
}

// disallow actuator commands if gas or brake (with vehicle moving) are pressed
// and the the latching controls_allowed flag is True
int pedal_pressed = brake_pressed_prev && vehicle_moving;
bool alt_exp_allow_gas = alternative_experience & ALT_EXP_DISABLE_DISENGAGE_ON_GAS;
if (!alt_exp_allow_gas) {
pedal_pressed = pedal_pressed || gas_pressed_prev;
}
bool current_controls_allowed = controls_allowed && !pedal_pressed;

// BRAKE: safety check
if (addr == 789) {
int brake = ((GET_BYTE(to_send, 0) & 0xFU) << 8) + GET_BYTE(to_send, 1);
brake = (0x1000 - brake) & 0xFFF;
if (!current_controls_allowed || !longitudinal_allowed) {
if (!longitudinal_allowed) {
if (brake != 0) {
tx = 0;
}
Expand All @@ -174,7 +165,7 @@ static int gm_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
bool violation = 0;
desired_torque = to_signed(desired_torque, 11);

if (current_controls_allowed) {
if (controls_allowed) {

// *** global torque limit check ***
violation |= max_limit_check(desired_torque, GM_MAX_STEER, -GM_MAX_STEER);
Expand All @@ -199,12 +190,12 @@ static int gm_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
}

// no torque if controls is not allowed
if (!current_controls_allowed && (desired_torque != 0)) {
if (!controls_allowed && (desired_torque != 0)) {
violation = 1;
}

// reset to 0 if either controls is not allowed or there's a violation
if (violation || !current_controls_allowed) {
if (violation || !controls_allowed) {
desired_torque_last = 0;
rt_torque_last = 0;
ts_torque_check_last = ts;
Expand All @@ -220,7 +211,7 @@ static int gm_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
int gas_regen = ((GET_BYTE(to_send, 2) & 0x7FU) << 5) + ((GET_BYTE(to_send, 3) & 0xF8U) >> 3);
// Disabled message is !engaged with gas
// value that corresponds to inactive regen.
if (!current_controls_allowed || !longitudinal_allowed) {
if (!longitudinal_allowed) {
if (gas_regen != GM_INACTIVE_REGEN) {
tx = 0;
}
Expand Down
22 changes: 7 additions & 15 deletions board/safety/safety_honda.h
Original file line number Diff line number Diff line change
Expand Up @@ -263,22 +263,14 @@ static int honda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
tx = msg_allowed(to_send, HONDA_N_TX_MSGS, sizeof(HONDA_N_TX_MSGS)/sizeof(HONDA_N_TX_MSGS[0]));
}

// disallow actuator commands if gas or brake (with vehicle moving) are pressed
// and the latching controls_allowed flag is True
int pedal_pressed = brake_pressed_prev && vehicle_moving;
bool alt_exp_allow_gas = alternative_experience & ALT_EXP_DISABLE_DISENGAGE_ON_GAS;
if (!alt_exp_allow_gas) {
pedal_pressed = pedal_pressed || gas_pressed_prev;
}
bool current_controls_allowed = controls_allowed && !(pedal_pressed);
int bus_pt = honda_get_pt_bus();
int bus_buttons = (honda_bosch_radarless) ? 2 : bus_pt; // the camera controls ACC on radarless Bosch cars

// ACC_HUD: safety check (nidec w/o pedal)
if ((addr == 0x30C) && (bus == bus_pt)) {
int pcm_speed = (GET_BYTE(to_send, 0) << 8) | GET_BYTE(to_send, 1);
int pcm_gas = GET_BYTE(to_send, 2);
if (!current_controls_allowed || !longitudinal_allowed) {
if (!longitudinal_allowed) {
if ((pcm_speed != 0) || (pcm_gas != 0)) {
tx = 0;
}
Expand All @@ -288,7 +280,7 @@ static int honda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
// BRAKE: safety check (nidec)
if ((addr == 0x1FA) && (bus == bus_pt)) {
honda_brake = (GET_BYTE(to_send, 0) << 2) + ((GET_BYTE(to_send, 1) >> 6) & 0x3U);
if (!current_controls_allowed || !longitudinal_allowed) {
if (!longitudinal_allowed) {
if (honda_brake != 0) {
tx = 0;
}
Expand All @@ -305,7 +297,7 @@ static int honda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
if ((addr == 0x1DF) && (bus == bus_pt)) {
int accel = (GET_BYTE(to_send, 3) << 3) | ((GET_BYTE(to_send, 4) >> 5) & 0x7U);
accel = to_signed(accel, 11);
if (!current_controls_allowed || !longitudinal_allowed) {
if (!longitudinal_allowed) {
if (accel != 0) {
tx = 0;
}
Expand All @@ -316,7 +308,7 @@ static int honda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {

int gas = (GET_BYTE(to_send, 0) << 8) | GET_BYTE(to_send, 1);
gas = to_signed(gas, 16);
if (!current_controls_allowed || !longitudinal_allowed) {
if (!longitudinal_allowed) {
if (gas != HONDA_BOSCH_NO_GAS_VALUE) {
tx = 0;
}
Expand All @@ -328,7 +320,7 @@ static int honda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {

// STEER: safety check
if ((addr == 0xE4) || (addr == 0x194)) {
if (!current_controls_allowed) {
if (!controls_allowed) {
bool steer_applied = GET_BYTE(to_send, 0) | GET_BYTE(to_send, 1);
if (steer_applied) {
tx = 0;
Expand All @@ -345,7 +337,7 @@ static int honda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {

// GAS: safety check (interceptor)
if (addr == 0x200) {
if (!current_controls_allowed || !longitudinal_allowed) {
if (!longitudinal_allowed) {
if (GET_BYTE(to_send, 0) || GET_BYTE(to_send, 1)) {
tx = 0;
}
Expand All @@ -355,7 +347,7 @@ static int honda_tx_hook(CANPacket_t *to_send, bool longitudinal_allowed) {
// 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 ((addr == 0x296) && !current_controls_allowed && (bus == bus_buttons)) {
if ((addr == 0x296) && !controls_allowed && (bus == bus_buttons)) {
if (((GET_BYTE(to_send, 0) >> 5) & 0x7U) != 2U) {
tx = 0;
}
Expand Down
53 changes: 1 addition & 52 deletions tests/safety/test_gm.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from panda import Panda
from panda.tests.safety import libpandasafety_py
import panda.tests.safety.common as common
from panda.tests.safety.common import CANPackerPanda, ALTERNATIVE_EXPERIENCE
from panda.tests.safety.common import CANPackerPanda

MAX_BRAKE = 400
MAX_GAS = 3072
Expand Down Expand Up @@ -105,57 +105,6 @@ def test_gas_safety_check(self, stock_longitudinal=False):
(not enabled and gas_regen == INACTIVE_REGEN)) and not stock_longitudinal)
self.assertEqual(should_tx, self._tx(self._send_gas_msg(gas_regen)), (enabled, gas_regen))

def test_tx_hook_on_pedal_pressed(self):
for pedal in ['brake', 'gas']:
if pedal == 'brake':
# brake_pressed_prev and vehicle_moving
self._rx(self._speed_msg(100))
self._rx(self._user_brake_msg(1))
elif pedal == 'gas':
# gas_pressed_prev
self._rx(self._user_gas_msg(MAX_GAS))

self.safety.set_controls_allowed(1)
self.assertFalse(self._tx(self._send_brake_msg(MAX_BRAKE)))
self.assertFalse(self._tx(self._torque_cmd_msg(self.MAX_RATE_UP)))
self.assertFalse(self._tx(self._send_gas_msg(MAX_GAS)))

# reset status
self.safety.set_controls_allowed(0)
self._tx(self._send_brake_msg(0))
self._tx(self._torque_cmd_msg(0))
if pedal == 'brake':
self._rx(self._speed_msg(0))
self._rx(self._user_brake_msg(0))
elif pedal == 'gas':
self._rx(self._user_gas_msg(0))

def test_tx_hook_on_pedal_pressed_on_alternative_gas_experience(self):
for pedal in ['brake', 'gas']:
self.safety.set_alternative_experience(ALTERNATIVE_EXPERIENCE.DISABLE_DISENGAGE_ON_GAS)
if pedal == 'brake':
# brake_pressed_prev and vehicle_moving
self._rx(self._speed_msg(100))
self._rx(self._user_brake_msg(1))
allow_ctrl = False
elif pedal == 'gas':
# gas_pressed_prev
self._rx(self._user_gas_msg(MAX_GAS))
allow_ctrl = True

# Test we allow lateral on gas press, but never longitudinal
self.safety.set_controls_allowed(1)
self.assertEqual(allow_ctrl, self._tx(self._torque_cmd_msg(self.MAX_RATE_UP)))
self.assertFalse(self._tx(self._send_brake_msg(MAX_BRAKE)))
self.assertFalse(self._tx(self._send_gas_msg(MAX_GAS)))

# reset status
if pedal == 'brake':
self._rx(self._speed_msg(0))
self._rx(self._user_brake_msg(0))
elif pedal == 'gas':
self._rx(self._user_gas_msg(0))


class TestGmAscmSafety(TestGmSafetyBase):
TX_MSGS = [[384, 0], [1033, 0], [1034, 0], [715, 0], [880, 0], # pt bus
Expand Down
51 changes: 1 addition & 50 deletions tests/safety/test_honda.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from panda import Panda
from panda.tests.safety import libpandasafety_py
import panda.tests.safety.common as common
from panda.tests.safety.common import CANPackerPanda, make_msg, MAX_WRONG_COUNTERS, ALTERNATIVE_EXPERIENCE
from panda.tests.safety.common import CANPackerPanda, make_msg, MAX_WRONG_COUNTERS

class Btn:
NONE = 0
Expand Down Expand Up @@ -138,39 +138,6 @@ def test_rx_hook(self):
self._rx(self._button_msg(Btn.SET, main_on=True))
self.assertTrue(self.safety.get_controls_allowed())

def test_tx_hook_on_pedal_pressed(self):
for mode in [ALTERNATIVE_EXPERIENCE.DEFAULT, ALTERNATIVE_EXPERIENCE.DISABLE_DISENGAGE_ON_GAS]:
for pedal in ['brake', 'gas']:
self.safety.set_alternative_experience(mode)
allow_ctrl = False
if pedal == 'brake':
# brake_pressed_prev and vehicle_moving
self._rx(self._speed_msg(100))
self._rx(self._user_brake_msg(1))
elif pedal == 'gas':
# gas_pressed_prev
self._rx(self._user_gas_msg(1))
allow_ctrl = mode == ALTERNATIVE_EXPERIENCE.DISABLE_DISENGAGE_ON_GAS

self.safety.set_controls_allowed(1)
hw = self.safety.get_honda_hw()
if hw == HONDA_NIDEC:
self.safety.set_honda_fwd_brake(False)
self.assertEqual(allow_ctrl, self._tx(self._send_brake_msg(self.MAX_BRAKE)))
self.assertEqual(allow_ctrl, self._tx(self._send_steer_msg(0x1000)))

# reset status
self.safety.set_controls_allowed(0)
self.safety.set_alternative_experience(ALTERNATIVE_EXPERIENCE.DEFAULT)
if hw == HONDA_NIDEC:
self._tx(self._send_brake_msg(0))
self._tx(self._send_steer_msg(0))
if pedal == 'brake':
self._rx(self._speed_msg(0))
self._rx(self._user_brake_msg(0))
elif pedal == 'gas':
self._rx(self._user_gas_msg(0))


class HondaPcmEnableBase(common.PandaSafetyTest):
# pylint: disable=no-member,abstract-method
Expand Down Expand Up @@ -362,22 +329,6 @@ def test_brake_safety_check(self):
self.assertEqual(send, self._tx(self._send_brake_msg(brake)))
self.safety.set_honda_fwd_brake(False)

def test_tx_hook_on_interceptor_pressed(self):
for mode in [ALTERNATIVE_EXPERIENCE.DEFAULT, ALTERNATIVE_EXPERIENCE.DISABLE_DISENGAGE_ON_GAS]:
self.safety.set_alternative_experience(mode)
# gas_interceptor_prev > INTERCEPTOR_THRESHOLD
self._rx(self._interceptor_user_gas(self.INTERCEPTOR_THRESHOLD + 1))
self._rx(self._interceptor_user_gas(self.INTERCEPTOR_THRESHOLD + 1))
allow_ctrl = mode == ALTERNATIVE_EXPERIENCE.DISABLE_DISENGAGE_ON_GAS

self.safety.set_controls_allowed(1)
self.safety.set_honda_fwd_brake(False)

# Test we allow lateral, but never longitudinal
self.assertFalse(self._tx(self._interceptor_gas_cmd(self.INTERCEPTOR_THRESHOLD)))
self.assertFalse(self._tx(self._send_brake_msg(self.MAX_BRAKE)))
self.assertEqual(allow_ctrl, self._tx(self._send_steer_msg(0x1000)))


class TestHondaNidecSafety(HondaPcmEnableBase, TestHondaNidecSafetyBase):
"""
Expand Down

0 comments on commit 187fdee

Please sign in to comment.