-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
esp32: Add support for IDF version v5.2. #13775
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
Conversation
This might be possible with some cmake code like this: if(IDF_VERSION_MINOR >= 2)
set(SDKCONFIG_DEFAULTS
${SDKCONFIG_DEFAULTS}
boards/sdkconfig.idf52
)
else()
set(SDKCONFIG_DEFAULTS
${SDKCONFIG_DEFAULTS}
boards/sdkconfig.idf50
)
endif() |
91fb390
to
4db0f16
Compare
Ah, yes, that does (with some changes) work nicely. I ended up implementing it slightly different: Instead of adding always the extra configuration file in the CMake code, I opted to make a specific variable Pushed an extra commit that implements this change. I think that makes it more explicit and clear for developers which Also updated MR description and added explicit commit that adds compatibility with v5.0.5 and v5.0.6, since that was just a matter of setting the correct version range in an existing check. |
I agree, I like this approach of being explicit. |
752a39b
to
b7279e4
Compare
Signed-off-by: Daniël van de Giessen <daniel@dvdgiessen.nl>
Signed-off-by: Daniël van de Giessen <daniel@dvdgiessen.nl>
The new IDF v5.2 deprecated the task cleanup callback we use, so support for the new option has been implemented in the previous commit. This also requires a change in the sdkconfig, via a new variable ${SDKCONFIG_IDF_VERSION_SPECIFIC} which is used in all mpconfigboard.cmake files to include an extra sdkconfig file based on the IDF version in use. Signed-off-by: Daniël van de Giessen <daniel@dvdgiessen.nl>
I had thought that MIcroPython was no longer going to support more than a single version of the IDF at any given time. It would eliminate all of the IDF version checking that needs to be done.... what happened there? This also doesn't add support for 5.2 properly. There is a bunch of things that have been deprecated between 5.1 and 5.2 and yet those things are still being used. I2C is one of the very large changes.. There are others as well. This is the bare bones minimum in order to get IDF5.2 to run. Doing this same kind of thing is what caused MicroPython to lag behind with the ESP-IDF versions in the past. In order to keep up with version changes anything that is deprecated needs to be removed/changed to make it fully compliant with the version of the IDF that is being supported. |
@kdschlosser I'd say this is a case of don't let perfect get in the way of good enough. While it'd be nice to not need any version checking in the code and stick to just one version, the "latest" version isn't always stable / tested for all platforms so there's a reason to keep it build-able at a tested working version. The reality is I'm sure there's not enough paid staff time to perfectly keep up with all the idf changes at all times. That's where community submissions like this come in. I for one am glad this was merged as is because it let me progress with the C6 support. If this MR was blocked until every change in IDF was accounted for and thoroughly tested it likely would have never been finished! That being said, it would be helpful to have a checklist somewhere of all the remaining changed / deprecated APIs that eventually will need updating, though even that requires someone to spend the time going through all the changelogs for each version change. This is the problem with trying to keep up with a platform as fast moving as the idf where there's a lot of api and platform changes. |
@DvdGiessen could you check if master still builds with v5.2.1 for you? It's fine for S3 etc but
|
Correct. I did see those changes and looked into how much work it would be to rework the current code to use those new interfaces, and estimated doing so would result in a lot more work (both for me in rewriting a lot of code, and for the maintainers needing to review a much larger PR). The old interfaces, although marked as deprecated, are not removed and work fine (they might not contain the improvements made in the newer interfaces, but it's usually not going to be worse than things already were by being on an older IDF version all together). So while indeed it might indeed be nice to move to these newer interfaces, I concluded that shouldn't delay or block me from making an PR for the smaller changes needed to run on the lastest IDF (which aside from these big interface changes also includes a lot of other stuff that is useful to have in the meantime).
Hm, the ESP32_GENERIC does indeed seem to slightly overflow the static IRAM space:
|
This MR adds support for building the ESP32 port with ESP-IDF v5.2. I've been using these for a little bit the past few weeks on three different ESP32 variants and it seems to work well so far. These changes are implemented in the second commit of this MR.
The first commit in this MR makes the build work with IDF v5.0.5 and later. Included here since all that needed was a small touch-up on the version ranges in existing version checks.
The new IDF v5.2 deprecated the task cleanup callback we use, so I implemented support for the new option. This also requires a change in the
sdkconfig
, for the third and last commit in this MR implements an new variable${SDKCONFIG_IDF_VERSION_SPECIFIC}
which is used in allmpconfigboard.cmake
files to include an extrasdkconfig
file based on the IDF version in use.EDIT: Added IDF-dependant
sdkconfig
file as suggested by @dpgeorge.