Merge branch 'esp8266-native-converter-hardening' into esp8266-native-library-converter

esphome/espidf/size_summary.py

@@ -46,92 +46,122 @@ def _parse_size(token: str) -> int:
     return int(token)
 
 
-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:
             continue
         ptype, psubtype, psize = cells[1], cells[2], cells[4]
         if ptype in ("app", "0") and psubtype in ("factory", "ota_0"):
-            return _parse_size(psize)
-    raise ValueError(f"No app+factory or app+ota_0 partition in {partitions_csv}")
+            try:
+                return _parse_size(psize)
+            except ValueError as err:
+                raise ValueError(
+                    f"{err} for partition {cells[0]} in {partitions_csv}"
+                ) from err
+    return None
 
 
 def print_summary(size_json: Path, partitions_csv: Path | None) -> None:
-    """Print PlatformIO-shaped RAM and Flash one-liners.
-
-    Failures are non-fatal: the build has already succeeded, we just couldn't
-    summarize. Logs the cause at warning level, so a missing RAM/Flash line
-    (which CI's memory-impact extraction greps for) is diagnosable.
-    """
+    """Print PlatformIO-shaped RAM and Flash one-liners; never fails the build."""
     try:
         _print_summary(size_json, partitions_csv)
     except Exception as e:  # noqa: BLE001  # pylint: disable=broad-exception-caught
-        # Backstop for shapes the named guards below miss
-        _LOGGER.warning("Skipping size summary: %s", e)
+        # Backstop for shapes the named guards below miss; warning so a
+        # regression here cannot go missing indefinitely
+        _LOGGER.warning(
+            "Skipping size summary for %s: %s: %s", size_json, type(e).__name__, e
+        )
         _LOGGER.debug("Size summary failure detail", exc_info=True)
 
 
 def _print_summary(size_json: Path, partitions_csv: Path | None) -> None:
-    if not size_json.is_file():
-        _LOGGER.warning("Skipping size summary: %s not found", size_json)
-        return
+    # The build's own POST_BUILD step writes this file; its absence or an
+    # 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:
         _LOGGER.warning("Skipping size summary: %s", e)
         return
-
     if not isinstance(data, dict):
-        # Valid JSON that is not an object (truncated tool output) must
-        # not raise past a build that already linked
+        # Non-object JSON has no .get
         _LOGGER.warning("Skipping size summary: unexpected shape in %s", size_json)
         return
 
+    if (ram := _ram_bar(data, size_json)) is not None:
+        print_size_line("RAM", *ram)
+    if (flash := _flash_bar(data, partitions_csv)) is not None:
+        print_size_line("Flash", *flash)
+
+
+def _dict_get(mapping: object, key: str) -> object:
+    """dict.get that reads None from any non-dict."""
+    return mapping.get(key) if isinstance(mapping, dict) else None
+
+
+def _present_but_not_dict(value: object) -> bool:
+    return value is not None and not isinstance(value, dict)
+
+
+def _is_number(value: object) -> bool:
+    return isinstance(value, (int, float))
+
+
+def _ram_bar(data: dict, size_json: Path) -> tuple[int, int] | None:
+    """The RAM bar's (used, total), or None (already logged) to skip it."""
     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 {}
-    if not isinstance(ram_region, dict):
-        ram_region = {}
-    ram_used = ram_region.get("used")
-    ram_total = ram_region.get("size")
-    # Numeric checks isolate a malformed RAM region from the Flash line below
-    if (
-        isinstance(ram_used, (int, float))
-        and isinstance(ram_total, (int, float))
-        and ram_total > 0
-    ):
-        print_size_line("RAM", int(ram_used), int(ram_total))
+    ram_region = _dict_get(memory_types, "DRAM") or _dict_get(memory_types, "DIRAM")
+    used = _dict_get(ram_region, "used")
+    total = _dict_get(ram_region, "size")
+    if _is_number(used) and _is_number(total) and total > 0:
+        return int(used), int(total)
+    if _present_but_not_dict(memory_types) or _present_but_not_dict(ram_region):
+        # A structurally corrupt report, not a variant without the region
+        _LOGGER.warning("Skipping RAM summary: malformed memory_types in %s", size_json)
     else:
         _LOGGER.warning(
             "Skipping RAM summary: no usable DRAM/DIRAM region in %s", size_json
         )
+    return None
 
+
+def _flash_bar(data: dict, partitions_csv: Path | None) -> tuple[int, int] | None:
+    """The Flash bar's (used, total), 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 %s", size_json)
-        return
+    if not _is_number(image_size):
+        _LOGGER.warning("Skipping Flash summary: no usable image_size")
+        return None
     if partitions_csv is None:
-        _LOGGER.warning("Skipping Flash summary: no partition table given")
-        return
+        _LOGGER.debug("Skipping Flash summary: no partition table given")
+        return None
     try:
         app_size = _find_app_partition_size(partitions_csv)
-    except (ValueError, OSError) as e:
+    except (ValueError, OSError, csv.Error) as e:
+        # The table is there but broken/unreadable: visible, like size 0
         _LOGGER.warning("Skipping Flash summary: %s", e)
-        return
+        return None
+    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
         _LOGGER.warning(
@@ -139,5 +169,5 @@ def _print_summary(size_json: Path, partitions_csv: Path | None) -> None:
             app_size,
             partitions_csv,
         )
-        return
-    print_size_line("Flash", image_size, app_size)
+        return None
+    return int(image_size), app_size

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