klipper icon indicating copy to clipboard operation
klipper copied to clipboard

TMC: CoolStep support

Open leptun opened this issue 1 year ago • 15 comments

This PR adds the necessary changes in order to get CoolStep working on all TMC drivers that support it. All parameters for CoolStep can be defined in the config file. In a followup PR I am planning on writing in the docs instructions for how to get CoolStep configured and calibrated. I've tried to keep the commits as split as possible for ease of reviewing.

CoolStep has these main fields. These fields are supported by all drivers that implement CoolStep:

  • SEMIN
  • SEUP
  • SEMAX
  • SEDN
  • SEIMIN

There is an additional field which can be helpful for CoolStep: SFILT. This one is not supported by tmc2209. It adds a filter on top of the StallGuard output that is used by CoolStep internally.

There are also velocity based thresholds that can be used with CoolStep. There can be set both a lower and upper velocity threshold for CoolStep since it does not perform well at very low or very high velocities. Both of these settings can be configured with vcoolthrs and vhigh, which are the registers tcoolthrs and thigh. The reason I chose to name them with 'v' instead of with 't' is because we aren't setting the raw register value in the config, but rather the velocity that we want, which then gets converted to the tstep based representation. This naming scheme is also used by Trinamic in their datasheets. As for driver support, all of the ones that support CoolStep also have vcoolthrs, but the tmc2209 is missing vhigh.

Since vhigh was implemented for CoolStep, I also implemented the "High velocity" mode of the driver since it was just two extra fields (vhighfs and vhighchm). They can be useful at really high velocities for maintainging a bit of extra torque. I wrote a bit more about this in the config reference.

leptun avatar Mar 26 '23 08:03 leptun

Love it, exactly what I need right now :) Do you think somebody is able to configure cool step without understanding the datasheet? Asking because you implementing vhigh and vcoolthrs. At least I could not do it without measuring with an oscilloscope and read the datasheet about 10 times! :) As the velocity of the printer makes no big differ instead the velocity of the motor is important I would like to also be able to set them directly...

However, in the end I don't care how they will be configured, but it is really nice they can be configured.

Thanks a lot boi, you're saving me lot of work!

masterxq avatar Mar 26 '23 23:03 masterxq

Todo note from testing: Homing needs to handle coolstep now that it can be configured.

  • SFILT: It does not work well with homing. If enabled, the axis is guaranteed to skip some steps and not home correctly. So it must be disabled before homing and restored afterwards.
  • CoolStep: It dynamically changes the current during the homing move. I feel like this could cause issues. In my setup, it seemed to be fine, but I think it would be safer if coolstep was disabled during homing by setting SEMIN to 0 at the beginning of homing and then restoring it afterwards.

leptun avatar Mar 27 '23 15:03 leptun

@masterxq It should be possible to calibrate it easily (+ a bit of trial and error) after I write the docs for how to set this up.

leptun avatar Mar 27 '23 15:03 leptun

Thanks. Sorry for the delay in responding.

In general the code looks fine to me. I do have some high-level comments and questions.

  1. Is coolstep actually useful to general users on general 3d prints in regular day-to-day usage? What typical advantages would a user see after configuring it?
  2. How does one tune the two key values (vcoolthrs and vhigh)? (In very high-level general terms - is it based on printer tests, based on electrical tests, based on hardware specifications, or something else.)
  3. I'm confused by the references to "homing" in the vcoolthrs config reference description - does this impact all homing or just sensorless homing? If it's just sensorless homing, doesn't the TMCVirtualPinHelper disable tcoolthrs - so how would vcoolthrs impact homing?
  4. Could we name "vcoolthrs" to something like "coolstep_low_velocity"? Could we name "vhigh" to something like "coolstep_high_velocity"?
  5. Not a blocker, but it seems odd that there is a separate TMCVhighHelper from TMCVcoolthrsHelper class. Could there just be a TMCCoolstepHelper that does both? (The helper can inspect fields to see if the driver has thigh.)
  6. If thigh impacts sensorless homing, should TMCVirtualPinHelper automatically disable and reenable it?
  7. For what it is worth, the description of vcoolthrs and vhigh seems to contain a lot of technical jargon. I wonder if a simpler description may be more appropriate - for example something like coolstep_low_velocity: The minimum velocity (in mm/s) to activate the driver "CoolStep" capability. The CoolStep functionality may reduce heat generated by the stepper and driver. See the Trinamic specifications for details and limitations of CoolStep.

Thanks again, -Kevin

KevinOConnor avatar May 11 '23 17:05 KevinOConnor

Regarding 1:

We activated and configured it for the following reasons:

Less noise, lower vibration, lower motor temperatures.

For lower velocities the forces, caused by high current, is not needed. Lower force -> lower vibrations much lower motor temperatures and less noise.

In the higher velocities the current and force is needed and will be adjusted dynamically by coolstep.

As an example, with coolstep not activated, our printers run very smoothly with a current of 0.8 A for velocities of less than 100 mm/s. With a current of 1.5 A and these low velocities is makes noise, there are vibrations and the motors get hot, but we can reach very high velocities and accelerations.

A current of 0.8A and velocities >100mm/s causes motor stalls and complete holds. With 1.5A velocities ~1500mm/s just works smooth.

Coolstep is regulating this dynamically what is in my eyes the perfect solutions for all velocities ranges. Additional if more power is needed in lower velocities, eg caused by bad lubing of the linear axis or what ever else can happen, the power will also be adjusted. So this is much better than a static velocity based current setting!

In our opinion, coolstep is a killer feature for greater printer and i have no idea why it gets so little attention. The only disadvantage I see, is that the setup can be hard :/

We tested coolstep now for about 6 weeks with about 40 prints and could not observe any negative side effects.

Regarding 2:

We did everything, electrical measurements, used hardware specifications and did a lot of printer tests, for learning and understanding coolstep.

But I think, that this can be reduced to printer tests only with a good guide. The TMC documentations is really terrible there is a loooot of space for improvements. I'm really looking forward for @leptun guiding :)

The async DUMP_TMC_STEPPER helps a lot by configuring. We think a functionality that repeatedly prints the needed register while printing, would have very high impact by configuring coolstep and should be enough for setting it up. Maybe even an algorithm could be written to do this automatically.

Regarding 3: I see no reasons why coolstep should affect homing with sensors. Sensorless homing is affected.

Regarding vhigh: In our example our configuration causes that velocities >150mm/s results in csactual=31 what is the maximum current. Being faster than this velocity does make coolstep inefficient. Reaching vhigh velocity will also cause fullstep stepping, what can be usefull for reaching higher velocities. So it is possible to decide to set vhigh to eg 300mm/s for starting fullstep mode there.

It could be nice if this would go on, so we can remove our remaining hacks :)

Thanks for your work!

masterxq avatar May 21 '23 13:05 masterxq

@masterxq Thanks for the detailed response. I agree with what you wrote about coolstep. I unfortunately have very little time at the moment to look further into this PR, so I'm sorry but I have to postpone this PR for the time being. I'll try to respond to Kevin's questions as soon as I can again.

As for measuring an axis while moving, I used this macro. It's not great, but it did its job for getting the SG_RESULT values during motion at different speeds. I then take the terminal output, remove the invalid data at both ends and compute the average of the values. Hope this helps with manual calibration.

[gcode_macro MEAS_AXIS]
gcode:
  {% set stepper = params.STEPPER %}
  {% set register = params.REGISTER|upper %}
  {% set velocity = params.VELOCITY|default(100)|float %}
  {% set distance = params.DISTANCE|default(100)|float %}
  {% set measCnt = ((distance / velocity) * 35 + 15)|int %}
  G1 X0 F{velocity * 60}
  M400
  G1 X{distance}
  {% for x in range(measCnt) %}
    DUMP_TMC STEPPER={stepper} REGISTER={register}
  {% endfor %}
  G1 X0

Example usage:

MEAS_AXIS STEPPER=stepper_x REGISTER=DRV_STATUS DISTANCE=200 VELOCITY=100

leptun avatar May 21 '23 21:05 leptun

@masterxq, thank you for providing your testing results and feedback.

-Kevin

KevinOConnor avatar May 25 '23 17:05 KevinOConnor

This PR was inadvertently closed due to a regression introduced by #6293.

-Kevin

KevinOConnor avatar Aug 21 '23 17:08 KevinOConnor

Is something blocking this PR from being merged?

goodwin avatar Sep 08 '23 09:09 goodwin

Let me refresh this topic. I am very interested in this feature and I can provide testing results - I have access to proper current probes etc for measurements. I can test with TMC2160, 2208 or 2209.

holgin avatar Jan 04 '24 20:01 holgin

Same here, 5160 and 2209

agravelot avatar Jan 04 '24 21:01 agravelot

Ohway, I only found this PR now. Okay, I made the very same, but slightly different way. https://github.com/Klipper3d/klipper/pull/6592

About measurements, I made some tools to measure SG in Klipper. Maybe I needed to make another PR? I can if someone thinks it is needed.

nefelim4ag avatar May 12 '24 01:05 nefelim4ag

@nefelim4ag Hiya! Thanks for also being interested in this feature. I've honestly forgot that this PR was even open, but I can rebase it onto the latest master. I'm not sure what Kevin's priorities are at the moment, as this is not exactly that important of a feature. As for measuring the stallguard, a better option would be amazing! Do you have something already I could play with? A separate PR for that would be great. I don't think it should have any dependency on any of the new coolstep code.

leptun avatar May 13 '24 15:05 leptun

@leptun, I have not made a pull request yet, because I waiting for feedback about (your or mine PR) thresholds. My code is based on those thresholds but can work without them of course.

https://github.com/Nefelim4ag/klipper/ - master And there is a discourse with macros and graphs: https://klipper.discourse.group/t/tmc-coolstep-and-how-to-make-it-works/16259/2


I only tested it with 2209/5160, but it shall easily be portable to other drivers just in case someone is interested - I can just add other drivers. Just in case, I'm also available on klipper discord @nefelim4ag

nefelim4ag avatar May 13 '24 17:05 nefelim4ag

Thanks. I merged in 3 of the 5 commits. I merged:

  • tmc: Do not pass the frequency directly to the helpers
  • tmc2130: implement missing HighVelocity fields in the config
  • tmc: Implement CoolStep fields for all drivers

The remaining two commits also seem fine to me and worth merging. However, I would ask for some minor config naming and documentation changes.

How about we use the name coolstep_threshold with documentation:

#coolstep_threshold:
#   The velocity (in mm/s) to set the TMC driver internal "CoolStep"
#   threshold to. When set, the coolstep feature will be enabled if
#   the stepper motor velocity is near or above this value. Important
#   - if coolstep_threshold is set and "sensorless homing" is used,
#   then one must ensure that the homing speed is above the coolstep
#   threshold! The default is to not enable the coolstep feature.

How about we use the name high_velocity_threshold with documentation:

#high_velocity_threshold:
#   The velocity (in mm/s) to set the TMC driver internal "high
#   velocity" threshold (THIGH) to. This is typically used to disable
#   the "CoolStep" feature at high speeds. Important - if
#   high_velocity_threshold is set and "sensorless homing" is used,
#   then one must ensure that the homing speed is below the high
#   velocity threshold! The default is to not set a TMC "high
#   velocity" threshold.

-Kevin

KevinOConnor avatar May 14 '24 17:05 KevinOConnor

@KevinOConnor Thanks for merging the first 3 commits! I've updated the PR with your proposal. I agree that the naming scheme you chose might be better for users, even if the naming scheme I previously used matched closer with the datasheet.

I've reordered the commits in the PR and modified the ones you requested to be changed. Since the PR was manually partially merged, I did not want to rebase on the latest master, as that would have lost the already merged commits form the github PR commits list. Now that means that my new commits are based on commits which are not on master. I think it should rebase cleanly 🤞.

leptun avatar May 14 '24 20:05 leptun

Thanks! Happy to merge. Alas, github wont let me merge directly. You can try a git rebase (which would then preserve the github history that this PR was merged), or I can manually merge if you prefer.

-Kevin

KevinOConnor avatar May 14 '24 21:05 KevinOConnor

Ok. I rebased, but as I suspected, the first three commits are no longer linked to this PR. Only two commits now show up. I'll update the top level comment to link the missing commits so they are not lost to history.

leptun avatar May 14 '24 22:05 leptun

Thanks. I committed this change, along with a minor wording change to the docs (commit e0cbd7b5).

-Kevin

KevinOConnor avatar May 14 '24 22:05 KevinOConnor