kart-core: fix ~10% STEER_STATUS loss — make CAN RX interrupt-driven

The steering link flapped (STEER_LINK_OK dropping/recovering, spamming the dash
notification): the Steervo sent STEER_STATUS at a steady 50 Hz but the Teensy
received only ~45/s with periodic ~100-300 ms zero-delivery gaps.

Diagnosed with live error counters on both nodes over a 70 s capture. Through
every gap: Steervo tx_error_counter=0, bus_error_count=0; Teensy REC=0, ESR1
ack/crc/frm/stf/bit0/bit1 all 0. Clean protocol engines on both sides while the
app still missed frames => loss above the protocol engine (not the bus, not
bit-timing; dropping 250k->125k changed nothing).

Root cause: RX was the polled 6-deep hardware FIFO (enableFIFO, no interrupt),
and FlexCAN_T4 read() services the FIFO only ~50% of calls (a FIFO/mailbox
fairness random() — the other half fall through to readMB and return 0). That
halved the drain rate, overflowed the tiny FIFO under normal loop jitter, and
dropped frames that the controller had already ACKed (hence Steervo TEC stayed
0). The 64-deep RX_SIZE_64 software ring was never used.

Fix: interrupt-driven RX. enableFIFOInterrupt() + onReceive(onCanFrame); the ISR
drains the hardware FIFO into the software ring on arrival, and events() (in the
loop) dispatches to onCanFrame() in loop context (g_steerLink stays
single-threaded). CANTEST reworked to count loopback frames through the same
ring path.

Verified on the bench (both firmwares reflashed at 250k): STEER_STATUS 50.6/s,
zero-delivery windows 0 (was 11/70 s), dashboard steerLink transitions 0 (was
18/25 s), 0% link-down (was 5.5%). CANTEST 20/20 PASS; 64/64 host tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: Ballistyxx

firmware/common/kart_common/kart_can.h

@@ -22,8 +22,15 @@ namespace kart {
 // selects its bit timing with a compile-time TWAI_TIMING_CONFIG_* switch — the
 // macro keeps that selection and the Teensy FlexCAN setBaudRate() in lockstep.
 // BOTH nodes must agree. Supported values (have a TWAI mapping in steervo):
-// 1000000, 500000, 250000, 125000. Drop to 125000 for even more margin if the
-// bench bus still logs errors at 250000.
+// 1000000, 500000, 250000, 125000.
+//
+// NB: an earlier ~4-6% STEER_STATUS loss (periodic ~100-300 ms gaps that flapped
+// STEER_LINK_OK) was NOT bitrate/bit-timing — dropping to 125000 changed nothing,
+// and CAN error counters on both nodes stayed clean through the gaps. Root cause
+// was Teensy-side: polled FlexCAN_T4 read() services the RX FIFO only ~50% of
+// calls, overflowing the 6-deep hardware FIFO and dropping (still-ACKed) frames.
+// Fixed by making RX interrupt-driven (enableFIFOInterrupt + onReceive; see
+// kart-core main.cpp). Bitrate stays at the intended 250000.
 #define KART_CAN_BITRATE 250000
 constexpr uint32_t kCanBitrate = KART_CAN_BITRATE;  // Hz; both nodes must match
 

firmware/kart-core/arduino/kart_core/kart_can.h

@@ -22,8 +22,15 @@ namespace kart {
 // selects its bit timing with a compile-time TWAI_TIMING_CONFIG_* switch — the
 // macro keeps that selection and the Teensy FlexCAN setBaudRate() in lockstep.
 // BOTH nodes must agree. Supported values (have a TWAI mapping in steervo):
-// 1000000, 500000, 250000, 125000. Drop to 125000 for even more margin if the
-// bench bus still logs errors at 250000.
+// 1000000, 500000, 250000, 125000.
+//
+// NB: an earlier ~4-6% STEER_STATUS loss (periodic ~100-300 ms gaps that flapped
+// STEER_LINK_OK) was NOT bitrate/bit-timing — dropping to 125000 changed nothing,
+// and CAN error counters on both nodes stayed clean through the gaps. Root cause
+// was Teensy-side: polled FlexCAN_T4 read() services the RX FIFO only ~50% of
+// calls, overflowing the 6-deep hardware FIFO and dropping (still-ACKed) frames.
+// Fixed by making RX interrupt-driven (enableFIFOInterrupt + onReceive; see
+// kart-core main.cpp). Bitrate stays at the intended 250000.
 #define KART_CAN_BITRATE 250000
 constexpr uint32_t kCanBitrate = KART_CAN_BITRATE;  // Hz; both nodes must match
 

firmware/kart-core/arduino/kart_core/kart_core.ino

@@ -46,7 +46,7 @@ namespace cfg = kart::cfg;
 
 namespace {
 
-constexpr const char *kVersion = "0.4.1-contactor";
+constexpr const char *kVersion = "0.4.2-canrx";
 
 // ── Pin map (docs/SMCSKart-Mainboard/README.md). Output lines are
 // MOSFET-switched grounds: HIGH = asserted at the ESC, LOW = released. ──
@@ -407,22 +407,36 @@ void sendCanStd(uint32_t id, const uint8_t *data, uint8_t len) {
   else g_canTxFail++;
 }
 
-// Drain inbound CAN: the only frames we consume are STEER_STATUS heartbeats.
-void serviceCan(uint32_t now) {
-  CAN_message_t msg;
-  while (g_can.read(msg)) {
-    g_canRxAny++;  // any traffic at all — proves the bus + our RX path are alive
-    if (msg.flags.extended) continue;  // no extended IDs are for us
-    if (msg.id == kart::kIdSteerStatus) {
-      kart::SteerStatus st;
-      if (kart::unpack_steer_status(msg.buf, msg.len, st)) {
-        g_steerLink.on_status(st, now);
-        g_steerStatusRx++;
-      }
+// Inbound CAN is interrupt-driven: the FIFO interrupt (enableFIFOInterrupt in
+// setup) drains the 6-deep hardware FIFO into the 64-deep software ring the
+// instant a frame arrives, and onCanFrame() below is dispatched from events()
+// in loop context (never the ISR — so g_steerLink stays single-threaded).
+//
+// This replaces the old polled `while (g_can.read())`: read() randomly services
+// the FIFO only ~50% of the time (a FlexCAN_T4 FIFO/mailbox fairness hack), so
+// under a FIFO-only RX config it left the tiny hardware FIFO to overflow, ACKing
+// then silently dropping ~10% of STEER_STATUS in ~100 ms bursts.
+void onCanFrame(const CAN_message_t &msg) {
+  g_canRxAny++;  // any traffic at all — proves the bus + our RX path are alive
+  if (msg.flags.extended) return;  // no extended IDs are for us
+  if (msg.id == kart::kIdSteerStatus) {
+    kart::SteerStatus st;
+    if (kart::unpack_steer_status(msg.buf, msg.len, st)) {
+      g_steerLink.on_status(st, millis());
+      g_steerStatusRx++;
     }
   }
 }
 
+// Drain the interrupt-filled RX ring, dispatching each frame to onCanFrame().
+// Bounded so a burst can't monopolize the loop; the ring holds the rest.
+void serviceCan(uint32_t now) {
+  (void)now;
+  for (int i = 0; i < 32; i++) {
+    if ((g_can.events() >> 12) == 0) break;  // rxBuffer drained
+  }
+}
+
 // STEER_SET at 50 Hz. The setpoint streams continuously; the ENABLE bit is set
 // only when steering is explicitly armed (`STEER ON`), the wheel is present,
 // and the Steervo link is fresh + calibrated + not faulted.
@@ -763,12 +777,15 @@ void cmdSteer(Stream &out) {
 // Steervo's NO_ACK self-test + the `canrx` counter to localize a dead bus.
 void cmdCanTest(Stream &out) {
   out.println("INFO CANTEST entering FlexCAN internal loopback (transceiver NOT tested)");
+  // RX is interrupt-driven (ISR -> ring -> events() -> onCanFrame), so count the
+  // self-received loopback frames through that same path via g_canRxAny rather
+  // than polling read(). Loopback disconnects the external bus, so g_canRxAny
+  // moves only for our own frames here.
   g_can.enableLoopBack(true);
   delay(2);
-  CAN_message_t m;
-  while (g_can.read(m)) {}  // drain
+  for (int i = 0; i < 64 && (g_can.events() >> 12); i++) {}  // drain ring clean
   const int kN = 20;
-  int rx = 0;
+  uint32_t before = g_canRxAny;
   for (int i = 0; i < kN; i++) {
     CAN_message_t tx{};
     tx.id = kart::kIdSteerSet;
@@ -777,18 +794,13 @@ void cmdCanTest(Stream &out) {
     tx.buf[0] = (uint8_t)i;
     g_can.write(tx);
     uint32_t t0 = millis();
-    while ((uint32_t)(millis() - t0) < 8) {
-      if (g_can.read(m) && !m.flags.extended && m.id == kart::kIdSteerSet &&
-          m.len == 1 && m.buf[0] == (uint8_t)i) {
-        rx++;
-        break;
-      }
-    }
+    while ((uint32_t)(millis() - t0) < 8) g_can.events();  // pump ring drain
   }
+  int rx = (int)(g_canRxAny - before);
   g_can.enableLoopBack(false);
-  while (g_can.read(m)) {}  // drain any stragglers before normal RX resumes
+  for (int i = 0; i < 64 && (g_can.events() >> 12); i++) {}  // drain stragglers
   // A healthy controller loops nearly all frames back; allow a couple of misses
-  // from FIFO/poll timing without calling it a failure.
+  // from timing without calling it a failure.
   bool pass = rx >= kN - 2;
   out.print("OK CANTEST loopback tx=");
   out.print(kN);
@@ -1130,12 +1142,16 @@ void setup() {
   attachInterrupt(digitalPinToInterrupt(kPinHallPulses), hallIsr, RISING);
 
   // Steering CAN bus (CAN3, mainboard pins 30/31). KART_CAN_BITRATE per can-ids.md.
-  // FIFO (accept-all) so read() polls inbound STEER_STATUS without per-mailbox
-  // RX setup; write() still uses the TX mailboxes.
+  // Interrupt-driven RX FIFO: the ISR drains the 6-deep hardware FIFO into the
+  // 64-deep software ring (RX_SIZE_64) on arrival, and onCanFrame() is dispatched
+  // from events() in loop(). Polled read() (with its ~50% FIFO/mailbox random
+  // skip) overflowed the tiny hardware FIFO and silently dropped frames.
   g_can.begin();
   g_can.setBaudRate(kart::kCanBitrate);
   g_can.setMaxMB(16);
   g_can.enableFIFO();
+  g_can.enableFIFOInterrupt();
+  g_can.onReceive(onCanFrame);
 
 #if KART_TRACTION_ONLY_BENCH
   g_dsm.set_traction_only_bench(true);

firmware/kart-core/src/main.cpp

@@ -46,7 +46,7 @@ namespace cfg = kart::cfg;
 
 namespace {
 
-constexpr const char *kVersion = "0.4.1-contactor";
+constexpr const char *kVersion = "0.4.2-canrx";
 
 // ── Pin map (docs/SMCSKart-Mainboard/README.md). Output lines are
 // MOSFET-switched grounds: HIGH = asserted at the ESC, LOW = released. ──
@@ -407,22 +407,36 @@ void sendCanStd(uint32_t id, const uint8_t *data, uint8_t len) {
   else g_canTxFail++;
 }
 
-// Drain inbound CAN: the only frames we consume are STEER_STATUS heartbeats.
-void serviceCan(uint32_t now) {
-  CAN_message_t msg;
-  while (g_can.read(msg)) {
-    g_canRxAny++;  // any traffic at all — proves the bus + our RX path are alive
-    if (msg.flags.extended) continue;  // no extended IDs are for us
-    if (msg.id == kart::kIdSteerStatus) {
-      kart::SteerStatus st;
-      if (kart::unpack_steer_status(msg.buf, msg.len, st)) {
-        g_steerLink.on_status(st, now);
-        g_steerStatusRx++;
-      }
+// Inbound CAN is interrupt-driven: the FIFO interrupt (enableFIFOInterrupt in
+// setup) drains the 6-deep hardware FIFO into the 64-deep software ring the
+// instant a frame arrives, and onCanFrame() below is dispatched from events()
+// in loop context (never the ISR — so g_steerLink stays single-threaded).
+//
+// This replaces the old polled `while (g_can.read())`: read() randomly services
+// the FIFO only ~50% of the time (a FlexCAN_T4 FIFO/mailbox fairness hack), so
+// under a FIFO-only RX config it left the tiny hardware FIFO to overflow, ACKing
+// then silently dropping ~10% of STEER_STATUS in ~100 ms bursts.
+void onCanFrame(const CAN_message_t &msg) {
+  g_canRxAny++;  // any traffic at all — proves the bus + our RX path are alive
+  if (msg.flags.extended) return;  // no extended IDs are for us
+  if (msg.id == kart::kIdSteerStatus) {
+    kart::SteerStatus st;
+    if (kart::unpack_steer_status(msg.buf, msg.len, st)) {
+      g_steerLink.on_status(st, millis());
+      g_steerStatusRx++;
     }
   }
 }
 
+// Drain the interrupt-filled RX ring, dispatching each frame to onCanFrame().
+// Bounded so a burst can't monopolize the loop; the ring holds the rest.
+void serviceCan(uint32_t now) {
+  (void)now;
+  for (int i = 0; i < 32; i++) {
+    if ((g_can.events() >> 12) == 0) break;  // rxBuffer drained
+  }
+}
+
 // STEER_SET at 50 Hz. The setpoint streams continuously; the ENABLE bit is set
 // only when steering is explicitly armed (`STEER ON`), the wheel is present,
 // and the Steervo link is fresh + calibrated + not faulted.
@@ -763,12 +777,15 @@ void cmdSteer(Stream &out) {
 // Steervo's NO_ACK self-test + the `canrx` counter to localize a dead bus.
 void cmdCanTest(Stream &out) {
   out.println("INFO CANTEST entering FlexCAN internal loopback (transceiver NOT tested)");
+  // RX is interrupt-driven (ISR -> ring -> events() -> onCanFrame), so count the
+  // self-received loopback frames through that same path via g_canRxAny rather
+  // than polling read(). Loopback disconnects the external bus, so g_canRxAny
+  // moves only for our own frames here.
   g_can.enableLoopBack(true);
   delay(2);
-  CAN_message_t m;
-  while (g_can.read(m)) {}  // drain
+  for (int i = 0; i < 64 && (g_can.events() >> 12); i++) {}  // drain ring clean
   const int kN = 20;
-  int rx = 0;
+  uint32_t before = g_canRxAny;
   for (int i = 0; i < kN; i++) {
     CAN_message_t tx{};
     tx.id = kart::kIdSteerSet;
@@ -777,18 +794,13 @@ void cmdCanTest(Stream &out) {
     tx.buf[0] = (uint8_t)i;
     g_can.write(tx);
     uint32_t t0 = millis();
-    while ((uint32_t)(millis() - t0) < 8) {
-      if (g_can.read(m) && !m.flags.extended && m.id == kart::kIdSteerSet &&
-          m.len == 1 && m.buf[0] == (uint8_t)i) {
-        rx++;
-        break;
-      }
-    }
+    while ((uint32_t)(millis() - t0) < 8) g_can.events();  // pump ring drain
   }
+  int rx = (int)(g_canRxAny - before);
   g_can.enableLoopBack(false);
-  while (g_can.read(m)) {}  // drain any stragglers before normal RX resumes
+  for (int i = 0; i < 64 && (g_can.events() >> 12); i++) {}  // drain stragglers
   // A healthy controller loops nearly all frames back; allow a couple of misses
-  // from FIFO/poll timing without calling it a failure.
+  // from timing without calling it a failure.
   bool pass = rx >= kN - 2;
   out.print("OK CANTEST loopback tx=");
   out.print(kN);
@@ -1130,12 +1142,16 @@ void setup() {
   attachInterrupt(digitalPinToInterrupt(kPinHallPulses), hallIsr, RISING);
 
   // Steering CAN bus (CAN3, mainboard pins 30/31). KART_CAN_BITRATE per can-ids.md.
-  // FIFO (accept-all) so read() polls inbound STEER_STATUS without per-mailbox
-  // RX setup; write() still uses the TX mailboxes.
+  // Interrupt-driven RX FIFO: the ISR drains the 6-deep hardware FIFO into the
+  // 64-deep software ring (RX_SIZE_64) on arrival, and onCanFrame() is dispatched
+  // from events() in loop(). Polled read() (with its ~50% FIFO/mailbox random
+  // skip) overflowed the tiny hardware FIFO and silently dropped frames.
   g_can.begin();
   g_can.setBaudRate(kart::kCanBitrate);
   g_can.setMaxMB(16);
   g_can.enableFIFO();
+  g_can.enableFIFOInterrupt();
+  g_can.onReceive(onCanFrame);
 
 #if KART_TRACTION_ONLY_BENCH
   g_dsm.set_traction_only_bench(true);