From e9a098761776896957fce61d632a221dcfb90ce6 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Thu, 7 Sep 2023 23:52:27 -0700 Subject: [PATCH] Hyundai CAN FD: utilize macros for common address checks (#1658) * draft * now clean up old comments * no more special broken ice addr checks * revert * fix * add test for ICE HDA1 * Revert "add test for ICE HDA1" This reverts commit 76d2b0f066cc62eb5c67a22c1ad7f9b1fc5f3838. * can be separate * macro for addr check struct * add checks for alt buttons (fixes race condition) * add macro for array len * add comment * misra * comments to help separate * can't put parenthesis around array item * review suggestions * no intermediary macros for making the structs (remove misra violation) * Update board/safety/safety_hyundai_canfd.h * single lines * can avoid double checking hda2 since we have addr checks for it now --- board/safety/safety_hyundai_canfd.h | 133 ++++++++++++++++------------ board/safety_declarations.h | 2 + 2 files changed, 76 insertions(+), 59 deletions(-) diff --git a/board/safety/safety_hyundai_canfd.h b/board/safety/safety_hyundai_canfd.h index 3ea7a486a5..55c7de1090 100644 --- a/board/safety/safety_hyundai_canfd.h +++ b/board/safety/safety_hyundai_canfd.h @@ -53,68 +53,79 @@ const CanMsg HYUNDAI_CANFD_HDA1_TX_MSGS[] = { {0x1E0, 0, 16}, // LFAHDA_CLUSTER }; + +// *** Addresses checked in rx hook *** +// EV, ICE, HYBRID: ACCELERATOR (0x35), ACCELERATOR_BRAKE_ALT (0x100), ACCELERATOR_ALT (0x105) +#define HYUNDAI_CANFD_COMMON_ADDR_CHECKS(pt_bus) \ + {.msg = {{0x35, (pt_bus), 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, \ + {0x100, (pt_bus), 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, \ + {0x105, (pt_bus), 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}}}, \ + {.msg = {{0x175, (pt_bus), 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 20000U}, { 0 }, { 0 }}}, \ + {.msg = {{0xa0, (pt_bus), 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, { 0 }, { 0 }}}, \ + {.msg = {{0xea, (pt_bus), 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, { 0 }, { 0 }}}, \ + +#define HYUNDAI_CANFD_BUTTONS_ADDR_CHECK(pt_bus) \ + {.msg = {{0x1cf, (pt_bus), 8, .check_checksum = false, .max_counter = 0xfU, .expected_timestep = 20000U}, { 0 }, { 0 }}}, \ + +#define HYUNDAI_CANFD_ALT_BUTTONS_ADDR_CHECK(pt_bus) \ + {.msg = {{0x1aa, (pt_bus), 16, .check_checksum = false, .max_counter = 0xffU, .expected_timestep = 20000U}, { 0 }, { 0 }}}, \ + +// SCC_CONTROL (from ADAS unit or camera) +#define HYUNDAI_CANFD_SCC_ADDR_CHECK(scc_bus) \ + {.msg = {{0x1a0, (scc_bus), 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 20000U}, { 0 }, { 0 }}}, \ + + +// *** Non-HDA2 checks *** +// Camera sends SCC messages on HDA1. +// Both button messages exist on some platforms, so we ensure we track the correct one using flag AddrCheckStruct hyundai_canfd_addr_checks[] = { - {.msg = {{0x35, 1, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, - {0x35, 0, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, - {0x105, 0, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}}}, - {.msg = {{0x175, 1, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 20000U}, - {0x175, 0, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 20000U}, { 0 }}}, - {.msg = {{0xa0, 1, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, - {0xa0, 0, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, { 0 }}}, - {.msg = {{0xea, 1, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, - {0xea, 0, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, { 0 }}}, - {.msg = {{0x1a0, 1, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 20000U}, - {0x1a0, 2, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 20000U}, { 0 }}}, - {.msg = {{0x1cf, 1, 8, .check_checksum = false, .max_counter = 0xfU, .expected_timestep = 20000U}, - {0x1cf, 0, 8, .check_checksum = false, .max_counter = 0xfU, .expected_timestep = 20000U}, - {0x1aa, 0, 16, .check_checksum = false, .max_counter = 0xffU, .expected_timestep = 20000U}}}, + HYUNDAI_CANFD_COMMON_ADDR_CHECKS(0) + HYUNDAI_CANFD_BUTTONS_ADDR_CHECK(0) + HYUNDAI_CANFD_SCC_ADDR_CHECK(2) +}; +AddrCheckStruct hyundai_canfd_alt_buttons_addr_checks[] = { + HYUNDAI_CANFD_COMMON_ADDR_CHECKS(0) + HYUNDAI_CANFD_ALT_BUTTONS_ADDR_CHECK(0) + HYUNDAI_CANFD_SCC_ADDR_CHECK(2) +}; + +// Longitudinal checks for HDA1 +AddrCheckStruct hyundai_canfd_long_addr_checks[] = { + HYUNDAI_CANFD_COMMON_ADDR_CHECKS(0) + HYUNDAI_CANFD_BUTTONS_ADDR_CHECK(0) +}; +AddrCheckStruct hyundai_canfd_long_alt_buttons_addr_checks[] = { + HYUNDAI_CANFD_COMMON_ADDR_CHECKS(0) + HYUNDAI_CANFD_ALT_BUTTONS_ADDR_CHECK(0) }; -#define HYUNDAI_CANFD_ADDR_CHECK_LEN (sizeof(hyundai_canfd_addr_checks) / sizeof(hyundai_canfd_addr_checks[0])) -// 0x1a0 is on bus 0 +// Radar sends SCC messages on these cars instead of camera AddrCheckStruct hyundai_canfd_radar_scc_addr_checks[] = { - {.msg = {{0x35, 1, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, - {0x35, 0, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, - {0x105, 0, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}}}, - {.msg = {{0x175, 1, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 20000U}, - {0x175, 0, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 20000U}, { 0 }}}, - {.msg = {{0xa0, 1, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, - {0xa0, 0, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, { 0 }}}, - {.msg = {{0xea, 1, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, - {0xea, 0, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, { 0 }}}, - {.msg = {{0x1a0, 0, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 20000U}, { 0 }, { 0 }}}, - {.msg = {{0x1cf, 1, 8, .check_checksum = false, .max_counter = 0xfU, .expected_timestep = 20000U}, - {0x1cf, 0, 8, .check_checksum = false, .max_counter = 0xfU, .expected_timestep = 20000U}, - {0x1aa, 0, 16, .check_checksum = false, .max_counter = 0xffU, .expected_timestep = 20000U}}}, + HYUNDAI_CANFD_COMMON_ADDR_CHECKS(0) + HYUNDAI_CANFD_BUTTONS_ADDR_CHECK(0) + HYUNDAI_CANFD_SCC_ADDR_CHECK(0) +}; +AddrCheckStruct hyundai_canfd_radar_scc_alt_buttons_addr_checks[] = { + HYUNDAI_CANFD_COMMON_ADDR_CHECKS(0) + HYUNDAI_CANFD_ALT_BUTTONS_ADDR_CHECK(0) + HYUNDAI_CANFD_SCC_ADDR_CHECK(0) }; -#define HYUNDAI_CANFD_RADAR_SCC_ADDR_CHECK_LEN (sizeof(hyundai_canfd_radar_scc_addr_checks) / sizeof(hyundai_canfd_radar_scc_addr_checks[0])) -AddrCheckStruct hyundai_canfd_long_addr_checks[] = { - {.msg = {{0x35, 1, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, - {0x35, 0, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, - {0x105, 0, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}}}, - {.msg = {{0x175, 1, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 20000U}, - {0x175, 0, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 20000U}, { 0 }}}, - {.msg = {{0xa0, 1, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, - {0xa0, 0, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, { 0 }}}, - {.msg = {{0xea, 1, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, - {0xea, 0, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, { 0 }}}, - {.msg = {{0x1cf, 1, 8, .check_checksum = false, .max_counter = 0xfU, .expected_timestep = 20000U}, - {0x1cf, 0, 8, .check_checksum = false, .max_counter = 0xfU, .expected_timestep = 20000U}, - {0x1aa, 0, 16, .check_checksum = false, .max_counter = 0xffU, .expected_timestep = 20000U}}}, + +// *** HDA2 checks *** +// E-CAN is on bus 1, ADAS unit sends SCC messages on HDA2. +// Does not use the alt buttons message +AddrCheckStruct hyundai_canfd_hda2_addr_checks[] = { + HYUNDAI_CANFD_COMMON_ADDR_CHECKS(1) + HYUNDAI_CANFD_BUTTONS_ADDR_CHECK(1) + HYUNDAI_CANFD_SCC_ADDR_CHECK(1) }; -#define HYUNDAI_CANFD_LONG_ADDR_CHECK_LEN (sizeof(hyundai_canfd_long_addr_checks) / sizeof(hyundai_canfd_long_addr_checks[0])) - -AddrCheckStruct hyundai_canfd_ice_addr_checks[] = { - {.msg = {{0x100, 0, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, { 0 }, { 0 }}}, - {.msg = {{0xa0, 0, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, { 0 }, { 0 }}}, - {.msg = {{0xea, 0, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, { 0 }, { 0 }}}, - {.msg = {{0x175, 0, 24, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 20000U}, { 0 }, { 0 }}}, - {.msg = {{0x1aa, 0, 16, .check_checksum = false, .max_counter = 0xffU, .expected_timestep = 20000U}, { 0 }, { 0 }}}, +AddrCheckStruct hyundai_canfd_hda2_long_addr_checks[] = { + HYUNDAI_CANFD_COMMON_ADDR_CHECKS(1) + HYUNDAI_CANFD_BUTTONS_ADDR_CHECK(1) }; -#define HYUNDAI_CANFD_ICE_ADDR_CHECK_LEN (sizeof(hyundai_canfd_ice_addr_checks) / sizeof(hyundai_canfd_ice_addr_checks[0])) -addr_checks hyundai_canfd_rx_checks = {hyundai_canfd_addr_checks, HYUNDAI_CANFD_ADDR_CHECK_LEN}; +addr_checks hyundai_canfd_rx_checks = SET_ADDR_CHECKS(hyundai_canfd_addr_checks); uint16_t hyundai_canfd_crc_lut[256]; @@ -369,14 +380,18 @@ static const addr_checks* hyundai_canfd_init(uint16_t param) { } if (hyundai_longitudinal) { - hyundai_canfd_rx_checks = (addr_checks){hyundai_canfd_long_addr_checks, HYUNDAI_CANFD_LONG_ADDR_CHECK_LEN}; + if (hyundai_canfd_hda2) { + hyundai_canfd_rx_checks = SET_ADDR_CHECKS(hyundai_canfd_hda2_long_addr_checks); + } else { + hyundai_canfd_rx_checks = hyundai_canfd_alt_buttons ? SET_ADDR_CHECKS(hyundai_canfd_long_alt_buttons_addr_checks) : SET_ADDR_CHECKS(hyundai_canfd_long_addr_checks); + } } else { - if (!hyundai_ev_gas_signal && !hyundai_hybrid_gas_signal) { - hyundai_canfd_rx_checks = (addr_checks){hyundai_canfd_ice_addr_checks, HYUNDAI_CANFD_ICE_ADDR_CHECK_LEN}; - } else if (!hyundai_camera_scc && !hyundai_canfd_hda2) { - hyundai_canfd_rx_checks = (addr_checks){hyundai_canfd_radar_scc_addr_checks, HYUNDAI_CANFD_RADAR_SCC_ADDR_CHECK_LEN}; + if (hyundai_canfd_hda2) { + hyundai_canfd_rx_checks = SET_ADDR_CHECKS(hyundai_canfd_hda2_addr_checks); + } else if (!hyundai_camera_scc) { + hyundai_canfd_rx_checks = hyundai_canfd_alt_buttons ? SET_ADDR_CHECKS(hyundai_canfd_radar_scc_alt_buttons_addr_checks) : SET_ADDR_CHECKS(hyundai_canfd_radar_scc_addr_checks); } else { - hyundai_canfd_rx_checks = (addr_checks){hyundai_canfd_addr_checks, HYUNDAI_CANFD_ADDR_CHECK_LEN}; + hyundai_canfd_rx_checks = hyundai_canfd_alt_buttons ? SET_ADDR_CHECKS(hyundai_canfd_alt_buttons_addr_checks) : SET_ADDR_CHECKS(hyundai_canfd_addr_checks); } } diff --git a/board/safety_declarations.h b/board/safety_declarations.h index 9064bb62e0..3050755cb9 100644 --- a/board/safety_declarations.h +++ b/board/safety_declarations.h @@ -4,6 +4,8 @@ #define GET_BYTE(msg, b) ((msg)->data[(b)]) #define GET_FLAG(value, mask) (((__typeof__(mask))(value) & (mask)) == (mask)) +#define SET_ADDR_CHECKS(name) ((addr_checks){(name), (sizeof((name)) / sizeof((name)[0]))}) + uint32_t GET_BYTES(const CANPacket_t *msg, int start, int len) { uint32_t ret = 0U; for (int i = 0; i < len; i++) {