FTMotion: Fix TMC2208 timing bug (attempt)

[dbuezas]

Description

TMC2208 Direction-Flip Delay

Some TMC2208 / TMC2208_STANDALONE setups may require a short delay after a DIR change
to avoid timing-sensitive glitches, especially when using stealthChop.

When enabled for an axis, FT Motion will "pause" that axis for > 750µs after a detected
direction flip by holding its trajectory coordinate constant for a multiple of FTM_TS
frames. For the default FTM_FS = 1000, it is a single 1ms frame.
Other axes keep moving normally, and the wait is cancelled if the axis flips again.

NOTE: Typically only needed if you actually use stealthChop on that axis.

Requirements

Benefits

Configurations

FTM + 2208 drivers in stealthchop

Related Issues

Hopefully fixes #28248

[yrabbit]

The situation with TMC2225 (TMC2208) has been corrected. My settings for FTM, Creality 4.2.7 board:

#define X_DRIVER_TYPE  TMC2208_STANDALONE
#define Y_DRIVER_TYPE  TMC2208_STANDALONE
#define Z_DRIVER_TYPE  TMC2208_STANDALONE
#define E0_DRIVER_TYPE TMC2208_STANDALONE

#define FTM_IS_DEFAULT_MOTION 
#define NO_STANDARD_MOTION 

#define FTM_DEFAULT_SHAPER_X      ftMotionShaper_ZVD
#define FTM_SHAPING_DEFAULT_FREQ_X   40.0f

#define FTM_DEFAULT_SHAPER_Y      ftMotionShaper_ZVD
#define FTM_SHAPING_DEFAULT_FREQ_Y   40.0f

#define FTM_DEFAULT_SHAPER_Z      ftMotionShaper_NONE
#define FTM_SHAPING_DEFAULT_FREQ_Z   21.0f

#define FTM_SHAPER_E
#define FTM_DEFAULT_SHAPER_E      ftMotionShaper_NONE
#define FTM_SHAPING_DEFAULT_FREQ_E   21.0f 

#define FTM_SMOOTHING
#define FTM_SMOOTHING_TIME_E        0.06f

#define FTM_POLYS 
#define FTM_TRAJECTORY_TYPE POLY5 

#define FTM_BUFFER_SIZE             2048
#define FTM_FS  

Before the fixes related to TMC2225 (TMC2208), the extruder would stop:

n-1.mp4

After corrections in this PR, stoppages due to TMC2225 (TMC2208) are no longer observed.

[narno2202]

@dbuezas , I missed @yrabbit success. In my mind this should be mandatory if a faulty driver is present: TMC2208 or TMC5160.

You could remove the related code in Configuration_adv.h and do if you want in drivers.h

#define _OR_ADTX(N,T)   || AXIS_DRIVER_TYPE_X##N(T)
#define HAS_X_DRIVER(T) (0 RREPEAT2(NUM_X_STEPPERS, _OR_ADTX, T))
#define _OR_ADTY(N,T)   || AXIS_DRIVER_TYPE_Y##N(T)
#define HAS_Y_DRIVER(T) (0 RREPEAT2(NUM_Y_STEPPERS, _OR_ADTY, T))
#define _OR_ADTZ(N,T)   || AXIS_DRIVER_TYPE_Z##N(T)
#define HAS_Z_DRIVER(T) (0 RREPEAT2(NUM_Z_STEPPERS, _OR_ADTZ, T))

then at the end of this file

#if ENABLED(FT_MOTION)
  #if HAS_X_DRIVER(TMC2208) || HAS_X_DRIVER(TMC2208_STANDALONE) || HAS_X_DRIVER(TMC5160) || HAS_X_DRIVER(TMC5160_STANDALONE)
      #define APPLY_TIMING_HACK_ON_X
    #endif
    #if HAS_Y_DRIVER(TMC2208) || HAS_Y_DRIVER(TMC2208_STANDALONE) || HAS_Y_DRIVER(TMC5160) || HAS_Y_DRIVER(TMC5160_STANDALONE)
      #define APPLY_TIMING_HACK_ON_Y
    #endif
    #if HAS_Z_DRIVER(TMC2208) || HAS_Z_DRIVER(TMC2208_STANDALONE) || HAS_Z_DRIVER(TMC5160) || HAS_Z_DRIVER(TMC5160_STANDALONE)
      #define APPLY_TIMING_HACK_ON_Z
    #endif
    #if HAS_E_DRIVER(TMC2208) || HAS_E_DRIVER(TMC2208_STANDALONE) || HAS_E_DRIVER(TMC5160) || HAS_E_DRIVER(TMC5160_STANDALONE)
      #define APPLY_TIMING_HACK_ON_E
    #endif
#endif // FT_MOTION

[dbuezas]

The issue only appears on stealthchop only, so better be configurable to avoid it applying on machines set to spreadcycle

[narno2202]

At least for standalone drivers (stealthchop by default and not modifiable).

[dbuezas]

Well, for non standalone too if the user set them as stealthchop, and I don't think the 5160s are affected.
Anyway this is experimental, we don't yet know if it works well

[oliof]

Well, for non standalone too if the user set them as stealthchop, and I don't think the 5160s are affected. Anyway this is experimental, we don't yet know if it works well

Most standalone drivers are set to stealthchop, you usually need to solder a pair of pads to enable spreadcycle if stepsticks. With onboard drivers, all bets are off.

[narno2202]

@thinkyhead, I tried to catch the DIR changes in ftMotion_stepper() (DIR_STEP and STEP - DIR) and add a delay of 800ns (searching gave 650ns) before and after DIR pulse but no success.. @dbuezas and @thinkyhead , what about catching the DIR changes in stepping.h, exit the do .. while loop in Stepper.isr() if necessary and change interval value to set the next stepper ISR (HAL_timer_set_compare(MF_TIMER_STEP, next_isr_ticks))?

[dbuezas]

I don't think it is a timing of the dir pin issue given all the experiments and the klipper workaround, i.e not a logic timing problem. Instead, it seems to be an oversensitive overcurrent protection when there's two contiguous steps in different directions.
IMO if it was pin timing issue it wouldn't require a monstrous wait of almost a ms.

[dbuezas]

@narno2202 , the user reported this PR to have fixed his issue, and it has no issues with lost steps or interrupting other axes already.

[thinkyhead]

two contiguous steps in different directions

So, in other words, you could do the DIR change at any point in-between the two STEP signals, but if there is too little time between the different-direction STEP signals the driver will go into STST.

Our basic motion assumes TMC2209 requires 100 ns for DIR signals to settle. And it assumes that if a STEP signal also needs time to settle, and then some additional time for the "instantaneous" step to physically complete. If the DIR change arrives in the middle of the step, either before it has settled or while the step is in-progress, presumably that will also cause STST.

Timing of signals could depend on the driver design, because theoretically in response to the STEP signal the driver could make a latched copy of the DIR state instead of using the source DIR state. It would still need to have a settled DIR signal to do that, so there's just no eliminating that 100 ns post-delay.

At the extreme, you could imagine two STEP signals in opposite directions causing a current-spike when there is some momentum in the axis, and then the detection of the spike is what causes the driver to choke.

I'm speculating a bit here. What would be most useful would be to see a healthy pulse train side-by-side with the kinds of pulse-trains that cause STST, and just make sure we adhere to all the required intervals, even when inserting babysteps into the signal train.

---

The solution here seems good to me. Direction changes are far less frequent occurrences than steps, so even though this adds some overhead on every direction change, it should not end up slowing anything significantly.

[dbuezas]

Warning, the extra changes before merging it broke the algorithm.

[dbuezas]

For the record: the parenthesis introduced here is wrong