New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LDF C++ parser error #837

Closed
blackketter opened this Issue Nov 23, 2016 · 20 comments

Comments

Projects
None yet
3 participants
@blackketter

blackketter commented Nov 23, 2016

I'm building platformio.ini file that targets two different architectures (ESP8266 and Teensy3) and need to include different sets of libraries. I'm using lib_deps and lib_ignore successfully, but the LDF C++ parser seems to be failing for me here. It is including both libraries in the build, though it should only include the first.

The code in the header file looks like this:

#if TEENSY31 == 1
#include <ILI9341_t3.h>
#else
#include <Adafruit_ILI9341.h>
#endif
and the platformio.ini flag looks like this:

build_flags = -O -D TEENSY31=1 -UUSB_SERIAL -DARDUINO_ARCH_AVR -DUSB_SERIAL_HID -DLAYOUT_US_ENGLISH -I./src
But the graph looks like this:

|-- <ILI9341_t3>
| |-- v1.0
...
|-- v1.0.0
| |-- v1.1.5
| |-- v1.0
If I simply delete the line:

#include <Adafruit_ILI9341.h>
It builds fine.

Attached is a trivial example to reproduce:

ldftest.zip

@ivankravets ivankravets added this to the 3.2.0 milestone Nov 23, 2016

@ivankravets ivankravets self-assigned this Nov 23, 2016

@jrobeson

This comment has been minimized.

Show comment
Hide comment
@jrobeson

jrobeson Nov 27, 2016

Contributor

Is this a regression, or not supported yet?

Contributor

jrobeson commented Nov 27, 2016

Is this a regression, or not supported yet?

@ivankravets

This comment has been minimized.

Show comment
Hide comment
@ivankravets

ivankravets Nov 27, 2016

Member

I'll take a look on it later. Temporary solution is to lib_ignore = LibName.

Member

ivankravets commented Nov 27, 2016

I'll take a look on it later. Temporary solution is to lib_ignore = LibName.

@ivankravets

This comment has been minimized.

Show comment
Hide comment
@ivankravets

This comment has been minimized.

Show comment
Hide comment
@ivankravets

ivankravets Nov 28, 2016

Member

This is a problem of SCons engine. The SCons C/C++ Preprocessor currently is disabled. I refactored disabled code and fixed all issues. See my PR to SCons https://bitbucket.org/scons/scons/pull-requests/380/improvements-for-scons-c-c-pre-processor/diff

All these changes were applied to PlatformIO's tool-scons package with new version 3.20401.1.

Please report if it works.

Member

ivankravets commented Nov 28, 2016

This is a problem of SCons engine. The SCons C/C++ Preprocessor currently is disabled. I refactored disabled code and fixed all issues. See my PR to SCons https://bitbucket.org/scons/scons/pull-requests/380/improvements-for-scons-c-c-pre-processor/diff

All these changes were applied to PlatformIO's tool-scons package with new version 3.20401.1.

Please report if it works.

ivankravets added a commit that referenced this issue Nov 29, 2016

@blackketter

This comment has been minimized.

Show comment
Hide comment
@blackketter

blackketter Nov 30, 2016

Sorry, @ivankravets, still not working for me, alas. The example doesn't find the appropriate libraries automatically and if I include them via lib_deps it automatically finds the wrong library, even though it's defined out..

blackketter commented Nov 30, 2016

Sorry, @ivankravets, still not working for me, alas. The example doesn't find the appropriate libraries automatically and if I include them via lib_deps it automatically finds the wrong library, even though it's defined out..

@ivankravets ivankravets reopened this Nov 30, 2016

@ivankravets

This comment has been minimized.

Show comment
Hide comment

ivankravets added a commit that referenced this issue Nov 30, 2016

@blackketter

This comment has been minimized.

Show comment
Hide comment
@blackketter

blackketter Nov 30, 2016

Apologies again @ivankravets, updated to the latest development version and we're a bit closer but the sample still requires the line:

lib_deps = Adafruit ILI9341, Adafruit GFX Library

under [env:d1_mini] to build. I don't need any other lib_deps or lib_ignore directives.

blackketter commented Nov 30, 2016

Apologies again @ivankravets, updated to the latest development version and we're a bit closer but the sample still requires the line:

lib_deps = Adafruit ILI9341, Adafruit GFX Library

under [env:d1_mini] to build. I don't need any other lib_deps or lib_ignore directives.

@ivankravets

This comment has been minimized.

Show comment
Hide comment
@ivankravets

ivankravets Nov 30, 2016

Member

Thanks! I can't imagine how we are going to release this PlatformIO 3.2 with these new changes 😢

Member

ivankravets commented Nov 30, 2016

Thanks! I can't imagine how we are going to release this PlatformIO 3.2 with these new changes 😢

@ivankravets ivankravets reopened this Nov 30, 2016

@blackketter

This comment has been minimized.

Show comment
Hide comment
@blackketter

blackketter Nov 30, 2016

Dude, I'm really sorry. I can make it work with lib_deps/lib_ignore so I wouldn't hold up a release for this issue.

blackketter commented Nov 30, 2016

Dude, I'm really sorry. I can make it work with lib_deps/lib_ignore so I wouldn't hold up a release for this issue.

@ivankravets

This comment has been minimized.

Show comment
Hide comment
@ivankravets

ivankravets Nov 30, 2016

Member

Hm... I've jsut re-tested again and all works very well!

  1. Ok, I've just released PlatformIO 3.2 Beta 4. Please make pio upgrade
  2. Check again.
Member

ivankravets commented Nov 30, 2016

Hm... I've jsut re-tested again and all works very well!

  1. Ok, I've just released PlatformIO 3.2 Beta 4. Please make pio upgrade
  2. Check again.
@ivankravets

This comment has been minimized.

Show comment
Hide comment
@ivankravets

ivankravets Nov 30, 2016

Member

There 2 options:

  • Install all dependencies globally pio lib -g install
  • Use lib_deps and PIO will maintain deps directly for this project. RECOMMENDED WAY.

See my output where I have global deps:

[Wed Nov 30 19:27:43 2016] Processing teensy31 (platform: teensy, build_flags: -O  -D TEENSY31=1 -UUSB_SERIAL -DARDUINO_ARCH_AVR -DUSB_SERIAL_HID -DLAYOUT_US_ENGLISH -I./src, board: teensy31, board_f_cpu: 96000000, framework: arduino)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option
Collected 98 compatible libraries
Looking for dependencies...
Library Dependency Graph
|-- <SPI> v1.0
|-- <ILI9341_t3>
|   |-- <SPI> v1.0
Linking .pioenvs/teensy31/firmware.elf
Building .pioenvs/teensy31/firmware.hex
Calculating size .pioenvs/teensy31/firmware.elf
text	   data	    bss	    dec	    hex	filename
25884	    704	   3864	  30452	   76f4	.pioenvs/teensy31/firmware.elf
================================================================================ [SUCCESS] Took 2.08 seconds ================================================================================

[Wed Nov 30 19:27:45 2016] Processing d1_mini (platform: espressif8266, board: d1_mini, framework: arduino)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option
Collected 36 compatible libraries
Looking for dependencies...
Library Dependency Graph
|-- <SPI> v1.0
|-- <Adafruit ILI9341> v1.0.2
|   |-- <Adafruit GFX Library> v1.1.5
|   |-- <SPI> v1.0
Calculating size .pioenvs/d1_mini/firmware.elf
text	   data	    bss	    dec	    hex	filename
227020	   2248	  29464	 258732	  3f2ac	.pioenvs/d1_mini/firmware.elf
================================================================================ [SUCCESS] Took 1.41 seconds ================================================================================

========================================================================================= [SUMMARY] =========================================================================================
Environment teensy31	[SUCCESS]
Environment d1_mini 	[SUCCESS]

As you can see, no lib_ignore or other hooks. All works very good.

P.S: This is forced me to detach from official SCons release. I've made SCons PR #380 and don't know will it be merged and when. Nevertheles, PlatformIO has own package managment system where I can have own patched packages. I did the same with SCon. Now PIO 3.2 depends on patched SCons 2.5.1 and package version is 3.20501.1.

Please check that you have PlatformIO 3.2.0b4.

Member

ivankravets commented Nov 30, 2016

There 2 options:

  • Install all dependencies globally pio lib -g install
  • Use lib_deps and PIO will maintain deps directly for this project. RECOMMENDED WAY.

See my output where I have global deps:

[Wed Nov 30 19:27:43 2016] Processing teensy31 (platform: teensy, build_flags: -O  -D TEENSY31=1 -UUSB_SERIAL -DARDUINO_ARCH_AVR -DUSB_SERIAL_HID -DLAYOUT_US_ENGLISH -I./src, board: teensy31, board_f_cpu: 96000000, framework: arduino)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option
Collected 98 compatible libraries
Looking for dependencies...
Library Dependency Graph
|-- <SPI> v1.0
|-- <ILI9341_t3>
|   |-- <SPI> v1.0
Linking .pioenvs/teensy31/firmware.elf
Building .pioenvs/teensy31/firmware.hex
Calculating size .pioenvs/teensy31/firmware.elf
text	   data	    bss	    dec	    hex	filename
25884	    704	   3864	  30452	   76f4	.pioenvs/teensy31/firmware.elf
================================================================================ [SUCCESS] Took 2.08 seconds ================================================================================

[Wed Nov 30 19:27:45 2016] Processing d1_mini (platform: espressif8266, board: d1_mini, framework: arduino)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option
Collected 36 compatible libraries
Looking for dependencies...
Library Dependency Graph
|-- <SPI> v1.0
|-- <Adafruit ILI9341> v1.0.2
|   |-- <Adafruit GFX Library> v1.1.5
|   |-- <SPI> v1.0
Calculating size .pioenvs/d1_mini/firmware.elf
text	   data	    bss	    dec	    hex	filename
227020	   2248	  29464	 258732	  3f2ac	.pioenvs/d1_mini/firmware.elf
================================================================================ [SUCCESS] Took 1.41 seconds ================================================================================

========================================================================================= [SUMMARY] =========================================================================================
Environment teensy31	[SUCCESS]
Environment d1_mini 	[SUCCESS]

As you can see, no lib_ignore or other hooks. All works very good.

P.S: This is forced me to detach from official SCons release. I've made SCons PR #380 and don't know will it be merged and when. Nevertheles, PlatformIO has own package managment system where I can have own patched packages. I did the same with SCon. Now PIO 3.2 depends on patched SCons 2.5.1 and package version is 3.20501.1.

Please check that you have PlatformIO 3.2.0b4.

@ivankravets

This comment has been minimized.

Show comment
Hide comment
@ivankravets

ivankravets Nov 30, 2016

Member

What is interesting, this huge refactoring to PIO LDF works 2-3x faster than previous PlatformIO release!!! 😊

Member

ivankravets commented Nov 30, 2016

What is interesting, this huge refactoring to PIO LDF works 2-3x faster than previous PlatformIO release!!! 😊

@blackketter

This comment has been minimized.

Show comment
Hide comment
@blackketter

blackketter Nov 30, 2016

Works as long as I have that single lib_deps line (I guess the auto-detect of some libraries isn't reliable.)

I'll continue to use lib_deps as necessary.

Keep up the AWESOME work!

blackketter commented Nov 30, 2016

Works as long as I have that single lib_deps line (I guess the auto-detect of some libraries isn't reliable.)

I'll continue to use lib_deps as necessary.

Keep up the AWESOME work!

@ivankravets

This comment has been minimized.

Show comment
Hide comment
@ivankravets

ivankravets Nov 30, 2016

Member

Works as long as I have that single lib_deps line (I guess the auto-detect of some libraries isn't reliable.)

  1. Please comment all lib deps in config
  2. Please remove .piolbdeps from project
  3. Install dependencies to global storage via pio lib -g install "ILI9341_t3" "Adafruit ILI9341" "Adafruit GFX Library"

Does it work now?

Member

ivankravets commented Nov 30, 2016

Works as long as I have that single lib_deps line (I guess the auto-detect of some libraries isn't reliable.)

  1. Please comment all lib deps in config
  2. Please remove .piolbdeps from project
  3. Install dependencies to global storage via pio lib -g install "ILI9341_t3" "Adafruit ILI9341" "Adafruit GFX Library"

Does it work now?

@blackketter

This comment has been minimized.

Show comment
Hide comment
@blackketter

blackketter Nov 30, 2016

Yes, it works perfectly.

I agree, lib_deps is preferred to global installs. :)

What I'm surprised by is how ILI9341_t3 is automatically detected and installed but the Adafruit libraries are not. Best to explicitly specify libraries with lib_deps.

blackketter commented Nov 30, 2016

Yes, it works perfectly.

I agree, lib_deps is preferred to global installs. :)

What I'm surprised by is how ILI9341_t3 is automatically detected and installed but the Adafruit libraries are not. Best to explicitly specify libraries with lib_deps.

@ivankravets

This comment has been minimized.

Show comment
Hide comment
@ivankravets

ivankravets Nov 30, 2016

Member

What I'm surprised by is how ILI9341_t3 is automatically detected and installed but the Adafruit libraries are not

What do you mean? Issue with Adafruit GFX Library?

Member

ivankravets commented Nov 30, 2016

What I'm surprised by is how ILI9341_t3 is automatically detected and installed but the Adafruit libraries are not

What do you mean? Issue with Adafruit GFX Library?

@blackketter

This comment has been minimized.

Show comment
Hide comment
@blackketter

blackketter Nov 30, 2016

AHA! I see. I was confused as to why the ILI9341_t3 library didn't need to be specified but the Adafruit ones did.

The ILI9341_t3 library is part of framework-arduinoteensy and is automatically available, though not visible via pio lib list

Everything is working as expected now.

blackketter commented Nov 30, 2016

AHA! I see. I was confused as to why the ILI9341_t3 library didn't need to be specified but the Adafruit ones did.

The ILI9341_t3 library is part of framework-arduinoteensy and is automatically available, though not visible via pio lib list

Everything is working as expected now.

@ivankravets

This comment has been minimized.

Show comment
Hide comment
@ivankravets

ivankravets Nov 30, 2016

Member

How about to remove extra libraries from Arduino framework for Teensy? I don't know why I added them. I propose to keep only basic libraries such as SPI, Wire, etc. WDYT?

Member

ivankravets commented Nov 30, 2016

How about to remove extra libraries from Arduino framework for Teensy? I don't know why I added them. I propose to keep only basic libraries such as SPI, Wire, etc. WDYT?

@blackketter

This comment has been minimized.

Show comment
Hide comment
@blackketter

blackketter Nov 30, 2016

The teensyduino installer from @PaulStoffregen includes all these libraries so teensy developers are used to having them all pre-installed and samples should "just work".

This bug was really about a problem with the LDF parser which is working awesome now. I wouldn't mess with the framework-arduinoteensy unless there's a specific issue.

My confusion was around not knowing which libraries I had installed, since pio lib [-g] list didn't tell me about the ones that came with the framework. That might be worthy of an enhancement

blackketter commented Nov 30, 2016

The teensyduino installer from @PaulStoffregen includes all these libraries so teensy developers are used to having them all pre-installed and samples should "just work".

This bug was really about a problem with the LDF parser which is working awesome now. I wouldn't mess with the framework-arduinoteensy unless there's a specific issue.

My confusion was around not knowing which libraries I had installed, since pio lib [-g] list didn't tell me about the ones that came with the framework. That might be worthy of an enhancement

@ivankravets

This comment has been minimized.

Show comment
Hide comment
@ivankravets

ivankravets Dec 8, 2016

Member

PlatformIO 3.2 has been released. Please note that you need to change lib_ldf_mode to chain+ to enable advanced C Preprocessor. See docs

Member

ivankravets commented Dec 8, 2016

PlatformIO 3.2 has been released. Please note that you need to change lib_ldf_mode to chain+ to enable advanced C Preprocessor. See docs

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