From 9da2c08f363df712eb601cde4eebf25eebfc37f4 Mon Sep 17 00:00:00 2001 From: Clyde Stubbs <2366188+clydebarrow@users.noreply.github.com> Date: Thu, 15 Jan 2026 14:27:26 +1100 Subject: [PATCH 1/4] [image] Correctly handle dimensions in physical units (#13209) --- esphome/components/image/__init__.py | 13 ++--- .../image/config/mm_dimensions.svg | 5 ++ tests/component_tests/image/test_init.py | 55 ++++++++++++++++++- 3 files changed, 63 insertions(+), 10 deletions(-) create mode 100644 tests/component_tests/image/config/mm_dimensions.svg diff --git a/esphome/components/image/__init__.py b/esphome/components/image/__init__.py index 3f8d909824..6ff75d7709 100644 --- a/esphome/components/image/__init__.py +++ b/esphome/components/image/__init__.py @@ -665,15 +665,10 @@ async def write_image(config, all_frames=False): if is_svg_file(path): import resvg_py - if resize: - width, height = resize - # resvg-py allows rendering by width/height directly - image_data = resvg_py.svg_to_bytes( - svg_path=str(path), width=int(width), height=int(height) - ) - else: - # Default size - image_data = resvg_py.svg_to_bytes(svg_path=str(path)) + resize = resize or (None, None) + image_data = resvg_py.svg_to_bytes( + svg_path=str(path), width=resize[0], height=resize[1], dpi=100 + ) # Convert bytes to Pillow Image image = Image.open(io.BytesIO(image_data)) diff --git a/tests/component_tests/image/config/mm_dimensions.svg b/tests/component_tests/image/config/mm_dimensions.svg new file mode 100644 index 0000000000..bb64433a4d --- /dev/null +++ b/tests/component_tests/image/config/mm_dimensions.svg @@ -0,0 +1,5 @@ + + + + + diff --git a/tests/component_tests/image/test_init.py b/tests/component_tests/image/test_init.py index 930bbac8d1..c9481a0e1d 100644 --- a/tests/component_tests/image/test_init.py +++ b/tests/component_tests/image/test_init.py @@ -5,17 +5,21 @@ from __future__ import annotations from collections.abc import Callable from pathlib import Path from typing import Any +from unittest.mock import MagicMock, patch import pytest from esphome import config_validation as cv from esphome.components.image import ( + CONF_INVERT_ALPHA, + CONF_OPAQUE, CONF_TRANSPARENCY, CONFIG_SCHEMA, get_all_image_metadata, get_image_metadata, + write_image, ) -from esphome.const import CONF_ID, CONF_RAW_DATA_ID, CONF_TYPE +from esphome.const import CONF_DITHER, CONF_FILE, CONF_ID, CONF_RAW_DATA_ID, CONF_TYPE from esphome.core import CORE @@ -350,3 +354,52 @@ def test_get_all_image_metadata_empty() -> None: "get_all_image_metadata should always return a dict" ) # Length could be 0 or more depending on what's in CORE at test time + + +@pytest.fixture +def mock_progmem_array(): + """Mock progmem_array to avoid needing a proper ID object in tests.""" + with patch("esphome.components.image.cg.progmem_array") as mock_progmem: + mock_progmem.return_value = MagicMock() + yield mock_progmem + + +@pytest.mark.asyncio +async def test_svg_with_mm_dimensions_succeeds( + component_config_path: Callable[[str], Path], + mock_progmem_array: MagicMock, +) -> None: + """Test that SVG files with dimensions in mm are successfully processed.""" + # Create a config for write_image without CONF_RESIZE + config = { + CONF_FILE: component_config_path("mm_dimensions.svg"), + CONF_TYPE: "BINARY", + CONF_TRANSPARENCY: CONF_OPAQUE, + CONF_DITHER: "NONE", + CONF_INVERT_ALPHA: False, + CONF_RAW_DATA_ID: "test_raw_data_id", + } + + # This should succeed without raising an error + result = await write_image(config) + + # Verify that write_image returns the expected tuple + assert isinstance(result, tuple), "write_image should return a tuple" + assert len(result) == 6, "write_image should return 6 values" + + prog_arr, width, height, image_type, trans_value, frame_count = result + + # Verify the dimensions are positive integers + # At 100 DPI, 10mm = ~39 pixels (10mm * 100dpi / 25.4mm_per_inch) + assert isinstance(width, int), "Width should be an integer" + assert isinstance(height, int), "Height should be an integer" + assert width > 0, "Width should be positive" + assert height > 0, "Height should be positive" + assert frame_count == 1, "Single image should have frame_count of 1" + # Verify we got reasonable dimensions from the mm-based SVG + assert 30 < width < 50, ( + f"Width should be around 39 pixels for 10mm at 100dpi, got {width}" + ) + assert 30 < height < 50, ( + f"Height should be around 39 pixels for 10mm at 100dpi, got {height}" + ) From 76082b3eb999e17e7d5489f1b0781476e2e11d35 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 14 Jan 2026 17:43:03 -1000 Subject: [PATCH 2/4] [core][mqtt] Add str_sanitize_to(), soft-deprecate str_sanitize() --- esphome/components/mqtt/mqtt_client.cpp | 3 ++- esphome/components/mqtt/mqtt_component.cpp | 5 +++-- esphome/core/helpers.cpp | 19 +++++++++++++++---- esphome/core/helpers.h | 18 ++++++++++++++++++ script/ci-custom.py | 2 ++ 5 files changed, 40 insertions(+), 7 deletions(-) diff --git a/esphome/components/mqtt/mqtt_client.cpp b/esphome/components/mqtt/mqtt_client.cpp index 0ab5b238b5..d68ce33361 100644 --- a/esphome/components/mqtt/mqtt_client.cpp +++ b/esphome/components/mqtt/mqtt_client.cpp @@ -635,7 +635,8 @@ void MQTTClientComponent::set_log_message_template(MQTTMessage &&message) { this const MQTTDiscoveryInfo &MQTTClientComponent::get_discovery_info() const { return this->discovery_info_; } void MQTTClientComponent::set_topic_prefix(const std::string &topic_prefix, const std::string &check_topic_prefix) { if (App.is_name_add_mac_suffix_enabled() && (topic_prefix == check_topic_prefix)) { - this->topic_prefix_ = str_sanitize(App.get_name()); + char buf[ESPHOME_DEVICE_NAME_MAX_LEN + 1]; + this->topic_prefix_ = str_sanitize_to(buf, App.get_name()); } else { this->topic_prefix_ = topic_prefix; } diff --git a/esphome/components/mqtt/mqtt_component.cpp b/esphome/components/mqtt/mqtt_component.cpp index 20c111de43..0090ef1e66 100644 --- a/esphome/components/mqtt/mqtt_component.cpp +++ b/esphome/components/mqtt/mqtt_component.cpp @@ -48,7 +48,8 @@ void MQTTComponent::set_subscribe_qos(uint8_t qos) { this->subscribe_qos_ = qos; void MQTTComponent::set_retain(bool retain) { this->retain_ = retain; } std::string MQTTComponent::get_discovery_topic_(const MQTTDiscoveryInfo &discovery_info) const { - std::string sanitized_name = str_sanitize(App.get_name()); + char sanitized_name[ESPHOME_DEVICE_NAME_MAX_LEN + 1]; + str_sanitize_to(sanitized_name, App.get_name()); const char *comp_type = this->component_type(); char object_id_buf[OBJECT_ID_MAX_LEN]; StringRef object_id = this->get_default_object_id_to_(object_id_buf); @@ -60,7 +61,7 @@ std::string MQTTComponent::get_discovery_topic_(const MQTTDiscoveryInfo &discove p = append_char(p, '/'); p = append_str(p, comp_type, strlen(comp_type)); p = append_char(p, '/'); - p = append_str(p, sanitized_name.data(), sanitized_name.size()); + p = append_str(p, sanitized_name, strlen(sanitized_name)); p = append_char(p, '/'); p = append_str(p, object_id.c_str(), object_id.size()); p = append_str(p, "/config", 7); diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index 309407fbec..3277b0be26 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -199,11 +199,22 @@ std::string str_snake_case(const std::string &str) { } return result; } -std::string str_sanitize(const std::string &str) { - std::string result = str; - for (char &c : result) { - c = to_sanitized_char(c); +char *str_sanitize_to(char *buffer, size_t buffer_size, const char *str) { + if (buffer_size == 0) { + return buffer; } + size_t i = 0; + while (*str && i < buffer_size - 1) { + buffer[i++] = to_sanitized_char(*str++); + } + buffer[i] = '\0'; + return buffer; +} + +std::string str_sanitize(const std::string &str) { + std::string result; + result.resize(str.size()); + str_sanitize_to(&result[0], str.size() + 1, str.c_str()); return result; } std::string str_snprintf(const char *fmt, size_t len, ...) { diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 2e9c0e6b13..c02c1a5282 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -545,7 +545,25 @@ std::string str_snake_case(const std::string &str); constexpr char to_sanitized_char(char c) { return (c == '-' || c == '_' || (c >= '0' && c <= '9') || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z')) ? c : '_'; } + +/** Sanitize a string to buffer, keeping only alphanumerics, dashes, and underscores. + * + * @param buffer Output buffer to write to. + * @param buffer_size Size of the output buffer. + * @param str Input string to sanitize. + * @return Pointer to buffer. + * + * Buffer size needed: strlen(str) + 1. + */ +char *str_sanitize_to(char *buffer, size_t buffer_size, const char *str); + +/// Sanitize a string to buffer. Automatically deduces buffer size. +template inline char *str_sanitize_to(char (&buffer)[N], const char *str) { + return str_sanitize_to(buffer, N, str); +} + /// Sanitizes the input string by removing all characters but alphanumerics, dashes and underscores. +/// @warning Allocates heap memory. Use str_sanitize_to() with a stack buffer instead. std::string str_sanitize(const std::string &str); /// Calculate FNV-1 hash of a string while applying snake_case + sanitize transformations. diff --git a/script/ci-custom.py b/script/ci-custom.py index e63e61e096..56954630de 100755 --- a/script/ci-custom.py +++ b/script/ci-custom.py @@ -687,6 +687,7 @@ HEAP_ALLOCATING_HELPERS = { "format_mac_address_pretty": "format_mac_addr_upper() with a stack buffer", "get_mac_address": "get_mac_address_into_buffer() with a stack buffer", "get_mac_address_pretty": "get_mac_address_pretty_into_buffer() with a stack buffer", + "str_sanitize": "str_sanitize_to() with a stack buffer", "str_truncate": "removal (function is unused)", "str_upper_case": "removal (function is unused)", "str_snake_case": "removal (function is unused)", @@ -704,6 +705,7 @@ HEAP_ALLOCATING_HELPERS = { r"format_mac_address_pretty|" r"get_mac_address_pretty(?!_)|" r"get_mac_address(?!_)|" + r"str_sanitize(?!_)|" r"str_truncate|" r"str_upper_case|" r"str_snake_case" From 3b90a8f210ff663bb689375e35a93e39a74a327f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 14 Jan 2026 17:46:04 -1000 Subject: [PATCH 3/4] .c_str() --- esphome/components/mqtt/mqtt_client.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/mqtt/mqtt_client.cpp b/esphome/components/mqtt/mqtt_client.cpp index d68ce33361..7d4050284f 100644 --- a/esphome/components/mqtt/mqtt_client.cpp +++ b/esphome/components/mqtt/mqtt_client.cpp @@ -636,7 +636,7 @@ const MQTTDiscoveryInfo &MQTTClientComponent::get_discovery_info() const { retur void MQTTClientComponent::set_topic_prefix(const std::string &topic_prefix, const std::string &check_topic_prefix) { if (App.is_name_add_mac_suffix_enabled() && (topic_prefix == check_topic_prefix)) { char buf[ESPHOME_DEVICE_NAME_MAX_LEN + 1]; - this->topic_prefix_ = str_sanitize_to(buf, App.get_name()); + this->topic_prefix_ = str_sanitize_to(buf, App.get_name().c_str()); } else { this->topic_prefix_ = topic_prefix; } From bbd8d90cbedc468c7c443268cb6df9d03e140ca5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 14 Jan 2026 17:46:05 -1000 Subject: [PATCH 4/4] .c_str() --- esphome/components/mqtt/mqtt_component.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/mqtt/mqtt_component.cpp b/esphome/components/mqtt/mqtt_component.cpp index 0090ef1e66..2aafd3bbba 100644 --- a/esphome/components/mqtt/mqtt_component.cpp +++ b/esphome/components/mqtt/mqtt_component.cpp @@ -49,7 +49,7 @@ void MQTTComponent::set_retain(bool retain) { this->retain_ = retain; } std::string MQTTComponent::get_discovery_topic_(const MQTTDiscoveryInfo &discovery_info) const { char sanitized_name[ESPHOME_DEVICE_NAME_MAX_LEN + 1]; - str_sanitize_to(sanitized_name, App.get_name()); + str_sanitize_to(sanitized_name, App.get_name().c_str()); const char *comp_type = this->component_type(); char object_id_buf[OBJECT_ID_MAX_LEN]; StringRef object_id = this->get_default_object_id_to_(object_id_buf);