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

- upload_using_ltchiptool: rename unused config arg to _config and note
  the signature parity with upload_using_platformio/upload_using_picotool
  so dispatch stays symmetric.
- upload_program: explicit exit_code = 1 when _rp2040_serial_reset_to_bootsel
  fails, instead of relying on the function-level default.
- upload_using_picotool: distinct error message for the prebuilt-dir case
  that names both candidates (idedata ELF + prebuilt firmware) instead of
  just pointing at the missing ELF.
- _load_idedata: document the prebuilt-dir idedata.json contract more
  loudly (absolute paths under prebuilt_dir, no schema validation, dashboard
  owns the rewrites).
- --prebuilt-dir help text: drop the placeholder docs URL; describe the
  dashboard-internal intent inline so users who hit the flag in --help
  understand they don't want it.
- New test: upload_using_esptool with ESP-IDF toolchain + --prebuilt-dir
  uses <prebuilt-dir>/firmware.factory.bin at offset 0x0.

Issue: esphome/device-builder#572
This commit is contained in:
J. Nick Koston
2026-05-10 23:18:48 -05:00
parent d96ad02b9f
commit 0adfd08270
3 changed files with 67 additions and 10 deletions
+24 -7
View File
@@ -966,7 +966,7 @@ def upload_using_esptool(
return run_esptool(115200)
def upload_using_ltchiptool(config: ConfigType, port: str) -> int:
def upload_using_ltchiptool(_config: ConfigType, port: str) -> int:
"""Upload a libretiny ``.uf2`` directly via ``ltchiptool flash write``.
Bypasses PlatformIO so the dashboard's transparent install can flash a
@@ -978,6 +978,10 @@ def upload_using_ltchiptool(config: ConfigType, port: str) -> int:
Chip family is auto-detected from the UF2 header so we don't need to
plumb through ``CORE.data[KEY_LIBRETINY][KEY_FAMILY]``.
The ``_config`` parameter is unused but kept for signature parity with
``upload_using_platformio`` / ``upload_using_picotool`` so the dispatch
in ``upload_program`` can stay symmetric.
"""
firmware = CORE.firmware_bin
if not firmware.is_file():
@@ -1060,6 +1064,14 @@ def upload_using_picotool(config: ConfigType) -> int:
firmware_file = elf_path
elif CORE.prebuilt_dir is not None and CORE.firmware_bin.is_file():
firmware_file = CORE.firmware_bin
elif CORE.prebuilt_dir is not None:
_LOGGER.error(
"No firmware found for picotool: neither %s (from idedata) nor "
"%s (prebuilt) exists.",
elf_path,
CORE.firmware_bin,
)
return 1
else:
_LOGGER.error(
"Firmware ELF file not found at %s. "
@@ -1290,6 +1302,12 @@ def upload_program(
# PlatformIO path is preserved for back-compat.
if _rp2040_serial_reset_to_bootsel(host):
exit_code = upload_using_picotool(config)
else:
# Touch reset failed: the helper already logged a specific
# error (port open failure, picotool missing, or BOOTSEL
# never enumerated). Be explicit about the failed exit code
# rather than relying on the function-level default of 1.
exit_code = 1
elif CORE.target_platform == PLATFORM_RP2040 or CORE.is_libretiny:
exit_code = upload_using_platformio(config, host)
# else: Unknown target platform, exit_code remains 1
@@ -2260,12 +2278,11 @@ def parse_args(argv):
parser_upload.add_argument(
"--prebuilt-dir",
help=(
"Directory of prebuilt artifacts to flash instead of re-deriving "
"paths from the local build tree. The configuration is still read "
"(to load OTA settings, target platform, esp32 variant, etc.) but "
"the upload path reads bytes from this directory. Layout is "
"documented at "
"https://developers.esphome.io/architecture/upload-prebuilt-dir/."
"Advanced: directory of prebuilt artifacts to flash instead of "
"re-deriving paths from the local build tree. Intended for the "
"ESPHome dashboard's transparent install flow on hosts that "
"don't compile firmware themselves. End users should not need "
"this flag."
),
)
parser_upload.add_argument(
+8 -3
View File
@@ -104,9 +104,14 @@ def _run_idedata(config):
def _load_idedata(config):
# `esphome upload --prebuilt-dir` ships a pre-rendered idedata.json next
# to the artifacts. When present we use it verbatim: ``firmware_bin_path``
# and ``extra.flash_images[*].path`` already point at absolute paths under
# the prebuilt directory, so the esptool / picotool helpers find the right
# bytes without re-running PlatformIO or consulting the local build tree.
# (i.e. ``prog_path`` with the ``.bin`` suffix), ``firmware_elf_path``,
# ``cc_path`` and ``extra.flash_images[*].path`` are expected to be
# absolute paths that resolve under ``CORE.prebuilt_dir``. The dashboard
# is responsible for rewriting those paths when it stages the directory.
# No schema validation or referenced-path existence check happens here;
# a malformed prebuilt idedata.json will surface as a downstream
# "file not found" from esptool / picotool. That's an acceptable trade
# for keeping this path zero-cost when the dashboard's contract is met.
if CORE.prebuilt_dir is not None:
prebuilt_idedata = CORE.prebuilt_dir / "idedata.json"
if prebuilt_idedata.is_file():
+35
View File
@@ -1343,6 +1343,41 @@ def test_upload_using_esptool_with_file_path(
assert firmware_path.endswith("custom_firmware.bin")
def test_upload_using_esptool_idf_prebuilt_factory_bin(
tmp_path: Path,
mock_run_external_command_main: Mock,
) -> None:
"""ESP-IDF builds flash a single ``firmware.factory.bin`` (bootloader +
partitions + app bundled) at offset 0x0. With --prebuilt-dir set the
esptool helper must pick up ``<prebuilt-dir>/firmware.factory.bin``
instead of ``CORE.relative_build_path("build", "firmware.factory.bin")``
so the dashboard can flash an ESP-IDF device without a local build."""
setup_core(platform=PLATFORM_ESP32, tmp_path=tmp_path, name="test")
CORE.toolchain = Toolchain.ESP_IDF
CORE.data[KEY_ESP32] = {KEY_VARIANT: VARIANT_ESP32}
prebuilt = tmp_path / "prebuilt"
prebuilt.mkdir()
factory_bin = prebuilt / "firmware.factory.bin"
factory_bin.write_bytes(b"\x00" * 1024)
CORE.prebuilt_dir = prebuilt
config = {CONF_ESPHOME: {"platformio_options": {}}}
result = upload_using_esptool(config, "/dev/ttyUSB0", None, None)
assert result == 0
cmd_list = list(mock_run_external_command_main.call_args[0][1:])
# ESP-IDF prebuilt: single image at offset 0x0 pointing at the prebuilt
# factory file, not the local build tree's <build>/firmware.factory.bin.
write_flash_idx = cmd_list.index("write-flash")
offset_idx = write_flash_idx + 4
assert cmd_list[offset_idx] == "0x0"
path_arg = cmd_list[offset_idx + 1]
assert path_arg == str(factory_bin)
@pytest.mark.parametrize(
"platform,device",
[