From 375ecdfb2c4489300ce281942ee16fad3a0f7bd4 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 3 Jun 2026 17:46:01 -0400 Subject: [PATCH] [esp32][core] Restore ESP-IDF version on logs/upload fast path and clean build on framework change (#16770) --- esphome/storage_json.py | 29 ++++++++++-- esphome/writer.py | 13 ++++-- tests/unit_tests/test_espidf_toolchain.py | 9 ++++ tests/unit_tests/test_storage_json.py | 56 ++++++++++++++++++++++- tests/unit_tests/test_writer.py | 27 +++++++++++ 5 files changed, 126 insertions(+), 8 deletions(-) diff --git a/esphome/storage_json.py b/esphome/storage_json.py index dc1576ab18..65444a2ed8 100644 --- a/esphome/storage_json.py +++ b/esphome/storage_json.py @@ -16,7 +16,7 @@ from esphome.const import ( KEY_TARGET_PLATFORM, Toolchain, ) -from esphome.core import CORE +from esphome.core import CORE, EsphomeError from esphome.helpers import write_file_if_changed from esphome.types import CoreType @@ -101,6 +101,7 @@ class StorageJSON: core_platform: str | None = None, toolchain: str | None = None, area: str | None = None, + framework_version: str | None = None, ) -> None: # Version of the storage JSON schema assert storage_version is None or isinstance(storage_version, int) @@ -141,6 +142,8 @@ class StorageJSON: self.toolchain = toolchain # The area of the node self.area = area + # The framework version the build used (for esp32, the resolved ESP-IDF version) + self.framework_version = framework_version def as_dict(self): return { @@ -162,6 +165,7 @@ class StorageJSON: "core_platform": self.core_platform, "toolchain": self.toolchain, "area": self.area, + "framework_version": self.framework_version, } def to_json(self): @@ -173,10 +177,12 @@ class StorageJSON: @staticmethod def from_esphome_core(esph: CoreType, old: StorageJSON | None) -> StorageJSON: hardware = esph.target_platform.upper() + framework_version: str | None = None if esph.is_esp32: from esphome.components import esp32 hardware = esp32.get_esp32_variant(esph) + framework_version = str(esp32.idf_version()) return StorageJSON( storage_version=1, name=esph.name, @@ -200,6 +206,7 @@ class StorageJSON: core_platform=esph.target_platform, toolchain=esph.toolchain.value if esph.toolchain is not None else None, area=esph.area, + framework_version=framework_version, ) @staticmethod @@ -249,6 +256,7 @@ class StorageJSON: core_platform = storage.get("core_platform") toolchain = storage.get("toolchain") area = storage.get("area") + framework_version = storage.get("framework_version") return StorageJSON( storage_version, name, @@ -268,6 +276,7 @@ class StorageJSON: core_platform, toolchain, area, + framework_version, ) @staticmethod @@ -311,10 +320,24 @@ class StorageJSON: # esp32.get_esp32_variant(). target_platform on disk is the variant # (e.g. "ESP32S3"); core_platform is the family (e.g. "esp32"). if target_platform == const.PLATFORM_ESP32: - from esphome.components.esp32.const import KEY_ESP32 + from esphome.components.esp32.const import KEY_ESP32, KEY_IDF_VERSION from esphome.const import KEY_VARIANT - CORE.data[KEY_ESP32] = {KEY_VARIANT: self.target_platform} + esp32_data = {KEY_VARIANT: self.target_platform} + if self.framework_version: + import esphome.config_validation as cv + + try: + esp32_data[KEY_IDF_VERSION] = cv.Version.parse( + self.framework_version + ) + except ValueError as err: + raise EsphomeError( + f"Could not parse the framework version " + f"{self.framework_version!r} from {storage_path()}. " + f"Please clean the build files and recompile." + ) from err + CORE.data[KEY_ESP32] = esp32_data def __eq__(self, o) -> bool: return isinstance(o, StorageJSON) and self.as_dict() == o.as_dict() diff --git a/esphome/writer.py b/esphome/writer.py index 192c9d68e8..67202ff925 100644 --- a/esphome/writer.py +++ b/esphome/writer.py @@ -93,9 +93,12 @@ def storage_should_clean(old: StorageJSON | None, new: StorageJSON) -> bool: ``src_version`` differs, ``build_path`` differs, the build ``toolchain`` differs (e.g. switching between the PlatformIO and native ESP-IDF toolchains, which produce incompatible build trees), - or a previously loaded integration was removed in *new*. Adding - integrations or changing unrelated fields (friendly name, esphome - version, etc.) does not trigger a clean. + the ``framework`` or ``framework_version`` differs (e.g. switching + arduino <-> esp-idf, or bumping the ESP-IDF version, which also + produce incompatible build trees), or a previously loaded + integration was removed in *new*. Adding integrations or changing + unrelated fields (friendly name, esphome version, etc.) does not + trigger a clean. Used by esphome-device-builder (esphome/device-builder) to gate its remote-build artifact materialiser so a local → remote → local @@ -113,6 +116,10 @@ def storage_should_clean(old: StorageJSON | None, new: StorageJSON) -> bool: return True if old.toolchain != new.toolchain: return True + if old.framework != new.framework: + return True + if old.framework_version != new.framework_version: + return True # Check if any components have been removed return bool(old.loaded_integrations - new.loaded_integrations) diff --git a/tests/unit_tests/test_espidf_toolchain.py b/tests/unit_tests/test_espidf_toolchain.py index adc8bfce63..15e2213816 100644 --- a/tests/unit_tests/test_espidf_toolchain.py +++ b/tests/unit_tests/test_espidf_toolchain.py @@ -56,3 +56,12 @@ def test_get_esphome_esp_idf_paths_no_override(): ) as mock_install: toolchain._get_esphome_esp_idf_paths("5.5.4") mock_install.assert_called_once_with("5.5.4", source_url=None) + + +def test_get_core_framework_version_from_core_data(): + """The version is read from CORE.data when validation populated it.""" + from esphome.components.esp32.const import KEY_ESP32, KEY_IDF_VERSION + import esphome.config_validation as cv + + CORE.data = {KEY_ESP32: {KEY_IDF_VERSION: cv.Version(5, 5, 4)}} + assert toolchain._get_core_framework_version() == "5.5.4" diff --git a/tests/unit_tests/test_storage_json.py b/tests/unit_tests/test_storage_json.py index 2a6f22abb1..5b318008e1 100644 --- a/tests/unit_tests/test_storage_json.py +++ b/tests/unit_tests/test_storage_json.py @@ -8,7 +8,7 @@ from unittest.mock import MagicMock, Mock, patch import pytest -from esphome import storage_json +from esphome import config_validation as cv, storage_json from esphome.const import CONF_DISABLED, CONF_MDNS, Toolchain from esphome.core import CORE @@ -206,6 +206,7 @@ def test_storage_json_as_dict() -> None: framework="arduino", core_platform="esp32", area="Living Room", + framework_version="5.3.1", ) result = storage.as_dict() @@ -235,6 +236,7 @@ def test_storage_json_as_dict() -> None: assert result["framework"] == "arduino" assert result["core_platform"] == "esp32" assert result["area"] == "Living Room" + assert result["framework_version"] == "5.3.1" def test_storage_json_to_json() -> None: @@ -313,8 +315,12 @@ def test_storage_json_from_esphome_core(setup_core: Path) -> None: mock_core.toolchain = Toolchain.ESP_IDF mock_core.area = "Living Room" - with patch("esphome.components.esp32.get_esp32_variant") as mock_variant: + with ( + patch("esphome.components.esp32.get_esp32_variant") as mock_variant, + patch("esphome.components.esp32.idf_version") as mock_idf_version, + ): mock_variant.return_value = "ESP32-C3" + mock_idf_version.return_value = cv.Version(5, 3, 1) result = storage_json.StorageJSON.from_esphome_core(mock_core, old=None) @@ -333,6 +339,7 @@ def test_storage_json_from_esphome_core(setup_core: Path) -> None: assert result.core_platform == "esp32" assert result.toolchain == "esp-idf" assert result.area == "Living Room" + assert result.framework_version == "5.3.1" def test_storage_json_from_esphome_core_mdns_enabled(setup_core: Path) -> None: @@ -545,6 +552,51 @@ def test_storage_json_apply_to_core_ignores_unknown_toolchain( assert CORE.toolchain is None +def test_storage_json_framework_version_round_trip(setup_core: Path) -> None: + """Sidecar framework_version restores CORE.data[esp32][idf_version].""" + from esphome.components.esp32.const import KEY_ESP32, KEY_IDF_VERSION + + storage = _make_storage_with_toolchain("esp-idf") + storage.framework_version = "5.3.1" + path = setup_core / "storage.json" + path.write_text(storage.to_json()) + + assert json.loads(path.read_text())["framework_version"] == "5.3.1" + + loaded = storage_json.StorageJSON.load(path) + assert loaded is not None + assert loaded.framework_version == "5.3.1" + + loaded.apply_to_core() + assert CORE.data[KEY_ESP32][KEY_IDF_VERSION] == cv.Version(5, 3, 1) + + +def test_storage_json_apply_to_core_without_framework_version( + setup_core: Path, +) -> None: + """Older sidecars lacking framework_version don't populate idf_version.""" + from esphome.components.esp32.const import KEY_ESP32, KEY_IDF_VERSION + + loaded = _make_storage_with_toolchain("esp-idf") + assert loaded.framework_version is None + + loaded.apply_to_core() + assert KEY_IDF_VERSION not in CORE.data[KEY_ESP32] + + +def test_storage_json_apply_to_core_raises_on_invalid_framework_version( + setup_core: Path, +) -> None: + """A malformed version string fails with an actionable error at parse time.""" + from esphome.core import EsphomeError + + loaded = _make_storage_with_toolchain("esp-idf") + loaded.framework_version = "not-a-version" + + with pytest.raises(EsphomeError, match="clean the build"): + loaded.apply_to_core() + + def test_esphome_storage_json_as_dict() -> None: """Test EsphomeStorageJSON.as_dict returns correct dictionary.""" storage = storage_json.EsphomeStorageJSON( diff --git a/tests/unit_tests/test_writer.py b/tests/unit_tests/test_writer.py index be37dd5d58..2e3499e8e3 100644 --- a/tests/unit_tests/test_writer.py +++ b/tests/unit_tests/test_writer.py @@ -76,6 +76,7 @@ def create_storage() -> Callable[..., StorageJSON]: framework=kwargs.get("framework", "arduino"), core_platform=kwargs.get("core_platform", "esp32"), toolchain=kwargs.get("toolchain", "platformio"), + framework_version=kwargs.get("framework_version"), ) return _create @@ -121,6 +122,32 @@ def test_storage_should_clean_when_toolchain_changes( assert storage_should_clean(old, new) is True +def test_storage_should_clean_when_framework_changes( + create_storage: Callable[..., StorageJSON], +) -> None: + """Test that clean is triggered when the framework changes. + + Switching between arduino and esp-idf produces incompatible build trees + even on the same toolchain, so the build must be wiped. + """ + old = create_storage(loaded_integrations=["api", "wifi"], framework="arduino") + new = create_storage(loaded_integrations=["api", "wifi"], framework="esp-idf") + assert storage_should_clean(old, new) is True + + +def test_storage_should_clean_when_framework_version_changes( + create_storage: Callable[..., StorageJSON], +) -> None: + """Test that clean is triggered when the framework version changes. + + A different framework/ESP-IDF version compiles against a different SDK, so + the stale build tree must be wiped. + """ + old = create_storage(loaded_integrations=["api", "wifi"], framework_version="5.3.1") + new = create_storage(loaded_integrations=["api", "wifi"], framework_version="5.4.0") + assert storage_should_clean(old, new) is True + + def test_storage_should_clean_when_component_removed( create_storage: Callable[..., StorageJSON], ) -> None: