From 0238de2049779974d7e94e63ebd0f6bd9bfbcbc2 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sat, 4 Jul 2026 03:31:26 -0400 Subject: [PATCH] fix(coercion): tolerate string/percentage input on numeric fields; widen visible Home Assistant templates render every value to a string, so numeric element fields raised TypeError deep inside the handlers. This routes dimension-like fields through CoordinateParser (which already coerces numeric strings and percentages) and str()-coerces text, and hardens the `visible` coercion. - A8: add coordinates.coerce_number() for plain numeric fields, and route dimension fields through parse_x/parse_y/parse_size: * progress_bar.progress ("50"), progress_bar/line/rect/circle/ellipse width * multiline.value (non-str), multiline.size/offset_y/y_padding/stroke_width * text.y_padding/stroke_width/max_width * icon.size / icon_sequence.size ("50%"), stroke_width, spacing * rectangle_pattern geometry (x_start "10%", sizes, offsets, repeats) * circle.radius (now parses "%" like arc.radius) * plot.x_start/x_end/y_start/y_end * dlimg.xsize/ysize, qrcode.boxsize/border - A9: extend the falsy `visible` strings with "0"/"no"/"off"/"none" so a template rendering {{ 0 }} ("0") hides the element instead of showing it. Adds unit tests for coerce_number and _coerce_visible, and integration tests covering the verified A8 crash cases. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01UVpLN4Rzdv7y3ud7QzutVJ --- src/odl_renderer/coordinates.py | 26 ++++ src/odl_renderer/core.py | 2 +- src/odl_renderer/elements/icons.py | 11 +- src/odl_renderer/elements/media.py | 10 +- src/odl_renderer/elements/shapes.py | 48 ++++--- src/odl_renderer/elements/text.py | 21 ++-- src/odl_renderer/elements/visualizations.py | 15 +-- tests/integration/test_string_coercion.py | 133 ++++++++++++++++++++ tests/unit/test_coordinates.py | 30 ++++- tests/unit/test_visible.py | 15 ++- 10 files changed, 262 insertions(+), 49 deletions(-) create mode 100644 tests/integration/test_string_coercion.py diff --git a/src/odl_renderer/coordinates.py b/src/odl_renderer/coordinates.py index 23f2a28..3e5b1cc 100644 --- a/src/odl_renderer/coordinates.py +++ b/src/odl_renderer/coordinates.py @@ -1,6 +1,32 @@ from typing import Any +def coerce_number(value: Any, default: float = 0.0) -> float: + """Coerce a scalar numeric value to a number, tolerating string input. + + Home Assistant templates render everything to strings, so numeric element + fields (widths, counts, progress, …) frequently arrive as ``"50"`` rather + than ``50``. Accepts ints, floats and numeric strings; returns *default* for + ``None`` or non-numeric input. Percentages are NOT handled here — use + :class:`CoordinateParser` for dimension fields that support ``"50%"``. + + Args: + value: The raw value (number or string). + default: Value returned when *value* cannot be parsed. + + Returns: + The parsed number, or *default*. + """ + if isinstance(value, bool): + return default + if isinstance(value, (int, float)): + return value + try: + return float(str(value).strip()) + except (ValueError, TypeError): + return default + + class CoordinateParser: """Helper class for parsing coordinates with percentage support. diff --git a/src/odl_renderer/core.py b/src/odl_renderer/core.py index 20bc71c..7691641 100644 --- a/src/odl_renderer/core.py +++ b/src/odl_renderer/core.py @@ -146,7 +146,7 @@ async def _render_transformed(ctx: DrawingContext, handler: Any, element: dict[s base.alpha_composite(layer) -_FALSY_VISIBLE_STRINGS = frozenset({"false", ""}) +_FALSY_VISIBLE_STRINGS = frozenset({"false", "", "0", "no", "off", "none"}) def _coerce_visible(value: Any) -> bool: diff --git a/src/odl_renderer/elements/icons.py b/src/odl_renderer/elements/icons.py index e43e194..0ceff36 100644 --- a/src/odl_renderer/elements/icons.py +++ b/src/odl_renderer/elements/icons.py @@ -14,6 +14,7 @@ from PIL import ImageDraw, ImageFont from odl_renderer.colors import BLACK +from odl_renderer.coordinates import coerce_number from odl_renderer.registry import element_handler from odl_renderer.types import DrawingContext, ElementType @@ -97,10 +98,10 @@ async def draw_icon(ctx: DrawingContext, element: dict[str, Any]) -> None: raise ValueError(f"Icon '{name}' not found. Search icons at https://pictogrammers.com/library/mdi/") char = chr(int(codepoint, 16)) - font = _get_mdi_font(element["size"]) + font = _get_mdi_font(ctx.coords.parse_size(element["size"], is_width=False)) color = ctx.colors.resolve(element.get("color") or element.get("fill", "black")) or BLACK anchor = element.get("anchor", "la") - stroke_width = element.get("stroke_width", 0) + stroke_width = int(coerce_number(element.get("stroke_width", 0), 0)) stroke_fill = ctx.colors.resolve(element.get("stroke_fill", "white")) draw = ImageDraw.Draw(ctx.img) @@ -144,11 +145,11 @@ async def draw_icon_sequence(ctx: DrawingContext, element: dict[str, Any]) -> No x_start = ctx.coords.parse_x(element["x"]) y_start = ctx.coords.parse_y(element["y"]) - size = element["size"] - spacing = element.get("spacing", size // 4) + size = ctx.coords.parse_size(element["size"], is_width=False) + spacing = int(coerce_number(element.get("spacing", size // 4), size // 4)) color = ctx.colors.resolve(element.get("color") or element.get("fill", "black")) or BLACK anchor = element.get("anchor", "la") - stroke_width = element.get("stroke_width", 0) + stroke_width = int(coerce_number(element.get("stroke_width", 0), 0)) stroke_fill = ctx.colors.resolve(element.get("stroke_fill", "white")) direction = element.get("direction", "right") diff --git a/src/odl_renderer/elements/media.py b/src/odl_renderer/elements/media.py index 2d9e1ac..f8e015c 100644 --- a/src/odl_renderer/elements/media.py +++ b/src/odl_renderer/elements/media.py @@ -8,6 +8,7 @@ from resizeimage import resizeimage # type: ignore[import-untyped] from odl_renderer.colors import BLACK +from odl_renderer.coordinates import coerce_number from odl_renderer.media_loader import load_image from odl_renderer.registry import element_handler from odl_renderer.types import DrawingContext, ElementType @@ -41,8 +42,8 @@ async def draw_qrcode(ctx: DrawingContext, element: dict[str, Any]) -> None: # Get QR code properties color = ctx.colors.resolve(element.get("color", "black")) or BLACK bgcolor = ctx.colors.resolve(element.get("bgcolor", "white")) or BLACK - border = element.get("border", 1) - boxsize = element.get("boxsize", 2) + border = int(coerce_number(element.get("border", 1), 1)) + boxsize = int(coerce_number(element.get("boxsize", 2), 2)) try: # Create QR code instance @@ -99,7 +100,10 @@ async def draw_downloaded_image(ctx: DrawingContext, element: dict[str, Any]) -> # Get image properties pos_x = ctx.coords.parse_x(element["x"]) pos_y = ctx.coords.parse_y(element["y"]) - target_size = (element["xsize"], element["ysize"]) + target_size = ( + ctx.coords.parse_size(element["xsize"], is_width=True), + ctx.coords.parse_size(element["ysize"], is_width=False), + ) rotate = element.get("rotate", 0) resize_method = element.get("resize_method", "stretch") diff --git a/src/odl_renderer/elements/shapes.py b/src/odl_renderer/elements/shapes.py index 77150fd..8491929 100644 --- a/src/odl_renderer/elements/shapes.py +++ b/src/odl_renderer/elements/shapes.py @@ -6,6 +6,7 @@ from PIL import ImageDraw from odl_renderer.colors import BLACK +from odl_renderer.coordinates import coerce_number from odl_renderer.registry import element_handler from odl_renderer.types import DrawingContext, ElementType @@ -36,10 +37,10 @@ async def draw_line(ctx: DrawingContext, element: dict[str, Any]) -> None: # Get line properties fill = ctx.colors.resolve(element.get("fill", "black")) - width = element.get("width", 1) + width = int(coerce_number(element.get("width", 1), 1)) dashed = element.get("dashed", False) - dash_length = element.get("dash_length", 5) - space_length = element.get("space_length", 3) + dash_length = int(coerce_number(element.get("dash_length", 5), 5)) + space_length = int(coerce_number(element.get("space_length", 3), 3)) x_start = ctx.coords.parse_x(element["x_start"]) x_end = ctx.coords.parse_x(element["x_end"]) @@ -74,8 +75,8 @@ async def draw_rectangle(ctx: DrawingContext, element: dict[str, Any]) -> None: # Get rectangle properties rect_fill = ctx.colors.resolve(element.get("fill")) rect_outline = ctx.colors.resolve(element.get("outline", "black")) - rect_width = element.get("width", 1) - radius = element.get("radius", 10 if "corners" in element else 0) + rect_width = int(coerce_number(element.get("width", 1), 1)) + radius = int(coerce_number(element.get("radius", 10 if "corners" in element else 0))) corners = get_rounded_corners(element.get("corners", "all" if "radius" in element else "")) # Draw rectangle @@ -111,22 +112,32 @@ async def draw_rectangle_pattern(ctx: DrawingContext, element: dict[str, Any]) - # Get pattern properties fill = ctx.colors.resolve(element.get("fill")) outline = ctx.colors.resolve(element.get("outline", "black")) - width = element.get("width", 1) - radius = element.get("radius", 10 if "corners" in element else 0) + width = int(coerce_number(element.get("width", 1), 1)) + radius = int(coerce_number(element.get("radius", 10 if "corners" in element else 0))) corners = get_rounded_corners(element.get("corners", "all" if "radius" in element else "")) - max_y = element["y_start"] + # Parse geometry once (supports numeric strings and percentages from templates). + x_start = ctx.coords.parse_x(element["x_start"]) + y_start = ctx.coords.parse_y(element["y_start"]) + x_size = ctx.coords.parse_size(element["x_size"], is_width=True) + y_size = ctx.coords.parse_size(element["y_size"], is_width=False) + x_offset = ctx.coords.parse_size(element["x_offset"], is_width=True) + y_offset = ctx.coords.parse_size(element["y_offset"], is_width=False) + x_repeat = int(coerce_number(element["x_repeat"])) + y_repeat = int(coerce_number(element["y_repeat"])) + + max_y = y_start # Draw rectangle grid - for x in range(element["x_repeat"]): - for y in range(element["y_repeat"]): + for x in range(x_repeat): + for y in range(y_repeat): # Calculate rectangle position - x_pos = element["x_start"] + x * (element["x_offset"] + element["x_size"]) - y_pos = element["y_start"] + y * (element["y_offset"] + element["y_size"]) + x_pos = x_start + x * (x_offset + x_size) + y_pos = y_start + y * (y_offset + y_size) # Draw individual rectangle draw.rounded_rectangle( - (x_pos, y_pos, x_pos + element["x_size"], y_pos + element["y_size"]), + (x_pos, y_pos, x_pos + x_size, y_pos + y_size), fill=fill, outline=outline, width=width, @@ -134,7 +145,7 @@ async def draw_rectangle_pattern(ctx: DrawingContext, element: dict[str, Any]) - corners=corners, ) - max_y = max(max_y, y_pos + element["y_size"]) + max_y = max(max_y, y_pos + y_size) ctx.pos_y = max_y @@ -185,17 +196,18 @@ async def draw_circle(ctx: DrawingContext, element: dict[str, Any]) -> None: # Get circle properties fill = ctx.colors.resolve(element.get("fill")) outline = ctx.colors.resolve(element.get("outline", "black")) - width = element.get("width", 1) + width = int(coerce_number(element.get("width", 1), 1)) + radius = ctx.coords.parse_size(element["radius"], is_width=True) # Draw circle draw.ellipse( - [(x - element["radius"], y - element["radius"]), (x + element["radius"], y + element["radius"])], + [(x - radius, y - radius), (x + radius, y + radius)], fill=fill, outline=outline, width=width, ) - ctx.pos_y = y + element["radius"] + ctx.pos_y = y + radius @element_handler(ElementType.ELLIPSE, requires=["x_start", "x_end", "y_start", "y_end"]) @@ -220,7 +232,7 @@ async def draw_ellipse(ctx: DrawingContext, element: dict[str, Any]) -> None: # Get ellipse properties fill = ctx.colors.resolve(element.get("fill")) outline = ctx.colors.resolve(element.get("outline", "black")) - width = element.get("width", 1) + width = int(coerce_number(element.get("width", 1), 1)) # Draw ellipse draw.ellipse([(x_start, y_start), (x_end, y_end)], fill=fill, outline=outline, width=width) diff --git a/src/odl_renderer/elements/text.py b/src/odl_renderer/elements/text.py index 291bddb..b305b65 100644 --- a/src/odl_renderer/elements/text.py +++ b/src/odl_renderer/elements/text.py @@ -7,6 +7,7 @@ from PIL import ImageDraw, ImageFont from odl_renderer.colors import COLOR_TOKEN_PATTERN +from odl_renderer.coordinates import coerce_number from odl_renderer.registry import element_handler from odl_renderer.types import DrawingContext, ElementType, TextSegment @@ -34,7 +35,7 @@ async def draw_text(ctx: DrawingContext, element: dict[str, Any]) -> None: x = ctx.coords.parse_x(element["x"]) if "y" not in element: - y = ctx.pos_y + element.get("y_padding", 10) + y = ctx.pos_y + int(coerce_number(element.get("y_padding", 10), 10)) else: y = ctx.coords.parse_y(element["y"]) # Get text properties @@ -47,12 +48,14 @@ async def draw_text(ctx: DrawingContext, element: dict[str, Any]) -> None: default_color = ctx.colors.resolve(element.get("color", "black")) anchor = element.get("anchor") spacing = element.get("spacing", 5) - stroke_width = element.get("stroke_width", 0) + stroke_width = int(coerce_number(element.get("stroke_width", 0), 0)) stroke_fill = ctx.colors.resolve(element.get("stroke_fill", "white")) # Process text content text = str(element["value"]) max_width = element.get("max_width") + if max_width is not None: + max_width = ctx.coords.parse_size(max_width, is_width=True) # Handle text wrapping if max_width is specified final_text = text @@ -182,14 +185,15 @@ async def draw_multiline(ctx: DrawingContext, element: dict[str, Any]) -> None: draw.fontmode = "1" # Get text properties - size = element.get("size", 20) + size = ctx.coords.parse_size(element.get("size", 20), is_width=False) font_name = element.get("font", "ppb.ttf") font = ctx.fonts.get_font(font_name, size) color = ctx.colors.resolve(element.get("color", "black")) align = element.get("align", "left") anchor = element.get("anchor", "lm") - stroke_width = element.get("stroke_width", 0) + stroke_width = int(coerce_number(element.get("stroke_width", 0), 0)) stroke_fill = ctx.colors.resolve(element.get("stroke_fill", "white")) + offset_y = int(coerce_number(element["offset_y"])) x = ctx.coords.parse_x(element["x"]) # Support both 'y' (standard) and 'start_y' (legacy) for backward compatibility @@ -198,10 +202,11 @@ async def draw_multiline(ctx: DrawingContext, element: dict[str, Any]) -> None: elif "start_y" in element: current_y = ctx.coords.parse_y(element["start_y"]) else: - current_y = ctx.pos_y + element.get("y_padding", 10) + current_y = ctx.pos_y + int(coerce_number(element.get("y_padding", 10), 10)) - # Split text using delimiter - lines = element["value"].replace("\n", "").split(element["delimiter"]) + # Split text using delimiter. str()-coerce the value because HA templates may + # render a non-string (e.g. a number) here. + lines = str(element["value"]).replace("\n", "").split(element["delimiter"]) max_y = current_y for line in lines: @@ -232,7 +237,7 @@ async def draw_multiline(ctx: DrawingContext, element: dict[str, Any]) -> None: stroke_width=stroke_width, stroke_fill=stroke_fill, ) - current_y += element["offset_y"] + current_y += offset_y max_y = current_y ctx.pos_y = max_y diff --git a/src/odl_renderer/elements/visualizations.py b/src/odl_renderer/elements/visualizations.py index a586176..fac8849 100644 --- a/src/odl_renderer/elements/visualizations.py +++ b/src/odl_renderer/elements/visualizations.py @@ -8,6 +8,7 @@ from PIL import ImageDraw +from odl_renderer.coordinates import coerce_number from odl_renderer.registry import element_handler from odl_renderer.types import DrawingContext, ElementType @@ -562,10 +563,10 @@ async def draw_plot(ctx: DrawingContext, element: dict[str, Any]) -> None: raise ValueError("plot element requires a data_provider in DrawingContext") draw = ImageDraw.Draw(ctx.img) - x_start = element.get("x_start", 0) - y_start = element.get("y_start", 0) - x_end = element.get("x_end", ctx.img.width - 1 - x_start) - y_end = element.get("y_end", ctx.img.height - 1 - y_start) + x_start = ctx.coords.parse_x(element.get("x_start", 0)) + y_start = ctx.coords.parse_y(element.get("y_start", 0)) + x_end = ctx.coords.parse_x(element["x_end"]) if "x_end" in element else ctx.img.width - 1 - x_start + y_end = ctx.coords.parse_y(element["y_end"]) if "y_end" in element else ctx.img.height - 1 - y_start font_name = element.get("font", "ppb.ttf") duration_seconds = float(element.get("duration", 60 * 60 * 24)) @@ -663,12 +664,12 @@ async def draw_progress_bar(ctx: DrawingContext, element: dict[str, Any]) -> Non x_end = ctx.coords.parse_x(element["x_end"]) y_end = ctx.coords.parse_y(element["y_end"]) - progress = min(100, max(0, element["progress"])) # Clamp to 0-100 + progress = min(100, max(0, coerce_number(element["progress"]))) # Clamp to 0-100 direction = element.get("direction", "right") background = ctx.colors.resolve(element.get("background", "white")) fill = ctx.colors.resolve(element.get("fill", "red")) outline = ctx.colors.resolve(element.get("outline", "black")) - width = element.get("width", 1) + width = int(coerce_number(element.get("width", 1), 1)) show_percentage = element.get("show_percentage", False) font_name = element.get("font_name", "ppb.ttf") @@ -702,7 +703,7 @@ async def draw_progress_bar(ctx: DrawingContext, element: dict[str, Any]) -> Non font_size = min(y_end - y_start - 4, x_end - x_start - 4, 20) font = ctx.fonts.get_font(font_name, font_size) - percentage_text = f"{progress}%" + percentage_text = f"{progress:g}%" # Get text dimensions text_bbox = draw.textbbox((0, 0), percentage_text, font=font) diff --git a/tests/integration/test_string_coercion.py b/tests/integration/test_string_coercion.py new file mode 100644 index 0000000..d03abf2 --- /dev/null +++ b/tests/integration/test_string_coercion.py @@ -0,0 +1,133 @@ +"""Regression tests for finding A8: numeric element fields must tolerate string +input (Home Assistant templates render everything to strings) and, where +sensible, percentage strings.""" + +import pytest + +from odl_renderer import generate_image + + +@pytest.mark.asyncio +class TestNumericStringCoercion: + async def test_progress_bar_string_progress(self): + """progress_bar.progress as a string does not crash and clamps correctly.""" + image = await generate_image( + width=200, + height=100, + elements=[ + { + "type": "progress_bar", + "x_start": 10, + "y_start": 10, + "x_end": 190, + "y_end": 40, + "progress": "50", + "show_percentage": True, + } + ], + ) + assert image.size == (200, 100) + + async def test_multiline_non_string_value(self): + """multiline.value given a number is coerced to str instead of crashing.""" + image = await generate_image( + width=200, + height=100, + elements=[ + { + "type": "multiline", + "x": 10, + "y": 10, + "value": 123, + "delimiter": "|", + "offset_y": "20", + "font": "ppb", + "size": "16", + } + ], + ) + assert image.size == (200, 100) + + async def test_icon_percentage_size(self): + """icon.size as a percentage string resolves instead of crashing.""" + image = await generate_image( + width=100, + height=100, + elements=[{"type": "icon", "value": "home", "x": 50, "y": 50, "size": "50%"}], + ) + assert image.size == (100, 100) + + async def test_rectangle_pattern_percentage_start(self): + """rectangle_pattern geometry accepts percentage/number strings.""" + image = await generate_image( + width=200, + height=100, + elements=[ + { + "type": "rectangle_pattern", + "x_start": "10%", + "y_start": "10%", + "x_size": "10", + "y_size": "10", + "x_offset": "5", + "y_offset": "5", + "x_repeat": "3", + "y_repeat": "3", + "fill": "black", + } + ], + ) + assert image.size == (200, 100) + + async def test_circle_percentage_radius(self): + """circle.radius accepts a percentage string (parity with arc.radius).""" + image = await generate_image( + width=200, + height=200, + elements=[{"type": "circle", "x": 100, "y": 100, "radius": "10%", "fill": "black"}], + ) + assert image.size == (200, 200) + + async def test_line_string_width(self): + """line.width as a string does not crash.""" + image = await generate_image( + width=200, + height=100, + elements=[{"type": "line", "x_start": 0, "y_start": 50, "x_end": 200, "width": "2"}], + ) + assert image.size == (200, 100) + + async def test_text_string_dimensions(self): + """text stroke_width / y_padding / max_width as strings do not crash.""" + image = await generate_image( + width=200, + height=100, + elements=[ + { + "type": "text", + "x": 10, + "value": "Hello world this is a fairly long line", + "font": "ppb", + "size": "16", + "y_padding": "10", + "stroke_width": "1", + "max_width": "150", + } + ], + ) + assert image.size == (200, 100) + + async def test_dlimg_string_size(self): + """dlimg xsize/ysize as strings are parsed (URL load failure is separate).""" + # A bad URL raises during load; we only assert the size parsing path does + # not raise a TypeError first. Use a data: URL that decodes to a 1x1 image. + one_by_one_png = ( + "data:image/png;base64," + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==" + ) + image = await generate_image( + width=100, + height=100, + elements=[{"type": "dlimg", "x": 10, "y": 10, "url": one_by_one_png, "xsize": "50", "ysize": "50"}], + ) + assert image.size == (100, 100) diff --git a/tests/unit/test_coordinates.py b/tests/unit/test_coordinates.py index 3731d46..58a7baa 100644 --- a/tests/unit/test_coordinates.py +++ b/tests/unit/test_coordinates.py @@ -2,7 +2,9 @@ from __future__ import annotations -from odl_renderer.coordinates import CoordinateParser +import pytest + +from odl_renderer.coordinates import CoordinateParser, coerce_number class TestCoordinateParser: @@ -52,3 +54,29 @@ def test_parse_coordinates_defaults_to_zero(self): x, y = self.parser.parse_coordinates({}) assert x == 0 assert y == 0 + + +class TestCoerceNumber: + """Unit tests for coerce_number (finding A8).""" + + @pytest.mark.parametrize( + ("value", "expected"), + [ + (50, 50), + (2.5, 2.5), + ("50", 50.0), + ("2.5", 2.5), + (" 7 ", 7.0), + ("-3", -3.0), + ], + ) + def test_parses_numbers_and_numeric_strings(self, value, expected): + assert coerce_number(value) == expected + + @pytest.mark.parametrize("value", ["abc", "", "50%", None, "1,2"]) + def test_non_numeric_returns_default(self, value): + assert coerce_number(value, default=99) == 99 + + def test_bool_returns_default(self): + # Booleans are not meaningful numeric dimensions here. + assert coerce_number(True, default=5) == 5 diff --git a/tests/unit/test_visible.py b/tests/unit/test_visible.py index 84727dc..ad11c9a 100644 --- a/tests/unit/test_visible.py +++ b/tests/unit/test_visible.py @@ -25,12 +25,15 @@ # Surrounding whitespace is stripped before comparison. (" false ", False), (" true ", True), - # Narrowed set: only "false"/empty are falsy. Other words stay truthy - # so a value that happens to be "no"/"0"/"none" is not silently hidden. - ("no", True), - ("off", True), - ("0", True), - ("none", True), + # Common falsy string forms hide too — a template rendering {{ 0 }} yields + # "0", and "no"/"off"/"none" read as hidden rather than silently shown. + ("no", False), + ("off", False), + ("0", False), + ("none", False), + ("No", False), + ("OFF", False), + # Other words stay truthy. ("null", True), ("1", True), ("yes", True),