MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.28k stars 19.24k forks source link

[FR] Per-axis stepper driver selection #11119

Closed iosonopersia closed 6 years ago

iosonopersia commented 6 years ago

@thinkyhead , @MagoKimbra , @GMagician , @Sineos , @ejtagle This is an idea which was born in the discussion here: #11098

The suggestion made below comes from me and @GMagician :


I propose a different approach, define driver for every installed axis like

Configuration.h:

#include "supported_drivers.h"
[...]
#define X_DRIVER  DRIVER_DRV8825
#define X2_DRIVER  DRIVER_DRV8825
#define Y_DRIVER  DRIVER_A4988
#define Z_DRIVER  DRIVER_TMC2100
#define E0_DRIVER DRIVER_TMC2130
[...]

and each driver include may do something like...

supported_drivers.h:

#define DRIVER_DRV8825  ./drivers/drv8825.h
#define DRIVER_A4988  ./drivers/a4988.h
#define DRIVER_TMC2130  ./drivers/tmc2130.h
[...]

drv8825.h

#ifndef MINIMUM_STEPPER_PULSE
  #define MINIMUM_STEPPER_PULSE 0
#endif

#if MINIMUM_STEPPER_PULSE < 2
  #undef MINIMUM_STEPPER_PULSE
  #define MINIMUM_STEPPER_PULSE 2
#endif

a4988.h

#ifndef MINIMUM_STEPPER_PULSE
  #define MINIMUM_STEPPER_PULSE 0
#endif

#if MINIMUM_STEPPER_PULSE < 1
  #undef MINIMUM_STEPPER_PULSE
  #define MINIMUM_STEPPER_PULSE 1
#endif

Drivers.h

#ifdef X_DRIVER
  #include X_DRIVER
#endif
#ifdef Y_DRIVER
  #include Y_DRIVER
#endif
[...]

This is more tedious for programmers but easier from user perspective. The user doesn't need to read datasheets to understand which is the slowest one.


Then, every file that will need to use driver-specific values (such as MINIMUM_STEPPER_PULSE), will need to include Drivers.h

If the idea is ok, we can open a PR to try implementing it. What do you think?

teemuatlut commented 6 years ago

I don't think we need separate files for each driver type if all it contains is min step pulse and max step frequency. This sounds a bit like it could be added to stepper_indirection. You also need to find the max value between all the configured drivers.

But overall as an idea I think it would be useful for Marlin to be aware of the driver types it is using. Even if they wouldn't require additional configuration and communication and other smart features.

I'll also post here what I said in the other thread about how the TMC drivers will be defined in the future:

Possibly related on driver types (coming soon): teemuatlut/Marlin@00d735e

Though not about timings but the TMC driver selection.

iosonopersia commented 6 years ago

Ok, I mostly agree. Just as a recap, we now have these driver-specific values:

#define MINIMUM_STEPPER_DIR_DELAY
#define MINIMUM_STEPPER_PULSE
#define MAXIMUM_STEPPER_RATE
forkoz commented 6 years ago

If I still had a different driver for E, I'd be screwed right now. Was going to buy only 3 TMC's too, there would be no way to fix it. A few weeks ago I installed 4th LV8729 by chance to play with E amperage so skated by pure chance.

Sineos commented 6 years ago

I have no strong feelings how it should be implemented. From my (users) point of view, it should:

GMagician commented 6 years ago

@Sineos I think that playing with defines and #if defined(xx) something may be done.

GMagician commented 6 years ago

@teemuatlut I don't want to overwrite your work. I'll try to address this when you will release yours (or maybe if you think that this solution will destroy all your work, and you think to stop it, I'll start sooner)

teemuatlut commented 6 years ago

It may take a while before I make the PR as it's not the first one in line that I have.

thinkyhead commented 6 years ago

Move discussion to #11133

github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.