SPI: log checksum errors in health (#1334)

* SPI: log checksum errors in health

* actually send it

* check in hitl teardown

* check that

* fix misra

board/drivers/spi.h

@@ -31,6 +31,8 @@ uint8_t spi_state = SPI_STATE_HEADER;
 uint8_t spi_endpoint;
 uint16_t spi_data_len_mosi;
 uint16_t spi_data_len_miso;
+uint16_t spi_checksum_error_count = 0;
+
 
 #define SPI_HEADER_SIZE 7U
 
@@ -63,6 +65,7 @@ bool check_checksum(uint8_t *data, uint16_t len) {
 
 void spi_handle_rx(void) {
   uint8_t next_rx_state = SPI_STATE_HEADER;
+  bool checksum_valid = false;
 
   // parse header
   spi_endpoint = spi_buf_rx[1];
@@ -70,7 +73,8 @@ void spi_handle_rx(void) {
   spi_data_len_miso = (spi_buf_rx[5] << 8) | spi_buf_rx[4];
 
   if (spi_state == SPI_STATE_HEADER) {
-    if ((spi_buf_rx[0] == SPI_SYNC_BYTE) && check_checksum(spi_buf_rx, SPI_HEADER_SIZE)) {
+    checksum_valid = check_checksum(spi_buf_rx, SPI_HEADER_SIZE);
+    if ((spi_buf_rx[0] == SPI_SYNC_BYTE) && checksum_valid) {
       // response: ACK and start receiving data portion
       spi_buf_tx[0] = SPI_HACK;
       next_rx_state = SPI_STATE_HEADER_ACK;
@@ -85,7 +89,8 @@ void spi_handle_rx(void) {
     // We got everything! Based on the endpoint specified, call the appropriate handler
     uint16_t response_len = 0U;
     bool reponse_ack = false;
-    if (check_checksum(&(spi_buf_rx[SPI_HEADER_SIZE]), spi_data_len_mosi + 1U)) {
+    checksum_valid = check_checksum(&(spi_buf_rx[SPI_HEADER_SIZE]), spi_data_len_mosi + 1U);
+    if (checksum_valid) {
       if (spi_endpoint == 0U) {
         if (spi_data_len_mosi >= sizeof(ControlPacket_t)) {
           ControlPacket_t ctrl;
@@ -142,6 +147,9 @@ void spi_handle_rx(void) {
   }
 
   spi_state = next_rx_state;
+  if (!checksum_valid && (spi_checksum_error_count < __UINT16_MAX__)) {
+    spi_checksum_error_count += 1U;
+  }
 }
 
 void spi_handle_tx(bool timed_out) {

board/health.h

@@ -1,7 +1,6 @@
 // When changing these structs, python/__init__.py needs to be kept up to date!
 
-#define HEALTH_PACKET_VERSION 11
-
+#define HEALTH_PACKET_VERSION 12
 struct __attribute__((packed)) health_t {
   uint32_t uptime_pkt;
   uint32_t voltage_pkt;
@@ -26,6 +25,7 @@ struct __attribute__((packed)) health_t {
   float interrupt_load;
   uint8_t fan_power;
   uint8_t safety_rx_checks_invalid;
+  uint16_t spi_checksum_error_count;
 };
 
 #define CAN_HEALTH_PACKET_VERSION 4

board/main_comms.h

@@ -31,6 +31,8 @@ int get_health_pkt(void *dat) {
   health->heartbeat_lost_pkt = (uint8_t)(heartbeat_lost);
   health->safety_rx_checks_invalid = safety_rx_checks_invalid;
 
+  health->spi_checksum_error_count = spi_checksum_error_count;
+
   health->fault_status_pkt = fault_status;
   health->faults_pkt = faults;
 

python/__init__.py

@@ -182,9 +182,9 @@ class Panda:
   HW_TYPE_TRES = b'\x09'
 
   CAN_PACKET_VERSION = 4
-  HEALTH_PACKET_VERSION = 11
+  HEALTH_PACKET_VERSION = 12
   CAN_HEALTH_PACKET_VERSION = 4
-  HEALTH_STRUCT = struct.Struct("<IIIIIIIIIBBBBBBHBBBHfBB")
+  HEALTH_STRUCT = struct.Struct("<IIIIIIIIIBBBBBBHBBBHfBBH")
   CAN_HEALTH_STRUCT = struct.Struct("<BIBBBBBBBBIIIIIIIHHBBB")
 
   F2_DEVICES = (HW_TYPE_PEDAL, )
@@ -558,6 +558,7 @@ class Panda:
       "interrupt_load": a[20],
       "fan_power": a[21],
       "safety_rx_checks_invalid": a[22],
+      "spi_checksum_error_count": a[23],
     }
 
   @ensure_can_health_packet_version

tests/hitl/conftest.py

@@ -132,6 +132,9 @@ def func_fixture_panda(request, module_panda):
   # Check for faults
   assert p.health()['fault_status'] == 0
 
+  # Check for SPI errors
+  assert p.health()['spi_checksum_error_count'] == 0
+
   # Check health of each CAN core after test, normal to fail for test_gen2_loopback on OBD bus, so skipping
   mark = request.node.get_closest_marker('panda_expect_can_error')
   expect_can_error = mark is not None