diff --git a/esphome/__main__.py b/esphome/__main__.py index 4ce40d48a1a..84e55294d66 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -959,8 +959,10 @@ def upload_using_ltchiptool(_config: ConfigType, port: str) -> int: if not firmware.is_file(): _LOGGER.error( "LibreTiny firmware file not found at %s. " - "Make sure the project has been compiled first or that " - "--prebuilt-dir points at a directory containing firmware.uf2.", + "Make sure the project has been compiled first, or that " + "--prebuilt-dir points at a directory containing firmware.uf2 " + "(or firmware.bin -- libretiny emits the same UF2 content under " + "both names).", firmware, ) return 1 @@ -1110,6 +1112,19 @@ def _rp2040_serial_reset_to_bootsel(port: str, timeout: float = 10.0) -> bool: """ import serial + # Look picotool up *before* triggering the reset. If it's missing, the + # touch would leave the device stranded in BOOTSEL with nothing able to + # flash it; bail out early so the user can recover (the device is still + # running the old firmware and re-enumerates as the same serial port). + picotool = _find_picotool() + if picotool is None: + _LOGGER.error( + "picotool not found; cannot flash RP2040 after BOOTSEL reset. " + "Ensure the RP2040 PlatformIO platform is installed (%s).", + PICOTOOL_PACKAGE, + ) + return False + _LOGGER.info("Rebooting %s into BOOTSEL via 1200bps touch...", port) try: ser = serial.Serial() @@ -1125,15 +1140,6 @@ def _rp2040_serial_reset_to_bootsel(port: str, timeout: float = 10.0) -> bool: _LOGGER.error("Failed to open %s at 1200 baud for BOOTSEL reset: %s", port, err) return False - picotool = _find_picotool() - if picotool is None: - _LOGGER.error( - "picotool not found; cannot detect BOOTSEL after 1200bps touch. " - "Ensure the RP2040 PlatformIO platform is installed (%s).", - PICOTOOL_PACKAGE, - ) - return False - start = time.monotonic() while time.monotonic() - start < timeout: if detect_rp2040_bootsel(picotool).device_count > 0: @@ -1247,10 +1253,7 @@ def _ensure_platform_packages_for_prebuilt_upload(config: ConfigType) -> int: from esphome.platformio import toolchain - result = toolchain.prepare_platform_for_upload(config, CORE.verbose) - # prepare_platform_for_upload returns str on capture_stdout=True or int - # on success/failure; in our call we don't capture stdout, so it's int. - return result if isinstance(result, int) else 0 + return toolchain.prepare_platform_for_upload(config, CORE.verbose) def upload_program( diff --git a/esphome/core/__init__.py b/esphome/core/__init__.py index 7bbd225589d..de9b4837c24 100644 --- a/esphome/core/__init__.py +++ b/esphome/core/__init__.py @@ -799,11 +799,16 @@ class EsphomeCore: @property def firmware_bin(self) -> Path: - # Prebuilt override: prefer firmware.bin then firmware.uf2 so the - # dashboard can ship a single canonical name per platform. - if ( - prebuilt := self.prebuilt_artifact_path("firmware.bin", "firmware.uf2") - ) is not None: + # Prebuilt override priority is platform-aware: on RP2040 and libretiny + # the canonical flash artifact is the UF2 (raw firmware.bin won't have + # the address/family header picotool / ltchiptool need); on ESP* it's + # firmware.bin. When the dashboard ships only the canonical name, the + # other entry is a harmless no-op. + if self.is_rp2040 or self.is_libretiny: + prebuilt_names = ("firmware.uf2", "firmware.bin") + else: + prebuilt_names = ("firmware.bin", "firmware.uf2") + if (prebuilt := self.prebuilt_artifact_path(*prebuilt_names)) is not None: return prebuilt # Check if using ESP-IDF toolchain if self.using_toolchain_esp_idf: diff --git a/esphome/platformio/toolchain.py b/esphome/platformio/toolchain.py index f86bf75781c..339854dd00b 100644 --- a/esphome/platformio/toolchain.py +++ b/esphome/platformio/toolchain.py @@ -84,26 +84,34 @@ def run_compile(config, verbose): return run_platformio_cli_run(config, verbose, *args) -def prepare_platform_for_upload(config, verbose) -> str | int: +def prepare_platform_for_upload(config, verbose) -> int: """Configure the PlatformIO build environment for ``CORE.name`` without compiling, so platform-specific flashing tools end up on disk. Used by ``esphome upload --prebuilt-dir`` on hosts that have never - compiled the target platform locally. ``pio pkg install`` alone isn't - enough on libretiny: the platform package downloads cleanly, but - ``ltchiptool`` lives in a platform-managed virtualenv at - ``~/.platformio/penv/.libretiny/`` that's only created by libretiny's - ``ConfigurePythonVenv`` SCons step, which runs during ``pio run`` (not - ``pio pkg install``). So we run ``pio run -t idedata`` instead: it - triggers SConscript -- creating the penv on libretiny, installing the - picotool tool package on RP2040 -- but skips the actual compile target - so this is much cheaper than a full build. + compiled the target platform locally. Runs ``pio run -t idedata``: the + target fires SConscript (which on libretiny triggers + ``ConfigurePythonVenv`` and pip-installs ``ltchiptool`` into the + platform's virtualenv, and on RP2040 installs the picotool tool + package) but skips the actual compile, so this is much cheaper than a + full build. The idedata JSON gets emitted to stdout as a side effect of the - target; we don't filter it out -- the install runs once per cold host - and the trailing JSON blob is harmless noise. + target; we don't filter it out because the install runs once per cold + host and the trailing JSON blob is harmless noise. + + Returns the subprocess exit code; non-zero means the platform install + failed and the upload caller should abort. """ - return run_platformio_cli_run(config, verbose, "-t", "idedata") + # ``capture_stdout`` is intentionally False so run_platformio_cli_run + # returns the int exit code (str only when capture_stdout=True). + result = run_platformio_cli_run(config, verbose, "-t", "idedata") + # Belt-and-suspenders: assert we got an int rather than silently + # treating a captured stdout string as success. + assert isinstance(result, int), ( + f"prepare_platform_for_upload expected int, got {type(result).__name__}" + ) + return result def _run_idedata(config): @@ -130,14 +138,19 @@ def _resolve_prebuilt_idedata_paths(data: dict, prebuilt_dir: Path) -> None: ``extra.flash_images[*].path`` to bare basenames before shipping the tarball (the receiver's build-host absolute paths don't resolve on the offloader). Accept both shapes: absolute paths pass through unchanged - so a hand-built --prebuilt-dir with absolute paths still works, and - bare basenames or other relative paths resolve to ``prebuilt_dir / p``. + so a hand-built ``--prebuilt-dir`` still works, and basenames / other + relative paths resolve to ``prebuilt_dir / p``. Mutates ``data`` in + place. - Mutates ``data`` in place. ``cc_path`` is left alone because it points - at a PlatformIO toolchain binary (~/.platformio/packages/...) that - lives outside the prebuilt dir; the offloader's local PIO install - provides the matching binary by virtue of running on the same machine - as ``esphome upload``. + ``cc_path`` is left alone because it points at a PlatformIO toolchain + binary (``~/.platformio/packages/...``) that lives outside the prebuilt + dir; the offloader's local PIO install provides the matching binary. + + Absoluteness follows ``pathlib.Path.is_absolute`` semantics: on Windows + a leading slash without a drive letter (e.g. ``/foo/bar``) is rooted + but **not** absolute, and will be resolved against ``prebuilt_dir``. + Hand-staged directories should use OS-appropriate absolute paths if + they want passthrough. """ prog = data.get("prog_path") if prog is not None and not Path(prog).is_absolute(): @@ -152,19 +165,12 @@ def _resolve_prebuilt_idedata_paths(data: dict, prebuilt_dir: Path) -> None: def _load_idedata(config): - # `esphome upload --prebuilt-dir` ships a pre-rendered idedata.json next - # to the artifacts. When present we use it as-is, with one rewrite pass: - # ``prog_path`` and ``extra.flash_images[*].path`` may be either absolute - # paths (hand-built directories) or bare basenames (the dashboard's wire - # format) -- relative paths are resolved against ``CORE.prebuilt_dir`` so - # both shapes work without the dashboard having to write a fresh - # idedata.json with absolute paths on every install. - # - # No schema validation or referenced-path existence check happens here; - # a missing path inside the idedata will surface later as a "file not - # found" from esptool / picotool. A malformed JSON file is caught here - # and surfaced as EsphomeError so the failure mode is a one-line - # diagnostic instead of an unhandled JSONDecodeError stack trace. + # --prebuilt-dir override: read the shipped idedata.json, resolve any + # relative paths inside it against the prebuilt dir, return it directly. + # Malformed JSON -> EsphomeError (clean one-line diagnostic, not a raw + # JSONDecodeError trace). No schema or path-existence validation; any + # missing artifact surfaces later as a "file not found" from the + # downstream flasher. if CORE.prebuilt_dir is not None: prebuilt_idedata = CORE.prebuilt_dir / "idedata.json" if prebuilt_idedata.is_file(): diff --git a/tests/unit_tests/test_core.py b/tests/unit_tests/test_core.py index 21a2f44986d..39736a1651d 100644 --- a/tests/unit_tests/test_core.py +++ b/tests/unit_tests/test_core.py @@ -929,6 +929,7 @@ class TestEsphomeCore: path.""" target.name = "test-device" target.toolchain = const.Toolchain.PLATFORMIO + target.data[const.KEY_CORE] = {const.KEY_TARGET_PLATFORM: "esp32"} target.prebuilt_dir = tmp_path (tmp_path / "firmware.bin").write_bytes(b"fw") @@ -939,11 +940,40 @@ class TestEsphomeCore: firmware.bin is absent so the dashboard only needs to ship one file.""" target.name = "test-device" target.toolchain = const.Toolchain.PLATFORMIO + target.data[const.KEY_CORE] = {const.KEY_TARGET_PLATFORM: "bk72xx"} target.prebuilt_dir = tmp_path (tmp_path / "firmware.uf2").write_bytes(b"uf2") assert target.firmware_bin == tmp_path / "firmware.uf2" + def test_firmware_bin__prebuilt_prefers_uf2_on_rp2040(self, target, tmp_path): + """Critical for RP2040: when both firmware.bin and firmware.uf2 are + staged, return the .uf2. picotool's load command needs the UF2 + header (address + family); a raw .bin would flash to the wrong + offset (or refuse to flash). The dashboard's wire format ships only + the canonical name, but a hand-staged dir might carry both.""" + target.name = "test-device" + target.toolchain = const.Toolchain.PLATFORMIO + target.data[const.KEY_CORE] = {const.KEY_TARGET_PLATFORM: const.PLATFORM_RP2040} + target.prebuilt_dir = tmp_path + (tmp_path / "firmware.bin").write_bytes(b"raw") + (tmp_path / "firmware.uf2").write_bytes(b"uf2-wrapped") + + assert target.firmware_bin == tmp_path / "firmware.uf2" + + def test_firmware_bin__prebuilt_prefers_bin_on_esp32(self, target, tmp_path): + """Mirror of the above for ESP32: the canonical artifact is .bin + (esptool flashes raw images at fixed offsets); if both are shipped + the .uf2 is the irrelevant one.""" + target.name = "test-device" + target.toolchain = const.Toolchain.PLATFORMIO + target.data[const.KEY_CORE] = {const.KEY_TARGET_PLATFORM: "esp32"} + target.prebuilt_dir = tmp_path + (tmp_path / "firmware.bin").write_bytes(b"raw") + (tmp_path / "firmware.uf2").write_bytes(b"uf2-wrapped") + + assert target.firmware_bin == tmp_path / "firmware.bin" + def test_partition_table_bin__prebuilt_override(self, target, tmp_path): target.name = "test-device" target.toolchain = const.Toolchain.PLATFORMIO diff --git a/tests/unit_tests/test_platformio_toolchain.py b/tests/unit_tests/test_platformio_toolchain.py index 5be37073072..73a291fef05 100644 --- a/tests/unit_tests/test_platformio_toolchain.py +++ b/tests/unit_tests/test_platformio_toolchain.py @@ -340,6 +340,37 @@ def test_load_idedata_absolute_paths_in_prebuilt_pass_through( assert result["extra"]["flash_images"][0]["path"] == abs_bootloader +def test_resolve_prebuilt_idedata_paths_missing_prog_path(tmp_path: Path) -> None: + """Defensive: a prebuilt idedata.json without prog_path (e.g. when the + dashboard only ships extra.flash_images) must not raise -- the resolver + skips fields that aren't present so partial-idedata shapes don't crash + upload dispatch.""" + data = {"extra": {"flash_images": []}, "cc_path": "/some/cc"} + toolchain._resolve_prebuilt_idedata_paths(data, tmp_path) + assert "prog_path" not in data + assert data["cc_path"] == "/some/cc" # left alone + + +def test_resolve_prebuilt_idedata_paths_no_extra_section(tmp_path: Path) -> None: + """Defensive: idedata.json without an `extra` section at all (some + platforms don't ship flash_images) must not crash.""" + data = {"prog_path": "firmware.elf"} + toolchain._resolve_prebuilt_idedata_paths(data, tmp_path) + assert data["prog_path"] == str(tmp_path / "firmware.elf") + + +def test_resolve_prebuilt_idedata_paths_empty_flash_images(tmp_path: Path) -> None: + """Defensive: empty extra.flash_images (libretiny / ESP8266 / nRF52 ship + this; whole image is one file at offset 0x0). Resolver must not raise.""" + data = { + "prog_path": "firmware.elf", + "extra": {"flash_images": []}, + } + toolchain._resolve_prebuilt_idedata_paths(data, tmp_path) + assert data["prog_path"] == str(tmp_path / "firmware.elf") + assert data["extra"]["flash_images"] == [] + + def test_load_idedata_prebuilt_malformed_json_raises_esphomeerror( setup_core: Path, mock_run_platformio_cli_run: Mock ) -> None: