cleanup CAN definitions (#1170)

board/can_definitions.h

@@ -1,10 +1,24 @@
-#include "dlc_to_len.h"
+#pragma once
 
+const uint8_t PANDA_CAN_CNT = 3U;
+const uint8_t PANDA_BUS_CNT = 4U;
+
+// bump this when changing the CAN packet
 #define CAN_PACKET_VERSION 2
+
+#define CANPACKET_HEAD_SIZE 5U
+
+#if !defined(STM32F4) && !defined(STM32F2)
+  #define CANFD
+  #define CANPACKET_DATA_SIZE_MAX 64U
+#else
+  #define CANPACKET_DATA_SIZE_MAX 8U
+#endif
+
 typedef struct {
   unsigned char reserved : 1;
   unsigned char bus : 3;
-  unsigned char data_len_code : 4;
+  unsigned char data_len_code : 4;  // lookup length with dlc_to_len
   unsigned char rejected : 1;
   unsigned char returned : 1;
   unsigned char extended : 1;
@@ -12,14 +26,8 @@ typedef struct {
   unsigned char data[CANPACKET_DATA_SIZE_MAX];
 } __attribute__((packed, aligned(4))) CANPacket_t;
 
+const unsigned char dlc_to_len[] = {0U, 1U, 2U, 3U, 4U, 5U, 6U, 7U, 8U, 12U, 16U, 20U, 24U, 32U, 48U, 64U};
+
 #define GET_BUS(msg) ((msg)->bus)
 #define GET_LEN(msg) (dlc_to_len[(msg)->data_len_code])
 #define GET_ADDR(msg) ((msg)->addr)
-
-// Flasher and pedal use raw mailbox access
-#define GET_MAILBOX_BYTE(msg, b) (((int)(b) > 3) ? (((msg)->RDHR >> (8U * ((unsigned int)(b) % 4U))) & 0xFFU) : (((msg)->RDLR >> (8U * (unsigned int)(b))) & 0xFFU))
-#define GET_MAILBOX_BYTES_04(msg) ((msg)->RDLR)
-#define GET_MAILBOX_BYTES_48(msg) ((msg)->RDHR)
-
-#define WORD_TO_BYTE_ARRAY(dst8, src32) 0[dst8] = ((src32) & 0xFFU); 1[dst8] = (((src32) >> 8U) & 0xFFU); 2[dst8] = (((src32) >> 16U) & 0xFFU); 3[dst8] = (((src32) >> 24U) & 0xFFU)
-#define BYTE_ARRAY_TO_WORD(dst32, src8) ((dst32) = 0[src8] | (1[src8] << 8U) | (2[src8] << 16U) | (3[src8] << 24U))

board/dlc_to_len.h

@@ -1 +0,0 @@
-unsigned char dlc_to_len[] = {0U, 1U, 2U, 3U, 4U, 5U, 6U, 7U, 8U, 12U, 16U, 20U, 24U, 32U, 48U, 64U};

board/drivers/can_common.h

@@ -66,6 +66,10 @@ can_buffer(tx3_q, 0x1A0)
 // cppcheck-suppress misra-c2012-9.3
 can_ring *can_queues[] = {&can_tx1_q, &can_tx2_q, &can_tx3_q, &can_txgmlan_q};
 
+// helpers
+#define WORD_TO_BYTE_ARRAY(dst8, src32) 0[dst8] = ((src32) & 0xFFU); 1[dst8] = (((src32) >> 8U) & 0xFFU); 2[dst8] = (((src32) >> 16U) & 0xFFU); 3[dst8] = (((src32) >> 24U) & 0xFFU)
+#define BYTE_ARRAY_TO_WORD(dst32, src8) ((dst32) = 0[src8] | (1[src8] << 8U) | (2[src8] << 16U) | (3[src8] << 24U))
+
 // ********************* interrupt safe queue *********************
 bool can_pop(can_ring *q, CANPacket_t *elem) {
   bool ret = 0;
@@ -169,7 +173,7 @@ bus_config_t bus_config[] = {
 
 void can_init_all(void) {
   bool ret = true;
-  for (uint8_t i=0U; i < CAN_CNT; i++) {
+  for (uint8_t i=0U; i < PANDA_CAN_CNT; i++) {
     if (!current_board->has_canfd) {
       bus_config[i].can_data_speed = 0U;
     }
@@ -224,7 +228,7 @@ bool can_tx_check_min_slots_free(uint32_t min) {
 
 void can_send(CANPacket_t *to_push, uint8_t bus_number, bool skip_tx_hook) {
   if (skip_tx_hook || safety_tx_hook(to_push) != 0) {
-    if (bus_number < BUS_CNT) {
+    if (bus_number < PANDA_BUS_CNT) {
       // add CAN packet to send queue
       if ((bus_number == 3U) && (bus_config[3].can_num_lookup == 0xFFU)) {
         gmlan_send_errs += bitbang_gmlan(to_push) ? 0U : 1U;

board/flasher.h

@@ -113,11 +113,11 @@ void comms_endpoint2_write(uint8_t *data, uint32_t len) {
 int spi_cb_rx(uint8_t *data, int len, uint8_t *data_out) {
   UNUSED(len);
   ControlPacket_t control_req;
-  
+
   int resp_len = 0;
   switch (data[0]) {
     case 0:
-      // control transfer      
+      // control transfer
       control_req.request = ((USB_Setup_TypeDef *)(data+4))->b.bRequest;
       control_req.param1 = ((USB_Setup_TypeDef *)(data+4))->b.wValue.w;
       control_req.param2 = ((USB_Setup_TypeDef *)(data+4))->b.wIndex.w;

board/main_comms.h

@@ -392,7 +392,7 @@ int comms_control_handler(ControlPacket_t *req, uint8_t *resp) {
       break;
     // **** 0xde: set can bitrate
     case 0xde:
-      if ((req->param1 < BUS_CNT) && is_speed_valid(req->param2, speeds, sizeof(speeds)/sizeof(speeds[0]))) {
+      if ((req->param1 < PANDA_BUS_CNT) && is_speed_valid(req->param2, speeds, sizeof(speeds)/sizeof(speeds[0]))) {
         bus_config[req->param1].can_speed = req->param2;
         bool ret = can_init(CAN_NUM_FROM_BUS_NUM(req->param1));
         UNUSED(ret);
@@ -488,7 +488,7 @@ int comms_control_handler(ControlPacket_t *req, uint8_t *resp) {
       if (req->param1 == 0xFFFFU) {
         puts("Clearing CAN Rx queue\n");
         can_clear(&can_rx_q);
-      } else if (req->param1 < BUS_CNT) {
+      } else if (req->param1 < PANDA_BUS_CNT) {
         puts("Clearing CAN Tx queue\n");
         can_clear(can_queues[req->param1]);
       } else {
@@ -541,7 +541,7 @@ int comms_control_handler(ControlPacket_t *req, uint8_t *resp) {
       break;
     // **** 0xf9: set CAN FD data bitrate
     case 0xf9:
-      if ((req->param1 < CAN_CNT) &&
+      if ((req->param1 < PANDA_CAN_CNT) &&
            current_board->has_canfd &&
            is_speed_valid(req->param2, data_speeds, sizeof(data_speeds)/sizeof(data_speeds[0]))) {
         bus_config[req->param1].can_data_speed = req->param2;
@@ -557,7 +557,7 @@ int comms_control_handler(ControlPacket_t *req, uint8_t *resp) {
       break;
     // **** 0xfc: set CAN FD non-ISO mode
     case 0xfc:
-      if ((req->param1 < CAN_CNT) && current_board->has_canfd) {
+      if ((req->param1 < PANDA_CAN_CNT) && current_board->has_canfd) {
         bus_config[req->param1].canfd_non_iso = (req->param2 != 0U);
         bool ret = can_init(CAN_NUM_FROM_BUS_NUM(req->param1));
         UNUSED(ret);

board/panda.h

@@ -12,15 +12,4 @@
 #define MAX_CAN_MSGS_PER_BULK_TRANSFER 51U
 #define MAX_EP1_CHUNK_PER_BULK_TRANSFER 16256U // max data stream chunk in bytes, shouldn't be higher than 16320 or counter will overflow
 
-// CAN definitions
-#define CANPACKET_HEAD_SIZE 5U
-
-#if !defined(STM32F4) && !defined(STM32F2)
-  #define CANPACKET_DATA_SIZE_MAX 64U
-#else
-  #define CANPACKET_DATA_SIZE_MAX 8U
-#endif
-
-#define CAN_CNT 3U
-#define BUS_CNT 4U
 #define CAN_INIT_TIMEOUT_MS 500U

board/safety.h

@@ -1,4 +1,5 @@
 #include "safety_declarations.h"
+#include "can_definitions.h"
 
 // include the safety policies.
 #include "safety/safety_defaults.h"
@@ -18,10 +19,6 @@
 #include "safety/safety_elm327.h"
 #include "safety/safety_body.h"
 
-#ifdef STM32H7
-#define CANFD
-#endif
-
 // CAN-FD only safety modes
 #ifdef CANFD
 #include "safety/safety_hyundai_canfd.h"

board/stm32fx/llbxcan.h

@@ -1,3 +1,8 @@
+// Flasher and pedal use raw mailbox access
+#define GET_MAILBOX_BYTE(msg, b) (((int)(b) > 3) ? (((msg)->RDHR >> (8U * ((unsigned int)(b) % 4U))) & 0xFFU) : (((msg)->RDLR >> (8U * (unsigned int)(b))) & 0xFFU))
+#define GET_MAILBOX_BYTES_04(msg) ((msg)->RDLR)
+#define GET_MAILBOX_BYTES_48(msg) ((msg)->RDHR)
+
 // SAE 2284-3 : minimum 16 tq, SJW 3, sample point at 81.3%
 #define CAN_QUANTA 16U
 #define CAN_SEQ1 12U

board/stm32fx/stm32fx_config.h

@@ -12,7 +12,7 @@
 
 #define CORE_FREQ 96U // in Mhz
 //APB1 - 48Mhz, APB2 - 96Mhz
-#define APB1_FREQ CORE_FREQ/2U 
+#define APB1_FREQ CORE_FREQ/2U
 #define APB2_FREQ CORE_FREQ/1U
 
 #define BOOTLOADER_ADDRESS 0x1FFF0004U

tests/hitl/3_usb_to_can.py

@@ -31,22 +31,6 @@ def test_can_loopback(p):
     assert 0x1aa == sr[0][0] == lb[0][0]
     assert b"message" == sr[0][2] == lb[0][2]
 
-@test_all_pandas
-@panda_connect_and_init
-def test_safety_nooutput(p):
-  p.set_safety_mode(Panda.SAFETY_SILENT)
-  p.set_can_loopback(True)
-
-  # send a message on bus 0
-  p.can_send(0x1aa, b"message", 0)
-
-  # confirm receive nothing
-  time.sleep(0.05)
-  r = p.can_recv()
-  # bus 192 is messages blocked by TX safety hook on bus 0
-  assert len([x for x in r if x[3] != 192]) == 0
-  assert len([x for x in r if x[3] == 192]) == 1
-
 @test_all_pandas
 @panda_connect_and_init
 def test_reliability(p):

tests/hitl/6_safety.py

@@ -0,0 +1,33 @@
+import time
+from nose.tools import assert_equal
+
+from panda import Panda
+from .helpers import test_all_pandas, panda_connect_and_init
+
+@test_all_pandas
+@panda_connect_and_init
+def test_safety_nooutput(p):
+  p.set_safety_mode(Panda.SAFETY_SILENT)
+  p.set_can_loopback(True)
+
+  # send a message on bus 0
+  p.can_send(0x1aa, b"message", 0)
+
+  # confirm receive nothing
+  time.sleep(0.05)
+  r = p.can_recv()
+  # bus 192 is messages blocked by TX safety hook on bus 0
+  assert len([x for x in r if x[3] != 192]) == 0
+  assert len([x for x in r if x[3] == 192]) == 1
+
+@test_all_pandas
+@panda_connect_and_init
+def test_canfd_safety_modes(p):
+  # works on all pandas
+  p.set_safety_mode(Panda.SAFETY_TOYOTA)
+  assert_equal(p.health()['safety_mode'], Panda.SAFETY_TOYOTA)
+
+  # shouldn't be able to set a CAN-FD safety mode on non CAN-FD panda
+  p.set_safety_mode(Panda.SAFETY_HYUNDAI_CANFD)
+  expected_mode = Panda.SAFETY_HYUNDAI_CANFD if p.get_type() in Panda.H7_DEVICES else Panda.SAFETY_SILENT
+  assert_equal(p.health()['safety_mode'], expected_mode)