Skip to content

[1.1.x] Configure stepper drivers per axis#11361

Merged
thinkyhead merged 2 commits intoMarlinFirmware:bugfix-1.1.xfrom
thinkyhead:bf1_define_drivers_th
Jul 25, 2018
Merged

[1.1.x] Configure stepper drivers per axis#11361
thinkyhead merged 2 commits intoMarlinFirmware:bugfix-1.1.xfrom
thinkyhead:bf1_define_drivers_th

Conversation

@thinkyhead
Copy link
Member

Based on #11264 (and #11133)

  • Add axis specific driver definitions to determine proper timing parameters like step pulse width, delay after asserting dir level and maximum stepper frequency.
  • The user can use settings in Configuration_adv.h to override the parsed driver IC defaults.
  • The new driver definitions are used to determine which TMC or L6470 are in use and need initialization.
  • By default the Allegro A4988 drivers are enabled for X, Y, Z and E0. More on this below.
  • Add new TMC macros that work by checking for a certain feature, rather than the dev checking for a list of driver types that support the feature.
  • References to TRAMS removed and will simply be handled by defining the four axis as TMC5130. However this PR doesn't yet move Marlin to the library that supports the driver.
  • Differentiating between TMC2130 in SPI configured mode and standalone mode is done by having two driver definitions; TMC2130 and TMC2130_STANDALONE. The way I see it, the default use case for that driver is in configured mode and using standalone mode is a special case. In standalone mode only the stepper timings are affected and they pretty much act the same as TMC2100 as far as Marlin is concerned.
  • Updates Travis to use the new driver definition method so that they actually are tested.

I decided to use the Allegro drivers as the default to match what is the most common configuration in the user base. Specifying any drivers is not forced but will default to DRV8825 configuration, except for DIR delay. This could be improved, or force defining at least one driver (unless overriden in _adv.h).
I think the main question is if we know the parameters different drivers require, which should be used as the default? I think we should not use 0 delays as there is no driver compatible with such timings.
However, this doesn't take into account the natural delay there is no matter what we do.
This also leads to a more general configuration change that affects the stepper ISR. Perhaps we should no longer check if MINIMUM_STEPPER_PULSE is defined, because from hereon it always is, and rather check if the defined pulse width is greater than 20 for example. @ejtagle ?


The compile time driver IC macros:

AXIS_DRIVER_TYPE(A, T)
Check if an axis A has a driver type T associated with it.

HAS_DRIVER(T)
Check if the user has driver type T in any of the axis enabled.

HAS_TRINAMIC
Check if any supported Trinamic drivers are enabled in any axis.

AXIS_IS_TMC(A)
Check if the specified axis A has a supported Trinamic driver enabled.

@thinkyhead thinkyhead merged commit 2b860ab into MarlinFirmware:bugfix-1.1.x Jul 25, 2018
@thinkyhead thinkyhead deleted the bf1_define_drivers_th branch July 25, 2018 09:05
* Override the default value based on the driver type set in Configuration.h.
*/
#define MINIMUM_STEPPER_DIR_DELAY 0
//#define MINIMUM_STEPPER_DIR_DELAY 650
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are assuming A4988 drivers as default, MINIMUM_STEPPER_DIR_DELAY should be 200 as default too, doesn't it? And the same for the rest of related parameters.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's alright. This is consistent with the previous arrangement, with A4988 (or no tuning) as the default, but DRV8825 (a very common driver) as the example for overrides.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right, sorry!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants