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

Compile under esp-idf new build sysytem #1562

Closed
wants to merge 7 commits into from
Closed

Conversation

qt1
Copy link
Contributor

@qt1 qt1 commented May 19, 2021

This is a non intrusive addition to CMmakeLists.txt that enables using as esp-idf component with the new build system.

For more details please see:

espressif/esp-idf#7024

@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.042% when pulling 66a8e9f on qt1:master into 68082e6 on bblanchon:6.x.

@bblanchon
Copy link
Owner

Hi @qt1,

Thank you very much for this PR 👍

Can you confirm that you tested the patch and that it works as expected?
Do you think it's possible to include the esp-idf component build in the continuous integration?
If so, can you add it yourself or give me some guidance?

Best regards,
Benoit

@qt1
Copy link
Contributor Author

qt1 commented May 19, 2021

Hi,

I can confirm that the errors from the build system are resolved.

I did not test the code itself as there are other, unrelated, errors to the build.
However since this is a header only component I assume that everything that used to work will continue to work.

Unfortunately I am not familiar with CI. Maybe @projectgus , who actually resolved this issue, can help.

@bblanchon
Copy link
Owner

I'll wait until you can confirm that this patch is fully functional, useful, and necessary.
The library is already bloated with many platform-specific hacks that I must maintain just in case; I prefer to think twice before adding a new one.

Moreover, I don't understand the motivation since ArduinoJson already implements good practices for CMake (as far as I know), supporting FetchContent and find_library (see How to use ArduinoJson with CMake?).

@andreaskuster
Copy link
Contributor

andreaskuster commented Jun 27, 2021

@bblanchon
I added a ci build pipeline that compiles all examples in ArduinoJson/examples in the esd-idf environment (with ArduinoJson as a component). Please approve the execution such that i can test its functionality (it works offline, but that is never a guarantee).

Functionality: It works locally, and we will soon see it working with the new ci pipeline step added.

Usefullness & Necessary: Well, I would like to see this functionality added. It is the way of handling ESP32 + Arduino projects with proper Cmake (i.e. without platformio or arduino IDE), which in my opinion is a lot nicer. I would use this functionality in here: https://github.com/andreaskuster/ESP32-WebCam in conjunction with the ESPAsyncWebServer.

About the motivation of using this extra build structure instead of the standard CMake find_library functionality: I totally agree with you that I do not understand why they cannot use the standard find_package functionality. I guess they will have their reasons for that and there is not much we can change. Here you can find the documentation about this component build system from their official documentation: https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/build-system.html#minimal-component-cmakelists

bblanchon pushed a commit that referenced this pull request Jun 28, 2021
@bblanchon bblanchon added this to the v6.18.1 milestone Jun 30, 2021
@bblanchon bblanchon closed this in cb2c029 Jun 30, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants