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

Advertise ESP-IDF compatibility in library.json #1490

Closed
wants to merge 3 commits into from

Conversation

maxgerhardt
Copy link

@maxgerhardt maxgerhardt commented Feb 2, 2021

Per PlatformIO forum thread this library can also be used with ESP-IDF. It's just that PlatformIO refuses by default to use this library in a project that has framework = espidf and not arduino because the library.json only advertises arduino compatibility and a user needs to do lib_compat_mode = off to bypass the compatibility check.

Per docs I've expanded the library.json.

You might also want to expand the detection code for

// Small or big machine?
#ifndef ARDUINOJSON_EMBEDDED_MODE
#if defined(ARDUINO) /* Arduino*/ \
|| defined(__IAR_SYSTEMS_ICC__) /* IAR Embedded Workbench */ \
|| defined(__XC) /* MPLAB XC compiler */ \
|| defined(__ARMCC_VERSION) /* Keil ARM Compiler */ \
|| defined(__AVR) /* Atmel AVR8/GNU C Compiler */
#define ARDUINOJSON_EMBEDDED_MODE 1
#else
#define ARDUINOJSON_EMBEDDED_MODE 0
#endif
#endif

So that the macro ends up having the value 1, the same as in Arduino. The check #if defined(IDF_VER) might e.g. be used for that (defined during build process -DIDF_VER=\"3.40200.210118\"), or check on __xtensa__ (compiler does #define __xtensa__ 1). I left that out in this PR because I'm not 100% sure on what you want this macro value to be in an ESP-IDF environment.

@coveralls
Copy link

coveralls commented Feb 3, 2021

Coverage Status

Coverage increased (+0.06%) to 98.329% when pulling 9ef98f0 on maxgerhardt:patch-1 into d2174d1 on bblanchon:6.x.

@bblanchon
Copy link
Owner

Hi @maxgerhardt,

Thank you very much for this PR 👍

After reading the documentation you sent, I think the right framework is "*".
I think you're right about the ARDUINOJSON_EMBEDDED_MODE macro; I would go for IDF_VER.

Can you update your PR according to this?
Also, please add a line in the CHANGELOG.

Thanks!

Best regards,
Benoit

@maxgerhardt
Copy link
Author

Hmm I'm not sure if it should advertise * if I've not tested it. I'll do a test on Arduino, mbed-os, ESP-IDF, Zephyr etc with different boards to be sure.

If it works, I'll modify this PR.

@@ -1,6 +1,6 @@
#!/bin/sh -eux

pip install --user platformio
pip3 install --user platformio
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should revert this change; I fixed the CI in 3975b07

This reverts commit afabcf8.
@bblanchon
Copy link
Owner

bblanchon commented Feb 5, 2021

The new CI issue should be fixed by 769e844.
Don't forget to squash your commits ;-)

@bblanchon
Copy link
Owner

I'd like to include this change in version 6.17.3.
Do you know when you'll have time to run the tests?

@maxgerhardt
Copy link
Author

Eh unfortunately not before tuesday :/

@bblanchon bblanchon closed this in 21f7b90 Feb 15, 2021
@bblanchon
Copy link
Owner

I published ArduinoJson 6.17.3 with frameworks set to *.

@maxgerhardt, I think we still have to set ARDUINOJSON_EMBEDDED_MODE to 1 when IDF_VER is detected.
Can you open a new PR?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants