Skip to content

Conversation

benedekkupper
Copy link
Contributor

This change is intended to make etl integration into the ESP-IDF ecosystem painless. Without this patch, it took me a while to figure out a way to integrate this library into the build, here is what's needed:

  1. In the ESP-IDF project CMakeLists.txt:
include(FetchContent)
FetchContent_Declare(etl SOURCE_DIR ${WORKSPACE_DIR}/etl)
FetchContent_MakeAvailable(etl)
  1. In the ESP-IDF component (e.g. main) CMakeLists.txt:
target_link_libraries(${COMPONENT_LIB} # e.g. __idf_main
    PUBLIC etl
)

This change aims to support etl with ESP-IDF's component system. The return from the cmake file processing early is necessary because ESP-IDF runs cmake in script mode, where all paths are flattened, and ${CMAKE_CURRENT_SOURCE_DIR}/cmake/helpers.cmake is resolved incorrectly, failing the build. With this modification, etl can be used with the established component syntax:

list(APPEND EXTRA_COMPONENT_DIRS "${WORKSPACE_DIR}/etl")
idf_component_register(
    REQUIRES
        etl
    INCLUDE_DIRS
    SRCS
        main.cpp
)

@coderabbitai
Copy link

coderabbitai bot commented Oct 15, 2025

Walkthrough

The CMake configuration adds an ESP_PLATFORM-specific branch at the top of CMakeLists.txt. When ESP_PLATFORM is defined, it invokes idf_component_register with INCLUDE_DIRS set to "include" and then terminates further processing (early return), bypassing the remainder of the CMake logic. This creates a short-circuit path for ESP builds that only registers the include directory via ESP-IDF’s component system. No changes are made to exported or public declarations.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly summarises the main change of integrating ETL into the ESP-IDF component system by supporting it as an ESP-IDF component, and does so in a concise and specific manner that aligns with the pull request intent.
Description Check ✅ Passed The description clearly explains the motivation for the change, outlines existing integration steps using FetchContent and target_link_libraries, and describes how the patch simplifies ETL usage within the ESP-IDF component system by resolving CMake script mode path issues.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a025d0c and 776202e.

📒 Files selected for processing (1)
  • CMakeLists.txt (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: GCC C++23 Linux - No STL (ubuntu-22.04)
  • GitHub Check: GCC C++23 Linux - STL - Force C++03 (ubuntu-22.04)
  • GitHub Check: GCC C++23 Linux - No STL - Force C++03 (ubuntu-22.04)
  • GitHub Check: Clang C++23 OSX - No STL - Force C++03 (macos-13)
  • GitHub Check: GCC C++23 Linux - STL (ubuntu-22.04)
  • GitHub Check: Clang C++23 Linux - No STL - Force C++03 (ubuntu-22.04)
  • GitHub Check: Clang C++17 Linux - STL (ubuntu-22.04)
  • GitHub Check: GCC C++20 Linux - No STL - Force C++03 (ubuntu-22.04)
  • GitHub Check: Clang C++14 Linux - STL (ubuntu-22.04)
  • GitHub Check: Clang C++23 Linux - No STL (ubuntu-22.04)
  • GitHub Check: Clang C++14 Linux - No STL (ubuntu-22.04)
  • GitHub Check: Clang C++17 Linux - No STL (ubuntu-22.04)
  • GitHub Check: Clang C++23 Linux - STL - Force C++03 (ubuntu-22.04)
  • GitHub Check: GCC C++20 Linux - STL (ubuntu-22.04)
  • GitHub Check: GCC C++17 Linux - STL (ubuntu-22.04)
  • GitHub Check: GCC C++20 Linux - No STL (ubuntu-22.04)
  • GitHub Check: GCC C++20 Linux - STL - Force C++03 (ubuntu-22.04)
  • GitHub Check: GCC C++11 Linux - STL (ubuntu-22.04)
  • GitHub Check: GCC C++11 Linux - No STL (ubuntu-22.04)
  • GitHub Check: GCC C++17 Linux - No STL (ubuntu-22.04)
🔇 Additional comments (1)
CMakeLists.txt (1)

6-8: Please confirm return() is supported with CMake 3.10

We still advertise a minimum CMake version of 3.10, yet the new branch relies on return(). That command was introduced in later CMake releases, so on a strict 3.10 toolchain this could hard-fail before ever reaching the helper include. Kindly double-check the minimum version shipped with the ESP-IDF pipeline (and bump cmake_minimum_required here if needed) to avoid a silent regression for downstream users.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant