Link top level error reporting to TMC2130 error flags

src/hal/avr/tmc2130.cpp

@@ -4,14 +4,7 @@
 namespace hal {
 namespace tmc2130 {
 
-TMC2130::TMC2130(const MotorParams &params, const MotorCurrents &currents, MotorMode mode)
+bool TMC2130::Init(const MotorParams &params, const MotorCurrents &currents, MotorMode mode) {
-    : mode(mode)
-    , currents(currents)
-    , sg_counter(0) {
-    Init(params);
-}
-
-bool TMC2130::Init(const MotorParams &params) {
     gpio::Init(params.csPin, gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::high));
     gpio::Init(params.sgPin, gpio::GPIO_InitTypeDef(gpio::Mode::input, gpio::Pull::up));
     gpio::Init(params.stepPin, gpio::GPIO_InitTypeDef(gpio::Mode::output, gpio::Level::low));
@@ -69,15 +62,11 @@ bool TMC2130::Init(const MotorParams &params) {
 }
 
 void TMC2130::SetMode(const MotorParams &params, MotorMode mode) {
-    this->mode = mode;
-
     ///0xFFF00 is used as a "Normal" mode threshold since stealthchop will be used at standstill.
     WriteRegister(params, Registers::TPWMTHRS, (mode == Stealth) ? 70 : 0xFFF00); // @todo should be configurable
 }
 
 void TMC2130::SetCurrents(const MotorParams &params, const MotorCurrents &currents) {
-    this->currents = currents;
-
     uint32_t ihold_irun = (uint32_t)(currents.iHold & 0x1F) << 0 //ihold
         | (uint32_t)(currents.iRun & 0x1F) << 8 //irun
         | (uint32_t)(15 & 0x0F) << 16; //IHOLDDELAY

src/hal/tmc2130.h

@@ -32,27 +32,24 @@ struct MotorCurrents {
     uint8_t iHold; ///< Holding current
 };
 
+struct __attribute__((packed)) ErrorFlags {
+    uint8_t reset_flag : 1; ///< driver restarted
+    uint8_t uv_cp : 1; ///< undervoltage on charge pump
+    uint8_t s2g : 1; ///< short to ground
+    uint8_t otpw : 1; ///< over temperature pre-warning
+    uint8_t ot : 1; ///< over temperature hard
+    inline ErrorFlags()
+        : reset_flag(0)
+        , uv_cp(0)
+        , s2g(0)
+        , otpw(0)
+        , ot(0) {}
+};
+
+/// TMC2130 interface - instances of this class are hidden in modules::motion::Motion::AxisData
 class TMC2130 {
-    MotorMode mode;
-    MotorCurrents currents;
-    struct __attribute__((packed)) ErrorFlags {
-        uint8_t reset_flag : 1;
-        uint8_t uv_cp : 1;
-        uint8_t s2g : 1;
-        uint8_t otpw : 1;
-        uint8_t ot : 1;
-        inline ErrorFlags()
-            : reset_flag(0)
-            , uv_cp(0)
-            , s2g(0)
-            , otpw(0)
-            , ot(0) {}
-
-    } errorFlags;
-    bool enabled = false;
-    uint8_t sg_counter;
-
 public:
+    /// TMC2130 register addresses
     enum class Registers : uint8_t {
         /// General Configuration Registers
         GCONF = 0x00,
@@ -76,28 +73,18 @@ public:
     };
 
     /// Constructor
-    TMC2130(const MotorParams &params,
+    TMC2130() = default;
-        const MotorCurrents &currents,
-        MotorMode mode);
 
     /// (re)initialization of the chip - please note this is necessary due to some HW flaws in the original MMU boards.
     /// And yes, the TMC may not get correctly initialized.
     /// @returns true if the TMC2130 was inited correctly
-    bool Init(const MotorParams &params);
+    bool Init(const MotorParams &params,
-
+        const MotorCurrents &currents,
-    /// Get the current motor mode
+        MotorMode mode);
-    MotorMode Mode() const {
-        return mode;
-    }
 
     /// Set the current motor mode
     void SetMode(const MotorParams &params, MotorMode mode);
 
-    /// Get the current motor currents
-    const MotorCurrents &Currents() const {
-        return currents;
-    }
-
     /// Set the current motor currents
     void SetCurrents(const MotorParams &params, const MotorCurrents &currents);
 
@@ -149,6 +136,10 @@ public:
 private:
     void _spi_tx_rx(const MotorParams &params, uint8_t (&pData)[5]);
     void _handle_spi_status(const MotorParams &params, uint8_t status);
+
+    ErrorFlags errorFlags;
+    bool enabled = false;
+    uint8_t sg_counter;
 };
 
 } // namespace tmc2130

src/logic/command_base.cpp

@@ -1,22 +1,80 @@
 #include "command_base.h"
 #include "../modules/idler.h"
 #include "../modules/selector.h"
+#include "../modules/motion.h"
 
 namespace mi = modules::idler;
 namespace ms = modules::selector;
+namespace mm = modules::motion;
 
 namespace logic {
 
+inline ErrorCode &operator|=(ErrorCode &a, ErrorCode b) {
+    return a = (ErrorCode)((uint16_t)a | (uint16_t)b);
+}
+
+static ErrorCode TMC2130ToErrorCode(const hal::tmc2130::TMC2130 &tmc, uint8_t tmcIndex) {
+    ErrorCode e = ErrorCode::OK;
+
+    if (tmc.GetErrorFlags().reset_flag) {
+        e |= ErrorCode::TMC_RESET;
+    }
+    if (tmc.GetErrorFlags().uv_cp) {
+        e |= ErrorCode::TMC_UNDERVOLTAGE_ON_CHARGE_PUMP;
+    }
+    if (tmc.GetErrorFlags().s2g) {
+        e |= ErrorCode::TMC_SHORT_TO_GROUND;
+    }
+    if (tmc.GetErrorFlags().otpw) {
+        e |= ErrorCode::TMC_OVER_TEMPERATURE_WARN;
+    }
+    if (tmc.GetErrorFlags().ot) {
+        e |= ErrorCode::TMC_OVER_TEMPERATURE_ERROR;
+    }
+
+    if (e != ErrorCode::OK) {
+        switch (tmcIndex) {
+        case config::Axis::Pulley:
+            e |= ErrorCode::TMC_PULLEY_BIT;
+            break;
+        case config::Axis::Selector:
+            e |= ErrorCode::TMC_SELECTOR_BIT;
+            break;
+        case config::Axis::Idler:
+            e |= ErrorCode::TMC_IDLER_BIT;
+            break;
+        default:
+            break;
+        }
+    }
+
+    return e;
+}
+
 bool CommandBase::Step() {
+    ErrorCode tmcErr = ErrorCode::OK;
     // check the global HW errors - may be we should avoid the modules layer and check for the HAL layer errors directly
     if (mi::idler.State() == mi::Idler::Failed) {
         state = ProgressCode::ERRTMCFailed;
-        error = ErrorCode::TMC_IOIN_MISMATCH;
+        tmcErr |= TMC2130ToErrorCode(mm::motion.DriverForAxis(mm::Axis::Idler), mm::Axis::Idler);
-        return true; // the HW error prevents us from continuing with the with the state machine - the MMU must be restarted/fixed before continuing
+    }
-    } else if (ms::selector.State() == ms::Selector::Failed) {
+    if (ms::selector.State() == ms::Selector::Failed) {
         state = ProgressCode::ERRTMCFailed;
-        error = ErrorCode::TMC_IOIN_MISMATCH;
+        tmcErr |= TMC2130ToErrorCode(mm::motion.DriverForAxis(mm::Axis::Selector), mm::Axis::Selector);
-        return true; // the HW error prevents us from continuing with the with the state machine - the MMU must be restarted/fixed before continuing
+    }
+    // may be we should model the Pulley as well...
+    //    if (ms::selector.State() == ms::Selector::Failed) {
+    //        state = ProgressCode::ERRTMCFailed;
+    //        error |= TMC2130ToErrorCode(mm::motion.DriverForAxis(mm::Axis::Selector), mm::Axis::Selector);
+    //        return true; // the HW error prevents us from continuing with the with the state machine - the MMU must be restarted/fixed before continuing
+    //    }
+
+    // @@TODO not sure how to prevent losing the previously accumulated error ... or do I really need to do it?
+    // May be the TMC error word just gets updated with new flags as the motion proceeds
+    // And how about the logical errors like FINDA_DIDNT_SWITCH_ON?
+    if (tmcErr != ErrorCode::OK) {
+        error |= tmcErr;
+        return true;
     }
 
     return StepInner();

src/logic/error_codes.h

@@ -56,5 +56,5 @@ enum class ErrorCode : uint_fast16_t {
     /// TMC driver over temperature error - we really shouldn't ever reach this error.
     /// It can still be recovered if the driver cools down below 120C.
     /// The driver needs to be disabled and enabled again for operation to resume after this error is cleared.
-    TMC_OVER_TEMPERATURE_ERROR = 0xC000,
+    TMC_OVER_TEMPERATURE_ERROR = 0xC000
 };

src/modules/idler.cpp

@@ -73,5 +73,10 @@ bool Idler::Step() {
     }
 }
 
+//hal::tmc2130::MotorParams Idler::TMCDriverParams() const {
+//    return
+
+//}
+
 } // namespace idler
 } // namespace modules

src/modules/idler.h

@@ -48,6 +48,8 @@ public:
     /// @returns the index of idle position of the idler, usually 5 in case of 0-4 valid indices of filament slots
     inline static constexpr uint8_t IdleSlotIndex() { return config::toolCount; }
 
+    //    hal::tmc2130::MotorParams TMCDriverParams()const;
+
 protected:
     virtual void PrepareMoveToPlannedSlot() override;
 

src/modules/motion.cpp

@@ -9,7 +9,7 @@ bool Motion::InitAxis(Axis axis) {
     // disable the axis and re-init the driver: this will clear the internal
     // StallGuard data as a result without special handling
     Disable(axis);
-    return axisData[axis].drv.Init(axisParams[axis].params);
+    return axisData[axis].drv.Init(axisParams[axis].params, axisParams[axis].currents, axisParams[axis].mode);
 }
 
 void Motion::SetEnabled(Axis axis, bool enabled) {

src/modules/motion.h

@@ -5,7 +5,6 @@
 
 namespace modules {
 
-/// @@TODO
 /// Logic of motor handling
 /// Ideally enable stepping of motors under ISR (all timers have higher priority than serial)
 namespace motion {
@@ -38,7 +37,7 @@ static constexpr MotorMode DefaultMotorMode(const config::AxisConfig &axis) {
 }
 
 /// Static axis configuration
-static constexpr AxisParams axisParams[NUM_AXIS] = {
+static AxisParams axisParams[NUM_AXIS] = {
     // Pulley
     {
         .name = 'P',
@@ -215,6 +214,11 @@ public:
     /// stop whatever moves are being done
     void AbortPlannedMoves();
 
+    /// @returns the TMC213 driver associated with the particular axis
+    inline const hal::tmc2130::TMC2130 &DriverForAxis(Axis axis) const {
+        return axisData[axis].drv;
+    }
+
 private:
     struct AxisData {
         TMC2130 drv; ///< Motor driver
@@ -226,11 +230,7 @@ private:
     /// Helper to initialize AxisData members
     static AxisData DataForAxis(Axis axis) {
         return {
-            .drv = {
+            .drv = {},
-                axisParams[axis].params,
-                axisParams[axis].currents,
-                axisParams[axis].mode,
-            },
             .ctrl = {
                 axisParams[axis].jerk,
                 axisParams[axis].accel,

tests/unit/modules/stubs/stub_tmc2130.cpp

@@ -3,17 +3,13 @@
 namespace hal {
 namespace tmc2130 {
 
-TMC2130::TMC2130(const MotorParams &params,
-    const MotorCurrents &currents,
-    MotorMode mode) {
-    // TODO
-}
-
 void TMC2130::SetMode(const MotorParams &params, MotorMode mode) {
     // TODO
 }
 
-bool TMC2130::Init(const MotorParams & /*params*/) {
+bool TMC2130::Init(const MotorParams &params,
+    const MotorCurrents &currents,
+    MotorMode mode) {
     // TODO
     return true;
 }