mirror of
https://github.com/esphome/esphome.git
synced 2026-09-24 05:24:14 +00:00
[cli] Address Copilot review on --prebuilt-dir
- get_ltchiptool_path: use 'Scripts/' on Windows, 'bin/' elsewhere when falling back to PlatformIO's libretiny penv. CPython venvs put scripts under Scripts/ on win32 and bin/ on POSIX; the prior hardcoded 'bin' would never have found ltchiptool on Windows. - _load_idedata: wrap json.loads on the prebuilt idedata.json in a try/except and re-raise as EsphomeError with a one-line diagnostic so the failure mode is a clean error instead of an unhandled JSONDecodeError stack trace. Update the surrounding comment to match the new behavior. - CORE.prebuilt_dir docstring: drop the dead 'docs/architecture/...' pointer (no docs/ tree in this repo); point at esphome-docs#6600 and device-builder#572 instead. New regression test: - test_load_idedata_prebuilt_malformed_json_raises_esphomeerror Updated test: - test_get_ltchiptool_path_pio_penv now uses Scripts/ on win32 to match the new platform-aware lookup. Issue: esphome/device-builder#572 Issue: esphome/device-builder#570
This commit is contained in:
@@ -570,7 +570,9 @@ class EsphomeCore:
|
||||
# Directory of prebuilt artifacts for `esphome upload --prebuilt-dir`.
|
||||
# When set, firmware/partition/bootloader resolution and the idedata
|
||||
# cache prefer files under this directory over the local build tree.
|
||||
# See docs/architecture/upload-prebuilt-dir.
|
||||
# User-facing docs live in esphome/esphome-docs#6600
|
||||
# (`guides/cli.mdx`); see esphome/device-builder#572 for the
|
||||
# cross-platform layout contract that motivates the flag.
|
||||
self.prebuilt_dir: Path | None = None
|
||||
# The validated configuration, this is None until the config has been validated
|
||||
self.config: ConfigType | None = None
|
||||
|
||||
@@ -161,13 +161,21 @@ def _load_idedata(config):
|
||||
# idedata.json with absolute paths on every install.
|
||||
#
|
||||
# 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.
|
||||
# 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.
|
||||
if CORE.prebuilt_dir is not None:
|
||||
prebuilt_idedata = CORE.prebuilt_dir / "idedata.json"
|
||||
if prebuilt_idedata.is_file():
|
||||
data = json.loads(prebuilt_idedata.read_text(encoding="utf-8"))
|
||||
try:
|
||||
data = json.loads(prebuilt_idedata.read_text(encoding="utf-8"))
|
||||
except json.JSONDecodeError as err:
|
||||
raise EsphomeError(
|
||||
f"Failed to parse {prebuilt_idedata}: {err}. The dashboard "
|
||||
"must stage a syntactically valid idedata.json under "
|
||||
"--prebuilt-dir; the upload cannot proceed without it."
|
||||
) from err
|
||||
_resolve_prebuilt_idedata_paths(data, CORE.prebuilt_dir)
|
||||
return data
|
||||
|
||||
|
||||
+11
-4
@@ -413,8 +413,10 @@ def get_ltchiptool_path() -> Path | None:
|
||||
|
||||
Order:
|
||||
1. ``ltchiptool`` on PATH (pip-installed system-wide or in a virtualenv).
|
||||
2. PlatformIO's libretiny penv at ``~/.platformio/penv/.libretiny/bin``
|
||||
(where platform-libretiny installs it during its package init).
|
||||
2. PlatformIO's libretiny penv (where platform-libretiny installs it
|
||||
during its package init). The script subdirectory follows the
|
||||
CPython venv convention: ``Scripts/`` on Windows, ``bin/``
|
||||
elsewhere.
|
||||
|
||||
Returns None if neither is available; callers should surface an
|
||||
actionable error pointing the user at one of those install paths.
|
||||
@@ -422,13 +424,18 @@ def get_ltchiptool_path() -> Path | None:
|
||||
on_path = shutil.which("ltchiptool")
|
||||
if on_path is not None:
|
||||
return Path(on_path)
|
||||
binary_name = "ltchiptool.exe" if sys.platform == "win32" else "ltchiptool"
|
||||
if sys.platform == "win32":
|
||||
bin_subdir = "Scripts"
|
||||
binary_name = "ltchiptool.exe"
|
||||
else:
|
||||
bin_subdir = "bin"
|
||||
binary_name = "ltchiptool"
|
||||
pio_penv = (
|
||||
Path.home()
|
||||
/ ".platformio"
|
||||
/ "penv"
|
||||
/ LTCHIPTOOL_PIO_PENV_NAME
|
||||
/ "bin"
|
||||
/ bin_subdir
|
||||
/ binary_name
|
||||
)
|
||||
if pio_penv.is_file():
|
||||
|
||||
@@ -336,6 +336,26 @@ def test_load_idedata_absolute_paths_in_prebuilt_pass_through(
|
||||
assert result["extra"]["flash_images"][0]["path"] == abs_bootloader
|
||||
|
||||
|
||||
def test_load_idedata_prebuilt_malformed_json_raises_esphomeerror(
|
||||
setup_core: Path, mock_run_platformio_cli_run: Mock
|
||||
) -> None:
|
||||
"""A malformed prebuilt idedata.json must surface as a one-line
|
||||
EsphomeError, not an unhandled JSONDecodeError stack trace; the
|
||||
dashboard's transparent-install flow needs a clean diagnostic to
|
||||
bubble back to the operator, not the contents of the broken file."""
|
||||
CORE.build_path = str(setup_core / "build" / "test")
|
||||
CORE.name = "test"
|
||||
|
||||
prebuilt_dir = setup_core / "prebuilt"
|
||||
prebuilt_dir.mkdir()
|
||||
(prebuilt_dir / "idedata.json").write_text("{not valid json")
|
||||
CORE.prebuilt_dir = prebuilt_dir
|
||||
|
||||
with pytest.raises(EsphomeError, match="Failed to parse"):
|
||||
toolchain._load_idedata({"name": "test"})
|
||||
mock_run_platformio_cli_run.assert_not_called()
|
||||
|
||||
|
||||
def test_load_idedata_falls_back_when_prebuilt_idedata_missing(
|
||||
setup_core: Path, mock_run_platformio_cli_run: Mock
|
||||
) -> None:
|
||||
|
||||
@@ -595,10 +595,18 @@ def test_get_ltchiptool_path_on_path(tmp_path: Path) -> None:
|
||||
|
||||
def test_get_ltchiptool_path_pio_penv(tmp_path: Path) -> None:
|
||||
"""Fall back to PlatformIO's libretiny penv when ltchiptool isn't on
|
||||
PATH; this is the install location platform-libretiny uses."""
|
||||
PATH; this is the install location platform-libretiny uses. The
|
||||
script subdir follows the CPython venv convention -- ``Scripts/``
|
||||
on Windows, ``bin/`` elsewhere -- which matches what PlatformIO
|
||||
creates."""
|
||||
fake_home = tmp_path / "home"
|
||||
binary_name = "ltchiptool.exe" if sys.platform == "win32" else "ltchiptool"
|
||||
pio_bin = fake_home / ".platformio" / "penv" / ".libretiny" / "bin"
|
||||
if sys.platform == "win32":
|
||||
bin_subdir = "Scripts"
|
||||
binary_name = "ltchiptool.exe"
|
||||
else:
|
||||
bin_subdir = "bin"
|
||||
binary_name = "ltchiptool"
|
||||
pio_bin = fake_home / ".platformio" / "penv" / ".libretiny" / bin_subdir
|
||||
pio_bin.mkdir(parents=True)
|
||||
expected = pio_bin / binary_name
|
||||
expected.touch()
|
||||
|
||||
Reference in New Issue
Block a user