From 6996b7ed1c2d6aa6abf639b705af0af928a9361c Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Fri, 5 Jun 2026 22:03:08 -0400 Subject: [PATCH] [ci] Add ESP32 Variants clang-tidy run (S3/P4/C6) (#16825) --- .clang-tidy.hash | 2 +- .github/workflows/ci.yml | 88 ++++++++++++++++++++++ .gitignore | 1 + esphome/core/defines.h | 2 + esphome/espidf/clang_tidy.py | 11 ++- platformio.ini | 11 +++ script/clang_tidy_hash.py | 11 +-- sdkconfig.defaults | 5 -- sdkconfig.defaults.esp32c6 | 14 ++++ sdkconfig.defaults.esp32p4 | 31 ++++++++ sdkconfig.defaults.esp32s3 | 12 +++ tests/script/test_clang_tidy_hash.py | 24 ++++++ tests/unit_tests/test_espidf_clang_tidy.py | 41 ++++++++-- 13 files changed, 233 insertions(+), 20 deletions(-) create mode 100644 sdkconfig.defaults.esp32c6 create mode 100644 sdkconfig.defaults.esp32p4 create mode 100644 sdkconfig.defaults.esp32s3 diff --git a/.clang-tidy.hash b/.clang-tidy.hash index 3bcf356f86..e89b4230ad 100644 --- a/.clang-tidy.hash +++ b/.clang-tidy.hash @@ -1 +1 @@ -d583091c0f465aed86a825138e309af6d9db6834106ab424f36712424a6c2223 +d9c755e5f019b2ecb324834717bc1fb8563e622f5751794cb7156d324884481e diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 40267240d8..a0d604f248 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -722,6 +722,93 @@ jobs: run: script/ci-suggest-changes if: always() + clang-tidy-esp32-variants: + name: ${{ matrix.name }} + runs-on: ubuntu-24.04 + needs: + - common + - determine-jobs + if: needs.determine-jobs.outputs.clang-tidy == 'true' + env: + GH_TOKEN: ${{ github.token }} + # The variant tidy envs install ESP-IDF natively; share the native IDF cache. + ESPHOME_ESP_IDF_PREFIX: ~/.esphome-idf + strategy: + fail-fast: false + max-parallel: 3 + matrix: + include: + - id: clang-tidy + name: Run script/clang-tidy for ESP32 S3 + options: --environment esp32s3-idf-tidy --grep USE_ESP32_VARIANT_ESP32S3 + - id: clang-tidy + name: Run script/clang-tidy for ESP32 P4 + # P4 has no native Wi-Fi/BLE; those run over the hosted co-processor, + # so their code paths differ -- lint them under the P4 build too. + # yamllint disable-line rule:line-length + options: --environment esp32p4-idf-tidy --grep USE_ESP32_VARIANT_ESP32P4 --grep USE_ESP32_HOSTED --grep USE_WIFI --grep USE_BLE + - id: clang-tidy + name: Run script/clang-tidy for ESP32 C6 + # yamllint disable-line rule:line-length + options: --environment esp32c6-idf-tidy --grep USE_ESP32_VARIANT_ESP32C6 --grep USE_OPENTHREAD --grep USE_ZIGBEE + + steps: + - name: Check out code from GitHub + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + # Need history for HEAD~1 to work for checking changed files + fetch-depth: 2 + + - name: Restore Python + uses: ./.github/actions/restore-python + with: + python-version: ${{ env.DEFAULT_PYTHON }} + cache-key: ${{ needs.common.outputs.cache-key }} + + - name: Cache ESP-IDF install + # Shared with the IDF/Arduino clang-tidy jobs + native-IDF build (same install). + uses: ./.github/actions/cache-esp-idf + + - name: Register problem matchers + run: | + echo "::add-matcher::.github/workflows/matchers/gcc.json" + echo "::add-matcher::.github/workflows/matchers/clang-tidy.json" + + - name: Check if full clang-tidy scan needed + id: check_full_scan + run: | + . venv/bin/activate + # determine-jobs.clang-tidy-full-scan is true when core C++ changed + # OR the ci-run-all label forced --force-all. Independent of the + # hash check, both must produce a full scan in the job itself. + if [ "${{ needs.determine-jobs.outputs.clang-tidy-full-scan }}" = "true" ]; then + echo "full_scan=true" >> $GITHUB_OUTPUT + echo "reason=determine_jobs" >> $GITHUB_OUTPUT + elif python script/clang_tidy_hash.py --check; then + echo "full_scan=true" >> $GITHUB_OUTPUT + echo "reason=hash_changed" >> $GITHUB_OUTPUT + else + echo "full_scan=false" >> $GITHUB_OUTPUT + echo "reason=normal" >> $GITHUB_OUTPUT + fi + + - name: Run clang-tidy + # Limited variant scan: only the files carrying that variant's code paths + # (no --all-headers; the comprehensive esp32-idf pass covers the shared tree). + run: | + . venv/bin/activate + if [ "${{ steps.check_full_scan.outputs.full_scan }}" = "true" ]; then + echo "Running FULL clang-tidy scan (reason: ${{ steps.check_full_scan.outputs.reason }})" + script/clang-tidy --fix ${{ matrix.options }} + else + echo "Running clang-tidy on changed files only" + script/clang-tidy --fix --changed ${{ matrix.options }} + fi + + - name: Suggested changes + run: script/ci-suggest-changes + if: always() + test-build-components-split: name: Test components batch (${{ matrix.components }}) runs-on: ubuntu-24.04 @@ -1273,6 +1360,7 @@ jobs: - clang-tidy-single - clang-tidy-nosplit - clang-tidy-split + - clang-tidy-esp32-variants - determine-jobs - device-builder - test-build-components-split diff --git a/.gitignore b/.gitignore index 4a4a88fd48..de3e4fa68e 100644 --- a/.gitignore +++ b/.gitignore @@ -141,6 +141,7 @@ tests/.esphome/ sdkconfig.* !sdkconfig.defaults +!sdkconfig.defaults.* .tests/ diff --git a/esphome/core/defines.h b/esphome/core/defines.h index 6c840f56ee..410858f904 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -64,6 +64,7 @@ #define USE_ESP32_BLE_PSRAM #define USE_ESP32_CAMERA_JPEG_CONVERSION #define USE_ESP32_HOSTED +#define USE_ESP32_HOSTED_HTTP_UPDATE #define USE_ESP32_IMPROV_STATE_CALLBACK #define USE_EVENT #define USE_FAN @@ -312,6 +313,7 @@ #define ESPHOME_WIFI_POWER_SAVE_LISTENERS 2 #define USE_WIFI_RUNTIME_POWER_SAVE #define USB_HOST_MAX_REQUESTS 16 +#define USB_HOST_MAX_PACKET_SIZE 64 #define USB_UART_OUTPUT_CHUNK_COUNT 5 #ifdef USE_ARDUINO diff --git a/esphome/espidf/clang_tidy.py b/esphome/espidf/clang_tidy.py index 7647db63f5..62d6f0d00d 100644 --- a/esphome/espidf/clang_tidy.py +++ b/esphome/espidf/clang_tidy.py @@ -339,11 +339,18 @@ def _write_tidy_project( # ESPHome's static-analysis sdkconfig (repo root): enables the flags any # component sets (e.g. CONFIG_BT_ENABLED) so sdkconfig-gated IDF components # register and expose their includes. IDF reads ``sdkconfig.defaults`` from - # the project root. + # the project root, plus a per-target ``sdkconfig.defaults.`` + # for variant-only components (e.g. openthread on c6/h2). + repo_root = esphome_dir.parent (work_dir / "sdkconfig.defaults").write_text( - (esphome_dir.parent / "sdkconfig.defaults").read_text(encoding="utf-8"), + (repo_root / "sdkconfig.defaults").read_text(encoding="utf-8"), encoding="utf-8", ) + target_defaults = repo_root / f"sdkconfig.defaults.{settings.idf_target}" + if target_defaults.is_file(): + (work_dir / target_defaults.name).write_text( + target_defaults.read_text(encoding="utf-8"), encoding="utf-8" + ) def _generate_compile_commands( diff --git a/platformio.ini b/platformio.ini index d7bcc49758..d3fde193b4 100644 --- a/platformio.ini +++ b/platformio.ini @@ -392,6 +392,17 @@ build_flags = ${flags:runtime.build_flags} -DUSE_ESP32_VARIANT_ESP32P4 +[env:esp32p4-idf-tidy] +extends = common:esp32-idf +board = esp32-p4-evboard +board_build.esp-idf.sdkconfig_path = .temp/sdkconfig-esp32p4-idf-tidy +build_flags = + ${common:esp32-idf.build_flags} + ${flags:clangtidy.build_flags} + -DUSE_ESP32_VARIANT_ESP32P4 +build_unflags = + ${common.build_unflags} + ;;;;;;;; ESP32-S2 ;;;;;;;; [env:esp32s2-arduino] diff --git a/script/clang_tidy_hash.py b/script/clang_tidy_hash.py index 1a6e4eb7be..62f76246b4 100755 --- a/script/clang_tidy_hash.py +++ b/script/clang_tidy_hash.py @@ -99,11 +99,12 @@ def calculate_clang_tidy_hash(repo_root: Path | None = None) -> str: platformio_content = read_file_bytes(platformio_path) hasher.update(platformio_content) - # Hash sdkconfig.defaults file - sdkconfig_path = repo_root / "sdkconfig.defaults" - if sdkconfig_path.exists(): - sdkconfig_content = read_file_bytes(sdkconfig_path) - hasher.update(sdkconfig_content) + # Hash sdkconfig.defaults and any per-target sdkconfig.defaults.: + # the per-target files flip CONFIG flags that change which variant code + # paths clang-tidy sees. Include the filename so a rename is detected. + for sdkconfig_path in sorted(repo_root.glob("sdkconfig.defaults*")): + hasher.update(sdkconfig_path.name.encode()) + hasher.update(read_file_bytes(sdkconfig_path)) # Hash esphome/idf_component.yml: its managed deps drive the ESP-IDF # build's include set, which clang-tidy analyzes. diff --git a/sdkconfig.defaults b/sdkconfig.defaults index b277ed18d0..8d177a7e26 100644 --- a/sdkconfig.defaults +++ b/sdkconfig.defaults @@ -15,8 +15,3 @@ CONFIG_BT_ENABLED=y # esp32_camera CONFIG_SPIRAM=y - -# zigbee -CONFIG_ZB_ENABLED=y -CONFIG_ZB_ZED=y -CONFIG_ZB_RADIO_NATIVE=y diff --git a/sdkconfig.defaults.esp32c6 b/sdkconfig.defaults.esp32c6 new file mode 100644 index 0000000000..6dd5f4f329 --- /dev/null +++ b/sdkconfig.defaults.esp32c6 @@ -0,0 +1,14 @@ +# Per-target ESP-IDF sdkconfig defaults for esp32c6 static analysis (clang-tidy) only. +# Read by IDF in addition to sdkconfig.defaults. Enables variant-only components so +# their headers register for the tidy translation unit (these are normally set at +# codegen via add_idf_sdkconfig_option, which the stub tidy build skips). + +# openthread (only the C6/H2 variants have the 802.15.4 radio) +CONFIG_IEEE802154_ENABLED=y +CONFIG_OPENTHREAD_ENABLED=y +CONFIG_OPENTHREAD_RADIO_NATIVE=y + +# zigbee +CONFIG_ZB_ENABLED=y +CONFIG_ZB_ZED=y +CONFIG_ZB_RADIO_NATIVE=y diff --git a/sdkconfig.defaults.esp32p4 b/sdkconfig.defaults.esp32p4 new file mode 100644 index 0000000000..b49dcf0ef2 --- /dev/null +++ b/sdkconfig.defaults.esp32p4 @@ -0,0 +1,31 @@ +# Per-target ESP-IDF sdkconfig defaults for esp32p4 static analysis (clang-tidy) only. +# Read by IDF in addition to sdkconfig.defaults. Enables variant-only components so +# their headers register for the tidy translation unit (these are normally set at +# codegen via add_idf_sdkconfig_option, which the stub tidy build skips). + +# esp32_hosted (P4 has no native Wi-Fi; it drives a co-processor over SDIO/SPI). +# Mirrors a default SDIO 4-bit setup (slot 1, ESP32-C6 slave) so the esp_hosted +# code paths compile under static analysis. +CONFIG_SLAVE_IDF_TARGET_ESP32C6=y +CONFIG_ESP_HOSTED_SDIO_SLOT_1=y +CONFIG_ESP_HOSTED_SDIO_4_BIT_BUS=y +CONFIG_ESP_HOSTED_CUSTOM_SDIO_PINS=y +CONFIG_ESP_HOSTED_SDIO_CLOCK_FREQ_KHZ=40000 +CONFIG_ESP_HOSTED_SDIO_RESET_ACTIVE_HIGH=y +CONFIG_ESP_HOSTED_SDIO_GPIO_RESET_SLAVE=54 +CONFIG_ESP_HOSTED_PRIV_SDIO_PIN_CLK_SLOT_1=18 +CONFIG_ESP_HOSTED_PRIV_SDIO_PIN_CMD_SLOT_1=19 +CONFIG_ESP_HOSTED_PRIV_SDIO_PIN_D0_SLOT_1=14 +CONFIG_ESP_HOSTED_PRIV_SDIO_PIN_D1_4BIT_BUS_SLOT_1=15 +CONFIG_ESP_HOSTED_PRIV_SDIO_PIN_D2_4BIT_BUS_SLOT_1=16 +CONFIG_ESP_HOSTED_PRIV_SDIO_PIN_D3_4BIT_BUS_SLOT_1=17 + +# BLE runs over the hosted co-processor on P4 (no native BT controller), so +# esp32_ble_tracker must take the hosted bluedroid path instead of . +CONFIG_ESP_HOSTED_ENABLE_BT_BLUEDROID=y + +# tinyusb CDC (usb_cdc_acm), same as esp32s3 +CONFIG_TINYUSB_CDC_ENABLED=y +CONFIG_TINYUSB_CDC_COUNT=1 +CONFIG_TINYUSB_CDC_RX_BUFSIZE=256 +CONFIG_TINYUSB_CDC_TX_BUFSIZE=256 diff --git a/sdkconfig.defaults.esp32s3 b/sdkconfig.defaults.esp32s3 new file mode 100644 index 0000000000..15b97eb1b4 --- /dev/null +++ b/sdkconfig.defaults.esp32s3 @@ -0,0 +1,12 @@ +# Per-target ESP-IDF sdkconfig defaults for esp32s3 static analysis (clang-tidy) only. +# Read by IDF in addition to sdkconfig.defaults. Enables variant-only components so +# their headers register for the tidy translation unit (these are normally set at +# codegen via add_idf_sdkconfig_option, which the stub tidy build skips). + +# tinyusb CDC (usb_cdc_acm) -- the esp_tinyusb managed component is already in +# esphome/idf_component.yml; these enable its CDC class so tud_cdc_* and the +# CONFIG_TINYUSB_CDC_* macros are declared. +CONFIG_TINYUSB_CDC_ENABLED=y +CONFIG_TINYUSB_CDC_COUNT=1 +CONFIG_TINYUSB_CDC_RX_BUFSIZE=256 +CONFIG_TINYUSB_CDC_TX_BUFSIZE=256 diff --git a/tests/script/test_clang_tidy_hash.py b/tests/script/test_clang_tidy_hash.py index e19e7886a2..194926a5df 100644 --- a/tests/script/test_clang_tidy_hash.py +++ b/tests/script/test_clang_tidy_hash.py @@ -63,6 +63,7 @@ def test_calculate_clang_tidy_hash_with_sdkconfig(tmp_path: Path) -> None: expected_hasher.update(clang_tidy_content) expected_hasher.update(requirements_version.encode()) expected_hasher.update(platformio_content) + expected_hasher.update(b"sdkconfig.defaults") expected_hasher.update(sdkconfig_content) expected_hash = expected_hasher.hexdigest() @@ -71,6 +72,29 @@ def test_calculate_clang_tidy_hash_with_sdkconfig(tmp_path: Path) -> None: assert result == expected_hash +def test_calculate_clang_tidy_hash_includes_per_target_sdkconfig( + tmp_path: Path, +) -> None: + """Per-target sdkconfig.defaults. files must be part of the hash.""" + (tmp_path / ".clang-tidy").write_bytes(b"Checks: '-*'\n") + (tmp_path / "platformio.ini").write_bytes(b"[env:esp32]\n") + (tmp_path / "requirements_dev.txt").write_text("clang-tidy==18.1.5\n") + (tmp_path / "sdkconfig.defaults").write_bytes(b"CONFIG_BASE=y\n") + + before = clang_tidy_hash.calculate_clang_tidy_hash(repo_root=tmp_path) + + # Adding a per-target file must change the hash. + per_target = tmp_path / "sdkconfig.defaults.esp32c6" + per_target.write_bytes(b"CONFIG_OPENTHREAD_ENABLED=y\n") + after_add = clang_tidy_hash.calculate_clang_tidy_hash(repo_root=tmp_path) + assert after_add != before + + # Editing the per-target file must change the hash again. + per_target.write_bytes(b"CONFIG_OPENTHREAD_ENABLED=n\n") + after_edit = clang_tidy_hash.calculate_clang_tidy_hash(repo_root=tmp_path) + assert after_edit != after_add + + def test_calculate_clang_tidy_hash_without_sdkconfig(tmp_path: Path) -> None: """Test calculating hash without sdkconfig.defaults file.""" clang_tidy_content = b"Checks: '-*,readability-*'\n" diff --git a/tests/unit_tests/test_espidf_clang_tidy.py b/tests/unit_tests/test_espidf_clang_tidy.py index 7a71dc26f4..9791dfc543 100644 --- a/tests/unit_tests/test_espidf_clang_tidy.py +++ b/tests/unit_tests/test_espidf_clang_tidy.py @@ -1,24 +1,51 @@ -"""Tests for esphome.espidf.clang_tidy tidy-project setup.""" +"""Tests for esphome.espidf.clang_tidy tidy-project generation.""" import os from pathlib import Path import pytest -from esphome.espidf.clang_tidy import _Settings, _setup_core +from esphome.espidf.clang_tidy import _Settings, _setup_core, _write_tidy_project + +REPO_ROOT = Path(__file__).resolve().parents[2] -def _settings(target_framework: str) -> _Settings: +def _settings(idf_target: str = "esp32", target_framework: str = "espidf") -> _Settings: return _Settings( - idf_target="esp32", - variant="ESP32", + idf_target=idf_target, + variant=idf_target.upper(), idf_version="5.5.4", target_framework=target_framework, - platform_defines=("USE_ESP32",), + platform_defines=( + "USE_ESP32", + f"USE_ESP32_VARIANT_{idf_target.upper()}", + "USE_ESP_IDF", + ), framework_deps={}, ) +def test_write_tidy_project_copies_base_sdkconfig(tmp_path: Path) -> None: + """The shared sdkconfig.defaults is always copied; no per-target file for esp32.""" + _write_tidy_project(tmp_path, [], {}, _settings("esp32")) + + assert (tmp_path / "sdkconfig.defaults").is_file() + # esp32 has no sdkconfig.defaults.esp32, so nothing extra is copied. + assert not (tmp_path / "sdkconfig.defaults.esp32").exists() + + +def test_write_tidy_project_copies_per_target_sdkconfig(tmp_path: Path) -> None: + """A repo-root sdkconfig.defaults. is also copied into the build dir.""" + _write_tidy_project(tmp_path, [], {}, _settings("esp32c6")) + + target = tmp_path / "sdkconfig.defaults.esp32c6" + assert (tmp_path / "sdkconfig.defaults").is_file() + assert target.is_file() + assert target.read_text(encoding="utf-8") == ( + REPO_ROOT / "sdkconfig.defaults.esp32c6" + ).read_text(encoding="utf-8") + + @pytest.mark.parametrize( ("target_framework", "expected"), [("arduino", "1"), ("espidf", "0")], @@ -34,6 +61,6 @@ def test_setup_core_sets_arduino_env( # restored after the test instead of leaking into later tests. monkeypatch.delenv("ESPHOME_ARDUINO", raising=False) - _setup_core(tmp_path / "proj", _settings(target_framework)) + _setup_core(tmp_path / "proj", _settings(target_framework=target_framework)) assert os.environ["ESPHOME_ARDUINO"] == expected