spi: nack on can tx endpoint if buffer is full (#1426)

* spi: nack on can tx endpoint if buffer is full

* handle in python lib

* fix timeout

* fix timeout

* fix linter

* cleanup

* fix

---------

Co-authored-by: Comma Device <device@comma.ai>

board/can_comms.h

@@ -110,8 +110,11 @@ void comms_can_reset(void) {
 }
 
 // TODO: make this more general!
-void usb_cb_ep3_out_complete(void) {
-  if (can_tx_check_min_slots_free(MAX_CAN_MSGS_PER_BULK_TRANSFER)) {
-    usb_outep3_resume_if_paused();
+void refresh_can_tx_slots_available(void) {
+  if (can_tx_check_min_slots_free(MAX_CAN_MSGS_PER_USB_BULK_TRANSFER)) {
+    can_tx_comms_resume_usb();
+  }
+  if (can_tx_check_min_slots_free(MAX_CAN_MSGS_PER_SPI_BULK_TRANSFER)) {
+    can_tx_comms_resume_spi();
   }
 }

board/config.h

@@ -13,7 +13,8 @@
 #define CAN_INIT_TIMEOUT_MS 500U
 #define DEEPSLEEP_WAKEUP_DELAY 3U
 #define USBPACKET_MAX_SIZE 0x40U
-#define MAX_CAN_MSGS_PER_BULK_TRANSFER 51U
+#define MAX_CAN_MSGS_PER_USB_BULK_TRANSFER 51U
+#define MAX_CAN_MSGS_PER_SPI_BULK_TRANSFER 170U
 
 #define VIN_READOUT_DIVIDER 11U
 

board/drivers/bxcan.h

@@ -151,7 +151,7 @@ void process_can(uint8_t can_number) {
           can_health[can_number].total_tx_checksum_error_cnt += 1U;
         }
 
-        usb_cb_ep3_out_complete();
+        refresh_can_tx_slots_available();
       }
     }
 

board/drivers/can_common.h

@@ -147,7 +147,7 @@ void can_clear(can_ring *q) {
   q->r_ptr = 0;
   EXIT_CRITICAL();
   // handle TX buffer full with zero ECUs awake on the bus
-  usb_cb_ep3_out_complete();
+  refresh_can_tx_slots_available();
 }
 
 // assign CAN numbering

board/drivers/fdcan.h

@@ -119,7 +119,7 @@ void process_can(uint8_t can_number) {
           can_health[can_number].total_tx_checksum_error_cnt += 1U;
         }
 
-        usb_cb_ep3_out_complete();
+        refresh_can_tx_slots_available();
       }
     }
 

board/drivers/spi.h

@@ -37,7 +37,7 @@ uint8_t spi_endpoint;
 uint16_t spi_data_len_mosi;
 uint16_t spi_data_len_miso;
 uint16_t spi_checksum_error_count = 0;
-
+bool spi_can_tx_ready = false;
 
 #define SPI_HEADER_SIZE 7U
 
@@ -46,6 +46,10 @@ void llspi_init(void);
 void llspi_mosi_dma(uint8_t *addr, int len);
 void llspi_miso_dma(uint8_t *addr, int len);
 
+void can_tx_comms_resume_spi(void) {
+  spi_can_tx_ready = true;
+}
+
 void spi_init(void) {
   // platform init
   llspi_init();
@@ -114,8 +118,14 @@ void spi_rx_done(void) {
         response_ack = true;
       } else if (spi_endpoint == 3U) {
         if (spi_data_len_mosi > 0U) {
-          comms_can_write(&spi_buf_rx[SPI_HEADER_SIZE], spi_data_len_mosi);
-          response_ack = true;
+          if (spi_can_tx_ready) {
+            spi_can_tx_ready = false;
+            comms_can_write(&spi_buf_rx[SPI_HEADER_SIZE], spi_data_len_mosi);
+            response_ack = true;
+          } else {
+            response_ack = false;
+            print("SPI: CAN NACK\n");
+          }
         } else {
           print("SPI: did expect data for can_write\n");
         }
@@ -126,7 +136,7 @@ void spi_rx_done(void) {
       // Checksum was incorrect
       response_ack = false;
       print("- incorrect data checksum ");
-      puth2(spi_data_len_mosi);
+      puth4(spi_data_len_mosi);
       print("\n");
       hexdump(spi_buf_rx, SPI_HEADER_SIZE);
       hexdump(&(spi_buf_rx[SPI_HEADER_SIZE]), MIN(spi_data_len_mosi, 64));

board/drivers/usb.h

@@ -27,8 +27,7 @@ bool usb_enumerated = false;
 uint16_t usb_last_frame_num = 0U;
 
 void usb_init(void);
-void usb_cb_ep3_out_complete(void);
-void usb_outep3_resume_if_paused(void);
+void refresh_can_tx_slots_available(void);
 
 // **** supporting defines ****
 
@@ -808,7 +807,7 @@ void usb_irqhandler(void) {
       #endif
       // NAK cleared by process_can (if tx buffers have room)
       outep3_processing = false;
-      usb_cb_ep3_out_complete();
+      refresh_can_tx_slots_available();
     } else if ((USBx_OUTEP(3)->DOEPINT & 0x2000) != 0) {
       #ifdef DEBUG_USB
         print("  OUT3 PACKET WTF\n");
@@ -926,7 +925,7 @@ void usb_irqhandler(void) {
   //USBx->GINTMSK = 0xFFFFFFFF & ~(USB_OTG_GINTMSK_NPTXFEM | USB_OTG_GINTMSK_PTXFEM | USB_OTG_GINTSTS_SOF | USB_OTG_GINTSTS_EOPF);
 }
 
-void usb_outep3_resume_if_paused(void) {
+void can_tx_comms_resume_usb(void) {
   ENTER_CRITICAL();
   if (!outep3_processing && (USBx_OUTEP(3)->DOEPCTL & USB_OTG_DOEPCTL_NAKSTS) != 0) {
     USBx_OUTEP(3)->DOEPTSIZ = (32U << 19) | 0x800U;

board/flasher.h

@@ -110,7 +110,7 @@ int comms_can_read(uint8_t *data, uint32_t max_len) {
   return 0;
 }
 
-void usb_cb_ep3_out_complete(void) {}
+void refresh_can_tx_slots_available(void) {}
 
 void comms_endpoint2_write(uint8_t *data, uint32_t len) {
   current_board->set_led(LED_RED, 0);

board/pedal/main.c

@@ -54,7 +54,7 @@ void comms_endpoint2_write(uint8_t *data, uint32_t len) {
   UNUSED(data);
   UNUSED(len);
 }
-void usb_cb_ep3_out_complete(void) {}
+void refresh_can_tx_slots_available(void) {}
 
 int comms_control_handler(ControlPacket_t *req, uint8_t *resp) {
   unsigned int resp_len = 0;

python/__init__.py

@@ -401,14 +401,16 @@ class Panda:
     return []
 
   def reset(self, enter_bootstub=False, enter_bootloader=False, reconnect=True):
+    # no response is expected since it resets right away
+    timeout = 5000 if isinstance(self._handle, PandaSpiHandle) else 15000
     try:
       if enter_bootloader:
-        self._handle.controlWrite(Panda.REQUEST_IN, 0xd1, 0, 0, b'')
+        self._handle.controlWrite(Panda.REQUEST_IN, 0xd1, 0, 0, b'', timeout=timeout)
       else:
         if enter_bootstub:
-          self._handle.controlWrite(Panda.REQUEST_IN, 0xd1, 1, 0, b'')
+          self._handle.controlWrite(Panda.REQUEST_IN, 0xd1, 1, 0, b'', timeout=timeout)
         else:
-          self._handle.controlWrite(Panda.REQUEST_IN, 0xd8, 0, 0, b'')
+          self._handle.controlWrite(Panda.REQUEST_IN, 0xd8, 0, 0, b'', timeout=timeout)
     except Exception:
       pass
     if not enter_bootloader and reconnect:

python/spi.py

@@ -120,8 +120,11 @@ class PandaSpiHandle(BaseHandle):
     logging.debug("starting transfer: endpoint=%d, max_rx_len=%d", endpoint, max_rx_len)
     logging.debug("==============================================")
 
+    n = 0
+    start_time = time.monotonic()
     exc = PandaSpiException()
-    for n in range(MAX_XFER_RETRY_COUNT):
+    while (time.monotonic() - start_time) < timeout*1e-3:
+      n += 1
       logging.debug("\ntry #%d", n+1)
       try:
         logging.debug("- send header")
@@ -129,16 +132,18 @@ class PandaSpiHandle(BaseHandle):
         packet += bytes([reduce(lambda x, y: x^y, packet) ^ CHECKSUM_START])
         spi.xfer2(packet)
 
+        to = timeout - (time.monotonic() - start_time)*1e3
         logging.debug("- waiting for header ACK")
-        self._wait_for_ack(spi, HACK, timeout, 0x11)
+        self._wait_for_ack(spi, HACK, int(to), 0x11)
 
         # send data
         logging.debug("- sending data")
         packet = bytes([*data, self._calc_checksum(data)])
         spi.xfer2(packet)
 
+        to = timeout - (time.monotonic() - start_time)*1e3
         logging.debug("- waiting for data ACK")
-        self._wait_for_ack(spi, DACK, timeout, 0x13)
+        self._wait_for_ack(spi, DACK, int(to), 0x13)
 
         # get response length, then response
         response_len_bytes = bytes(spi.xfer2(b"\x00" * 2))
@@ -154,7 +159,7 @@ class PandaSpiHandle(BaseHandle):
         return dat[:-1]
       except PandaSpiException as e:
         exc = e
-        logging.debug("SPI transfer failed, %d retries left", MAX_XFER_RETRY_COUNT - n - 1, exc_info=True)
+        logging.debug("SPI transfer failed, retrying", exc_info=True)
     raise exc
 
   # libusb1 functions

tests/hitl/8_spi.py

@@ -33,14 +33,14 @@ class TestSpi:
   def test_bad_header(self, mocker, p):
     with patch('panda.python.spi.SYNC', return_value=0):
       with pytest.raises(PandaSpiNackResponse):
-        p.health()
+        p._handle.controlRead(Panda.REQUEST_IN, 0xd2, 0, 0, p.HEALTH_STRUCT.size, timeout=50)
     self._ping(mocker, p)
 
   def test_bad_checksum(self, mocker, p):
     cnt = p.health()['spi_checksum_error_count']
     with patch('panda.python.spi.PandaSpiHandle._calc_checksum', return_value=0):
       with pytest.raises(PandaSpiNackResponse):
-        p.health()
+        p._handle.controlRead(Panda.REQUEST_IN, 0xd2, 0, 0, p.HEALTH_STRUCT.size, timeout=50)
     self._ping(mocker, p)
     assert (p.health()['spi_checksum_error_count'] - cnt) > 0
 
@@ -48,11 +48,11 @@ class TestSpi:
     for _ in range(10):
       ep = random.randint(4, 20)
       with pytest.raises(PandaSpiNackResponse):
-        p._handle.bulkRead(ep, random.randint(1, 1000))
+        p._handle.bulkRead(ep, random.randint(1, 1000), timeout=50)
 
       self._ping(mocker, p)
 
       with pytest.raises(PandaSpiNackResponse):
-        p._handle.bulkWrite(ep, b"abc")
+        p._handle.bulkWrite(ep, b"abc", timeout=50)
 
       self._ping(mocker, p)

tests/libpanda/panda.c

@@ -8,8 +8,9 @@ void process_can(uint8_t can_number) { }
 //int safety_tx_hook(CANPacket_t *to_send) { return 1; }
 
 typedef struct harness_configuration harness_configuration;
-void usb_cb_ep3_out_complete(void);
-void usb_outep3_resume_if_paused(void) { };
+void refresh_can_tx_slots_available(void);
+void can_tx_comms_resume_usb(void) { };
+void can_tx_comms_resume_spi(void) { };
 
 #include "health.h"
 #include "faults.h"