esp32: Remove support for ESP-IDF <5.2 #16128
Conversation
|
Sorry andrew and damien, I thought I'd cancelled the review request before opening (as realised it's still a draft). |
|
You (and others) did a lot of testing of IDF 5.2.2 in #15523, so I'm happy to move forward with this as the minimum supported version. Also, firmware size was reduced a lot, so that means users won't struggle to upgrade if they are short on flash. Since we will make a v1.24.1 (and possibly further patch releases on that v1.24 branch), we can have v1.24 as the last MicroPython version to support IDF < 5.2.2, and users can stick to that release branch if necessary. |
4dc88b6 to
c92538a
Compare
|
Rebased now the linked PRs are merged, this PR now includes a revert of #16127. Re-ran unit tests on S3 and verified Wi-Fi is working on C3. |
ports/esp32/mpthreadport.c
Outdated
| @@ -41,7 +41,7 @@ | |||
| #define MP_THREAD_DEFAULT_STACK_SIZE (MP_THREAD_MIN_STACK_SIZE + MICROPY_STACK_CHECK_MARGIN) | |||
| #define MP_THREAD_PRIORITY (ESP_TASK_PRIO_MIN + 1) | |||
|
|
|||
| #if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 2, 0) && !CONFIG_FREERTOS_ENABLE_STATIC_TASK_CLEAN_UP | |||
| #if !CONFIG_FREERTOS_ENABLE_STATIC_TASK_CLEAN_UP | |||
There was a problem hiding this comment.
Is this if logic needed at all now that this option is always disabled?
There was a problem hiding this comment.
Oh yeah, have pulled all this out!
c92538a to
7b8fa01
Compare
|
Have rebased onto the v5.3 changes, ran one lot of unit tests on C6 and also re-ran the before/after binary size comparison on the boards mentioned in the PR description. |
Specifically, remove all conditional compilation for these earlier versions and change the idf_component.yml specifiers to require >=5.2.0. Signed-off-by: Angus Gratton <angus@redyak.com.au>
This reverts commit 27279e6 (plus removes some additional references to the SDKCONFIG_IDF_VERSION_SPECIFIC CMake variable.) Relevant sdkconfig options are added into sdkconfig.base now that IDF >=5.2.0 is required. Signed-off-by: Angus Gratton <angus@redyak.com.au>
Now we only support the case of !CONFIG_FREERTOS_ENABLE_STATIC_TASK_CLEAN_UP, can simplify the cleanup code. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
7b8fa01 to
d4d1d47
Compare
Great, thanks for re-testing! Now merged. |
Summary
When we merged support for ESP-IDF V5.2.2 in #15523, we discussed dropping support for older versions after the new version was bedded in. (Related Discord discussion here). This is a draft for that PR!
In two commits, one to remove all the #ifdef-ed code and one that reverts 27279e6 from #13775 (adding a 5.2-specific sdkconfig variable).
Testing
Trade-offs and Alternatives
SDKCONFIG_IDF_VERSION_SPECIFICvariable from esp32: Add support for IDF version v5.2. #13775 in case we need it in the future, but I reckon it's better to revert it here and we can always un-revert it later if a need arises for it.