-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
ESP32: support ESP-IDF v5.0.4, v5.1.2 #12952
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
@andrewleech Maybe it will interest you. |
Good work, I started idf 5.1 compatibility here too, maybe you'd like to compare: #11869 I thought ADC also needed updating, or has that already been updated? |
The CI setup is configured here: Line 121 in a00c9d5
You could change the version there to show that this MR builds with 5.1 I think there's a desire to not build multiple versions in CI, better to pick one compatible version and just focus on that, that would be my preference at least. |
I've been using the current ESP-IDF 5.2 preview on the ESP32-S3. No issues so far with BLE. So it's probably related to the C6. |
13dfc2f
to
7b551e3
Compare
I also agree that it's better to pick one compatible version and focus on it. |
7b551e3
to
c140aea
Compare
The deprecated warning on the pm_config struct caused by ESP-IDF commit 8f415a7f448d0438091f2456c173b5e32a829dd1 can be fixed easily: diff --git a/ports/esp32/modmachine.c b/ports/esp32/modmachine.c
index 1e83935a8..35b1ad0fe 100644
--- a/ports/esp32/modmachine.c
+++ b/ports/esp32/modmachine.c
@@ -78,15 +80,7 @@ STATIC mp_obj_t machine_freq(size_t n_args, const mp_obj_t *args) {
mp_raise_ValueError(MP_ERROR_TEXT("frequency must be 20MHz, 40MHz, 80Mhz, 160MHz or 240MHz"));
#endif
}
- #if CONFIG_IDF_TARGET_ESP32
- esp_pm_config_esp32_t pm;
- #elif CONFIG_IDF_TARGET_ESP32C3
- esp_pm_config_esp32c3_t pm;
- #elif CONFIG_IDF_TARGET_ESP32S2
- esp_pm_config_esp32s2_t pm;
- #elif CONFIG_IDF_TARGET_ESP32S3
- esp_pm_config_esp32s3_t pm;
- #endif
+ esp_pm_config_t pm;
pm.max_freq_mhz = freq;
pm.min_freq_mhz = freq;
pm.light_sleep_enable = false; |
713d335
to
6652fc8
Compare
There's another change in coming in 5.3 where support for the ESP32-C6 LP UART changes some of the API for all uarts. Like in 57b3f6b, but there's another I've already found too. This PR is for 5.1, but maybe 5.2 will be out before it ever gets merged. Maybe it should be possible to add support for ESP-IDF versions sooner? The further behind one gets the harder it is to catch up. Thankfully, when using 5.3-dev there were only a few things to fix to build, so I fixed them. If it had been a miles long list that filled the console scrollback, I'd probably just give up because I don't have time to do all that now, and not fix anything. |
8df96d3
to
1a5b899
Compare
It's not released. Look at the master branch. They've already made a v5.3-dev tag. |
1a5b899
to
6178740
Compare
ports/esp32/machine_dac.c
Outdated
@@ -32,23 +32,33 @@ | |||
#include "modmachine.h" | |||
|
|||
#if MICROPY_PY_MACHINE_DAC | |||
#if SOC_DAC_SUPPORTED |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding this #if, I suggest changing mpconfigport.h
to:
#ifndef MICROPY_PY_MACHINE_DAC
#define MICROPY_PY_MACHINE_DAC (SOC_DAC_SUPPORTED)
#endif
Then, we can remove all the #define MICROPY_PY_MACHINE_DAC (0)
from the board specific mpconfigboard.h
files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see that this has been fixed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry. After "Done" was terrible rebase...
Thanks @IhorNehrutsa for working on this, it's a good step forward. However, building with this new IDF version the firmware size has increased by about 60k-70k! So now the D2WD (and probably also OTA) board variants no longer fit in their flash partition. Furthermore, it looks like the available RAM has decreased as well. EDIT: the following is wrong! See next comment for updated and correct RAM numbers. For a standard ESP32, on current firmware running we get:
With this PR:
Comparing the heap regions:
So in total we previously had 135536 total available (max 110592 contiguous), but now we have only 99772 (49152 max contiguous). That's a loss of 35764 bytes of heap, which is a huge amount considering the standard ESP32 doesn't have that much to begin with. I'm happy to merge this PR so that we can support newer versions of the IDF, but we should stick to building with v5.0.2 (or move to v5.0.4) until we can reclaim some of that flash and RAM. |
The above analysis of free RAM is wrong, I had already started WiFi on the v5.1.2 version. Here is a correct analysis. Current master on ESP32_GENERIC:
With this PR:
That's after a fresh With this PR there's about 1k less due to the region 87144 shrinking down to 86184. But that's about it. The big region of 113840 bytes is unchanged. When enabling WiFi and connecting to an access point the memory use is: Current master:
This PR:
So in this case this PR uses about 5k extra IDF heap. After connecting to WiFi and running a few requests, both HTTP and HTTPS, the memory use is: Current master:
This PR:
Again, this PR uses about 5k extra IDF heap (which is not due to the requests, just WiFi connection). |
6178740
to
68771b3
Compare
This PR with IDFv5.0.4
@dpgeorge I suggest switching MicroPython master to the IDF v5.0.4 |
8fa053f
to
271f8b6
Compare
271f8b6
to
a7a2251
Compare
ports/esp32/modmachine.c
Outdated
) { | ||
#if CONFIG_IDF_TARGET_ESP32C3 | ||
mp_raise_ValueError(MP_ERROR_TEXT("frequency must be 20MHz, 40MHz, 80Mhz or 160MHz")); | ||
STATIC mp_obj_t machine_freq(size_t n_args, const mp_obj_t *args) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be two functions: mp_machine_set_freq()
, and mp_machine_get_freq()
. The CI is failing at the moment because if this error.
Yes I think that's a good idea. Please can you also update the |
Signed-off-by: IhorNehrutsa <Ihor.Nehrutsa@gmail.com>
Signed-off-by: IhorNehrutsa <IhorNehrutsa@gmail.com>
Signed-off-by: IhorNehrutsa <Ihor.Nehrutsa@gmail.com>
Signed-off-by: IhorNehrutsa <IhorNehrutsa@gmail.com>
Signed-off-by: IhorNehrutsa <Ihor.Nehrutsa@gmail.com>
Co-Authored-By: Trent Piepho <35062987+xyzzy42@users.noreply.github.com> Signed-off-by: IhorNehrutsa <Ihor.Nehrutsa@gmail.com>
Signed-off-by: IhorNehrutsa <Ihor.Nehrutsa@gmail.com>
esp32/DAC: Use SOC_DAC_SUPPORTED as MICROPY_PY_MACHINE_DAC. Signed-off-by: IhorNehrutsa <Ihor.Nehrutsa@gmail.com>
Signed-off-by: IhorNehrutsa <Ihor.Nehrutsa@gmail.com>
79f07ef
to
941bf67
Compare
Signed-off-by: IhorNehrutsa <Ihor.Nehrutsa@gmail.com>
Signed-off-by: IhorNehrutsa <Ihor.Nehrutsa@gmail.com>
Code size report:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #12952 +/- ##
=======================================
Coverage 98.36% 98.36%
=======================================
Files 159 159
Lines 20989 20989
=======================================
Hits 20646 20646
Misses 343 343 ☔ View full report in Codecov by Sentry. |
Has anyone tried building for the C6 with these changes? I'll do so when I get a chance but it may be a day or two away... |
I get an error
~/micropython/ports/esp32$ python ~/.espressif/python_env/idf5.0_py3.11_env/bin/mpy-cross --version
micropython/tools/mpy-tool.py
mpyfiles.rst
|
If you're using the latest master version of MicroPython then you need to build and use |
@dpgeorge Thanks, solved. |
Backward compatibility to the v5.0.2 is saved.
@dpgeorge @jimmo
Is it possible to add ESP-IDF v5.1.2 to the MicroPython CI process?
EDITED:
I suggest switching MicroPython master to the IDF v5.0.4