Marlin icon indicating copy to clipboard operation
Marlin copied to clipboard

Remove tmc configuration section

Open ManuelMcLure opened this issue 3 years ago • 4 comments

Use values in tmc_smart section for TMC26x drivers

Description

Removes the tmc configuration section from Configuration_adv.h and instead uses the configuration values in tmc_smart for TMC26X drivers.

Requirements

A board with TMC26X drivers.

Benefits

Very few boards use TMC26x drivers, yet the fact that there are two configuration sections for Trinamic drivers leads to confusion as to which section to use. All the options required to configure TMC26x drivers are available in the tmc_smart configuration section (albeit with different names) so it doesn't make a lot of sense to have separate sections.

This would reduce the number of support questions in Discord.

Configurations

Stock Configuration.h with just any driver type changed to TMC26x

Related Issues

No related issues.

ManuelMcLure avatar Jun 19 '22 19:06 ManuelMcLure

This may cause more issues, just with TMC26X users this time. Most users do not check their r sense resistor

You have converted from _SENSE_RESISTOR to _RSENSE

but the default for TMC26X is 91 and for TMC2208/9 is 0.11

with the updated macro (ST##_RSENSE * 1000) The default value will be 110 not 91, this is bad..

Users will not know to update _RSENSE to 0.91 for TMC26X stepper drivers.

ellensp avatar Jun 20 '22 10:06 ellensp

Our configuration repository does not list any printers that use TMC26X drivers (found by running

find * -name 'Configuration.h' -print0 | xargs -0 grep '_DRIVER_TYPE\s*TMC26X'

from the root of the configuration repo.

Note that this affects only TMC26X (TMC260/TMC261/TMC262) drivers, not TMC26xx drivers such as the TMC2660 which already use the values in the tmc_smart section, which already results in even more confusion since people with 2660s would be drawn to use the TMC26X section because of the similar name.

Also, although we default the RSENSE to 0.11, this isn't valid for all TMC drivers anyway and is just a safe default, as is the 91mA that we set for TMC26X. In fact, the 91mA for TMC26X is too small for a CS of 31 per the TMC261 data sheet, although it is valid for the TMC262.

If needed I could add a #ifdef to each [AXIS]_RSENSE value, but again, using TMC26X as the name to compare against would be confusing for new users that have 2660s.

ManuelMcLure avatar Jun 20 '22 16:06 ManuelMcLure

I have added a comment about the driver default changing, also in the sanity-check error message. But, is the *_MAX_CURRENT really the same thing as *_CURRENT? Correct or not, the impression I get from the names of the settings is that one provides a cap on dynamic current, while the other sets a constant current. Or, are they actually doing the same thing, just one of them is badly named?

thinkyhead avatar Jun 22 '22 03:06 thinkyhead

From my reading of the code in TMC26XStepper disables CoolStep on the driver so it is a current setting and not a max current setting.

ManuelMcLure avatar Jun 22 '22 04:06 ManuelMcLure