Skip to content

esp32: switch esp-idf to v4.2, add CI, support MICROPY_BOARD #6731

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

Closed
wants to merge 7 commits into from

Conversation

tve
Copy link
Contributor

@tve tve commented Dec 25, 2020

This PR puts together several comments in #6473 to make the esp32 cmake branch more usable and move it to esp-idf v4.2.

I did some hacky stuff WRT include directories for the soc component in main/CMakeLists.txt. I believe the fundamental issue is that the include dirs are not "exported" by the cmake stuff for that component 'cause they're not supposed to be public. I'm cmake illiterate and am sure there's a better way...

Unfortunately IRAM is becoming very, very tight and I played with some config settings in sdkconfig.ble ('cause without BLE it's a non-issue). In this PR I'm disabling sdcard support because with it iram0_0_seg is 30 or so bytes over. There are some other suggestions linked in #6473 (comment) and I'm not particularly set on which solution to pick. I did kill a day trying all kinds of options... make size-components is your friend, but it only works if things fit, so at least this PR gets us into a "it works" state.

The CI just does esp32 cmake, the master branch needs to be merged into this branch to get the full complement. I commented out the old make-based CI steps, I hope they can become history. One issue is that the cmake stuff doesn't know how to do make submodules as far as I can tell.

Also, would be good to rename this branch to reflect the esp-idf version.

I have lots of follow-on improvements, including all the stranded ussl stuff, but unless the ball gets rolling there's not much point queuing them up.

@tve
Copy link
Contributor Author

tve commented Dec 25, 2020

I took another stab at handling the board definitions. That's the "update boards and add toothless configuration" commit. cmake is a torture! I'm sure there's a better way, but at least what I've got seems to work.

To build for GENERIC_OTA, say, do a mkdir build-GENERIC_OTA, cd there, then
cmake -DMICROPY_BOARD=GENERIC_OTA .., followed by the usual make flash or similar.

Each board has a board.cmake with the set(SDKCONFIG_DEFAULT ...) but possibly additional cmake statements (hence the rename from sdkconfig.cmake). I had hoped I could override variables set main/CMakeList.txt but it's a mess, so right now everything configurable looks ugly, see e.g. the bluetooth option or the manifest.
cmake=1 vs. tve=0

I extended the CI to build all the boards, I think that's useful and another sanity check. The boards build sequentially, that could be improved.

I have not tackled setting the port or speed for flashing, I assume setting the right env vars when running make flash can do the trick, but I don't know...

@tve
Copy link
Contributor Author

tve commented Dec 25, 2020

I've been tinkering some more (which I threw away...) and came to the following conclusions:

  • espressif provides a nice tool, idf.py, that simplifies running cmake, make, flashing, etc. micropython should use it (I went down the path of scripting tasks using pyinvoke until I realized that a lot of what I was trying to do is already in idf.py, if only I could use it)
  • micropython should use esp-idf components, for example, all the bluetooth stuff should be in a component that can be enabled/disabled using the std esp-idf component selection
  • configuration options should use the std menuconfig, to enable use of idf.py

Right now I don't have energy to play with all this and what's in this PR works for me for now...

@tve
Copy link
Contributor Author

tve commented Dec 26, 2020

I've been trying to get a project of mine running on the cmake version and so far have not been successful. I worked around #6733 by setting sys.path explicitly, but after initializing everything and connecting with MQTTS the esp running my app crashes. I tried two different builds (with BT and without) and get similar but not identical crashes. The last one looks like this:

PC      : 0x401a82ef  PS      : 0x00060530  A0      : 0x800f71de  A1      : 0x3ffc0860
A2      : 0x00001004  A3      : 0x00000004  A4      : 0x00000001  A5      : 0x00000000
A6      : 0x00000001  A7      : 0x3ffc0880  A8      : 0x00000003  A9      : 0x00000000
A10     : 0x00000000  A11     : 0x00000004  A12     : 0x0000001c  A13     : 0x3ffc089c
A14     : 0x00000001  A15     : 0x00000001  SAR     : 0x0000001c  EXCCAUSE: 0x0000001c
EXCVADDR: 0x00001020  LBEG    : 0x4000c46c  LEND    : 0x4000c477  LCOUNT  : 0x00000000

Backtrace:0x401a82ec:0x3ffc0860 0x400f71db:0x3ffc0880 0x400dc6ee:0x3ffc08d0 0x400e31f5:0x3ffc08f0 0x
400e5af9:0x3ffc0910 0x400dcb6c:0x3ffc09a0 0x400dcbd4:0x3ffc09c0 0x400dcc44:0x3ffc09f0 0x400dc729:0x3
ffc0a10 0x400e31f5:0x3ffc0a30 0x400e3305:0x3ffc0a50 0x400e5baa:0x3ffc0a70 0x400dc890:0x3ffc0b00 0x40
0e31f5:0x3ffc0b30 0x400e5af9:0x3ffc0b50 0x400dc890:0x3ffc0be0 0x400e31f5:0x3ffc0c30 0x400e3305:0x3ff
c0c50 0x400e5baa:0x3ffc0c70 0x400dc890:0x3ffc0d00 0x400e31f5:0x3ffc0d70 0x400e5af9:0x3ffc0d90 0x400d
c890:0x3ffc0e20 0x400e31f5:0x3ffc0e90 0x400e321e:0x3ffc0eb0 0x400f4cab:0x3ffc0ed0 0x400f4f59:0x3ffc0
f60 0x400f4fb5:0x3ffc0f80 0x400d60e5:0x3ffc0fa0 0x4008a865:0x3ffc0fd0
PC: 0x401a82ef is in heap_caps_match (/home/src/esp32/mpy-esp-idf/esp-idf/components/heap/heap_caps.
c:84).
84          return heap->heap != NULL && ((get_all_caps(heap) & caps) == caps);
BT-1: 0x400dcb6c is in mp_obj_gen_resume (/home/src/esp32/mpy-esp-idf/micropython/py/objgenerator.c:
200).
200             ret_kind = mp_execute_bytecode(&self->code_state, throw_value);
BT-2: 0x400dcbd4 is in gen_resume_and_raise (/home/src/esp32/mpy-esp-idf/micropython/py/objgenerator
.c:241).
241         switch (mp_obj_gen_resume(self_in, send_value, throw_value, &ret)) {
BT-3: 0x400dcc44 is in gen_instance_send (/home/src/esp32/mpy-esp-idf/micropython/py/objgenerator.c:
264).
264         mp_obj_t ret = gen_resume_and_raise(self_in, send_value, MP_OBJ_NULL);
BT-4: 0x400dc729 is in fun_builtin_2_call (/home/src/esp32/mpy-esp-idf/micropython/py/objfun.c:86).
86          return self->fun._2(args[0], args[1]);
BT-5: 0x400e31f5 is in mp_call_function_n_kw (/home/src/esp32/mpy-esp-idf/micropython/py/runtime.c:6
50).
650             return type->call(fun_in, n_args, n_kw, args);
BT-6: 0x400e3305 is in mp_call_method_n_kw (/home/src/esp32/mpy-esp-idf/micropython/py/runtime.c:666
).
etc...

In both cases the crash happens in the IDF heap components as it's called from the VM and in both cases the EXCVADDR is some small number (0x2 in the other case).

I then remembered that there are unit tests (DUH!). The basic tests pass, but extmod/usocket_tcp_basic.py hangs. and vfs_fat_finaliser vfs_lfs_mtime inf_nan_arith fail. I have not yet investigated.

While fixing #6734 I noticed that py/py.mk has some special CFLAGS for a bunch of source files (lines 273-286) and I don't see anything like that in neither py.cmake nor mkrules.cmake. Dunno whether these are elsewhere, not needed, or not dealt with yet.

Sadly it looks like the cmake branch needs quite some more work before it is usable. Hard to tell whether the problems are related to cmake, to edp-idf-v4.2, or to the xtensa compiler suite change.

@pumelo
Copy link

pumelo commented Dec 27, 2020

I'm having similar issue. Is the crash happening in a call to esp32.idf_heap_info. if yes, this is not related to cmake but also happend for me on a legacy make build with idf 4.2. honestly i did not investigate further but just dropped calling that function...

@pumelo
Copy link

pumelo commented Dec 27, 2020

Gotcha:

if (heap_caps_match(heap, cap)) {

Here a check for a null pointer should be added as done in idf:

https://github.com/espressif/esp-idf/blob/b0150615dff529662772a60dcb57d5b559f480e2/components/heap/heap_caps.c#L524

This is probably worth another pr and issue.

... Not tested yet!

@pumelo
Copy link

pumelo commented Jan 3, 2021

Sorry, ignore my previous comment. That is just random rubbish. Turns out this does not solve the problem.

heap_caps_print_heap_info from idf is almost the same function as the one im mp and it does not test for a NULL pointer.
Looks likes the problem is more wired than what I thought at first.
Calling heap_caps_print_heap_info from mp works for me without crashing ...

mp uses private imports here which is probably odd anyway.

#include "../heap_private.h"

might this be related to code and symbol placing?
idf forces the functions to ram (noflash):
https://github.com/espressif/esp-idf/blob/b0150615dff529662772a60dcb57d5b559f480e2/components/heap/linker.lf

@pumelo
Copy link

pumelo commented Jan 3, 2021

The following solutions gets rid of the private import, works (i just tested) and most likely is backwards compatible.
However, it only provides cumulated information for all matching heaps. idf does not seem to provide a public functions to get the available heaps at runtime
Also I am not sure if info.total_allocated_bytes + info.total_free_bytes equals the total heap size ..

STATIC mp_obj_t esp32_idf_heap_info(const mp_obj_t cap_in) {
    mp_int_t cap = mp_obj_get_int(cap_in);
    // heap_caps_print_heap_info(cap);
    multi_heap_info_t info;
    heap_caps_get_info(&info, cap);
    mp_obj_t heap_list = mp_obj_new_list(0, 0);
    mp_obj_t data[] = {
        MP_OBJ_NEW_SMALL_INT(info.total_allocated_bytes + info.total_free_bytes), // total heap size
        MP_OBJ_NEW_SMALL_INT(info.total_free_bytes),   // total free bytes
        MP_OBJ_NEW_SMALL_INT(info.largest_free_block), // largest free contiguous
        MP_OBJ_NEW_SMALL_INT(info.minimum_free_bytes), // minimum free seen
    };
    mp_obj_t this_heap = mp_obj_new_tuple(4, data);
    mp_obj_list_append(heap_list, this_heap);
    return heap_list;
}

@tve
Copy link
Contributor Author

tve commented Jan 4, 2021

@pumelo
Copy link

pumelo commented Jan 5, 2021

I know. But this function seems of no use if there is no way to actually get the pointers to the heaps. And the linked list to get the heaps is in the private header. Did you find a public way to get the pointers to the heaps at runtime?

@pumelo
Copy link

pumelo commented Jan 11, 2021

Some more random BS related to 4.2 and psram:
This espressif/esp-idf@368d623 change made memory worse if having psram.

manually setting cache_tx_buf_num to 0 fixed the issue for me:

in modnetwork.c

STATIC mp_obj_t get_wlan(size_t n_args, const mp_obj_t *args) {
    static int initialized = 0;
    if (!initialized) {
        wifi_init_config_t cfg = WIFI_INIT_CONFIG_DEFAULT();
        cfg.cache_tx_buf_num = 0;
        ESP_LOGD("modnetwork", "Initializing WiFi");
        ESP_EXCEPTIONS(esp_wifi_init(&cfg));
        ESP_EXCEPTIONS(esp_wifi_set_storage(WIFI_STORAGE_RAM));
        ESP_LOGD("modnetwork", "Initialized");
        initialized = 1;
    }

EDIT:
Setting the value to zero did render WLAN in AP unusable (Can't connect). Using something small like 4 worked for me tough.

@tve
Copy link
Contributor Author

tve commented Jan 12, 2021

And why is setting CONFIG_ESP32_WIFI_CACHE_TX_BUFFER_NUM to 0 not sufficient?

@pumelo
Copy link

pumelo commented Jan 12, 2021

You cannot (https://github.com/espressif/esp-idf/blob/c40f2590bf759ff60ef122afa79b4ec04e7633d2/components/esp_wifi/Kconfig#L96). The build system claims that 0 is out of range (if you have psram enabled) and the minimum is 16. Also be aware, that with a setting of zero wifi ap does not work anymore. I'm currently using 4 which seems to be a hacky tradeoff ...

I asked in the esp32 forum but did not get any answer yet: https://www.esp32.com/viewtopic.php?f=2&t=18966

@dpgeorge
Copy link
Member

dpgeorge commented Feb 9, 2021

I'm just catching up with this now.

Sadly it looks like the cmake branch needs quite some more work before it is usable. Hard to tell whether the problems are related to cmake, to edp-idf-v4.2, or to the xtensa compiler suite change.

Ok, that makes it hard, too many things changing at once. I had hoped that we could just abandon IDFv3 completely and move to the latest IDFv4, and that would fix everything. But it sounds like that's too optimistic, that there are too many moving pieces, and IDFv4 is not yet stable enough (at least, it has changes which reduce available RAM which we cannot take in yet).

Given the new MCUs (S2, C3) and the fact they need separate toolchains, I don't think it's feasible to maintain a standard Makefile based build system. We need to switch to cmake and leverage as much of the IDF's tooling as possible.

So how about sticking to IDF 4.0.1 (or minor update to 4.0.2) to start with and just converting to cmake? Then we can update te IDF 4.1.1, then IDF 4.2.

@tve
Copy link
Contributor Author

tve commented Feb 9, 2021

Agreed that it's probably wise to first get 4.0.2 working and not change everything all at once. The biggest issue I have, though, is that I think it would be great to really switch build systems and use the idf tool and menuconfig.

@dpgeorge
Copy link
Member

dpgeorge commented Feb 9, 2021

Agreed that it's probably wise to first get 4.0.2 working and not change everything all at once.

Ok, I'll try this.

I think it would be great to really switch build systems and use the idf tool and menuconfig.

You mean use idf.py build? Yes I was trying that out as well. Would be good. (Also I'm looking at their install.sh and export.sh scripts to do all that toolchain/env stuff.)

But for menuconfig, do you mean to replace MICROPY_xxx with menuconfig?

@tve
Copy link
Contributor Author

tve commented Feb 9, 2021

But for menuconfig, do you mean to replace MICROPY_xxx with menuconfig?

Yes, at least partially. I'm not sure how it would work. But the current situation is not good. Specifically:

  • MICROPY_ can enable/disable modules, such as bluetooth, that are coupled with esp-idf config options. It's painful to get all that right and later to retrace why which options were chosen and what to do with new options that are introduced in esp-idf
  • As esp-idf changes and also as new cpu cores are introduced the set of config options changes and it becomes a manual task to discover what changed and to try and replicate the right thing in MP.

I realize it wouldn't make sense to move/mirror all MICROPY_ options in menuconfig.

@dpgeorge
Copy link
Member

See #6906 .

@dpgeorge
Copy link
Member

ESP-IDF v4.2 is now supported, as of a915002

Probably this PR can be closed now.

@dpgeorge dpgeorge closed this Feb 15, 2021
@dpgeorge
Copy link
Member

Feel free to re-open if there are outstanding items here.

tannewt pushed a commit to tannewt/circuitpython that referenced this pull request Aug 10, 2022
Decode percent encoded file paths and set charset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants