[cli] Address self-review on --prebuilt-dir

- CORE.firmware_bin priority is now platform-aware: RP2040 + libretiny
  prefer firmware.uf2 over firmware.bin (picotool / ltchiptool need the
  UF2 header for address/family info); ESP* prefer firmware.bin. The
  prior code returned .bin first for all platforms, which would have
  silently flashed the wrong artifact on RP2040 if a hand-staged dir
  shipped both files. New tests guard both directions.

- _rp2040_serial_reset_to_bootsel: check picotool exists *before*
  triggering the 1200bps touch. If picotool is missing, the touch
  would have left the device stranded in BOOTSEL with nothing able to
  flash it; with this order the device stays on the old firmware and
  can be retried.

- upload_using_ltchiptool: error message now mentions both firmware.uf2
  and firmware.bin since CORE.firmware_bin resolves either.

- prepare_platform_for_upload: return type tightened to int (capture_stdout
  is hardcoded False; assert the run helper returns int so a future caller
  that flips capture_stdout fails loudly instead of silently treating a
  string as success). Caller in __main__.py is now a one-liner.

- _load_idedata: narrative comment in the prebuilt branch shortened.
  _resolve_prebuilt_idedata_paths docstring now notes the
  POSIX-absolute-on-Windows quirk ("/foo/bar" is rooted but not absolute
  on win32; hand-staged dirs need OS-appropriate absolute paths).

- New defensive-coverage tests: _resolve_prebuilt_idedata_paths with
  missing prog_path, no extra section, empty flash_images list.

Issue: esphome/device-builder#572
Issue: esphome/device-builder#570

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(

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:

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():

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

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: