Skip to content

esp32: IDF v5.4(v5.5-dev) compile. #16565

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

Merged
merged 2 commits into from
Jan 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ports/esp32/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ manage the ESP32 microcontroller, as well as a way to manage the required
build environment and toolchains needed to build the firmware.

The ESP-IDF changes quickly and MicroPython only supports certain versions.
Currently MicroPython supports v5.2, v5.2.2, and v5.3.
Currently MicroPython supports v5.2, v5.2.2, v5.3 and v5.4.

To install the ESP-IDF the full instructions can be found at the
[Espressif Getting Started guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/get-started/index.html#installation-step-by-step).
Expand Down
9 changes: 7 additions & 2 deletions ports/esp32/network_ppp.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,12 @@ static mp_obj_t ppp_make_new(mp_obj_t stream) {
}
MP_DEFINE_CONST_FUN_OBJ_1(esp_network_ppp_make_new_obj, ppp_make_new);

static u32_t ppp_output_callback(ppp_pcb *pcb, u8_t *data, u32_t len, void *ctx) {
#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 4, 0)
static u32_t ppp_output_callback(ppp_pcb *pcb, const void *data, u32_t len, void *ctx)
#else
static u32_t ppp_output_callback(ppp_pcb *pcb, u8_t *data, u32_t len, void *ctx)
#endif
{
ppp_if_obj_t *self = ctx;

mp_obj_t stream = self->stream;
Expand All @@ -109,7 +114,7 @@ static u32_t ppp_output_callback(ppp_pcb *pcb, u8_t *data, u32_t len, void *ctx)
}

int err;
return mp_stream_rw(stream, data, len, &err, MP_STREAM_RW_WRITE);
return mp_stream_rw(stream, (void *)data, len, &err, MP_STREAM_RW_WRITE);
}

static void pppos_client_task(void *self_in) {
Expand Down
8 changes: 7 additions & 1 deletion ports/esp32/network_wlan.c
Original file line number Diff line number Diff line change
Expand Up @@ -763,14 +763,20 @@ static const mp_rom_map_elem_t wlan_if_locals_dict_table[] = {
#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 3, 0)
{ MP_ROM_QSTR(MP_QSTR_SEC_DPP), MP_ROM_INT(WIFI_AUTH_DPP) },
#endif
#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 4, 0)
{ MP_ROM_QSTR(MP_QSTR_SEC_WPA3_ENT), MP_ROM_INT(WIFI_AUTH_WPA3_ENTERPRISE) },
{ MP_ROM_QSTR(MP_QSTR_SEC_WPA2_WPA3_ENT), MP_ROM_INT(WIFI_AUTH_WPA2_WPA3_ENTERPRISE) },
#endif

{ MP_ROM_QSTR(MP_QSTR_PM_NONE), MP_ROM_INT(WIFI_PS_NONE) },
{ MP_ROM_QSTR(MP_QSTR_PM_PERFORMANCE), MP_ROM_INT(WIFI_PS_MIN_MODEM) },
{ MP_ROM_QSTR(MP_QSTR_PM_POWERSAVE), MP_ROM_INT(WIFI_PS_MAX_MODEM) },
};
static MP_DEFINE_CONST_DICT(wlan_if_locals_dict, wlan_if_locals_dict_table);

#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 3, 0)
#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 4, 0)
_Static_assert(WIFI_AUTH_MAX == 16, "Synchronize WIFI_AUTH_XXX constants with the ESP-IDF. Look at esp-idf/components/esp_wifi/include/esp_wifi_types_generic.h");

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, done.

#elif ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 3, 0)
_Static_assert(WIFI_AUTH_MAX == 14, "Synchronize WIFI_AUTH_XXX constants with the ESP-IDF. Look at esp-idf/components/esp_wifi/include/esp_wifi_types_generic.h");
#elif ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 2, 0)
_Static_assert(WIFI_AUTH_MAX == 13, "Synchronize WIFI_AUTH_XXX constants with the ESP-IDF. Look at esp-idf/components/esp_wifi/include/esp_wifi_types.h");
Expand Down
2 changes: 1 addition & 1 deletion py/persistentcode.c
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ static mp_raw_code_t *load_raw_code(mp_reader_t *reader, mp_module_context_t *co

#if MICROPY_EMIT_MACHINE_CODE
} else {
const uint8_t *prelude_ptr;
const uint8_t *prelude_ptr = NULL;
Copy link
Member

Choose a reason for hiding this comment

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

This change is unrelated to this PR, and shouldn't be needed.

Copy link
Contributor

@DvdGiessen DvdGiessen Jan 12, 2025

Choose a reason for hiding this comment

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

I discovered independently this change is required to prevent an error while building against IDFv5.4 with a compiler error; perhaps a change in the compiler version included used by the IDF caused this issue to be exposed?

Without this change:

-- The C compiler identification is GNU 14.2.0
(...)
[1325/1467] Building C object esp-idf/main_esp32s3/CMakeFiles/__idf_main_esp32s3.dir/build/micropython/py/persistentcode.c.obj
FAILED: esp-idf/main_esp32s3/CMakeFiles/__idf_main_esp32s3.dir/build/micropython/py/persistentcode.c.obj (...)
/build/micropython/py/persistentcode.c: In function 'load_raw_code':
/build/micropython/py/persistentcode.c:439:38: error: 'prelude_ptr' may be used uninitialized [-Werror=maybe-uninitialized]
  439 |                 children[n_children] = (void *)prelude_ptr;
      |                 ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
/build/micropython/py/persistentcode.c:405:24: note: 'prelude_ptr' was declared here
  405 |         const uint8_t *prelude_ptr; // = NULL;
      |                        ^~~~~~~~~~~
cc1: some warnings being treated as errors
(...)
ninja failed with exit code 1

EDIT: Although I agree it shouldn't be needed; reading it myself it appears correct regardless of which ifdef block is used. Not sure why the compiler disagrees.

Copy link
Member

Choose a reason for hiding this comment

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

Although I agree it shouldn't be needed; reading it myself it appears correct regardless of which ifdef block is used. Not sure why the compiler disagrees.

Yeah, it looks like a compiler optimisation bug here.

I'm using gcc 14.2.1 and arm-none-eabi-gcc 14.2.0 here, and neither of them complain (with MICROPY_EMIT_NATIVE_PRELUDE_SEPARATE_FROM_MACHINE_CODE enabled).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the end, are we forced to leave this fix as is?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we can leave this change in. It doesn't affect any other ports (the code size diff is zero for them all).

#if MICROPY_EMIT_NATIVE_PRELUDE_SEPARATE_FROM_MACHINE_CODE
if (kind == MP_CODE_NATIVE_PY) {
// Executable code cannot be accessed byte-wise on this architecture, so copy
Expand Down
Loading