-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tmc5160.py incorrectly calculates CS value to where it is always 31 #6644
base: master
Are you sure you want to change the base?
Conversation
To clarify: This uses the tuning spreadsheet to calculate CS values versus globalscaler. pg. 74 of the datasheet shows that Irms is calculated by the tmc5160: Irms = (globalscaler/256) * ((CS+1)/32) * (Vfs/Rsense) * (1/sqrt(2)) This leaves us with one equation and two unknowns, which results in an unsolvable equation. The current code in Klipper then ignores the CS value and solves the above equation for global scaler: globalscaler = int((current * 256. * math.sqrt(2.) * self.sense_resistor / VREF) + .5) Effective what this does, since multiplying by 1 results in the same number that we started with, is sets ((CS+1)/32) = 1. This means that CS is 31. Irms = (globalscaler/256) * ((31+1)/32) * (Vfs/Rsense) * (1/sqrt(2)) Irms = (globalscaler/256) * 1 * (Vfs/Rsense) * (1/sqrt(2)) Klipper then back calculates the CS value. The problem with this is the way the driver implements this is it uses BOTH CS and Globalscaler in the same equation. So we can't back calculate because those maths don't math. One equation, two unknown functions are unsolvable, we would need two equations for this. |
As far as I am aware, the existing code is correct. (That is, it will faithfully take a requested If your drivers are running too hot, you should request a lower If you feel the Klipper code is not correct it is best to start by reporting the problem as described at https://www.klipper3d.org/Contact.html . In particular, make sure you are running an unmodified version of Klipper, set an appropriate -Kevin |
This is correct, to a degree. The existing code calculates the correct current, but does not use CS to do it, only globalscaler. This is unintentional, though. The code attempts to calculate CS, but only finds that it's already set to 31, because by not including CS when calculating globalscaler, we are setting CS to 31. The current is therefore correctly calculated, although not as correctly as it would be if it were to use CS to program it also. Setting the current is a non-issue, as it is correctly (to a degree) set. Edit: irun, hold, cs_actual are always 31 in the existing code. The issue is that CS is not correctly set and is needed by the driver to set the chopper parameters. Otherwise, you end up with a chopper that doesn’t work correctly, noisy, hot motors, ect.
My motors were running hot because the chopper settings can not be correctly set, per the datasheet. The drivers are fine. The motors were running at 120C before I implemented the code I have submitted. Once the chopper parameters are correctly set by my code changes, they run at 65-70C.
This has been raised as an issue before on Discourse and was overlooked. Please see the video for the practical implications of always setting the CS value to 31. I can submit a log file when my print is finished, along with dump_tmc, with the original code, and with my code changes. CS gets set to 31 on the current code, my code changes it to 22 based on my settings. -Brandon |
This is the output from the dump_tmc stepper=stepper_x on the existing code:
This is the output with the proposed code changes: |
Interesting. Let me share my 2 cents on the matter. Reading TMC5160 datasheet, section 9 (rev 1.18) 'Selecting Sense Resistors' suggests that both CS and GLOBALSCALER affect the maximum current, and they both are used to scale the TMC sine wave table. FWIW, they both appear to be a software coefficients, meaning that they do not affect the current measurements done by the driver in any way, and therefore they do not affect the accuracy of the current measurements. That accuracy is ultimately determined by the R_sense resistor and the reference voltage V_SRT. Since most of the TMC5160 drivers are designed to drive high-current motors, using them to drive lower currents (e.g. ~1A) results in the lower accuracy stepping, regardless of how you adjust CS and GLOBALSCALER individually. Now to your point: it does not seem that there is a 'correct way' to calculate the CS and GLOBALSCALER params. That is, they together must be set such that I_RMS is equal to the configured current. It is possible that CS has influence on other chopper parameters. However, the tmc5160_calculations.xlsx provided by trinamic uses CS=31 as the default to calculate the chopper parameters. So if anything, CS=31 seems to be a more appropriate and compatible choice that is already implemented in Klipper. Meaning that an average user that uses the spreadsheet from trinamic should get the best results with CS=31 implemented by Klipper. Now as to why you may be getting the results that you are getting. Having chopper parameters out of tune may result in noisy stepper operation and excessive heat dissipation, as per trinamic datasheet. I see that you adjusted chopper parameters from their default values supposedly to better suite your stepper configuration. However, it may be that the specs published by the stepper manufacturer (especially inductance) may deviate from the actual stepper parameters, thus fine-tuning the chopper parameters using manufacturer specs may not produce good results. And since it does not appear that you re-tuned the chopper parameters for the updated CS value for your new code, you may have accidentally produced better chopper configuration with a different CS value (in your modified code you get CS=22 AFAICT). I would re-tune chopper parameters using the TMC spreadsheet with an updated CS value and see if you get the same (bad) results as with CS=31. Then separately I see you have TBL=0, which is not recommended due to higher noise in the current measurements (use at least TBL=1). |
Hello Dmitry, thanks for your feedback. If you take a look at the HystStart_MIN formula in cell C3 of the tuning spreadsheet, you'll see that the formula provided by trinamic is: HystStart_MIN = MAX(0.5+(Coil_Current_Drop)2248*(Current_Scale+1)/I_coil_Peak/32-8,-2) This indicates that the CS value is directly used to calculate the hysteresis settings in the driver. In my case, I am using two LDO 2804 motors paralleled together on one driver. The Thevenin equivalent for this circuit gives me .3mH, .35ohms, 5.6A peak current, 60V, 12MHz, TBL = 0, toff setting = 3.
Agreed. The current measurements performed by the driver are hardware only.
The driver uses the R_sense, globalscaler, and CS to calculate the Irms as used by the software within the driver, per "Calculation of RMS current," pg. 74 of the datasheet. Mild inaccuracies will result by not correctly setting the CS value, but not enough to be noteworthy.
The user will get the best results by setting the CS value to where it results in an Rsense that is +/-3% of the actual value of the user's preinstalled sense resistor. Alternatively the user may swap out their sense resistor, however in my application that still wouldn't help as it would result in inadequate hysteresis settings. Regardless of all of this, the current Klipper code for the TMC5160 drivers unintentionally hard sets CS to 31. Everyone who is on Klipper and happens to use a tmc5160 driver has their CS set to 31. Systems of equations says we can't find non-zero variables from one equation with two unknowns, correct?
I'm glad we can agree that the CS value needs to change and not be hard set to 31 otherwise it will result in poorly tuned steppers. Please see the attached spreadsheets, with my values prefilled. One sets CS to 31, the other sets it to 22. If you have ideas for how I can tune these more effectively I am all ears! -Brandon, BSECE |
I'm sorry if my message sounded that way, this was not my intention. So far, I did not find any indications that the CS=31 value used by Klipper causes any problems. So, all I wanted to say is that if you do modify CS, you should calculate the chopper parameters accordingly. I just saw that in both of your logs you have
so I assumed you didn't change these parameters between CS=31 and CS=22 values. However, if in practice you did, then good. However,
I'm unsure why you attached TMC22xx spreadsheets for calculations related to TMC5160. Are you using TMC22xx spreadsheet to calculate the parameters for TMC5160? Separately,
I'm afraid this isn't a setup recommended for stepper motor drivers and I suspect it won't work well. If I am not mistaken, the only 'kind of recommended' way to connect two stepper motors to one driver is in series. This way, at least, the stepper motor driver can reliably measure the current that goes through the stepper motors coils. The parallel setup does not allow that. |
I changed the CS value in both spreadsheets to demonstrate my point. The CS value in my instance, regardless of good practice, needs to be set to 22 to properly tune the chopper. Setting it to 31 results in inadequate hysteresis. Klipper prohibits the CS value from changing from 31. Which is not intentional via the code. There is an attempt to calculate it, albeit implemented incorrectly. `def_calc_globalscaler(self, current):
def _calc_current_bits(self, current, globalscaler):
This will always result in a value of 31. Why include the formula, why not just say cs=31? Setting CS to 31 is the recommended ideal situation, as is setting Globalscaler to 256. |
Yes, I understand where CS in Klipper comes from and why it gets 31. However, let me again stress my points:
In light of (1), it seems that the change in its current form is dangerous to merge. An alternative implementation would be to expose CS parameter in TMC stepper configuration, and add a command like In light of (2), it would be great to see more broad testing of this feature. Klipper Discourse may be a good starting point for such conversations. Please do not take my message as discouragement of your work, and also note that I am not the one taking the ultimate call here. I'm merely providing my view on the matter, and I hope this can go into some positive direction. If anything, I'm also curious to test non-default CS values on my printer with TMC5160 drivers now. |
Agreed, merging in the form it's in would likely have wide ranging, negative consequences for existing driver settings. However, I'm not so sure that these consequences don't already exist by leaving it as it is in the current form. I understand my situation is likely unique, and not recommended. I am pushing the TMC5160 to the limits, and really a tiny bit beyond them, as I'm sure you're aware from the spreadsheet. But nonetheless, situations do exist where this causes an issue.
I appreciate the discussion, not discouraged, just trying to defend my case. I will raise the issue on Discourse and see if I can get some input. Let me know the results of your testing. |
Updated: I added a field called tmc_cs: which can be called under: [tmc5160 stepper_*] Code needs cleaning up. If no tmc_cs is defined, it will default to 31. |
It is very much intentional. The code was specifically written to maximize the value of IRUN (aka CS). The trinamic specifications can be esoteric, arcane, and difficult to read. On close inspection, though, it is clear that a low IRUN value can reduce the precision of its sine wave table. That is, the stepper drivers set a particular stepper position by altering the current through the two stepper motor coils (different coil currents thus control the stepper shaft angle). The trinamic drivers can use this same mechanism to lower the overall current through both coils, and this is what IRUN does. However, if IRUN is low enough, then it ultimately impacts the precision of each stepper motor microstep position. All of the trinamic drivers have a mechanism to set the current outside of IRUN - tmc2130/tmc2209 use VSENSE, tmc5160/tmc2240 use GLOBALSCALER. On all drivers, Klipper attempts to set the requested current while maximizing the IRUN (or CS) value. This is purposely done to maximize the microstep precision. This is also, as near as I can tell from the tmc specs, the recommendation from Trinamic. If you have some unusual setup that requires special fields, then you can use the -Kevin |
Hey Kevin, I created a thread on Discourse on the topic to discuss further.
The code just sets IRUN to 31 in every instance of tmc5160 Klipper. It doesn't calculate it. I am unsure why we would include _calc_current_bits, if the goal were to just set CS=31. It is not mathematically possible to arrive at any value other than CS=31 in the current implementation.
Agreed, simply put the datasheet leaves a lot to be desired. By leaving this value at 31 and choosing microstep precision, excessive heat, resonance (which causes worse precision, but I digress), and decreased torque is being invited in. In addition, it prevents chopper parameters from being tuned. |
That is incorrect. For low It's actually, unfortunately, a bit common. Some extruder setups need a very low current that can only be obtained by configuring both a low GLOBALSCALER and low IRUN setting.
I don't believe that to be correct. As I understand it, VSENSE and GLOBALSCALER are effectively used to scale the sense_resistor, while IRUN is effectively used to scale the sine wave table. I haven't seen anything that indicates scaling the sine wave table is a fundamentally better way to manage torque, heat, nor resonance. (Nor that they are even directly related.) -Kevin |
Kevin, Klipper uses a single equation to find two non-zero unknowns. This is a mathematical impossibility. This is the subject of linear algebra. |
Just a small notice on:
Unfortunately, the datasheet does not speak very clearly about GLOBALSCALER, however it says and "Fine-tune the current to the specific motor via the 8-bit GLOBALSCALER. Situation specific motor current adaptation is done by 5-bit scalers (actual scale can be read via CS)" and then the formulae for I_RMS suggest that both GLOBALSCALER and CS are equivalent. Meaning that we don't get better microstepping by configuring higher CS and lower GLOBALSCALER. Just that CS can be modified by the driver itself (e.g. if IHOLD != IRUN, the driver will choose CS based on the current operation mode). And @HonestBrothers, in light of that message "This value should be chosen before tuning other settings because it also influences chopper hysteresis." for GLOBALSCALER, I think you are mistaken to think that if you adjust CS and keep GLOBALSCALER larger, you will get different hysteresis parameters. Basically, TMC spreadsheet simply does not account for the existence of GLOBALSCALER and assumes it is set to 0 (==256, full scale). |
Do we know what the hysteresis settings are based on? There are indications that some parameters, PWM_SCALE_SUM pg. 63 for instance, only uses CS_ACTUAL in it's calculations. I know this is for stealthchop, but we are given very limited information about spreadcycle. You may very well be right about my tuning, but implementing this in the way in which it's implemented uses a lot of assumptions...
Back calculating CS after setting Globalscaler results in CS never changing. The other way around is as defined in the spreadsheet, doesn't result in an underdetermined situation, and scales both CS and Globalscaler. Also, This speaks to me to set CS first, then get a more exact Irms with Globalscaler. |
Try to calculate any of the values from the spreadsheet using a Globalscaler set to 32. Set global scaler to 32, CS to 31, then solve the equation on pg 74 using an Rsense found in one of the TMC5160 drivers. |
I do not know, the documentation is very limited. The available information, however, suggests that GLOBALSCALER and CS behave the same way, just GLOBALSCALER has higher resolution and can only be changed via SPI, and CS has lower resolution, but can be adjusted by the driver itself based on other settings (like IRUN and IHOLD). So, instead of focusing too much on Klipper 'incorrect behavior' (which, again, is correct, just does not match your desired behavior), try to take the chopper parameters you tuned for supposedly lower CS value and pass them to mainline Klipper without any changes in the code, and see if they will behave the same way as on modified Klipper. Then you can test for yourself whether reducing GLOBALSCALER and CS has the same effect on chopper parameters or not. |
The tmc specs can be obtuse and confusing. However it is clear that IRUN (and CS) scale the microstep table. All the specs (tmc2130, tmc2660, tmc2240, and tmc5160) have some variant of:
Similarly, although we don't get a lot of details on how GLOBALSCALER is actually implemented, we do know it replaced VSENSE in tmc2130 (and similar) drivers. In the tmc2130 spec:
So, I think there is good reason to believe both VSENSE and GLOBALSCALER are implemented using a separate mechanism from IRUN (and CS). Further, from the description of HEND we get a glimpse of how hysteresis, the sine table, and IRUN interact. It seems the math is something like: Cheers, EDIT: Just to be clear, it's the integer operation of |
Also, FYI, CS and IRUN are the same thing - you can see this in the tmc2660 spec:
Which is an identical description of IRUN in newer drivers. It seems that when tmc introduced IHOLD they decided to rename CS to IRUN, but it's clearly the same thing (at least when the motor is moving, when the motor is not moving then CS would be equal to IHOLD). -Kevin |
If this were the case (chopper parameters depend only on CS but not on GLOBALSCALER), then it may make sense to expose CS as a configuration option for the drivers. At the very least, TMC spreadsheet clearly uses CS as an input in the calculations - by default it is set to 31 there, but changing it modifies recommended HSTRT and HEND values. So it can happen that for certain combinations of motors and drivers the default CS=31 will produce recommended values for HSTRT and HEND outside of the permitted range (and modifying TBL and TOFF parameters does not bring them to the permitted range). At least I can attest that under some conditions the TMC spreadsheet will not produce valid recommendations for chopper parameters with CS=31 (as it happens for tmc5160 driver, LDO-42STH48-2804 stepper motor, running at 48 volts). This is why I asked @HonestBrothers to test the chopper parameters optimized for CS < 31 on the default Klipper code. If they happen to work well the same way as with Klipper modified to compute different CS values - then I believe no modifications to Klipper code are necessary. If they will not work, perhaps the configuration option should be made available. Though I agree that reducing CS (as, IRUN/IHOLD) is suboptimal due to the loss of microstepping precision, so it should be consider a 'power option' for the users understanding what they are doing. Alas, I cannot reproduce the problem on my hardware - I do have a printer with tmc5160 drivers, and I can produce chopper parameters for CS=31 and reduced CS (projected from GLOBALSCALER value), however I do not observe noticeable difference in steppers performance for both sets of parameters. |
@HonestBrothers look, there is no question that CS can be calculated differently. You are missing the point: Klipper calculates CS and GLOBALSCALER in a specific way (such as to maximize CS while keeping GLOBALSCALER in the valid range). The question is, is it an optimal approach? From microstepping precision standpoint - yes, it is the best we can do. The only question remains is, are there any other considerations where it may be suboptimal? The only argument I can think of is that maybe it is not good for chopper performance. However, optimizing CS such that recommended RSense matches the actual RSense clearly is not the right approach - I honestly don't see any reason for that. Like, maybe one possibility would be to maximize CS such that the chopper parameters are still in the valid range - but I don't see putting such code into Klipper - it would be on user to fiddle with various chopper parameters anyway to arrive to the best possible combination for TOFF, TBL and CS for the given stepper. This is why if you could test your good working chopper parameters on default Klipper, that would be great, we'd at least have some data point whether CS modifications are necessary at all. Otherwise, we go in circles on this discussion, I'm afraid. |
Edit: Sineos coming in with pg 111 I understand the reasoning behind wanting to maintain the resolution, but I believe the datasheet gives us parameters recommended to maintain the resolution, CS > 16, Globalscaler > 128. It appears like the instantaneous motor current is calculated with both CS and Globalscaler, but I'm not confident Globalscaler is used in hysteresis calcs. Hand calculating with CS that is tuned via the spreadsheet and whatever Globalscaler gets set to does arrive at a more precise Irms, as would be expected. I can not get the spreadsheet to produce valid hysteresis results using a CS of 31. I can not get Klipper to produce a result for CS other than 31. I am not aware of anyone having a CS of a value other than 31, and I believe this to be because of the faux pas. The result, for me at least, is the spreadsheet says to use fast decay only. Using CS=31, I end up with a Globalscaler of 97, below the recommended min of 128. No settings for hysteresis will get me anywhere near acceptable settings via the spreadsheet. If I change CS, I can get the acceptable hysteresis settings, lower motor temps, and it seems to have reduced some artifacts that I thought were from belts, but now I'm not so sure. This is anecdotal though. The only thing now is Rsense is higher than what I have on the driver in the spreadsheet. If my understanding is correct, if I change the Rsense to a slightly higher value, CS and Globalscaler will increase to a more acceptable value, too. HystStart_MIN is 27, needs to be under 22, when using a CS of 31. I will see if I can get some more measurable testing data. |
Section 9.1 (I realize this is 2209, I'm assuming it works the same way). CS scales the VREF used in the chopper comparators. So if 5160 works this way, 100% leaving CS at 31 leads to poor chopper performance. CS is used as a way to scale Rsense. https://www.analog.com/media/en/technical-documentation/data-sheets/tmc2209_datasheet_rev1.09.pdf |
This is 100% my understanding of this datasheet, since we are not choosing Rsense. There is nothing preventing the end user from swapping Rsense to better match their motors. Keeping CS=31 does however prevent the user from matching their motors to the drivers. There are certain circumstances where keeping CS=31 just prevents the lowering of hysteresis values enough to be usable. 2804s at 48V could work perfectly at CS=22, Rsense=0.075, which are the step sticks. If we keep CS=31 and the user swaps out their Rsense to to match the 0.114 output in C46, they will still have too high of hysteresis. Implementing changes to code should probably cap the users ability to adjust CS lower than 16. If they reach this case they need to change RSense or buy a different driver. |
@Sineos is the flow chart assuming Rsense was correctly chosen so that max current flows at or slightly above CS=31? This datasheet has too many holes. |
I'd just like to reiterate:
|
Heh, you are asking the wrong person here. I'm just trying to interpret the spec and this chapter has the heading "Current Settings and First Steps" and actually it makes sense for me to set the desired maximum current at startup to the upper limit of the scaling range. For me the key aspects in the spec are: Page 36Page 38Page 52Page 62Page 74Page 75 (complementary to page 36 and 38)Page 111The "Quick Configuration Guide" SummaryFWIW, this is just the basics. There are so many dependencies, like running Stealth Chop or Spread Cycle etc, that I'm far from claiming to understand them. IMO, it would be much more worthwhile if OEMs producing such drivers would stop chasing the next marketing award and offer e.g. 3 lines of TMC5160s or TMCs in general:
The number of printer designs that use NEMA34 steppers seems somewhat limited. (Hint @bigtreetech) Edit and side note:Almost every electrical/electronic device that has any kind of control or conversion functionality (even a "stupid" toroidal transformer) has an optimal operating point that is usually neither at the low end nor at the high end of its range. Leaving this point leads to artifacts, low efficiency, excess heat, etc. What might actually be interesting, if Klipper had a |
Part of problem is, the current code doesn't switch CS until Globalscaler hits 32. This violates best results you show from pg 36. Even if we changed that limit of 32 to 128, I don't believe the code will behave in the way Trinamic intended due to my point 3 in my last comment. Imot uses both CS and Globalscaler.
For me changing CS to a lower value helped reduce the heat and decrease artifacts. |
@dmbutyugin doing some testing today, I can actually get that audible resonance during printing by changing IRUN. I'll see if I can get a video of it. |
Thank you for your contribution to Klipper. Unfortunately, a reviewer has not assigned themselves to this GitHub Pull Request. All Pull Requests are reviewed before merging, and a reviewer will need to volunteer. Further information is available at: https://www.klipper3d.org/CONTRIBUTING.html There are some steps that you can take now:
Unfortunately, if a reviewer does not assign themselves to this GitHub Pull Request then it will be automatically closed. If this happens, then it is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available. Best regards, PS: I'm just an automated script, not a human being. |
Edit: Changed naming conventions, and changed CS to allow 0. I updated the code:
|
TMC5160.py incorrectly implements CS values to where they are always 31.
The calculation of globalscaler in current Klipper code leaves out the CS values, and then calculates CS after globalscaler.
According to page 74 of the datasheet, Calculation of RMS Current, the end result of not specifying a CS value, automatically sets the CS to 31.
(CS+1)/32 = 1.
In the current code, globalscaler is calculated and then CS is back calculated using globalscaler, but the only value it can be set to is 31, which the globalscaler calculation has already determined by setting it to 1.
Analog Devices is less than forthcoming on how to calculate the CS value, as the equation used on page 74 has two unknowns and thus can not be calculated. However, the tuning spreadsheet TMC220x_TMC222x_Calculations.xlsx allows us to back calculate the CS value:
cs = int(100 * (current * math.sqrt(2)) * self.sense_resistor) - int(1 - 2 * (current * math.sqrt(2)))
Implementing this bug fix allows us to effectively set the CS value before globalscaler, and then set globalscaler based on the chosen CS value. Which is how I believe Analog Devices intended this driver to be tuned. This has the end result of CS being used to effectively scale Rsense, ie. when the driver is rated for more currrent than is being used, Rsense does not need to be changed and instead CS can scale the Rsense input.
The practical issue is that the way Klipper currrently calculates CS makes the motors hard to tune on high amp 5160 drivers, and high voltages practically impossible on some motors because the chopper parameters can not be correctly set. Current word around town is that 2804's don't do well above 24V and will result in VFAs if used at 48V.
I have implemented this fix on my own device and am using 2804's at 60V, which run at a cool 65C. Before implementing the motors were very noisy and melted my motor mounts. Now they are regular level of 1980's printer noisy.
The CS value is calculated automatically so it requires no user input. This fix should allow better motor tuning, the calc sheet to be used effectively, and certain motor, voltage, driver combos to be used where they could not before.
I apologize ahead of time for errors. I don't program often, just took a whirl at fixing this issue. Also sorry for reopening this, I made a hot mess of the last one.
Signed off by: Brandon Smith honestbrotherstv@gmail.com