teemuatlut / TMCStepper

MIT License
502 stars 196 forks source link

TMC5160 push() #44

Closed kAdonis closed 5 years ago

kAdonis commented 5 years ago

I am not sure, if this is an issue with marlin or the TMCStepper library, but I post here first. In marlin when I restore power to my TMC5160 (M80) the steppers dont move anymore. In stepper_indirection.h restore_stepper_drivers() is calling push() I dont quite understand, I think the push() function is using the wrong (TMC2130) PWMCONF registers? I copied the push()function from TMC2130Stepper.cpp into TMC2160Stepper.cpp and the drivers get restored as before with library v0.3.5.

void TMC2160Stepper::push() {
  GCONF(GCONF_register.sr);
  IHOLD_IRUN(IHOLD_IRUN_register.sr);
  TPOWERDOWN(TPOWERDOWN_register.sr);
  TPWMTHRS(TPWMTHRS_register.sr);
  TCOOLTHRS(TCOOLTHRS_register.sr);
  THIGH(THIGH_register.sr);
  XDIRECT(XDIRECT_register.sr);
  VDCMIN(VDCMIN_register.sr);
  CHOPCONF(CHOPCONF_register.sr);
  COOLCONF(COOLCONF_register.sr);
  DCCTRL(DCCTRL_register.sr);
  PWMCONF(PWMCONF_register.sr);
  ENCM_CTRL(ENCM_CTRL_register.sr);
}

But I think there is probably a better solution than this. I hope I made clear what I mean, feel free to ask if not

teemuatlut commented 5 years ago

I think that's about the cleanest approach without resorting to virtual functions.

teemuatlut commented 5 years ago

https://github.com/teemuatlut/TMCStepper/releases/tag/v0.4.4

https://github.com/teemuatlut/TMCStepper/commit/f543e352bf03bdcb9149a813de0f0fcd7f5d463a

kAdonis commented 5 years ago

Thank you! I made some short tests with v0.4.4. and I need to disable //SHORT_CONF(SHORT_CONF_register.sr);in TMC5160Stepper::push() to get my steppers turning after power restore, but thats probably just because SHORT_CONF needs reasonable initializers in marlin. I will also disable DRV_CONF(DRV_CONF_register.sr);(and maybe more) until marlin is able to handle the new push function. I need to read the datasheet again :unamused:

And I think ENCM_CTRL(ENCM_CTRL_register.sr); is obsolete in TMC2160Stepper::push()

teemuatlut commented 5 years ago

Yea I think there's a unfortunately consequence that I'm now pushing registers that don't yet have correct boot time default values. Should've done things in a different order it seems.

kAdonis commented 5 years ago

I tried with this initalizers for TMC5160 in marlin (stepper_indirection.cpp) using Trinamic reset defaults

    SHORT_CONF_t shortconf{0};
    shortconf.shortdelay = 0;
    shortconf.shortfilter = 0b01;
    shortconf.s2g_level = 6;
    shortconf.s2vs_level = 6;
    st.SHORT_CONF(shortconf.sr);

    DRV_CONF_t drvconf{0};
    drvconf.filt_isense = 0b00;
    drvconf.drvstrength = 0b10;
    drvconf.otselect = 0b00;
    drvconf.bbmclks = 4;
    drvconf.bbmtime = 0; //?
    st.DRV_CONF(drvconf.sr);

and everything seems working as expected, with unchanged lib 0.4.4.

teemuatlut commented 5 years ago

https://github.com/teemuatlut/TMCStepper/releases/tag/v0.4.5

https://github.com/teemuatlut/TMCStepper/commit/c76554d7e59549d98d8957397872a408df23a321#diff-f31e3fec8025d2621cd1c254126ed86d

kAdonis commented 5 years ago

Great!, v0.4.5. is working fine for me,

Suggestion: why not the "short version"(see below)? Just for the cleaner look.:smiley: for TMC2160 and TMC5160

  SHORT_CONF_register.s2vs_level = 6;
  SHORT_CONF_register.s2g_level = 6;
  SHORT_CONF_register.shortfilter = 0b01;
  SHORT_CONF_register.shortdelay = 0; 

short version is: SHORT_CONF_register.sr = 0x00010606;

and

  DRV_CONF_register.bbmtime = 0;
  DRV_CONF_register.bbmclks = 4;
  DRV_CONF_register.otselect = 0b00;
  DRV_CONF_register.drvstrength = 0b10;
  DRV_CONF_register.filt_isense = 0b00;

short version is: DRV_CONF_register.sr = 0x00080400;

for TMC2208

GCONF_register.i_scale_analog = 1;
GCONF_register.internal_rsense = 0; // OTP
GCONF_register.en_spreadcycle = 0; // OTP
GCONF_register.multistep_filt = 1; // OTP

short version is: DRV_CONF_register.sr = 0x00000101;

And as I wrote above:ENCM_CTRL(ENCM_CTRL_register.sr); is obsolete in TMC2160Stepper::push() TMC2160 has no ENCM_CTRL - register (0x72 is PWM_AUTO)

teemuatlut commented 5 years ago

It's easier to check against the datasheet if you stick to the same formatting.