From d3655597eaf18284983cc288b9140908b9cab7eb Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 15 Jul 2026 22:26:23 -0400 Subject: [PATCH 01/40] [as3935_i2c] Use repeated start when reading registers (#17584) --- esphome/components/as3935_i2c/as3935_i2c.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/esphome/components/as3935_i2c/as3935_i2c.cpp b/esphome/components/as3935_i2c/as3935_i2c.cpp index 4c1020daa7..b3d015114f 100644 --- a/esphome/components/as3935_i2c/as3935_i2c.cpp +++ b/esphome/components/as3935_i2c/as3935_i2c.cpp @@ -24,11 +24,7 @@ void I2CAS3935Component::write_register(uint8_t reg, uint8_t mask, uint8_t bits, uint8_t I2CAS3935Component::read_register(uint8_t reg) { uint8_t value; - if (write(®, 1) != i2c::ERROR_OK) { - ESP_LOGW(TAG, "Writing register failed!"); - return 0; - } - if (read(&value, 1) != i2c::ERROR_OK) { + if (!this->read_byte(reg, &value)) { ESP_LOGW(TAG, "Reading register failed!"); return 0; } From ea01c909b7b500d3b034a2f7ed231cc65a3e97b8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 16 Jul 2026 02:09:30 -1000 Subject: [PATCH 02/40] [micro_wake_word] Include the local model file in bundles (#17604) --- esphome/bundle.py | 37 +++++- .../components/micro_wake_word/__init__.py | 45 ++++++- .../micro_wake_word/__init__.py | 0 .../micro_wake_word/test_init.py | 110 ++++++++++++++++++ tests/unit_tests/test_bundle.py | 65 +++++++++++ 5 files changed, 251 insertions(+), 6 deletions(-) create mode 100644 tests/component_tests/micro_wake_word/__init__.py create mode 100644 tests/component_tests/micro_wake_word/test_init.py diff --git a/esphome/bundle.py b/esphome/bundle.py index d38f68ebfd..88df87c3ba 100644 --- a/esphome/bundle.py +++ b/esphome/bundle.py @@ -7,7 +7,7 @@ and compiled directly: ``esphome compile my_device.esphomebundle.tar.gz`` from __future__ import annotations -from dataclasses import dataclass +from dataclasses import dataclass, field from enum import StrEnum import io import json @@ -32,6 +32,8 @@ from esphome.core import CORE, EsphomeError _LOGGER = logging.getLogger(__name__) +DOMAIN = "bundle" + BUNDLE_EXTENSION = ".esphomebundle.tar.gz" MANIFEST_FILENAME = "manifest.json" CURRENT_MANIFEST_VERSION = 1 @@ -120,6 +122,32 @@ def _find_used_secret_keys(yaml_files: list[Path]) -> set[str]: return keys +@dataclass +class BundleData: + """Files components asked to include, keyed under DOMAIN in CORE.data.""" + + extra_files: list[Path] = field(default_factory=list) + + +def _get_data() -> BundleData: + if DOMAIN not in CORE.data: + CORE.data[DOMAIN] = BundleData() + return CORE.data[DOMAIN] + + +def add_bundle_file(path: Path) -> None: + """Register a file that a bundle must include. + + Bundle discovery walks the validated config, so it only finds files the config + names. Components call this during validation for files it cannot see, such as a + file that is referenced from inside another file. + + A relative path is taken as relative to the config directory. Files outside the + config directory are skipped when the bundle is built. + """ + _get_data().extra_files.append(CORE.relative_config_path(path)) + + @dataclass class BundleFile: """A file to include in the bundle.""" @@ -286,13 +314,18 @@ class ConfigBundleCreator: with known file extensions are also resolved and checked. Core ESPHome concepts that use relative paths or directories - are handled explicitly. + are handled explicitly. Files the config does not name at all are + registered by their component with add_bundle_file(). """ config = self._config # Generic walk: find all file paths in the validated config self._walk_config_for_files(config) + # Files registered by components during validation + for extra_file in _get_data().extra_files: + self._add_file(extra_file) + # --- Core ESPHome concepts needing explicit handling --- # esphome.includes / includes_c - can be relative paths and directories diff --git a/esphome/components/micro_wake_word/__init__.py b/esphome/components/micro_wake_word/__init__.py index cba6bcfa50..4b309551ba 100644 --- a/esphome/components/micro_wake_word/__init__.py +++ b/esphome/components/micro_wake_word/__init__.py @@ -6,6 +6,7 @@ from urllib.parse import urljoin from esphome import automation, external_files, git from esphome.automation import register_action, register_condition +from esphome.bundle import add_bundle_file import esphome.codegen as cg from esphome.components import esp32, microphone, ota, psram import esphome.config_validation as cv @@ -28,6 +29,7 @@ from esphome.const import ( TYPE_LOCAL, ) from esphome.core import CORE, HexInt +from esphome.types import ConfigType _LOGGER = logging.getLogger(__name__) @@ -236,10 +238,45 @@ HTTP_SCHEMA = cv.All( _process_http_source, ) -LOCAL_SCHEMA = cv.Schema( - { - cv.Required(CONF_PATH): cv.All(_validate_json_filename, cv.file_), - } + +def _register_local_model_file(config: ConfigType) -> ConfigType: + """Register the model file that the manifest points to, so bundles include it. + + The manifest names its model file relative to itself, so that path never appears + in the YAML and bundle discovery cannot find it on its own. + + Problems with the manifest are logged and ignored here rather than raised. Loading + the manifest later reports them with better messages, and raising would be + swallowed by the shorthand validator, which then reports a confusing error about a + missing file in a git repository. Logging keeps the skipped registration + diagnosable if the manifest is only briefly unreadable, since the bundle would + then be built without the model file. + """ + manifest_path: Path = config[CONF_PATH] + try: + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + model = manifest[CONF_MODEL] + except (OSError, ValueError, KeyError, TypeError) as err: + _LOGGER.debug("Not registering a model file from %s: %s", manifest_path, err) + return config + if not isinstance(model, str): + _LOGGER.debug( + "Not registering a model file from %s: 'model' is %s, expected a string", + manifest_path, + type(model).__name__, + ) + return config + add_bundle_file(manifest_path.parent / model) + return config + + +LOCAL_SCHEMA = cv.All( + cv.Schema( + { + cv.Required(CONF_PATH): cv.All(_validate_json_filename, cv.file_), + } + ), + _register_local_model_file, ) diff --git a/tests/component_tests/micro_wake_word/__init__.py b/tests/component_tests/micro_wake_word/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/component_tests/micro_wake_word/test_init.py b/tests/component_tests/micro_wake_word/test_init.py new file mode 100644 index 0000000000..5e57653585 --- /dev/null +++ b/tests/component_tests/micro_wake_word/test_init.py @@ -0,0 +1,110 @@ +"""Tests for micro_wake_word local model validation.""" + +import json +import logging +from pathlib import Path +from typing import Any + +import pytest + +from esphome.components.micro_wake_word import LOCAL_SCHEMA +from esphome.core import CORE + +MANIFEST: dict[str, Any] = { + "type": "micro", + "model": "hey_jarvis.tflite", + "author": "someone", + "version": 2, + "wake_word": "hey jarvis", + "trained_languages": ["en"], + "micro": { + "feature_step_size": 10, + "tensor_arena_size": 30000, + "probability_cutoff": 0.97, + "sliding_window_size": 5, + "minimum_esphome_version": "2024.7.0", + }, +} + + +def _registered_files() -> list[Path]: + """Files components registered for bundling this run.""" + data = CORE.data.get("bundle") + return list(data.extra_files) if data else [] + + +@pytest.fixture +def config_dir(tmp_path: Path) -> Path: + """A config dir holding a manifest and its model file.""" + (tmp_path / "models").mkdir() + (tmp_path / "models" / "hey_jarvis.tflite").write_bytes(b"fake model") + (tmp_path / "models" / "hey_jarvis.json").write_text(json.dumps(MANIFEST)) + CORE.config_path = tmp_path / "test.yaml" + return tmp_path + + +def test_local_schema_registers_model_file(config_dir: Path) -> None: + """The model file named by the manifest is registered so bundles include it.""" + LOCAL_SCHEMA({"path": "models/hey_jarvis.json"}) + + assert _registered_files() == [config_dir / "models" / "hey_jarvis.tflite"] + + +def test_local_schema_registers_model_file_in_subdirectory(config_dir: Path) -> None: + """The model reference is resolved relative to the manifest, not the config dir.""" + nested = config_dir / "models" / "nested" + nested.mkdir() + (nested / "model.tflite").write_bytes(b"fake model") + (config_dir / "models" / "nested.json").write_text( + json.dumps({**MANIFEST, "model": "nested/model.tflite"}) + ) + + LOCAL_SCHEMA({"path": "models/nested.json"}) + + assert _registered_files() == [nested / "model.tflite"] + + +def test_local_schema_leaves_config_untouched(config_dir: Path) -> None: + """Registration is a side effect; the model file is not a config key.""" + config = LOCAL_SCHEMA({"path": "models/hey_jarvis.json"}) + + assert config == {"path": config_dir / "models" / "hey_jarvis.json"} + + +def test_local_schema_missing_model_file_still_validates(config_dir: Path) -> None: + """A model file that does not exist is registered, not rejected. + + Raising here would be swallowed by the shorthand validator, which would then + report a confusing error about a missing file in a git repository. + """ + (config_dir / "models" / "hey_jarvis.tflite").unlink() + + LOCAL_SCHEMA({"path": "models/hey_jarvis.json"}) + + assert _registered_files() == [config_dir / "models" / "hey_jarvis.tflite"] + + +@pytest.mark.parametrize( + "contents", + [ + pytest.param("{not valid json", id="malformed"), + pytest.param(json.dumps({"type": "micro"}), id="no_model_key"), + pytest.param(json.dumps(["a", "list"]), id="not_an_object"), + pytest.param(json.dumps({"model": 42}), id="model_not_a_string"), + ], +) +def test_local_schema_bad_manifest_does_not_raise( + config_dir: Path, contents: str, caplog: pytest.LogCaptureFixture +) -> None: + """Manifest problems are left to later stages, which report them better. + + The skipped registration is logged so a bundle built without the model file can + be diagnosed. + """ + (config_dir / "models" / "hey_jarvis.json").write_text(contents) + + with caplog.at_level(logging.DEBUG): + LOCAL_SCHEMA({"path": "models/hey_jarvis.json"}) + + assert _registered_files() == [] + assert "Not registering a model file" in caplog.text diff --git a/tests/unit_tests/test_bundle.py b/tests/unit_tests/test_bundle.py index f15bbf2e29..6cecb63c2d 100644 --- a/tests/unit_tests/test_bundle.py +++ b/tests/unit_tests/test_bundle.py @@ -22,6 +22,7 @@ from esphome.bundle import ( _add_bytes_to_tar, _default_target_dir, _find_used_secret_keys, + add_bundle_file, extract_bundle, is_bundle_path, prepare_bundle_for_compile, @@ -611,6 +612,70 @@ def test_discover_files_includes_config(tmp_path: Path) -> None: assert "test.yaml" in paths +def test_discover_files_includes_registered_files(tmp_path: Path) -> None: + """Files registered with add_bundle_file() are included. + + The config does not name them, so discovery cannot find them on its own. + """ + config_dir = _setup_config_dir( + tmp_path, + files={"models/model.tflite": "fake model data"}, + ) + add_bundle_file(config_dir / "models" / "model.tflite") + + creator = ConfigBundleCreator({}) + files = creator.discover_files() + + paths = [f.path for f in files] + assert "models/model.tflite" in paths + + +def test_discover_files_registered_relative_file(tmp_path: Path) -> None: + """A relative registered path is taken as relative to the config directory. + + Not the working directory, which is where Path.resolve() would put it. + """ + _setup_config_dir( + tmp_path, + files={"models/model.tflite": "fake model data"}, + ) + add_bundle_file(Path("models/model.tflite")) + + creator = ConfigBundleCreator({}) + files = creator.discover_files() + + paths = [f.path for f in files] + assert "models/model.tflite" in paths + + +def test_discover_files_registered_file_outside_config_dir(tmp_path: Path) -> None: + """A registered file outside the config directory is skipped, not bundled.""" + _setup_config_dir(tmp_path) + outside = tmp_path / "outside.tflite" + outside.write_text("fake model data") + add_bundle_file(outside) + + creator = ConfigBundleCreator({}) + files = creator.discover_files() + + assert [f.path for f in files] == ["test.yaml"] + + +def test_discover_files_registered_file_deduplicated(tmp_path: Path) -> None: + """Registering the same file twice adds it once.""" + config_dir = _setup_config_dir( + tmp_path, + files={"models/model.tflite": "fake model data"}, + ) + add_bundle_file(config_dir / "models" / "model.tflite") + add_bundle_file(config_dir / "models" / "model.tflite") + + creator = ConfigBundleCreator({}) + files = creator.discover_files() + + assert [f.path for f in files].count("models/model.tflite") == 1 + + def test_discover_files_finds_path_objects(tmp_path: Path) -> None: """Path objects in validated config are discovered.""" config_dir = _setup_config_dir( From 95a01ac2ab71f21397dc83be5d1c99de5c58e6e7 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:22:19 -0400 Subject: [PATCH 03/40] [core] Improve framework mirror selection, download errors, and version parsing (#17615) --- esphome/config_validation.py | 6 +- esphome/espidf/framework.py | 31 ++++-- esphome/framework_helpers.py | 71 +++++++++++-- tests/component_tests/esp32/test_esp32.py | 25 +++++ tests/unit_tests/test_config_validation.py | 36 ++++++- tests/unit_tests/test_espidf_framework.py | 23 +++++ tests/unit_tests/test_framework_helpers.py | 111 ++++++++++++++++++++- 7 files changed, 281 insertions(+), 22 deletions(-) diff --git a/esphome/config_validation.py b/esphome/config_validation.py index 16f0a63aa0..3f7c8ff783 100644 --- a/esphome/config_validation.py +++ b/esphome/config_validation.py @@ -422,12 +422,14 @@ class Version: @classmethod def parse(cls, value: str) -> Version: - match = re.match(r"^(\d+).(\d+).(\d+)[-.]?(\w*)$", value) + # The patch component is optional and defaults to 0, so "6.0" and + # "6.0-rc1" parse as 6.0.0 and 6.0.0-rc1. + match = re.match(r"^(\d+)\.(\d+)(?:\.(\d+))?[-.]?(\w*)$", value) if match is None: raise ValueError(f"Not a valid version number {value}") major = int(match[1]) minor = int(match[2]) - patch = int(match[3]) + patch = int(match[3] or 0) extra = match[4] or "" return Version(major=major, minor=minor, patch=patch, extra=extra) diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 810a63476f..18aa966bff 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -63,7 +63,7 @@ ESPHOME_IDF_FRAMEWORK_MIRRORS = str_to_lst_of_str( os.environ.get("ESPHOME_IDF_FRAMEWORK_MIRRORS") or [ "https://github.com/esphome-libs/esp-idf/releases/download/v{VERSION}/esp-idf-v{VERSION}.tar.xz", - "https://github.com/esphome-libs/esp-idf/releases/download/v{MAJOR}.{MINOR}{EXTRA}/esp-idf-v{MAJOR}.{MINOR}{EXTRA}.tar.xz", + "https://github.com/esphome-libs/esp-idf/releases/download/v{SHORT_VERSION}/esp-idf-v{SHORT_VERSION}.tar.xz", ] ) @@ -536,10 +536,14 @@ def _check_esphome_idf_framework_install( env: Optional dictionary of environment variables to set source_url: Optional override URL for the framework tarball. Supports the same ``{VERSION}`` / ``{MAJOR}`` / ``{MINOR}`` / ``{PATCH}`` / - ``{EXTRA}`` substitutions as ESPHOME_IDF_FRAMEWORK_MIRRORS - (``{EXTRA}`` includes its leading ``-``, e.g. ``-rc1``, or is empty). - When set, it replaces the default mirror list — no implicit fallback, - so a misspelled URL fails loudly. + ``{EXTRA}`` / ``{SHORT_VERSION}`` substitutions as + ESPHOME_IDF_FRAMEWORK_MIRRORS (``{EXTRA}`` includes its leading + ``-``, e.g. ``-rc1``, or is empty; ``{SHORT_VERSION}`` is ``x.y`` + plus any extra and only available for x.y.0 versions — a URL + referencing it is skipped for other versions). When set, it + replaces the default mirror list — no implicit fallback, so a + misspelled or skipped URL fails loudly with an EsphomeError naming + the URL. Returns: tuple of (framework_path, install_flag) @@ -588,7 +592,11 @@ def _check_esphome_idf_framework_install( with tempfile.NamedTemporaryFile() as tmp: _LOGGER.info("Downloading ESP-IDF %s framework ...", version) - # Create substitutions for the URLs + # Create substitutions for the URLs. SHORT_VERSION (x.y with + # optional -extra) is only provided for x.y.0 releases, since + # the vX.Y release tags only exist for those; templates that + # reference it are skipped for other versions by + # download_from_mirrors. substitutions = {"VERSION": version} try: ver = Version.parse(version) @@ -596,8 +604,17 @@ def _check_esphome_idf_framework_install( substitutions["MINOR"] = str(ver.minor) substitutions["PATCH"] = str(ver.patch) substitutions["EXTRA"] = f"-{ver.extra}" if ver.extra else "" + if ver.patch == 0: + substitutions["SHORT_VERSION"] = ( + f"{ver.major}.{ver.minor}{substitutions['EXTRA']}" + ) except ValueError: - pass + _LOGGER.warning( + "ESP-IDF version '%s' is not a valid version number; " + "only the {VERSION} substitution is available for " + "mirror URLs", + version, + ) mirrors = [source_url] if source_url else ESPHOME_IDF_FRAMEWORK_MIRRORS download_from_mirrors(mirrors, substitutions, tmp.file) diff --git a/esphome/framework_helpers.py b/esphome/framework_helpers.py index 70d440d995..6c055dded3 100644 --- a/esphome/framework_helpers.py +++ b/esphome/framework_helpers.py @@ -552,6 +552,17 @@ def archive_extract_all( matched_fct(archive_ref, extract_dir, progress_header=progress_header) +def _failure_reason(e: Exception) -> str: + """Format a download exception for the aggregated error message. + + ``requests`` appends " for url: " to HTTP errors; the URL is already + printed on the line above, so strip the suffix to keep lines short. Falls + back to the repr for exceptions with no message (e.g. ``TimeoutError()``) + so the line always names the failure. + """ + return str(e).split(" for url: ", maxsplit=1)[0] or repr(e) + + def download_from_mirrors( mirrors: list[str], substitutions: dict[str, str], @@ -570,14 +581,22 @@ def download_from_mirrors( Returns: The source URL. + Mirror URL templates that reference a substitution not present in + ``substitutions`` are skipped, so callers can offer templates that only + apply to some downloads. + Raises: ValueError: If mirrors list is empty. - Exception: If all download attempts fail. + EsphomeError: If all download attempts fail; the message lists every + attempted URL with its individual failure reason. Also raised if + no template matched the provided substitutions. """ # Imported lazily: requests is a heavy import (~85ms) and is only needed # when actually downloading a toolchain, never during config validation. import requests + from esphome.core import EsphomeError + # 1. Open target file for writing if path given with ExitStack() as stack: if isinstance(target, (str, os.PathLike)): @@ -590,13 +609,31 @@ def download_from_mirrors( ) # 2. Try each mirror in order - last_exception = None + failures: list[tuple[str, Exception]] = [] + skipped: list[tuple[str, str]] = [] for mirror in mirrors: # 3. Apply substitutions to URL - url = mirror.format(**substitutions) + try: + url = mirror.format(**substitutions) + except KeyError as e: + # The template references a substitution not provided for + # this download (e.g. SHORT_VERSION only exists for x.y.0 + # versions) - expected, the template just doesn't apply. + _LOGGER.debug("Skipping mirror %s: %s not available", mirror, e) + skipped.append((mirror, f"not applicable ({e.args[0]} not available)")) + continue + except (IndexError, ValueError) as e: + # A malformed template (unbalanced braces, bad format spec) + # is an authoring error, not an expected fallthrough - warn + # even if a later mirror succeeds. + _LOGGER.warning( + "Skipping malformed mirror URL template %s: %r", mirror, e + ) + skipped.append((mirror, f"skipped ({e!r})")) + continue - _LOGGER.debug("Trying downloading from %s", url) + _LOGGER.debug("Trying to download from %s", url) try: # 4. Reset file pointer and download @@ -631,9 +668,27 @@ def download_from_mirrors( except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught _LOGGER.debug("Failed to download %s: %s", url, str(e)) - last_exception = e + failures.append((url, e)) - # 7. Raise last exception if all mirrors failed - if last_exception: - raise last_exception + # 7. Report every attempted URL if all mirrors failed. Falling back + # past an early mirror is normal (e.g. only one of the framework URL + # templates matches a given version's tag), so raising only the last + # error would hide the failure that actually matters. + if failures: + attempts = "".join( + f"\n {url}\n {_failure_reason(e)}" for url, e in failures + ) + attempts += "".join( + f"\n {mirror}\n {reason}" for mirror, reason in skipped + ) + raise EsphomeError( + f"Failed to download from all mirrors:{attempts}" + ) from failures[0][1] + if skipped: + details = "".join( + f"\n {mirror}\n {reason}" for mirror, reason in skipped + ) + raise EsphomeError( + f"No mirror URL template matched the provided substitutions:{details}" + ) raise ValueError("download_from_mirrors called with an empty mirrors list") diff --git a/tests/component_tests/esp32/test_esp32.py b/tests/component_tests/esp32/test_esp32.py index fdca70bf2c..8a116ccc27 100644 --- a/tests/component_tests/esp32/test_esp32.py +++ b/tests/component_tests/esp32/test_esp32.py @@ -740,3 +740,28 @@ def test_signed_ota_keys_invalid_combinations(config: dict, match: str) -> None: with pytest.raises(cv.Invalid, match=match): _validate_signed_ota_keys(config) + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + # Full x.y.z versions are rewritten into pioarduino release URLs + ( + "55.3.30", + "https://github.com/pioarduino/platform-espressif32/releases/download/55.03.30/platform-espressif32.zip", + ), + ( + "55.3.31-2", + "https://github.com/pioarduino/platform-espressif32/releases/download/55.03.31-2/platform-espressif32.zip", + ), + # Non-version values pass through untouched + ( + "https://github.com/pioarduino/platform-espressif32.git#develop", + "https://github.com/pioarduino/platform-espressif32.git#develop", + ), + ], +) +def test_parse_pio_platform_version(value: str, expected: str) -> None: + from esphome.components.esp32 import _parse_pio_platform_version + + assert _parse_pio_platform_version(value) == expected diff --git a/tests/unit_tests/test_config_validation.py b/tests/unit_tests/test_config_validation.py index 17dfaad9b8..fd21ac92ea 100644 --- a/tests/unit_tests/test_config_validation.py +++ b/tests/unit_tests/test_config_validation.py @@ -1436,9 +1436,41 @@ def test_version_parse_with_extra() -> None: assert version.extra == "dev20240101" -def test_version_parse_invalid() -> None: +def test_version_parse_without_patch() -> None: + """A two-part version parses with patch defaulting to 0, so framework + shorthands like '6.0' and '6.0-rc1' are accepted.""" + version = cv.Version.parse("6.0") + assert (version.major, version.minor, version.patch, version.extra) == ( + 6, + 0, + 0, + "", + ) + version = cv.Version.parse("6.0-rc1") + assert (version.major, version.minor, version.patch, version.extra) == ( + 6, + 0, + 0, + "rc1", + ) + + +def test_version_parse_numeric_extra() -> None: + """Four-part versions keep the trailing component as extra (pioarduino + packaging revisions, e.g. 5.5.3.1).""" + version = cv.Version.parse("5.5.3.1") + assert (version.major, version.minor, version.patch, version.extra) == ( + 5, + 5, + 3, + "1", + ) + + +@pytest.mark.parametrize("value", ["not.a.version", "6", "a.b", ""]) +def test_version_parse_invalid(value: str) -> None: with pytest.raises(ValueError, match="Not a valid version number"): - cv.Version.parse("not.a.version") + cv.Version.parse(value) def test_version_is_beta() -> None: diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index c5d9ddbaf1..de02a6b227 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -489,6 +489,29 @@ def test_check_esp_idf_install_unparseable_version( espidf_mocks.extract.assert_called_once() +@pytest.mark.parametrize( + ("version", "short_version"), + [ + ("6.0.0", "6.0"), + ("6.0.0-rc1", "6.0-rc1"), + ("5.5.4", None), # vX.Y tags only exist for X.Y.0 releases + ], +) +def test_check_esp_idf_install_short_version_substitution( + espidf_mocks: SimpleNamespace, version: str, short_version: str | None +) -> None: + """SHORT_VERSION is only offered for x.y.0 releases, so the vX.Y mirror + template is never tried for versions whose tag cannot exist.""" + _get_framework_path(version).mkdir(parents=True, exist_ok=True) + check_esp_idf_install(version, force=True) + + # First call downloads the framework archive; a later call fetches the + # constraints file with its own substitutions. + substitutions = espidf_mocks.download.call_args_list[0][0][1] + assert substitutions.get("SHORT_VERSION") == short_version + assert substitutions["VERSION"] == version + + # --------------------------------------------------------------------------- # _patch_tools_json_for_linux_arm64 (arm64-only ninja backport) # --------------------------------------------------------------------------- diff --git a/tests/unit_tests/test_framework_helpers.py b/tests/unit_tests/test_framework_helpers.py index 69b9f20eaa..e662d2d015 100644 --- a/tests/unit_tests/test_framework_helpers.py +++ b/tests/unit_tests/test_framework_helpers.py @@ -16,6 +16,7 @@ import zipfile import pytest import requests as req +from esphome.core import EsphomeError from esphome.framework_helpers import ( _7z_extract_all, _detect_archive_root, @@ -546,6 +547,99 @@ class TestDownloadFromMirrors: ) assert mock_get.call_args[0][0] == "https://example.com/1.2.3.bin" + def test_template_with_missing_substitution_is_skipped( + self, tmp_path: Path + ) -> None: + """A template referencing an unavailable substitution is skipped, not + formatted into a bogus URL (e.g. SHORT_VERSION only exists for x.y.0 + framework versions).""" + with patch( + "requests.get", + return_value=_mock_response(b"x"), + ) as mock_get: + url = download_from_mirrors( + [ + "https://example.com/{SHORT_VERSION}.bin", + "https://example.com/{VERSION}.bin", + ], + {"VERSION": "1.2.3"}, + tmp_path / "out.bin", + ) + assert url == "https://example.com/1.2.3.bin" + assert mock_get.call_count == 1 + + def test_all_templates_skipped_raises_esphome_error(self, tmp_path: Path) -> None: + with ( + patch("requests.get") as mock_get, + pytest.raises(EsphomeError, match="No mirror URL template matched") as ei, + ): + download_from_mirrors( + ["https://example.com/{MISSING}.bin"], + {"VERSION": "1.2.3"}, + tmp_path / "out.bin", + ) + mock_get.assert_not_called() + # The skipped template and its missing substitution are named + assert "https://example.com/{MISSING}.bin" in str(ei.value) + assert "MISSING" in str(ei.value) + + def test_failure_message_includes_skipped_templates(self, tmp_path: Path) -> None: + """When downloads fail, templates that were skipped for missing + substitutions are also listed so a typo'd custom mirror is + attributable.""" + with ( + patch( + "requests.get", + return_value=_mock_response(b"", ok=False), + ), + pytest.raises(EsphomeError, match="all mirrors") as ei, + ): + download_from_mirrors( + [ + "https://example.com/{TYPO}.bin", + "https://example.com/{VERSION}.bin", + ], + {"VERSION": "1.2.3"}, + tmp_path / "out.bin", + ) + message = str(ei.value) + assert "https://example.com/1.2.3.bin" in message + assert ( + "https://example.com/{TYPO}.bin\n not applicable (TYPO not available)" + in message + ) + + def test_malformed_template_warns_and_is_reported( + self, tmp_path: Path, caplog: pytest.LogCaptureFixture + ) -> None: + """A structurally malformed template is an authoring error: warned + about even when another mirror succeeds, and named in the aggregate + error when everything fails.""" + with ( + patch("requests.get", return_value=_mock_response(b"x")), + caplog.at_level(logging.WARNING, logger="esphome.framework_helpers"), + ): + url = download_from_mirrors( + ["https://example.com/{oops.bin", "https://example.com/{VERSION}.bin"], + {"VERSION": "1.2.3"}, + tmp_path / "out.bin", + ) + assert url == "https://example.com/1.2.3.bin" + assert "malformed mirror URL template" in caplog.text + + with ( + patch("requests.get", return_value=_mock_response(b"", ok=False)), + pytest.raises(EsphomeError, match="all mirrors") as ei, + ): + download_from_mirrors( + ["https://example.com/{oops.bin", "https://example.com/{VERSION}.bin"], + {"VERSION": "1.2.3"}, + tmp_path / "out.bin", + ) + assert "https://example.com/{oops.bin\n skipped (ValueError(" in str( + ei.value + ) + def test_falls_back_to_second_mirror(self, tmp_path: Path) -> None: with patch( "requests.get", @@ -559,15 +653,26 @@ class TestDownloadFromMirrors: assert url == "https://mirror2.com/f" assert (tmp_path / "out.bin").read_bytes() == b"second" - def test_all_mirrors_fail_reraises_last_exception(self, tmp_path: Path) -> None: + def test_all_mirrors_fail_raises_error_listing_every_attempt( + self, tmp_path: Path + ) -> None: with ( patch( "requests.get", return_value=_mock_response(b"", ok=False), ), - pytest.raises(req.HTTPError), + pytest.raises(EsphomeError, match="all mirrors") as excinfo, ): - download_from_mirrors(["https://example.com/f"], {}, tmp_path / "out.bin") + download_from_mirrors( + ["https://mirror1.com/f", "https://mirror2.com/f"], + {}, + tmp_path / "out.bin", + ) + # Every attempted URL appears in the message, and the first mirror's + # exception (the primary URL, usually the one that matters) is chained. + assert "https://mirror1.com/f" in str(excinfo.value) + assert "https://mirror2.com/f" in str(excinfo.value) + assert isinstance(excinfo.value.__cause__, req.HTTPError) def test_empty_mirrors_raises_value_error(self, tmp_path: Path) -> None: with pytest.raises(ValueError, match="empty mirrors list"): From 2797349c7514c2953b259b9cb98b94c3b71eef9e Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:34:49 -0400 Subject: [PATCH 04/40] [http_request] Fix use-after-return of header collection state in IDF backend (#17627) --- .../components/http_request/http_request_idf.cpp | 15 +++++---------- .../components/http_request/http_request_idf.h | 2 ++ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index 3e341395a4..a437540241 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -19,11 +19,6 @@ namespace esphome::http_request { static const char *const TAG = "http_request.idf"; static constexpr uint32_t ERROR_DURATION_MS = 1000; -struct UserData { - const std::vector &lower_case_collect_headers; - std::vector
&response_headers; -}; - void HttpRequestIDF::dump_config() { HttpRequestComponent::dump_config(); ESP_LOGCONFIG(TAG, @@ -34,15 +29,15 @@ void HttpRequestIDF::dump_config() { } esp_err_t HttpRequestIDF::http_event_handler(esp_http_client_event_t *evt) { - UserData *user_data = (UserData *) evt->user_data; + auto *container = (HttpContainerIDF *) evt->user_data; switch (evt->event_id) { case HTTP_EVENT_ON_HEADER: { const std::string header_name = str_lower_case(evt->header_key); // NOLINT - if (should_collect_header(user_data->lower_case_collect_headers, header_name)) { + if (should_collect_header(container->collect_headers_, header_name)) { const std::string header_value = evt->header_value; ESP_LOGD(TAG, "Received response header, name: %s, value: %s", header_name.c_str(), header_value.c_str()); - user_data->response_headers.push_back({header_name, header_value}); + container->response_headers_.push_back({header_name, header_value}); } break; } @@ -124,8 +119,8 @@ std::shared_ptr HttpRequestIDF::perform(const std::string &url, c container->set_secure(secure); - auto user_data = UserData{lower_case_collect_headers, container->response_headers_}; - esp_http_client_set_user_data(client, static_cast(&user_data)); + container->collect_headers_ = lower_case_collect_headers; + esp_http_client_set_user_data(client, static_cast(container.get())); for (const auto &header : request_headers) { esp_http_client_set_header(client, header.name.c_str(), header.value.c_str()); diff --git a/esphome/components/http_request/http_request_idf.h b/esphome/components/http_request/http_request_idf.h index 8a803b5469..16a5b6a161 100644 --- a/esphome/components/http_request/http_request_idf.h +++ b/esphome/components/http_request/http_request_idf.h @@ -24,6 +24,8 @@ class HttpContainerIDF : public HttpContainer { protected: friend class HttpRequestIDF; esp_http_client_handle_t client_; + // Owned copy (not a reference): must outlive perform() for the response-header event handler + std::vector collect_headers_; }; class HttpRequestIDF final : public HttpRequestComponent { From 05e2c6b133175a43151ac9f7a5fee770fc30ba05 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:59:53 -0400 Subject: [PATCH 05/40] [web_server_idf] Use core format_hex_to helper for digest auth (fixes Arduino build) (#17608) --- .../web_server_idf/web_server_idf.cpp | 20 +++++-------------- .../components/web_server/test.esp32-ard.yaml | 6 ++++++ 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/esphome/components/web_server_idf/web_server_idf.cpp b/esphome/components/web_server_idf/web_server_idf.cpp index bf5a8666dc..993fb6c035 100644 --- a/esphome/components/web_server_idf/web_server_idf.cpp +++ b/esphome/components/web_server_idf/web_server_idf.cpp @@ -381,16 +381,6 @@ void AsyncWebServerRequest::init_response_(AsyncWebServerResponse *rsp, int code #ifdef USE_WEBSERVER_AUTH_DIGEST namespace { -// Hex-encode `len` bytes into `out`, which must hold at least 2 * len + 1 bytes. Null-terminated. -void bytes_to_hex(const uint8_t *data, size_t len, char *out) { - static const char HEX[] = "0123456789abcdef"; - for (size_t i = 0; i < len; i++) { - out[i * 2] = HEX[data[i] >> 4]; - out[i * 2 + 1] = HEX[data[i] & 0x0f]; - } - out[len * 2] = '\0'; -} - // Extract the value of a Digest auth parameter (e.g. "nonce") from the comma-separated // parameter list. Values may be quoted or bare. Returns an empty ref when the key is absent. // Only whole parameter names match, so "nc" does not match inside "cnonce". @@ -468,7 +458,7 @@ bool check_digest_auth(const char *username, const char *password, const std::st esp_rom_md5_update(&ctx, ":", 1); esp_rom_md5_update(&ctx, password, strlen(password)); esp_rom_md5_final(digest, &ctx); - bytes_to_hex(digest, sizeof(digest), ha1); + format_hex_to(ha1, digest, sizeof(digest)); // HA2 = MD5(method:uri) -- uses the uri the client echoed back. char ha2[33]; @@ -477,7 +467,7 @@ bool check_digest_auth(const char *username, const char *password, const std::st esp_rom_md5_update(&ctx, ":", 1); esp_rom_md5_update(&ctx, uri.c_str(), uri.size()); esp_rom_md5_final(digest, &ctx); - bytes_to_hex(digest, sizeof(digest), ha2); + format_hex_to(ha2, digest, sizeof(digest)); // expected = MD5(HA1:nonce:nc:cnonce:qop:HA2) char expected[33]; @@ -494,7 +484,7 @@ bool check_digest_auth(const char *username, const char *password, const std::st esp_rom_md5_update(&ctx, ":", 1); esp_rom_md5_update(&ctx, ha2, 32); esp_rom_md5_final(digest, &ctx); - bytes_to_hex(digest, sizeof(digest), expected); + format_hex_to(expected, digest, sizeof(digest)); // Constant-time comparison of the two 32-char hex digests. uint8_t result = 0; @@ -592,9 +582,9 @@ void AsyncWebServerRequest::requestAuthentication() const { char opaque[33]; char header[160]; esp_fill_random(random_bytes, sizeof(random_bytes)); - bytes_to_hex(random_bytes, sizeof(random_bytes), nonce); + format_hex_to(nonce, random_bytes, sizeof(random_bytes)); esp_fill_random(random_bytes, sizeof(random_bytes)); - bytes_to_hex(random_bytes, sizeof(random_bytes), opaque); + format_hex_to(opaque, random_bytes, sizeof(random_bytes)); snprintf(header, sizeof(header), R"(Digest realm="Login Required", qop="auth", nonce="%s", opaque="%s")", nonce, opaque); httpd_resp_set_hdr(*this, "WWW-Authenticate", header); diff --git a/tests/components/web_server/test.esp32-ard.yaml b/tests/components/web_server/test.esp32-ard.yaml index 11ad5456ef..2e091b905a 100644 --- a/tests/components/web_server/test.esp32-ard.yaml +++ b/tests/components/web_server/test.esp32-ard.yaml @@ -1,2 +1,8 @@ packages: web_server: !include common_v2.yaml + +web_server: + auth: + username: admin + password: password + type: digest From 3a10d2c1873daf75e9ce852930b4aa45af95996a Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Sat, 18 Jul 2026 00:04:35 +1200 Subject: [PATCH 06/40] [nrf52] Set ZEPHYR_SDK_INSTALL_DIR for Zephyr SDK discovery (#17633) --- esphome/components/nrf52/framework.py | 10 +++++- tests/unit_tests/test_nrf52_framework.py | 39 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/esphome/components/nrf52/framework.py b/esphome/components/nrf52/framework.py index fa6f7d57ad..623cd4eef3 100644 --- a/esphome/components/nrf52/framework.py +++ b/esphome/components/nrf52/framework.py @@ -133,7 +133,15 @@ 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") + # ZEPHYR_SDK_INSTALL_DIR is the variable Zephyr documents for pointing at + # the SDK: FindZephyr-sdk.cmake reads it (from the environment, via + # zephyr_get) and passes it straight to find_package as a HINT. This + # matters because the SDK lives in the esphome cache dir, which is not on + # the module's static search path (/usr, /opt, $HOME, ...). A generic + # "Zephyr-sdk_DIR" environment hint proved unreliable here: containerized + # non-root builds failed to locate the SDK with it, while + # ZEPHYR_SDK_INSTALL_DIR fixed the same invocation. + env["ZEPHYR_SDK_INSTALL_DIR"] = str(_get_toolchain_path(TOOLCHAIN_VERSION)) return env diff --git a/tests/unit_tests/test_nrf52_framework.py b/tests/unit_tests/test_nrf52_framework.py index bb5bc8c064..830e9efba5 100644 --- a/tests/unit_tests/test_nrf52_framework.py +++ b/tests/unit_tests/test_nrf52_framework.py @@ -1,6 +1,7 @@ """Tests for esphome.components.nrf52.framework helpers.""" import hashlib +import os from pathlib import Path from types import SimpleNamespace from unittest.mock import patch @@ -12,11 +13,13 @@ from esphome.components.nrf52.framework import ( TOOLCHAIN_VERSION, _get_toolchain_platform_info, check_and_install, + get_build_env, get_sdk_nrf_tools_path, ) from esphome.config_validation import Version from esphome.const import KEY_CORE, KEY_FRAMEWORK_VERSION from esphome.core import CORE, EsphomeError +from esphome.framework_helpers import get_python_env_executable_path @pytest.fixture(autouse=True) @@ -252,6 +255,42 @@ class TestCheckAndInstall: assert substitutions["extension"] == "tar.xz" +# --------------------------------------------------------------------------- +# get_build_env tests +# --------------------------------------------------------------------------- + + +def test_get_build_env( + nrf52_dirs: SimpleNamespace, monkeypatch: pytest.MonkeyPatch +) -> None: + """get_build_env exposes ZEPHYR_SDK_INSTALL_DIR pointing at the toolchain root. + + ZEPHYR_SDK_INSTALL_DIR is the variable Zephyr's FindZephyr-sdk.cmake + explicitly consumes (from the environment) and uses as a find_package + HINT. The old Zephyr-sdk_DIR environment hint proved unreliable in + containerized non-root builds and was removed. + """ + monkeypatch.setenv("SOME_PREEXISTING_VAR", "kept") + + env = get_build_env() + + tools = get_sdk_nrf_tools_path() + venv_bin_dir = get_python_env_executable_path( + tools / "penvs" / f"v{_TEST_SDK_VERSION}", "python" + ).parent + assert env["PATH"].startswith(str(venv_bin_dir) + os.pathsep) + assert env["ZEPHYR_BASE"] == str( + tools / "frameworks" / f"v{_TEST_SDK_VERSION}" / "zephyr" + ) + # Toolchain root, not the cmake/ subdir + assert env["ZEPHYR_SDK_INSTALL_DIR"] == str( + tools / "toolchains" / TOOLCHAIN_VERSION + ) + assert "Zephyr-sdk_DIR" not in env + # The rest of the process environment is inherited + assert env["SOME_PREEXISTING_VAR"] == "kept" + + # --------------------------------------------------------------------------- # get_sdk_nrf_tools_path tests # --------------------------------------------------------------------------- From 2b4d9c0af9709adb554a4fc60313e5e833d5fdf4 Mon Sep 17 00:00:00 2001 From: "esphome[bot]" <115708604+esphome[bot]@users.noreply.github.com> Date: Fri, 17 Jul 2026 08:08:28 -0400 Subject: [PATCH 07/40] Bump bundled esphome-device-builder to 1.6.2 (#17640) Co-authored-by: esphome[bot] <115708604+esphome[bot]@users.noreply.github.com> --- docker/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index 84fd658594..7710256318 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -22,7 +22,7 @@ RUN \ -r /requirements.txt # Install the ESPHome Device Builder dashboard. -RUN uv pip install --no-cache-dir esphome-device-builder==1.6.1 +RUN uv pip install --no-cache-dir esphome-device-builder==1.6.2 RUN \ platformio settings set enable_telemetry No \ From b3172ecae8b8a917fc028680cd73a8a2e449efa4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 17 Jul 2026 12:00:54 -1000 Subject: [PATCH 08/40] Bump aioesphomeapi from 45.6.0 to 45.6.1 (#17653) Signed-off-by: dependabot[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index b36e70ef5d..3c9d71e5ce 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,7 +9,7 @@ pyserial==3.5 platformio==6.1.19 esptool==5.3.1 click==8.3.3 -aioesphomeapi==45.6.0 +aioesphomeapi==45.6.1 zeroconf==0.150.0 puremagic==2.2.0 ruamel.yaml==0.19.1 # dashboard_import From e08bdf8cca312f245f3840400f20ff4fdb3d0251 Mon Sep 17 00:00:00 2001 From: "esphome[bot]" <115708604+esphome[bot]@users.noreply.github.com> Date: Fri, 17 Jul 2026 12:25:18 -1000 Subject: [PATCH 09/40] Bump bundled esphome-device-builder to 1.6.3 (#17651) --- docker/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index 7710256318..b60bfac7a2 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -22,7 +22,7 @@ RUN \ -r /requirements.txt # Install the ESPHome Device Builder dashboard. -RUN uv pip install --no-cache-dir esphome-device-builder==1.6.2 +RUN uv pip install --no-cache-dir esphome-device-builder==1.6.3 RUN \ platformio settings set enable_telemetry No \ From 7afe7750cd26d9ddf197bd8692293a5930c33b1e Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Fri, 17 Jul 2026 19:46:45 -0400 Subject: [PATCH 10/40] [espidf] Suggest installing missing system libraries when the tools install fails (#17619) --- esphome/espidf/framework.py | 9 ++++++++ tests/unit_tests/test_espidf_framework.py | 28 +++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 18aa966bff..b8e0d4cfca 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -1,5 +1,6 @@ """ESP-IDF framework tools for ESPHome.""" +from ctypes.util import find_library import json import logging import os @@ -668,6 +669,14 @@ def _check_esphome_idf_framework_install( env=env, stream_output=True, ): + if platform.system() == "Linux" and find_library("usb-1.0") is None: + _LOGGER.error( + "libusb-1.0.so.0 was not found on this system and the ESP-IDF " + "tools need it (openocd fails its install check without it). " + "Install the libusb 1.0 package, e.g. libusb-1.0-0 " + "(Debian/Ubuntu), libusb1 (Fedora) or libusb (Alpine/Arch), " + "then run the build again." + ) raise RuntimeError(f"ESP-IDF {version} framework installation failure") _write_stamp(env_stamp_file, stamp_info) diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index de02a6b227..a1af5ae54c 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -478,6 +478,34 @@ def test_check_esp_idf_install_python_stamp_mismatch_rebuilds_venv( espidf_mocks.venv.assert_called_once() +@pytest.mark.parametrize( + ("lib", "expect_hint"), + [ + (None, True), + ("libusb-1.0.so.0", False), + ], +) +def test_check_esp_idf_install_failure_libusb_hint( + espidf_mocks: SimpleNamespace, + caplog: pytest.LogCaptureFixture, + lib: str | None, + expect_hint: bool, +) -> None: + """A failed tools install only shows the libusb hint when libusb-1.0 is + actually missing.""" + espidf_mocks.run_ok.return_value = False + # Fake Linux so the gate is exercised on all CI hosts; faking Linux is safe + # everywhere (unlike faking Windows, which pulls in winreg on other hosts) + with ( + patch("esphome.espidf.framework.find_library", return_value=lib), + patch("esphome.espidf.framework.platform.system", return_value="Linux"), + caplog.at_level(logging.ERROR, logger="esphome.espidf.framework"), + pytest.raises(RuntimeError, match="framework installation failure"), + ): + check_esp_idf_install(_IDF_VERSION, force=True) + assert ("libusb-1.0.so.0 was not found" in caplog.text) == expect_hint + + def test_check_esp_idf_install_unparseable_version( espidf_mocks: SimpleNamespace, ) -> None: From cd40fb1c684ec6bc025be11a9d48553eec7abc0e Mon Sep 17 00:00:00 2001 From: Brandon Harvey <8107750+bharvey88@users.noreply.github.com> Date: Fri, 17 Jul 2026 20:45:13 -0500 Subject: [PATCH 11/40] [core] Fix srcFilter exclusions being silently ignored on Windows (#17648) --- esphome/platformio/library.py | 6 ++++ tests/unit_tests/test_espidf_component.py | 43 +++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 291bedb5cd..0ffac65e0d 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -292,6 +292,12 @@ def collect_filtered_files(src_dir: PathType, src_filters: list[str]) -> list[st for root, _, files in os.walk(item): matched.extend([str(Path(root) / f) for f in files]) + # glob keeps the pattern's literal separators for non-wildcard path + # components, so on Windows the same file can surface with different + # separators depending on where the wildcards sit; normalize so the + # include/exclude set operations below compare equal paths. + matched = [os.path.normpath(m) for m in matched] + # FILTER_REGEX only ever captures "+" or "-", so the else is the "-" case. if sign == "+": selected.update(matched) diff --git a/tests/unit_tests/test_espidf_component.py b/tests/unit_tests/test_espidf_component.py index a50024b8e9..055e9c8502 100644 --- a/tests/unit_tests/test_espidf_component.py +++ b/tests/unit_tests/test_espidf_component.py @@ -1,3 +1,4 @@ +import glob import hashlib import json import os @@ -86,6 +87,48 @@ def test_collect_filtered_files_exclude(tmp_path): assert str(f2) not in result +def test_collect_filtered_files_exclude_pattern_in_subdir(tmp_path): + src = tmp_path / "lib" / "src" + src.mkdir(parents=True) + kept = src / "a.c" + excluded = src / "hasty.c" + kept.write_text("int a;") + excluded.write_text("int b;") + + result = collect_filtered_files(tmp_path, ["+", "-"]) + assert str(kept) in result + assert str(excluded) not in result + + +def test_collect_filtered_files_exclude_unnormalized_glob_output(tmp_path, monkeypatch): + # On Windows, glob keeps the pattern's literal separators for non-wildcard + # path components, so the "+" wildcard pattern and the "-" literal pattern + # yield the same file spelled differently and the exclude set difference + # misses it. Backslash is a regular filename character on POSIX (such paths + # fail the final is_file filter), so reproduce the unnormalized-output + # mismatch portably with dot segments, which normpath also collapses. + src = tmp_path / "lib" / "src" + src.mkdir(parents=True) + kept = src / "a.c" + excluded = src / "hasty.c" + kept.write_text("int a;") + excluded.write_text("int b;") + + real_glob = glob.glob + + def unnormalized_glob(pattern, recursive=False): + if "*" in pattern: + base = str(tmp_path) + return [base + "/lib/./src/a.c", base + "/lib/./src/hasty.c"] + return real_glob(pattern, recursive=recursive) + + monkeypatch.setattr(glob, "glob", unnormalized_glob) + + result = collect_filtered_files(tmp_path, ["+", "-"]) + assert [Path(r).name for r in result] == ["a.c"] + assert str(kept) in result + + def test_split_list_by_condition(): items = ["-Iinclude", "-Llib", "-Wall"] From 4062f0a3238323c85442d76a29ce74b0e99ca7a5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 17 Jul 2026 15:46:16 -1000 Subject: [PATCH 12/40] Pin cryptography to 48.0.1 on Intel macOS (#17658) --- requirements.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 3c9d71e5ce..9c78597360 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,7 @@ -cryptography==49.0.0 +# cryptography 49+ ships no Intel macOS wheels (arm64 only); esptool caps <49 there. +# Keep 48.0.1, the last universal2 release, so esphome stays installable on Intel Macs. +cryptography==49.0.0; platform_system != "Darwin" or platform_machine != "x86_64" +cryptography==48.0.1; platform_system == "Darwin" and platform_machine == "x86_64" voluptuous==0.16.0 PyYAML==6.0.3 paho-mqtt==1.6.1 From 572fc033cd6148146dfe079639a67d3bae581b0d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 17 Jul 2026 15:59:12 -1000 Subject: [PATCH 13/40] Ship component requirements.txt files in the sdist and wheel (#17660) --- MANIFEST.in | 1 + 1 file changed, 1 insertion(+) diff --git a/MANIFEST.in b/MANIFEST.in index e426627e8d..1626261fb6 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -6,3 +6,4 @@ recursive-include esphome *.cpp *.h *.tcc *.c recursive-include esphome *.py.script recursive-include esphome *.jinja recursive-include esphome LICENSE.txt +recursive-include esphome requirements.txt From 0dfb573e186fc47a84a20dcc6f0f4f8ca4035f4d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 17 Jul 2026 17:16:14 -1000 Subject: [PATCH 14/40] [logs] Cap the logs reconnect backoff for deep-sleep devices (#17656) --- esphome/components/api/client.py | 3 ++ .../unit_tests/components/api/test_client.py | 34 +++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/esphome/components/api/client.py b/esphome/components/api/client.py index 44edc035f9..3473deec83 100644 --- a/esphome/components/api/client.py +++ b/esphome/components/api/client.py @@ -152,6 +152,9 @@ async def async_run_logs( name=name, subscribe_states=subscribe_states, allow_plaintext_fallback=True, + # A top-level ``deep_sleep:`` block means the device is only awake + # briefly; cap the reconnect backoff so a wake window is not missed. + deep_sleep="deep_sleep" in config, ) try: await asyncio.Event().wait() diff --git a/tests/unit_tests/components/api/test_client.py b/tests/unit_tests/components/api/test_client.py index 333ef70b22..379705f534 100644 --- a/tests/unit_tests/components/api/test_client.py +++ b/tests/unit_tests/components/api/test_client.py @@ -2,11 +2,14 @@ from __future__ import annotations -from unittest.mock import patch +from unittest.mock import AsyncMock, patch + +import pytest from esphome.components import esp32 from esphome.components.api import client as api_client -from esphome.core import EsphomeError +from esphome.const import CONF_PORT, KEY_CORE, KEY_TARGET_PLATFORM +from esphome.core import CORE, EsphomeError def test_decoder_swallows_esphome_error() -> None: @@ -112,3 +115,30 @@ def test_decoder_uses_platform_handler_when_provided() -> None: assert calls == [(config, "BT0: 0x4010496e", False)] assert mock_generic.called is False assert processor.backtrace_state is True + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("extra_config", "expected_deep_sleep"), + [({"deep_sleep": {}}, True), ({}, False)], +) +async def test_async_run_logs_passes_deep_sleep( + extra_config: dict, expected_deep_sleep: bool +) -> None: + """async_run_logs tells async_run whether the device deep sleeps, from the config.""" + CORE.data[KEY_CORE] = {KEY_TARGET_PLATFORM: "esp32"} + config = {"esphome": {"name": "test"}, "api": {CONF_PORT: 6053}, **extra_config} + # async_run blocks forever after connecting; raise to unwind async_run_logs + # once we have captured how it was called. + sentinel = RuntimeError("stop the wait") + + with ( + patch.object( + api_client, "async_run", AsyncMock(side_effect=sentinel) + ) as mock_run, + patch.object(api_client, "APIClient"), + pytest.raises(RuntimeError, match="stop the wait"), + ): + await api_client.async_run_logs(config, ["1.2.3.4"]) + + assert mock_run.call_args.kwargs["deep_sleep"] is expected_deep_sleep From d4443be0c191ce6db74051e6167ce06a65c02ea2 Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Sat, 18 Jul 2026 15:22:23 +1200 Subject: [PATCH 15/40] [nrf52] Install PlatformIO toolchain Python packages into a dedicated venv (#17635) --- esphome/components/nrf52/__init__.py | 12 +- esphome/components/nrf52/framework.py | 82 ++++++++++ tests/unit_tests/test_nrf52_framework.py | 181 +++++++++++++++++++++++ tests/unit_tests/test_nrf52_upload.py | 66 +++++++++ 4 files changed, 340 insertions(+), 1 deletion(-) diff --git a/esphome/components/nrf52/__init__.py b/esphome/components/nrf52/__init__.py index 8d522a8740..5b3c250f34 100644 --- a/esphome/components/nrf52/__init__.py +++ b/esphome/components/nrf52/__init__.py @@ -69,7 +69,12 @@ from .const import ( BOOTLOADER_ADAFRUIT_NRF52_SD140_V6, BOOTLOADER_ADAFRUIT_NRF52_SD140_V7, ) -from .framework import check_and_install, get_build_env, get_build_paths +from .framework import ( + check_and_install, + get_build_env, + get_build_paths, + setup_platformio_python_env, +) # force import gpio to register pin schema from .gpio import nrf52_pin_to_code # noqa: F401 @@ -514,6 +519,7 @@ def _upload_using_platformio( ) -> int | str: from esphome.platformio import toolchain + setup_platformio_python_env() if port is not None: upload_args += ["--upload-port", port] return toolchain.run_platformio_cli_run(config, CORE.verbose, *upload_args) @@ -809,6 +815,10 @@ def _copy_if_exists(src: Path, dst: Path) -> None: def run_compile(args, config: ConfigType) -> bool: if CORE.using_toolchain_platformio: + # The actual build is done by PlatformIO (the caller falls through to + # it when this returns False); prepare the Python environment its + # Zephyr build script expects first. + setup_platformio_python_env() return False if not CORE.using_toolchain_sdk_nrf: raise EsphomeError( diff --git a/esphome/components/nrf52/framework.py b/esphome/components/nrf52/framework.py index 623cd4eef3..7392ad2d60 100644 --- a/esphome/components/nrf52/framework.py +++ b/esphome/components/nrf52/framework.py @@ -4,6 +4,7 @@ import os from pathlib import Path import platform import shutil +import sys import tempfile import platformdirs @@ -27,6 +28,11 @@ _LOGGER = logging.getLogger(__name__) _REQUIREMENTS = Path(__file__).parent / "requirements.txt" TOOLCHAIN_VERSION = "0.17.4" +# Packages the PlatformIO toolchain's Zephyr build script needs beyond west +# (which comes from requirements.txt). Keep the pin in sync with +# framework-sdk-nrf scripts/platformio/platformio-build.py. +_PLATFORMIO_PENV_REQUIREMENTS: tuple[str, ...] = ("cbor2==5.6.5",) + SDK_NG_TOOLCHAIN_MIRRORS = str_to_lst_of_str( os.environ.get( "ESPHOME_SDK_NG_TOOLCHAIN_MIRRORS", @@ -145,6 +151,82 @@ def get_build_env() -> dict: return env +def _get_platformio_penv_path() -> Path: + return get_sdk_nrf_tools_path() / "penvs" / "platformio" + + +def _get_penv_site_packages(penv_path: Path) -> Path: + if os.name == "nt": + return penv_path / "Lib" / "site-packages" + python_dir = f"python{sys.version_info.major}.{sys.version_info.minor}" + return penv_path / "lib" / python_dir / "site-packages" + + +def _prepend_env_path(name: str, entry: str) -> None: + """Prepend ``entry`` to the ``os.pathsep``-separated env var ``name``.""" + current = os.environ.get(name, "") + entries = current.split(os.pathsep) if current else [] + if entry not in entries: + os.environ[name] = os.pathsep.join([entry, *entries]) + + +def setup_platformio_python_env() -> None: + """Make the Zephyr build's Python packages available to PlatformIO. + + The PlatformIO toolchain's Zephyr framework build script pip-installs + west and cbor2 (and pyocd on x86_64) into the Python environment running + PlatformIO whenever they are not importable. That environment is not + always writable — for example the docker image run as a non-root user, + where ESPHome lives in the system Python — so the install fails with + "Permission denied". Instead, pre-install those packages into a dedicated + venv under the sdk-nrf tools dir and expose it to the PlatformIO + subprocesses through the environment: + + * PYTHONPATH makes the venv's packages importable from the interpreter + that runs PlatformIO/SCons, so the build script skips its installs. + * VIRTUAL_ENV redirects any install the build script still performs via + uv (pyocd is fetched on demand) into the writable venv. + * PATH exposes console scripts installed into the venv (e.g. pyocd). + """ + penv_path = _get_platformio_penv_path() + env_python_path = get_python_env_executable_path(penv_path, "python") + sentinel = penv_path / ".ready" + # Include the Python version: the venv breaks when the interpreter it + # was created from is upgraded, so it must be rebuilt. + requirements_hash = hashlib.sha256( + _REQUIREMENTS.read_bytes() + + "\n".join(_PLATFORMIO_PENV_REQUIREMENTS).encode() + + f"python{sys.version_info.major}.{sys.version_info.minor}".encode() + ).hexdigest() + if ( + not sentinel.exists() + or sentinel.read_text(encoding="utf-8") != requirements_hash + ): + rmdir(penv_path, msg="Clean up PlatformIO toolchain Python environment") + + create_venv(penv_path, msg="PlatformIO toolchain") + + _LOGGER.info("Installing PlatformIO toolchain requirements ...") + cmd = [ + str(env_python_path), + "-m", + "pip", + "install", + "-r", + str(_REQUIREMENTS), + *_PLATFORMIO_PENV_REQUIREMENTS, + ] + if not run_command_ok(cmd): + raise EsphomeError( + "Install requirements for PlatformIO toolchain Python environment failure" + ) + sentinel.write_text(requirements_hash, encoding="utf-8") + + os.environ["VIRTUAL_ENV"] = str(penv_path) + _prepend_env_path("PYTHONPATH", str(_get_penv_site_packages(penv_path))) + _prepend_env_path("PATH", str(env_python_path.parent)) + + def _patch_uf2conv_escape_sequences(framework_path: Path) -> None: # SDK v2.6.1 ships uf2conv.py with '\s+' — an unrecognised escape that # Python 3.12+ flags with SyntaxWarning (a future version will reject it). diff --git a/tests/unit_tests/test_nrf52_framework.py b/tests/unit_tests/test_nrf52_framework.py index 830e9efba5..8a5f4377d3 100644 --- a/tests/unit_tests/test_nrf52_framework.py +++ b/tests/unit_tests/test_nrf52_framework.py @@ -3,18 +3,23 @@ import hashlib import os from pathlib import Path +import sys from types import SimpleNamespace from unittest.mock import patch import pytest from esphome.components.nrf52.framework import ( + _PLATFORMIO_PENV_REQUIREMENTS, _REQUIREMENTS, TOOLCHAIN_VERSION, + _get_penv_site_packages, + _get_platformio_penv_path, _get_toolchain_platform_info, check_and_install, get_build_env, get_sdk_nrf_tools_path, + setup_platformio_python_env, ) from esphome.config_validation import Version from esphome.const import KEY_CORE, KEY_FRAMEWORK_VERSION @@ -255,6 +260,182 @@ class TestCheckAndInstall: assert substitutions["extension"] == "tar.xz" +# --------------------------------------------------------------------------- +# setup_platformio_python_env tests +# --------------------------------------------------------------------------- + + +def _platformio_requirements_hash() -> str: + return hashlib.sha256( + _REQUIREMENTS.read_bytes() + + "\n".join(_PLATFORMIO_PENV_REQUIREMENTS).encode() + + f"python{sys.version_info.major}.{sys.version_info.minor}".encode() + ).hexdigest() + + +@pytest.fixture +def platformio_penv_dir() -> Path: + """Pre-create the PlatformIO penv dir so sentinel writes succeed. + + create_venv is mocked in these tests, so the directory it would have + created must exist for ``sentinel.write_text`` to work. + """ + penv_path = _get_platformio_penv_path() + penv_path.mkdir(parents=True, exist_ok=True) + return penv_path + + +class TestSetupPlatformioPythonEnv: + def test_fresh_install_creates_venv_and_sets_env( + self, + platformio_penv_dir: Path, + mock_nrf52_ops: SimpleNamespace, + ) -> None: + """No sentinel → venv created, requirements installed, env exported.""" + with patch.dict(os.environ): + os.environ.pop("PYTHONPATH", None) + + setup_platformio_python_env() + + mock_nrf52_ops.rmdir.assert_called_once() + mock_nrf52_ops.create_venv.assert_called_once_with( + platformio_penv_dir, msg="PlatformIO toolchain" + ) + mock_nrf52_ops.run_command_ok.assert_called_once() + cmd = mock_nrf52_ops.run_command_ok.call_args[0][0] + assert cmd[1:4] == ["-m", "pip", "install"] + assert "-r" in cmd + assert str(_REQUIREMENTS) in cmd + for requirement in _PLATFORMIO_PENV_REQUIREMENTS: + assert requirement in cmd + sentinel = platformio_penv_dir / ".ready" + assert sentinel.read_text(encoding="utf-8") == ( + _platformio_requirements_hash() + ) + + assert os.environ["VIRTUAL_ENV"] == str(platformio_penv_dir) + site_packages = str(_get_penv_site_packages(platformio_penv_dir)) + assert os.environ["PYTHONPATH"] == site_packages + bin_dir = str( + get_python_env_executable_path(platformio_penv_dir, "python").parent + ) + assert os.environ["PATH"].split(os.pathsep)[0] == bin_dir + + def test_ready_sentinel_skips_install_but_sets_env( + self, + platformio_penv_dir: Path, + mock_nrf52_ops: SimpleNamespace, + ) -> None: + """Current sentinel → no install work, env vars still exported.""" + (platformio_penv_dir / ".ready").write_text( + _platformio_requirements_hash(), encoding="utf-8" + ) + + with patch.dict(os.environ): + setup_platformio_python_env() + + mock_nrf52_ops.rmdir.assert_not_called() + mock_nrf52_ops.create_venv.assert_not_called() + mock_nrf52_ops.run_command_ok.assert_not_called() + assert os.environ["VIRTUAL_ENV"] == str(platformio_penv_dir) + + def test_stale_sentinel_reinstalls( + self, + platformio_penv_dir: Path, + mock_nrf52_ops: SimpleNamespace, + ) -> None: + """A sentinel from different requirements → venv rebuilt from scratch.""" + sentinel = platformio_penv_dir / ".ready" + sentinel.write_text("stale-hash", encoding="utf-8") + + with patch.dict(os.environ): + setup_platformio_python_env() + + mock_nrf52_ops.rmdir.assert_called_once() + mock_nrf52_ops.create_venv.assert_called_once() + mock_nrf52_ops.run_command_ok.assert_called_once() + assert sentinel.read_text(encoding="utf-8") == _platformio_requirements_hash() + + def test_install_failure_raises( + self, + platformio_penv_dir: Path, + mock_nrf52_ops: SimpleNamespace, + ) -> None: + """Failing pip install raises EsphomeError and writes no sentinel.""" + mock_nrf52_ops.run_command_ok.return_value = False + + with ( + patch.dict(os.environ), + pytest.raises( + EsphomeError, match="Install requirements for PlatformIO toolchain" + ), + ): + setup_platformio_python_env() + + assert not (platformio_penv_dir / ".ready").exists() + + def test_repeated_calls_do_not_duplicate_env_entries( + self, + platformio_penv_dir: Path, + mock_nrf52_ops: SimpleNamespace, + ) -> None: + """Compile then upload in one process must not grow PYTHONPATH/PATH.""" + (platformio_penv_dir / ".ready").write_text( + _platformio_requirements_hash(), encoding="utf-8" + ) + site_packages = str(_get_penv_site_packages(platformio_penv_dir)) + bin_dir = str( + get_python_env_executable_path(platformio_penv_dir, "python").parent + ) + + with patch.dict(os.environ): + setup_platformio_python_env() + setup_platformio_python_env() + + assert os.environ["PYTHONPATH"].split(os.pathsep).count(site_packages) == 1 + assert os.environ["PATH"].split(os.pathsep).count(bin_dir) == 1 + + def test_existing_pythonpath_preserved( + self, + platformio_penv_dir: Path, + mock_nrf52_ops: SimpleNamespace, + ) -> None: + """A pre-existing PYTHONPATH keeps its entries after the venv entry.""" + (platformio_penv_dir / ".ready").write_text( + _platformio_requirements_hash(), encoding="utf-8" + ) + site_packages = str(_get_penv_site_packages(platformio_penv_dir)) + + with patch.dict(os.environ, {"PYTHONPATH": "/existing/path"}): + setup_platformio_python_env() + + assert os.environ["PYTHONPATH"] == os.pathsep.join( + [site_packages, "/existing/path"] + ) + + +@pytest.mark.parametrize( + ("os_name", "expected_parts"), + [ + ( + "posix", + ( + "lib", + f"python{sys.version_info.major}.{sys.version_info.minor}", + "site-packages", + ), + ), + ("nt", ("Lib", "site-packages")), + ], +) +def test_get_penv_site_packages( + tmp_path: Path, os_name: str, expected_parts: tuple[str, ...] +) -> None: + penv_path = tmp_path / "penv" + with patch("os.name", os_name): + assert _get_penv_site_packages(penv_path) == penv_path.joinpath(*expected_parts) + + # --------------------------------------------------------------------------- # get_build_env tests # --------------------------------------------------------------------------- diff --git a/tests/unit_tests/test_nrf52_upload.py b/tests/unit_tests/test_nrf52_upload.py index a60e23a337..9b738ebc81 100644 --- a/tests/unit_tests/test_nrf52_upload.py +++ b/tests/unit_tests/test_nrf52_upload.py @@ -146,6 +146,72 @@ class TestUploadProgramPyocd: upload_program(config={}, args=None, host="PYOCD") +# --------------------------------------------------------------------------- +# PlatformIO toolchain paths +# --------------------------------------------------------------------------- + + +class TestRunCompilePlatformio: + def test_prepares_python_env_and_delegates_to_platformio( + self, setup_core: Path, tmp_path: Path + ) -> None: + """The PlatformIO toolchain prepares the env, then returns False so PlatformIO builds.""" + from esphome.components.nrf52 import run_compile + + _setup_nrf52_core(toolchain=Toolchain.PLATFORMIO, build_path=tmp_path / "build") + + with patch( + "esphome.components.nrf52.setup_platformio_python_env" + ) as mock_setup: + assert run_compile(args=None, config={}) is False + + mock_setup.assert_called_once_with() + + +class TestUploadProgramSerialPlatformio: + def _upload(self, host: str, tmp_path: Path, run_result: int) -> tuple: + from esphome.components.nrf52 import upload_program + from esphome.upload_targets import PortType + + _setup_nrf52_core(toolchain=Toolchain.PLATFORMIO, build_path=tmp_path / "build") + CORE.config_path = tmp_path / "test.yaml" + + with ( + patch("esphome.upload_targets.get_port_type", return_value=PortType.SERIAL), + patch("esphome.__main__.check_permissions"), + patch("esphome.components.nrf52.setup_platformio_python_env") as mock_setup, + patch( + "esphome.platformio.toolchain.run_platformio_cli_run", + return_value=run_result, + ) as mock_run, + ): + result = upload_program(config={}, args=None, host=host) + return result, mock_setup, mock_run + + def test_serial_upload_prepares_env_and_runs_platformio( + self, setup_core: Path, tmp_path: Path + ) -> None: + """Serial upload with the PlatformIO toolchain runs pio with -t upload.""" + host = "/dev/ttyACM0" + result, mock_setup, mock_run = self._upload(host, tmp_path, run_result=0) + + assert result is True + mock_setup.assert_called_once_with() + mock_run.assert_called_once() + run_args = mock_run.call_args[0] + assert "-t" in run_args + assert "upload" in run_args + assert "--upload-port" in run_args + assert host in run_args + + def test_serial_upload_failure_raises( + self, setup_core: Path, tmp_path: Path + ) -> None: + """A non-zero PlatformIO result must raise EsphomeError.""" + with pytest.raises(EsphomeError, match="Upload failed"): + self._upload("/dev/ttyACM0", tmp_path, run_result=1) + + # --------------------------------------------------------------------------- # Serial DFU upload path # --------------------------------------------------------------------------- From a0fb14bf548c3705a5408c7c46a6fc76c0c3faa5 Mon Sep 17 00:00:00 2001 From: "esphome[bot]" <115708604+esphome[bot]@users.noreply.github.com> Date: Fri, 17 Jul 2026 17:44:52 -1000 Subject: [PATCH 16/40] Bump bundled esphome-device-builder to 1.6.4 (#17662) --- docker/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index b60bfac7a2..f804ebd148 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -22,7 +22,7 @@ RUN \ -r /requirements.txt # Install the ESPHome Device Builder dashboard. -RUN uv pip install --no-cache-dir esphome-device-builder==1.6.3 +RUN uv pip install --no-cache-dir esphome-device-builder==1.6.4 RUN \ platformio settings set enable_telemetry No \ From f37dad683b87c64578b79c883cadfa7a86ebc76e Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Sat, 18 Jul 2026 17:14:42 -0400 Subject: [PATCH 17/40] [esp32] Bump recommended ESP-IDF to 5.5.5 and Arduino to 3.3.10 (#17669) --- esphome/components/esp32/__init__.py | 15 +++++++++------ platformio.ini | 6 +++--- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/esphome/components/esp32/__init__.py b/esphome/components/esp32/__init__.py index 9b568dd629..7911b172d3 100644 --- a/esphome/components/esp32/__init__.py +++ b/esphome/components/esp32/__init__.py @@ -814,14 +814,15 @@ def _is_framework_url(source: str) -> bool: # The default/recommended arduino framework version # - https://github.com/espressif/arduino-esp32/releases ARDUINO_FRAMEWORK_VERSION_LOOKUP = { - "recommended": cv.Version(3, 3, 9), - "latest": cv.Version(3, 3, 9), - "dev": cv.Version(3, 3, 9), + "recommended": cv.Version(3, 3, 10), + "latest": cv.Version(3, 3, 10), + "dev": cv.Version(3, 3, 10), } ARDUINO_PLATFORM_VERSION_LOOKUP = { cv.Version( 4, 0, 0, "alpha1" ): "https://github.com/pioarduino/platform-espressif32.git#prep_IDF6", + cv.Version(3, 3, 10): cv.Version(55, 3, 39), cv.Version(3, 3, 9): cv.Version(55, 3, 39), cv.Version(3, 3, 8): cv.Version(55, 3, 38, "1"), cv.Version(3, 3, 7): cv.Version(55, 3, 37), @@ -844,6 +845,7 @@ ARDUINO_PLATFORM_VERSION_LOOKUP = { # See: https://github.com/pioarduino/esp-idf/releases ARDUINO_IDF_VERSION_LOOKUP = { cv.Version(4, 0, 0, "alpha1"): cv.Version(6, 0, 1), + cv.Version(3, 3, 10): cv.Version(5, 5, 5), cv.Version(3, 3, 9): cv.Version(5, 5, 4), cv.Version(3, 3, 8): cv.Version(5, 5, 4), cv.Version(3, 3, 7): cv.Version(5, 5, 3, "1"), @@ -865,9 +867,9 @@ ARDUINO_IDF_VERSION_LOOKUP = { # The default/recommended esp-idf framework version # - https://github.com/espressif/esp-idf/releases ESP_IDF_FRAMEWORK_VERSION_LOOKUP = { - "recommended": cv.Version(5, 5, 4), - "latest": cv.Version(5, 5, 4), - "dev": cv.Version(5, 5, 4), + "recommended": cv.Version(5, 5, 5), + "latest": cv.Version(5, 5, 5), + "dev": cv.Version(5, 5, 5), } ESP_IDF_PLATFORM_VERSION_LOOKUP = { @@ -877,6 +879,7 @@ ESP_IDF_PLATFORM_VERSION_LOOKUP = { cv.Version( 6, 0, 0 ): "https://github.com/pioarduino/platform-espressif32.git#prep_IDF6", + cv.Version(5, 5, 5): cv.Version(55, 3, 39), cv.Version(5, 5, 4): cv.Version(55, 3, 39), cv.Version(5, 5, 3, "1"): cv.Version(55, 3, 37), cv.Version(5, 5, 3): cv.Version(55, 3, 37), diff --git a/platformio.ini b/platformio.ini index 061e92a64a..2ab90e63ad 100644 --- a/platformio.ini +++ b/platformio.ini @@ -143,8 +143,8 @@ extra_scripts = post:esphome/components/esp8266/post_build.py.script extends = common:arduino platform = https://github.com/pioarduino/platform-espressif32/releases/download/55.03.39/platform-espressif32.zip platform_packages = - pioarduino/framework-arduinoespressif32@https://github.com/espressif/arduino-esp32/releases/download/3.3.9/esp32-core-3.3.9.tar.xz - pioarduino/framework-espidf@https://github.com/pioarduino/esp-idf/releases/download/v5.5.4/esp-idf-v5.5.4.tar.xz + pioarduino/framework-arduinoespressif32@https://github.com/espressif/arduino-esp32/releases/download/3.3.10/esp32-core-3.3.10.tar.xz + pioarduino/framework-espidf@https://github.com/pioarduino/esp-idf/releases/download/v5.5.5/esp-idf-v5.5.5.tar.xz framework = arduino, espidf ; Arduino as an ESP-IDF component lib_deps = @@ -180,7 +180,7 @@ extra_scripts = extends = common:idf platform = https://github.com/pioarduino/platform-espressif32/releases/download/55.03.39/platform-espressif32.zip platform_packages = - pioarduino/framework-espidf@https://github.com/pioarduino/esp-idf/releases/download/v5.5.4/esp-idf-v5.5.4.tar.xz + pioarduino/framework-espidf@https://github.com/pioarduino/esp-idf/releases/download/v5.5.5/esp-idf-v5.5.5.tar.xz framework = espidf lib_deps = From f05e15522cd00989aa68bca1f314b7881d140e93 Mon Sep 17 00:00:00 2001 From: "esphome[bot]" <115708604+esphome[bot]@users.noreply.github.com> Date: Sat, 18 Jul 2026 11:46:14 -1000 Subject: [PATCH 18/40] Bump bundled esphome-device-builder to 1.6.5 (#17675) --- docker/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index f804ebd148..ecf1f0479a 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -22,7 +22,7 @@ RUN \ -r /requirements.txt # Install the ESPHome Device Builder dashboard. -RUN uv pip install --no-cache-dir esphome-device-builder==1.6.4 +RUN uv pip install --no-cache-dir esphome-device-builder==1.6.5 RUN \ platformio settings set enable_telemetry No \ From 2f99466f3ada990b3e07ade8f93e1e8a1a36dd11 Mon Sep 17 00:00:00 2001 From: "esphome[bot]" <115708604+esphome[bot]@users.noreply.github.com> Date: Sat, 18 Jul 2026 21:25:56 -1000 Subject: [PATCH 19/40] Bump bundled esphome-device-builder to 1.6.6 (#17681) --- docker/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index ecf1f0479a..5ab0e71008 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -22,7 +22,7 @@ RUN \ -r /requirements.txt # Install the ESPHome Device Builder dashboard. -RUN uv pip install --no-cache-dir esphome-device-builder==1.6.5 +RUN uv pip install --no-cache-dir esphome-device-builder==1.6.6 RUN \ platformio settings set enable_telemetry No \ From 3d340f4d907f8a956dd10b358735a5b932abe06d Mon Sep 17 00:00:00 2001 From: "esphome[bot]" <115708604+esphome[bot]@users.noreply.github.com> Date: Sun, 19 Jul 2026 09:17:53 -1000 Subject: [PATCH 20/40] Bump bundled esphome-device-builder to 1.6.7 (#17696) --- docker/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index 5ab0e71008..331585f123 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -22,7 +22,7 @@ RUN \ -r /requirements.txt # Install the ESPHome Device Builder dashboard. -RUN uv pip install --no-cache-dir esphome-device-builder==1.6.6 +RUN uv pip install --no-cache-dir esphome-device-builder==1.6.7 RUN \ platformio settings set enable_telemetry No \ From 2223b147947c0b398155c9a979ffee99128983a9 Mon Sep 17 00:00:00 2001 From: Brandon Harvey <8107750+bharvey88@users.noreply.github.com> Date: Sun, 19 Jul 2026 17:20:43 -0500 Subject: [PATCH 21/40] Split multi-token build.flags entries when generating ESP-IDF component CMakeLists (#17649) --- esphome/espidf/component.py | 21 ++++++++++++ tests/unit_tests/test_espidf_component.py | 41 +++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/esphome/espidf/component.py b/esphome/espidf/component.py index e9ec170a5e..51d023099e 100644 --- a/esphome/espidf/component.py +++ b/esphome/espidf/component.py @@ -83,6 +83,10 @@ def generate_cmakelists_txt(component: IDFComponent) -> str: Returns: str: The complete CMakeLists.txt content as a string """ + # Late import: this module loads with the esp32 platform on every + # validate/compile, but shlex is only needed when generating component + # CMakeLists. + import shlex def escape_entry(p: PathType) -> str: # In CMakeLists.txt, backslashes need to be escaped @@ -105,6 +109,23 @@ def generate_cmakelists_txt(component: IDFComponent) -> str: build_flags = ensure_list( component.data.get("build", {}).get("flags", DEFAULT_BUILD_FLAGS) ) + # PlatformIO shell-lexes each build.flags entry, so one entry can carry a + # flag and its argument (e.g. "-include cp_custom_alloc.h"). Split the + # same way; emitting such an entry as a single quoted compile option + # hands the compiler one argv with an embedded space. + build_flags = [token for entry in build_flags for token in shlex.split(entry)] + # Re-glue bare -I/-L/-l tokens to their argument ("-I foo" -> "-Ifoo") so + # the prefix classifiers below still route them to INCLUDE_DIRS and the + # link handling. + tokens, build_flags = build_flags, [] + i = 0 + while i < len(tokens): + if tokens[i] in ("-I", "-L", "-l") and i + 1 < len(tokens): + build_flags.append(tokens[i] + tokens[i + 1]) + i += 2 + else: + build_flags.append(tokens[i]) + i += 1 # List all sources files build_src_files = collect_filtered_files( diff --git a/tests/unit_tests/test_espidf_component.py b/tests/unit_tests/test_espidf_component.py index 055e9c8502..89d5ce3cf2 100644 --- a/tests/unit_tests/test_espidf_component.py +++ b/tests/unit_tests/test_espidf_component.py @@ -193,6 +193,47 @@ target_link_libraries(${{COMPONENT_LIB}} INTERFACE ) +def test_generate_cmakelists_txt_multi_token_flag(tmp_component): + # PlatformIO shell-lexes each build.flags entry, so a single entry can + # carry a flag and its argument. The generated CMakeLists must emit them + # as separate compile options, not one argument with an embedded space. + src_dir = tmp_component.path / "src" + src_dir.mkdir() + (src_dir / "main.c").write_text("int main() {}") + + tmp_component.data = {"build": {"flags": ["-include cp_custom_alloc.h", "-DTEST"]}} + + content = generate_cmakelists_txt(tmp_component) + assert '"-include cp_custom_alloc.h"' not in content + assert ' "-include"\n "cp_custom_alloc.h"\n' in content + + +def test_generate_cmakelists_txt_space_separated_classified_flags(tmp_component): + # Space-separated -I/-L/-l entries routed to INCLUDE_DIRS and the link + # handling before the shlex split was added; splitting must not leak + # them into raw compile options. + src_dir = tmp_component.path / "src" + src_dir.mkdir() + (src_dir / "main.c").write_text("int main() {}") + (tmp_component.path / "extra_inc").mkdir() + + tmp_component.data = { + "build": {"flags": ["-I extra_inc", "-L extra_lib", "-l extralib", "-DTEST"]} + } + + content = generate_cmakelists_txt(tmp_component) + assert 'INCLUDE_DIRS "src" "extra_inc"' in content + assert 'target_link_directories(${COMPONENT_LIB} INTERFACE\n "extra_lib"\n)' in ( + content + ) + assert 'target_link_libraries(${COMPONENT_LIB} INTERFACE\n "extralib"\n)' in ( + content + ) + assert '"-I"' not in content + assert '"-L"' not in content + assert '"-l"' not in content + + def test_generate_cmakelists_txt_references_project_managed_components_variable( tmp_component: IDFComponent, ) -> None: From 231a2897c060f0edce711cd5c5543862e00e046b Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Mon, 20 Jul 2026 15:51:46 +1200 Subject: [PATCH 22/40] [ssd1306] Fix offset_x being ignored on SH1106/SH1107 displays (#17700) --- .../components/ssd1306_i2c/ssd1306_i2c.cpp | 20 +++++++++---------- .../components/ssd1306_spi/ssd1306_spi.cpp | 15 ++++++++------ 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/esphome/components/ssd1306_i2c/ssd1306_i2c.cpp b/esphome/components/ssd1306_i2c/ssd1306_i2c.cpp index 8ff908fe7a..00c864a217 100644 --- a/esphome/components/ssd1306_i2c/ssd1306_i2c.cpp +++ b/esphome/components/ssd1306_i2c/ssd1306_i2c.cpp @@ -41,17 +41,17 @@ void I2CSSD1306::command(uint8_t value) { this->write_byte(0x00, value); } void HOT I2CSSD1306::write_display_data() { if (this->is_sh1106_() || this->is_sh1107_()) { uint32_t i = 0; + // Some panels wire their visible columns to a window of the controller RAM + // that does not start at column 0 (e.g. SH1107 M5Stack Unit OLED needs offset_x: 32). + // SH1106 keeps its historical 0x02 base column on top of any offset. + uint8_t start_column = this->offset_x_; + if (this->is_sh1106_()) { + start_column += 0x02; + } for (uint8_t page = 0; page < (uint8_t) this->get_height_internal() / 8; page++) { - this->command(0xB0 + page); // row - if (this->is_sh1106_()) { - this->command(0x02); // lower column - 0x02 is historical SH1106 value - } else { - // Other SH1107 drivers use 0x00 - // Column values dont change and it seems they can be set only once, - // but we follow SH1106 implementation and resend them - this->command(0x00); - } - this->command(0x10); // higher column + this->command(0xB0 + page); // row + this->command(start_column & 0x0F); // lower column + this->command(0x10 | (start_column >> 4)); // higher column for (uint8_t x = 0; x < (uint8_t) this->get_width_internal() / 16; x++) { uint8_t data[16]; for (uint8_t &j : data) diff --git a/esphome/components/ssd1306_spi/ssd1306_spi.cpp b/esphome/components/ssd1306_spi/ssd1306_spi.cpp index 5c9369f1a2..0534deeb03 100644 --- a/esphome/components/ssd1306_spi/ssd1306_spi.cpp +++ b/esphome/components/ssd1306_spi/ssd1306_spi.cpp @@ -38,14 +38,17 @@ void SPISSD1306::command(uint8_t value) { } void HOT SPISSD1306::write_display_data() { if (this->is_sh1106_() || this->is_sh1107_()) { + // Some panels wire their visible columns to a window of the controller RAM + // that does not start at column 0 (e.g. SH1107 M5Stack Unit OLED needs offset_x: 32). + // SH1106 keeps its historical 0x02 base column on top of any offset. + uint8_t start_column = this->offset_x_; + if (this->is_sh1106_()) { + start_column += 0x02; + } for (uint8_t y = 0; y < (uint8_t) this->get_height_internal() / 8; y++) { this->command(0xB0 + y); - if (this->is_sh1106_()) { - this->command(0x02); - } else { - this->command(0x00); - } - this->command(0x10); + this->command(start_column & 0x0F); // lower column + this->command(0x10 | (start_column >> 4)); // higher column this->dc_pin_->digital_write(true); for (uint8_t x = 0; x < (uint8_t) this->get_width_internal(); x++) { this->enable(); From b571d2a5abfd17101d4b0bfc94fb234797061e23 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 19 Jul 2026 18:16:15 -1000 Subject: [PATCH 23/40] [micro_wake_word] Download models in parallel (#17701) --- .../components/micro_wake_word/__init__.py | 97 +++++++--- esphome/external_files.py | 12 +- .../components/micro_wake_word/__init__.py | 0 .../components/micro_wake_word/test_init.py | 169 ++++++++++++++++++ 4 files changed, 247 insertions(+), 31 deletions(-) create mode 100644 tests/unit_tests/components/micro_wake_word/__init__.py create mode 100644 tests/unit_tests/components/micro_wake_word/test_init.py diff --git a/esphome/components/micro_wake_word/__init__.py b/esphome/components/micro_wake_word/__init__.py index 4b309551ba..c427f28028 100644 --- a/esphome/components/micro_wake_word/__init__.py +++ b/esphome/components/micro_wake_word/__init__.py @@ -2,6 +2,7 @@ import hashlib import json import logging from pathlib import Path +import re from urllib.parse import urljoin from esphome import automation, external_files, git @@ -9,6 +10,7 @@ from esphome.automation import register_action, register_condition from esphome.bundle import add_bundle_file import esphome.codegen as cg from esphome.components import esp32, microphone, ota, psram +from esphome.components.http_request import validate_url import esphome.config_validation as cv from esphome.const import ( CONF_FILE, @@ -209,33 +211,13 @@ def _validate_manifest_version(manifest_data): raise cv.Invalid("Invalid manifest file, missing 'version' key") -def _process_http_source(config): - url = config[CONF_URL] - path = _compute_local_file_path(config) - - json_path = path / "manifest.json" - - json_contents = external_files.download_content(url, json_path) - - manifest_data = json.loads(json_contents) - if not isinstance(manifest_data, dict): - raise cv.Invalid("Manifest file must contain a JSON object") - - model = manifest_data[CONF_MODEL] - model_url = urljoin(url, model) - - model_path = path / model - - external_files.download_content(str(model_url), model_path) - - return config - - -HTTP_SCHEMA = cv.All( +HTTP_SCHEMA = cv.Schema( { - cv.Required(CONF_URL): cv.url, - }, - _process_http_source, + # validate_url only accepts http(s); the shorthand validator relies + # on this branch rejecting git shorthands ("github://...") so they + # fall through to the git branch. + cv.Required(CONF_URL): validate_url, + } ) @@ -280,6 +262,13 @@ LOCAL_SCHEMA = cv.All( ) +# Bare model names in the official model repository ("okay_nabu"). Must not +# overlap with local paths, http(s) urls, or git shorthands +# ("github://user/repo/file.json@ref"), which the shorthand validator tries +# next; anything containing "/", ":" or "@" is not a model name. +_MODEL_NAME_RE = re.compile(r"[A-Za-z0-9_.-]+") + + def _validate_source_model_name(value): if not isinstance(value, str): raise cv.Invalid("Model name must be a string") @@ -287,6 +276,9 @@ def _validate_source_model_name(value): if value.endswith(".json"): raise cv.Invalid("Model name must not end with .json") + if not _MODEL_NAME_RE.fullmatch(value): + raise cv.Invalid("Model name may only contain letters, numbers, . _ -") + return MODEL_SOURCE_SCHEMA( { CONF_TYPE: TYPE_HTTP, @@ -376,6 +368,58 @@ def _maybe_empty_vad_schema(value): return VAD_MODEL_SCHEMA(value) +def _download_http_models(config: ConfigType) -> ConfigType: + """Download every http-sourced manifest and model file in two concurrent + batches (all manifests, then all model files). + + The model file's URL only becomes known once its manifest has been + fetched and parsed, so the two stages cannot be merged into one batch. + """ + model_parameters = [*config[CONF_MODELS]] + if vad := config.get(CONF_VAD): + model_parameters.append(vad) + # Keyed by cache path so a URL referenced twice is fetched and parsed once + http_models: dict[Path, str] = { + _compute_local_file_path(model_config): model_config[CONF_URL] + for parameters in model_parameters + if (model_config := parameters.get(CONF_MODEL)) is not None + and model_config.get(CONF_TYPE) == TYPE_HTTP + } + if not http_models: + return config + + external_files.download_content_many( + ((url, path / "manifest.json") for path, url in http_models.items()), + description="wake word manifest(s)", + ) + + model_files: list[tuple[str, Path]] = [] + errors: list[cv.Invalid] = [] + for path, url in http_models.items(): + try: + manifest_data = json.loads((path / "manifest.json").read_bytes()) + except (OSError, ValueError) as e: + errors.append(cv.Invalid(f"Invalid manifest file at {url}: {e}")) + continue + if not isinstance(manifest_data, dict): + errors.append( + cv.Invalid(f"Manifest file at {url} must contain a JSON object") + ) + continue + model = manifest_data.get(CONF_MODEL) + if not isinstance(model, str): + errors.append( + cv.Invalid(f"Manifest file at {url} is missing the 'model' key") + ) + continue + model_files.append((urljoin(url, model), path / model)) + if errors: + raise cv.MultipleInvalid(errors) + + external_files.download_content_many(model_files, description="wake word model(s)") + return config + + CONFIG_SCHEMA = cv.All( cv.Schema( { @@ -409,6 +453,7 @@ CONFIG_SCHEMA = cv.All( } ).extend(cv.COMPONENT_SCHEMA), cv.only_on_esp32, + _download_http_models, ) diff --git a/esphome/external_files.py b/esphome/external_files.py index 4e73c8dc21..69423d3999 100644 --- a/esphome/external_files.py +++ b/esphome/external_files.py @@ -165,11 +165,8 @@ def download_content(url: str, path: Path, timeout: int = NETWORK_TIMEOUT) -> by _LOGGER.debug("Remote file has not changed %s", url) return path.read_bytes() - _LOGGER.debug( - "Remote file has changed, downloading from %s to %s", - url, - path, - ) + _LOGGER.info("Downloading %s", url) + _LOGGER.debug("Saving to %s", path) try: req = requests.get( @@ -210,9 +207,13 @@ def download_content_many( items: Iterable[tuple[str, Path]], timeout: int = NETWORK_TIMEOUT, max_workers: int = DEFAULT_DOWNLOAD_WORKERS, + description: str = "remote file(s)", ) -> None: """Run `download_content` for each (url, path) pair concurrently. + `description` names the kind of files in the progress log line, e.g. + "wake word manifest(s)". + Wall time drops from `sum(latency)` to roughly `max(latency)` for cached files where the HEAD round-trip dominates. All workers run to completion before this returns; every `cv.Invalid` raised by a worker @@ -230,6 +231,7 @@ def download_content_many( seen: dict[Path, str] = {path: url for url, path in items} if not seen: return + _LOGGER.info("Checking %d %s for updates", len(seen), description) if len(seen) == 1: path, url = next(iter(seen.items())) download_content(url, path, timeout) diff --git a/tests/unit_tests/components/micro_wake_word/__init__.py b/tests/unit_tests/components/micro_wake_word/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit_tests/components/micro_wake_word/test_init.py b/tests/unit_tests/components/micro_wake_word/test_init.py new file mode 100644 index 0000000000..84371ab906 --- /dev/null +++ b/tests/unit_tests/components/micro_wake_word/test_init.py @@ -0,0 +1,169 @@ +"""Tests for the micro_wake_word model source validation and downloads.""" + +import json +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +from esphome.components import micro_wake_word as mww +import esphome.config_validation as cv +from esphome.const import ( + CONF_FILE, + CONF_MODEL, + CONF_PATH, + CONF_REF, + CONF_TYPE, + CONF_URL, +) + + +@pytest.fixture +def mock_download_content_many() -> MagicMock: + """Patch the concurrent download helper so no network is involved.""" + with patch( + "esphome.components.micro_wake_word.external_files.download_content_many" + ) as m: + yield m + + +def test_shorthand_model_name_resolves_without_network( + mock_download_content_many: MagicMock, +) -> None: + config = mww._validate_source_shorthand("okay_nabu") + assert config[CONF_TYPE] == mww.TYPE_HTTP + assert config[CONF_URL] == ( + "https://github.com/esphome/micro-wake-word-models/raw/main/models/v2/okay_nabu.json" + ) + mock_download_content_many.assert_not_called() + + +def test_shorthand_git_with_ref_not_captured_as_model_name( + setup_core: Path, tmp_path: Path +) -> None: + repo_dir = tmp_path / "repo" + repo_dir.mkdir() + (repo_dir / "model.json").write_text("{}") + with patch( + "esphome.components.micro_wake_word.git.clone_or_update", + return_value=(repo_dir, None), + ): + config = mww._validate_source_shorthand("github://user/repo/model.json@main") + assert config[CONF_TYPE] == "git" + assert config[CONF_URL] == "https://github.com/user/repo.git" + assert config[CONF_FILE] == "model.json" + assert config[CONF_REF] == "main" + + +def test_shorthand_local_path_not_captured_as_model_name( + setup_core: Path, tmp_path: Path +) -> None: + manifest = tmp_path / "model.json" + manifest.write_text("{}") + config = mww.MODEL_SOURCE_SCHEMA(str(manifest)) + assert config[CONF_TYPE] == "local" + assert Path(config[CONF_PATH]) == manifest + + +@pytest.mark.parametrize( + "value", ["some/path/file", "name@ref", "bad:name", "okay_nabu\n", "héllo"] +) +def test_model_name_rejects_non_identifiers(value: str) -> None: + with pytest.raises(cv.Invalid): + mww._validate_source_model_name(value) + + +def _http_model(name: str) -> dict: + return { + CONF_MODEL: { + CONF_TYPE: mww.TYPE_HTTP, + CONF_URL: f"https://example.com/models/{name}.json", + } + } + + +def _write_manifest(model_config: dict, contents: str) -> Path: + path = mww._compute_local_file_path(model_config[CONF_MODEL]) + path.mkdir(parents=True, exist_ok=True) + manifest = path / "manifest.json" + manifest.write_text(contents) + return path + + +def test_download_http_models_batches_manifests_then_models( + setup_core: Path, mock_download_content_many: MagicMock +) -> None: + names = ("okay_nabu", "hey_mycroft", "vad") + models = {name: _http_model(name) for name in names} + paths = { + name: _write_manifest(models[name], json.dumps({"model": f"{name}.tflite"})) + for name in names + } + config = { + mww.CONF_MODELS: [ + models["okay_nabu"], + models["hey_mycroft"], + # non-http sources must be ignored + {CONF_MODEL: {CONF_TYPE: "local", CONF_PATH: "x"}}, + ], + mww.CONF_VAD: models["vad"], + } + + assert mww._download_http_models(config) is config + + assert mock_download_content_many.call_count == 2 + manifest_items = list(mock_download_content_many.call_args_list[0].args[0]) + assert manifest_items == [ + (f"https://example.com/models/{name}.json", paths[name] / "manifest.json") + for name in names + ] + model_items = list(mock_download_content_many.call_args_list[1].args[0]) + assert model_items == [ + (f"https://example.com/models/{name}.tflite", paths[name] / f"{name}.tflite") + for name in names + ] + + +def test_download_http_models_no_http_sources_skips_download( + mock_download_content_many: MagicMock, +) -> None: + config = {mww.CONF_MODELS: [{CONF_MODEL: {CONF_TYPE: "local", CONF_PATH: "x"}}]} + assert mww._download_http_models(config) is config + mock_download_content_many.assert_not_called() + + +@pytest.mark.parametrize( + ("contents", "message"), + [ + ("not json", "Invalid manifest file"), + ("[1, 2]", "must contain a JSON object"), + ("{}", "missing the 'model' key"), + ], +) +def test_download_http_models_bad_manifest_raises( + setup_core: Path, + mock_download_content_many: MagicMock, + contents: str, + message: str, +) -> None: + model = _http_model("okay_nabu") + config = {mww.CONF_MODELS: [model]} + _write_manifest(model, contents) + + with pytest.raises(cv.Invalid, match=message): + mww._download_http_models(config) + # manifests were still fetched in one batch; the model batch never ran + assert mock_download_content_many.call_count == 1 + + +def test_download_http_models_collects_all_manifest_errors( + setup_core: Path, mock_download_content_many: MagicMock +) -> None: + models = {name: _http_model(name) for name in ("one", "two")} + config = {mww.CONF_MODELS: list(models.values())} + _write_manifest(models["one"], "not json") + _write_manifest(models["two"], "[1]") + + with pytest.raises(cv.MultipleInvalid) as excinfo: + mww._download_http_models(config) + assert len(excinfo.value.errors) == 2 From 5df922e0df505a67a4615c5ecea03dc9479406a7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 19 Jul 2026 18:32:20 -1000 Subject: [PATCH 24/40] [platformio] Accept git URLs passed as the library name (#17697) --- esphome/platformio/library.py | 39 ++++++++--- tests/unit_tests/test_espidf_component.py | 78 +++++++++++++++++++++ tests/unit_tests/test_platformio_library.py | 42 +++++++++++ 3 files changed, 150 insertions(+), 9 deletions(-) diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 0ffac65e0d..72a50b795b 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -25,7 +25,7 @@ from pathlib import Path import re import tempfile from typing import Any -from urllib.parse import urlparse, urlsplit, urlunsplit +from urllib.parse import urlsplit, urlunsplit from esphome import git from esphome.core import CORE, Library @@ -523,6 +523,17 @@ class _LibNode: edges: set[str] = field(default_factory=set) +def _url_or_none(value: Any) -> str | None: + """Return ``value`` if it parses as a URL (scheme and host), else None.""" + if not value or not isinstance(value, str): + return None + try: + parsed = urlsplit(value) + except ValueError: + return None + return value if parsed.scheme and parsed.netloc else None + + def _node_key( name: str | None, version: str | None, repository: str | None ) -> tuple[str, bool, tuple[str | None, str | None]]: @@ -533,9 +544,23 @@ def _node_key( inconsistently -- bare ``name`` vs ``owner/name``, or git vs registry -- maps to distinct keys and isn't deduplicated; ``convert_libraries`` warns about that after resolution rather than merging the nodes. + + PlatformIO's Library Manager also accepted a git URL in the *name* + position (``add_library("https://github.com/x/y", None)``), including the + ``git+`` VCS prefix and the ``CustomName=URL`` form; recognize those here + so such specs resolve as git sources instead of failing a registry lookup. """ + if not repository and name and "://" in name: + # Try the whole name first so a bare URL whose query contains ``=`` + # stays intact; fall back to the ``CustomName=URL`` form, where the + # key derives from the URL path and the custom name is irrelevant. + repository = _url_or_none(name) or _url_or_none(name.split("=", 1)[-1]) + if repository is None: + # Anything with ``://`` was meant to be a URL; failing it fast + # beats a confusing registry "package not found" error. + raise RuntimeError(f"Invalid PIO library URL: {name}") if repository: - split_result = urlsplit(repository) + split_result = urlsplit(repository.removeprefix("git+")) key = str(split_result.path).strip("/").removesuffix(".git") ref = split_result.fragment.strip() or None url = urlunsplit(split_result._replace(fragment="")) @@ -687,13 +712,9 @@ def convert_libraries( continue # The version field may actually be a URL (git/archive dependency). dep_version = dependency["version"] - dep_url = None - try: - parsed = urlparse(dep_version) - if all([parsed.scheme, parsed.netloc]): - dep_url, dep_version = dep_version, None - except (TypeError, ValueError): - pass + dep_url = _url_or_none(dep_version) + if dep_url is not None: + dep_version = None dep_key = add_spec(dep_name, dep_version, dep_url) node.edges.add(dep_key) worklist.append(dep_key) diff --git a/tests/unit_tests/test_espidf_component.py b/tests/unit_tests/test_espidf_component.py index 89d5ce3cf2..f9ed44b8d2 100644 --- a/tests/unit_tests/test_espidf_component.py +++ b/tests/unit_tests/test_espidf_component.py @@ -456,6 +456,84 @@ def test_node_key_git_no_ref(): assert locator == ("https://github.com/foo/bar.git", None) +def test_node_key_url_in_name_is_git(): + # add_library("https://github.com/x/y", None): PlatformIO accepted a bare + # git URL as the library name, so the converter must too. + key, is_git, locator = _node_key( + "https://github.com/pstolarz/OneWireNg", None, None + ) + assert key == "pstolarz/OneWireNg" + assert is_git is True + assert locator == ("https://github.com/pstolarz/OneWireNg", None) + + +def test_node_key_url_in_name_with_ref(): + key, is_git, locator = _node_key( + "https://github.com/foo/bar.git#v1.2.3", None, None + ) + assert (key, is_git, locator) == ( + "foo/bar", + True, + ("https://github.com/foo/bar.git", "v1.2.3"), + ) + + +def test_node_key_url_in_name_git_plus_prefix(): + key, is_git, locator = _node_key("git+https://github.com/foo/bar", None, None) + assert (key, is_git, locator) == ( + "foo/bar", + True, + ("https://github.com/foo/bar", None), + ) + + +def test_node_key_git_plus_prefix_in_repository(): + _key, is_git, locator = _node_key("name", None, "git+https://github.com/foo/bar") + assert (is_git, locator) == (True, ("https://github.com/foo/bar", None)) + + +def test_node_key_custom_name_equals_url_is_git(): + key, is_git, locator = _node_key( + "OneWireNg=https://github.com/pstolarz/OneWireNg", None, None + ) + assert (key, is_git, locator) == ( + "pstolarz/OneWireNg", + True, + ("https://github.com/pstolarz/OneWireNg", None), + ) + + +def test_node_key_url_in_name_with_query_containing_equals(): + # A bare URL whose query string contains ``=`` must not be split by the + # CustomName=URL handling. + key, is_git, locator = _node_key("https://host/x/y.git?ref=main", None, None) + assert (key, is_git, locator) == ( + "x/y", + True, + ("https://host/x/y.git?ref=main", None), + ) + + +@pytest.mark.parametrize("name", ["http://[::1", "CustomName=http://[::1"]) +def test_node_key_malformed_url_in_name_raises(name: str) -> None: + # A name that was clearly meant to be a URL but does not parse must fail + # fast instead of degrading to a confusing registry lookup error. + with pytest.raises(RuntimeError, match="Invalid PIO library URL"): + _node_key(name, None, None) + + +def test_node_key_name_with_equals_but_no_url_is_registry(): + key, is_git, locator = _node_key("FOO=BAR", "1.0", None) + assert (key, is_git, locator) == ("FOO=BAR", False, (None, "FOO=BAR")) + + +def test_node_key_version_url_still_ignored_when_name_plain(): + # A version that is a URL is handled by the dependency walk, not here; + # a plain name must stay a registry spec regardless of version shape. + key, is_git, _locator = _node_key("bar", "https://github.com/foo/bar", None) + assert (key, is_git) == ("bar", False) + + def test_node_key_registry_owner_name(): key, is_git, locator = _node_key("foo/bar", "^1.0.0", None) assert (key, is_git, locator) == ("foo/bar", False, ("foo", "bar")) diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index 03360eab37..6a4c057469 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -212,6 +212,48 @@ def test_convert_libraries_handles_unparsable_dependency_version(tmp_path, monke assert [d.name for d in top[0].dependencies] == ["C"] +@pytest.mark.parametrize( + ("value", "expected"), + [ + (None, None), + ("", None), + ("http://[::1", None), # malformed IPv6 makes urlsplit raise ValueError + ("foo/bar", None), + ("file:///no/host", None), + ("https://github.com/x/y", "https://github.com/x/y"), + ], +) +def test_url_or_none(value: str | None, expected: str | None) -> None: + assert lib._url_or_none(value) == expected + + +def test_convert_libraries_url_in_name_resolves_as_git( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # add_library("https://github.com/x/y", None) puts a git URL in the name + # position; it must resolve as a git source and never hit the registry. + _patch_download_with_manifests( + monkeypatch, tmp_path, {"pstolarz/OneWireNg": {"name": "OneWireNg"}} + ) + + def fail_registry(owner: str, pkgname: str, requirements: set[str]) -> None: + raise AssertionError(f"registry consulted for {owner}/{pkgname}") + + # After the helper so this stub wins over the helper's benign one + monkeypatch.setattr(lib, "_resolve_registry_version", fail_registry) + + top = convert_libraries( + [Library("https://github.com/pstolarz/OneWireNg", None, None)], _backend() + ) + + assert [c.name for c in top] == ["pstolarz/OneWireNg"] + assert top[0].data["name"] == "OneWireNg" + source = top[0].source + assert isinstance(source, GitSource) + assert source.url == "https://github.com/pstolarz/OneWireNg" + assert source.ref is None + + def test_convert_libraries_skips_incompatible_dependency(tmp_path, monkeypatch): # A dependency that declares an incompatible platform is skipped (the # top-level library still builds). From 9de7bd74618450860848e299b845b451ee946363 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 19 Jul 2026 18:34:29 -1000 Subject: [PATCH 25/40] [git] Detect interrupted clones and re-clone automatically (#17690) --- esphome/git.py | 71 +++++++- tests/unit_tests/test_git.py | 309 ++++++++++++++++++++++++++++++++++- 2 files changed, 373 insertions(+), 7 deletions(-) diff --git a/esphome/git.py b/esphome/git.py index c4a612753b..0c1ad56367 100644 --- a/esphome/git.py +++ b/esphome/git.py @@ -10,14 +10,22 @@ import time import urllib.parse import esphome.config_validation as cv -from esphome.core import CORE, TimePeriodSeconds -from esphome.helpers import rmtree +from esphome.core import CORE, EsphomeError, TimePeriodSeconds +from esphome.helpers import rmtree, write_file _LOGGER = logging.getLogger(__name__) # Special value to indicate never refresh NEVER_REFRESH = TimePeriodSeconds(seconds=-1) +# Written inside .git only after every clone step (clone, ref fetch, reset, +# submodule init) has completed. A directory without it is an interrupted +# clone (e.g. the process was killed mid-clone) and must be re-cloned; without +# this check such a directory would be trusted forever when the caller uses +# NEVER_REFRESH. Lives in .git so stash/reset/checkout can never touch it and +# it does not pollute the worktree. +_CLONE_COMPLETE_MARKER = "esphome_clone_complete" + class GitException(cv.Invalid): """Base exception for git-related errors.""" @@ -95,6 +103,26 @@ def _compute_destination_path(key: str, domain: str) -> Path: return base_dir / h.hexdigest()[:8] +def _clone_complete_marker_path(repo_dir: Path) -> Path: + return repo_dir / ".git" / _CLONE_COMPLETE_MARKER + + +def _remove_repo_dir(repo_dir: Path) -> None: + """Remove a repo directory, deleting the completion marker first. + + Marker-first ordering guarantees an interrupted removal can never leave a + marker behind next to a partially deleted worktree. The unlink is best + effort: if it fails (e.g. a file lock on Windows), rmtree below still + gets the chance to remove the directory, marker included. + """ + try: + _clone_complete_marker_path(repo_dir).unlink(missing_ok=True) + except OSError as err: + _LOGGER.debug("Could not delete clone completion marker first: %s", err) + if repo_dir.is_dir(): + rmtree(repo_dir) + + def resolve_symlink_stub(repo_dir: Path, file_path: Path) -> Path | None: """Return the symlink target if ``file_path`` is a Windows-checked-out symlink stub. @@ -201,9 +229,19 @@ def clone_or_update( ) repo_dir = _compute_destination_path(key, domain) + hash_dir_name = repo_dir.name if subpath: repo_dir = repo_dir / subpath + if repo_dir.is_dir() and not _clone_complete_marker_path(repo_dir).is_file(): + # The last clone never finished (killed process, container stop) or + # predates the marker; either way it cannot be trusted, especially + # with NEVER_REFRESH where it would otherwise be reused forever. + _LOGGER.warning( + "Removing incomplete clone of %s at %s, will re-clone", key, repo_dir + ) + _remove_repo_dir(repo_dir) + if not repo_dir.is_dir(): _LOGGER.info("Cloning %s", key) _LOGGER.debug("Location: %s", repo_dir) @@ -233,14 +271,28 @@ def clone_or_update( + submodules, git_dir=repo_dir, ) + except GitException: # Remove incomplete clone to prevent stale state. Without this, # a failed ref fetch leaves a clone on the default branch, and # subsequent calls skip the update due to the refresh window. - if repo_dir.is_dir(): - rmtree(repo_dir) + _remove_repo_dir(repo_dir) raise + # Every git step succeeded; the key and hash dir name are recorded + # purely to make cache debugging easier. The marker is only a + # validity signal, so a failed write must not fail an otherwise + # complete clone: the only cost is a re-clone on the next run. + try: + write_file( + _clone_complete_marker_path(repo_dir), + f"key={key}\nhash={hash_dir_name}\n", + ) + except EsphomeError as err: + _LOGGER.warning( + "Could not write clone completion marker for %s: %s", key, err + ) + else: if refresh == NEVER_REFRESH or CORE.skip_external_update: _LOGGER.debug("Skipping update for %s (refresh disabled)", key) @@ -250,7 +302,13 @@ def clone_or_update( # On first clone, FETCH_HEAD does not exist if not file_timestamp.exists(): file_timestamp = Path(repo_dir / ".git" / "HEAD") - age_seconds = time.time() - file_timestamp.stat().st_mtime + try: + age_seconds = time.time() - file_timestamp.stat().st_mtime + except OSError: + # A .git with neither FETCH_HEAD nor HEAD is corrupt (e.g. a + # partially deleted clone). Force the update path so the + # broken-repository recovery below removes and re-clones it. + age_seconds = float("inf") if refresh is None or age_seconds > refresh.total_seconds: # Try to update the repository, recovering from broken state if needed old_sha: str | None = None @@ -303,7 +361,7 @@ def clone_or_update( err, ) _LOGGER.info("Removing broken repository at %s", repo_dir) - rmtree(repo_dir) + _remove_repo_dir(repo_dir) _LOGGER.info("Successfully removed broken repository, re-cloning...") # Recursively call clone_or_update to re-clone @@ -316,6 +374,7 @@ def clone_or_update( username=username, password=password, submodules=submodules, + subpath=subpath, _recover_broken=False, ) _LOGGER.info("Repository %s successfully recovered", key) diff --git a/tests/unit_tests/test_git.py b/tests/unit_tests/test_git.py index 62d2344069..c9e0339ad7 100644 --- a/tests/unit_tests/test_git.py +++ b/tests/unit_tests/test_git.py @@ -1,5 +1,6 @@ """Tests for git.py module.""" +from collections.abc import Callable import os from pathlib import Path import time @@ -9,7 +10,7 @@ from unittest.mock import Mock, patch import pytest from esphome import git -from esphome.core import CORE, TimePeriodSeconds +from esphome.core import CORE, EsphomeError, TimePeriodSeconds from esphome.git import GitCommandError @@ -19,6 +20,15 @@ def _compute_repo_dir(url: str, ref: str | None, domain: str) -> Path: return git._compute_destination_path(key, domain) +# The tests must probe the exact location the implementation uses +_marker_path = git._clone_complete_marker_path + + +def _mark_clone_complete(repo_dir: Path) -> None: + """Write the completion marker so a hand-made repo dir is treated as valid.""" + _marker_path(repo_dir).write_text("test") + + def _setup_old_repo(repo_dir: Path, days_old: int = 2) -> None: """Helper to set up a git repo directory structure with an old timestamp. @@ -30,6 +40,7 @@ def _setup_old_repo(repo_dir: Path, days_old: int = 2) -> None: repo_dir.mkdir(parents=True) git_dir = repo_dir / ".git" git_dir.mkdir() + _mark_clone_complete(repo_dir) # Create FETCH_HEAD file with old timestamp fetch_head = git_dir / "FETCH_HEAD" @@ -54,6 +65,25 @@ def _get_git_command_type(cmd: list[str]) -> str | None: return None +def _simulate_cloned_repo(repo_dir: Path) -> None: + """Create the directory structure a successful git clone would leave.""" + repo_dir.mkdir(parents=True, exist_ok=True) + (repo_dir / ".git").mkdir(exist_ok=True) + + +def _make_clone_side_effect(repo_dir: Path) -> Callable[..., str]: + """Return a run_git_command side effect whose clone creates the repo dir.""" + + def git_command_side_effect( + cmd: list[str], cwd: str | None = None, **kwargs: Any + ) -> str: + if _get_git_command_type(cmd) == "clone": + _simulate_cloned_repo(repo_dir) + return "" + + return git_command_side_effect + + def test_run_git_command_success(tmp_path: Path) -> None: """Test that run_git_command returns output on success.""" # Create a simple git repo to test with @@ -217,6 +247,7 @@ def test_clone_or_update_with_never_refresh( repo_dir.mkdir(parents=True) git_dir = repo_dir / ".git" git_dir.mkdir() + _mark_clone_complete(repo_dir) # Create FETCH_HEAD file with current timestamp fetch_head = git_dir / "FETCH_HEAD" @@ -250,6 +281,7 @@ def test_clone_or_update_skips_when_core_skip_external_update( repo_dir.mkdir(parents=True) git_dir = repo_dir / ".git" git_dir.mkdir() + _mark_clone_complete(repo_dir) (git_dir / "FETCH_HEAD").write_text("test") CORE.skip_external_update = True @@ -281,6 +313,7 @@ def test_clone_or_update_with_refresh_updates_old_repo( repo_dir.mkdir(parents=True) git_dir = repo_dir / ".git" git_dir.mkdir() + _mark_clone_complete(repo_dir) # Create FETCH_HEAD file with old timestamp (2 days ago) fetch_head = git_dir / "FETCH_HEAD" @@ -329,6 +362,7 @@ def test_clone_or_update_with_refresh_skips_fresh_repo( repo_dir.mkdir(parents=True) git_dir = repo_dir / ".git" git_dir.mkdir() + _mark_clone_complete(repo_dir) # Create FETCH_HEAD file with recent timestamp (1 hour ago) fetch_head = git_dir / "FETCH_HEAD" @@ -371,6 +405,8 @@ def test_clone_or_update_clones_missing_repo( # repo_dir should NOT exist assert not repo_dir.exists() + mock_run_git_command.side_effect = _make_clone_side_effect(repo_dir) + # Test with NEVER_REFRESH - should still clone since repo doesn't exist result_dir, revert = git.clone_or_update( url=url, @@ -405,6 +441,7 @@ def test_clone_or_update_with_none_refresh_always_updates( repo_dir.mkdir(parents=True) git_dir = repo_dir / ".git" git_dir.mkdir() + _mark_clone_complete(repo_dir) # Create FETCH_HEAD file with very recent timestamp (1 second ago) fetch_head = git_dir / "FETCH_HEAD" @@ -486,6 +523,9 @@ def test_clone_or_update_recovers_from_git_failures( # Default successful responses if cmd_type == "rev-parse": return "abc123" + if cmd_type == "clone": + # Simulate the recovery re-clone creating the repo directory + _simulate_cloned_repo(repo_dir) return "" mock_run_git_command.side_effect = git_command_side_effect @@ -813,6 +853,273 @@ def test_clone_or_update_stale_clone_is_retried_after_cleanup( assert call_count["fetch"] == 2 +def test_clone_or_update_recloned_when_marker_missing_with_never_refresh( + tmp_path: Path, mock_run_git_command: Mock +) -> None: + """A repo dir without the completion marker is an interrupted clone. + + It must be removed and re-cloned even with NEVER_REFRESH, which would + otherwise trust the broken directory forever. + """ + CORE.config_path = tmp_path / "test.yaml" + + url = "https://github.com/test/repo" + ref = "1.8.4" + domain = "test" + repo_dir = _compute_repo_dir(url, ref, domain) + + # Simulate an interrupted clone: directory exists, no marker + repo_dir.mkdir(parents=True) + (repo_dir / ".git").mkdir() + + mock_run_git_command.side_effect = _make_clone_side_effect(repo_dir) + + result_dir, _ = git.clone_or_update( + url=url, ref=ref, refresh=git.NEVER_REFRESH, domain=domain + ) + + clone_calls = [c for c in mock_run_git_command.call_args_list if "clone" in c[0][0]] + assert len(clone_calls) == 1 + assert result_dir == repo_dir + # The fresh clone completed, so the marker must now be present + assert _marker_path(repo_dir).is_file() + + +def test_clone_or_update_recloned_when_marker_missing_with_skip_external_update( + tmp_path: Path, mock_run_git_command: Mock +) -> None: + """skip_external_update must not preserve an interrupted clone.""" + CORE.config_path = tmp_path / "test.yaml" + + url = "https://github.com/test/repo" + domain = "test" + repo_dir = _compute_repo_dir(url, None, domain) + + repo_dir.mkdir(parents=True) + (repo_dir / ".git").mkdir() + + mock_run_git_command.side_effect = _make_clone_side_effect(repo_dir) + + CORE.skip_external_update = True + result_dir, _ = git.clone_or_update( + url=url, ref=None, refresh=TimePeriodSeconds(days=1), domain=domain + ) + + clone_calls = [c for c in mock_run_git_command.call_args_list if "clone" in c[0][0]] + assert len(clone_calls) == 1 + assert result_dir == repo_dir + assert _marker_path(repo_dir).is_file() + + +def test_fresh_clone_writes_completion_marker_with_debug_info( + tmp_path: Path, mock_run_git_command: Mock +) -> None: + """The marker is written after a fresh clone and records key and hash dir.""" + CORE.config_path = tmp_path / "test.yaml" + + url = "https://github.com/test/repo" + ref = "main" + domain = "test" + repo_dir = _compute_repo_dir(url, ref, domain) + + mock_run_git_command.side_effect = _make_clone_side_effect(repo_dir) + + git.clone_or_update(url=url, ref=ref, refresh=git.NEVER_REFRESH, domain=domain) + + marker = _marker_path(repo_dir) + assert marker.is_file() + content = marker.read_text() + assert f"{url}@{ref}" in content + assert repo_dir.name in content + + +def test_marker_is_deleted_before_rmtree( + tmp_path: Path, mock_run_git_command: Mock +) -> None: + """The marker must be gone even if rmtree fails partway. + + Simulated by an rmtree that does nothing: the directory survives but the + marker must already have been deleted, so the next run still re-clones + instead of trusting a partially deleted worktree. + """ + CORE.config_path = tmp_path / "test.yaml" + + url = "https://github.com/test/repo" + ref = "main" + domain = "test" + repo_dir = _compute_repo_dir(url, ref, domain) + + _setup_old_repo(repo_dir) + assert _marker_path(repo_dir).is_file() + + def git_command_side_effect( + cmd: list[str], cwd: str | None = None, **kwargs: Any + ) -> str: + if _get_git_command_type(cmd) == "stash": + raise GitCommandError("fatal: unable to write new index file") + return "abc123" + + mock_run_git_command.side_effect = git_command_side_effect + + with ( + patch("esphome.git.rmtree"), + pytest.raises(GitCommandError), + ): + git.clone_or_update( + url=url, ref=ref, refresh=TimePeriodSeconds(days=1), domain=domain + ) + + # rmtree never deleted anything, yet the marker is gone + assert repo_dir.is_dir() + assert not _marker_path(repo_dir).is_file() + + +def test_failed_marker_write_does_not_fail_the_clone( + tmp_path: Path, + mock_run_git_command: Mock, + caplog: pytest.LogCaptureFixture, +) -> None: + """A marker write failure must not fail an otherwise complete clone. + + The clone is valid; the missing marker only costs a re-clone on the next + run, so the error is logged as a warning instead of propagating. + """ + CORE.config_path = tmp_path / "test.yaml" + + url = "https://github.com/test/repo" + domain = "test" + repo_dir = _compute_repo_dir(url, None, domain) + + mock_run_git_command.side_effect = _make_clone_side_effect(repo_dir) + + with patch( + "esphome.git.write_file", side_effect=EsphomeError("Could not write file") + ): + result_dir, _ = git.clone_or_update( + url=url, ref=None, refresh=git.NEVER_REFRESH, domain=domain + ) + + assert result_dir == repo_dir + assert not _marker_path(repo_dir).is_file() + assert "Could not write clone completion marker" in caplog.text + + +def test_corrupt_git_dir_without_head_recovers( + tmp_path: Path, mock_run_git_command: Mock +) -> None: + """A .git with neither FETCH_HEAD nor HEAD must recover, not crash. + + The age check stats FETCH_HEAD falling back to HEAD; if both are gone + (partially deleted clone) the stat raised an unhandled FileNotFoundError + before the broken-repository recovery could run. + """ + CORE.config_path = tmp_path / "test.yaml" + + url = "https://github.com/test/repo" + domain = "test" + repo_dir = _compute_repo_dir(url, None, domain) + + # Marker present but .git gutted: no FETCH_HEAD, no HEAD + repo_dir.mkdir(parents=True) + (repo_dir / ".git").mkdir() + _mark_clone_complete(repo_dir) + + def git_command_side_effect( + cmd: list[str], cwd: str | None = None, **kwargs: Any + ) -> str: + cmd_type = _get_git_command_type(cmd) + if cmd_type == "rev-parse": + raise GitCommandError("ambiguous argument 'HEAD': unknown revision") + if cmd_type == "clone": + _simulate_cloned_repo(repo_dir) + return "" + + mock_run_git_command.side_effect = git_command_side_effect + + result_dir, _ = git.clone_or_update( + url=url, ref=None, refresh=TimePeriodSeconds(days=1), domain=domain + ) + + assert result_dir == repo_dir + clone_calls = [c for c in mock_run_git_command.call_args_list if "clone" in c[0][0]] + assert len(clone_calls) == 1 + assert _marker_path(repo_dir).is_file() + + +def test_remove_repo_dir_tolerates_marker_unlink_failure(tmp_path: Path) -> None: + """A locked marker file must not abort the directory removal.""" + repo_dir = tmp_path / "repo" + repo_dir.mkdir() + (repo_dir / ".git").mkdir() + _mark_clone_complete(repo_dir) + + with patch.object(Path, "unlink", side_effect=PermissionError("locked")): + git._remove_repo_dir(repo_dir) + + # rmtree still removed the directory, marker included + assert not repo_dir.exists() + + +def test_clone_or_update_recovery_preserves_subpath( + tmp_path: Path, mock_run_git_command: Mock +) -> None: + """Recovery must re-clone into the same subpath-ed directory. + + Without passing subpath through, the recursive recovery call would + recompute the destination without the subpath and clone (and write the + completion marker) at the wrong location. + """ + CORE.config_path = tmp_path / "test.yaml" + + url = "https://github.com/test/repo" + ref = "main" + domain = "test" + subpath = Path("mylib") + repo_dir = _compute_repo_dir(url, ref, domain) / subpath + + _setup_old_repo(repo_dir) + + call_counts: dict[str, int] = {} + + def git_command_side_effect( + cmd: list[str], cwd: str | None = None, **kwargs: Any + ) -> str: + cmd_type = _get_git_command_type(cmd) + if cmd_type: + call_counts[cmd_type] = call_counts.get(cmd_type, 0) + 1 + # First rev-parse fails (broken repo) to trigger recovery + if cmd_type == "rev-parse" and call_counts[cmd_type] == 1: + raise GitCommandError( + "ambiguous argument 'HEAD': unknown revision or path not in the working tree." + ) + if cmd_type == "clone": + # Create whatever directory the clone was asked to target + target = Path(cmd[-1]) + target.mkdir(parents=True, exist_ok=True) + (target / ".git").mkdir(exist_ok=True) + if cmd_type == "rev-parse": + return "abc123" + return "" + + mock_run_git_command.side_effect = git_command_side_effect + + result_dir, _ = git.clone_or_update( + url=url, + ref=ref, + refresh=TimePeriodSeconds(days=1), + domain=domain, + subpath=subpath, + ) + + # The recovery re-clone must target the subpath-ed directory and the + # completion marker must land there too + clone_calls = [c for c in mock_run_git_command.call_args_list if "clone" in c[0][0]] + assert len(clone_calls) == 1 + assert clone_calls[0][0][0][-1] == str(repo_dir) + assert result_dir == repo_dir + assert _marker_path(repo_dir).is_file() + + def test_clone_with_ref_uses_shallow_fetch( tmp_path: Path, mock_run_git_command: Mock ) -> None: From 3ffc3a961033c3ddedb3dfa2ec47da60291e0621 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 19 Jul 2026 19:05:50 -1000 Subject: [PATCH 26/40] [espidf] Make openocd-esp32 optional so its libusb check cannot break installs (#17686) --- esphome/espidf/framework.py | 130 ++++++++++++++++------ tests/unit_tests/test_espidf_framework.py | 49 ++++++++ 2 files changed, 147 insertions(+), 32 deletions(-) diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index b8e0d4cfca..fce6a88ccf 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -1,5 +1,6 @@ """ESP-IDF framework tools for ESPHome.""" +from collections.abc import Callable from ctypes.util import find_library import json import logging @@ -467,17 +468,21 @@ _NINJA_ARM64_BACKPORT: dict[str, dict[str, str | int]] = { } -def _patch_tools_json_for_linux_arm64(framework_path: Path) -> None: - """Inject ninja linux-arm64 entries into the framework's tools.json on aarch64. +def _patch_tools_json( + framework_path: Path, + apply_patch: Callable[[dict], bool], + patched_log: str, +) -> None: + """Apply an in-place fixup to the framework's tools/tools.json. - Idempotent: a tools.json that already has the entry, or a host that - isn't aarch64, is a no-op. Applied unconditionally on every install - check so a build dir extracted before the backport got fixed up - without forcing a clean. + Shared plumbing for the tools.json patches below: a missing file is a + no-op, an unparseable file logs a warning and skips, and when + ``apply_patch`` reports a change the file is written back atomically. + ``patched_log`` is the info log line, with a single ``%s`` placeholder + for the tools.json path. Patches are idempotent and applied on every + install check, so an already-extracted framework picks them up on the + next build without forcing a clean. """ - if platform.machine() != "aarch64": - return - tools_json = framework_path / "tools" / "tools.json" if not tools_json.is_file(): return @@ -485,37 +490,93 @@ def _patch_tools_json_for_linux_arm64(framework_path: Path) -> None: try: with tools_json.open(encoding="utf-8") as f: data = json.load(f) - except (json.JSONDecodeError, OSError) as e: + # apply_patch also raises inside the guard: a tools.json that is + # valid JSON but not the expected shape (e.g. a top-level list) + # must skip the patch, not crash the install check this patch is + # meant to recover. + changed = apply_patch(data) + except (json.JSONDecodeError, OSError, AttributeError, TypeError, KeyError) as e: _LOGGER.warning( - "Could not parse %s for linux-arm64 backport (%s); " - "skipping. A clean reinstall of the framework directory " - "may be needed.", + "Could not apply tools.json patch to %s (%s); skipping. A clean " + "reinstall of the framework directory may be needed.", tools_json, e, ) return - changed = False - for tool in data.get("tools", []): - if tool.get("name") != "ninja": - continue - for ver in tool.get("versions", []): - entry = _NINJA_ARM64_BACKPORT.get(ver.get("name")) - if entry is None or ver.get("linux-arm64"): - continue - ver["linux-arm64"] = entry - changed = True - if changed: # write_file_if_changed stages a tempfile in the destination dir # and atomically replaces — safe against mid-write interruption # and concurrent invocations. write_file_if_changed(tools_json, json.dumps(data, indent=2) + "\n") - _LOGGER.info( - "Patched %s to add ninja linux-arm64 download " - "(espressif/esp-idf#18272 backport).", - tools_json, - ) + _LOGGER.info(patched_log, tools_json) + + +def _patch_tools_json_for_linux_arm64(framework_path: Path) -> None: + """Inject ninja linux-arm64 entries into the framework's tools.json on aarch64. + + A tools.json that already has the entry, or a host that isn't aarch64, + is a no-op. + """ + if platform.machine() != "aarch64": + return + + def apply_patch(data: dict) -> bool: + changed = False + for tool in data.get("tools", []): + if tool.get("name") != "ninja": + continue + for ver in tool.get("versions", []): + entry = _NINJA_ARM64_BACKPORT.get(ver.get("name")) + if entry is None or ver.get("linux-arm64"): + continue + ver["linux-arm64"] = entry + changed = True + return changed + + _patch_tools_json( + framework_path, + apply_patch, + "Patched %s to add ninja linux-arm64 download " + "(espressif/esp-idf#18272 backport).", + ) + + +def _patch_tools_json_demote_openocd(framework_path: Path) -> None: + """Demote openocd-esp32 from ``install: always`` to ``install: on_request``. + + ``idf_tools.py install required`` installs every tool marked ``always`` in + tools.json and validates each one after extraction by running its version + command. openocd links against libusb-1.0, which minimal systems (bare LXC + containers, slim images) often lack, so that one validation aborted the + whole framework install and left it permanently retrying (#17685) — even + though ESPHome never runs openocd (it is a JTAG debugging tool). Demoting + it drops it from the ``required`` set: it is no longer downloaded or + validated, and the tool-path export treats a missing ``on_request`` tool + as fine. A user who wants it can still name ``openocd-esp32`` explicitly + in ESPHOME_IDF_DEFAULT_TOOLS; explicit names bypass install-type + filtering. + + Because this runs on every install check, an install stuck in the + failing state (which never wrote its stamp file) heals on the next + build without a clean. + """ + + def apply_patch(data: dict) -> bool: + changed = False + for tool in data.get("tools", []): + if tool.get("name") == "openocd-esp32" and tool.get("install") == "always": + tool["install"] = "on_request" + changed = True + return changed + + _patch_tools_json( + framework_path, + apply_patch, + "Patched %s to make openocd-esp32 optional (not needed for " + "building, and its install check fails on systems without " + "libusb-1.0).", + ) def _check_esphome_idf_framework_install( @@ -636,6 +697,11 @@ def _check_esphome_idf_framework_install( # a pre-patch tools.json get fixed up without forcing a clean. _patch_tools_json_for_linux_arm64(framework_path) + # Drop openocd-esp32 from the required tool set on every invocation so + # an install that previously failed on its libusb check recovers on the + # next build. + _patch_tools_json_demote_openocd(framework_path) + # 3. Check if the framework tools are the same and correctly installed if not install: install = True @@ -671,9 +737,9 @@ def _check_esphome_idf_framework_install( ): if platform.system() == "Linux" and find_library("usb-1.0") is None: _LOGGER.error( - "libusb-1.0.so.0 was not found on this system and the ESP-IDF " - "tools need it (openocd fails its install check without it). " - "Install the libusb 1.0 package, e.g. libusb-1.0-0 " + "libusb-1.0.so.0 was not found on this system. If the error " + "above mentions it (openocd fails its install check without " + "it), install the libusb 1.0 package, e.g. libusb-1.0-0 " "(Debian/Ubuntu), libusb1 (Fedora) or libusb (Alpine/Arch), " "then run the build again." ) diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index a1af5ae54c..79a50059cd 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -25,6 +25,7 @@ from esphome.espidf.framework import ( _get_python_env_path, _get_python_version, _parse_git_source, + _patch_tools_json_demote_openocd, _patch_tools_json_for_linux_arm64, _windows_long_paths_enabled, _write_idf_version_txt, @@ -331,6 +332,7 @@ def espidf_mocks(setup_core: Path): patch("esphome.espidf.framework._clone_idf_with_submodules") as clone, patch("esphome.espidf.framework._write_idf_version_txt"), patch("esphome.espidf.framework._patch_tools_json_for_linux_arm64"), + patch("esphome.espidf.framework._patch_tools_json_demote_openocd"), patch("esphome.espidf.framework._write_stamp"), patch("esphome.espidf.framework._check_stamp", return_value=True), patch("esphome.espidf.framework._get_idf_version", return_value=_IDF_VERSION), @@ -612,6 +614,53 @@ def test_patch_tools_json_already_patched_is_noop(tmp_path: Path) -> None: assert tools_json.read_text(encoding="utf-8") == before +# --------------------------------------------------------------------------- +# _patch_tools_json_demote_openocd (openocd-esp32 made optional) +# --------------------------------------------------------------------------- + + +def test_demote_openocd_patches_install_type(tmp_path: Path) -> None: + tools_json = _write_tools_json( + tmp_path, + { + "tools": [ + {"name": "openocd-esp32", "install": "always"}, + {"name": "cmake", "install": "always"}, + ] + }, + ) + _patch_tools_json_demote_openocd(tmp_path) + + data = json.loads(tools_json.read_text(encoding="utf-8")) + openocd = next(t for t in data["tools"] if t["name"] == "openocd-esp32") + cmake = next(t for t in data["tools"] if t["name"] == "cmake") + assert openocd["install"] == "on_request" + # other tools are left untouched + assert cmake["install"] == "always" + + +def test_patch_tools_json_unexpected_structure_warns_and_skips( + tmp_path: Path, +) -> None: + """Valid JSON with an unexpected shape must skip the patch, not raise.""" + tools_dir = tmp_path / "tools" + tools_dir.mkdir() + tools_json = tools_dir / "tools.json" + tools_json.write_text('["not", "a", "dict"]', encoding="utf-8") + before = tools_json.read_text(encoding="utf-8") + _patch_tools_json_demote_openocd(tmp_path) # AttributeError -> skip + assert tools_json.read_text(encoding="utf-8") == before + + +def test_demote_openocd_already_patched_is_noop(tmp_path: Path) -> None: + tools_json = _write_tools_json( + tmp_path, {"tools": [{"name": "openocd-esp32", "install": "on_request"}]} + ) + before = tools_json.read_text(encoding="utf-8") + _patch_tools_json_demote_openocd(tmp_path) + assert tools_json.read_text(encoding="utf-8") == before + + # --------------------------------------------------------------------------- # Subprocess-backed helpers (_exec -> run_command rename) and get_framework_env # --------------------------------------------------------------------------- From 629afd38f6c8e23a01267663d0532a43bbbc9969 Mon Sep 17 00:00:00 2001 From: Clyde Stubbs <2366188+clydebarrow@users.noreply.github.com> Date: Mon, 20 Jul 2026 15:09:02 +1000 Subject: [PATCH 27/40] [light] Fix pulse and other effects (#17645) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> --- esphome/components/light/light_call.cpp | 13 +-- esphome/components/light/light_state.cpp | 8 ++ .../light_effect_zero_brightness.yaml | 35 +++++++ .../fixtures/light_initial_state.yaml | 18 ++++ tests/integration/test_light_calls.py | 10 +- .../test_light_effect_zero_brightness.py | 91 +++++++++++++++++++ tests/integration/test_light_initial_state.py | 15 +++ 7 files changed, 177 insertions(+), 13 deletions(-) create mode 100644 tests/integration/fixtures/light_effect_zero_brightness.yaml create mode 100644 tests/integration/test_light_effect_zero_brightness.py diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index 2b13b40a16..67fd175ce6 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -219,14 +219,11 @@ LightColorValues LightCall::validate_() { this->set_flag_(FLAG_HAS_STATE); } - // Make sure a turn-on makes the light visible: if the resulting brightness would be zero - // (e.g. restored from a brightness=0 turn-off), reset it to full brightness. - if (this->has_state() && this->state_ && (color_mode & ColorCapability::BRIGHTNESS)) { - float brightness = this->has_brightness() ? this->brightness_ : this->parent_->remote_values.get_brightness(); - if (brightness == 0.0f) { - this->brightness_ = 1.0f; - this->set_flag_(FLAG_HAS_BRIGHTNESS); - } + // Make sure a simple (no specific brightness) turn-on makes the light visible + if (this->has_state() && this->state_ && (color_mode & ColorCapability::BRIGHTNESS) && !this->has_brightness() && + this->parent_->remote_values.get_brightness() == 0.0f) { + this->brightness_ = 1.0f; + this->set_flag_(FLAG_HAS_BRIGHTNESS); } // Set color brightness to 100% if currently zero and a color is set. diff --git a/esphome/components/light/light_state.cpp b/esphome/components/light/light_state.cpp index bd778926d5..9d0181a05c 100644 --- a/esphome/components/light/light_state.cpp +++ b/esphome/components/light/light_state.cpp @@ -71,6 +71,14 @@ void LightState::setup() { break; } + // A light coming up on boot must never end up on-but-invisible: if the resolved restore + // state is on but its brightness is zero (e.g. a stale/persisted value from before a + // forced-on restore mode, or an inverted restore flipping a dim-to-0 off state to on), + // reset it to full brightness. + if (recovered.state && recovered.brightness == 0.0f) { + recovered.brightness = 1.0f; + } + call.set_color_mode_if_supported(recovered.color_mode); call.set_state(recovered.state); call.set_brightness_if_supported(recovered.brightness); diff --git a/tests/integration/fixtures/light_effect_zero_brightness.yaml b/tests/integration/fixtures/light_effect_zero_brightness.yaml new file mode 100644 index 0000000000..b98bed84db --- /dev/null +++ b/tests/integration/fixtures/light_effect_zero_brightness.yaml @@ -0,0 +1,35 @@ +esphome: + name: light-effect-zero-bright +host: +api: # Port will be automatically injected +logger: + level: DEBUG + +output: + - platform: template + id: pulse_output + type: float + write_action: + - logger.log: + format: "PULSE_OUTPUT:%.4f" + args: [state] + +light: + - platform: monochromatic + name: "Test Pulse Light" + id: test_pulse_light + output: pulse_output + effects: + - pulse: + name: "Fast Pulse" + transition_length: 20ms + update_interval: 50ms + min_brightness: 0% + max_brightness: 100% + - strobe: + name: "Fast Strobe" + colors: + - state: true + duration: 50ms + - state: false + duration: 50ms diff --git a/tests/integration/fixtures/light_initial_state.yaml b/tests/integration/fixtures/light_initial_state.yaml index 2654c76aa0..052de0a4e5 100644 --- a/tests/integration/fixtures/light_initial_state.yaml +++ b/tests/integration/fixtures/light_initial_state.yaml @@ -21,6 +21,11 @@ output: type: float write_action: - lambda: "" + - platform: template + id: test_restore_and_on_output + type: float + write_action: + - lambda: "" light: - platform: rgb @@ -37,3 +42,16 @@ light: red: 1.0 green: 0.5 blue: 0.0 + + - platform: monochromatic + name: "Test Restore And On Light" + id: test_restore_and_on_light + output: test_restore_and_on_output + restore_mode: RESTORE_AND_ON + # Simulates a stale/persisted zero brightness: RESTORE_AND_ON always forces the light + # on at boot regardless of the recovered state, so a leftover brightness of 0 must not + # leave the light on-but-invisible. + initial_state: + color_mode: BRIGHTNESS + state: false + brightness: 0% diff --git a/tests/integration/test_light_calls.py b/tests/integration/test_light_calls.py index a3a4103f5c..b75e2fac62 100644 --- a/tests/integration/test_light_calls.py +++ b/tests/integration/test_light_calls.py @@ -341,14 +341,14 @@ async def test_light_calls( assert state.state is True assert state.brightness == pytest.approx(1.0) - # Test 31b: An explicit turn-on with brightness 0 still resets to full - # brightness - a turn-on must never leave the light on-but-invisible. This - # is the same path the restore logic exercises (set_state(true) + - # set_brightness(0) from a persisted brightness=0 turn-off). + # Test 31b: An explicit turn-on with brightness 0 respects the explicit value and + # stays dark. Only a turn-on with no brightness specified (Test 31) restores + # visibility -- an explicit brightness request (e.g. from a light effect's dark + # phase) is never overridden. client.light_command(key=rgbcw_light.key, state=True, brightness=0.0) state = await wait_for_state_change(rgbcw_light.key) assert state.state is True - assert state.brightness == pytest.approx(1.0) + assert state.brightness == pytest.approx(0.0) # Test 32: Turning a light on when it already has nonzero brightness leaves # the brightness unchanged (the reset only happens when brightness is 0). diff --git a/tests/integration/test_light_effect_zero_brightness.py b/tests/integration/test_light_effect_zero_brightness.py new file mode 100644 index 0000000000..6c386d4229 --- /dev/null +++ b/tests/integration/test_light_effect_zero_brightness.py @@ -0,0 +1,91 @@ +"""Integration test verifying light effects can dim to 0% brightness while staying on. + +Regression test for https://github.com/esphome/esphome/issues/17639, where PR #17103's +"make turn-on visible" logic in LightCall::validate_() also clobbered brightness set by a +running effect (e.g. pulse, strobe), forcing it back to 100% and breaking the dark phase +of those effects. + +Effect ticks are published with `publish: false` (so Home Assistant isn't spammed with +every frame), so the effect's actual output can't be observed via API state broadcasts. +Instead, this test reads the output component's log lines, which are written on every +update regardless of the publish flag. +""" + +from __future__ import annotations + +import asyncio +import re +from typing import Any + +from aioesphomeapi import EntityState, LightState +import pytest + +from .state_utils import InitialStateHelper +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_light_effect_zero_brightness( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Pulse and strobe effects must be able to reach 0% brightness while the light stays on.""" + output_pattern = re.compile(r"PULSE_OUTPUT:([\d.]+)") + observed: list[float] = [] + + def on_log_line(line: str) -> None: + match = output_pattern.search(line) + if match: + observed.append(float(match.group(1))) + + async with ( + run_compiled(yaml_config, line_callback=on_log_line), + api_client_connected() as client, + ): + entities, _ = await client.list_entities_services() + light = next(e for e in entities if e.object_id == "test_pulse_light") + + state_futures: dict[int, asyncio.Future[LightState]] = {} + + def on_state(state: EntityState) -> None: + if isinstance(state, LightState) and state.key in state_futures: + future = state_futures[state.key] + if not future.done(): + future.set_result(state) + + # ESPHome sends the current state of every entity right after connecting; drain + # that initial burst so it can't be mistaken for the response to a command below. + initial_state_helper = InitialStateHelper(entities) + client.subscribe_states(initial_state_helper.on_state_wrapper(on_state)) + await initial_state_helper.wait_for_initial_states() + + async def send_and_wait(timeout: float = 5.0, **kwargs: Any) -> LightState: + """Send a light command and wait for the matching state response.""" + state_futures[light.key] = asyncio.get_running_loop().create_future() + client.light_command(key=light.key, **kwargs) + return await asyncio.wait_for(state_futures[light.key], timeout=timeout) + + # Turn the light on first so the effect starts from a known, visible state. + state = await send_and_wait(state=True, brightness=1.0) + assert state.state is True + assert state.brightness == pytest.approx(1.0) + + for effect_name in ("Fast Pulse", "Fast Strobe"): + observed.clear() + state = await send_and_wait(effect=effect_name) + assert state.effect == effect_name + # Let several effect cycles run (update_interval/duration is 50ms in the fixture). + await asyncio.sleep(1.0) + + assert observed, f"No output observed while running effect {effect_name!r}" + assert min(observed) == pytest.approx(0.0, abs=0.01), ( + f"Effect {effect_name!r} never dimmed to 0% brightness while the light " + f"stayed on -- got min={min(observed):.4f} (values: {observed})" + ) + assert max(observed) > 0.5, ( + f"Effect {effect_name!r} never reached full brightness -- " + f"got max={max(observed):.4f}" + ) + + client.light_command(key=light.key, effect="None") diff --git a/tests/integration/test_light_initial_state.py b/tests/integration/test_light_initial_state.py index f1cd96dbf0..657e273fe7 100644 --- a/tests/integration/test_light_initial_state.py +++ b/tests/integration/test_light_initial_state.py @@ -11,6 +11,14 @@ from .state_utils import InitialStateHelper, require_entity from .types import APIClientConnectedFactory, RunCompiledFunction +@pytest.fixture(autouse=True) +def isolated_preferences(monkeypatch: pytest.MonkeyPatch, tmp_path) -> None: + """Keep host preferences per-test so RESTORE_AND_ON never loads a stale value left + behind by a previous run (host preferences otherwise persist to ~/.esphome/prefs, + keyed only by device name).""" + monkeypatch.setenv("ESPHOME_PREFDIR", str(tmp_path / "prefs")) + + @pytest.mark.asyncio async def test_light_initial_state( yaml_config: str, @@ -36,3 +44,10 @@ async def test_light_initial_state( assert state.red == pytest.approx(1.0, abs=0.01) assert state.green == pytest.approx(0.5, abs=0.01) assert state.blue == pytest.approx(0.0, abs=0.01) + + # Regression test: RESTORE_AND_ON always forces the light on at boot, even when + # the recovered/initial brightness was 0 -- it must never come up on-but-invisible. + restore_and_on_light = require_entity(entities, "test_restore_and_on_light") + restore_and_on_state = helper.initial_states[restore_and_on_light.key] + assert restore_and_on_state.state is True + assert restore_and_on_state.brightness == pytest.approx(1.0) From 83092ea05ccd2b17a6ed3c897c29600c5ccc5488 Mon Sep 17 00:00:00 2001 From: "esphome[bot]" <115708604+esphome[bot]@users.noreply.github.com> Date: Sun, 19 Jul 2026 19:32:22 -1000 Subject: [PATCH 28/40] Bump bundled esphome-device-builder to 1.6.8 (#17708) --- docker/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index 331585f123..1a34fda520 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -22,7 +22,7 @@ RUN \ -r /requirements.txt # Install the ESPHome Device Builder dashboard. -RUN uv pip install --no-cache-dir esphome-device-builder==1.6.7 +RUN uv pip install --no-cache-dir esphome-device-builder==1.6.8 RUN \ platformio settings set enable_telemetry No \ From 786b47d8c27a580432c2855681696ee4e916f1f3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 19 Jul 2026 19:32:38 -1000 Subject: [PATCH 29/40] [core] Auto-clean the PlatformIO build environment when the Python version changes (#17671) Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> --- esphome/platformio/toolchain.py | 187 ++++++++- esphome/writer.py | 15 +- requirements.txt | 1 + tests/unit_tests/test_platformio_toolchain.py | 360 ++++++++++++++++++ 4 files changed, 550 insertions(+), 13 deletions(-) diff --git a/esphome/platformio/toolchain.py b/esphome/platformio/toolchain.py index c97df812e3..105d4a8283 100644 --- a/esphome/platformio/toolchain.py +++ b/esphome/platformio/toolchain.py @@ -1,17 +1,35 @@ +from collections.abc import Iterable import json import logging import os from pathlib import Path import re import sys +from typing import TYPE_CHECKING from esphome.const import CONF_COMPILE_PROCESS_LIMIT, CONF_ESPHOME, KEY_CORE from esphome.core import CORE, EsphomeError -from esphome.helpers import add_git_ceiling_directory +from esphome.helpers import add_git_ceiling_directory, rmtree, write_file from esphome.util import FlashImage, run_external_process +if TYPE_CHECKING: + from platformio.project.config import ProjectConfig + _LOGGER = logging.getLogger(__name__) +# PlatformIO cache subdirs resolved via ProjectConfig. A full ``clean-all`` wipes +# these plus the whole ``core_dir``; a Python-version heal wipes these plus the +# penv while keeping ``core_dir`` (so the sibling stamp/lock survive). +_PIO_CACHE_DIRS = ("cache_dir", "packages_dir", "platforms_dir") + +# Marker recording the Python major.minor the PlatformIO cache was provisioned +# under, plus the lock guarding the check/wipe. Both live in the dir resolved +# by ``_pio_stamp_dir`` (NOT wiped by the heal), so they survive the wipe and +# are rewritten after it. +_PIO_PYTHON_STAMP_FILE = ".esphome.pio.stamp.json" +_PIO_PYTHON_STAMP_LOCK = ".esphome.pio.stamp.lock" +_PIO_PYTHON_STAMP_SCHEMA = "0" + def _strip_win_long_path_prefix(path: str) -> str: r"""Strip the Windows extended-length path prefix from ``path``. @@ -44,7 +62,174 @@ def _strip_win_long_path_prefix(path: str) -> str: return path +def get_platformio_config() -> "ProjectConfig | None": + """Return PlatformIO's ``ProjectConfig``, or None when PlatformIO is absent.""" + try: + from platformio.project.config import ProjectConfig + except ImportError: + return None + return ProjectConfig.get_instance() + + +def _pio_stamp_dir(config: "ProjectConfig") -> Path: + """Return the persistent home for the python-version stamp and lock. + + The parent of ``platforms_dir``, not ``core_dir``: the container/add-on + images relocate the platform/package caches to a persistent volume while + ``core_dir`` stays at the ephemeral default (its ``appstate.json`` must not + move), so a stamp under ``core_dir`` would be wiped on every image update + while the stale cache it guards survives. Everywhere else ``platforms_dir`` + sits inside ``core_dir`` and this resolves to ``core_dir``. + """ + return Path(config.get("platformio", "platforms_dir")).parent + + +def _delete_platformio_dirs(config: "ProjectConfig", pio_dirs: Iterable[str]) -> None: + """Delete each named PlatformIO dir resolved from *config*.""" + for pio_dir in pio_dirs: + path = Path(config.get("platformio", pio_dir)) + if path.is_dir(): + _LOGGER.info("Deleting PlatformIO %s %s", pio_dir, path) + rmtree(path) + + +def clean_platformio_cache() -> None: + """Wipe the whole PlatformIO cache (cache/packages/platforms/core). + + The full set ``clean-all`` (Reset Build Environment) clears. No-op when + PlatformIO is unavailable. + """ + config = get_platformio_config() + if config is None: + return + _delete_platformio_dirs(config, [*_PIO_CACHE_DIRS, "core_dir"]) + + +def _clean_platformio_python_env(config: "ProjectConfig", core_dir: Path) -> None: + """Wipe the cache subdirs + penv for a Python-version change. + + Keeps ``core_dir`` itself (and the stamp/lock siblings under it); otherwise + the same cache set ``clean-all`` clears. + """ + _delete_platformio_dirs(config, _PIO_CACHE_DIRS) + penv = core_dir / "penv" + if penv.is_dir(): + _LOGGER.info("Deleting PlatformIO penv %s", penv) + rmtree(penv) + + +def _current_python_minor() -> str: + """Return the running interpreter's ``major.minor`` (e.g. ``3.13``).""" + return f"{sys.version_info.major}.{sys.version_info.minor}" + + +def _read_pio_stamp_python(stamp_file: Path) -> str | None: + """Return the ``python_version`` recorded in *stamp_file*, or None.""" + try: + with stamp_file.open(encoding="utf-8") as f: + data = json.load(f) + except FileNotFoundError: + return None + except (json.JSONDecodeError, OSError) as err: + # A present-but-unreadable stamp is a distinct signal from an absent + # one, and it drives a cache clean; surface why at normal verbosity. + _LOGGER.warning("Could not read %s: %s", stamp_file, err) + return None + if not isinstance(data, dict): + return None + version = data.get("python_version") + return version if isinstance(version, str) else None + + +def _write_pio_stamp_python(stamp_file: Path, python_version: str) -> None: + """Atomically write the PlatformIO python-version stamp.""" + write_file( + stamp_file, + json.dumps( + { + "schema_version": _PIO_PYTHON_STAMP_SCHEMA, + "python_version": python_version, + } + ), + ) + + +def heal_platformio_python_env() -> None: + """Wipe the PlatformIO cache unless it is stamped for the running Python. + + A PlatformIO platform/tool package pins the Python versions it accepts when + it is provisioned, and ESPHome pins platforms to exact, immutable versions, + so a later interpreter bump (a container upgrading its base Python) leaves + the cached platform rejecting the new interpreter ("Python version must be + between ...") until the cache is wiped. A stamp records the ``major.minor`` + the cache was provisioned for; when it doesn't match the running + interpreter (or has never been written for an existing cache), the same + PlatformIO dirs ``clean-all`` wipes are cleaned so PlatformIO + re-provisions, matching Reset Build Environment automatically. The native + ESP-IDF toolchain already self-heals through its own stamp; this covers the + PlatformIO path. No-op when PlatformIO is unavailable. + """ + config = get_platformio_config() + if config is None: + return + try: + _check_platformio_python_stamp(config) + except (EsphomeError, OSError) as err: + # The check is a best-effort repair; a full or read-only cache volume + # must not abort a build that might otherwise work. The stamp write + # surfaces as EsphomeError (write_file wraps OSError). + _LOGGER.warning("PlatformIO build environment check failed: %s", err) + + +def _check_platformio_python_stamp(config: "ProjectConfig") -> None: + """Compare the stamp to the running interpreter; wipe and restamp on mismatch.""" + current = _current_python_minor() + stamp_dir = _pio_stamp_dir(config) + # Host the stamp/lock even before PlatformIO's first run creates the dir. + stamp_dir.mkdir(parents=True, exist_ok=True) + stamp_file = stamp_dir / _PIO_PYTHON_STAMP_FILE + + from filelock import FileLock + + with FileLock(str(stamp_dir / _PIO_PYTHON_STAMP_LOCK)): + provisioned = _read_pio_stamp_python(stamp_file) + if provisioned == current: + return + core_dir = Path(config.get("platformio", "core_dir")) + has_cache = ( + any( + Path(config.get("platformio", pio_dir)).is_dir() + for pio_dir in _PIO_CACHE_DIRS + ) + or (core_dir / "penv").is_dir() + ) + if has_cache: + if provisioned is None: + # An existing cache with no stamp predates the stamp: its + # provisioning interpreter is unknown, so clean once rather + # than leave a possibly-stale cache failing every build. + _LOGGER.info( + "Cleaning the PlatformIO build environment once so it " + "re-provisions for Python %s", + current, + ) + else: + _LOGGER.info( + "Python version changed (%s -> %s); cleaning PlatformIO " + "build environment so it re-provisions for the new " + "interpreter", + provisioned, + current, + ) + _clean_platformio_python_env(config, core_dir) + _write_pio_stamp_python(stamp_file, current) + + def run_platformio_cli(*args, **kwargs) -> str | int: + # Re-provision the PlatformIO cache if the interpreter's major.minor changed + # since it was last built; a stale platform otherwise rejects the new Python + # with "Python version must be between ..." until Reset Build Environment. + heal_platformio_python_env() os.environ["PLATFORMIO_FORCE_COLOR"] = "true" os.environ["PLATFORMIO_BUILD_DIR"] = str(CORE.relative_pioenvs_path().absolute()) os.environ.setdefault( diff --git a/esphome/writer.py b/esphome/writer.py index b7eeec916d..866377d2f5 100644 --- a/esphome/writer.py +++ b/esphome/writer.py @@ -670,18 +670,9 @@ def clean_all(configuration: list[str]): rmtree(install_path) # Clean PlatformIO project files - try: - from platformio.project.config import ProjectConfig - except ImportError: - # PlatformIO is not available, skip cleaning - pass - else: - config = ProjectConfig.get_instance() - for pio_dir in ["cache_dir", "packages_dir", "platforms_dir", "core_dir"]: - path = Path(config.get("platformio", pio_dir)) - if path.is_dir(): - _LOGGER.info("Deleting PlatformIO %s %s", pio_dir, path) - rmtree(path) + from esphome.platformio.toolchain import clean_platformio_cache + + clean_platformio_cache() GITIGNORE_CONTENT = """# Gitignore settings for ESPHome diff --git a/requirements.txt b/requirements.txt index 9c78597360..cc081d66f2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -27,6 +27,7 @@ smpclient==7.2.0 requests==2.34.2 py7zr==1.1.3 platformdirs==4.10.0 # native esp-idf toolchain global cache dir +filelock==3.29.0 # lock guarding the PlatformIO python-version cache heal # esp-idf >= 5.0 requires this pyparsing >= 3.3.2 diff --git a/tests/unit_tests/test_platformio_toolchain.py b/tests/unit_tests/test_platformio_toolchain.py index 568b43a259..013030d38f 100644 --- a/tests/unit_tests/test_platformio_toolchain.py +++ b/tests/unit_tests/test_platformio_toolchain.py @@ -2,12 +2,14 @@ # pylint: disable=protected-access +from collections.abc import Generator from contextlib import contextmanager from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer import json import os from pathlib import Path import shutil +import sys import threading from types import SimpleNamespace from unittest.mock import MagicMock, Mock, call, patch @@ -1093,3 +1095,361 @@ def test_filter_platformio_lines_blocks_noisy_messages(msg: str) -> None: def test_filter_platformio_lines_allows_other_messages(msg: str) -> None: """Test that non-noisy platformio output lines pass through RedirectText.""" assert _filter_through_redirect(msg) == msg + "\n" + + +# --------------------------------------------------------------------------- +# PlatformIO python-version cache heal +# --------------------------------------------------------------------------- + +_CURRENT_MINOR = f"{sys.version_info.major}.{sys.version_info.minor}" +# Captured before the autouse guard patches the name, so tests can exercise the +# real implementation. +_REAL_GET_PLATFORMIO_CONFIG = toolchain.get_platformio_config + + +@pytest.fixture(autouse=True) +def _guard_real_platformio() -> Generator[None, None, None]: + """Default the PlatformIO config lookup to None so no test in this module + touches a real ~/.platformio; the heal tests re-patch it at a temp dir.""" + with patch.object(toolchain, "get_platformio_config", return_value=None): + yield + + +def _pio_layout(core_dir: Path) -> dict[str, Path]: + """Return the PlatformIO dir layout with cache/packages/platforms under core.""" + return { + "core_dir": core_dir, + "packages_dir": core_dir / "packages", + "platforms_dir": core_dir / "platforms", + "cache_dir": core_dir / ".cache", + } + + +def _split_pio_layout(tmp_path: Path) -> dict[str, Path]: + """Container-shape layout: caches on a persistent root, core_dir ephemeral.""" + persistent = tmp_path / "data" / "platformio" + return { + "core_dir": tmp_path / "root" / ".platformio", + "platforms_dir": persistent / "platforms", + "packages_dir": persistent / "packages", + "cache_dir": persistent / "cache", + } + + +def _seed_layout(layout: dict[str, Path]) -> None: + """Populate each cache dir (and the core penv) with a marker file.""" + for key in ("platforms_dir", "packages_dir", "cache_dir"): + layout[key].mkdir(parents=True, exist_ok=True) + (layout[key] / "marker").write_text("x", encoding="utf-8") + penv = layout["core_dir"] / "penv" + penv.mkdir(parents=True, exist_ok=True) + (penv / "marker").write_text("x", encoding="utf-8") + + +def _make_pio_config(layout: dict[str, Path] | Path) -> MagicMock: + """A ProjectConfig stand-in resolving platformio dir options from *layout*.""" + resolved = _pio_layout(layout) if isinstance(layout, Path) else layout + config = MagicMock() + config.get.side_effect = lambda section, option: ( + str(resolved[option]) if section == "platformio" else "" + ) + return config + + +@contextmanager +def _use_pio_config(layout: dict[str, Path] | Path) -> Generator[MagicMock, None, None]: + """Point ``get_platformio_config`` at a temp layout for the block.""" + config = _make_pio_config(layout) + with patch.object(toolchain, "get_platformio_config", return_value=config): + yield config + + +def _stamp_version(core_dir: Path) -> str | None: + """Read the python version recorded in the heal stamp under *core_dir*.""" + return toolchain._read_pio_stamp_python(core_dir / toolchain._PIO_PYTHON_STAMP_FILE) + + +def _cache_wiped(core_dir: Path) -> bool: + """True when the seeded cache subdir markers are gone.""" + return not any( + (core_dir / sub / "marker").exists() + for sub in ("packages", "platforms", ".cache") + ) + + +@pytest.fixture +def pio_core_dir(tmp_path: Path) -> Path: + """A populated PlatformIO core dir (packages/platforms/.cache/penv seeded).""" + core = tmp_path / "dot-platformio" + for sub in ("packages", "platforms", ".cache", "penv"): + seeded = core / sub + seeded.mkdir(parents=True) + (seeded / "marker").write_text("x", encoding="utf-8") + return core + + +def test_current_python_minor_matches_running_interpreter() -> None: + """_current_python_minor returns major.minor of the running interpreter.""" + assert toolchain._current_python_minor() == _CURRENT_MINOR + + +def test_pio_stamp_round_trip(tmp_path: Path) -> None: + """The stamp writer/reader round-trips and records the schema version.""" + stamp = tmp_path / toolchain._PIO_PYTHON_STAMP_FILE + toolchain._write_pio_stamp_python(stamp, "3.13") + assert toolchain._read_pio_stamp_python(stamp) == "3.13" + assert json.loads(stamp.read_text()) == { + "schema_version": toolchain._PIO_PYTHON_STAMP_SCHEMA, + "python_version": "3.13", + } + + +def test_read_pio_stamp_missing(tmp_path: Path) -> None: + """A missing stamp file yields None.""" + assert toolchain._read_pio_stamp_python(tmp_path / "nope.json") is None + + +def test_read_pio_stamp_malformed(tmp_path: Path) -> None: + """A corrupt stamp file yields None instead of raising.""" + stamp = tmp_path / "bad.json" + stamp.write_text("{not json", encoding="utf-8") + assert toolchain._read_pio_stamp_python(stamp) is None + + +def test_read_pio_stamp_unreadable_logs_warning( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A present-but-unreadable stamp yields None and warns.""" + stamp = tmp_path / "stamp.json" + stamp.mkdir() + with caplog.at_level("WARNING"): + assert toolchain._read_pio_stamp_python(stamp) is None + assert "Could not read" in caplog.text + + +def test_read_pio_stamp_without_python_version(tmp_path: Path) -> None: + """A stamp missing python_version yields None.""" + stamp = tmp_path / "s.json" + stamp.write_text(json.dumps({"schema_version": "0"}), encoding="utf-8") + assert toolchain._read_pio_stamp_python(stamp) is None + + +@pytest.mark.parametrize("payload", ["42", '"x"', "[1, 2]", "null"]) +def test_read_pio_stamp_non_object_json(tmp_path: Path, payload: str) -> None: + """Valid-but-non-object JSON in the stamp yields None, not a crash.""" + stamp = tmp_path / "s.json" + stamp.write_text(payload, encoding="utf-8") + assert toolchain._read_pio_stamp_python(stamp) is None + + +def test_clean_platformio_cache_none_config_is_noop() -> None: + """clean_platformio_cache is a no-op when PlatformIO is unavailable.""" + with patch.object(toolchain, "get_platformio_config", return_value=None): + toolchain.clean_platformio_cache() + + +def test_clean_platformio_cache_wipes_everything(pio_core_dir: Path) -> None: + """clean_platformio_cache removes cache/packages/platforms and core_dir.""" + with _use_pio_config(pio_core_dir): + toolchain.clean_platformio_cache() + assert not pio_core_dir.exists() + + +def test_heal_none_config_is_noop() -> None: + """Heal is a no-op (no error) when PlatformIO is unavailable.""" + with patch.object(toolchain, "get_platformio_config", return_value=None): + toolchain.heal_platformio_python_env() + + +def test_heal_fresh_cache_stamps_without_wipe(tmp_path: Path) -> None: + """A fresh core dir (no stamp, no penv) is stamped, not wiped.""" + core = tmp_path / "pio" + with _use_pio_config(core): + toolchain.heal_platformio_python_env() + assert _stamp_version(core) == _CURRENT_MINOR + + +def test_heal_stamp_matches_current_no_wipe(pio_core_dir: Path) -> None: + """A stamp matching the running interpreter leaves the cache untouched.""" + toolchain._write_pio_stamp_python( + pio_core_dir / toolchain._PIO_PYTHON_STAMP_FILE, _CURRENT_MINOR + ) + with _use_pio_config(pio_core_dir): + toolchain.heal_platformio_python_env() + assert not _cache_wiped(pio_core_dir) + assert (pio_core_dir / "penv" / "marker").exists() + + +def test_heal_stale_stamp_wipes_and_restamps(pio_core_dir: Path) -> None: + """A stamp from an older interpreter triggers a wipe + restamp; core_dir stays.""" + toolchain._write_pio_stamp_python( + pio_core_dir / toolchain._PIO_PYTHON_STAMP_FILE, "2.7" + ) + with _use_pio_config(pio_core_dir): + toolchain.heal_platformio_python_env() + assert _cache_wiped(pio_core_dir) + assert not (pio_core_dir / "penv").exists() + assert pio_core_dir.is_dir() + assert _stamp_version(pio_core_dir) == _CURRENT_MINOR + + +def test_heal_no_stamp_existing_cache_wipes_once( + pio_core_dir: Path, caplog: pytest.LogCaptureFixture +) -> None: + """An existing cache with no stamp is cleaned once and stamped.""" + with _use_pio_config(pio_core_dir), caplog.at_level("INFO"): + toolchain.heal_platformio_python_env() + assert _cache_wiped(pio_core_dir) + assert not (pio_core_dir / "penv").exists() + assert _stamp_version(pio_core_dir) == _CURRENT_MINOR + assert "once" in caplog.text + + +def test_heal_no_stamp_penv_only_counts_as_cache(tmp_path: Path) -> None: + """A core dir holding only a penv still triggers the one-time clean.""" + core = tmp_path / "pio" + penv = core / "penv" + penv.mkdir(parents=True) + (penv / "marker").write_text("x", encoding="utf-8") + with _use_pio_config(core): + toolchain.heal_platformio_python_env() + assert not penv.exists() + assert _stamp_version(core) == _CURRENT_MINOR + + +def test_heal_oserror_is_nonfatal( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A filesystem failure during the check warns instead of aborting the build.""" + blocker = tmp_path / "pio" + blocker.write_text("not a directory", encoding="utf-8") + with _use_pio_config(blocker), caplog.at_level("WARNING"): + toolchain.heal_platformio_python_env() + assert "build environment check failed" in caplog.text + + +def test_heal_stamp_write_failure_is_nonfatal( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A failed stamp write (EsphomeError from write_file) warns, not aborts.""" + with ( + _use_pio_config(tmp_path / "pio"), + patch.object( + toolchain, + "_write_pio_stamp_python", + side_effect=EsphomeError("disk full"), + ), + caplog.at_level("WARNING"), + ): + toolchain.heal_platformio_python_env() + assert "build environment check failed" in caplog.text + + +def test_heal_is_idempotent_across_runs(pio_core_dir: Path) -> None: + """After a heal writes the stamp, a re-provisioned cache is not wiped again.""" + with _use_pio_config(pio_core_dir): + toolchain.heal_platformio_python_env() + repop = pio_core_dir / "packages" + repop.mkdir(exist_ok=True) + (repop / "marker").write_text("x", encoding="utf-8") + toolchain.heal_platformio_python_env() + assert (pio_core_dir / "packages" / "marker").exists() + + +def test_pio_stamp_dir_is_platforms_parent(tmp_path: Path) -> None: + """The stamp home is the parent of platforms_dir, not core_dir.""" + layout = _split_pio_layout(tmp_path) + config = _make_pio_config(layout) + assert toolchain._pio_stamp_dir(config) == layout["platforms_dir"].parent + nested = _make_pio_config(tmp_path / "pio") + assert toolchain._pio_stamp_dir(nested) == tmp_path / "pio" + + +def test_heal_container_layout_stamps_persistent_root(tmp_path: Path) -> None: + """Container shape: the stamp lands on the persistent cache root.""" + layout = _split_pio_layout(tmp_path) + with _use_pio_config(layout): + toolchain.heal_platformio_python_env() + persistent = layout["platforms_dir"].parent + assert _stamp_version(persistent) == _CURRENT_MINOR + assert not (layout["core_dir"] / toolchain._PIO_PYTHON_STAMP_FILE).exists() + + +def test_heal_container_layout_stale_stamp_wipes_persistent_cache( + tmp_path: Path, +) -> None: + """Container shape: a stale stamp wipes the relocated persistent caches.""" + layout = _split_pio_layout(tmp_path) + _seed_layout(layout) + persistent = layout["platforms_dir"].parent + toolchain._write_pio_stamp_python( + persistent / toolchain._PIO_PYTHON_STAMP_FILE, "2.7" + ) + with _use_pio_config(layout): + toolchain.heal_platformio_python_env() + for key in ("platforms_dir", "packages_dir", "cache_dir"): + assert not layout[key].exists() + assert not (layout["core_dir"] / "penv").exists() + assert _stamp_version(persistent) == _CURRENT_MINOR + + +def test_heal_container_layout_survives_core_dir_wipe(tmp_path: Path) -> None: + """A python change is still detected after an image update wiped core_dir.""" + layout = _split_pio_layout(tmp_path) + _seed_layout(layout) + shutil.rmtree(layout["core_dir"]) + persistent = layout["platforms_dir"].parent + toolchain._write_pio_stamp_python( + persistent / toolchain._PIO_PYTHON_STAMP_FILE, "2.7" + ) + with _use_pio_config(layout): + toolchain.heal_platformio_python_env() + for key in ("platforms_dir", "packages_dir", "cache_dir"): + assert not layout[key].exists() + assert _stamp_version(persistent) == _CURRENT_MINOR + + +def test_get_platformio_config_returns_project_config() -> None: + """The real lookup returns a usable ProjectConfig when PlatformIO is present.""" + config = _REAL_GET_PLATFORMIO_CONFIG() + assert config is not None + assert hasattr(config, "get") + + +def test_get_platformio_config_none_when_platformio_absent() -> None: + """The lookup returns None when PlatformIO cannot be imported.""" + with patch.dict(sys.modules, {"platformio.project.config": None}): + assert _REAL_GET_PLATFORMIO_CONFIG() is None + + +def test_delete_platformio_dirs_skips_missing(tmp_path: Path) -> None: + """A named dir that does not exist is skipped without error.""" + (tmp_path / "packages").mkdir() + (tmp_path / "packages" / "marker").write_text("x", encoding="utf-8") + config = _make_pio_config(tmp_path) + # platforms_dir does not exist; packages_dir does. + toolchain._delete_platformio_dirs(config, ["packages_dir", "platforms_dir"]) + assert not (tmp_path / "packages").exists() + + +def test_heal_stale_stamp_wipes_when_penv_absent(pio_core_dir: Path) -> None: + """The penv wipe is skipped cleanly when no penv exists.""" + shutil.rmtree(pio_core_dir / "penv") + toolchain._write_pio_stamp_python( + pio_core_dir / toolchain._PIO_PYTHON_STAMP_FILE, "2.7" + ) + with _use_pio_config(pio_core_dir): + toolchain.heal_platformio_python_env() + assert _cache_wiped(pio_core_dir) + assert _stamp_version(pio_core_dir) == _CURRENT_MINOR + + +def test_run_platformio_cli_invokes_heal( + setup_core: Path, mock_run_external_process: Mock +) -> None: + """run_platformio_cli runs the heal before spawning PlatformIO.""" + CORE.build_path = str(setup_core / "build" / "test") + mock_run_external_process.return_value = 0 + with patch.object(toolchain, "heal_platformio_python_env") as mock_heal: + toolchain.run_platformio_cli("test") + mock_heal.assert_called_once() From 5a86e26f680b2da5581bf4e003372abc68bc3621 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 19 Jul 2026 19:34:56 -1000 Subject: [PATCH 30/40] [platformio] Re-download library when cached copy is missing its manifest (#17691) Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com> --- esphome/platformio/library.py | 20 ++++- tests/unit_tests/test_platformio_library.py | 84 ++++++++++++++++++--- 2 files changed, 90 insertions(+), 14 deletions(-) diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 72a50b795b..b3fd24c2b7 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -669,9 +669,25 @@ def convert_libraries( library_json_path = component.path / "library.json" library_properties_path = component.path / "library.properties" - if library_json_path.is_file(): + has_json = library_json_path.is_file() + has_properties = library_properties_path.is_file() + if not has_json and not has_properties: + # The shared cache can hold a broken copy (e.g. a clone or an + # extraction interrupted by a killed process). Force one + # re-download so a bad cache entry self-heals instead of failing + # every build until the user runs a full clean. + _LOGGER.warning( + "Library %s at %s is missing library.json and library.properties; " + "re-downloading", + key, + component.path, + ) + component.download(force=True, salt=salt, namespace=backend.cache_key) + has_json = library_json_path.is_file() + has_properties = library_properties_path.is_file() + if has_json: component.data = _parse_library_json(library_json_path) - elif library_properties_path.is_file(): + elif has_properties: component.data = _parse_library_properties(library_properties_path) else: raise RuntimeError( diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index 6a4c057469..d2ca71bad6 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -133,18 +133,8 @@ def test_resolve_registry_version_raises_without_pkg_file(monkeypatch): _resolve_registry_version("owner", "pkg", set()) -def _patch_download_with_manifests(monkeypatch, tmp_path, manifests, *, properties=()): - """Fake ConvertedLibrary.download to materialize canned manifests on disk.""" - - def fake_download(self, force=False, salt="", namespace=""): - self.path = tmp_path / self.get_sanitized_name().replace("/", "__") - self.path.mkdir(parents=True, exist_ok=True) - if self.name in properties: - (self.path / "library.properties").write_text(manifests[self.name]) - else: - (self.path / "library.json").write_text(json.dumps(manifests[self.name])) - - monkeypatch.setattr(ConvertedLibrary, "download", fake_download) +def _patch_registry_resolve(monkeypatch: pytest.MonkeyPatch) -> None: + """Stub the registry lookup so tests never touch the network.""" monkeypatch.setattr( lib, "_resolve_registry_version", @@ -157,6 +147,21 @@ def _patch_download_with_manifests(monkeypatch, tmp_path, manifests, *, properti ) +def _patch_download_with_manifests(monkeypatch, tmp_path, manifests, *, properties=()): + """Fake ConvertedLibrary.download to materialize canned manifests on disk.""" + + def fake_download(self, force=False, salt="", namespace=""): + self.path = tmp_path / self.get_require_name() + self.path.mkdir(parents=True, exist_ok=True) + if self.name in properties: + (self.path / "library.properties").write_text(manifests[self.name]) + else: + (self.path / "library.json").write_text(json.dumps(manifests[self.name])) + + monkeypatch.setattr(ConvertedLibrary, "download", fake_download) + _patch_registry_resolve(monkeypatch) + + def test_convert_libraries_parses_library_properties(tmp_path, monkeypatch): # A manifest provided as library.properties (Arduino style) instead of # library.json must still be parsed and converted. @@ -212,6 +217,61 @@ def test_convert_libraries_handles_unparsable_dependency_version(tmp_path, monke assert [d.name for d in top[0].dependencies] == ["C"] +def _patch_download_without_manifest( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, *, manifest_on_force: bool +) -> list[bool]: + """Fake ConvertedLibrary.download that leaves the manifest missing. + + When ``manifest_on_force`` is set, a forced re-download writes a valid + library.json, simulating a broken cache entry that heals on retry. + Returns the list of ``force`` values download was called with. + """ + calls: list[bool] = [] + + def fake_download( + self: ConvertedLibrary, force: bool = False, salt: str = "", namespace: str = "" + ) -> None: + calls.append(force) + self.path = tmp_path / self.get_require_name() + self.path.mkdir(parents=True, exist_ok=True) + if force and manifest_on_force: + (self.path / "library.json").write_text(json.dumps({"name": "A"})) + + monkeypatch.setattr(ConvertedLibrary, "download", fake_download) + _patch_registry_resolve(monkeypatch) + return calls + + +def test_convert_libraries_redownloads_when_manifest_missing( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # A cached copy without any manifest (e.g. an interrupted clone or + # extraction) triggers exactly one forced re-download and then succeeds. + calls = _patch_download_without_manifest( + monkeypatch, tmp_path, manifest_on_force=True + ) + + top = convert_libraries([Library("esphome/A", "1.0.0", None)], _backend()) + + assert calls == [False, True] + assert top[0].data["name"] == "A" + + +def test_convert_libraries_raises_when_manifest_missing_after_retry( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # If the forced re-download still yields no manifest, the error is raised + # after exactly one retry (no retry loop). + calls = _patch_download_without_manifest( + monkeypatch, tmp_path, manifest_on_force=False + ) + + with pytest.raises(RuntimeError, match="Invalid PIO library"): + convert_libraries([Library("esphome/A", "1.0.0", None)], _backend()) + + assert calls == [False, True] + + @pytest.mark.parametrize( ("value", "expected"), [ From 7738464f0bef5f278af606d673ec8a99978cd974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Seux?= Date: Mon, 20 Jul 2026 19:07:28 +0200 Subject: [PATCH 31/40] [http_request] Fix usage of http response body (#17713) Co-authored-by: J. Nick Koston --- esphome/components/http_request/http_request.h | 4 ++-- .../components/http_request/http_request.yaml | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/esphome/components/http_request/http_request.h b/esphome/components/http_request/http_request.h index df1bb462ab..4471dffdc2 100644 --- a/esphome/components/http_request/http_request.h +++ b/esphome/components/http_request/http_request.h @@ -488,10 +488,10 @@ template class HttpRequestSendAction final : public Actionbody_.value(x...); } if (!this->json_.empty()) { - body = json::build_json([this, x...](JsonObject root) { this->encode_json_(x..., root); }); + body = json::build_json([this, x...](JsonObject root) mutable { this->encode_json_(x..., root); }); } if (this->json_func_ != nullptr) { - body = json::build_json([this, x...](JsonObject root) { this->json_func_(x..., root); }); + body = json::build_json([this, x...](JsonObject root) mutable { this->json_func_(x..., root); }); } std::vector
request_headers; request_headers.reserve(this->request_headers_.size()); diff --git a/tests/components/http_request/http_request.yaml b/tests/components/http_request/http_request.yaml index 46d4b88ec5..4b3c2ca36b 100644 --- a/tests/components/http_request/http_request.yaml +++ b/tests/components/http_request/http_request.yaml @@ -59,6 +59,24 @@ esphome: id: test_regression_light brightness: 100% effect: "None" + - http_request.get: + url: https://esphome.io + capture_response: true + on_response: + then: + # Regression test: http_request.post with json: (dict variant) inside + # on_response of a capture_response: true request puts std::string& + # (body) into the nested action's Ts..., which exposes a + # const-correctness bug in HttpRequestSendAction::play() where + # encode_json_ receives const copies of non-const reference args. + - http_request.post: + url: https://esphome.io + json: + status: "ok" + # Same with json: lambda variant, exercises json_func_ path + - http_request.post: + url: https://esphome.io + json: !lambda "root[\"status\"] = \"ok\";" http_request: useragent: esphome/tagreader From 5f2adcf9b3020a51788909779423b9d882707138 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 20 Jul 2026 10:08:46 -1000 Subject: [PATCH 32/40] [platformio] Include cache path in invalid library error (#17692) --- esphome/platformio/library.py | 2 +- tests/unit_tests/test_platformio_library.py | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index b3fd24c2b7..7c8566b77a 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -692,7 +692,7 @@ def convert_libraries( else: raise RuntimeError( f"Invalid PIO library {key}: missing library.json and " - "library.properties" + f"library.properties in {component.path}" ) try: diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index d2ca71bad6..c0a0c678db 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -261,15 +261,18 @@ def test_convert_libraries_raises_when_manifest_missing_after_retry( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: # If the forced re-download still yields no manifest, the error is raised - # after exactly one retry (no retry loop). + # after exactly one retry (no retry loop). The error must name the cache + # directory so users can find the broken entry instead of guessing where + # the library was unpacked. calls = _patch_download_without_manifest( monkeypatch, tmp_path, manifest_on_force=False ) - with pytest.raises(RuntimeError, match="Invalid PIO library"): + with pytest.raises(RuntimeError, match="Invalid PIO library") as excinfo: convert_libraries([Library("esphome/A", "1.0.0", None)], _backend()) assert calls == [False, True] + assert str(tmp_path / "esphome__A") in str(excinfo.value) @pytest.mark.parametrize( From 307faa6c389bd8265b82305bc7270783c60387b1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 20 Jul 2026 10:12:17 -1000 Subject: [PATCH 33/40] [espidf] Resume interrupted toolchain downloads instead of restarting (#17706) --- esphome/espidf/framework.py | 213 ++++-- esphome/espidf/get_tool_downloads.py | 88 +++ esphome/framework_helpers.py | 547 ++++++++++--- .../fixtures/idf_tools_stub/idf_tools.py | 120 +++ tests/unit_tests/test_espidf_framework.py | 317 +++++++- tests/unit_tests/test_framework_helpers.py | 718 +++++++++++++++++- 6 files changed, 1844 insertions(+), 159 deletions(-) create mode 100644 esphome/espidf/get_tool_downloads.py create mode 100644 tests/unit_tests/fixtures/idf_tools_stub/idf_tools.py diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index fce6a88ccf..b54a0c294b 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -9,7 +9,7 @@ from pathlib import Path import platform import re import shutil -import tempfile +from typing import NoReturn import platformdirs @@ -20,6 +20,7 @@ from esphome.framework_helpers import ( archive_extract_all, create_venv, download_from_mirrors, + download_with_resume, get_python_env_executable_path, get_system_python_path, rmdir, @@ -231,6 +232,40 @@ def _write_stamp(file: PathType, data: dict[str, str]): json.dump(data, fp) +def _run_idf_tools_script( + idf_framework_root: PathType, + script_name: str, + msg: str, + args: list[str] | None = None, + env: dict[str, str] | None = None, +) -> tuple[bool, str | None, str | None]: + """Run one of the sibling idf_tools-backed helper scripts. + + The script is executed with the framework's ``tools`` directory on + PYTHONPATH so it imports the framework's own ``idf_tools`` module. + """ + cmd = [ + get_system_python_path(), + str(_SCRIPTS_DIR / script_name), + str(idf_framework_root), + *(args or []), + ] + return run_command( + cmd, + msg=msg, + env=(env or os.environ) + | {"PYTHONPATH": str(Path(idf_framework_root) / "tools")}, + ) + + +def _raise_script_failure(what: str, root: PathType, stderr: str | None) -> NoReturn: + """Raise RuntimeError for a failed helper script, appending stderr detail.""" + detail = (stderr or "").strip() + raise RuntimeError( + f"Can't get {what} of {root}" + (f": {detail}" if detail else "") + ) + + def _get_idf_version( idf_framework_root: PathType, env: dict[str, str] | None = None ) -> str: @@ -248,26 +283,13 @@ def _get_idf_version( RuntimeError: If ESP-IDF version cannot be determined """ - cmd = [ - get_system_python_path(), - str(_SCRIPTS_DIR / "get_idf_version.py"), - str(idf_framework_root), - ] - - success, stdout, stderr = run_command( - cmd, - msg="ESP-IDF version", - env=(env or os.environ) - | {"PYTHONPATH": str(Path(idf_framework_root) / "tools")}, + success, stdout, stderr = _run_idf_tools_script( + idf_framework_root, "get_idf_version.py", "ESP-IDF version", env=env ) if stdout: stdout = stdout.strip() if not success or not stdout: - detail = (stderr or "").strip() - raise RuntimeError( - f"Can't get ESP-IDF version of {idf_framework_root}" - + (f": {detail}" if detail else "") - ) + _raise_script_failure("ESP-IDF version", idf_framework_root, stderr) return stdout @@ -288,24 +310,11 @@ def _get_idf_tool_paths( RuntimeError: If ESP-IDF tool paths cannot be determined """ - cmd = [ - get_system_python_path(), - str(_SCRIPTS_DIR / "get_idf_tool_paths.py"), - str(idf_framework_root), - ] - - success, stdout, stderr = run_command( - cmd, - msg="ESP-IDF tool paths", - env=(env or os.environ) - | {"PYTHONPATH": str(Path(idf_framework_root) / "tools")}, + success, stdout, stderr = _run_idf_tools_script( + idf_framework_root, "get_idf_tool_paths.py", "ESP-IDF tool paths", env=env ) if not success or not stdout: - detail = (stderr or "").strip() - raise RuntimeError( - f"Can't get ESP-IDF tool paths of {idf_framework_root}" - + (f": {detail}" if detail else "") - ) + _raise_script_failure("ESP-IDF tool paths", idf_framework_root, stderr) # Extract json values try: @@ -579,6 +588,69 @@ def _patch_tools_json_demote_openocd(framework_path: Path) -> None: ) +def _prefetch_idf_tool_archives( + framework_path: Path, + targets_str: str, + tools: list[str], + env: dict[str, str] | None, +) -> None: + """Pre-download the tool archives ``idf_tools.py install`` would fetch. + + ``idf_tools.py``'s own downloader restarts from byte zero on every retry, + which makes large archives effectively impossible to fetch on unstable + connections (#17703). This asks the framework's idf_tools (via + ``get_tool_downloads.py``) which archives the coming install needs, then + downloads each into ``/dist`` with + ``download_with_resume``. The installer then finds the verified archives + already in place ("file ... is already downloaded") and never touches the + network. + + Strictly best-effort: any failure here just logs and returns, leaving + ``idf_tools.py install`` to download whatever is missing exactly as + before. Leftover ``.part`` files live in ``dist/`` and are removed by the + post-install cache prune. + """ + try: + success, stdout, stderr = _run_idf_tools_script( + framework_path, + "get_tool_downloads.py", + "ESP-IDF tool download list", + args=[targets_str, *tools], + env=env, + ) + if not success or not stdout: + _LOGGER.warning( + "Could not determine ESP-IDF tool downloads: %s", + (stderr or "").strip(), + ) + return + dist_path = get_idf_tools_path() / "dist" + entries = [ + entry + for entry in json.loads(stdout) + if not (dist_path / entry["dest"]).is_file() + ] + for index, entry in enumerate(entries, start=1): + _LOGGER.info( + "Downloading %s (%d/%d) ...", entry["name"], index, len(entries) + ) + try: + download_with_resume( + entry["url"], + dist_path / entry["dest"], + sha256=entry["sha256"], + size=entry["size"], + ) + except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught + # Keep prefetching the remaining archives; the installer + # will retry this one itself (without resume). + _LOGGER.warning("Could not prefetch %s: %s", entry["name"], e) + except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught + # The installer downloads anything missing itself; never let the + # prefetch become a new way for the install to fail. + _LOGGER.warning("ESP-IDF tool prefetch failed: %s", e) + + def _check_esphome_idf_framework_install( version: str, targets: list[str], @@ -650,41 +722,51 @@ def _check_esphome_idf_framework_install( git_url, ref = git_source _clone_idf_with_submodules(framework_path, git_url, ref) else: - # Download in temporary file - with tempfile.NamedTemporaryFile() as tmp: - _LOGGER.info("Downloading ESP-IDF %s framework ...", version) + _LOGGER.info("Downloading ESP-IDF %s framework ...", version) - # Create substitutions for the URLs. SHORT_VERSION (x.y with - # optional -extra) is only provided for x.y.0 releases, since - # the vX.Y release tags only exist for those; templates that - # reference it are skipped for other versions by - # download_from_mirrors. - substitutions = {"VERSION": version} - try: - ver = Version.parse(version) - substitutions["MAJOR"] = str(ver.major) - substitutions["MINOR"] = str(ver.minor) - substitutions["PATCH"] = str(ver.patch) - substitutions["EXTRA"] = f"-{ver.extra}" if ver.extra else "" - if ver.patch == 0: - substitutions["SHORT_VERSION"] = ( - f"{ver.major}.{ver.minor}{substitutions['EXTRA']}" - ) - except ValueError: - _LOGGER.warning( - "ESP-IDF version '%s' is not a valid version number; " - "only the {VERSION} substitution is available for " - "mirror URLs", - version, + # Create substitutions for the URLs. SHORT_VERSION (x.y with + # optional -extra) is only provided for x.y.0 releases, since + # the vX.Y release tags only exist for those; templates that + # reference it are skipped for other versions by + # download_from_mirrors. + substitutions = {"VERSION": version} + try: + ver = Version.parse(version) + substitutions["MAJOR"] = str(ver.major) + substitutions["MINOR"] = str(ver.minor) + substitutions["PATCH"] = str(ver.patch) + substitutions["EXTRA"] = f"-{ver.extra}" if ver.extra else "" + if ver.patch == 0: + substitutions["SHORT_VERSION"] = ( + f"{ver.major}.{ver.minor}{substitutions['EXTRA']}" ) - - mirrors = [source_url] if source_url else ESPHOME_IDF_FRAMEWORK_MIRRORS - download_from_mirrors(mirrors, substitutions, tmp.file) - - _LOGGER.info("Extracting ESP-IDF %s framework ...", version) - archive_extract_all( - tmp.file, framework_path, progress_header="Extracting" + except ValueError: + _LOGGER.warning( + "ESP-IDF version '%s' is not a valid version number; " + "only the {VERSION} substitution is available for " + "mirror URLs", + version, ) + + mirrors = [source_url] if source_url else ESPHOME_IDF_FRAMEWORK_MIRRORS + # Download to a persistent file in the tool download cache (not + # a temp file) so an interrupted download resumes on the next + # run; the cache is pruned after a successful install anyway. + tarball_path = get_idf_tools_path() / "dist" / f"esp-idf-{version}.tar.xz" + download_from_mirrors(mirrors, substitutions, tarball_path) + + _LOGGER.info("Extracting ESP-IDF %s framework ...", version) + try: + with tarball_path.open("rb") as tarball: + archive_extract_all( + tarball, framework_path, progress_header="Extracting" + ) + finally: + # Success: drop the archive rather than caching ~70MB twice. + # Failure: a corrupt archive (e.g. torn by an unclean + # shutdown) must not be reused — without a checksum only a + # failed extraction can expose it, so force a re-download. + tarball_path.unlink(missing_ok=True) extracted_marker.touch() # Idempotent post-extract patch: written every invocation so a build @@ -722,6 +804,7 @@ def _check_esphome_idf_framework_install( if install: _LOGGER.info("Installing ESP-IDF %s framework ...", version) targets_str = ",".join(targets) + _prefetch_idf_tool_archives(framework_path, targets_str, tools, env) cmd = [ get_system_python_path(), str(idf_tools_path), diff --git a/esphome/espidf/get_tool_downloads.py b/esphome/espidf/get_tool_downloads.py new file mode 100644 index 0000000000..37a6126fae --- /dev/null +++ b/esphome/espidf/get_tool_downloads.py @@ -0,0 +1,88 @@ +"""Print JSON download info for the ESP-IDF tools an install would fetch. + +Run via ``python ...``. +PYTHONPATH must include ``/tools`` so ``idf_tools`` is +importable, and IDF_TOOLS_PATH must be set. Prints a JSON list of +``{name, url, size, sha256, dest}`` for every tool version that is not yet +installed, where ``dest`` is the archive filename ``idf_tools.py install`` +expects to find in ``/dist``. Tools with no download for the +current platform are skipped; already-installed versions are skipped so a +pruned download cache is not re-fetched. + +The target/tool expansion mirrors ``idf_tools.py install`` (targets passed to +``add_and_check_targets`` accumulate with idf-env.json) but nothing is saved +or written — this script only reports what the install would download. +""" + +# pylint: disable=import-error # idf_tools is on PYTHONPATH at runtime only + +from contextlib import redirect_stdout +import json +import os +from pathlib import Path +import sys + +from idf_tools import ( + CURRENT_PLATFORM, + TOOLS_FILE, + IDFEnv, + ToolBinaryError, + add_and_check_targets, + expand_tools_arg, + g, + get_idf_download_url_apply_mirrors, + load_tools_info, +) + + +def collect_downloads() -> list[dict]: + g.idf_path = sys.argv[1] + g.idf_tools_path = os.environ.get("IDF_TOOLS_PATH") + g.tools_json = str(Path(g.idf_path) / TOOLS_FILE) + + targets = add_and_check_targets(IDFEnv.get_idf_env(), sys.argv[2]) + tools_info = load_tools_info() + downloads: list[dict] = [] + + for name in expand_tools_arg(sys.argv[3:], tools_info, targets): + if "@" in name: + name, version = name.split("@", 1) + else: + version = None + tool = tools_info.get(name) + if tool is None or not tool.compatible_with_platform(): + continue + version = version or tool.get_recommended_version() + if version is None: + continue + try: + tool.find_installed_versions() + except ToolBinaryError as e: + # A broken installed binary is idf_tools' problem to repair on + # install; note it and treat the version as not installed. + print(f"tool {name} failed its binary check: {e}", file=sys.stderr) + if version in tool.versions_installed or version not in tool.versions: + continue + download = tool.versions[version].get_download_for_platform(CURRENT_PLATFORM) + if download is None: + continue + downloads.append( + { + "name": f"{name}@{version}", + # Apply the same IDF_MIRROR_PREFIX_MAP / IDF_GITHUB_ASSETS + # rewriting the installer's own downloader applies, so users + # behind a mirror prefetch from the mirror too. + "url": get_idf_download_url_apply_mirrors(None, download.url), + "size": download.size, + "sha256": download.sha256, + "dest": download.rename_dist or Path(download.url).name, + } + ) + return downloads + + +# idf_tools prints informational lines (e.g. mirror URL rewrites) to stdout; +# route them to stderr so stdout carries only the JSON result. +with redirect_stdout(sys.stderr): + result = collect_downloads() +print(json.dumps(result)) diff --git a/esphome/framework_helpers.py b/esphome/framework_helpers.py index 6c055dded3..202d4a2bfb 100644 --- a/esphome/framework_helpers.py +++ b/esphome/framework_helpers.py @@ -2,21 +2,31 @@ from collections.abc import Iterable from contextlib import ExitStack +import hashlib import io +import json import logging import os from pathlib import Path import subprocess import sys import time -from typing import IO +from typing import IO, TYPE_CHECKING from esphome.helpers import ProgressBar, rmtree +if TYPE_CHECKING: + import requests + PathType = str | os.PathLike _LOGGER = logging.getLogger(__name__) +# Attempts per mirror URL before falling through to the next mirror; only +# mid-stream drops retry (resuming when the server gave a validator), +# connect errors move on immediately. +_MIRROR_ATTEMPTS = 3 + def get_project_link_flags() -> list[str]: """Return the sorted -Wl, linker flags from the current build.""" @@ -394,17 +404,23 @@ def _zip_extract_all( progress.update(1) -def _rename_with_retry(src: Path, dst: Path, attempts: int = 5) -> None: +def _rename_with_retry( + src: Path, dst: Path, attempts: int = 5, overwrite: bool = False +) -> None: """Rename ``src`` to ``dst`` with backoff retries on Windows sharing violations. Antivirus/indexer handles on freshly-written files can briefly block ``os.rename`` with ERROR_SHARING_VIOLATION / ERROR_ACCESS_DENIED. The handle is released within tens of ms in practice, so exponential backoff - works. + works. With ``overwrite`` an existing ``dst`` is replaced instead of + failing. """ for i in range(attempts): try: - src.rename(dst) + if overwrite: + src.replace(dst) + else: + src.rename(dst) return except PermissionError: if i == attempts - 1: @@ -525,8 +541,8 @@ def archive_extract_all( ValueError: If archive format is unsupported """ - # 1. Handle different archive input types with ExitStack() as stack: + # 1. Handle different archive input types archive_ref: io.BufferedIOBase if isinstance(archive, (str, os.PathLike)): archive_ref = stack.enter_context(Path(archive).open("rb")) @@ -552,6 +568,311 @@ def archive_extract_all( matched_fct(archive_ref, extract_dir, progress_header=progress_header) +def _open_ranged( + url: str, offset: int, timeout: int, validator: str | None = None +) -> tuple["requests.Response | None", int]: + """Open a streaming GET, asking the server to resume at ``offset``. + + ``validator`` is an ETag or Last-Modified value from the interrupted + response; it is sent as ``If-Range`` so the server only honors the Range + when the content is unchanged, replying 200 (full body, restart) if the + file was replaced between requests — the resumed bytes can then never be + stitched onto a different file's prefix. + + Returns ``(response, effective_offset)``. The response is None when the + server answered 416 Range Not Satisfiable: the file holds every byte the + server has (a previous attempt was interrupted after the last byte), so + there is nothing to stream and the caller's verification decides whether + the file is good. The offset drops to 0 when the server ignored the + ``Range`` header (no 206), meaning the caller must restart the file. + Raises on connect errors and HTTP error statuses; the response is closed + on failure. + """ + import requests + + headers = {"Range": f"bytes={offset}-"} if offset else {} + if offset and validator: + headers["If-Range"] = validator + resp = requests.get(url, stream=True, timeout=timeout, headers=headers) + if offset and resp.status_code == 416: + resp.close() + return None, offset + if offset and resp.status_code != 206: + _LOGGER.debug( + "Server did not resume %s (HTTP %s), restarting", url, resp.status_code + ) + offset = 0 + if not resp.ok: + resp.close() + resp.raise_for_status() + if offset: + _LOGGER.info("Resuming download at %d bytes ...", offset) + return resp, offset + + +def _verify_file(path: Path, sha256: str | None, size: int | None) -> None: + """Raise EsphomeError when ``path`` fails an available sha256/size check.""" + from esphome.core import EsphomeError + + if size is not None and path.stat().st_size != size: + raise EsphomeError(f"size mismatch: expected {size}, got {path.stat().st_size}") + if sha256 is not None: + with path.open("rb") as f: + digest = hashlib.file_digest(f, "sha256").hexdigest() + if digest != sha256: + raise EsphomeError(f"sha256 mismatch: got {digest}") + + +def _load_download_meta(meta: Path, url: str) -> tuple[str | None, int]: + """Return the ``(validator, total)`` a previous run recorded for ``url``. + + ``(None, 0)`` when there is no sidecar, it is unreadable, or it belongs + to a different URL (e.g. a different mirror was tried last time). + """ + try: + with meta.open(encoding="utf-8") as f: + data = json.load(f) + except (OSError, json.JSONDecodeError): + return None, 0 + if not isinstance(data, dict) or data.get("url") != url: + return None, 0 + validator = data.get("validator") + total = data.get("total") + return ( + validator if isinstance(validator, str) else None, + total if isinstance(total, int) else 0, + ) + + +def _write_download_meta( + meta: Path, url: str, validator: str | None, total: int +) -> None: + """Persist resume metadata next to the part file; best-effort. + + Without a validator there is nothing a later run could resume against, + so any stale sidecar is removed instead. + """ + try: + if validator is None: + meta.unlink(missing_ok=True) + else: + meta.write_text( + json.dumps({"url": url, "validator": validator, "total": total}), + encoding="utf-8", + ) + except OSError as e: + _LOGGER.debug("Could not update download metadata %s: %s", meta, e) + + +def _content_length(resp: "requests.Response") -> int: + """Return the response's Content-Length, or 0 when absent or malformed. + + 0 means "unknown", which downstream disables the progress bar and the + resume/completeness logic — a garbage header from a broken proxy must + degrade to a plain single-stream download, not crash the attempt. + """ + try: + return int(resp.headers.get("content-length", 0)) + except ValueError: + return 0 + + +def _response_validator(resp: "requests.Response") -> str | None: + """Return the response's strong validator for ``If-Range`` resumes. + + Weak ETags (``W/...``) are not usable for byte-range conditionals, so + fall back to Last-Modified, or None when the server offers neither. + """ + etag = resp.headers.get("ETag") + if etag and not etag.startswith("W/"): + return etag + return resp.headers.get("Last-Modified") + + +def _stream_response_to_file( + resp: "requests.Response", f: IO[bytes], offset: int, size: int | None = None +) -> None: + """Stream an open ``_open_ranged`` response body into ``f`` at ``offset``. + + Truncates ``f`` to ``offset`` first, so a server-rejected resume + (effective offset 0) discards the stale bytes. ``offset`` also seeds the + progress bar so a resumed download shows overall progress. ``size`` is + the known full file size; when None it is derived from the response's + content-length, and without either there is no progress bar. + """ + f.seek(offset) + f.truncate(offset) + total_size = size or offset + _content_length(resp) + downloaded = offset + progress = ProgressBar("Downloading") if total_size > 0 else None + for chunk in resp.iter_content(chunk_size=256 * 1024): + if chunk: + f.write(chunk) + downloaded += len(chunk) + if progress is not None: + progress.update(downloaded / total_size) + if progress is not None: + progress.update(1) + + +def download_with_resume( + url: str, + dest: PathType, + sha256: str | None = None, + size: int | None = None, + # More attempts than _MIRROR_ATTEMPTS: a single-URL download has no + # mirror fallback, and each retry only re-fetches the remainder. + attempts: int = 5, + timeout: int = 30, + retry_connect_errors: bool = True, +) -> None: + """Download ``url`` to ``dest``, resuming partial downloads. + + The body streams into ``.part``, which persists across attempts and + esphome runs: a mid-stream connection drop only costs one attempt and the + next continues from where it stopped, so an unstable connection converges + on a complete file instead of restarting from zero each retry (#17703). + When ``size`` / ``sha256`` are given the completed file is verified and a + mismatch restarts from scratch; success renames the part file into place. + An already-present ``dest`` that passes verification is kept as-is. + + Resuming a part file from an earlier run needs proof the content is + unchanged: ``sha256`` when the caller has one, or otherwise the server's + If-Range validator recorded in a ``.part.meta`` sidecar by the run + that started the download — a size alone cannot detect a same-length + content change on the server. + + With ``retry_connect_errors`` disabled, a failure before any body bytes + flow (connect error, HTTP error status) propagates immediately instead + of consuming attempts — for callers with their own fallback, like + ``download_from_mirrors``. + + Raises EsphomeError when all attempts are exhausted. + """ + # Imported lazily: requests is a heavy import (~85ms) and is only needed + # when actually downloading a toolchain, never during config validation. + import requests + + from esphome.core import EsphomeError + + dest = Path(dest) + part = dest.with_name(dest.name + ".part") + meta = part.with_name(part.name + ".meta") + dest.parent.mkdir(parents=True, exist_ok=True) + last_error: Exception | None = None + + # An earlier run already completed this download. Only trust it when + # there is something to verify it against; without sha/size the remote + # content may have changed (e.g. a refreshed constraints file), so + # re-download and atomically replace it. + if dest.is_file() and (sha256 is not None or size is not None): + try: + _verify_file(dest, sha256, size) + return + except EsphomeError: + dest.unlink() + + # Adopt the validator/total the run that started this part file recorded, + # so an unfinished download resumes across runs even without a sha256. + validator, expected_total = _load_download_meta(meta, url) + + for _ in range(attempts): + streamed = False + try: + offset = part.stat().st_size if part.is_file() else 0 + # A stitched resume needs two proofs: content identity (the + # bytes being appended belong to the same file as the prefix) + # and completeness. sha256 provides both, across runs. Without + # it, identity needs this run's If-Range validator — a size + # alone cannot detect a same-length content change, so a + # leftover part file from an earlier run must restart — and + # completeness needs a known total length. + if ( + offset + and sha256 is None + and (validator is None or not (size or expected_total)) + ): + _LOGGER.debug( + "Restarting %s from zero: cannot prove a resumed " + "file correct (no sha256, validator=%s, total=%s)", + url, + validator is not None, + size or expected_total, + ) + offset = 0 + if size is None or offset < size: + resp, offset = _open_ranged(url, offset, timeout, validator) + # A None response means HTTP 416: the part file already holds + # every byte the server has; fall through to verification. + if resp is not None: + with resp, part.open("ab") as f: + streamed = True + if offset == 0: + validator = _response_validator(resp) + expected_total = _content_length(resp) + # Recorded so a later run can prove an If-Range + # resume of this part file safe. + _write_download_meta(meta, url, validator, expected_total) + _stream_response_to_file(resp, f, offset, size) + # else: a previous run already wrote every byte (or more) but + # was killed before the rename below. Skip the network entirely + # — a Range request past EOF would draw HTTP 416 — and let + # verification decide whether to promote the file or discard it + # and start over. + + expected_size = size if size is not None else expected_total + _verify_file(part, sha256, expected_size or None) + if not expected_size and sha256 is None: + # No sha, no size, and the server sent no usable + # content-length: nothing can prove the download complete + # (urllib3 still errors on most short bodies, but not on a + # cleanly closed chunked stream). Promote with a debug + # note rather than fail or warn: some servers (e.g. the + # Espressif constraints host) never send a length, the user + # can do nothing about it, and every current caller + # extracts or parses the file afterwards, where corruption + # fails loudly. + _LOGGER.debug( + "Downloaded %s without any way to verify completeness", + dest.name, + ) + # Retry on Windows sharing violations: an antivirus handle on the + # freshly-written file must not get the verified download deleted + # as corrupt by the except clause below. If even the backoff + # retries fail, keep the verified part so the next attempt (or + # run) only has to redo the rename, not the download. + try: + _rename_with_retry(part, dest, overwrite=True) + except PermissionError as e: + _LOGGER.debug("Could not move %s into place: %s", part, e) + last_error = e + continue + meta.unlink(missing_ok=True) + return + except requests.RequestException as e: + # Network failures — including connect errors, since a single + # URL has no mirror-list fallback — keep the part file for the + # next attempt (or the next esphome run) to resume from. Checked + # before OSError: RequestException subclasses IOError. + if not retry_connect_errors and not streamed: + # The caller falls back to another URL on pre-body failures. + raise + _LOGGER.debug("Download of %s interrupted: %s", url, e) + last_error = e + except (OSError, EsphomeError) as e: + # A completed-but-corrupt file (or local disk error) can't be + # trusted for resume; start over. + _LOGGER.debug("Discarding %s: %s", part, e) + part.unlink(missing_ok=True) + meta.unlink(missing_ok=True) + last_error = e + + raise EsphomeError( + f"Failed to download {url} after {attempts} attempts: " + f"{_failure_reason(last_error)}" + ) from last_error + + def _failure_reason(e: Exception) -> str: """Format a download exception for the aggregated error message. @@ -585,110 +906,166 @@ def download_from_mirrors( ``substitutions`` are skipped, so callers can offer templates that only apply to some downloads. + A path target downloads through ``download_with_resume``, so an + interrupted download resumes on the next esphome run; a file-like target + only resumes mid-stream drops within this call. + Raises: ValueError: If mirrors list is empty. EsphomeError: If all download attempts fail; the message lists every attempted URL with its individual failure reason. Also raised if no template matched the provided substitutions. """ - # Imported lazily: requests is a heavy import (~85ms) and is only needed - # when actually downloading a toolchain, never during config validation. + # Imported lazily: requests is a heavy import (~85ms) and is only + # needed when actually downloading, never during config validation. import requests from esphome.core import EsphomeError - # 1. Open target file for writing if path given - with ExitStack() as stack: - if isinstance(target, (str, os.PathLike)): - f = stack.enter_context(Path(target).open("wb")) - elif isinstance(target, (io.RawIOBase, io.IOBase)): - f = target - else: - raise TypeError( - f"target must be str, Path, or file-like object: {type(target)}" - ) + # 1. Classify the target: filesystem path or open file object + path_target: Path | None = None + f: IO[bytes] | None = None + if isinstance(target, (str, os.PathLike)): + path_target = Path(target) + elif isinstance(target, (io.RawIOBase, io.IOBase)): + f = target + else: + raise TypeError( + f"target must be str, Path, or file-like object: {type(target)}" + ) - # 2. Try each mirror in order - failures: list[tuple[str, Exception]] = [] - skipped: list[tuple[str, str]] = [] + # 2. Try each mirror in order + failures: list[tuple[str, Exception]] = [] + skipped: list[tuple[str, str]] = [] - for mirror in mirrors: - # 3. Apply substitutions to URL + for mirror in mirrors: + # 3. Apply substitutions to URL + try: + url = mirror.format(**substitutions) + except KeyError as e: + # The template references a substitution not provided for + # this download (e.g. SHORT_VERSION only exists for x.y.0 + # versions) - expected, the template just doesn't apply. + _LOGGER.debug("Skipping mirror %s: %s not available", mirror, e) + skipped.append((mirror, f"not applicable ({e.args[0]} not available)")) + continue + except (IndexError, ValueError) as e: + # A malformed template (unbalanced braces, bad format spec) + # is an authoring error, not an expected fallthrough - warn + # even if a later mirror succeeds. + _LOGGER.warning("Skipping malformed mirror URL template %s: %r", mirror, e) + skipped.append((mirror, f"skipped ({e!r})")) + continue + + _LOGGER.debug("Trying to download from %s", url) + + # Path targets delegate to download_with_resume so a partial + # download persists (and resumes) across esphome runs. + if path_target is not None: try: - url = mirror.format(**substitutions) - except KeyError as e: - # The template references a substitution not provided for - # this download (e.g. SHORT_VERSION only exists for x.y.0 - # versions) - expected, the template just doesn't apply. - _LOGGER.debug("Skipping mirror %s: %s not available", mirror, e) - skipped.append((mirror, f"not applicable ({e.args[0]} not available)")) - continue - except (IndexError, ValueError) as e: - # A malformed template (unbalanced braces, bad format spec) - # is an authoring error, not an expected fallthrough - warn - # even if a later mirror succeeds. - _LOGGER.warning( - "Skipping malformed mirror URL template %s: %r", mirror, e + download_with_resume( + url, + path_target, + attempts=_MIRROR_ATTEMPTS, + timeout=timeout, + # Pre-body failures (connect/HTTP errors) fall to the + # next mirror immediately; only mid-stream drops + # retry-with-resume on the same URL. + retry_connect_errors=False, ) - skipped.append((mirror, f"skipped ({e!r})")) + return url + except (requests.RequestException, OSError, EsphomeError) as e: + # Everything download_with_resume classifies as a download + # failure; programming errors propagate. + _LOGGER.debug("Failed to download %s: %s", url, str(e)) + failures.append((url, e)) continue - _LOGGER.debug("Trying to download from %s", url) + # 4. Download; mid-stream failures retry the same mirror with + # resume (see download_with_resume) instead of starting over. + # There is no checksum to verify a resumed file against, so a + # stitch is only trusted when the server proves consistency: the + # If-Range validator guarantees 206 only for unchanged content, + # and the expected total length (when the first response carried + # one) guards against short or shifted bodies. Without a + # validator the retry restarts from zero. + offset = 0 + expected_total = 0 + validator = None + for attempt in range(_MIRROR_ATTEMPTS): + try: + resp, offset = _open_ranged(url, offset, timeout, validator) + except (requests.RequestException, OSError) as e: + # Connect/HTTP error, no bytes flowed — next mirror. + _LOGGER.debug("Failed to download %s: %s", url, str(e)) + failures.append((url, e)) + break try: - # 4. Reset file pointer and download - f.seek(0) - f.truncate(0) + # A None response means HTTP 416: the file already holds + # every byte the server has (a drop after the last byte); + # only the length check below remains. + if resp is not None: + with resp: + if offset == 0: + validator = _response_validator(resp) + expected_total = _content_length(resp) + _stream_response_to_file(resp, f, offset) - with requests.get(url, stream=True, timeout=timeout) as r: - r.raise_for_status() - - total_size = int(r.headers.get("content-length", 0)) - downloaded = 0 - - progress = ProgressBar("Downloading") if total_size > 0 else None - - for chunk in r.iter_content(chunk_size=8192): - if chunk: - f.write(chunk) - - downloaded += len(chunk) - - if progress is not None: - progress.update(downloaded / total_size) - - if progress is not None: - progress.update(1) + if expected_total and f.tell() != expected_total: + raise EsphomeError( + f"size mismatch: expected {expected_total}, got {f.tell()}" + ) + if not expected_total: + # Same trust decision as download_with_resume's + # unverifiable promotion; surface it at the same level. + _LOGGER.debug( + "Downloaded %s without any way to verify completeness", + url, + ) _LOGGER.debug("Downloaded successfully from: %s", url) - # 6. Reset file pointer and return + # 5. Reset file pointer and return f.seek(0) return url - except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught + except (requests.RequestException, OSError, EsphomeError) as e: + # Mid-stream drop: keep the received bytes and retry this + # mirror from the current position — but only when the + # server gave a validator to resume against safely AND a + # total length to prove the stitched file complete (the + # length check above is the only verification here). _LOGGER.debug("Failed to download %s: %s", url, str(e)) - failures.append((url, e)) + if validator and expected_total: + offset = f.tell() + else: + _LOGGER.debug( + "Restarting %s from zero: cannot prove a " + "resumed file complete (validator=%s, total=%s)", + url, + validator is not None, + expected_total, + ) + offset = 0 + if attempt == _MIRROR_ATTEMPTS - 1: + failures.append((url, e)) - # 7. Report every attempted URL if all mirrors failed. Falling back - # past an early mirror is normal (e.g. only one of the framework URL - # templates matches a given version's tag), so raising only the last - # error would hide the failure that actually matters. - if failures: - attempts = "".join( - f"\n {url}\n {_failure_reason(e)}" for url, e in failures - ) - attempts += "".join( - f"\n {mirror}\n {reason}" for mirror, reason in skipped - ) - raise EsphomeError( - f"Failed to download from all mirrors:{attempts}" - ) from failures[0][1] - if skipped: - details = "".join( - f"\n {mirror}\n {reason}" for mirror, reason in skipped - ) - raise EsphomeError( - f"No mirror URL template matched the provided substitutions:{details}" - ) - raise ValueError("download_from_mirrors called with an empty mirrors list") + # 6. Report every attempted URL if all mirrors failed. Falling back + # past an early mirror is normal (e.g. only one of the framework URL + # templates matches a given version's tag), so raising only the last + # error would hide the failure that actually matters. + if failures: + attempts = "".join( + f"\n {url}\n {_failure_reason(e)}" for url, e in failures + ) + attempts += "".join(f"\n {mirror}\n {reason}" for mirror, reason in skipped) + raise EsphomeError( + f"Failed to download from all mirrors:{attempts}" + ) from failures[0][1] + if skipped: + details = "".join(f"\n {mirror}\n {reason}" for mirror, reason in skipped) + raise EsphomeError( + f"No mirror URL template matched the provided substitutions:{details}" + ) + raise ValueError("download_from_mirrors called with an empty mirrors list") diff --git a/tests/unit_tests/fixtures/idf_tools_stub/idf_tools.py b/tests/unit_tests/fixtures/idf_tools_stub/idf_tools.py new file mode 100644 index 0000000000..aeff24fb57 --- /dev/null +++ b/tests/unit_tests/fixtures/idf_tools_stub/idf_tools.py @@ -0,0 +1,120 @@ +"""Minimal idf_tools stand-in for get_tool_downloads.py tests.""" + +from collections.abc import Iterable +import os + +CURRENT_PLATFORM = "linux-amd64" +TOOLS_FILE = "tools/tools.json" + + +class ToolBinaryError(RuntimeError): + pass + + +class _G: + idf_path: str | None = None + idf_tools_path: str | None = None + tools_json: str | None = None + + +g = _G() + + +class IDFEnv: + @classmethod + def get_idf_env(cls) -> "IDFEnv": + return cls() + + +def add_and_check_targets(idf_env_obj: IDFEnv, targets_str: str) -> list[str]: + return targets_str.split(",") + + +class _Download: + def __init__(self, url: str, size: int, sha256: str, rename_dist: str = "") -> None: + self.url = url + self.size = size + self.sha256 = sha256 + self.rename_dist = rename_dist + + +class _Version: + def __init__(self, download: _Download | None) -> None: + self._download = download + + def get_download_for_platform(self, platform_name: str) -> _Download | None: + return self._download + + +class _Tool: + def __init__( + self, + versions: dict[str, _Version], + recommended: str | None, + installed: Iterable[str] = (), + broken: bool = False, + ) -> None: + self.versions = versions + self._recommended = recommended + self.versions_installed = list(installed) + self._broken = broken + + def compatible_with_platform(self) -> bool: + return True + + def get_recommended_version(self) -> str | None: + return self._recommended + + def find_installed_versions(self) -> None: + if self._broken: + raise ToolBinaryError("broken binary") + + +_TOOLS = { + "cmake": _Tool( + {"3.30.2": _Version(_Download("https://gh.test/cmake.tar.gz", 11, "aa"))}, + "3.30.2", + ), + "ninja": _Tool( + { + "1.12.1": _Version( + _Download("https://gh.test/ninja-mac.zip", 22, "bb", "ninja-v1.zip") + ) + }, + "1.12.1", + ), + "installed-tool": _Tool( + {"1.0": _Version(_Download("https://gh.test/x.tar.gz", 33, "cc"))}, + "1.0", + installed=["1.0"], + ), + "broken-tool": _Tool( + {"2.0": _Version(_Download("https://gh.test/y.tar.gz", 44, "dd"))}, + "2.0", + broken=True, + ), + "no-recommended-tool": _Tool({"3.0": _Version(None)}, None), + "no-download-tool": _Tool({"4.0": _Version(None)}, "4.0"), +} + + +def load_tools_info() -> dict[str, _Tool]: + return _TOOLS + + +def expand_tools_arg( + tools_spec: list[str], overall_tools: dict[str, _Tool], targets: list[str] +) -> list[str]: + if "required" in tools_spec: + return list(overall_tools) + return [t for t in tools_spec if "@" not in t] + [t for t in tools_spec if "@" in t] + + +def get_idf_download_url_apply_mirrors( + args: object = None, download_url: str = "" +) -> str: + print(f"Changed download URL: {download_url}") # noise on stdout, like idf_tools + prefix = os.environ.get("TEST_MIRROR_PREFIX") + if prefix: + return prefix + download_url + return download_url diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index 79a50059cd..eb68b17572 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -3,10 +3,14 @@ # pylint: disable=protected-access from contextlib import contextmanager +import importlib.util import io import json import logging +import os from pathlib import Path +import runpy +import subprocess import sys import tarfile from types import SimpleNamespace @@ -27,6 +31,7 @@ from esphome.espidf.framework import ( _parse_git_source, _patch_tools_json_demote_openocd, _patch_tools_json_for_linux_arm64, + _prefetch_idf_tool_archives, _windows_long_paths_enabled, _write_idf_version_txt, _write_stamp, @@ -311,6 +316,21 @@ class TestTarExtractHardLinkPrefixStripping: _IDF_VERSION = "5.1.2" +def _fake_download_from_mirrors( + mirrors: list[str], + substitutions: dict[str, str], + target: object, + **kwargs: object, +) -> str: + """Stand-in for download_from_mirrors that creates path targets, since + the framework code opens the downloaded tarball afterwards.""" + if isinstance(target, (str, os.PathLike)): + path = Path(target) + path.parent.mkdir(parents=True, exist_ok=True) + path.touch() + return "https://example.com/idf.tar.xz" + + @pytest.fixture def espidf_mocks(setup_core: Path): """Patch the heavy I/O of check_esp_idf_install and pre-create the framework dir.""" @@ -321,7 +341,7 @@ def espidf_mocks(setup_core: Path): patch("esphome.espidf.framework.rmdir"), patch( "esphome.espidf.framework.download_from_mirrors", - return_value="https://example.com/idf.tar.xz", + side_effect=_fake_download_from_mirrors, ) as download, patch("esphome.espidf.framework.archive_extract_all") as extract, patch("esphome.espidf.framework.create_venv") as venv, @@ -333,6 +353,7 @@ def espidf_mocks(setup_core: Path): patch("esphome.espidf.framework._write_idf_version_txt"), patch("esphome.espidf.framework._patch_tools_json_for_linux_arm64"), patch("esphome.espidf.framework._patch_tools_json_demote_openocd"), + patch("esphome.espidf.framework._prefetch_idf_tool_archives"), patch("esphome.espidf.framework._write_stamp"), patch("esphome.espidf.framework._check_stamp", return_value=True), patch("esphome.espidf.framework._get_idf_version", return_value=_IDF_VERSION), @@ -391,6 +412,20 @@ def test_check_esp_idf_install_already_installed(espidf_mocks: SimpleNamespace) espidf_mocks.venv.assert_not_called() +def test_corrupt_tarball_removed_when_extraction_fails( + espidf_mocks: SimpleNamespace, +) -> None: + """A tarball that fails to extract (e.g. torn by an unclean shutdown) is + deleted so the next run re-downloads instead of failing forever.""" + espidf_mocks.extract.side_effect = RuntimeError("xz: unexpected end of input") + tarball = get_idf_tools_path() / "dist" / f"esp-idf-{_IDF_VERSION}.tar.xz" + + with pytest.raises(RuntimeError, match="unexpected end of input"): + check_esp_idf_install(_IDF_VERSION, force=True) + + assert not tarball.exists() + + def test_check_esp_idf_install_framework_failure(espidf_mocks: SimpleNamespace) -> None: """A failing idf_tools install raises.""" espidf_mocks.run_ok.side_effect = [False] @@ -614,6 +649,286 @@ def test_patch_tools_json_already_patched_is_noop(tmp_path: Path) -> None: assert tools_json.read_text(encoding="utf-8") == before +# --------------------------------------------------------------------------- +# _prefetch_idf_tool_archives +# --------------------------------------------------------------------------- + + +_PREFETCH_JSON = json.dumps( + [ + { + "name": "cmake@3.30.2", + "url": "https://example.com/cmake.tar.gz", + "size": 123, + "sha256": "ab" * 32, + "dest": "cmake-3.30.2.tar.gz", + }, + { + "name": "ninja@1.12.1", + "url": "https://example.com/ninja.zip", + "size": 45, + "sha256": "cd" * 32, + "dest": "ninja.zip", + }, + ] +) + + +def test_prefetch_downloads_each_archive_with_resume(tmp_path: Path) -> None: + with ( + patch( + "esphome.espidf.framework.run_command", + return_value=(True, _PREFETCH_JSON, ""), + ), + patch("esphome.espidf.framework.download_with_resume") as download, + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + ): + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + + dist = get_idf_tools_path() / "dist" + assert download.call_count == 2 + assert download.call_args_list[0][0] == ( + "https://example.com/cmake.tar.gz", + dist / "cmake-3.30.2.tar.gz", + ) + assert download.call_args_list[0][1] == {"sha256": "ab" * 32, "size": 123} + + +def test_prefetch_skips_already_downloaded_archives(tmp_path: Path) -> None: + dist = get_idf_tools_path() / "dist" + dist.mkdir(parents=True) + (dist / "cmake-3.30.2.tar.gz").write_bytes(b"cached") + with ( + patch( + "esphome.espidf.framework.run_command", + return_value=(True, _PREFETCH_JSON, ""), + ), + patch("esphome.espidf.framework.download_with_resume") as download, + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + ): + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + + # only the missing archive is downloaded + assert download.call_count == 1 + assert download.call_args[0][1] == dist / "ninja.zip" + + +@pytest.mark.parametrize( + ("run_result", "download_error", "expected_log"), + [ + ((False, "", "script exploded"), None, "tool downloads"), # script failure + ((True, "{ not json", ""), None, "prefetch failed"), # unparsable output + ( + (True, _PREFETCH_JSON, ""), + OSError("network down"), + "Could not prefetch", + ), # download failure + ], +) +def test_prefetch_failures_never_raise( + tmp_path: Path, + caplog: pytest.LogCaptureFixture, + run_result: tuple[bool, str, str], + download_error: Exception | None, + expected_log: str, +) -> None: + """The prefetch is best-effort; idf_tools downloads whatever is missing.""" + with ( + patch("esphome.espidf.framework.run_command", return_value=run_result), + patch( + "esphome.espidf.framework.download_with_resume", + side_effect=download_error, + ), + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + ): + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + + assert expected_log in caplog.text + + +def test_prefetch_one_failed_archive_does_not_stop_the_rest( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A single archive failing its download must not abort the prefetch of + the remaining archives.""" + with ( + patch( + "esphome.espidf.framework.run_command", + return_value=(True, _PREFETCH_JSON, ""), + ), + patch( + "esphome.espidf.framework.download_with_resume", + side_effect=[OSError("network down"), None], + ) as download, + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + ): + _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None) + + assert download.call_count == 2 + assert "Could not prefetch cmake@3.30.2" in caplog.text + + +def test_prefetch_passes_targets_and_tools_to_script(tmp_path: Path) -> None: + with ( + patch( + "esphome.espidf.framework.run_command", return_value=(True, "[]", "") + ) as run, + patch("esphome.espidf.framework.get_system_python_path", return_value="python"), + ): + _prefetch_idf_tool_archives( + tmp_path, "esp32,esp32c3", ["required", "cmake"], {"IDF_TOOLS_PATH": "/x"} + ) + + cmd = run.call_args[0][0] + assert cmd[-3:] == ["esp32,esp32c3", "required", "cmake"] + assert cmd[1].endswith("get_tool_downloads.py") + # the script inherits the caller's env plus the framework tools PYTHONPATH + env = run.call_args[1]["env"] + assert env["IDF_TOOLS_PATH"] == "/x" + assert env["PYTHONPATH"] == str(tmp_path / "tools") + + +def test_framework_install_prefetches_before_installer( + espidf_mocks: SimpleNamespace, +) -> None: + """The prefetch runs before idf_tools.py install so the installer finds + the archives already in dist/.""" + calls: list[str] = [] + with ( + patch( + "esphome.espidf.framework._prefetch_idf_tool_archives", + side_effect=lambda *a, **k: calls.append("prefetch"), + ), + ): + espidf_mocks.run_ok.side_effect = lambda *a, **k: ( + calls.append("install") or True + ) + check_esp_idf_install(_IDF_VERSION, force=True) + + assert calls.index("prefetch") < calls.index("install") + + +# --------------------------------------------------------------------------- +# get_tool_downloads.py (against the stub idf_tools module in fixtures/) +# --------------------------------------------------------------------------- + + +_IDF_TOOLS_STUB_DIR = Path(__file__).parent / "fixtures" / "idf_tools_stub" + + +def _run_downloads_script( + tmp_path: Path, *args: str, env_extra: dict[str, str] | None = None +) -> subprocess.CompletedProcess[str]: + """Run the real get_tool_downloads.py against the stub idf_tools module.""" + script = Path(__file__).parents[2] / "esphome" / "espidf" / "get_tool_downloads.py" + env = os.environ | { + "PYTHONPATH": str(_IDF_TOOLS_STUB_DIR), + "IDF_TOOLS_PATH": str(tmp_path / "tp"), + } + if env_extra: + env |= env_extra + return subprocess.run( + [sys.executable, str(script), str(tmp_path / "fw"), *args], + capture_output=True, + text=True, + env=env, + check=False, + ) + + +def test_get_tool_downloads_lists_missing_tools(tmp_path: Path) -> None: + """Installed versions are skipped, tools that fail their binary check are + still listed, rename_dist decides the dist filename, and idf_tools' stdout + chatter stays off the JSON channel.""" + result = _run_downloads_script(tmp_path, "esp32", "required") + + assert result.returncode == 0, result.stderr + downloads = {d["name"]: d for d in json.loads(result.stdout)} + # installed-tool@1.0 is already installed and must not be listed + assert set(downloads) == {"cmake@3.30.2", "ninja@1.12.1", "broken-tool@2.0"} + assert downloads["cmake@3.30.2"]["dest"] == "cmake.tar.gz" + assert downloads["cmake@3.30.2"]["size"] == 11 + assert downloads["cmake@3.30.2"]["sha256"] == "aa" + # rename_dist overrides the URL basename + assert downloads["ninja@1.12.1"]["dest"] == "ninja-v1.zip" + # the stub prints informational lines; they must be on stderr + assert "Changed download URL" in result.stderr + + +def test_get_tool_downloads_applies_mirror_rewrite(tmp_path: Path) -> None: + result = _run_downloads_script( + tmp_path, + "esp32", + "required", + env_extra={"TEST_MIRROR_PREFIX": "https://mirror.test/"}, + ) + + assert result.returncode == 0, result.stderr + downloads = json.loads(result.stdout) + assert all(d["url"].startswith("https://mirror.test/") for d in downloads) + + +def _run_downloads_inprocess( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + *args: str, +) -> list[dict]: + """Execute get_tool_downloads.py in-process against the stub idf_tools. + + Unlike the subprocess variant this runs under coverage, exercising the + script's own lines. + """ + spec = importlib.util.spec_from_file_location( + "idf_tools", _IDF_TOOLS_STUB_DIR / "idf_tools.py" + ) + stub = importlib.util.module_from_spec(spec) + spec.loader.exec_module(stub) + monkeypatch.setitem(sys.modules, "idf_tools", stub) + monkeypatch.setenv("IDF_TOOLS_PATH", str(tmp_path / "tp")) + script = Path(__file__).parents[2] / "esphome" / "espidf" / "get_tool_downloads.py" + monkeypatch.setattr(sys, "argv", [str(script), str(tmp_path / "fw"), *args]) + runpy.run_path(str(script)) + return json.loads(capsys.readouterr().out) + + +def test_get_tool_downloads_inprocess_full_flow( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """In-process run covering the whole script: required expansion, + installed/broken tools, rename_dist, and version pinning via tool@version.""" + downloads = { + d["name"]: d + for d in _run_downloads_inprocess( + tmp_path, monkeypatch, capsys, "esp32", "required" + ) + } + assert set(downloads) == {"cmake@3.30.2", "ninja@1.12.1", "broken-tool@2.0"} + assert downloads["ninja@1.12.1"]["dest"] == "ninja-v1.zip" + assert downloads["cmake@3.30.2"]["url"] == "https://gh.test/cmake.tar.gz" + + +def test_get_tool_downloads_inprocess_explicit_tool_specs( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """Explicit tool names and tool@version specs resolve; unknown tools and + unknown versions are skipped.""" + downloads = _run_downloads_inprocess( + tmp_path, + monkeypatch, + capsys, + "esp32", + "cmake@3.30.2", + "no-such-tool", + "cmake@9.9.9", + ) + assert [d["name"] for d in downloads] == ["cmake@3.30.2"] + + # --------------------------------------------------------------------------- # _patch_tools_json_demote_openocd (openocd-esp32 made optional) # --------------------------------------------------------------------------- diff --git a/tests/unit_tests/test_framework_helpers.py b/tests/unit_tests/test_framework_helpers.py index e662d2d015..b8aa19d6ae 100644 --- a/tests/unit_tests/test_framework_helpers.py +++ b/tests/unit_tests/test_framework_helpers.py @@ -2,8 +2,10 @@ # pylint: disable=protected-access +import hashlib import importlib.util import io +import json import logging import os from pathlib import Path @@ -16,6 +18,7 @@ import zipfile import pytest import requests as req +from esphome import framework_helpers from esphome.core import EsphomeError from esphome.framework_helpers import ( _7z_extract_all, @@ -26,6 +29,7 @@ from esphome.framework_helpers import ( archive_extract_all, create_venv, download_from_mirrors, + download_with_resume, get_project_compile_flags, get_project_cxx_compile_flags, get_project_link_flags, @@ -507,7 +511,7 @@ class TestArchiveExtractAll: # --------------------------------------------------------------------------- -# download_from_mirrors +# download_from_mirrors / download_with_resume # --------------------------------------------------------------------------- @@ -515,6 +519,8 @@ def _mock_response(content: bytes, ok: bool = True) -> MagicMock: r = MagicMock() r.__enter__.return_value = r r.__exit__.return_value = False + r.status_code = 200 + r.ok = ok if ok: r.raise_for_status.return_value = None else: @@ -524,6 +530,563 @@ def _mock_response(content: bytes, ok: bool = True) -> MagicMock: return r +def _interrupted_response(content: bytes, etag: str | None = None) -> MagicMock: + """A response whose body yields ``content`` and then drops mid-stream. + + ``etag`` makes the response resumable: without a validator the retry + logic restarts from zero rather than stitching unverified bytes. + """ + + def body(chunk_size): + yield content + raise req.exceptions.ChunkedEncodingError("connection dropped") + + r = _mock_response(b"") + if etag is not None: + r.headers = {**r.headers, "ETag": etag} + r.iter_content.side_effect = body + return r + + +def _resumed_response(content: bytes) -> MagicMock: + """An HTTP 206 response continuing an interrupted download.""" + r = _mock_response(content) + r.status_code = 206 + return r + + +class TestOpenRanged: + def test_fresh_download_sends_no_range(self) -> None: + with patch("requests.get", return_value=_mock_response(b"x")) as mock_get: + resp, offset = framework_helpers._open_ranged("https://e.com/f", 0, 30) + assert offset == 0 + assert mock_get.call_args[1]["headers"] == {} + assert resp is mock_get.return_value + + def test_resume_kept_on_206(self) -> None: + with patch("requests.get", return_value=_resumed_response(b"x")): + _, offset = framework_helpers._open_ranged("https://e.com/f", 7, 30) + assert offset == 7 + + def test_resume_downgraded_on_200(self) -> None: + """A server that ignores the Range header forces a restart.""" + with patch("requests.get", return_value=_mock_response(b"x")): + _, offset = framework_helpers._open_ranged("https://e.com/f", 7, 30) + assert offset == 0 + + def test_http_error_closes_response_and_raises(self) -> None: + r = _mock_response(b"", ok=False) + with ( + patch("requests.get", return_value=r), + pytest.raises(req.HTTPError), + ): + framework_helpers._open_ranged("https://e.com/f", 0, 30) + r.close.assert_called_once() + + def test_connect_error_propagates(self) -> None: + with ( + patch("requests.get", side_effect=req.ConnectionError("refused")), + pytest.raises(req.ConnectionError), + ): + framework_helpers._open_ranged("https://e.com/f", 0, 30) + + +class TestDownloadWithResume: + def test_downloads_and_renames(self, tmp_path: Path) -> None: + dest = tmp_path / "tool.tar.gz" + with patch("requests.get", return_value=_mock_response(b"data")) as mock_get: + download_with_resume("https://example.com/t", dest) + assert dest.read_bytes() == b"data" + assert not (tmp_path / "tool.tar.gz.part").exists() + # a fresh download must not send a Range header + assert "Range" not in mock_get.call_args[1]["headers"] + + def test_mid_stream_drop_resumes_with_range(self, tmp_path: Path) -> None: + dest = tmp_path / "tool.tar.gz" + first = _interrupted_response(b"1234", etag='"v1"') + first.headers = {**first.headers, "content-length": "8"} + with patch( + "requests.get", + side_effect=[first, _resumed_response(b"5678")], + ) as mock_get: + download_with_resume("https://example.com/t", dest) + # earlier bytes were kept, remainder appended conditionally + assert dest.read_bytes() == b"12345678" + assert mock_get.call_args_list[1][1]["headers"] == { + "Range": "bytes=4-", + "If-Range": '"v1"', + } + + def test_unverifiable_drop_without_length_restarts(self, tmp_path: Path) -> None: + """A validator alone is not enough to stitch when nothing can prove + the stitched file complete (no sha/size and no content-length).""" + dest = tmp_path / "tool.tar.gz" + with patch( + "requests.get", + side_effect=[ + _interrupted_response(b"1234", etag='"v1"'), + _mock_response(b"full"), + ], + ) as mock_get: + download_with_resume("https://example.com/t", dest) + assert dest.read_bytes() == b"full" + assert "Range" not in mock_get.call_args_list[1][1]["headers"] + + def test_resumed_clean_but_short_body_discarded(self, tmp_path: Path) -> None: + """A resumed stream that ends cleanly but short of the advertised + total is rejected and re-downloaded, not promoted.""" + dest = tmp_path / "tool.tar.gz" + first = _interrupted_response(b"abcd", etag='"v1"') + first.headers = {**first.headers, "content-length": "8"} + # resume ends cleanly after only 2 of the 4 missing bytes + short = _resumed_response(b"ef") + full = _mock_response(b"abcdefgh") + full.headers = {**full.headers, "content-length": "8"} + with patch("requests.get", side_effect=[first, short, full]) as mock_get: + download_with_resume("https://example.com/t", dest) + assert dest.read_bytes() == b"abcdefgh" + # the short stitch was discarded; the final attempt started fresh + assert "Range" not in mock_get.call_args_list[2][1]["headers"] + + def test_unverifiable_drop_without_validator_restarts(self, tmp_path: Path) -> None: + """No sha/size and no server validator: the retry must not stitch.""" + dest = tmp_path / "tool.tar.gz" + with patch( + "requests.get", + side_effect=[_interrupted_response(b"1234"), _mock_response(b"full")], + ) as mock_get: + download_with_resume("https://example.com/t", dest) + assert dest.read_bytes() == b"full" + assert "Range" not in mock_get.call_args_list[1][1]["headers"] + + def test_resume_across_invocations_from_part_file(self, tmp_path: Path) -> None: + """A .part file left by a previous run is resumed, not restarted, + when sha/size verification will vouch for the stitched result.""" + dest = tmp_path / "tool.tar.gz" + (tmp_path / "tool.tar.gz.part").write_bytes(b"12345") + good = hashlib.sha256(b"12345678").hexdigest() + with patch("requests.get", return_value=_resumed_response(b"678")) as mock_get: + download_with_resume("https://example.com/t", dest, sha256=good, size=8) + assert dest.read_bytes() == b"12345678" + assert mock_get.call_args[1]["headers"] == {"Range": "bytes=5-"} + + def test_unverifiable_leftover_part_file_ignored(self, tmp_path: Path) -> None: + """Without sha/size there is no way to vouch for a cross-run stitch, + so a leftover part file starts over.""" + dest = tmp_path / "tool.tar.gz" + (tmp_path / "tool.tar.gz.part").write_bytes(b"12345") + with patch("requests.get", return_value=_mock_response(b"fresh")) as mock_get: + download_with_resume("https://example.com/t", dest) + assert dest.read_bytes() == b"fresh" + assert "Range" not in mock_get.call_args[1]["headers"] + + def test_server_without_range_support_restarts(self, tmp_path: Path) -> None: + """HTTP 200 in response to a Range request truncates and restarts.""" + dest = tmp_path / "tool.tar.gz" + (tmp_path / "tool.tar.gz.part").write_bytes(b"sta") + good = hashlib.sha256(b"fresh").hexdigest() + with patch("requests.get", return_value=_mock_response(b"fresh")) as mock_get: + download_with_resume("https://example.com/t", dest, sha256=good, size=5) + # the Range request was sent (verifiable resume) and downgraded + assert mock_get.call_args[1]["headers"] == {"Range": "bytes=3-"} + assert dest.read_bytes() == b"fresh" + + def test_size_only_leftover_part_restarts(self, tmp_path: Path) -> None: + """A size alone cannot detect a same-length content change on the + server, so a cross-run part without sha256 restarts from zero.""" + dest = tmp_path / "tool.tar.gz" + (tmp_path / "tool.tar.gz.part").write_bytes(b"12") + with patch("requests.get", return_value=_mock_response(b"1234")) as mock_get: + download_with_resume("https://example.com/t", dest, size=4) + assert "Range" not in mock_get.call_args[1]["headers"] + assert dest.read_bytes() == b"1234" + + def test_size_only_in_run_drop_resumes_with_validator(self, tmp_path: Path) -> None: + """Within a run the If-Range validator proves identity, so size-only + callers still resume mid-stream drops.""" + dest = tmp_path / "tool.tar.gz" + with patch( + "requests.get", + side_effect=[ + _interrupted_response(b"12", etag='"v1"'), + _resumed_response(b"34"), + ], + ) as mock_get: + download_with_resume("https://example.com/t", dest, size=4) + assert dest.read_bytes() == b"1234" + assert mock_get.call_args_list[1][1]["headers"] == { + "Range": "bytes=2-", + "If-Range": '"v1"', + } + + def test_unverifiable_download_logged( + self, tmp_path: Path, caplog: pytest.LogCaptureFixture + ) -> None: + """No sha, no size, no content-length: the download is promoted with + a debug note (routine for e.g. the constraints host, so not a + warning) that completeness could not be verified.""" + dest = tmp_path / "tool.tar.gz" + with ( + caplog.at_level(logging.DEBUG, logger="esphome.framework_helpers"), + patch("requests.get", return_value=_mock_response(b"data")), + ): + download_with_resume("https://example.com/t", dest) + assert dest.read_bytes() == b"data" + assert "without any way to verify completeness" in caplog.text + + def test_416_promotes_complete_part_when_size_unknown(self, tmp_path: Path) -> None: + """sha256-only caller with a byte-complete part file: the server's + 416 confirms nothing is missing, verification promotes in place, and + the 416 must not loop as a retryable error.""" + dest = tmp_path / "tool.tar.gz" + (tmp_path / "tool.tar.gz.part").write_bytes(b"data") + good = hashlib.sha256(b"data").hexdigest() + r416 = _mock_response(b"", ok=False) + r416.status_code = 416 + with patch("requests.get", return_value=r416) as mock_get: + download_with_resume("https://example.com/t", dest, sha256=good) + assert mock_get.call_count == 1 + r416.close.assert_called_once() + assert dest.read_bytes() == b"data" + + def test_416_with_corrupt_part_discards_and_redownloads( + self, tmp_path: Path + ) -> None: + dest = tmp_path / "tool.tar.gz" + (tmp_path / "tool.tar.gz.part").write_bytes(b"bad!") + good = hashlib.sha256(b"data").hexdigest() + r416 = _mock_response(b"", ok=False) + r416.status_code = 416 + with patch( + "requests.get", side_effect=[r416, _mock_response(b"data")] + ) as mock_get: + download_with_resume("https://example.com/t", dest, sha256=good) + assert "Range" not in mock_get.call_args_list[1][1]["headers"] + assert dest.read_bytes() == b"data" + + def test_hash_mismatch_discards_and_retries(self, tmp_path: Path) -> None: + dest = tmp_path / "tool.tar.gz" + good = hashlib.sha256(b"good").hexdigest() + with patch( + "requests.get", + side_effect=[_mock_response(b"bad!"), _mock_response(b"good")], + ) as mock_get: + download_with_resume("https://example.com/t", dest, sha256=good, size=4) + assert dest.read_bytes() == b"good" + # the corrupt part file was discarded, so the retry starts fresh + assert "Range" not in mock_get.call_args_list[1][1]["headers"] + + def test_size_mismatch_discards_part(self, tmp_path: Path) -> None: + dest = tmp_path / "tool.tar.gz" + with ( + patch("requests.get", return_value=_mock_response(b"xx")), + pytest.raises(EsphomeError, match="after 2 attempts"), + ): + download_with_resume("https://example.com/t", dest, size=99, attempts=2) + assert not (tmp_path / "tool.tar.gz.part").exists() + assert not dest.exists() + + def test_attempts_exhausted_keeps_part_file(self, tmp_path: Path) -> None: + """Mid-stream failures keep the partial file so a later run resumes.""" + dest = tmp_path / "tool.tar.gz" + first = _interrupted_response(b"12", etag='"v1"') + first.headers = {**first.headers, "content-length": "4"} + second = _interrupted_response(b"34") + second.status_code = 206 + with ( + patch("requests.get", side_effect=[first, second]), + pytest.raises(EsphomeError, match="after 2 attempts"), + ): + download_with_resume("https://example.com/t", dest, attempts=2) + assert (tmp_path / "tool.tar.gz.part").read_bytes() == b"1234" + + def test_multiple_drops_accumulate_across_attempts(self, tmp_path: Path) -> None: + """Each attempt appends its bytes; three partial responses complete + the file.""" + dest = tmp_path / "tool.tar.gz" + first = _interrupted_response(b"ab", etag='"v1"') + first.headers = {**first.headers, "content-length": "6"} + second = _interrupted_response(b"cd") + second.status_code = 206 + third = _resumed_response(b"ef") + with patch( + "requests.get", + side_effect=[first, second, third], + ) as mock_get: + download_with_resume("https://example.com/t", dest) + assert dest.read_bytes() == b"abcdef" + expected = {"Range": "bytes=2-", "If-Range": '"v1"'} + assert mock_get.call_args_list[1][1]["headers"] == expected + expected = {"Range": "bytes=4-", "If-Range": '"v1"'} + assert mock_get.call_args_list[2][1]["headers"] == expected + + def test_connect_error_then_success(self, tmp_path: Path) -> None: + """A connect error (no response at all) consumes an attempt and the + next attempt succeeds.""" + dest = tmp_path / "tool.tar.gz" + with patch( + "requests.get", + side_effect=[req.ConnectionError("refused"), _mock_response(b"data")], + ): + download_with_resume("https://example.com/t", dest) + assert dest.read_bytes() == b"data" + + def test_http_error_keeps_part_file(self, tmp_path: Path) -> None: + """A transient HTTP error (e.g. 503) must not discard resume state.""" + dest = tmp_path / "tool.tar.gz" + (tmp_path / "tool.tar.gz.part").write_bytes(b"keep") + error = _mock_response(b"", ok=False) + error.status_code = 503 + with ( + patch("requests.get", return_value=error), + pytest.raises(EsphomeError, match="after 1 attempts"), + ): + download_with_resume("https://example.com/t", dest, attempts=1) + assert (tmp_path / "tool.tar.gz.part").read_bytes() == b"keep" + + def test_creates_missing_parent_directories(self, tmp_path: Path) -> None: + dest = tmp_path / "dist" / "nested" / "tool.tar.gz" + with patch("requests.get", return_value=_mock_response(b"data")): + download_with_resume("https://example.com/t", dest) + assert dest.read_bytes() == b"data" + + def test_verifies_both_size_and_sha(self, tmp_path: Path) -> None: + dest = tmp_path / "tool.tar.gz" + good = hashlib.sha256(b"data").hexdigest() + with patch("requests.get", return_value=_mock_response(b"data")): + download_with_resume("https://example.com/t", dest, sha256=good, size=4) + assert dest.read_bytes() == b"data" + + def test_corrupt_partial_resumed_then_discarded_then_redownloaded( + self, tmp_path: Path + ) -> None: + """The full recovery cycle for a corrupted partial download: the + resume completes it, verification fails, the poisoned part file is + discarded, and the next attempt re-downloads from scratch.""" + dest = tmp_path / "tool.tar.gz" + # a previous run left a corrupted 4-byte prefix behind + (tmp_path / "tool.tar.gz.part").write_bytes(b"BAD!") + good = hashlib.sha256(b"data66").hexdigest() + with patch( + "requests.get", + side_effect=[ + _resumed_response(b"66"), # resume "completes" the bad part + _mock_response(b"data66"), # clean retry from zero + ], + ) as mock_get: + download_with_resume("https://example.com/t", dest, sha256=good, size=6) + # first attempt resumed at the corrupt offset, failed verification; + # second attempt started fresh (no Range header) and succeeded + assert mock_get.call_args_list[0][1]["headers"] == {"Range": "bytes=4-"} + assert "Range" not in mock_get.call_args_list[1][1]["headers"] + assert dest.read_bytes() == b"data66" + assert not (tmp_path / "tool.tar.gz.part").exists() + + def test_existing_dest_passing_verification_kept(self, tmp_path: Path) -> None: + """A dest completed by an earlier run is reused without any request.""" + dest = tmp_path / "tool.tar.gz" + dest.write_bytes(b"data") + good = hashlib.sha256(b"data").hexdigest() + with patch("requests.get") as mock_get: + download_with_resume("https://example.com/t", dest, sha256=good, size=4) + mock_get.assert_not_called() + assert dest.read_bytes() == b"data" + + @pytest.mark.parametrize( + "stale", + [ + pytest.param(b"corrupt!", id="wrong-size"), + pytest.param(b"bad!", id="right-size-wrong-hash"), + ], + ) + def test_existing_dest_failing_verification_redownloaded( + self, tmp_path: Path, stale: bytes + ) -> None: + dest = tmp_path / "tool.tar.gz" + dest.write_bytes(stale) + good = hashlib.sha256(b"data").hexdigest() + with patch("requests.get", return_value=_mock_response(b"data")): + download_with_resume("https://example.com/t", dest, sha256=good, size=4) + assert dest.read_bytes() == b"data" + + def test_existing_dest_with_size_only_kept(self, tmp_path: Path) -> None: + dest = tmp_path / "tool.tar.gz" + dest.write_bytes(b"data") + with patch("requests.get") as mock_get: + download_with_resume("https://example.com/t", dest, size=4) + mock_get.assert_not_called() + + def test_existing_dest_with_sha_only_kept(self, tmp_path: Path) -> None: + """sha-only verification also authorizes reusing a completed dest.""" + dest = tmp_path / "tool.tar.gz" + dest.write_bytes(b"data") + good = hashlib.sha256(b"data").hexdigest() + with patch("requests.get") as mock_get: + download_with_resume("https://example.com/t", dest, sha256=good) + mock_get.assert_not_called() + + def test_meta_write_failure_is_best_effort(self, tmp_path: Path) -> None: + """A failure to persist the resume sidecar must not fail the + download itself.""" + dest = tmp_path / "f.tar.xz" + first = _mock_response(b"data") + first.headers = {**first.headers, "ETag": '"v1"', "content-length": "4"} + with ( + patch("requests.get", return_value=first), + patch.object(Path, "write_text", side_effect=OSError("read-only")), + ): + download_with_resume("https://example.com/f", dest) + assert dest.read_bytes() == b"data" + + def test_meta_sidecar_written_and_removed(self, tmp_path: Path) -> None: + """The validator sidecar appears while downloading and is cleaned up + with the promotion.""" + dest = tmp_path / "f.tar.xz" + meta = tmp_path / "f.tar.xz.part.meta" + seen: list[bool] = [] + first = _interrupted_response(b"1234", etag='"v1"') + first.headers = {**first.headers, "content-length": "8"} + responses = [first] + + def get(*args: object, **kwargs: object) -> MagicMock: + if responses: + return responses.pop(0) + # the resume request: the sidecar written by the first response + # must already be on disk at this point + seen.append(meta.is_file()) + return _resumed_response(b"5678") + + with patch("requests.get", side_effect=get): + download_with_resume("https://example.com/f", dest) + assert dest.read_bytes() == b"12345678" + assert seen == [True] # sidecar existed during the resume attempt + assert not meta.exists() # cleaned up on success + + def test_locked_promotion_keeps_verified_part(self, tmp_path: Path) -> None: + """A rename that stays blocked (e.g. a long-lived Windows file lock) + must not delete the verified download; the next attempt retries just + the rename without touching the network.""" + dest = tmp_path / "tool.tar.gz" + good = hashlib.sha256(b"data").hexdigest() + with ( + patch("requests.get", return_value=_mock_response(b"data")) as mock_get, + patch( + "esphome.framework_helpers._rename_with_retry", + side_effect=[PermissionError("locked"), None], + ) as rename, + ): + download_with_resume("https://example.com/t", dest, sha256=good, size=4) + # one download; the second attempt only redid the rename + assert mock_get.call_count == 1 + assert rename.call_count == 2 + + def test_locked_promotion_exhausted_keeps_part_for_next_run( + self, tmp_path: Path + ) -> None: + dest = tmp_path / "tool.tar.gz" + good = hashlib.sha256(b"data").hexdigest() + with ( + patch("requests.get", return_value=_mock_response(b"data")), + patch( + "esphome.framework_helpers._rename_with_retry", + side_effect=PermissionError("locked"), + ), + pytest.raises(EsphomeError, match="after 1 attempts"), + ): + download_with_resume( + "https://example.com/t", dest, sha256=good, size=4, attempts=1 + ) + # the verified bytes survive for the next run + assert (tmp_path / "tool.tar.gz.part").read_bytes() == b"data" + + def test_meta_sidecar_resumes_across_runs_without_sha(self, tmp_path: Path) -> None: + """A later run resumes an unfinished download using the validator the + first run stored — the cross-run fix for the framework tarball.""" + dest = tmp_path / "f.tar.xz" + (tmp_path / "f.tar.xz.part").write_bytes(b"1234") + (tmp_path / "f.tar.xz.part.meta").write_text( + json.dumps( + {"url": "https://example.com/f", "validator": '"v1"', "total": 8} + ) + ) + with patch("requests.get", return_value=_resumed_response(b"5678")) as mock_get: + download_with_resume("https://example.com/f", dest) + assert dest.read_bytes() == b"12345678" + assert mock_get.call_args[1]["headers"] == { + "Range": "bytes=4-", + "If-Range": '"v1"', + } + + def test_meta_sidecar_for_other_url_ignored(self, tmp_path: Path) -> None: + """Metadata from a different mirror URL must not authorize a stitch.""" + dest = tmp_path / "f.tar.xz" + (tmp_path / "f.tar.xz.part").write_bytes(b"1234") + (tmp_path / "f.tar.xz.part.meta").write_text( + json.dumps({"url": "https://other.com/f", "validator": '"v1"', "total": 8}) + ) + full = _mock_response(b"12345678") + with patch("requests.get", return_value=full) as mock_get: + download_with_resume("https://example.com/f", dest) + assert "Range" not in mock_get.call_args[1]["headers"] + assert dest.read_bytes() == b"12345678" + + def test_complete_part_file_promoted_without_network(self, tmp_path: Path) -> None: + """A .part holding every byte (killed between write and rename) is + verified in place and promoted; no request is made, so no 416 loop.""" + dest = tmp_path / "tool.tar.gz" + (tmp_path / "tool.tar.gz.part").write_bytes(b"data") + good = hashlib.sha256(b"data").hexdigest() + with patch("requests.get") as mock_get: + download_with_resume("https://example.com/t", dest, sha256=good, size=4) + mock_get.assert_not_called() + assert dest.read_bytes() == b"data" + + def test_complete_but_corrupt_part_file_redownloaded(self, tmp_path: Path) -> None: + """A full-size .part with a wrong hash is discarded and re-downloaded + from scratch.""" + dest = tmp_path / "tool.tar.gz" + (tmp_path / "tool.tar.gz.part").write_bytes(b"bad!") + good = hashlib.sha256(b"data").hexdigest() + with patch("requests.get", return_value=_mock_response(b"data")) as mock_get: + download_with_resume("https://example.com/t", dest, sha256=good, size=4) + assert "Range" not in mock_get.call_args[1]["headers"] + assert dest.read_bytes() == b"data" + + def test_oversized_part_file_discarded(self, tmp_path: Path) -> None: + """A .part larger than the expected size fails verification and is + replaced by a fresh download.""" + dest = tmp_path / "tool.tar.gz" + (tmp_path / "tool.tar.gz.part").write_bytes(b"toolong") + good = hashlib.sha256(b"data").hexdigest() + with patch("requests.get", return_value=_mock_response(b"data")): + download_with_resume("https://example.com/t", dest, sha256=good, size=4) + assert dest.read_bytes() == b"data" + + def test_malformed_content_length_degrades_gracefully(self, tmp_path: Path) -> None: + """A garbage Content-Length must not crash the attempt; it means + "unknown", so a drop restarts instead of stitching and a clean + download still succeeds.""" + dest = tmp_path / "tool.tar.gz" + first = _interrupted_response(b"1234", etag='"v1"') + first.headers = {**first.headers, "content-length": "explode"} + retry = _mock_response(b"full") + retry.headers = {**retry.headers, "content-length": "explode"} + with patch("requests.get", side_effect=[first, retry]) as mock_get: + download_with_resume("https://example.com/t", dest) + assert dest.read_bytes() == b"full" + # unknown length -> completeness unprovable -> no resume attempted + assert "Range" not in mock_get.call_args_list[1][1]["headers"] + + def test_zero_byte_part_file_sends_no_range(self, tmp_path: Path) -> None: + """An empty leftover part file is a fresh download, not a resume.""" + dest = tmp_path / "tool.tar.gz" + (tmp_path / "tool.tar.gz.part").write_bytes(b"") + with patch("requests.get", return_value=_mock_response(b"data")) as mock_get: + download_with_resume("https://example.com/t", dest) + assert mock_get.call_args[1]["headers"] == {} + assert dest.read_bytes() == b"data" + + class TestDownloadFromMirrors: def test_success_returns_url_and_writes_content(self, tmp_path: Path) -> None: target = tmp_path / "out.bin" @@ -640,7 +1203,8 @@ class TestDownloadFromMirrors: ei.value ) - def test_falls_back_to_second_mirror(self, tmp_path: Path) -> None: + def test_falls_back_to_second_mirror(self) -> None: + buf = io.BytesIO() with patch( "requests.get", side_effect=[_mock_response(b"", ok=False), _mock_response(b"second")], @@ -648,14 +1212,152 @@ class TestDownloadFromMirrors: url = download_from_mirrors( ["https://mirror1.com/f", "https://mirror2.com/f"], {}, - tmp_path / "out.bin", + buf, ) assert url == "https://mirror2.com/f" - assert (tmp_path / "out.bin").read_bytes() == b"second" + assert buf.getvalue() == b"second" - def test_all_mirrors_fail_raises_error_listing_every_attempt( - self, tmp_path: Path - ) -> None: + def test_mid_stream_drop_resumes_same_mirror(self) -> None: + """A mid-stream failure retries the same mirror with Range and + If-Range headers, keeping the bytes already received, before falling + to the next.""" + first = _interrupted_response(b"1234", etag='"v1"') + first.headers = {**first.headers, "content-length": "8"} + buf = io.BytesIO() + with patch( + "requests.get", + side_effect=[first, _resumed_response(b"5678")], + ) as mock_get: + url = download_from_mirrors( + ["https://mirror1.com/f", "https://mirror2.com/f"], + {}, + buf, + ) + assert url == "https://mirror1.com/f" + assert buf.getvalue() == b"12345678" + assert mock_get.call_count == 2 + assert mock_get.call_args_list[1][0][0] == "https://mirror1.com/f" + # the resume is conditional on the content being unchanged + assert mock_get.call_args_list[1][1]["headers"] == { + "Range": "bytes=4-", + "If-Range": '"v1"', + } + + def test_mid_stream_drop_without_validator_restarts(self) -> None: + """A server offering no ETag/Last-Modified cannot be resumed safely; + the retry restarts from zero instead of stitching unverified bytes.""" + buf = io.BytesIO() + with patch( + "requests.get", + side_effect=[_interrupted_response(b"1234"), _mock_response(b"full")], + ) as mock_get: + download_from_mirrors(["https://mirror1.com/f"], {}, buf) + assert buf.getvalue() == b"full" + assert "Range" not in mock_get.call_args_list[1][1]["headers"] + + def test_drop_after_last_byte_recovers_via_416(self) -> None: + """A connection drop after the final body byte leaves a complete file; + the retry's 416 answer plus the length check turn it into success + instead of a wasted refetch.""" + first = _interrupted_response(b"1234", etag='"v1"') + first.headers = {**first.headers, "content-length": "4"} + r416 = _mock_response(b"", ok=False) + r416.status_code = 416 + buf = io.BytesIO() + with patch("requests.get", side_effect=[first, r416]) as mock_get: + url = download_from_mirrors(["https://mirror1.com/f"], {}, buf) + assert url == "https://mirror1.com/f" + assert buf.getvalue() == b"1234" + assert mock_get.call_count == 2 + + def test_mirror_drop_without_length_restarts(self) -> None: + """With no content-length there is no way to prove a stitched file + complete, so the retry restarts even though a validator exists.""" + buf = io.BytesIO() + with patch( + "requests.get", + side_effect=[ + _interrupted_response(b"1234", etag='"v1"'), + _mock_response(b"full"), + ], + ) as mock_get: + download_from_mirrors(["https://mirror1.com/f"], {}, buf) + assert buf.getvalue() == b"full" + assert "Range" not in mock_get.call_args_list[1][1]["headers"] + + def test_path_target_resumes_across_runs(self, tmp_path: Path) -> None: + """A path target routes through download_with_resume: a part file and + metadata from a previous run resume instead of restarting.""" + dest = tmp_path / "idf.tar.xz" + (tmp_path / "idf.tar.xz.part").write_bytes(b"1234") + (tmp_path / "idf.tar.xz.part.meta").write_text( + json.dumps( + {"url": "https://mirror1.com/f", "validator": '"v1"', "total": 8} + ) + ) + with patch("requests.get", return_value=_resumed_response(b"5678")) as mock_get: + url = download_from_mirrors(["https://mirror1.com/f"], {}, dest) + assert url == "https://mirror1.com/f" + assert dest.read_bytes() == b"12345678" + assert mock_get.call_args[1]["headers"] == { + "Range": "bytes=4-", + "If-Range": '"v1"', + } + + def test_path_target_falls_back_to_next_mirror(self, tmp_path: Path) -> None: + dest = tmp_path / "idf.tar.xz" + with patch( + "requests.get", + side_effect=[req.ConnectionError("down"), _mock_response(b"data")], + ): + url = download_from_mirrors( + ["https://mirror1.com/f", "https://mirror2.com/f"], {}, dest + ) + assert url == "https://mirror2.com/f" + assert dest.read_bytes() == b"data" + + def test_resumed_short_body_fails_length_check(self) -> None: + """A stitched file whose final length disagrees with the advertised + total is rejected instead of reported as success.""" + first = _interrupted_response(b"1234", etag='"v1"') + first.headers = {**first.headers, "content-length": "8"} + # the resume ends early (5 of 8 bytes); the poisoned part is then + # discarded and the fresh retry also delivers a short body + short_resume = _resumed_response(b"5") + short_fresh = _mock_response(b"56") + short_fresh.headers = {**short_fresh.headers, "content-length": "8"} + buf = io.BytesIO() + with ( + patch("requests.get", side_effect=[first, short_resume, short_fresh]), + pytest.raises(EsphomeError, match="all mirrors"), + ): + download_from_mirrors(["https://mirror1.com/f"], {}, buf) + + def test_failed_mirror_leftovers_not_kept_for_next_mirror(self) -> None: + """Bytes from a mirror that failed all attempts must not leak into the + next mirror's download (no bogus Range request, fresh content).""" + exhausted = [_interrupted_response(b"AAAA", etag='"a1"')] + for _ in range(2): + r = _interrupted_response(b"BB") + r.status_code = 206 + exhausted.append(r) + buf = io.BytesIO() + with patch( + "requests.get", + side_effect=exhausted + [_mock_response(b"clean")], + ) as mock_get: + url = download_from_mirrors( + ["https://mirror1.com/f", "https://mirror2.com/f"], + {}, + buf, + ) + assert url == "https://mirror2.com/f" + assert buf.getvalue() == b"clean" + # the second mirror starts fresh, without a Range header + assert mock_get.call_args_list[3][0][0] == "https://mirror2.com/f" + assert "Range" not in mock_get.call_args_list[3][1]["headers"] + + def test_all_mirrors_fail_raises_error_listing_every_attempt(self) -> None: with ( patch( "requests.get", @@ -666,7 +1368,7 @@ class TestDownloadFromMirrors: download_from_mirrors( ["https://mirror1.com/f", "https://mirror2.com/f"], {}, - tmp_path / "out.bin", + io.BytesIO(), ) # Every attempted URL appears in the message, and the first mirror's # exception (the primary URL, usually the one that matters) is chained. From 7c55de311f9c7562aa12ce27c5202a6015e4fa66 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 20 Jul 2026 10:13:56 -1000 Subject: [PATCH 34/40] [wireguard] Mark private keys sensitive, stop redacting public keys (#17736) --- esphome/__main__.py | 12 +++++- esphome/components/wireguard/__init__.py | 4 +- tests/component_tests/wireguard/__init__.py | 1 + tests/component_tests/wireguard/test_init.py | 44 ++++++++++++++++++++ tests/unit_tests/test_main.py | 40 ++++++++++++++++++ 5 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 tests/component_tests/wireguard/__init__.py create mode 100644 tests/component_tests/wireguard/test_init.py diff --git a/esphome/__main__.py b/esphome/__main__.py index 4abd18d239..553a8b390f 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -1510,10 +1510,18 @@ def _redact_with_legacy_fallback(output: str) -> str: m = _LEGACY_REDACTION_RE.search(line) if m is None: continue + key = m.group("key") if not in_substitutions: - unmarked.add(m.group("key")) + # Public keys (e.g. wireguard's peer_public_key) are not secret; + # redacting them and telling maintainers to mark them cv.sensitive + # would be wrong on both counts. Substitution keys are user-named + # with no schema behind them, so anything secret-shaped there + # (public or not) stays conservatively redacted. + if "public" in key.split("_"): + continue + unmarked.add(key) lines[i] = ( - f"{line[: m.start()]}{m.group('key')}: " + f"{line[: m.start()]}{key}: " f"\\033[8m{m.group('val')}\\033[28m{line[m.end() :]}" ) output = "\n".join(lines) diff --git a/esphome/components/wireguard/__init__.py b/esphome/components/wireguard/__init__.py index e128b8476d..31de6639da 100644 --- a/esphome/components/wireguard/__init__.py +++ b/esphome/components/wireguard/__init__.py @@ -63,11 +63,11 @@ CONFIG_SCHEMA = cv.Schema( cv.GenerateID(CONF_TIME_ID): cv.use_id(time.RealTimeClock), cv.Required(CONF_ADDRESS): cv.ipv4address, cv.Optional(CONF_NETMASK, default="255.255.255.255"): cv.ipv4address, - cv.Required(CONF_PRIVATE_KEY): _wireguard_key, + cv.Required(CONF_PRIVATE_KEY): cv.sensitive(_wireguard_key), cv.Required(CONF_PEER_ENDPOINT): cv.string, cv.Required(CONF_PEER_PUBLIC_KEY): _wireguard_key, cv.Optional(CONF_PEER_PORT, default=51820): cv.port, - cv.Optional(CONF_PEER_PRESHARED_KEY): _wireguard_key, + cv.Optional(CONF_PEER_PRESHARED_KEY): cv.sensitive(_wireguard_key), cv.Optional(CONF_PEER_ALLOWED_IPS, default=["0.0.0.0/0"]): cv.ensure_list( _cidr_network ), diff --git a/tests/component_tests/wireguard/__init__.py b/tests/component_tests/wireguard/__init__.py new file mode 100644 index 0000000000..82b57e8fef --- /dev/null +++ b/tests/component_tests/wireguard/__init__.py @@ -0,0 +1 @@ +"""Tests for the wireguard component.""" diff --git a/tests/component_tests/wireguard/test_init.py b/tests/component_tests/wireguard/test_init.py new file mode 100644 index 0000000000..556d14cd00 --- /dev/null +++ b/tests/component_tests/wireguard/test_init.py @@ -0,0 +1,44 @@ +"""Tests for the wireguard component schema.""" + +import pytest + +from esphome.components.wireguard import CONFIG_SCHEMA +from esphome.const import PlatformFramework +from esphome.yaml_util import SensitiveStr +from tests.component_tests.types import SetCoreConfigCallable + +# Any 42 base64 chars plus a valid terminator satisfies _WG_KEY_REGEX. +PRIVATE_KEY = "a" * 42 + "A=" +PEER_PUBLIC_KEY = "b" * 42 + "A=" +PEER_PRESHARED_KEY = "c" * 42 + "A=" + + +@pytest.mark.parametrize( + ("field", "value", "sensitive"), + [ + ("private_key", PRIVATE_KEY, True), + ("peer_preshared_key", PEER_PRESHARED_KEY, True), + ("peer_public_key", PEER_PUBLIC_KEY, False), + ], +) +def test_key_sensitivity( + field: str, + value: str, + sensitive: bool, + set_core_config: SetCoreConfigCallable, +) -> None: + """The private and preshared keys are secrets and must be tagged so dump + tooling redacts them deterministically; the peer's public key is not a + secret and must stay readable in redacted dumps (see issue #17718).""" + set_core_config(PlatformFramework.ESP32_IDF) + config = CONFIG_SCHEMA( + { + "address": "10.0.0.2", + "private_key": PRIVATE_KEY, + "peer_endpoint": "wg.example.com", + "peer_public_key": PEER_PUBLIC_KEY, + "peer_preshared_key": PEER_PRESHARED_KEY, + } + ) + assert isinstance(config[field], SensitiveStr) == sensitive + assert config[field] == value diff --git a/tests/unit_tests/test_main.py b/tests/unit_tests/test_main.py index 9a9aafec43..a1ed89bf5d 100644 --- a/tests/unit_tests/test_main.py +++ b/tests/unit_tests/test_main.py @@ -442,6 +442,46 @@ def test_redact_with_legacy_fallback__does_not_match_fragment_as_suffix( assert not any("legacy substring" in rec.message for rec in caplog.records) +@pytest.mark.parametrize("field", ["public_key", "peer_public_key"]) +def test_redact_with_legacy_fallback__skips_public_key_fields( + field: str, + caplog: pytest.LogCaptureFixture, +) -> None: + """Public keys are not secret; fields with a ``public`` name segment + must pass through unredacted and without the migration warning + (see issue #17718).""" + text = f"{field}: c29tZXB1YmxpY2tleQ==\n" + with caplog.at_level(logging.WARNING, logger="esphome.__main__"): + out = _redact_with_legacy_fallback(text) + assert out == text + assert not any("legacy substring" in rec.message for rec in caplog.records) + + +def test_redact_with_legacy_fallback__public_substitution_still_redacted( + caplog: pytest.LogCaptureFixture, +) -> None: + """Substitution keys are user-named with no schema behind them, so the + public-key exemption does not apply there; a ``public``-named substitution + keeps the conservative silent redaction.""" + text = "substitutions:\n public_key: something\nesphome:\n name: x\n" + with caplog.at_level(logging.WARNING, logger="esphome.__main__"): + out = _redact_with_legacy_fallback(text) + assert "public_key: \\033[8msomething\\033[28m" in out + assert not any("legacy substring" in rec.message for rec in caplog.records) + + +def test_redact_with_legacy_fallback__public_must_be_a_whole_segment( + caplog: pytest.LogCaptureFixture, +) -> None: + """The exemption matches ``public`` as an underscore-separated segment, + not a substring; an unrelated name like ``republic_key`` keeps the + conservative redaction.""" + with caplog.at_level(logging.WARNING, logger="esphome.__main__"): + out = _redact_with_legacy_fallback("republic_key: abc\n") + assert "republic_key: \\033[8mabc\\033[28m" in out + assert any("'republic_key'" in rec.message for rec in caplog.records) + + def test_redact_with_legacy_fallback__substitutions_redacted_without_warning( caplog: pytest.LogCaptureFixture, ) -> None: From f5f1c48f510873da3bc6f049335b727acec88553 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 20 Jul 2026 10:17:01 -1000 Subject: [PATCH 35/40] [esp8266] Fail fast when Rosetta 2 is missing on Apple Silicon Macs (#17737) --- esphome/__main__.py | 7 +++ esphome/components/esp8266/__init__.py | 37 ++++++++++++- tests/unit_tests/components/test_esp8266.py | 61 ++++++++++++++++++++- tests/unit_tests/test_main.py | 37 +++++++++++++ 4 files changed, 138 insertions(+), 4 deletions(-) diff --git a/esphome/__main__.py b/esphome/__main__.py index 553a8b390f..27bb64a4df 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -776,6 +776,13 @@ def compile_program(args: ArgsProtocol, config: ConfigType) -> int: check_placeholder_credentials(config) + # Keep this here, NOT in codegen: config-hash and --only-generate must keep + # working on machines that cannot run the toolchain. + if CORE.is_esp8266: + from esphome.components.esp8266 import check_rosetta + + check_rosetta() + # NOTE: "Build path:" format is parsed by script/ci_memory_impact_extract.py # If you change this format, update the regex in that script as well _LOGGER.info("Compiling app... Build path: %s", CORE.build_path) diff --git a/esphome/components/esp8266/__init__.py b/esphome/components/esp8266/__init__.py index 0e0e2f77d7..7ce10d465d 100644 --- a/esphome/components/esp8266/__init__.py +++ b/esphome/components/esp8266/__init__.py @@ -1,5 +1,6 @@ import logging from pathlib import Path +import platform import re import subprocess @@ -20,9 +21,15 @@ from esphome.const import ( PLATFORM_ESP8266, ThreadModel, ) -from esphome.core import CORE, CoroPriority, Lambda, coroutine_with_priority +from esphome.core import ( + CORE, + CoroPriority, + EsphomeError, + Lambda, + coroutine_with_priority, +) from esphome.core.config import BOARD_MAX_LENGTH -from esphome.helpers import copy_file_if_changed +from esphome.helpers import IS_MACOS, copy_file_if_changed from esphome.types import ConfigType from .boards import BOARDS, ESP8266_LD_SCRIPTS @@ -237,6 +244,32 @@ CONFIG_SCHEMA = cv.All( ) +def check_rosetta() -> None: + """Fail fast when the x86_64 ESP8266 toolchain cannot run on this Mac. + + There is no native arm64 build of the xtensa-lx106 toolchain; on Apple + Silicon it runs under Rosetta 2, which macOS updates can remove. + """ + if not IS_MACOS or platform.machine() != "arm64": + return + try: + result = subprocess.run( + ["/usr/bin/arch", "-x86_64", "/usr/bin/true"], + capture_output=True, + close_fds=False, + check=False, + ) + except OSError: + return # arch(1) unavailable; let the build proceed + if result.returncode != 0: + raise EsphomeError( + "ESP8266 builds on Apple Silicon Macs use an Intel (x86_64) " + "compiler that requires Rosetta 2, which is not installed on " + "this system. Install it with:\n" + " softwareupdate --install-rosetta --agree-to-license" + ) + + @coroutine_with_priority(CoroPriority.PLATFORM) async def to_code(config): cg.add(esp8266_ns.setup_preferences()) diff --git a/tests/unit_tests/components/test_esp8266.py b/tests/unit_tests/components/test_esp8266.py index 318fd2d889..fb0e437d24 100644 --- a/tests/unit_tests/components/test_esp8266.py +++ b/tests/unit_tests/components/test_esp8266.py @@ -1,9 +1,15 @@ """Tests for ESP8266 component.""" +from __future__ import annotations + +from collections.abc import Generator +from unittest.mock import MagicMock, patch + import pytest -from esphome.components.esp8266 import lambdas_use_scanf_float -from esphome.core import Lambda +from esphome.components import esp8266 +from esphome.components.esp8266 import check_rosetta, lambdas_use_scanf_float +from esphome.core import EsphomeError, Lambda from esphome.types import ConfigType @@ -60,3 +66,54 @@ def test_lambdas_use_scanf_float_nested() -> None: """Test detection in deeply nested config.""" config: ConfigType = {"a": {"b": {"c": [Lambda('sscanf(buf, "%f", &v)')]}}} assert lambdas_use_scanf_float(config) is True + + +@pytest.fixture +def apple_silicon_run(monkeypatch: pytest.MonkeyPatch) -> Generator[MagicMock]: + """Simulate an Apple Silicon Mac and yield the mocked subprocess.run.""" + monkeypatch.setattr(esp8266, "IS_MACOS", True) + with ( + patch("esphome.components.esp8266.platform.machine", return_value="arm64"), + patch("esphome.components.esp8266.subprocess.run") as mock_run, + ): + yield mock_run + + +@pytest.mark.parametrize( + ("is_macos", "machine"), + [ + (False, "arm64"), + (True, "x86_64"), + ], +) +def test_check_rosetta_skips_other_systems( + monkeypatch: pytest.MonkeyPatch, is_macos: bool, machine: str +) -> None: + """The check only probes on Apple Silicon Macs.""" + monkeypatch.setattr(esp8266, "IS_MACOS", is_macos) + with ( + patch("esphome.components.esp8266.platform.machine", return_value=machine), + patch("esphome.components.esp8266.subprocess.run") as mock_run, + ): + check_rosetta() + mock_run.assert_not_called() + + +def test_check_rosetta_installed(apple_silicon_run: MagicMock) -> None: + """No error when the x86_64 probe succeeds (Rosetta present).""" + apple_silicon_run.return_value = MagicMock(returncode=0) + check_rosetta() + apple_silicon_run.assert_called_once() + + +def test_check_rosetta_missing(apple_silicon_run: MagicMock) -> None: + """A failing x86_64 probe raises an actionable error.""" + apple_silicon_run.return_value = MagicMock(returncode=1) + with pytest.raises(EsphomeError, match="softwareupdate --install-rosetta"): + check_rosetta() + + +def test_check_rosetta_arch_unavailable(apple_silicon_run: MagicMock) -> None: + """The build proceeds when arch(1) cannot be executed.""" + apple_silicon_run.side_effect = OSError("no such file") + check_rosetta() diff --git a/tests/unit_tests/test_main.py b/tests/unit_tests/test_main.py index a1ed89bf5d..7de11d0568 100644 --- a/tests/unit_tests/test_main.py +++ b/tests/unit_tests/test_main.py @@ -5450,6 +5450,43 @@ def _setup_build_info_test( return build_info_path, firmware_path +def test_compile_program_esp8266_runs_rosetta_check(tmp_path: Path) -> None: + """Test that compile_program runs the Rosetta preflight for ESP8266 targets.""" + setup_core(platform=PLATFORM_ESP8266, tmp_path=tmp_path, name="test_device") + + config: dict[str, Any] = {CONF_ESPHOME: {CONF_NAME: "test_device"}} + args = MockArgs() + + with ( + patch( + "esphome.components.esp8266.check_rosetta", + side_effect=EsphomeError("Rosetta 2 is not installed"), + ) as mock_check, + pytest.raises(EsphomeError, match="Rosetta 2 is not installed"), + ): + compile_program(args, config) + + mock_check.assert_called_once() + + +def test_compile_program_skips_rosetta_check_on_other_platforms( + tmp_path: Path, + mock_compile_build_info_run_compile: Mock, + mock_compile_build_info_get_idedata: Mock, +) -> None: + """Test that the Rosetta preflight does not run for non-ESP8266 targets.""" + _setup_build_info_test(tmp_path, firmware_first=True) + + config: dict[str, Any] = {CONF_ESPHOME: {CONF_NAME: "test_device"}} + args = MockArgs() + + with patch("esphome.components.esp8266.check_rosetta") as mock_check: + result = compile_program(args, config) + + assert result == 0 + mock_check.assert_not_called() + + def test_compile_program_emits_build_info_when_firmware_rebuilt( tmp_path: Path, caplog: pytest.LogCaptureFixture, From a8ebdcb8f22276558dc13bdd0bccf80892569e9e Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Tue, 21 Jul 2026 08:25:51 +1200 Subject: [PATCH 36/40] Bump version to 2026.7.1 --- Doxyfile | 2 +- esphome/const.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doxyfile b/Doxyfile index 46f96b459a..abaaa7b7aa 100644 --- a/Doxyfile +++ b/Doxyfile @@ -48,7 +48,7 @@ PROJECT_NAME = ESPHome # could be handy for archiving the generated documentation or if some version # control system is used. -PROJECT_NUMBER = 2026.7.0 +PROJECT_NUMBER = 2026.7.1 # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer a diff --git a/esphome/const.py b/esphome/const.py index 0b0d3c2e4a..cc44622c86 100644 --- a/esphome/const.py +++ b/esphome/const.py @@ -4,7 +4,7 @@ from enum import Enum from esphome.enum import StrEnum -__version__ = "2026.7.0" +__version__ = "2026.7.1" ALLOWED_NAME_CHARS = "abcdefghijklmnopqrstuvwxyz0123456789-_" VALID_SUBSTITUTIONS_CHARACTERS = ( From 388e41146957353ab25729890e5f47b50a5abe5f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 17 Jul 2026 13:32:47 -1000 Subject: [PATCH 37/40] Bump aioesphomeapi from 45.6.1 to 45.6.2 (#17654) Signed-off-by: dependabot[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index cc081d66f2..7168c8a488 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,7 +12,7 @@ pyserial==3.5 platformio==6.1.19 esptool==5.3.1 click==8.3.3 -aioesphomeapi==45.6.1 +aioesphomeapi==45.6.2 zeroconf==0.150.0 puremagic==2.2.0 ruamel.yaml==0.19.1 # dashboard_import From fc6664d7376ca9132c94bd18bf523768af7280f5 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 15 Jul 2026 21:16:24 -0400 Subject: [PATCH 38/40] [emc2101] Fix negative external temperatures reported as large positives (#17494) --- esphome/components/emc2101/emc2101.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/esphome/components/emc2101/emc2101.cpp b/esphome/components/emc2101/emc2101.cpp index 464f49fe51..f46082f5e7 100644 --- a/esphome/components/emc2101/emc2101.cpp +++ b/esphome/components/emc2101/emc2101.cpp @@ -145,9 +145,9 @@ float Emc2101Component::get_external_temperature() { return NAN; } - // join msb and lsb (5 least significant bits are not used) - uint16_t raw = (msb << 8 | lsb) >> 5; - return raw * 0.125; + // join msb and lsb (5 least significant bits are not used); msb is signed, so read as int16_t + int16_t raw = static_cast((msb << 8) | lsb) >> 5; + return raw * 0.125f; } float Emc2101Component::get_speed() { From 24a8634a4b00aab122a912bb7f051bbfebd9ffbb Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 15 Jul 2026 21:16:38 -0400 Subject: [PATCH 39/40] [haier] Fix outdoor defrost temperature reporting the coil temperature (#17492) --- esphome/components/haier/hon_climate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/haier/hon_climate.cpp b/esphome/components/haier/hon_climate.cpp index f68404afd9..88d446829a 100644 --- a/esphome/components/haier/hon_climate.cpp +++ b/esphome/components/haier/hon_climate.cpp @@ -825,7 +825,7 @@ haier_protocol::HandlerError HonClimate::process_status_message_(const uint8_t * #ifdef USE_SENSOR this->update_sub_sensor_(SubSensorType::INDOOR_COIL_TEMPERATURE, bd_packet->indoor_coil_temperature / 2.0 - 20); this->update_sub_sensor_(SubSensorType::OUTDOOR_COIL_TEMPERATURE, bd_packet->outdoor_coil_temperature - 64); - this->update_sub_sensor_(SubSensorType::OUTDOOR_DEFROST_TEMPERATURE, bd_packet->outdoor_coil_temperature - 64); + this->update_sub_sensor_(SubSensorType::OUTDOOR_DEFROST_TEMPERATURE, bd_packet->outdoor_defrost_temperature - 64); this->update_sub_sensor_(SubSensorType::OUTDOOR_IN_AIR_TEMPERATURE, bd_packet->outdoor_in_air_temperature - 64); this->update_sub_sensor_(SubSensorType::OUTDOOR_OUT_AIR_TEMPERATURE, bd_packet->outdoor_out_air_temperature - 64); this->update_sub_sensor_(SubSensorType::POWER, encode_uint16(bd_packet->power[0], bd_packet->power[1])); From 718b04c15a91122a81025a8726d04a3a3dddb582 Mon Sep 17 00:00:00 2001 From: Guanzhong Chen Date: Thu, 16 Jul 2026 07:57:30 -0400 Subject: [PATCH 40/40] [zephyr] implement ISRInternalGPIOPin::digital_write (#17601) --- esphome/components/zephyr/gpio.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/esphome/components/zephyr/gpio.cpp b/esphome/components/zephyr/gpio.cpp index 1e4201d8f5..23da2cafac 100644 --- a/esphome/components/zephyr/gpio.cpp +++ b/esphome/components/zephyr/gpio.cpp @@ -173,6 +173,14 @@ bool IRAM_ATTR ISRInternalGPIOPin::digital_read() { return bool(gpio_pin_get(arg->gpio, arg->pin % arg->gpio_size) != arg->inverted); } +void IRAM_ATTR ISRInternalGPIOPin::digital_write(bool value) { + auto *arg = (zephyr::ISRPinArg *) this->arg_; + if (arg == nullptr || arg->gpio == nullptr) { + return; + } + gpio_pin_set(arg->gpio, arg->pin % arg->gpio_size, value != arg->inverted ? 1 : 0); +} + } // namespace esphome #endif