Marlin: [2.0.x] stepper_indirection.h includes "TMCStepper.h", but only "TMC2130Stepper.h" exists.

[marcio-ao]

After pulling the new Marlin, and updating the TMC2130Stepper library to 2.5.0, I get the following error:

  CXX   src/Marlin.cpp
In file included from src/module/stepper.h:224:0,
                 from src/Marlin.cpp:36:
src/module/stepper_indirection.h:61:26: fatal error: TMCStepper.h: No such file or directory
   #include <TMCStepper.h>
                          ^
compilation terminated.
Makefile:832: recipe for target 'applet/src/Marlin.o' failed
make: *** [applet/src/Marlin.o] Error 1

Does this include need to be changed?

[marcio-ao]

Okay, so I decided to try the TMCStepper library 0.2.0 instead, since it seems to have more recent commits. This got me further, but now I get this error:

src/feature/tmc_util.cpp: In function 'void tmc_set_report_status(bool)':
src/feature/tmc_util.cpp:503:10: error: 'report_tmc_status' was not declared in this scope
     if ((report_tmc_status = status))

UPDATE: This only happens when TMC_DEBUG is enabled.

[marcio-ao]

It looks like the new library has changed significantly and there is no documentation. I'll try muddling through it, but if I could get any pointers on converting the following macros into the new library, it would be helpful:

    #define LULZBOT_ENABLE_COOLSTEP_WITH_STALLGUARD(st) \
        /* Disable steathchop */ \
        st.stealthChop(0); \
        /* Enable coolstep for all velocities */ \
        st.coolstep_min_speed(LULZBOT_STALLGUARD_VELOCITY_THRESHOLD); \
        st.sg_min(1); \
        st.sg_max(3);

    #define LULZBOT_ENABLE_STEALTHCHOP(st) \
        /* Enable steathchop */ \
        st.stealthChop(1); \
        /* Disable coolstep */ \
        st.coolstep_min_speed(0); \
        st.sg_min(0); \
        st.sg_max(0);

        #define LULZBOT_TMC_LOW_HEAT(st) \
            st.toff(  1); /* TOFF   = [1..15]  */ \
            st.hstrt( 4); /* HSTART = [0..7]   */ \
            st.hend(  0); /* HEND   = [0..15]  */ \
            st.tbl(   1); /* TBL    = [0..3]   */

        #define LULZBOT_TMC_LOW_NOISE(st) \
            st.toff(  1); /* TOFF   = [1..15]  */ \
            st.hstrt( 0); /* HSTART = [0..7]   */ \
            st.hend(  0); /* HEND   = [0..15]  */ \
            st.tbl(   1); /* TBL    = [0..3]   */

    #define LULZBOT_TMC_INIT(st) \
        /* The EinsyRambo connects both diag pins to the same */ \
        /* microcontroller pin and provides a pull up resistor, */ \
        /* so configure the pin as active low. */ \
        st.diag0_active_high(0); \
        st.diag1_active_high(0); \
        st.shaft_dir(LULZBOT_SHAFT_DIR); \
        st.external_ref(0);     /* I_scale_analog  = 0 */ \
        st.internal_sense_R(0); /* internal_Rsense = 0 */ \

[marcio-ao]

Okay, one more error, now when trying to compile for another printer model:

In file included from ../ArduinoAddons/arduino-1.8.5/libraries/TMCStepper/src/source/CHOPCONF.cpp:1:0:
../ArduinoAddons/arduino-1.8.5/libraries/TMCStepper/src/TMCStepper.h:14:29: fatal error: SoftwareSerial.h: No such file or directory
  #include <SoftwareSerial.h>

UPDATE: This one was a Makefile issue. I'll probably make a new PR with a Makefile soon, as I've done a lot of work like adding support for 32-bit Archim.

[teemuatlut]

We're moving to a new TMC library that allows me to easily support more driver models. I don't want to create a new library for each.
I recommend using the latest commit for the library. I know there is a compile issue with the latest release tag. This is not an issue with PIO, as that pulls from a direct git link.
SoftwareSerial is a framework library and I've tried to exclude platforms that don't have the library. That _should_ include the DUE platform.

The interface between the TMC2130Stepper and TMCStepper should be mostly the same. But for one, you will not be able to use the human readable names for the register options as those are not ported over yet.

Let me know if you have anything else.

[teemuatlut]

report_tmc_status is because the variable was moved under compile condition for MONITOR_DRIVER_STATUS. I'll see about fixing this with the new PR.

[thinkyhead]

Just submitted the patch, above.

[marcio-ao]

Okay, so I've been experimenting with the default Marlin TMC settings (without my various tunings above) and pretty much everything works fairly well except for one thing. The Z axis steppers on our belt driven machines put out a loud squeal when operating in spreadCycle mode. In our previous FW, we had gotten around this by enabling stealthChop for Z, but keeping spreadCycle for XY (since stealth is more likely to cause shifted layers on XY).

This is not an option in Marlin without code modifications since STEALTHCHOP in Configuration_adv.h switches the mode an all axis. Is there any chance that enabling or disabling stealthchop could be controlled independently for Z as a configuration option in Marlin?

[teemuatlut]

@marcio-ao
See https://github.com/teemuatlut/Marlin/commit/219476b8aa8a20ad94a7cc0c42df478fdfbc0f16

[teemuatlut]

By any chance, have you measured the chopper frequency with your modified driver parameters?
The one we now have upstream is quite low and causes unwanted noise and this was my focus during the summer at one point.

[marcio-ao]

@thinkyhead: No, because tmc_stallguard() overrides whatever you set with TMC_ADV, so even if you enable spreadCycle or stealthchop as you want in it TMC_ADV, that gets overriden when sensorless homing happens.

Right now, I have to accomplish what I need by defining STEALTHCHOP in "Configuration_adv.h" and then modifying tmc_stallguard() to set the steppers to spreadCycle. This works for us, because we only do sensorless homing on XY and those are the two axis we don't want to have STEALTHCHOP enabled. But it's sort of a hack...

[thinkyhead]

@teemuatlut — Good stuff. Looking forward to the PR!

[marcio-ao]

@teemuatlut: A lot of our tuning was done "by ear". We sent a printer to Trinamic hoping they could help us tune our steppers, but they took our printer and never got back to us! :(

[thinkyhead]

that gets [overridden] when sensorless homing happens.

@teemuatlut — An error on our part? Should we be setting the state differently after sensorless homing depending on configured options?

[thinkyhead]

@teemuatlut — Should we update this comment to point to the new TMCStepper library homepage, or do the same commands still apply?

  /**
   * You can set your own advanced settings by filling in predefined functions.
   * A list of available functions can be found on the library github page
   * https://github.com/teemuatlut/TMC2130Stepper
   * https://github.com/teemuatlut/TMC2208Stepper
   *
   * Example:
   * #define TMC_ADV() { \
   *   stepperX.diag0_temp_prewarn(1); \
   *   stepperY.interpolate(0); \
   * }
   */

[marcio-ao]

@thinkyhead: I think the commit @teemuatlut referenced above solves the issue. It looks like it gives the option to set STEALTHCHOP independently for each axis, and then tmc_stallguard is made smarter enough to restore it to the desired state.

@teemuatlut: Looking forward to that commit making it into the codebase. It looks like it solves the problem.

[teemuatlut]

@marcio-ao Haha :D That's great.
You can gain quite a bit in chopper frequency by raising the voltage to 24V, but I don't know if that is an option for you.
I've got another feature coming in where the user can configure the chopper parameters for better noise performance. But the values are treated as "magic numbers" and are given by an external example sketch included in the library.
The new default parameters we'll be using get us a better frequency but it's not tested much at this point.

Yes, the split stealthChop should solve the problem. The chopper modes are tracked throughout the homing process and are restored to the same state as they were when starting.

@thinkyhead Well, yes and no. The current stealthChop implementation is a global configuration option that can only be overridden by the TMC_ADV, but if a user is disabling stealthChop through those means, we don't currently take that into account. But as said, this I've already fixed and just need to push it upstream when its turn comes.
Actually no, they don't apply and sure do need updating. I also need to start writing some documentation in the library page.

[thinkyhead]

Okay, I'll hold off on updating the TMC_ADV comment until there'a a list added to the README for https://github.com/teemuatlut/TMCStepper

[marcio-ao]

You can gain quite a bit in chopper frequency by raising the voltage to 24V, but I don't know if that is an option for you.

Yes, our printers are 24V. How would I increase the chopper frequency? I managed to "solve" the Z whine by enabling stealthChop on that axis, but increasing the chopper frequency might be another possible solution.

[teemuatlut]

It's a factor of off time, blank time, hysteresis start and hysteresis end.
Blank time you can leave at setting 1.
Off time can be around 3-5. I think the goal is not to reduce this too much but still get the frequency to the desired range.
Effective hysteresis is end+start and also plays role in the resulting frequency. Lower effective setting increases the frequency. Our new defaults will be hstrt=0 and hend=1, but are made with 12V in mind. You also don't want to go too high on the frequency as that can have adverse effects as well. I think mainly increased heat output with higher frequencies.

You can see Trinamic's Application Note 001 for details that explains the tuning process better than what I know.

Here's how the chopper states look like when looked at with a scope. No idea what the parameters were when I took the picture.

Starting from the first vertical line you can first see the ON phase, then the first off phase / slow decay (determined by off time), then fast decay phase (current flowing in the opposite direction), and lastly the second slow decay before starting over again.

[marcio-ao]

Effective hysteresis is end+start and also plays role in the resulting frequency. Lower effective setting increases the frequency.

Ah, very interesting. If you look at the #define's I had posted above, I think you'll see that I stumbled upon this myself:

        #define LULZBOT_TMC_LOW_NOISE(st) \
            st.toff(  1); /* TOFF   = [1..15]  */ \
            st.hstrt( 0); /* HSTART = [0..7]   */ \
            st.hend(  0); /* HEND   = [0..15]  */ \
            st.tbl(   1); /* TBL    = [0..3]   */

Our new defaults will be hstrt=0 and hend=1, but are made with 12V in mind.

I think this explains why I am now finding the defaults are quieter, as they match up more or less what I had been using for lowest noise.

-- Marcio

[marcio-ao]

Another compilation error to report:

src/feature/tmc_util.cpp: In instantiation of ‘void tmc_status(TMC&, TMC_debug_enum, float) [with TMC = TMCMarlin<TMC2130Stepper, 'X', '0'>]’:
src/feature/tmc_util.cpp:391:64:   required from here
src/feature/tmc_util.cpp:358:60: error: ‘class TMCMarlin<TMC2130Stepper, 'X', '0'>’ has no member named ‘getOTPW’
       case TMC_OTPW_TRIGGERED: serialprintPGM(st.getOTPW() ? PSTR("true") : PSTR("false")); break;
                                                            ^
compilation terminated due to -Wfatal-errors.
Makefile:833: recipe for target 'applet/src/feature/tmc_util.o' failed
make: *** [applet/src/feature/tmc_util.o] Error 1

[teemuatlut]

Looks like another one that requires the monitoring option.

[thinkyhead]

I’ll leave the PR for that one to you, @teemuatlut

[marcio-ao]

Okay, so I enabled MONITOR_DRIVER_STATUS (because it looks very useful) and now I get this:

In file included from src/feature/tmc_util.h:28:0,
                 from src/feature/tmc_util.cpp:27:
../ArduinoAddons/arduino-1.8.5/libraries/TMCStepper/src/TMCStepper.h: In function ‘uint8_t get_status_response(TMC2130Stepper&)’:
../ArduinoAddons/arduino-1.8.5/libraries/TMCStepper/src/TMCStepper.h:334:11: error: ‘uint8_t TMC2130Stepper::status_response’ is protected
   uint8_t status_response;

[teemuatlut]

@marcio-ao

I recommend using the latest commit for the library. I know there is a compile issue with the latest release tag. This is not an issue with PIO, as that pulls from a direct git link.

@thinkyhead Sure thing. I'll likely make a new PR today anyway for the TMC2660 suppor. I can include a fix there.

[marcio-ao]

@teemuatlut: Okay, I'll give the library from your repo a try (in the past I've relied only on the released versions).

[teemuatlut]

In general that's fine. I just haven't gotten around to making the new tag for a release yet.

[marcio-ao]

@teemuatlut: Looks good with the version from the repo. No more compile errors with all the options I have enabled. Thanks for all the help!

[marcio-ao]

Closing this ticket since all issues have been resolved.

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.