From a19e817d28f4873fdab42cc5505f46a64fb2fdd6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 12 May 2026 17:29:29 -0500 Subject: [PATCH] [core] Shrink apply_to_core to what upload/logs actually read apply_to_core was over-populating: it restored friendly_name, loaded_integrations, and loaded_platforms even though every consumer of those three lives inside a component validator (esp32_camera, esp32, deep_sleep, zigbee, lvgl, zephyr_mcumgr), and the whole point of the fast path is to skip validation. Drop them. CORE.__init__ already leaves all three at safe defaults (None / empty set) for any incidental reader. What's left is exactly what upload/logs walk: - CORE.name (api.client.run_logs, firmware_bin path, mDNS) - CORE.build_path (firmware_bin / partition_table_bin / bootloader_bin) - CORE.data[KEY_CORE][KEY_TARGET_PLATFORM] (module dispatch, .is_esp32 etc) - CORE.data[KEY_CORE][KEY_TARGET_FRAMEWORK] (.is_arduino, firmware_bin branch) Method body shrinks from 9 statements to 4; setdefault + two conditional inserts collapse into one dict literal; the function-local import moves to module top. Drift surface drops from 7 paired fields to 4. The wizard-only-sidecar None case is gated once at the load_compiled_config boundary so apply_to_core no longer has to defend against it. --- esphome/compiled_config.py | 5 +++ esphome/storage_json.py | 42 +++++++++++------------- tests/unit_tests/test_compiled_config.py | 10 ++++-- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/esphome/compiled_config.py b/esphome/compiled_config.py index bb127a0de9..ac817bd906 100644 --- a/esphome/compiled_config.py +++ b/esphome/compiled_config.py @@ -92,5 +92,10 @@ def load_compiled_config(conf_path: Path) -> ConfigType | None: storage = StorageJSON.load(ext_storage_path(conf_path.name)) if storage is None: return None + # `apply_to_core` assumes the sidecar was written by `from_esphome_core` + # after a real compile, which always sets at least one of these. A + # wizard-only sidecar (no compile) can't drive upload/logs. + if not storage.core_platform and not storage.target_platform: + return None storage.apply_to_core() return config diff --git a/esphome/storage_json.py b/esphome/storage_json.py index a9a83f5896..0fd4a5cc0a 100644 --- a/esphome/storage_json.py +++ b/esphome/storage_json.py @@ -8,7 +8,13 @@ import os from pathlib import Path from esphome import const -from esphome.const import CONF_DISABLED, CONF_MDNS +from esphome.const import ( + CONF_DISABLED, + CONF_MDNS, + KEY_CORE, + KEY_TARGET_FRAMEWORK, + KEY_TARGET_PLATFORM, +) from esphome.core import CORE from esphome.helpers import write_file_if_changed from esphome.types import CoreType @@ -257,31 +263,23 @@ class StorageJSON: return None def apply_to_core(self) -> None: - """Populate ``CORE`` from this sidecar. + """Populate CORE with the metadata upload/logs read. - Used by the ``--from-storage-json`` fast path in - ``esphome upload`` / ``esphome logs``: those subcommands read - a handful of ``CORE`` attributes (``target_platform``, - ``build_path``, ``name``, ``loaded_integrations``) that the - normal flow populates during ``read_config``. Lifting them off - the sidecar lets us skip the validation pass entirely. + Inverse of :meth:`from_esphome_core`'s CORE→StorageJSON + projection. Keep paired -- a new CORE attribute the + ``upload`` / ``logs`` fast path needs has to be captured by + ``from_esphome_core`` too. Validators (``loaded_integrations``, + ``loaded_platforms``, ``friendly_name``) are deliberately not + restored: they're consumed by component validation, which the + fast path skips, and ``CORE.__init__`` already leaves them at + safe defaults. """ - from esphome.const import KEY_CORE, KEY_TARGET_FRAMEWORK, KEY_TARGET_PLATFORM - CORE.name = self.name - CORE.friendly_name = self.friendly_name CORE.build_path = self.build_path - CORE.loaded_integrations = set(self.loaded_integrations) - CORE.loaded_platforms = set(self.loaded_platforms) - - core_platform = self.core_platform or ( - self.target_platform.lower() if self.target_platform else None - ) - CORE.data.setdefault(KEY_CORE, {}) - if core_platform is not None: - CORE.data[KEY_CORE][KEY_TARGET_PLATFORM] = core_platform - if self.framework is not None: - CORE.data[KEY_CORE][KEY_TARGET_FRAMEWORK] = self.framework + CORE.data[KEY_CORE] = { + KEY_TARGET_PLATFORM: self.core_platform or self.target_platform.lower(), + KEY_TARGET_FRAMEWORK: self.framework, + } def __eq__(self, o) -> bool: return isinstance(o, StorageJSON) and self.as_dict() == o.as_dict() diff --git a/tests/unit_tests/test_compiled_config.py b/tests/unit_tests/test_compiled_config.py index d4736f38a3..5dc6df6944 100644 --- a/tests/unit_tests/test_compiled_config.py +++ b/tests/unit_tests/test_compiled_config.py @@ -114,12 +114,18 @@ def test_load_compiled_config_happy_path(fresh_cache_files: Path) -> None: assert config[CONF_API]["encryption"]["key"] == "6dGhpcyBpcyBhIHRlc3Q=" assert config["ota"][0]["password"] == "secret" - # apply_to_core ran as part of the orchestration. + # apply_to_core populated exactly what upload/logs read off CORE. assert CORE.name == "lite_test" assert CORE.build_path == Path("/build/lite_test") assert CORE.data[KEY_CORE][KEY_TARGET_PLATFORM] == "esp32" assert CORE.data[KEY_CORE][KEY_TARGET_FRAMEWORK] == "arduino" - assert "api" in CORE.loaded_integrations + + # The validator-only attributes are deliberately left at their + # CORE.__init__ defaults. The fast path skips validation, so + # nothing reads these. + assert CORE.loaded_integrations == set() + assert CORE.loaded_platforms == set() + assert CORE.friendly_name is None @pytest.mark.parametrize(