Skip to content
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

framework=espidf, arduino isn't setting flags early enough for the Libraray Dependency Finder #1080

Closed
j4m3s opened this issue Apr 20, 2023 · 8 comments

Comments

@j4m3s
Copy link

j4m3s commented Apr 20, 2023

In trying to write a concise title for the issue I have made an assumption based on my investigation.

TLDR:
The Library Dependency Finder creates a different dependency tree (for code in a subdir in the lib folder) when it's run with framework = espidf, arduino vs framework = arduino, where an include is conditional upon the ARDUINO build flag (using lib_ldf_mode = chain+ and even when lib_compat_mode = off).

So:

Given a folder structure of:

|_ lib
  |_A
    |_ src
      |_libAHeader.h
|_ src
  |_ main.cpp

And the libAHeader.h file containing a conditional include as follows:

#ifdef ARDUINO
#    include <FastLED.h>
#endif

const colour = CRGB{255,255,255};

The dependency tree for A includes FastLED, so including the libAHeader.h in src/main.cpp succeeds, when compiling with framework = arduino.
The dependency tree for A does not include FastLED when compiling with framework = espidf, arduino so the compile fails

I suspect this is because the ARDUINO flag is not set before the dependency finder runs for framework = espidf, arduino, but it is set beforehand for framework = arduino? It's certainly set by the time the actual compilation happens. I expect the same applies to ESP32 and ARDUINO_ARCH_ESP32?

@j4m3s
Copy link
Author

j4m3s commented Apr 20, 2023

As a workaround I've created a library.json file in lib/A that hard-codes the dependencies, which seems to be working. It's not ideal though as this isn't a proper library it's just the way the code is organised to allow tests to be separated between native and MCU.

@valeros valeros closed this as completed in da13338 May 3, 2023
@valeros
Copy link
Member

valeros commented May 3, 2023

Hi @j4m3s thanks for reporting, I believe I've found the reason why it doesn't work. Could you please revert back your workaround with library.json and compile your project using the latest platform revision from the development branch (a Git client is requried)? For example:

[env:esp32dev]
platform = https://github.com/platformio/platform-espressif32.git
framework = arduino, espidf
board = esp32dev
lib_ldf_mode = chain+

@j4m3s
Copy link
Author

j4m3s commented May 3, 2023

Hi @valeros!

Thanks for looking at this. Sadly though, no it hasn't fixed it. It did pull down the new version of the platform/ framework too:

Platform Manager: espressif32@6.2.0+sha.da13338 has been installed!
Tool Manager: Installing platformio/framework-arduinoespressif32 @ ~3.20008.0
Downloading  [####################################]  100%          
Unpacking  [####################################]  100%          
Tool Manager: framework-arduinoespressif32@3.20008.0 has been installed!

@valeros
Copy link
Member

valeros commented May 3, 2023

@j4m3s Mind sharing a minimal project to reproduce the issue?

@valeros valeros reopened this May 3, 2023
@j4m3s
Copy link
Author

j4m3s commented May 3, 2023

The project this is hitting is far from minimal but I'll create a small one, NP :)

@j4m3s
Copy link
Author

j4m3s commented May 3, 2023

Interestingly I've yet to reproduce the issue in the simple project, but I don't understand why.

Running pio run -v when lib_ldf_mode=chain results in a dependency graph that shows A depends on FastLED:

|-- A (License: Unknown, Path: pio-issue-1080/lib/A)
|   |-- FastLED @ 3.5.0 (License: MIT, Path: pio-issue-1080/.pio/libdeps/esp32dev/FastLED)
|-- B (License: Unknown, Path: pio-issue-1080/lib/B)
|   |-- A (License: Unknown, Path: pio-issue-1080/lib/A)
|   |   |-- FastLED @ 3.5.0 (License: MIT, Path: pio-issue-1080/.pio/libdeps/esp32dev/FastLED)
|-- FastLED @ 3.5.0 (License: MIT, Path: pio-issue-1080/.pio/libdeps/esp32dev/FastLED)

When lib_ldf_mode=chain+, LDF fails to find the dependency from A on FastLED which suggests that the ARDUINO flag that's used in an #ifdef wrapping the include isn't set:

|-- A (License: Unknown, Path: pio-issue-1080/lib/A)
|-- B (License: Unknown, Path: pio-issue-1080/lib/B)
|   |-- A (License: Unknown, Path: pio-issue-1080/lib/A)
|-- FastLED @ 3.5.0 (License: MIT, Path: pio-issue-1080/.pio/libdeps/esp32dev/FastLED)

but yet library A compiles successfully. Given that the LDF doesn't know that A depends on FastLED, surely the compile should fail? It does on my bigger project :)

Based on my understanding of how the LDF works I would have expected it to compile successfully when lib_ldf_mode=chain but fail when it's chain+.

valeros added a commit that referenced this issue May 8, 2023
@valeros
Copy link
Member

valeros commented May 8, 2023

@j4m3s I've pushed a possible fix, could you please pull the latest revision 2988da9 and try again?

@j4m3s
Copy link
Author

j4m3s commented May 8, 2023

Nice work @valeros, you've cracked it! Works great now in both the minimal example and the big project, thank you! I have to admit that's a big relief - I had no idea where to look next to understand why the minimal project worked but the big complex one didn't :)

@j4m3s j4m3s closed this as completed May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants