diff --git a/board/safety/safety_subaru.h b/board/safety/safety_subaru.h index 1b917bcab2..aa74a8f691 100644 --- a/board/safety/safety_subaru.h +++ b/board/safety/safety_subaru.h @@ -38,12 +38,6 @@ const LongitudinalLimits SUBARU_LONG_LIMITS = { #define MSG_SUBARU_ES_LKAS_State 0x322 #define MSG_SUBARU_ES_Infotainment 0x323 -#define MSG_SUBARU_ES_UDS_Request 0x787 - -#define MSG_SUBARU_ES_HighBeamAssist 0x121 -#define MSG_SUBARU_ES_STATIC_1 0x22a -#define MSG_SUBARU_ES_STATIC_2 0x325 - #define SUBARU_MAIN_BUS 0 #define SUBARU_ALT_BUS 1 #define SUBARU_CAM_BUS 2 @@ -56,15 +50,8 @@ const LongitudinalLimits SUBARU_LONG_LIMITS = { {MSG_SUBARU_ES_Infotainment, SUBARU_MAIN_BUS, 8}, \ #define SUBARU_COMMON_LONG_TX_MSGS(alt_bus) \ - {MSG_SUBARU_ES_Brake, alt_bus, 8}, \ - {MSG_SUBARU_ES_Status, alt_bus, 8}, \ - -#define SUBARU_ADDITIONAL_TX_MSGS(alt_bus) \ - {MSG_SUBARU_ES_UDS_Request, SUBARU_CAM_BUS, 8}, \ - {MSG_SUBARU_ES_HighBeamAssist, SUBARU_MAIN_BUS, 8}, \ - {MSG_SUBARU_ES_STATIC_1, SUBARU_MAIN_BUS, 8}, \ - {MSG_SUBARU_ES_STATIC_2, SUBARU_MAIN_BUS, 8}, \ - + {MSG_SUBARU_ES_Brake, SUBARU_MAIN_BUS, 8}, \ + {MSG_SUBARU_ES_Status, SUBARU_MAIN_BUS, 8}, \ #define SUBARU_COMMON_ADDR_CHECKS(alt_bus) \ {.msg = {{MSG_SUBARU_Throttle, SUBARU_MAIN_BUS, 8, .check_checksum = true, .max_counter = 15U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, \ @@ -89,13 +76,6 @@ const CanMsg SUBARU_GEN2_TX_MSGS[] = { }; #define SUBARU_GEN2_TX_MSGS_LEN (sizeof(SUBARU_GEN2_TX_MSGS) / sizeof(SUBARU_GEN2_TX_MSGS[0])) -const CanMsg SUBARU_GEN2_LONG_TX_MSGS[] = { - SUBARU_COMMON_TX_MSGS(SUBARU_ALT_BUS) - SUBARU_COMMON_LONG_TX_MSGS(SUBARU_ALT_BUS) - SUBARU_ADDITIONAL_TX_MSGS(SUBARU_ALT_BUS) -}; -#define SUBARU_GEN2_LONG_TX_MSGS_LEN (sizeof(SUBARU_GEN2_LONG_TX_MSGS) / sizeof(SUBARU_GEN2_LONG_TX_MSGS[0])) - AddrCheckStruct subaru_addr_checks[] = { SUBARU_COMMON_ADDR_CHECKS(SUBARU_MAIN_BUS) }; @@ -195,12 +175,7 @@ static int subaru_tx_hook(CANPacket_t *to_send) { bool violation = false; if (subaru_gen2) { - if(subaru_longitudinal){ - tx = msg_allowed(to_send, SUBARU_GEN2_LONG_TX_MSGS, SUBARU_GEN2_LONG_TX_MSGS_LEN); - } - else{ - tx = msg_allowed(to_send, SUBARU_GEN2_TX_MSGS, SUBARU_GEN2_TX_MSGS_LEN); - } + tx = msg_allowed(to_send, SUBARU_GEN2_TX_MSGS, SUBARU_GEN2_TX_MSGS_LEN); } else if (subaru_longitudinal) { tx = msg_allowed(to_send, SUBARU_LONG_TX_MSGS, SUBARU_LONG_TX_MSGS_LEN); } else { @@ -243,15 +218,6 @@ static int subaru_tx_hook(CANPacket_t *to_send) { violation |= longitudinal_transmission_rpm_checks(transmission_rpm, SUBARU_LONG_LIMITS); } - if (addr == MSG_SUBARU_ES_UDS_Request){ - int sid = GET_BYTES(to_send, 1, 1); - - // Allow "tester present", "read data by identifier", and "communication control" - bool allowed_sid = (sid == 0x3e) || (sid == 0x22) || (sid == 0x28); - - violation |= (!allowed_sid); - } - if (violation){ tx = 0; } @@ -289,7 +255,7 @@ static const addr_checks* subaru_init(uint16_t param) { subaru_gen2 = GET_FLAG(param, SUBARU_PARAM_GEN2); #ifdef ALLOW_DEBUG - subaru_longitudinal = GET_FLAG(param, SUBARU_PARAM_LONGITUDINAL); + subaru_longitudinal = GET_FLAG(param, SUBARU_PARAM_LONGITUDINAL) && !subaru_gen2; #endif if (subaru_gen2) { diff --git a/tests/safety/common.py b/tests/safety/common.py index 3bead37982..36df296eac 100644 --- a/tests/safety/common.py +++ b/tests/safety/common.py @@ -955,7 +955,7 @@ def test_tx_hook_on_wrong_safety_mode(self): if attr.startswith('TestToyota') and current_test.startswith('TestToyota'): continue if {attr, current_test}.issubset({'TestSubaruGen1TorqueStockLongitudinalSafety', 'TestSubaruGen2TorqueStockLongitudinalSafety', - 'TestSubaruGen1LongitudinalSafety', 'TestSubaruGen2LongitudinalSafety'}): + 'TestSubaruGen1LongitudinalSafety'}): continue if {attr, current_test}.issubset({'TestVolkswagenPqSafety', 'TestVolkswagenPqStockSafety', 'TestVolkswagenPqLongSafety'}): continue diff --git a/tests/safety/test_subaru.py b/tests/safety/test_subaru.py index ab992e53d2..617d342f96 100755 --- a/tests/safety/test_subaru.py +++ b/tests/safety/test_subaru.py @@ -7,23 +7,18 @@ from functools import partial -MSG_SUBARU_Brake_Status = 0x13c -MSG_SUBARU_CruiseControl = 0x240 -MSG_SUBARU_Throttle = 0x40 -MSG_SUBARU_Steering_Torque = 0x119 -MSG_SUBARU_Wheel_Speeds = 0x13a -MSG_SUBARU_ES_LKAS = 0x122 -MSG_SUBARU_ES_Brake = 0x220 -MSG_SUBARU_ES_Distance = 0x221 -MSG_SUBARU_ES_Status = 0x222 -MSG_SUBARU_ES_DashStatus = 0x321 -MSG_SUBARU_ES_LKAS_State = 0x322 -MSG_SUBARU_ES_Infotainment = 0x323 -MSG_SUBARU_ES_UDS_Request = 0x787 -MSG_SUBARU_ES_HighBeamAssist = 0x121 -MSG_SUBARU_ES_STATIC_1 = 0x22a -MSG_SUBARU_ES_STATIC_2 = 0x325 - +MSG_SUBARU_Brake_Status = 0x13c +MSG_SUBARU_CruiseControl = 0x240 +MSG_SUBARU_Throttle = 0x40 +MSG_SUBARU_Steering_Torque = 0x119 +MSG_SUBARU_Wheel_Speeds = 0x13a +MSG_SUBARU_ES_LKAS = 0x122 +MSG_SUBARU_ES_Brake = 0x220 +MSG_SUBARU_ES_Distance = 0x221 +MSG_SUBARU_ES_Status = 0x222 +MSG_SUBARU_ES_DashStatus = 0x321 +MSG_SUBARU_ES_LKAS_State = 0x322 +MSG_SUBARU_ES_Infotainment = 0x323 SUBARU_MAIN_BUS = 0 SUBARU_ALT_BUS = 1 @@ -37,15 +32,9 @@ def lkas_tx_msgs(alt_bus): [MSG_SUBARU_ES_LKAS_State, SUBARU_MAIN_BUS], [MSG_SUBARU_ES_Infotainment, SUBARU_MAIN_BUS]] -def long_tx_msgs(alt_bus): - return [[MSG_SUBARU_ES_Brake, alt_bus], - [MSG_SUBARU_ES_Status, alt_bus]] - -def additional_tx_msgs(alt_bus): - return [[MSG_SUBARU_ES_UDS_Request, SUBARU_CAM_BUS], - [MSG_SUBARU_ES_HighBeamAssist, SUBARU_MAIN_BUS], - [MSG_SUBARU_ES_STATIC_1, SUBARU_MAIN_BUS], - [MSG_SUBARU_ES_STATIC_2, SUBARU_MAIN_BUS]] +def long_tx_msgs(): + return [[MSG_SUBARU_ES_Brake, SUBARU_MAIN_BUS], + [MSG_SUBARU_ES_Status, SUBARU_MAIN_BUS]] def fwd_blacklisted_addr(): return {SUBARU_CAM_BUS: [MSG_SUBARU_ES_LKAS, MSG_SUBARU_ES_DashStatus, MSG_SUBARU_ES_LKAS_State, MSG_SUBARU_ES_Infotainment]} @@ -181,29 +170,7 @@ class TestSubaruGen2TorqueStockLongitudinalSafety(TestSubaruStockLongitudinalSaf class TestSubaruGen1LongitudinalSafety(TestSubaruLongitudinalSafetyBase, TestSubaruTorqueSafetyBase): FLAGS = Panda.FLAG_SUBARU_LONG - TX_MSGS = lkas_tx_msgs(SUBARU_MAIN_BUS) + long_tx_msgs(SUBARU_MAIN_BUS) - - -class TestSubaruGen2LongitudinalSafety(TestSubaruLongitudinalSafetyBase, TestSubaruTorqueSafetyBase): - ALT_MAIN_BUS = SUBARU_ALT_BUS - ALT_CAM_BUS = SUBARU_ALT_BUS - - MAX_RATE_UP = 40 - MAX_RATE_DOWN = 40 - MAX_TORQUE = 1000 - - FLAGS = Panda.FLAG_SUBARU_LONG | Panda.FLAG_SUBARU_GEN2 - TX_MSGS = lkas_tx_msgs(SUBARU_ALT_BUS) + long_tx_msgs(SUBARU_ALT_BUS) + additional_tx_msgs(SUBARU_ALT_BUS) - - def _es_uds_msg(self, sid: int): - return libpanda_py.make_CANPacket(MSG_SUBARU_ES_UDS_Request, 2, b'\x00' + sid.to_bytes(1) + b'\x00' * 6) - - def test_es_uds_message(self): - allowed_sids = [0x3e, 0x22, 0x28] - - for sid in range(0xFF): - should_tx = sid in allowed_sids - self.assertEqual(self._tx(self._es_uds_msg(sid)), should_tx) + TX_MSGS = lkas_tx_msgs(SUBARU_MAIN_BUS) + long_tx_msgs() if __name__ == "__main__":