diff --git a/esphome/espidf/size_summary.py b/esphome/espidf/size_summary.py index e412d90dbe..cf15832885 100644 --- a/esphome/espidf/size_summary.py +++ b/esphome/espidf/size_summary.py @@ -44,24 +44,20 @@ def _parse_size(token: str) -> int: return int(token) -class _MalformedPartitionRow(ValueError): - """A matched app row whose size cell cannot be parsed; warned, since the - table is present but broken.""" - - -def _find_app_partition_size(partitions_csv: Path) -> int: - """Return the size of the firmware's app partition. +def _find_app_partition_size(partitions_csv: Path) -> int | None: + """The firmware's app partition size; None when there is nothing to find. Mirrors PlatformIO's ``platform-espressif32/builder/main.py:: _update_max_upload_size``: take the first ``app``-type partition whose subtype is ``factory`` or ``ota_0``. Order matters because layouts like Adafruit's ``partitions-4MB-tinyuf2.csv`` repurpose ``factory`` for a UF2 bootloader before the real OTA slot, so a - naive "prefer factory" rule would pick the wrong row. Raises - ``ValueError`` if no qualifying partition is present. + naive "prefer factory" rule would pick the wrong row. A missing + table or no qualifying row is legitimate absence (None); a raise + always means a present-but-broken table. """ if not partitions_csv.is_file(): - raise ValueError(f"partitions.csv not found at {partitions_csv}") + return None for row in csv.reader(partitions_csv.read_text(encoding="utf-8").splitlines()): cells = [c.strip() for c in row] if not cells or cells[0].startswith("#") or len(cells) < 5: @@ -71,10 +67,10 @@ def _find_app_partition_size(partitions_csv: Path) -> int: try: return _parse_size(psize) except ValueError as err: - raise _MalformedPartitionRow( + raise ValueError( f"{err} for partition {cells[0]} in {partitions_csv}" ) from err - raise ValueError(f"No app+factory or app+ota_0 partition in {partitions_csv}") + return None def _format_bar(used: int, total: int) -> str: @@ -102,10 +98,8 @@ def print_summary(size_json: Path, partitions_csv: Path | None) -> None: def _print_summary(size_json: Path, partitions_csv: Path | None) -> None: # The build's own POST_BUILD step writes this file; its absence or an - # unexpected shape is a regression signal, so these skips warn - if not size_json.is_file(): - _LOGGER.warning("Skipping size summary: %s not found", size_json) - return + # unexpected shape is a regression signal, so these skips warn. + # FileNotFoundError lands in the OSError arm with the path in its text. try: data = json.loads(size_json.read_text(encoding="utf-8")) except (OSError, json.JSONDecodeError) as e: @@ -116,12 +110,10 @@ def _print_summary(size_json: Path, partitions_csv: Path | None) -> None: _LOGGER.warning("Skipping size summary: unexpected shape in %s", size_json) return - # Buffered so a late failure prints nothing instead of half a report - lines: list[str] = [] memory_types = data.get("memory_types") if not isinstance(memory_types, dict): memory_types = {} - ram_region = memory_types.get("DRAM") or memory_types.get("DIRAM") or {} + ram_region = memory_types.get("DRAM") or memory_types.get("DIRAM") if not isinstance(ram_region, dict): ram_region = {} ram_used = ram_region.get("used") @@ -131,38 +123,38 @@ def _print_summary(size_json: Path, partitions_csv: Path | None) -> None: and isinstance(ram_total, (int, float)) and ram_total > 0 ): - lines.append(f"RAM: {_format_bar(int(ram_used), int(ram_total))}") + print(f"RAM: {_format_bar(int(ram_used), int(ram_total))}") else: _LOGGER.warning( "Skipping RAM summary: no usable DRAM/DIRAM region in %s", size_json ) - app_size = _resolve_app_size(data, partitions_csv) - if app_size is not None: - lines.append(f"Flash: {_format_bar(data['image_size'], app_size)}") - for line in lines: - print(line) + if (flash := _flash_line(data, partitions_csv)) is not None: + print(flash) -def _resolve_app_size(data: dict, partitions_csv: Path | None) -> int | None: - """The Flash bar's denominator, or None (already logged) to skip it.""" +def _flash_line(data: dict, partitions_csv: Path | None) -> str | None: + """The formatted Flash line, or None (already logged) to skip it. + + Owns both sides of the bar, so nothing after a print can raise: the + blanket guard is left for genuinely unforeseen shapes. + """ image_size = data.get("image_size") - if image_size is None: - _LOGGER.warning("Skipping Flash summary: no image_size in the size report") + if not isinstance(image_size, (int, float)): + _LOGGER.warning("Skipping Flash summary: no usable image_size") return None if partitions_csv is None: _LOGGER.debug("Skipping Flash summary: no partition table given") return None try: app_size = _find_app_partition_size(partitions_csv) - except (_MalformedPartitionRow, OSError) as e: + except (ValueError, OSError) as e: # The table is there but broken/unreadable: visible, like size 0 _LOGGER.warning("Skipping Flash summary: %s", e) return None - except ValueError as e: - # Missing file or no qualifying partition: legitimate for - # non-app layouts, keep quiet - _LOGGER.debug("Skipping Flash summary: %s", e) + if app_size is None: + # No table or no qualifying row: legitimate for non-app layouts + _LOGGER.debug("Skipping Flash summary: no app partition in %s", partitions_csv) return None if app_size <= 0: # Skipping also fails CI's Flash extraction, the right outcome here @@ -172,4 +164,4 @@ def _resolve_app_size(data: dict, partitions_csv: Path | None) -> int | None: partitions_csv, ) return None - return app_size + return f"Flash: {_format_bar(int(image_size), app_size)}" diff --git a/esphome/espidf/toolchain.py b/esphome/espidf/toolchain.py index 07ba03e2cf..f2facd0a4b 100644 --- a/esphome/espidf/toolchain.py +++ b/esphome/espidf/toolchain.py @@ -456,8 +456,8 @@ def run_compile(config, verbose: bool) -> int: rc = run_idf_py(*args, jobs=config[CONF_ESPHOME].get(CONF_COMPILE_PROCESS_LIMIT)) if rc == 0: size_json = CORE.relative_build_path("build", "esp_idf_size.json") - partitions = CORE.relative_build_path("partitions.csv") - print_summary(size_json, partitions if partitions.is_file() else None) + # size_summary owns the missing-table policy + print_summary(size_json, CORE.relative_build_path("partitions.csv")) return rc diff --git a/tests/unit_tests/test_size_summary.py b/tests/unit_tests/test_size_summary.py index c10291b447..c47444fecc 100644 --- a/tests/unit_tests/test_size_summary.py +++ b/tests/unit_tests/test_size_summary.py @@ -70,6 +70,18 @@ def _s3_size_data() -> dict: } +def _dram_size_data(image_size: int = 100) -> dict: + return {"memory_types": {"DRAM": {"used": 1, "size": 2}}, "image_size": image_size} + + +def _write_partitions( + tmp_path: Path, size: str, ptype: str = "app", subtype: str = "ota_0" +) -> Path: + partitions = tmp_path / "partitions.csv" + partitions.write_text(f"app0, {ptype}, {subtype}, 0x10000, {size},\n") + return partitions + + def test_print_summary_esp32_uses_dram( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: @@ -143,12 +155,8 @@ def test_print_summary_unreadable_partitions_is_skipped( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: """An OSError reading the partition table skips the summary, not the build.""" - size_json = tmp_path / "size.json" - size_json.write_text( - '{"memory_types": {"DRAM": {"used": 1, "size": 2}}, "image_size": 100}' - ) - partitions = tmp_path / "partitions.csv" - partitions.write_text("app0, app, ota_0, 0x10000, 1M,\n") + size_json = _write_size_json(tmp_path, _dram_size_data()) + partitions = _write_partitions(tmp_path, "1M") real_read_text = Path.read_text def fail_partitions_read(self: Path, *args: object, **kwargs: object) -> str: @@ -166,12 +174,8 @@ def test_print_summary_zero_app_partition_is_skipped( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: """A 0-size app partition drops the Flash bar instead of rendering 0%.""" - size_json = tmp_path / "size.json" - size_json.write_text( - '{"memory_types": {"DRAM": {"used": 1, "size": 2}}, "image_size": 100}' - ) - partitions = tmp_path / "partitions.csv" - partitions.write_text("app0, app, ota_0, 0x10000, 0,\n") + size_json = _write_size_json(tmp_path, _dram_size_data()) + partitions = _write_partitions(tmp_path, "0") print_summary(size_json, partitions) out = capsys.readouterr().out assert "RAM:" in out and "Flash:" not in out @@ -185,8 +189,7 @@ def test_print_summary_happy_path_prints_both_bars( size_json.write_text( '{"memory_types": {"DRAM": {"used": 1000, "size": 2000}}, "image_size": 100000}' ) - partitions = tmp_path / "partitions.csv" - partitions.write_text("app0, app, ota_0, 0x10000, 0x180000,\n") + partitions = _write_partitions(tmp_path, "0x180000") print_summary(size_json, partitions) out = capsys.readouterr().out assert "RAM:" in out and "Flash:" in out @@ -215,21 +218,35 @@ def test_print_summary_nested_bad_shapes_never_raise( assert "Skipping size summary for" not in caplog.text +def test_print_summary_non_numeric_image_size_warns_by_name( + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + caplog: pytest.LogCaptureFixture, +) -> None: + """A non-numeric image_size hits the named guard, not the blanket.""" + size_json = _write_size_json( + tmp_path, + {"memory_types": {"DRAM": {"used": 1, "size": 2}}, "image_size": "x"}, + ) + print_summary(size_json, _write_partitions(tmp_path, "0x100000")) + assert "Flash:" not in capsys.readouterr().out + assert "no usable image_size" in caplog.text + assert "Skipping size summary for" not in caplog.text + + def test_print_summary_blanket_guard_catches_the_rest( tmp_path: Path, capsys: pytest.CaptureFixture[str], caplog: pytest.LogCaptureFixture, ) -> None: - """Shapes the named guards miss (non-numeric image_size) warn via the - blanket backstop, and the buffered report prints nothing at all.""" - size_json = _write_size_json( - tmp_path, - {"memory_types": {"DRAM": {"used": 1, "size": 2}}, "image_size": "x"}, - ) - partitions = tmp_path / "partitions.csv" - partitions.write_text("app0, app, ota_0, 0x10000, 0x100000,\n") - print_summary(size_json, partitions) - assert capsys.readouterr().out == "" + """A genuinely unforeseen failure warns via the blanket backstop and + never raises past a linked build.""" + size_json = _write_size_json(tmp_path, _dram_size_data()) + with patch( + "esphome.espidf.size_summary._flash_line", + side_effect=RuntimeError("unforeseen"), + ): + print_summary(size_json, None) assert "Skipping size summary for" in caplog.text @@ -239,12 +256,8 @@ def test_print_summary_blank_size_cell_names_the_row( caplog: pytest.LogCaptureFixture, ) -> None: """A blank size cell raises ValueError instead of parsing to 0.""" - size_json = _write_size_json( - tmp_path, - {"memory_types": {"DRAM": {"used": 1, "size": 2}}, "image_size": 100}, - ) - partitions = tmp_path / "partitions.csv" - partitions.write_text("app0, app, ota_0, 0x10000, ,\n") + size_json = _write_size_json(tmp_path, _dram_size_data()) + partitions = _write_partitions(tmp_path, "") print_summary(size_json, partitions) out = capsys.readouterr().out assert "RAM:" in out and "Flash:" not in out @@ -258,10 +271,7 @@ def test_print_summary_suffixed_size_cell( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: """K/M suffixes parse like PlatformIO's rule (1M = 1048576 bytes).""" - size_json = _write_size_json( - tmp_path, - {"memory_types": {"DRAM": {"used": 1, "size": 2}}, "image_size": 100}, - ) + size_json = _write_size_json(tmp_path, _dram_size_data()) partitions = tmp_path / "partitions.csv" partitions.write_text("# comment row\nshort,row\napp0, app, ota_0, 0x10000, 1M,\n") print_summary(size_json, partitions) @@ -275,13 +285,9 @@ def test_print_summary_missing_or_appless_partitions_stay_quiet( ) -> None: """A missing table or one without a qualifying app row is a legitimate layout: the Flash line drops at debug, never at warning.""" - size_json = _write_size_json( - tmp_path, - {"memory_types": {"DRAM": {"used": 1, "size": 2}}, "image_size": 100}, - ) + size_json = _write_size_json(tmp_path, _dram_size_data()) print_summary(size_json, tmp_path / "nope.csv") - partitions = tmp_path / "partitions.csv" - partitions.write_text("st0, data, spiffs, 0x10000, 0x1000,\n") + partitions = _write_partitions(tmp_path, "0x1000", ptype="data", subtype="spiffs") print_summary(size_json, partitions) out = capsys.readouterr().out assert "Flash:" not in out