From 51a254f78ef3b6fdc0235e3524d64f57b959d789 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Wed, 6 Sep 2023 23:08:34 -0700 Subject: [PATCH 1/5] common addrs behind macros --- board/safety/safety_hyundai.h | 36 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/board/safety/safety_hyundai.h b/board/safety/safety_hyundai.h index 66cf5c2ec4..dec473b53d 100644 --- a/board/safety/safety_hyundai.h +++ b/board/safety/safety_hyundai.h @@ -51,40 +51,38 @@ const CanMsg HYUNDAI_CAMERA_SCC_TX_MSGS[] = { {0x485, 0, 4}, // LFAHDA_MFC Bus 0 }; +#define HYUNDAI_COMMON_ADDR_CHECKS(legacy) \ + {.msg = {{0x260, 0, 8, .check_checksum = true, .max_counter = 3U, .expected_timestep = 10000U}, \ + {0x371, 0, 8, .expected_timestep = 10000U}, { 0 }}}, \ + {.msg = {{0x386, 0, 8, .check_checksum = !legacy, .max_counter = legacy ? 0U : 15U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, \ + {.msg = {{0x394, 0, 8, .check_checksum = !legacy, .max_counter = legacy ? 0U : 7U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, \ + +#define HYUNDAI_SCC12_ADDR_CHECK(scc_bus) \ + {.msg = {{0x421, scc_bus, 8, .check_checksum = true, .max_counter = 15U, .expected_timestep = 20000U}, { 0 }, { 0 }}}, \ + AddrCheckStruct hyundai_addr_checks[] = { - {.msg = {{0x260, 0, 8, .check_checksum = true, .max_counter = 3U, .expected_timestep = 10000U}, - {0x371, 0, 8, .expected_timestep = 10000U}, { 0 }}}, - {.msg = {{0x386, 0, 8, .check_checksum = true, .max_counter = 15U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, - {.msg = {{0x394, 0, 8, .check_checksum = true, .max_counter = 7U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, - {.msg = {{0x421, 0, 8, .check_checksum = true, .max_counter = 15U, .expected_timestep = 20000U}, { 0 }, { 0 }}}, + HYUNDAI_COMMON_ADDR_CHECKS(false) + HYUNDAI_SCC12_ADDR_CHECK(0) }; #define HYUNDAI_ADDR_CHECK_LEN (sizeof(hyundai_addr_checks) / sizeof(hyundai_addr_checks[0])) AddrCheckStruct hyundai_cam_scc_addr_checks[] = { - {.msg = {{0x260, 0, 8, .check_checksum = true, .max_counter = 3U, .expected_timestep = 10000U}, - {0x371, 0, 8, .expected_timestep = 10000U}, { 0 }}}, - {.msg = {{0x386, 0, 8, .check_checksum = true, .max_counter = 15U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, - {.msg = {{0x394, 0, 8, .check_checksum = true, .max_counter = 7U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, - {.msg = {{0x421, 2, 8, .check_checksum = true, .max_counter = 15U, .expected_timestep = 20000U}, { 0 }, { 0 }}}, + HYUNDAI_COMMON_ADDR_CHECKS(false) + HYUNDAI_SCC12_ADDR_CHECK(2) }; #define HYUNDAI_CAM_SCC_ADDR_CHECK_LEN (sizeof(hyundai_cam_scc_addr_checks) / sizeof(hyundai_cam_scc_addr_checks[0])) AddrCheckStruct hyundai_long_addr_checks[] = { - {.msg = {{0x260, 0, 8, .check_checksum = true, .max_counter = 3U, .expected_timestep = 10000U}, - {0x371, 0, 8, .expected_timestep = 10000U}, { 0 }}}, - {.msg = {{0x386, 0, 8, .check_checksum = true, .max_counter = 15U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, - {.msg = {{0x394, 0, 8, .check_checksum = true, .max_counter = 7U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, + HYUNDAI_COMMON_ADDR_CHECKS(false) + // Use CLU11 (buttons) to manage controls allowed instead of SCC cruise state {.msg = {{0x4F1, 0, 4, .check_checksum = false, .max_counter = 15U, .expected_timestep = 20000U}, { 0 }, { 0 }}}, }; #define HYUNDAI_LONG_ADDR_CHECK_LEN (sizeof(hyundai_long_addr_checks) / sizeof(hyundai_long_addr_checks[0])) // older hyundai models have less checks due to missing counters and checksums AddrCheckStruct hyundai_legacy_addr_checks[] = { - {.msg = {{0x260, 0, 8, .check_checksum = true, .max_counter = 3U, .expected_timestep = 10000U}, - {0x371, 0, 8, .expected_timestep = 10000U}, { 0 }}}, - {.msg = {{0x386, 0, 8, .expected_timestep = 10000U}, { 0 }, { 0 }}}, - {.msg = {{0x394, 0, 8, .expected_timestep = 10000U}, { 0 }, { 0 }}}, - {.msg = {{0x421, 0, 8, .check_checksum = true, .max_counter = 15U, .expected_timestep = 20000U}, { 0 }, { 0 }}}, + HYUNDAI_COMMON_ADDR_CHECKS(true) + HYUNDAI_SCC12_ADDR_CHECK(0) }; #define HYUNDAI_LEGACY_ADDR_CHECK_LEN (sizeof(hyundai_legacy_addr_checks) / sizeof(hyundai_legacy_addr_checks[0])) From e41b796a005b66bf8fa49694639205e319103849 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Thu, 7 Sep 2023 00:38:08 -0700 Subject: [PATCH 2/5] draft so far --- board/safety.h | 5 ++++- board/safety/safety_hyundai.h | 2 +- board/safety/safety_hyundai_canfd.h | 14 +++++++++++++- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/board/safety.h b/board/safety.h index 2c6daddbd9..dc8962627d 100644 --- a/board/safety.h +++ b/board/safety.h @@ -247,8 +247,11 @@ bool addr_safety_check(CANPacket_t *to_push, } else { rx_checks->check[index].valid_quality_flag = true; } + return is_msg_valid(rx_checks->check, index); + } else { + // return invalid to safety modes for messages not in checks + return false; } - return is_msg_valid(rx_checks->check, index); } void generic_rx_checks(bool stock_ecu_detected) { diff --git a/board/safety/safety_hyundai.h b/board/safety/safety_hyundai.h index dec473b53d..1e55ec5835 100644 --- a/board/safety/safety_hyundai.h +++ b/board/safety/safety_hyundai.h @@ -224,7 +224,7 @@ static int hyundai_rx_hook(CANPacket_t *to_push) { } generic_rx_checks(stock_ecu_detected); } - return valid; + return true; } static int hyundai_tx_hook(CANPacket_t *to_send) { diff --git a/board/safety/safety_hyundai_canfd.h b/board/safety/safety_hyundai_canfd.h index 3ea7a486a5..1d6efec79e 100644 --- a/board/safety/safety_hyundai_canfd.h +++ b/board/safety/safety_hyundai_canfd.h @@ -53,6 +53,18 @@ const CanMsg HYUNDAI_CANFD_HDA1_TX_MSGS[] = { {0x1E0, 0, 16}, // LFAHDA_CLUSTER }; +#define HYUNDAI_CANFD_EV_ADDR_CHECKS \ + /* ACCELERATOR: buses are swapped for HDA2 variants */ \ + {.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}, { 0 }}}, \ + /* TCS buses are swapped for HDA2 variants */ \ + {.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 }}}, \ + +#define HYUNDAI_CANFD_HYBRID_ADDR_CHECKS \ + /* ACCELERATOR_ALT: TODO: bus is always PT? no HDA hybrids? */ \ + {.msg = {{0x105, 0, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, { 0 }, { 0 }}}, \ + 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}, @@ -253,7 +265,7 @@ static int hyundai_canfd_rx_hook(CANPacket_t *to_push) { } generic_rx_checks(stock_ecu_detected); - return valid; + return true; } static int hyundai_canfd_tx_hook(CANPacket_t *to_send) { From 345a258de62bd6b71b6db831591deabf7e27057b Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Thu, 7 Sep 2023 03:17:11 -0700 Subject: [PATCH 3/5] Revert "draft so far" This reverts commit e41b796a005b66bf8fa49694639205e319103849. --- board/safety.h | 5 +---- board/safety/safety_hyundai.h | 2 +- board/safety/safety_hyundai_canfd.h | 14 +------------- 3 files changed, 3 insertions(+), 18 deletions(-) diff --git a/board/safety.h b/board/safety.h index dc8962627d..2c6daddbd9 100644 --- a/board/safety.h +++ b/board/safety.h @@ -247,11 +247,8 @@ bool addr_safety_check(CANPacket_t *to_push, } else { rx_checks->check[index].valid_quality_flag = true; } - return is_msg_valid(rx_checks->check, index); - } else { - // return invalid to safety modes for messages not in checks - return false; } + return is_msg_valid(rx_checks->check, index); } void generic_rx_checks(bool stock_ecu_detected) { diff --git a/board/safety/safety_hyundai.h b/board/safety/safety_hyundai.h index 1e55ec5835..dec473b53d 100644 --- a/board/safety/safety_hyundai.h +++ b/board/safety/safety_hyundai.h @@ -224,7 +224,7 @@ static int hyundai_rx_hook(CANPacket_t *to_push) { } generic_rx_checks(stock_ecu_detected); } - return true; + return valid; } static int hyundai_tx_hook(CANPacket_t *to_send) { diff --git a/board/safety/safety_hyundai_canfd.h b/board/safety/safety_hyundai_canfd.h index 1d6efec79e..3ea7a486a5 100644 --- a/board/safety/safety_hyundai_canfd.h +++ b/board/safety/safety_hyundai_canfd.h @@ -53,18 +53,6 @@ const CanMsg HYUNDAI_CANFD_HDA1_TX_MSGS[] = { {0x1E0, 0, 16}, // LFAHDA_CLUSTER }; -#define HYUNDAI_CANFD_EV_ADDR_CHECKS \ - /* ACCELERATOR: buses are swapped for HDA2 variants */ \ - {.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}, { 0 }}}, \ - /* TCS buses are swapped for HDA2 variants */ \ - {.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 }}}, \ - -#define HYUNDAI_CANFD_HYBRID_ADDR_CHECKS \ - /* ACCELERATOR_ALT: TODO: bus is always PT? no HDA hybrids? */ \ - {.msg = {{0x105, 0, 32, .check_checksum = true, .max_counter = 0xffU, .expected_timestep = 10000U}, { 0 }, { 0 }}}, \ - 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}, @@ -265,7 +253,7 @@ static int hyundai_canfd_rx_hook(CANPacket_t *to_push) { } generic_rx_checks(stock_ecu_detected); - return true; + return valid; } static int hyundai_canfd_tx_hook(CANPacket_t *to_send) { From 3a0e118f4f6d69e41d71b2d7baec97a30afc0dc1 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Thu, 7 Sep 2023 03:20:56 -0700 Subject: [PATCH 4/5] MISRA --- board/safety/safety_hyundai.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/board/safety/safety_hyundai.h b/board/safety/safety_hyundai.h index dec473b53d..459534d575 100644 --- a/board/safety/safety_hyundai.h +++ b/board/safety/safety_hyundai.h @@ -51,14 +51,14 @@ const CanMsg HYUNDAI_CAMERA_SCC_TX_MSGS[] = { {0x485, 0, 4}, // LFAHDA_MFC Bus 0 }; -#define HYUNDAI_COMMON_ADDR_CHECKS(legacy) \ - {.msg = {{0x260, 0, 8, .check_checksum = true, .max_counter = 3U, .expected_timestep = 10000U}, \ - {0x371, 0, 8, .expected_timestep = 10000U}, { 0 }}}, \ - {.msg = {{0x386, 0, 8, .check_checksum = !legacy, .max_counter = legacy ? 0U : 15U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, \ - {.msg = {{0x394, 0, 8, .check_checksum = !legacy, .max_counter = legacy ? 0U : 7U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, \ - -#define HYUNDAI_SCC12_ADDR_CHECK(scc_bus) \ - {.msg = {{0x421, scc_bus, 8, .check_checksum = true, .max_counter = 15U, .expected_timestep = 20000U}, { 0 }, { 0 }}}, \ +#define HYUNDAI_COMMON_ADDR_CHECKS(legacy) \ + {.msg = {{0x260, 0, 8, .check_checksum = true, .max_counter = 3U, .expected_timestep = 10000U}, \ + {0x371, 0, 8, .expected_timestep = 10000U}, { 0 }}}, \ + {.msg = {{0x386, 0, 8, .check_checksum = !(legacy), .max_counter = (legacy) ? 0U : 15U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, \ + {.msg = {{0x394, 0, 8, .check_checksum = !(legacy), .max_counter = (legacy) ? 0U : 7U, .expected_timestep = 10000U}, { 0 }, { 0 }}}, \ + +#define HYUNDAI_SCC12_ADDR_CHECK(scc_bus) \ + {.msg = {{0x421, (scc_bus), 8, .check_checksum = true, .max_counter = 15U, .expected_timestep = 20000U}, { 0 }, { 0 }}}, \ AddrCheckStruct hyundai_addr_checks[] = { HYUNDAI_COMMON_ADDR_CHECKS(false) From a785e8e3e03e575824533c6bf51167392bef6ee3 Mon Sep 17 00:00:00 2001 From: Shane Smiskol Date: Thu, 7 Sep 2023 23:56:15 -0700 Subject: [PATCH 5/5] use new SET_ADDR_CHECKS macro --- board/safety/safety_hyundai.h | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/board/safety/safety_hyundai.h b/board/safety/safety_hyundai.h index 459534d575..c3d2576144 100644 --- a/board/safety/safety_hyundai.h +++ b/board/safety/safety_hyundai.h @@ -64,31 +64,27 @@ AddrCheckStruct hyundai_addr_checks[] = { HYUNDAI_COMMON_ADDR_CHECKS(false) HYUNDAI_SCC12_ADDR_CHECK(0) }; -#define HYUNDAI_ADDR_CHECK_LEN (sizeof(hyundai_addr_checks) / sizeof(hyundai_addr_checks[0])) AddrCheckStruct hyundai_cam_scc_addr_checks[] = { HYUNDAI_COMMON_ADDR_CHECKS(false) HYUNDAI_SCC12_ADDR_CHECK(2) }; -#define HYUNDAI_CAM_SCC_ADDR_CHECK_LEN (sizeof(hyundai_cam_scc_addr_checks) / sizeof(hyundai_cam_scc_addr_checks[0])) AddrCheckStruct hyundai_long_addr_checks[] = { HYUNDAI_COMMON_ADDR_CHECKS(false) // Use CLU11 (buttons) to manage controls allowed instead of SCC cruise state {.msg = {{0x4F1, 0, 4, .check_checksum = false, .max_counter = 15U, .expected_timestep = 20000U}, { 0 }, { 0 }}}, }; -#define HYUNDAI_LONG_ADDR_CHECK_LEN (sizeof(hyundai_long_addr_checks) / sizeof(hyundai_long_addr_checks[0])) // older hyundai models have less checks due to missing counters and checksums AddrCheckStruct hyundai_legacy_addr_checks[] = { HYUNDAI_COMMON_ADDR_CHECKS(true) HYUNDAI_SCC12_ADDR_CHECK(0) }; -#define HYUNDAI_LEGACY_ADDR_CHECK_LEN (sizeof(hyundai_legacy_addr_checks) / sizeof(hyundai_legacy_addr_checks[0])) bool hyundai_legacy = false; -addr_checks hyundai_rx_checks = {hyundai_addr_checks, HYUNDAI_ADDR_CHECK_LEN}; +addr_checks hyundai_rx_checks = SET_ADDR_CHECKS(hyundai_addr_checks); static uint8_t hyundai_get_counter(CANPacket_t *to_push) { int addr = GET_ADDR(to_push); @@ -327,11 +323,11 @@ static const addr_checks* hyundai_init(uint16_t param) { } if (hyundai_longitudinal) { - hyundai_rx_checks = (addr_checks){hyundai_long_addr_checks, HYUNDAI_LONG_ADDR_CHECK_LEN}; + hyundai_rx_checks = SET_ADDR_CHECKS(hyundai_long_addr_checks); } else if (hyundai_camera_scc) { - hyundai_rx_checks = (addr_checks){hyundai_cam_scc_addr_checks, HYUNDAI_CAM_SCC_ADDR_CHECK_LEN}; + hyundai_rx_checks = SET_ADDR_CHECKS(hyundai_cam_scc_addr_checks); } else { - hyundai_rx_checks = (addr_checks){hyundai_addr_checks, HYUNDAI_ADDR_CHECK_LEN}; + hyundai_rx_checks = SET_ADDR_CHECKS(hyundai_addr_checks); } return &hyundai_rx_checks; } @@ -342,7 +338,7 @@ static const addr_checks* hyundai_legacy_init(uint16_t param) { hyundai_longitudinal = false; hyundai_camera_scc = false; - hyundai_rx_checks = (addr_checks){hyundai_legacy_addr_checks, HYUNDAI_LEGACY_ADDR_CHECK_LEN}; + hyundai_rx_checks = SET_ADDR_CHECKS(hyundai_legacy_addr_checks); return &hyundai_rx_checks; }