8000 pkg/esp32_sdk: Fix broken `rtc_io.h` declaration and prevent `sys/uio.h` inclusion outside RIOT by jmkim · Pull Request #21408 · RIOT-OS/RIOT · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

pkg/esp32_sdk: Fix broken rtc_io.h declaration and prevent sys/uio.h inclusion outside RIOT #21408

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 14, 2025
Merged

pkg/esp32_sdk: Fix broken rtc_io.h declaration and prevent sys/uio.h inclusion outside RIOT #21408

merged 2 commits into from
Apr 14, 2025

Conversation

jmkim
Copy link
Contributor
@jmkim jmkim commented Apr 14, 2025

Contribution description

This PR includes two esp32_sdk patches to ensure that the RIOT-bundled ESP-IDF remains compatible when used independently of the RIOT environment.

These changes are minimal and isolated, and they make the RIOT-patched ESP-IDF usable when ESP-IDF is used directly.

1. [driver/rtc_io] Fixes an incorrect function declaration in ESP-IDF components/driver/include/driver/rtc_io.h.

The implementation was renamed in 0022-driver-gpio-fix-undefined-reference-to-rtc_gpio_forc.patch as follows:

  • From: rtc_gpio_force_hold_all()
  • To: rtc_gpio_force_hold_en_all()

However, the declaration in the header file was not updated accordingly.

This patch corrects the function declaration to match the renamed implementation.

2. [newlib] Prevents inclusion of <sys/uio.h> when RIOT is not used.

RIOT's sys/libc/include/sys/uio.h header attempts to override the system header, which causes build errors when the bundled ESP-IDF is used independently.

This patch wraps the include with #ifdef RIOT_VERSION, so <sys/uio.h> is only included when building with RIOT.

Testing procedure

  1. Set RIOTBASE, IDF_PATH, and PATH:

    export RIOTBASE="$(pwd)/RIOT"                    # Replace with your actual clone path of github.com/jmkim/RIOT.git
export IDF_PATH="$RIOTBASE/build/pkg/esp32_sdk"
export PATH="$IDF_PATH/tools:$PATH"

  2. Download the RIOT bundled ESP-IDF:

    esp-idf.Makefile:

    RIOTBASE                ?= $(CURDIR)/RIOT
PKGDIRBASE              ?= $(RIOTBASE)/build/pkg
PKG_DIR                 ?= $(RIOTBASE)/pkg/esp32_sdk
PKG_SOURCE_DIR          ?= $(PKGDIRBASE)/esp32_sdk
PKG_BUILD_OUT_OF_SOURCE ?= 1    
include $(PKG_DIR)/Makefile

    Run the esp-idf.Makefile:

    make -f esp-idf.Makefile

  3. Ensure that all the patches are applied. You should see output similar to:

    ...
Applying: pkg/esp32: correct declaration of renamed function rtc_gpio_force_hold_en_all
Applying: pkg/esp32: avoid sys/uio.h inclusion outside of RIOT
touch .../RIOT/build/pkg/esp32_sdk/.pkg-state.git-patched
...

  4. Build a standalone ESP-IDF example project:

    cd $IDF_PATH/examples/get-started/hello_world
idf.py set-target esp32s3
idf.py build

Without these patches:

  • A linker error occurs due to rtc_gpio_force_hold_all() being undefined.
  • The build fails due to <sys/uio.h> not being found.

With these patches applied:

  • The project builds and links successfully.

Issues/PRs references

None

jmkim added 2 commits April 14, 2025 15:18
@jmkim
pkg/esp32: correct declaration of renamed function rtc_gpio_force_hol…
876bfa2 
…d_en_all

The implementation of `rtc_gpio_force_hold_all()` was renamed to`rtc_gpio_force_hold_en_all()`
in the patch `0022-driver-gpio-fix-undefined-reference-to-rtc_gpio_forc.patch`.

This patch corrects the function declaration to match the renamed implementation.

Signed-off-by: Jongmin Kim <jmkim@debian.org>
@jmkim
pkg/esp32: avoid sys/uio.h inclusion outside of RIOT
a1cd48a 
The RIOT header `<sys/uio.h>` is not available when using the bundled ESP-IDF
outside of RIOT.

This patch ensures that `<sys/uio.h>` is only included when `RIOT_VERSION` is
defined.

Signed-off-by: Jongmin Kim <jmkim@debian.org>
@github-actions github-actions bot added the Area: pkg Area: External package ports label Apr 14, 2025
@jmkim jmkim changed the title Fix esp idf pkg/esp32_sdk: Fix broken rtc_io.h declaration and prevent sys/uio.h inclusion outside RIOT Apr 14, 2025
@benpicco benpicco requested a review from gschorcht April 14, 2025 07:53
jmkim added a commit to doubleo-dev/nand-submodules that referenced this pull request Apr 14, 2025
@jmkim
esp-idf: Add local patches to fix build errors with RIOT-bundled ESP-IDF
2fcc25e 
pkg/esp32_sdk: Fix broken rtc_io.h declaration and prevent sys/uio.h
               inclusion outside RIOT

Forwarded: RIOT-OS/RIOT#21408

Signed-off-by: Jongmin Kim <jmkim@debian.org>
@gschorcht
Copy link
Contributor

Although I don't see a reasonable use case for using the patched IDF from the RIOT build tree instead of the original IDF for applications that don't use RIOT at all, I would be fine adding these patches if they don't affect RIOT compilation at all.

However, this will make the migration to ESP-IDF v5.4 (PR #21261) more difficult, since all the existing patches are removed for the migration and numerous new patches will be added that are completely independent of the previous patches. So this PR would produce conflicts when rebasing PR #21261.

Therefore, I am not sure whether we should really include the patches for the old IDF at this time.

jmkim added a commit to doubleo-dev/nand-submodules that referenced this pull request Apr 14, 2025
@jmkim
esp-idf: Add local patches to fix build errors with RIOT-bundled ESP-IDF
95ca489 
pkg/esp32_sdk: Fix broken rtc_io.h declaration and prevent sys/uio.h
               inclusion outside RIOT

Forwarded: RIOT-OS/RIOT#21408

Signed-off-by: Jongmin Kim <jmkim@debian.org>
@jmkim
Copy link
Contributor Author
jmkim commented Apr 14, 2025

This is required when we need to use ESP-IDF functions directly from RIOT OS app — for example, including specific headers and their implementations from ESP-IDF directly. For my employer's need, I have documented how to set the build environment.

Currently, without these patches, the build fails:

RIOT/build/pkg/esp32_sdk/components/driver/gpio.c: In function 'gpio_force_hold_all':
RIOT/build/pkg/esp32_sdk/components/driver/gpio.c:696:5: error: implicit declaration of function 'rtc_gpio_force_hold_en_all'; did you mean 'rtc_gpio_force_hold_dis_all'? [-Werror=implicit-function-declaration]
     rtc_gpio_force_hold_en_all();
     ^~~~~~~~~~~~~~~~~~~~~~~~~~
     rtc_gpio_force_hold_dis_all
cc1: some warnings being treated as errors
In file included from RIOT/build/pkg/esp32_sdk/components/asio/asio/asio/include/asio/detail/socket_types.hpp:78,
                 from RIOT/build/pkg/esp32_sdk/components/asio/asio/asio/include/asio/impl/error_code.ipp:29,
                 from RIOT/build/pkg/esp32_sdk/components/asio/asio/asio/include/asio/impl/src.hpp:23,
                 from RIOT/build/pkg/esp32_sdk/components/asio/asio/asio/src/asio.cpp:11:
RIOT/build/pkg/esp32_sdk/components/newlib/platform_include/sys/uio.h:9:15: fatal error: sys/uio.h: No such file or directory
 #include_next <sys/uio.h>
               ^~~~~~~~~~~

With these patches, all builds complete successfully without any issues.

I just noticed that your ESP-IDF 5.4 transition is ongoing #21261, so this PR may complicate your transition work, as you mentioned.

I can't make the call myself, so I'll leave the decision up to you, @gschorcht. o/

@gschorcht
Copy link
Contributor

Ok, let's merge the PR. It doesn't affect the compilation of RIOT applications, but allows the use of IDF functions if this is required in special use cases.

Immediately after that I will rebase PR #21261 and check if these patches are still necessary.

@gschorcht gschorcht added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Apr 14, 2025
@riot-ci
Copy link
riot-ci commented Apr 14, 2025

Murdock results

✔️ PASSED

a1cd48a pkg/esp32: avoid sys/uio.h inclusion outside of RIOT

Success Failures Total Runtime
10294 0 10295 10m:10s

Artifacts

@benpicco benpicco enabled auto-merge April 14, 2025 12:19
@benpicco benpicco added this pull request to the merge queue Apr 14, 2025
Merged via the queue into RIOT-OS:master with commit 73f494a Apr 14, 2025
29 checks passed
@mguetschow mguetschow added this to the Release 2025.04 milestone Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gschorcht gschorcht gschorcht approved these changes

Assignees
No one assigned
Labels
Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2025.04
Development

Successfully merging this pull request may close these issues.
5 participants
@jmkim @gschorcht @riot-ci @benpicco @mguetschow
0