Tmc2240 fixes

[ellensp]

Description

_HIT_STATE in Marlin/src/inc/SanityCheck.h doesn't work due to missing brackets. Updated

TMCStepper points to MarlinFirmware/TMCStepper/archive/marlin-2.1.3.x.zip but if someone adds more commits
platformio doesn't notice and continues to use its cached older copy. Updated to point to a particular commit

Add missing Function Declarations to Marlin/src/feature/tmc_util.h
bool tmc_enable_stallguard(TMC2240Stepper &st);
void tmc_disable_stallguard(TMC2240Stepper &st, const bool restore_stealth);

There are other issues with tmc2240 and sensorless. See Comments.
undefined reference to `TMC2240Stepper::sgt()'

Requirements

TMC2240

Benefits

_HIT_STATE test works.
Correct version of TMCStepper library is used.
tmc_enable_stallguard and tmc_disable_stallguard are declared.

Needs

MarlinFirmware/TMCStepper#6 merged and the TMCStepper library updated to point to the updated merged library to fix sgt() issue

[ellensp]

enabling HYBRID_THRESHOLD seems to break new tmc2240 code
As found in CI test [Test FYSETC_F6] Mixed TMC | Sensorless | RRDFGSC | Games...

Marlin/src/module/stepper/../../feature/tmc_util.h:321:62: error: 'TPWMTHRS' is not a member of 'TMC2240Stepper'
         return _tmc_thrs(this->microsteps(), TMC2240Stepper::TPWMTHRS(), planner.settings.axis_steps_per_mm[AXIS_ID]);
 

[dbuezas]

I think you picked the last commit from master instead of marlin.bugfix

[ellensp]

To fix the missing TMC2240Stepper::sgt()
need a library update, temporarily can use my fork.
HAS_TRINAMIC_CONFIG = TMCStepper=https://github.com/ellensp/TMCStepper/archive/refs/heads/add-missing-sgt.zip

there is a PR waiting to be added to marlin TMCStepper lib MarlinFirmware/TMCStepper#6

I have no way to test this beyond building the code.

[thisiskeithb]

Now might be a good time to add a reference 2240s in the various config headings/sections:

Marlin/Marlin/Configuration_adv.h

Line 3000 in db137df

* To use TMC2130, TMC2160, TMC2660, TMC5130, TMC5160 stepper drivers in SPI mode:

Marlin/Marlin/Configuration_adv.h

Line 3261 in db137df

* Override default SPI pins for TMC2130, TMC2160, TMC2660, TMC5130 and TMC5160 drivers here.

Marlin/Marlin/Configuration_adv.h

Line 3288 in db137df

* Software option for SPI driven drivers (TMC2130, TMC2160, TMC2660, TMC5130 and TMC5160).

Marlin/Marlin/Configuration_adv.h

Line 3473 in db137df

* SPI_ENDSTOPS *** TMC2130/TMC5160 Only ***

Marlin/Marlin/Configuration_adv.h

Line 3501 in db137df

//#define SPI_ENDSTOPS // TMC2130/TMC5160 only

...and of course, catch up the configs repo

---

I have no way to test this beyond building the code.

I plan to do some testing today. Thanks again for looking into this!

[thisiskeithb]

Testing was done on a BIQU BX with 2240s installed on X, Y, Z1, Z2, and E0 with sensorless homing.

Summary

I can move each axis around, but sensorless homing does not work. There are also still compile issues when enabling TMC_DEBUG.

Details

I tried a standard G28 and there's an slight attempt to home X & Y, but they instantly trigger (or so I thought) and I can no longer command X or Y to move until a power cycle.

I tried enabling TMC_DEBUG to collect more info, but it won't compile with this PR and latest TMCStepper version (even after purging my PIO cache when the tag was moved).

I had to disable the following in tmc_util.cpp to get TMC_DEBUG to compile:

Click me for the diff!

diff --git a/Marlin/src/feature/tmc_util.cpp b/Marlin/src/feature/tmc_util.cpp
index fb17e56..e7f0414 100644
--- a/Marlin/src/feature/tmc_util.cpp
+++ b/Marlin/src/feature/tmc_util.cpp
@@ -715,13 +715,13 @@
   #endif
 
   template <typename TMC>
-  void print_tstep(TMC &st) {
-    const uint32_t tstep_value = st.TSTEP();
-    if (tstep_value != 0xFFFFF)
-      SERIAL_ECHO(tstep_value);
-    else
-      SERIAL_ECHOPGM("max");
-  }
+  // void print_tstep(TMC &st) {
+  //   const uint32_t tstep_value = st.TSTEP();
+  //   if (tstep_value != 0xFFFFF)
+  //     SERIAL_ECHO(tstep_value);
+  //   else
+  //     SERIAL_ECHOPGM("max");
+  // }
   void print_tstep(TMC2660Stepper &st) { }
 
   template <typename TMC>
@@ -749,7 +749,7 @@
       case TMC_CS_ACTUAL: print_cs_actual(st); break;
       case TMC_VSENSE: print_vsense(st); break;
       case TMC_MICROSTEPS: SERIAL_ECHO(st.microsteps()); break;
-      case TMC_TSTEP: print_tstep(st); break;
+      //case TMC_TSTEP: print_tstep(st); break;
       #if ENABLED(HYBRID_THRESHOLD)
         case TMC_TPWMTHRS: SERIAL_ECHO(uint32_t(st.TPWMTHRS())); break;
         case TMC_TPWMTHRS_MMS: {
@@ -1006,7 +1006,7 @@
         PRINT_TMC_REGISTER(GSTAT);
         PRINT_TMC_REGISTER(IOIN);
         PRINT_TMC_REGISTER(TPOWERDOWN);
-        PRINT_TMC_REGISTER(TSTEP);
+        //PRINT_TMC_REGISTER(TSTEP);
         PRINT_TMC_REGISTER(TPWMTHRS);
         PRINT_TMC_REGISTER(CHOPCONF);
         PRINT_TMC_REGISTER(PWMCONF);

With TMC_DEBUG enabled, I found that the following differences between M122 reports before & after homing:

Before homing:

        X    Y    Z    Z2    E
Enabled        false    false    true    true    false
[...snip...]
Driver registers:
        X    0x81:00:40:00
        Y    0x81:00:40:00
        Z    0x81:0F:40:00
        Z2    0x81:0F:40:00
        E    0x81:00:40:00

After homing:

        X    Y    Z    Z2    E
Enabled        true    true    true    true    false
[...snip...]
Driver registers:
        X    0x80:0F:50:22
        Y    0x80:0F:50:24
        Z    0x80:1F:40:7C
        Z2    0x80:1F:40:10
        E    0x81:00:40:00

@dbuezas found that according to the datasheet, this is a driver shutdown due to overheating. That is not really the case since drivers are cool to the touch and they are triggering instantly while homing, even at the least sensitive StallGuard value.

Further Testing

Based on reports in the #trinamic channel on our Discord, you can use 5130 as a substitute for 2240, but I didn't have 100% success with that either. Drivers no longer shutdown, but they do not trigger while homing, even at the most sensitive StallGuard setting (opposite of above since hit state has to be flipped per the sanity checks).

There's also a report of a printer rebooting with this PR, but I have not experienced that issue.

Another interesting find is that 2240s are a lot quieter when configured as 5130s vs. 2240s.

[dbuezas]

Another interesting find is that 2240s are a lot quieter when configured as 5130s vs. 2240s.

That's most likely because the current setting is different and so they will be running at much lower current than configured when set as another chip

[thisiskeithb]

That's most likely because the current setting is different and so they will be running at much lower current than configured when set as another chip

Ahh. Noise level is on par with 2209s/2226s when configured as 5130s.

[dbuezas]

@dbuezas found that according to the datasheet, this is a driver shutdown due to overheating

Note I didn't double check, I may have read the bits backwards

[ellensp]

Can we push this through, and add any additional fixes later in a separate PR. Currently bugfix users, who already have TMCStepper cached, cannot build latest bugfix. (if they have any drivers that use TMCStepper library)
Until they update TMCStepper library in features.ini or clear .platformio/.cache