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.19k stars 19.22k forks source link

[BUG] [PIO] L6470 stepper driver compilation error #15946

Closed erhancm closed 4 years ago

erhancm commented 4 years ago

Compiling with the L6470 stepper drivers result in a error.

Log

![output](https://user-images.githubusercontent.com/19529087/69126186-e0780680-0aa7-11ea-9d3b-2e0e630a5e1f.png)

I'm designing a board w/ 6 or so L6470 stepper drivers that are all in a daisy chain configuration. I've made a custom board file named "pins_MYBOARD.h" The part for the stepper drivers are:

pins_MYBOARD.h

```C++ // // Steppers // #define L6470_CHAIN_SCK_PIN 76 #define L6470_CHAIN_MISO_PIN 74 #define L6470_CHAIN_MOSI_PIN 75 #define L6470_CHAIN_SS_PIN 77 #define L6470_RESET_CHAIN_PIN 4 #define X_STEP_PIN 35 #define X_DIR_PIN 34 #define X_ENABLE_PIN 37 #ifndef X_CS_PIN #define X_CS_PIN L6470_CHAIN_SS_PIN #endif #define Y_STEP_PIN 28 #define Y_DIR_PIN 23 #define Y_ENABLE_PIN 33 #ifndef Y_CS_PIN #define Y_CS_PIN L6470_CHAIN_SS_PIN #endif #define Z_STEP_PIN 25 #define Z_DIR_PIN 26 #define Z_ENABLE_PIN 24 #ifndef Z_CS_PIN #define Z_CS_PIN L6470_CHAIN_SS_PIN #endif #define E0_STEP_PIN 47 #define E0_DIR_PIN 46 #define E0_ENABLE_PIN 48 #ifndef E0_CS_PIN #define E0_CS_PIN L6470_CHAIN_SS_PIN #endif ```

I know that the L6470 doesn't need the DIR and ENABLE pins, but if I remove it, then the sanitycheck will start throwing more errors. I would go around this when I can atleast get past compilation.

Configuration.h

```C++ #define X_DRIVER_TYPE L6470 #define Y_DRIVER_TYPE L6470 #define Z_DRIVER_TYPE L6470 //#define X2_DRIVER_TYPE A4988 //#define Y2_DRIVER_TYPE A4988 //#define Z2_DRIVER_TYPE A4988 //#define Z3_DRIVER_TYPE A4988 #define E0_DRIVER_TYPE L6470 //#define E1_DRIVER_TYPE A4988 //#define E2_DRIVER_TYPE A4988 //#define E3_DRIVER_TYPE A4988 //#define E4_DRIVER_TYPE A4988 //#define E5_DRIVER_TYPE A4988 ```

Configuration_adv.h

```C++ /** * L6470 Stepper Driver options * * Arduino-L6470 library (0.7.0 or higher) is required for this stepper driver. * https://github.com/ameyer/Arduino-L6470 * * Requires the following to be defined in your pins_YOUR_BOARD file * L6470_CHAIN_SCK_PIN * L6470_CHAIN_MISO_PIN * L6470_CHAIN_MOSI_PIN * L6470_CHAIN_SS_PIN * L6470_RESET_CHAIN_PIN (optional) */ #if HAS_DRIVER(L6470) //#define L6470_CHITCHAT // Display additional status info #if AXIS_DRIVER_TYPE_X(L6470) #define X_MICROSTEPS 128 // Number of microsteps (VALID: 1, 2, 4, 8, 16, 32, 128) #define X_OVERCURRENT 2000 // (mA) Current where the driver detects an over current (VALID: 375 x (1 - 16) - 6A max - rounds down) #define X_STALLCURRENT 1500 // (mA) Current where the driver detects a stall (VALID: 31.25 * (1-128) - 4A max - rounds down) #define X_MAX_VOLTAGE 127 // 0-255, Maximum effective voltage seen by stepper #define X_CHAIN_POS 1 // Position in SPI chain. (<=0 : Not in chain. 1 : Nearest MOSI) #endif #if AXIS_DRIVER_TYPE_X2(L6470) #define X2_MICROSTEPS 128 #define X2_OVERCURRENT 2000 #define X2_STALLCURRENT 1500 #define X2_MAX_VOLTAGE 127 #define X2_CHAIN_POS -1 #endif #if AXIS_DRIVER_TYPE_Y(L6470) #define Y_MICROSTEPS 128 #define Y_OVERCURRENT 2000 #define Y_STALLCURRENT 1500 #define Y_MAX_VOLTAGE 127 #define Y_CHAIN_POS 2 #endif #if AXIS_DRIVER_TYPE_Y2(L6470) #define Y2_MICROSTEPS 128 #define Y2_OVERCURRENT 2000 #define Y2_STALLCURRENT 1500 #define Y2_MAX_VOLTAGE 127 #define Y2_CHAIN_POS -1 #endif #if AXIS_DRIVER_TYPE_Z(L6470) #define Z_MICROSTEPS 128 #define Z_OVERCURRENT 2000 #define Z_STALLCURRENT 1500 #define Z_MAX_VOLTAGE 127 #define Z_CHAIN_POS 3 #endif #if AXIS_DRIVER_TYPE_Z2(L6470) #define Z2_MICROSTEPS 128 #define Z2_OVERCURRENT 2000 #define Z2_STALLCURRENT 1500 #define Z2_MAX_VOLTAGE 127 #define Z2_CHAIN_POS -1 #endif #if AXIS_DRIVER_TYPE_Z3(L6470) #define Z3_MICROSTEPS 128 #define Z3_OVERCURRENT 2000 #define Z3_STALLCURRENT 1500 #define Z3_MAX_VOLTAGE 127 #define Z3_CHAIN_POS -1 #endif #if AXIS_DRIVER_TYPE_E0(L6470) #define E0_MICROSTEPS 128 #define E0_OVERCURRENT 2000 #define E0_STALLCURRENT 1500 #define E0_MAX_VOLTAGE 127 #define E0_CHAIN_POS 4 #endif #if AXIS_DRIVER_TYPE_E1(L6470) #define E1_MICROSTEPS 128 #define E1_OVERCURRENT 2000 #define E1_STALLCURRENT 1500 #define E1_MAX_VOLTAGE 127 #define E1_CHAIN_POS -1 #endif #if AXIS_DRIVER_TYPE_E2(L6470) #define E2_MICROSTEPS 128 #define E2_OVERCURRENT 2000 #define E2_STALLCURRENT 1500 #define E2_MAX_VOLTAGE 127 #define E2_CHAIN_POS -1 #endif #if AXIS_DRIVER_TYPE_E3(L6470) #define E3_MICROSTEPS 128 #define E3_OVERCURRENT 2000 #define E3_STALLCURRENT 1500 #define E3_MAX_VOLTAGE 127 #define E3_CHAIN_POS -1 #endif #if AXIS_DRIVER_TYPE_E4(L6470) #define E4_MICROSTEPS 128 #define E4_OVERCURRENT 2000 #define E4_STALLCURRENT 1500 #define E4_MAX_VOLTAGE 127 #define E4_CHAIN_POS -1 #endif #if AXIS_DRIVER_TYPE_E5(L6470) #define E5_MICROSTEPS 128 #define E5_OVERCURRENT 2000 #define E5_STALLCURRENT 1500 #define E5_MAX_VOLTAGE 127 #define E5_CHAIN_POS -1 #endif /** * Monitor L6470 drivers for error conditions like over temperature and over current. * In the case of over temperature Marlin can decrease the drive until the error condition clears. * Other detected conditions can be used to stop the current print. * Relevant g-codes: * M906 - I1/2/3/4/5 Set or get motor drive level using axis codes X, Y, Z, E. Report values if no axis codes given. * I not present or I0 or I1 - X, Y, Z or E0 * I2 - X2, Y2, Z2 or E1 * I3 - Z3 or E3 * I4 - E4 * I5 - E5 * M916 - Increase drive level until get thermal warning * M917 - Find minimum current thresholds * M918 - Increase speed until max or error * M122 S0/1 - Report driver parameters */ //#define MONITOR_L6470_DRIVER_STATUS #if ENABLED(MONITOR_L6470_DRIVER_STATUS) #define KVAL_HOLD_STEP_DOWN 1 //#define L6470_STOP_ON_ERROR #endif #endif // L6470 ```

I'm currently using commit 0f57818f2, but I've also tried it with 865055e. I'm using platform IO with Visual Studio Code. I've tried compiling it on win10 and ubuntu. Both operating systems throw the same error. error: 'L6470' does not name a type

Steps to Reproduce

Required: Please include a ZIP file containing your Configuration.h and Configuration_adv.h files. files.zip

Steps

  1. [Configuration.h] Uncomment ..._DRIVER_TYPE and define it to L6470
  2. [Configuration_adv.h] Edit the chain position of each driver
  3. [..._pins.h] Add defines for L6470 SPI peripheral

Expected behavior: I'm expecting that it can compile successfully

Actual behavior: An error occurs

Additional Information

The hardware is not finished yet, but it would contain a sam3x8e. (Arduino Due clone). The pin configuration is as follows:

SAM3X L6470
76 SCLK
74 MISO
75 MOSI
77 SS
4 STBY/

I hope I was clear on the issue. Any help is much appeciated.

erhancm commented 4 years ago

I've forked bugfix-2.0.x 865055e07 so I could try a couple of things. See 477d041.

I think the error is that in marlin.h, the type (stepperX,Y,etc) is not referred to the correct class. libs/L6470/L6470_Marlin.h does not seem to have any linkage to the L6470 class. Its also confusing since L6470_Marlin.h/cpp also makes an object named L6470.

So i've added an include that refers to a L6470 libary that seems to have the correct class. And a couple of other things. However when I'm doing this, I seem to get a couple of more errors. See:

console log ```console Building in release mode Compiling .pio\build\DUE_USB\src\src\HAL\shared\HAL_spi_L6470.cpp.o Compiling .pio\build\DUE_USB\src\src\Marlin.cpp.o Compiling .pio\build\DUE_USB\src\src\core\serial.cpp.o Compiling .pio\build\DUE_USB\src\src\core\utility.cpp.o Compiling .pio\build\DUE_USB\src\src\feature\I2CPositionEncoder.cpp.o Compiling .pio\build\DUE_USB\src\src\feature\Max7219_Debug_LEDs.cpp.o Compiling .pio\build\DUE_USB\src\src\feature\babystep.cpp.o Marlin\src\HAL\shared\HAL_spi_L6470.cpp: In function 'uint8_t L6470_transfer(uint8_t, int16_t, uint8_t)': Compiling .pio\build\DUE_USB\src\src\feature\backlash.cpp.o Marlin\src\HAL\shared\HAL_spi_L6470.cpp:88:50: error: 'spi_abort' was not declared in this scope for (uint8_t i = L6470::chain[0]; (i >= 1) && !spi_abort; i--) { // stop sending data if spi_abort is active ^ Marlin\src\HAL\shared\HAL_spi_L6470.cpp: In function 'void L6470_transfer(uint8_t*, uint8_t)': Marlin\src\HAL\shared\HAL_spi_L6470.cpp:102:7: error: 'spi_active' was not declared in this scope if (spi_active) { // interrupted SPI transfer so need to ^ *** [.pio\build\DUE_USB\src\src\HAL\shared\HAL_spi_L6470.cpp.o] Error 1 Marlin\src\Marlin.cpp: In function 'void setup()': Marlin\src\Marlin.cpp:823:10: error: expected unqualified-id before '.' token L6470.init(); // setup SPI and then init chips ^ *** [.pio\build\DUE_USB\src\src\Marlin.cpp.o] Error 1 ```

I'm still a novice in programming, so I could be completely wrong, but am I going in the right direction ?

boelle commented 4 years ago

@erhancm since 2.0 was just released a few days ago has this changed this issue at all?

erhancm commented 4 years ago

Hi @boelle

The same issue seems to still exist with the released version of marlin 2.0. I've since got it to compile with some modifications i've made myself. But it isn't neat. See 1b1f3dff17dd3b26dfa4922218774b021e760c69.

edit: https://github.com/erhancm/Marlin.git L6470_dev_0 branch

boelle commented 4 years ago

https://github.com/MarlinFirmware/Marlin/commit/1b1f3dff17dd3b26dfa4922218774b021e760c69

seems like a simple change, did you send that in as a PR ?

boelle commented 4 years ago

all you did was add #include "../../../src/HAL/shared/HAL_spi_L6470.h" after line 24

erhancm commented 4 years ago

Look at the commit history. If you think its ok, then I'll send it in as a PR.

boelle commented 4 years ago

it might be ok, but @thinkyhead is the maintainer and do the merge of PR i'm just the guy sweeping the floor

erhancm commented 4 years ago

Ok. We can mark this as solved. Thanks

Bob-the-Kuhn commented 4 years ago

@erhancm - please submit a PR to fix this issue. This may be the solution for issue #12824.

Bob-the-Kuhn commented 4 years ago

@erhancm - why are you using the L6470?

Once I discovered the TMC5160 I dropped my L64xx work.

Bob-the-Kuhn commented 4 years ago

Curiosity question - why design your own board?

If 6 stepper support and 32 bits are the main drivers then maybe the Cohesion3D Remix may interest you. It's still available on Taobao.. Taobao's web site isn't English friendly and shipping to the US is a seperate order/transaction.

erhancm commented 4 years ago

Hi Bob,

I want to use the L6470 because I can save a crazy amount of IOs by utilizing serial comms. Also, if utilized correctly, then we could make better use of TI's integrated motion system and just send the nr of steps instead of pulsing an IO on the microcontroller. It seems like a better approach in my opinion. I'm designing the board more for a hobby.

Regarding submitting the PR, I'm still a bit new to the overall approach with using git, so I'm trying to rebase all the previous commits into a sensible release. I will get around to it this week.

erhancm commented 4 years ago

PR submitted

Bob-the-Kuhn commented 4 years ago

I'm trying to discourage use of the L64xx series. Once I discovered the TMC5160 I dropped all my L64xx work.

The 5160 has more features, better support and doesn't need a heatsink.

Both series can use SPI daisy chaining so there is no IO penalty except for the DIR pins.

I think you are planning to solder the L6470 directly to the board as there are no pololo style driver modules for it. I strongly urge you to switch to the pololo plug in style because 1) chances are excellent that one of the drivers will be destroyed during testing and 2) it gives you options to switch to other types of drivers.


FYI - I lost two 720W power supplies when using the L6470. I'd gone 2 years without a power supply failure when I was using the A4988. It's been 10 months since switching to the TMC5160 and no failures. My system uses five 2A steppers and a 1.5A stepper. Bench testing with six L470s went OK. Both supplies failed within 4 hours of when I hooked them into my printer.

I also had a nasty failure when hooked up to my printer. There was one incident where the printer was at idle and sparks started to fly from one of the L6470s. I lost 4 of the 6 L6470s, my DUE controller and the RAMPS_FD_V2 board. Only one L6470 was actually dead. All the other stuff was walking wounded - mostly worked but not completely.


I also expect that your software work will be greatly simplified if you switch to the TMC series. Much better software support and a huge user community that answers questions.

boelle commented 4 years ago

@erhancm is the issue still the same with all the updates in the last 7 days?

also you closed your PR?

erhancm commented 4 years ago

@boelle The minimum I got to work was that I would atleast get it to compile. Whether it is functional is still untested. Which is why I closed the PR. I feel I need to test/verify before submitting a PR. I was a bit too hasty in this regard. My mistake.

@Bob-the-Kuhn I'm going to consider if I'm going to keep developing the L6470's instead of switching to TMC's. My ultimate goal would be to utilize SPI exclusively instead of relying on STEP-DIR-EN signals. Ultimately, this would mean the custom PCB I've been planning on ordering would become obsolete.

I am going to close this thread now. I didn't know I was the one who needed to close it. But now I see a "Close and comment" button. Thank you for helping me where you could.

Bob-the-Kuhn commented 4 years ago

See PR #16452 for my latest L64xx code.

Bob-the-Kuhn commented 4 years ago

Bugfix-2.0.x will fully support your board once PR #16579 is merged.

The only items not supported are:

I've been able to connect my browser to the WIFI (that was a pain) but I have not been able to figure out how to make the UART interface functional.

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.