teemuatlut / TMC2130Stepper

Arduino library for Trinamic TMC2130 Stepper driver
MIT License
159 stars 50 forks source link

Error while building https://github.com/p3p/Marlin/tree/32bit-bugfix-1.1.x-LPC1768 #4

Closed TGMods closed 7 years ago

TGMods commented 7 years ago

I have been building every update of the LPC1768 version of Marlin. Today I built the version dated 18/07/2017 with the errors shown in the attached file. Something changed between the 17/07/2017 and the above version. The Re-ARM also failed with the following error:

Compiling .pioenvs\Re-ARM\lib\TMC2130Stepper\source\TMC2130Stepper.o
.piolibdeps\TMC2130Stepper\src\source\TMC2130Stepper.cpp:1:17: fatal error: SPI.h: No such file or directory

#include <SPI.h>
^
compilation terminated.
*** [.pioenvs\Re-ARM\lib\TMC2130Stepper\source\TMC2130Stepper.o] Error 1
 [ERROR] Took 371.83 seconds
Environment Re-ARM              [ERROR]
 [ERROR] Took 371.83 seconds

 [SUMMARY]
Environment megaatmega2560      [SKIP]
Environment megaatmega1280      [SKIP]
Environment printrboard         [SKIP]
Environment brainwavepro        [SKIP]
Environment rambo               [SKIP]
Environment DUE                 [SKIP]
Environment teensy35            [SKIP]

Mega2560_failure_report.txt

teemuatlut commented 7 years ago

The TMC2130Stepper is including the SPI.h file that's an Arduino/Teensy library and for some reason the PIO decides that it wants to use Marlin/SPI.h. It's also compiling the TMC2130Stepper files even if the library shouldn't even be in use.

I built the upstream 1.1.4 just fine. Then built the HAL pr by p3p, that was fine too. Then after I had tried and failed to build the LPC1768 branch, they both started failing.

So to me this looks like PIO being dumb and not a bug in the TMC2130Stepper library.

TGMods commented 7 years ago

I can build 1.1.4 and the LPC1768 branch dated 17/07/2017 13:08 without any problems. So whatever is causing the problem is what was done to the LPC1768 branch after that date and time.

teemuatlut commented 7 years ago

I was able to compile the latest LPC commit by removing the line #26 and removing the TMC2130Stepper folder from Marlin/.piolibdeps. This will prevent PIO from compiling the TMC2130Stepper library files and also prevent it from re-dowloading the package.

teemuatlut commented 7 years ago

The compiling also works if you rename Marlin/spi.h to Marlin/private_spi.h Internal files really shouldn't use the same names as standard libraries commonly used. @p3p @thinkyhead

p3p commented 7 years ago

Can you confirm that this is still an issue with the latest platformio.ini, I thought I'd fixed PlatformIO compiling all downloaded libs and can't seem to get it to fail anymore.

lib_ignore =
  LiquidCrystal_I2C
  LiquidTWI2
  TMC2130Stepper
teemuatlut commented 7 years ago

I was building the AVR platform.

p3p commented 7 years ago

ah that's even worse as I've never had that fail and Travis is passing, whats your OS?

teemuatlut commented 7 years ago

W10. For some reason, PIO used the spi.h file in Marlin folder when TMC2130Stepper.cpp instructed the #include <spi.h>. I believe the proper fix is to rename the file in Marlin.

p3p commented 7 years ago

Well that explains the discrepancy, Linux is case sensitive for filenames so there wouldn't be a conflict, I guess I need to start checking things on Windows before pushing.

teemuatlut commented 7 years ago

Even so, the difference should be clear between include "..." and include <...>

Kohteesta: Chris Pepper Lähetetty: keskiviikko 19. heinäkuuta klo 20.00 Aihe: Re: [teemuatlut/TMC2130Stepper] Error while building https://github.com/p3p/Marlin/tree/32bit-bugfix-1.1.x-LPC1768 (#4) Vastaanottaja: teemuatlut/TMC2130Stepper Kopio: teemuatlut, Comment

Well that explains the discrepancy, Linux is case sensitive for filenames so there wouldn't be a conflict, I guess I need to start checking things on Windows before pushing. — You are receiving this because you commented. Reply to this email directly, viewhttps://github.com/teemuatlut/TMC2130Stepper/issues/4#issuecomment-316451186 https://github.com/teemuatlut/TMC2130Stepper/issues/4#issuecomment-316451186 ithttps://github.com/teemuatlut/TMC2130Stepper/issues/4#issuecomment-316451186 on https://github.com/teemuatlut/TMC2130Stepper/issues/4#issuecomment-316451186 GitHubhttps://github.com/teemuatlut/TMC2130Stepper/issues/4#issuecomment-316451186, or mutehttps://github.com/notifications/unsubscribe-auth/AHZy_gjW5g6kUOgzHhQ4weDWBSDk6S8Qks5sPjYxgaJpZM4OcrlK https://github.com/notifications/unsubscribe-auth/AHZy_gjW5g6kUOgzHhQ4weDWBSDk6S8Qks5sPjYxgaJpZM4OcrlK thehttps://github.com/notifications/unsubscribe-auth/AHZy_gjW5g6kUOgzHhQ4weDWBSDk6S8Qks5sPjYxgaJpZM4OcrlK https://github.com/notifications/unsubscribe-auth/AHZy_gjW5g6kUOgzHhQ4weDWBSDk6S8Qks5sPjYxgaJpZM4OcrlK threadhttps://github.com/notifications/unsubscribe-auth/AHZy_gjW5g6kUOgzHhQ4weDWBSDk6S8Qks5sPjYxgaJpZM4OcrlK.

p3p commented 7 years ago

oh I agree I wasn't defending PlatformIOs dependency scanner, I was just happy to understand why there was an issue when all my builds and Travis were working fine.

teemuatlut commented 7 years ago

I kinda like the idea of a dependency scanner but there should be a way to tell it whether or not the library is needed in the first place. Maybe the dependency scanner is only for libraries that are absolutely needed for compiling and so the TMC libraries (others too? u8g?) don't need to be there. Unutilized libraries also add to the compiling time.

p3p commented 7 years ago

The dependency graphs are different between OS, I'l have to look into this more.

Linux:

Library Dependency Graph
|-- <U8glib> v1.19.1
|-- <LiquidCrystal_I2C>
|   |-- <Wire> v1.0
|-- <LiquidCrystal> v1.0.5
|-- <Wire> v1.0
|-- <LiquidTWI2>
|   |-- <Wire> v1.0
|-- <SPI> v1.0
|-- <TMC2130Stepper> v2.0.0
|   |-- <SPI> v1.0

Windows:

Library Dependency Graph
|-- <U8glib> v1.19.1
|-- <LiquidCrystal_I2C>
|   |-- <Wire> v1.0
|-- <Wire> v1.0
|-- <LiquidTWI2>
|   |-- <Wire> v1.0
|-- <LiquidCrystal> v1.0.5
|-- <TMC2130Stepper> v2.0.0
TGMods commented 7 years ago

Everything is working now. Thanks guys.