From 5738c60206b2792634ac4dfe05712d675235d0ec Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Fri, 3 Jul 2026 18:09:01 -0400 Subject: [PATCH] [nrf52] Run clang-tidy against the native sdk-nrf toolchain (#17364) --- .github/actions/cache-sdk-nrf/action.yml | 49 ++++ .github/workflows/ci.yml | 19 +- .../components/http_request/http_request.h | 2 +- esphome/components/logger/logger_zephyr.cpp | 2 +- esphome/components/nrf52/__init__.py | 11 +- esphome/components/nrf52/clang_tidy.py | 249 ++++++++++++++++++ esphome/components/nrf52/framework.py | 24 +- esphome/core/defines.h | 2 +- script/clang-tidy | 19 +- script/clang_tidy_hash.py | 2 + script/helpers_zephyr.py | 149 ++++------- tests/unit_tests/test_nrf52_framework.py | 26 +- 12 files changed, 432 insertions(+), 122 deletions(-) create mode 100644 .github/actions/cache-sdk-nrf/action.yml create mode 100644 esphome/components/nrf52/clang_tidy.py diff --git a/.github/actions/cache-sdk-nrf/action.yml b/.github/actions/cache-sdk-nrf/action.yml new file mode 100644 index 0000000000..71c09bfe14 --- /dev/null +++ b/.github/actions/cache-sdk-nrf/action.yml @@ -0,0 +1,49 @@ +name: Cache sdk-nrf +description: > + Resolve the pinned sdk-nrf version and cache the native sdk-nrf install + (west workspace, Zephyr SDK toolchain, python env) at ~/.esphome-sdk-nrf. + Every job that installs sdk-nrf natively (the nrf52 clang-tidy job and, + once the component tests build natively, their batches) shares one cache. + Callers must set env ESPHOME_SDK_NRF_PREFIX: ~/.esphome-sdk-nrf and have + the Python venv already restored. +inputs: + restore-only: + description: > + When "true", only restore -- never save the cache, even on dev. Use from + jobs that may not produce a complete install (e.g. a component batch + that fails mid-install), so a partial install is never written. + default: "false" +runs: + using: composite + steps: + - name: Resolve sdk-nrf and toolchain versions for cache key + # Both versions are pinned in code, not in any file that feeds the + # other cache keys, so resolve them explicitly. Keying on them means + # the cache invalidates when either is bumped (actions/cache never + # overwrites a key). + id: version + shell: bash + run: | + . venv/bin/activate + version=$(python -c ' + from esphome.components.nrf52 import RECOMMENDED_SDK_NRF_VERSION + from esphome.components.nrf52.framework import TOOLCHAIN_VERSION + print(f"{RECOMMENDED_SDK_NRF_VERSION}-{TOOLCHAIN_VERSION}")') + echo "version=$version" >> "$GITHUB_OUTPUT" + # Mirror cache-esp-idf: only dev-branch runs write the shared cache (so it + # lives in the default-branch scope readable by all PRs); PRs are + # restore-only and never push multi-GB artifacts into their own scope. + - name: Cache sdk-nrf install (write on dev) + if: github.ref == 'refs/heads/dev' && inputs.restore-only != 'true' + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ~/.esphome-sdk-nrf + # yamllint disable-line rule:line-length + key: ${{ runner.os }}-esphome-sdk-nrf-${{ steps.version.outputs.version }}-${{ hashFiles('esphome/components/nrf52/requirements.txt') }} + - name: Cache sdk-nrf install (restore-only off dev) + if: github.ref != 'refs/heads/dev' || inputs.restore-only == 'true' + uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ~/.esphome-sdk-nrf + # yamllint disable-line rule:line-length + key: ${{ runner.os }}-esphome-sdk-nrf-${{ steps.version.outputs.version }}-${{ hashFiles('esphome/components/nrf52/requirements.txt') }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9310b45b4a..caf6453c1b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -475,6 +475,8 @@ jobs: GH_TOKEN: ${{ github.token }} # esp32-arduino-tidy installs ESP-IDF natively; share the native IDF cache. ESPHOME_ESP_IDF_PREFIX: ~/.esphome-idf + # nrf52-tidy installs sdk-nrf natively; pin it to a cacheable path. + ESPHOME_SDK_NRF_PREFIX: ~/.esphome-sdk-nrf strategy: fail-fast: false max-parallel: 2 @@ -491,7 +493,7 @@ jobs: - id: clang-tidy name: Run script/clang-tidy for ZEPHYR options: --environment nrf52-tidy --grep USE_ZEPHYR --grep USE_NRF52 - pio_cache_key: tidy-zephyr + cache_sdk_nrf: true ignore_errors: false steps: @@ -527,6 +529,10 @@ jobs: with: framework: arduino + - name: Cache sdk-nrf install + if: matrix.cache_sdk_nrf + uses: ./.github/actions/cache-sdk-nrf + - name: Register problem matchers run: | echo "::add-matcher::.github/workflows/matchers/gcc.json" @@ -805,6 +811,9 @@ jobs: # esp32 component builds use the native ESP-IDF toolchain (default), so # share the tidy jobs' install location -- the restore below lands here. ESPHOME_ESP_IDF_PREFIX: ~/.esphome-idf + # nrf52 component builds install sdk-nrf natively; pin it to the shared + # cacheable path so the restore below lands where the build looks. + ESPHOME_SDK_NRF_PREFIX: ~/.esphome-sdk-nrf strategy: fail-fast: false max-parallel: ${{ (startsWith(github.base_ref, 'beta') || startsWith(github.base_ref, 'release')) && 8 || 4 }} @@ -840,6 +849,14 @@ jobs: uses: ./.github/actions/cache-esp-idf with: restore-only: true + - name: Cache sdk-nrf install (restore-only) + # Only batches whose test platforms include nrf52 need the native + # sdk-nrf install; never save -- just reuse the shared install the + # dev nrf52 tidy job cached when present. + if: matrix.batch.needs_nrf + uses: ./.github/actions/cache-sdk-nrf + with: + restore-only: true - name: Validate and compile components with intelligent grouping run: | . venv/bin/activate diff --git a/esphome/components/http_request/http_request.h b/esphome/components/http_request/http_request.h index 5025a5c12d..df1bb462ab 100644 --- a/esphome/components/http_request/http_request.h +++ b/esphome/components/http_request/http_request.h @@ -510,9 +510,9 @@ template class HttpRequestSendAction final : public Actionmax_response_buffer_size_; #ifdef USE_HTTP_REQUEST_RESPONSE if (this->capture_response_.value(x...)) { + size_t max_length = this->max_response_buffer_size_; std::string response_body; RAMAllocator allocator; uint8_t *buf = allocator.allocate(max_length); diff --git a/esphome/components/logger/logger_zephyr.cpp b/esphome/components/logger/logger_zephyr.cpp index 240bcc57c7..b7884b702b 100644 --- a/esphome/components/logger/logger_zephyr.cpp +++ b/esphome/components/logger/logger_zephyr.cpp @@ -57,7 +57,7 @@ void Logger::pre_setup() { if (this->baud_rate_ > 0) { static const struct device *uart_dev = nullptr; switch (this->uart_) { - case UART_SELECTION_UART0: + case UART_SELECTION_UART0: // NOLINT(bugprone-branch-clone) uart_dev = DEVICE_DT_GET_OR_NULL(DT_NODELABEL(uart0)); break; case UART_SELECTION_UART1: diff --git a/esphome/components/nrf52/__init__.py b/esphome/components/nrf52/__init__.py index 7c17eadd1a..a5f2018d55 100644 --- a/esphome/components/nrf52/__init__.py +++ b/esphome/components/nrf52/__init__.py @@ -79,6 +79,11 @@ AUTO_LOAD = ["zephyr", "preferences"] IS_TARGET_PLATFORM = True _LOGGER = logging.getLogger(__name__) +# Default framework versions per toolchain. The sdk-nrf one also keys the CI +# sdk-nrf install cache and pins the clang-tidy project's SDK. +RECOMMENDED_PLATFORMIO_VERSION = "2.6.1-b" +RECOMMENDED_SDK_NRF_VERSION = "2.9.2" + FAKE_BOARD_MANIFEST = """ { "frameworks": [ @@ -123,7 +128,11 @@ def _resolve_toolchain(config: ConfigType) -> ConfigType: def set_framework(config: ConfigType) -> ConfigType: if CONF_VERSION not in config[CONF_FRAMEWORK]: - default_version = "2.6.1-b" if CORE.using_toolchain_platformio else "2.9.2" + default_version = ( + RECOMMENDED_PLATFORMIO_VERSION + if CORE.using_toolchain_platformio + else RECOMMENDED_SDK_NRF_VERSION + ) config = { **config, CONF_FRAMEWORK: {**config[CONF_FRAMEWORK], CONF_VERSION: default_version}, diff --git a/esphome/components/nrf52/clang_tidy.py b/esphome/components/nrf52/clang_tidy.py new file mode 100644 index 0000000000..2dd4b7bd09 --- /dev/null +++ b/esphome/components/nrf52/clang_tidy.py @@ -0,0 +1,249 @@ +"""Generate clang-tidy compile commands via the native sdk-nrf toolchain. + +Produces a ``compile_commands.json`` for the nrf52/Zephyr clang-tidy +environment **without an ESPHome YAML config**, mirroring +``esphome.espidf.clang_tidy``: generate a minimal Zephyr application, run a +configure-only west build with the native sdk-nrf toolchain, and let +``script/helpers_zephyr.py`` extract idedata from the resulting compile +commands. + +* the stub app is C++ so the compile commands carry C++ flags, matching how + clang-tidy analyzes ESPHome's sources; +* ``prj.conf`` enables the Kconfig superset ESPHome components need (BT, ADC, + mcumgr, zigbee) so their include paths land in the compile commands; +* the platform defines (USE_ZEPHYR, USE_NRF52) match what a real ESPHome + nrf52 build adds via its generated project. + +``ESPHOME_ZEPHYR_COMPILE_COMMANDS`` may point at an existing build's +``compile_commands.json`` to skip generation (fast iteration). +""" + +from __future__ import annotations + +import logging +from pathlib import Path + +TIDY_PROJECT_NAME = "esphome_tidy" + +# Analyzed against the native toolchain's default SDK version +# (RECOMMENDED_SDK_NRF_VERSION), which also keys the CI install cache. +_TIDY_BOARD = "adafruit_itsybitsy_nrf52840" + +# Never compiled (the build is configure-only): the file exists only so the +# app target emits a C++ compile command to harvest flags/includes from. +_TIDY_MAIN_CPP = "int main() { return 0; }\n" + +# Kconfig superset enabling every subsystem an ESPHome nrf52 component may +# use, so the compile commands carry all of their include paths. +_TIDY_PRJ_CONF = """\ +CONFIG_CPP=y +CONFIG_STD_CPP20=y +CONFIG_REQUIRES_FULL_LIBCPP=y +CONFIG_NEWLIB_LIBC=y +CONFIG_BT=y +CONFIG_ADC=y +# posix (time sets POSIX_CLOCK, socket sets POSIX_API); without it the +# Zephyr POSIX headers clash with the libc ones under analysis +CONFIG_POSIX_API=y +#mcumgr begin +CONFIG_NET_BUF=y +CONFIG_ZCBOR=y +CONFIG_MCUMGR=y +CONFIG_MCUMGR_GRP_IMG=y +CONFIG_IMG_MANAGER=y +CONFIG_STREAM_FLASH=y +CONFIG_FLASH_MAP=y +CONFIG_FLASH=y +CONFIG_IMG_ERASE_PROGRESSIVELY=y +CONFIG_BOOTLOADER_MCUBOOT=y +CONFIG_MCUMGR_MGMT_NOTIFICATION_HOOKS=y +CONFIG_MCUMGR_GRP_IMG_STATUS_HOOKS=y +CONFIG_MCUMGR_GRP_IMG_UPLOAD_CHECK_HOOK=y +CONFIG_MCUMGR_TRANSPORT_UART=y +#mcumgr end +#zigbee begin +CONFIG_ZIGBEE=y +CONFIG_CRYPTO=y +CONFIG_NVS=y +CONFIG_SETTINGS=y +#zigbee end +""" + + +def _tidy_cmakelists(library_include_dirs: str) -> str: + # The defines a real ESPHome nrf52 build puts on the app target. + # ESPHOME_LOG_LEVEL must be set up front -- otherwise log.h's ``#ifndef`` + # sets it to NONE, a macro-redefined warning across nearly every source. + return f"""\ +# Auto-generated by ESPHome (clang-tidy compile-commands project) +cmake_minimum_required(VERSION 3.20.0) +set(Zephyr_DIR "$ENV{{ZEPHYR_BASE}}/share/zephyr-package/cmake/") +find_package(Zephyr REQUIRED) +project({TIDY_PROJECT_NAME}) +target_sources(app PRIVATE main.cpp) +target_compile_definitions(app PRIVATE + USE_ZEPHYR + USE_NRF52 + ESPHOME_LOG_LEVEL=ESPHOME_LOG_LEVEL_VERY_VERBOSE +) +target_include_directories(app PRIVATE +{library_include_dirs} +) +""" + + +def _parse_lib_deps(platformio_ini: Path) -> list: + """Parse the nrf52 env's ``lib_deps`` from platformio.ini into Library specs. + + These are the PlatformIO libraries ESPHome components pull in via + ``cg.add_library`` (ArduinoJson, dlms_parser, ...); their headers must be + on the tidy translation unit's include path. Mirrors the pio nrf52 env's + ``lib_deps`` composition (``common.lib_deps_base`` + + ``common:idf-component-libs``). + """ + import configparser + + from esphome.core import Library + + parser = configparser.ConfigParser(interpolation=None, strict=False) + parser.read(platformio_ini) + + tokens: list[str] = [] + for section, key in ( + ("common", "lib_deps_base"), + ("common:idf-component-libs", "lib_deps"), + ): + if parser.has_option(section, key): + tokens += parser.get(section, key).splitlines() + + libs: list[Library] = [] + for token in tokens: + token = token.split(";", 1)[0].strip() # drop trailing ; comment + if not token or token.startswith(("${", "+<")): + continue + if "://" in token or ".git" in token: + libs.append(Library(token, None, token)) # git repository (with #ref) + elif "@" in token: + name, _, version = token.partition("@") + libs.append(Library(name, version)) + return libs + + +def _library_include_dirs(platformio_ini: Path) -> list[str]: + """Resolve the pio libraries and return their include roots.""" + from esphome.platformio.library import LibraryBackend, convert_libraries + + dirs: list[str] = [] + + def emit(component) -> None: + build = component.data.get("build", {}) + candidates = {build.get("includeDir", "include"), build.get("srcDir", "src")} + candidates.update({"src", "."}) + for candidate in sorted(candidates): + path = (component.path / candidate).resolve() + if path.is_dir(): + dirs.append(str(path)) + + backend = LibraryBackend( + platform="nordicnrf52", framework="zephyr", emit=emit, cache_key="zephyr" + ) + convert_libraries(_parse_lib_deps(platformio_ini), backend) + return sorted(set(dirs)) + + +def _setup_core(work_dir: Path) -> None: + """Point CORE at the tidy project + SDK version, without any YAML config.""" + from esphome.components.zephyr.const import KEY_ZEPHYR + import esphome.config_validation as cv + from esphome.const import ( + KEY_CORE, + KEY_FRAMEWORK_VERSION, + KEY_TARGET_FRAMEWORK, + KEY_TARGET_PLATFORM, + PLATFORM_NRF52, + Toolchain, + ) + from esphome.core import CORE + + from . import RECOMMENDED_SDK_NRF_VERSION + + CORE.name = TIDY_PROJECT_NAME + # config_path's parent is the data-dir root for per-run artifacts. The + # sdk-nrf install is in the global cache dir, independent of this path. + CORE.config_path = work_dir.parent / "tidy.yaml" + CORE.build_path = work_dir + CORE.toolchain = Toolchain.SDK_NRF + CORE.data.setdefault(KEY_CORE, {})[KEY_TARGET_PLATFORM] = PLATFORM_NRF52 + CORE.data[KEY_CORE][KEY_TARGET_FRAMEWORK] = KEY_ZEPHYR + CORE.data[KEY_CORE][KEY_FRAMEWORK_VERSION] = cv.Version.parse( + RECOMMENDED_SDK_NRF_VERSION + ) + + +def generate_compile_commands(work_dir: Path, platformio_ini: Path) -> Path: + """Generate the tidy Zephyr project and run a configure-only west build. + + Returns the path to the generated ``compile_commands.json``. + """ + from esphome.core import EsphomeError + from esphome.framework_helpers import run_command_ok + from esphome.helpers import rmtree + + from .framework import check_and_install, get_build_env, get_build_paths + + # Surface ESPHome's INFO logs (sdk-nrf download/west update) -- they go + # through logging, which the clang-tidy script otherwise leaves at + # WARNING, so the first-run installation looks silent without this. + logging.basicConfig(level=logging.INFO, format="%(message)s") + + _setup_core(work_dir) + check_and_install() + + library_include_dirs = "\n".join( + f' "{d}"' for d in _library_include_dirs(platformio_ini) + ) + source_dir = work_dir / "zephyr" + source_dir.mkdir(parents=True, exist_ok=True) + (source_dir / "CMakeLists.txt").write_text( + _tidy_cmakelists(library_include_dirs), encoding="utf-8" + ) + (source_dir / "main.cpp").write_text(_TIDY_MAIN_CPP, encoding="utf-8") + (source_dir / "prj.conf").write_text(_TIDY_PRJ_CONF, encoding="utf-8") + + # Always configure from scratch: west can't pristine a dir whose CMake + # cache is stale/missing, and a configure-only run is cheap. + build_dir = work_dir / "build" + if build_dir.is_dir(): + rmtree(build_dir) + + paths = get_build_paths() + # Build only the generated-headers target (syscall_list.h, offsets.h, ...) + # on top of the configure: clang-tidy needs those headers to exist, but a + # full firmware build would be wasted work. --no-sysbuild keeps sdk-nrf + # 2.9+ from wrapping the build in a multi-image sysbuild project, which + # would nest the compile commands and hide the headers target. + west_cmd = [ + str(paths["python_executable"]), + "-m", + "west", + "build", + "--no-sysbuild", + "-b", + _TIDY_BOARD, + "-d", + str(build_dir), + str(source_dir), + "-t", + "zephyr_generated_headers", + "--", + "-DCMAKE_EXPORT_COMPILE_COMMANDS=ON", + ] + if not run_command_ok( + west_cmd, + env=get_build_env(), + stream_output=True, + cwd=str(paths["framework_path"]), + ): + raise EsphomeError("nRF52 clang-tidy configure failed") + + return build_dir / "compile_commands.json" diff --git a/esphome/components/nrf52/framework.py b/esphome/components/nrf52/framework.py index 640aa07fbf..fa6f7d57ad 100644 --- a/esphome/components/nrf52/framework.py +++ b/esphome/components/nrf52/framework.py @@ -1,3 +1,4 @@ +import hashlib import logging import os from pathlib import Path @@ -24,7 +25,7 @@ from esphome.helpers import get_str_env _LOGGER = logging.getLogger(__name__) _REQUIREMENTS = Path(__file__).parent / "requirements.txt" -_TOOLCHAIN_VERSION = "0.17.4" +TOOLCHAIN_VERSION = "0.17.4" SDK_NG_TOOLCHAIN_MIRRORS = str_to_lst_of_str( os.environ.get( @@ -132,7 +133,7 @@ def get_build_env() -> dict: env = os.environ.copy() env["PATH"] = str(venv_bin_dir) + os.pathsep + env.get("PATH", "") env["ZEPHYR_BASE"] = str(_get_framework_path(version) / "zephyr") - env["Zephyr-sdk_DIR"] = str(_get_toolchain_path(_TOOLCHAIN_VERSION) / "cmake") + env["Zephyr-sdk_DIR"] = str(_get_toolchain_path(TOOLCHAIN_VERSION) / "cmake") return env @@ -158,9 +159,10 @@ def check_and_install() -> None: python_env_path = _get_python_env_path(version) env_python_path = get_python_env_executable_path(python_env_path, "python") sentinel = python_env_path / ".ready" + requirements_hash = hashlib.sha256(_REQUIREMENTS.read_bytes()).hexdigest() install_venv = ( not sentinel.exists() - or _REQUIREMENTS.stat().st_mtime > sentinel.stat().st_mtime + or sentinel.read_text(encoding="utf-8") != requirements_hash ) if install_venv: rmdir(python_env_path, msg=f"Clean up {version} Python environment") @@ -182,7 +184,7 @@ def check_and_install() -> None: raise EsphomeError( f"Install requirements for {version} Python environment failure" ) - sentinel.touch() + sentinel.write_text(requirements_hash, encoding="utf-8") framework_path = _get_framework_path(version) sentinel = framework_path / ".ready" @@ -238,19 +240,17 @@ def check_and_install() -> None: raise EsphomeError(f"Install Zephyr requirements for {version} failure") zephyr_sentinel.touch() - toolchains_dir = _get_toolchain_path(_TOOLCHAIN_VERSION) + toolchains_dir = _get_toolchain_path(TOOLCHAIN_VERSION) sentinel = toolchains_dir / ".ready" if not sentinel.exists(): - rmdir( - toolchains_dir, msg=f"Clean up {_TOOLCHAIN_VERSION} toolchain environment" - ) + rmdir(toolchains_dir, msg=f"Clean up {TOOLCHAIN_VERSION} toolchain environment") sysname, machine, extension = _get_toolchain_platform_info() with tempfile.NamedTemporaryFile() as tmp: - _LOGGER.info("Downloading Zephyr SDK %s minimal ...", _TOOLCHAIN_VERSION) + _LOGGER.info("Downloading Zephyr SDK %s minimal ...", TOOLCHAIN_VERSION) download_from_mirrors( SDK_NG_MINIMAL_MIRRORS, { - "VERSION": _TOOLCHAIN_VERSION, + "VERSION": TOOLCHAIN_VERSION, "sysname": sysname, "machine": machine, "extension": extension, @@ -259,11 +259,11 @@ def check_and_install() -> None: ) archive_extract_all(tmp.file, toolchains_dir, progress_header="Extracting") with tempfile.NamedTemporaryFile() as tmp: - _LOGGER.info("Downloading %s toolchain ...", _TOOLCHAIN_VERSION) + _LOGGER.info("Downloading %s toolchain ...", TOOLCHAIN_VERSION) download_from_mirrors( SDK_NG_TOOLCHAIN_MIRRORS, { - "VERSION": _TOOLCHAIN_VERSION, + "VERSION": TOOLCHAIN_VERSION, "sysname": sysname, "machine": machine, "extension": extension, diff --git a/esphome/core/defines.h b/esphome/core/defines.h index 1c0138f9d1..ff4bccc693 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -148,6 +148,7 @@ #define USE_NEXTION_TRIGGER_CUSTOM_TEXT_SENSOR #define USE_NEXTION_WAVEFORM #define USE_NUMBER +#define USE_OTA_STATE_LISTENER #define USE_OUTPUT #define USE_OUTPUT_FLOAT_POWER_SCALING #define USE_POWER_SUPPLY @@ -211,7 +212,6 @@ #define USE_RUNTIME_STATS #define USE_OTA #define USE_OTA_PASSWORD -#define USE_OTA_STATE_LISTENER #define USE_OTA_VERSION 2 #define USE_TIME_TIMEZONE #define USE_WIFI diff --git a/script/clang-tidy b/script/clang-tidy index 1416b9b332..7df46cb2d2 100755 --- a/script/clang-tidy +++ b/script/clang-tidy @@ -145,14 +145,16 @@ def clang_options(idedata): # defines cmd.extend(f"-D{define}" for define in idedata["defines"]) - # add toolchain include directories using -isystem to suppress their errors + # toolchain include directories, using -isystem to suppress their errors # idedata contains include directories for all toolchains of this platform, only use those from the one in use toolchain_dir = os.path.normpath(f"{idedata['cxx_path']}/../../") + toolchain_includes = [] for directory in idedata["includes"]["toolchain"]: if directory.startswith(toolchain_dir) and "picolibc" not in directory: - cmd.extend(["-isystem", directory]) + toolchain_includes.extend(["-isystem", directory]) - # add library include directories using -isystem to suppress their errors + # library include directories, using -isystem to suppress their errors + build_includes = [] for directory in list(idedata["includes"]["build"]): # skip our own directories, we add those later if ( @@ -166,7 +168,16 @@ def clang_options(idedata): ) or (directory.startswith(f"{root_path}") and "/.pio/" in directory) ): - cmd.extend(["-isystem", directory]) + build_includes.extend(["-isystem", directory]) + + if "zephyr" in triplet: + # Zephyr's POSIX layer shadows libc headers (sys/select.h, ...) with + # coherently-guarded versions; the real build searches the Zephyr + # include dirs before the toolchain's, and the shadowed headers clash + # (e.g. newlib's sigset_t vs Zephyr's) in the opposite order. + cmd.extend(build_includes + toolchain_includes) + else: + cmd.extend(toolchain_includes + build_includes) # add the esphome include directory using -I cmd.extend(["-I", root_path]) diff --git a/script/clang_tidy_hash.py b/script/clang_tidy_hash.py index 00bcaf45b0..57ca90711c 100644 --- a/script/clang_tidy_hash.py +++ b/script/clang_tidy_hash.py @@ -21,6 +21,8 @@ CLANG_TIDY_GLOBAL_FILES = ( "platformio.ini", "requirements_dev.txt", "esphome/idf_component.yml", + "esphome/components/esp32/__init__.py", + "esphome/components/nrf52/__init__.py", ) # sdkconfig.defaults and per-target sdkconfig.defaults. files flip the diff --git a/script/helpers_zephyr.py b/script/helpers_zephyr.py index 66ef6ffc98..c26ad7f2cd 100644 --- a/script/helpers_zephyr.py +++ b/script/helpers_zephyr.py @@ -1,59 +1,32 @@ +"""Load clang-tidy idedata for the nrf52/Zephyr environment. + +The compile commands come from a configure-only build of a minimal Zephyr +project using the native sdk-nrf toolchain (see +``esphome.components.nrf52.clang_tidy``); this module extracts the include +paths, defines and compiler flags clang-tidy needs from them. +""" + import json +import os from pathlib import Path import re +import shlex import subprocess def load_idedata(environment, temp_folder, platformio_ini): - build_environment = environment.replace("-tidy", "") - build_dir = Path(temp_folder) / f"build-{build_environment}" - Path(build_dir).mkdir(exist_ok=True) - Path(build_dir / "platformio.ini").write_text( - Path(platformio_ini).read_text(encoding="utf-8"), encoding="utf-8" - ) - esphome_dir = Path(build_dir / "esphome") - esphome_dir.mkdir(exist_ok=True) - Path(esphome_dir / "main.cpp").write_text( - """ -#include -int main() { return 0;} -extern "C" void zboss_signal_handler() {}; -""", - encoding="utf-8", - ) - zephyr_dir = Path(build_dir / "zephyr") - zephyr_dir.mkdir(exist_ok=True) - Path(zephyr_dir / "prj.conf").write_text( - """ -CONFIG_NEWLIB_LIBC=y -CONFIG_BT=y -CONFIG_ADC=y -#mcumgr begin -CONFIG_NET_BUF=y -CONFIG_ZCBOR=y -CONFIG_MCUMGR=y -CONFIG_MCUMGR_GRP_IMG=y -CONFIG_IMG_MANAGER=y -CONFIG_STREAM_FLASH=y -CONFIG_FLASH_MAP=y -CONFIG_FLASH=y -CONFIG_IMG_ERASE_PROGRESSIVELY=y -CONFIG_BOOTLOADER_MCUBOOT=y -CONFIG_MCUMGR_MGMT_NOTIFICATION_HOOKS=y -CONFIG_MCUMGR_GRP_IMG_STATUS_HOOKS=y -CONFIG_MCUMGR_GRP_IMG_UPLOAD_CHECK_HOOK=y -CONFIG_MCUMGR_TRANSPORT_UART=y -#mcumgr end -#zigbee begin -CONFIG_ZIGBEE=y -CONFIG_CRYPTO=y -CONFIG_NVS=y -CONFIG_SETTINGS=y -#zigbee end -""", - encoding="utf-8", - ) - subprocess.run(["pio", "run", "-e", build_environment, "-d", build_dir], check=True) + if explicit := os.environ.get("ESPHOME_ZEPHYR_COMPILE_COMMANDS"): + compile_commands_path = Path(explicit) + else: + from esphome.components.nrf52.clang_tidy import generate_compile_commands + + work_dir = (Path(temp_folder) / f"zephyr-{environment}").resolve() + compile_commands_path = generate_compile_commands( + work_dir, Path(platformio_ini) + ) + + if not compile_commands_path.is_file(): + raise RuntimeError(f"compile_commands.json not found: {compile_commands_path}") def extract_include_paths(command): include_paths = [] @@ -62,7 +35,7 @@ CONFIG_SETTINGS=y split_strings = re.split( r"\s*-\s*(?:I|isystem)", list(filter(lambda x: x, match))[0] ) - include_paths.append(split_strings[1]) + include_paths.append(split_strings[1].strip()) return include_paths def extract_defines(command): @@ -74,15 +47,6 @@ CONFIG_SETTINGS=y if not any(match.startswith(prefix) for prefix in ignore_prefixes) ] - def find_cxx_path(commands): - for entry in commands: - command = entry["command"] - cxx_path = command.split()[0] - if not cxx_path.endswith("++"): - continue - return cxx_path - return None - def get_builtin_include_paths(compiler): result = subprocess.run( [compiler, "-E", "-x", "c++", "-", "-v"], @@ -105,47 +69,48 @@ CONFIG_SETTINGS=y return include_paths def extract_cxx_flags(command): - # Extracts CXXFLAGS from the command string, excluding includes and defines. + # Extracts CXXFLAGS from the command string, excluding includes and + # defines. Anchored per token: a substring match would extract a bogus + # "-format-zero-length" from -Wno-format-zero-length. flag_pattern = re.compile( - r"(-O[0-3s]|-g|-std=[^\s]+|-Wall|-Wextra|-Werror|--[^\s]+|-f[^\s]+|-m[^\s]+|-imacros\s*[^\s]+)" + r"^(-O[0-3s]|-g|-std=.+|-Wall|-Wextra|-Werror|--.+|-f.+|-m.+|-imacros.+)$" ) - return [ - match.replace("-imacros ", "-imacros") - for match in flag_pattern.findall(command) - ] + flags = [] + tokens = shlex.split(command) + for i, token in enumerate(tokens): + if token == "-imacros" and i + 1 < len(tokens): + flags.append(f"-imacros{tokens[i + 1]}") + elif flag_pattern.match(token): + flags.append(token) + return flags def transform_to_idedata_format(compile_commands): - cxx_path = find_cxx_path(compile_commands) - idedata = { + # Use only the tidy app TU (main.cpp): as the app target, its compile + # command already carries the full Zephyr include set. Unioning every + # TU instead would drag in per-library internal include dirs (e.g. the + # Zephyr POSIX shim, whose signal.h redefines newlib's sigset_t) that + # no ESPHome source compiles against. + entry = next( + (e for e in compile_commands if e["file"].endswith("main.cpp")), None + ) + if entry is None: + raise RuntimeError("tidy main.cpp not found in compile_commands.json") + command = entry["command"] + # Find the compiler by name: the command may be prefixed with a + # launcher (Zephyr auto-enables ccache when present). + cxx_path = next((t for t in shlex.split(command) if t.endswith("++")), None) + if cxx_path is None: + raise RuntimeError(f"no C++ compiler in compile command: {command}") + + return { "includes": { "toolchain": get_builtin_include_paths(cxx_path), - "build": set(), + "build": extract_include_paths(command), }, - "defines": set(), + "defines": extract_defines(command), "cxx_path": cxx_path, - "cxx_flags": set(), + "cxx_flags": extract_cxx_flags(command), } - for entry in compile_commands: - command = entry["command"] - exec = command.split()[0] - if exec != cxx_path: - continue - - idedata["includes"]["build"].update(extract_include_paths(command)) - idedata["defines"].update(extract_defines(command)) - idedata["cxx_flags"].update(extract_cxx_flags(command)) - - # Convert sets to lists for JSON serialization - idedata["includes"]["build"] = list(idedata["includes"]["build"]) - idedata["defines"] = list(idedata["defines"]) - idedata["cxx_flags"] = list(idedata["cxx_flags"]) - - return idedata - - compile_commands = json.loads( - Path( - build_dir / ".pio" / "build" / build_environment / "compile_commands.json" - ).read_text(encoding="utf-8") - ) + compile_commands = json.loads(compile_commands_path.read_text(encoding="utf-8")) return transform_to_idedata_format(compile_commands) diff --git a/tests/unit_tests/test_nrf52_framework.py b/tests/unit_tests/test_nrf52_framework.py index 2b3d1f6db8..bb5bc8c064 100644 --- a/tests/unit_tests/test_nrf52_framework.py +++ b/tests/unit_tests/test_nrf52_framework.py @@ -1,5 +1,6 @@ """Tests for esphome.components.nrf52.framework helpers.""" +import hashlib from pathlib import Path from types import SimpleNamespace from unittest.mock import patch @@ -7,7 +8,8 @@ from unittest.mock import patch import pytest from esphome.components.nrf52.framework import ( - _TOOLCHAIN_VERSION, + _REQUIREMENTS, + TOOLCHAIN_VERSION, _get_toolchain_platform_info, check_and_install, get_sdk_nrf_tools_path, @@ -71,7 +73,7 @@ def nrf52_dirs(setup_core: Path) -> SimpleNamespace: tools = get_sdk_nrf_tools_path() python_env = tools / "penvs" / f"v{_TEST_SDK_VERSION}" framework = tools / "frameworks" / f"v{_TEST_SDK_VERSION}" - toolchain_dir = tools / "toolchains" / _TOOLCHAIN_VERSION + toolchain_dir = tools / "toolchains" / TOOLCHAIN_VERSION for d in (python_env, framework, toolchain_dir): d.mkdir(parents=True, exist_ok=True) zephyr_scripts = framework / "zephyr" / "scripts" @@ -113,6 +115,12 @@ def mock_nrf52_ops(): # --------------------------------------------------------------------------- +def _mark_venv_ready(python_env: Path) -> None: + """Write the venv sentinel with the current requirements hash.""" + requirements_hash = hashlib.sha256(_REQUIREMENTS.read_bytes()).hexdigest() + (python_env / ".ready").write_text(requirements_hash, encoding="utf-8") + + class TestCheckAndInstall: def test_all_installed_skips_all_steps( self, @@ -120,7 +128,7 @@ class TestCheckAndInstall: mock_nrf52_ops: SimpleNamespace, ) -> None: """All three sentinels present → nothing downloaded or compiled.""" - (nrf52_dirs.python_env / ".ready").touch() + _mark_venv_ready(nrf52_dirs.python_env) (nrf52_dirs.python_env / ".zephyr_reqs_ready").touch() (nrf52_dirs.framework / ".ready").touch() (nrf52_dirs.toolchain / ".ready").touch() @@ -157,7 +165,7 @@ class TestCheckAndInstall: mock_nrf52_ops: SimpleNamespace, ) -> None: """Venv ready but framework missing → skip venv creation, run SDK init+update.""" - (nrf52_dirs.python_env / ".ready").touch() + _mark_venv_ready(nrf52_dirs.python_env) check_and_install() @@ -173,7 +181,7 @@ class TestCheckAndInstall: mock_nrf52_ops: SimpleNamespace, ) -> None: """Venv and framework ready → only toolchain downloaded and extracted.""" - (nrf52_dirs.python_env / ".ready").touch() + _mark_venv_ready(nrf52_dirs.python_env) (nrf52_dirs.python_env / ".zephyr_reqs_ready").touch() (nrf52_dirs.framework / ".ready").touch() @@ -202,7 +210,7 @@ class TestCheckAndInstall: mock_nrf52_ops: SimpleNamespace, ) -> None: """Failing west init raises EsphomeError.""" - (nrf52_dirs.python_env / ".ready").touch() + _mark_venv_ready(nrf52_dirs.python_env) mock_nrf52_ops.run_command_ok.return_value = False with pytest.raises(EsphomeError, match="Can't initialize"): @@ -214,7 +222,7 @@ class TestCheckAndInstall: mock_nrf52_ops: SimpleNamespace, ) -> None: """Failing west update raises EsphomeError.""" - (nrf52_dirs.python_env / ".ready").touch() + _mark_venv_ready(nrf52_dirs.python_env) # init succeeds, update fails mock_nrf52_ops.run_command_ok.side_effect = [True, False] @@ -227,7 +235,7 @@ class TestCheckAndInstall: mock_nrf52_ops: SimpleNamespace, ) -> None: """download_from_mirrors receives VERSION + platform triple from _get_toolchain_platform_info.""" - (nrf52_dirs.python_env / ".ready").touch() + _mark_venv_ready(nrf52_dirs.python_env) (nrf52_dirs.framework / ".ready").touch() with patch( @@ -238,7 +246,7 @@ class TestCheckAndInstall: args, _ = mock_nrf52_ops.download_from_mirrors.call_args substitutions = args[1] - assert substitutions["VERSION"] == _TOOLCHAIN_VERSION + assert substitutions["VERSION"] == TOOLCHAIN_VERSION assert substitutions["sysname"] == "linux" assert substitutions["machine"] == "x86_64" assert substitutions["extension"] == "tar.xz"