Improved canbus diagnostics (#6784) (#568)

* stm32: Add support for reporting canbus state from can.c Signed-off-by: Kevin O'Connor <kevin@koconnor.net> * stm32: Add support for reporting canbus state from fdcan.c Signed-off-by: Kevin O'Connor <kevin@koconnor.net> * rp2040: Add support for reporting canbus state Signed-off-by: Kevin O'Connor <kevin@koconnor.net> * atsamd: Add support for reporting canbus state Signed-off-by: Kevin O'Connor <kevin@koconnor.net> * atsam: Add support for reporting canbus state Signed-off-by: Kevin O'Connor <kevin@koconnor.net> * canbus_stats: Periodically report canbus interface statistics Add support for a new get_canbus_status command to canserial.c . Add new canbus_stats.py module that will periodically query canbus mcus for connection status information. Signed-off-by: Kevin O'Connor <kevin@koconnor.net> * usb_canbus: Detect canbus stalls when in usb to canbus bridge mode If the low-level canbus stops working then it could become impossible to send messages to and from the canbus bridge node itself. This can make it difficult to diagnose canbus problems. Change the canbus bridge code to detect if message transmits become stalled for 50+ milliseconds and go into a "discarding" state. In this discarding state, messages destined for the canbus will be discarded until the canbus becomes active again. In this discarding state it will therefore be possible to transmit messages to and from the canbus bridge node. Signed-off-by: Kevin O'Connor <kevin@koconnor.net> --------- Signed-off-by: Kevin O'Connor <kevin@koconnor.net> Co-authored-by: Kevin O'Connor <kevin@koconnor.net>
KalicoCrew · Feb 7, 2025 · b107647 · b107647
1 parent 38052fc
commit b107647
Show file tree

Hide file tree

Showing 11 changed files with 426 additions and 13 deletions.
diff --git a/docs/Status_Reference.md b/docs/Status_Reference.md
@@ -48,6 +48,27 @@ The following information is available in
 - `printer["belay <config_name>"].enabled`: Returns True if the belay is
   currently enabled.
 
+## canbus_stats
+
+The following information is available in the `canbus_stats
+some_mcu_name` object (this object is automatically available if an
+mcu is configured to use canbus):
+- `rx_error`: The number of receive errors detected by the
+  micro-controller canbus hardware.
+- `tx_error`: The number of transmit errors detected by the
+  micro-controller canbus hardware.
+- `tx_retries`: The number of transmit attempts that were retried due
+  to bus contention or errors.
+- `bus_state`: The status of the interface (typically "active" for a
+  bus in normal operation, "warn" for a bus with recent errors,
+  "passive" for a bus that will no longer transmit canbus error
+  frames, or "off" for a bus that will no longer transmit or receive
+  messages).
+
+Note that only the rp2XXX micro-controllers report a non-zero
+`tx_retries` field and the rp2XXX micro-controllers always report
+`tx_error` as zero and `bus_state` as "active".
+
 ## configfile
 
 The following information is available in the `configfile` object

diff --git a/klippy/extras/canbus_stats.py b/klippy/extras/canbus_stats.py
@@ -0,0 +1,111 @@
+# Report canbus connection status
+#
+# Copyright (C) 2025  Kevin O'Connor <kevin@koconnor.net>
+#
+# This file may be distributed under the terms of the GNU GPLv3 license.
+import logging
+
+
+class PrinterCANBusStats:
+    def __init__(self, config):
+        self.printer = config.get_printer()
+        self.reactor = self.printer.get_reactor()
+        self.name = config.get_name().split()[-1]
+        self.mcu = None
+        self.get_canbus_status_cmd = None
+        self.status = {
+            "rx_error": None,
+            "tx_error": None,
+            "tx_retries": None,
+            "bus_state": None,
+        }
+        self.printer.register_event_handler(
+            "klippy:connect", self.handle_connect
+        )
+        self.printer.register_event_handler(
+            "klippy:shutdown", self.handle_shutdown
+        )
+
+    def handle_shutdown(self):
+        status = self.status.copy()
+        if status["bus_state"] is not None:
+            # Clear bus_state on shutdown to note that the values may be stale
+            status["bus_state"] = "unknown"
+            self.status = status
+
+    def handle_connect(self):
+        # Lookup mcu
+        mcu_name = self.name
+        if mcu_name != "mcu":
+            mcu_name = "mcu " + mcu_name
+        self.mcu = self.printer.lookup_object(mcu_name)
+        # Lookup status query command
+        if self.mcu.try_lookup_command("get_canbus_status") is None:
+            return
+        self.get_canbus_status_cmd = self.mcu.lookup_query_command(
+            "get_canbus_status",
+            "canbus_status rx_error=%u tx_error=%u tx_retries=%u"
+            " canbus_bus_state=%u",
+        )
+        # Register usb_canbus_state message handling (for usb to canbus bridge)
+        self.mcu.register_response(
+            self.handle_usb_canbus_state, "usb_canbus_state"
+        )
+        # Register periodic query timer
+        self.reactor.register_timer(self.query_event, self.reactor.NOW)
+
+    def handle_usb_canbus_state(self, params):
+        discard = params["discard"]
+        if discard:
+            logging.warning(
+                "USB CANBUS bridge '%s' is discarding!" % (self.name,)
+            )
+        else:
+            logging.warning(
+                "USB CANBUS bridge '%s' is no longer discarding." % (self.name,)
+            )
+
+    def query_event(self, eventtime):
+        prev_rx = self.status["rx_error"]
+        prev_tx = self.status["tx_error"]
+        prev_retries = self.status["tx_retries"]
+        if prev_rx is None:
+            prev_rx = prev_tx = prev_retries = 0
+        params = self.get_canbus_status_cmd.send()
+        rx = prev_rx + ((params["rx_error"] - prev_rx) & 0xFFFFFFFF)
+        tx = prev_tx + ((params["tx_error"] - prev_tx) & 0xFFFFFFFF)
+        retries = prev_retries + (
+            (params["tx_retries"] - prev_retries) & 0xFFFFFFFF
+        )
+        state = params["canbus_bus_state"]
+        self.status = {
+            "rx_error": rx,
+            "tx_error": tx,
+            "tx_retries": retries,
+            "bus_state": state,
+        }
+        return self.reactor.monotonic() + 1.0
+
+    def stats(self, eventtime):
+        status = self.status
+        if status["rx_error"] is None:
+            return (False, "")
+        return (
+            False,
+            "canstat_%s: bus_state=%s rx_error=%d"
+            " tx_error=%d tx_retries=%d"
+            % (
+                self.name,
+                status["bus_state"],
+                status["rx_error"],
+                status["tx_error"],
+                status["tx_retries"],
+            ),
+        )
+
+    def get_status(self, eventtime):
+        return self.status
+
+
+def load_config_prefix(config):
+    return PrinterCANBusStats(config)
diff --git a/klippy/mcu.py b/klippy/mcu.py
@@ -757,6 +757,7 @@ def __init__(self, config, clocksync):
             self._canbus_iface = config.get("canbus_interface", "can0")
             cbid = self._printer.load_object(config, "canbus_ids")
             cbid.add_uuid(config, canbus_uuid, self._canbus_iface)
+            self._printer.load_object(config, "canbus_stats %s" % (self._name,))
         else:
             self._serialport = config.get("serial")
             if not (

diff --git a/src/atsam/fdcan.c b/src/atsam/fdcan.c
@@ -1,11 +1,12 @@
 // CANbus support on atsame70 chips
 //
-// Copyright (C) 2021-2022  Kevin O'Connor <kevin@koconnor.net>
+// Copyright (C) 2021-2025  Kevin O'Connor <kevin@koconnor.net>
 // Copyright (C) 2019 Eug Krashtan <eug.krashtan@gmail.com>
 // Copyright (C) 2020 Pontus Borg <glpontus@gmail.com>
 //
 // This file may be distributed under the terms of the GNU GPLv3 license.
 
+#include "board/irq.h" // irq_save
 #include "command.h" // DECL_CONSTANT_STR
 #include "generic/armcm_boot.h" // armcm_enable_irq
 #include "generic/canbus.h" // canbus_notify_tx
@@ -147,6 +148,38 @@ canhw_set_filter(uint32_t id)
     CANx->MCAN_CCCR &= ~MCAN_CCCR_INIT;
 }
 
+static struct {
+    uint32_t rx_error, tx_error;
+} CAN_Errors;
+
+// Report interface status
+void
+canhw_get_status(struct canbus_status *status)
+{
+    irqstatus_t flag = irq_save();
+    uint32_t psr = CANx->MCAN_PSR, lec = psr & MCAN_PSR_LEC_Msk;
+    if (lec && lec != 7) {
+        // Reading PSR clears it - so update state here
+        if (lec >= 3 && lec <= 5)
+            CAN_Errors.tx_error += 1;
+        else
+            CAN_Errors.rx_error += 1;
+    }
+    uint32_t rx_error = CAN_Errors.rx_error, tx_error = CAN_Errors.tx_error;
+    irq_restore(flag);
+
+    status->rx_error = rx_error;
+    status->tx_error = tx_error;
+    if (psr & MCAN_PSR_BO)
+        status->bus_state = CANBUS_STATE_OFF;
+    else if (psr & MCAN_PSR_EP)
+        status->bus_state = CANBUS_STATE_PASSIVE;
+    else if (psr & MCAN_PSR_EW)
+        status->bus_state = CANBUS_STATE_WARN;
+    else
+        status->bus_state = 0;
+}
+
 // This function handles CAN global interrupts
 void
 CAN_IRQHandler(void)
@@ -183,6 +216,18 @@ CAN_IRQHandler(void)
         CANx->MCAN_IR = FDCAN_IE_TC;
         canbus_notify_tx();
     }
+    if (ir & (MCAN_IR_PED | MCAN_IR_PEA)) {
+        // Bus error
+        uint32_t psr = CANx->MCAN_PSR;
+        CANx->MCAN_IR = MCAN_IR_PED | MCAN_IR_PEA;
+        uint32_t lec = psr & MCAN_PSR_LEC_Msk;
+        if (lec && lec != 7) {
+            if (lec >= 3 && lec <= 5)
+                CAN_Errors.tx_error += 1;
+            else
+                CAN_Errors.rx_error += 1;
+        }
+    }
 }
 
 static inline const uint32_t
@@ -302,6 +347,6 @@ can_init(void)
     /*##-3- Configure Interrupts #################################*/
     armcm_enable_irq(CAN_IRQHandler, CANx_IRQn, 1);
     CANx->MCAN_ILE = MCAN_ILE_EINT0;
-    CANx->MCAN_IE = MCAN_IE_RF0NE | FDCAN_IE_TC;
+    CANx->MCAN_IE = MCAN_IE_RF0NE | FDCAN_IE_TC | MCAN_IE_PEDE | MCAN_IE_PEAE;
 }
 DECL_INIT(can_init);
diff --git a/src/atsamd/fdcan.c b/src/atsamd/fdcan.c
@@ -1,11 +1,12 @@
 // CANbus support on atsame51 chips
 //
-// Copyright (C) 2021-2022  Kevin O'Connor <kevin@koconnor.net>
+// Copyright (C) 2021-2025  Kevin O'Connor <kevin@koconnor.net>
 // Copyright (C) 2019 Eug Krashtan <eug.krashtan@gmail.com>
 // Copyright (C) 2020 Pontus Borg <glpontus@gmail.com>
 //
 // This file may be distributed under the terms of the GNU GPLv3 license.
 
+#include "board/irq.h" // irq_save
 #include "command.h" // DECL_CONSTANT_STR
 #include "generic/armcm_boot.h" // armcm_enable_irq
 #include "generic/canbus.h" // canbus_notify_tx
@@ -163,6 +164,38 @@ canhw_set_filter(uint32_t id)
     CANx->CCCR.reg &= ~CAN_CCCR_INIT;
 }
 
+static struct {
+    uint32_t rx_error, tx_error;
+} CAN_Errors;
+
+// Report interface status
+void
+canhw_get_status(struct canbus_status *status)
+{
+    irqstatus_t flag = irq_save();
+    uint32_t psr = CANx->PSR.reg, lec = psr & CAN_PSR_LEC_Msk;
+    if (lec && lec != 7) {
+        // Reading PSR clears it - so update state here
+        if (lec >= 3 && lec <= 5)
+            CAN_Errors.tx_error += 1;
+        else
+            CAN_Errors.rx_error += 1;
+    }
+    uint32_t rx_error = CAN_Errors.rx_error, tx_error = CAN_Errors.tx_error;
+    irq_restore(flag);
+
+    status->rx_error = rx_error;
+    status->tx_error = tx_error;
+    if (psr & CAN_PSR_BO)
+        status->bus_state = CANBUS_STATE_OFF;
+    else if (psr & CAN_PSR_EP)
+        status->bus_state = CANBUS_STATE_PASSIVE;
+    else if (psr & CAN_PSR_EW)
+        status->bus_state = CANBUS_STATE_WARN;
+    else
+        status->bus_state = 0;
+}
+
 // This function handles CAN global interrupts
 void
 CAN_IRQHandler(void)
@@ -199,6 +232,18 @@ CAN_IRQHandler(void)
         CANx->IR.reg = FDCAN_IE_TC;
         canbus_notify_tx();
     }
+    if (ir & (CAN_IR_PED | CAN_IR_PEA)) {
+        // Bus error
+        uint32_t psr = CANx->PSR.reg;
+        CANx->IR.reg = CAN_IR_PED | CAN_IR_PEA;
+        uint32_t lec = psr & CAN_PSR_LEC_Msk;
+        if (lec && lec != 7) {
+            if (lec >= 3 && lec <= 5)
+                CAN_Errors.tx_error += 1;
+            else
+                CAN_Errors.rx_error += 1;
+        }
+    }
 }
 
 static inline const uint32_t
@@ -309,6 +354,6 @@ can_init(void)
     /*##-3- Configure Interrupts #################################*/
     armcm_enable_irq(CAN_IRQHandler, CANx_IRQn, 1);
     CANx->ILE.reg = CAN_ILE_EINT0;
-    CANx->IE.reg = CAN_IE_RF0NE | FDCAN_IE_TC;
+    CANx->IE.reg = CAN_IE_RF0NE | FDCAN_IE_TC | CAN_IE_PEDE | CAN_IE_PEAE;
 }
 DECL_INIT(can_init);
diff --git a/src/generic/canbus.h b/src/generic/canbus.h
@@ -17,9 +17,20 @@ struct canbus_msg {
 
 #define CANMSG_DATA_LEN(msg) ((msg)->dlc > 8 ? 8 : (msg)->dlc)
 
+struct canbus_status {
+    uint32_t rx_error, tx_error, tx_retries;
+    uint32_t bus_state;
+};
+
+enum {
+    CANBUS_STATE_ACTIVE, CANBUS_STATE_WARN, CANBUS_STATE_PASSIVE,
+    CANBUS_STATE_OFF,
+};
+
 // callbacks provided by board specific code
 int canhw_send(struct canbus_msg *msg);
 void canhw_set_filter(uint32_t id);
+void canhw_get_status(struct canbus_status *status);
 
 // canbus.c
 int canbus_send(struct canbus_msg *msg);

diff --git a/src/generic/canserial.c b/src/generic/canserial.c
@@ -2,7 +2,7 @@
 //
 // Copyright (C) 2019 Eug Krashtan <eug.krashtan@gmail.com>
 // Copyright (C) 2020 Pontus Borg <glpontus@gmail.com>
-// Copyright (C) 2021  Kevin O'Connor <kevin@koconnor.net>
+// Copyright (C) 2021-2025  Kevin O'Connor <kevin@koconnor.net>
 //
 // This file may be distributed under the terms of the GNU GPLv3 license.
 
@@ -336,6 +336,25 @@ DECL_TASK(canserial_rx_task);
  * Setup and shutdown
  ****************************************************************/
 
+DECL_ENUMERATION("canbus_bus_state", "active", CANBUS_STATE_ACTIVE);
+DECL_ENUMERATION("canbus_bus_state", "warn", CANBUS_STATE_WARN);
+DECL_ENUMERATION("canbus_bus_state", "passive", CANBUS_STATE_PASSIVE);
+DECL_ENUMERATION("canbus_bus_state", "off", CANBUS_STATE_OFF);
+
+void
+command_get_canbus_status(uint32_t *args)
+{
+    struct canbus_status status;
+    memset(&status, 0, sizeof(status));
+    canhw_get_status(&status);
+    sendf("canbus_status rx_error=%u tx_error=%u tx_retries=%u"
+          " canbus_bus_state=%u"
+          , status.rx_error, status.tx_error, status.tx_retries
+          , status.bus_state);
+}
+DECL_COMMAND_FLAGS(command_get_canbus_status, HF_IN_SHUTDOWN
+                   , "get_canbus_status");
+
 void
 command_get_canbus_id(uint32_t *args)
 {