From 20927674da69d21148d9a8ee72a995e93a34c2ff Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 7 Jan 2026 08:24:09 -1000 Subject: [PATCH] [sun] Eliminate heap allocation in text sensor (#13037) --- .../sun/text_sensor/sun_text_sensor.h | 4 +- esphome/core/time.cpp | 23 ++-- esphome/core/time.h | 16 ++- tests/integration/fixtures/strftime_to.yaml | 53 +++++++++ tests/integration/test_strftime_to.py | 111 ++++++++++++++++++ 5 files changed, 197 insertions(+), 10 deletions(-) create mode 100644 tests/integration/fixtures/strftime_to.yaml create mode 100644 tests/integration/test_strftime_to.py diff --git a/esphome/components/sun/text_sensor/sun_text_sensor.h b/esphome/components/sun/text_sensor/sun_text_sensor.h index ce7d21fb86..9345a32223 100644 --- a/esphome/components/sun/text_sensor/sun_text_sensor.h +++ b/esphome/components/sun/text_sensor/sun_text_sensor.h @@ -28,7 +28,9 @@ class SunTextSensor : public text_sensor::TextSensor, public PollingComponent { return; } - this->publish_state(res->strftime(this->format_)); + char buf[ESPTime::STRFTIME_BUFFER_SIZE]; + size_t len = res->strftime_to(buf, this->format_.c_str()); + this->publish_state(buf, len); } void dump_config() override; diff --git a/esphome/core/time.cpp b/esphome/core/time.cpp index d30dac4394..4047033f84 100644 --- a/esphome/core/time.cpp +++ b/esphome/core/time.cpp @@ -1,6 +1,7 @@ #include "time.h" // NOLINT #include "helpers.h" +#include #include namespace esphome { @@ -17,6 +18,18 @@ size_t ESPTime::strftime(char *buffer, size_t buffer_len, const char *format) { return ::strftime(buffer, buffer_len, format, &c_tm); } +size_t ESPTime::strftime_to(std::span buffer, const char *format) { + struct tm c_tm = this->to_c_tm(); + size_t len = ::strftime(buffer.data(), buffer.size(), format, &c_tm); + if (len > 0) { + return len; + } + // Write "ERROR" to buffer on failure for consistent behavior + constexpr char error_str[] = "ERROR"; + std::copy_n(error_str, sizeof(error_str), buffer.data()); + return sizeof(error_str) - 1; // Length excluding null terminator +} + ESPTime ESPTime::from_c_tm(struct tm *c_tm, time_t c_time) { ESPTime res{}; res.second = uint8_t(c_tm->tm_sec); @@ -47,13 +60,9 @@ struct tm ESPTime::to_c_tm() { } std::string ESPTime::strftime(const char *format) { - struct tm c_tm = this->to_c_tm(); - char buf[128]; - size_t len = ::strftime(buf, sizeof(buf), format, &c_tm); - if (len > 0) { - return std::string(buf, len); - } - return "ERROR"; + char buf[STRFTIME_BUFFER_SIZE]; + size_t len = this->strftime_to(buf, format); + return std::string(buf, len); } std::string ESPTime::strftime(const std::string &format) { return this->strftime(format.c_str()); } diff --git a/esphome/core/time.h b/esphome/core/time.h index 68826dabdc..f6f1d57dbb 100644 --- a/esphome/core/time.h +++ b/esphome/core/time.h @@ -3,6 +3,7 @@ #include #include #include +#include #include namespace esphome { @@ -13,6 +14,9 @@ uint8_t days_in_month(uint8_t month, uint16_t year); /// A more user-friendly version of struct tm from time.h struct ESPTime { + /// Buffer size required for strftime output + static constexpr size_t STRFTIME_BUFFER_SIZE = 128; + /** seconds after the minute [0-60] * @note second is generally 0-59; the extra range is to accommodate leap seconds. */ @@ -43,14 +47,22 @@ struct ESPTime { */ size_t strftime(char *buffer, size_t buffer_len, const char *format); + /** Format time into a fixed-size buffer, returns length written. + * + * This is the preferred method for avoiding heap allocations. The buffer size is enforced at compile-time. + * On format error, writes "ERROR" to the buffer and returns 5. + * @see https://www.gnu.org/software/libc/manual/html_node/Formatting-Calendar-Time.html#index-strftime + */ + size_t strftime_to(std::span buffer, const char *format); + /** Convert this ESPTime struct to a string as specified by the format argument. * @see https://en.cppreference.com/w/c/chrono/strftime * * @warning This method returns a dynamically allocated string which can cause heap fragmentation with some - * microcontrollers. + * microcontrollers. Prefer strftime_to() for heap-free formatting. * * @warning This method can return "ERROR" when the underlying strftime() call fails or when the - * output exceeds 128 bytes. + * output exceeds STRFTIME_BUFFER_SIZE bytes. */ std::string strftime(const std::string &format); diff --git a/tests/integration/fixtures/strftime_to.yaml b/tests/integration/fixtures/strftime_to.yaml new file mode 100644 index 0000000000..bd157e110c --- /dev/null +++ b/tests/integration/fixtures/strftime_to.yaml @@ -0,0 +1,53 @@ +esphome: + name: strftime-to-test +host: +api: +logger: + +time: + - platform: homeassistant + id: ha_time + +text_sensor: + # Test strftime_to with a valid format + - platform: template + name: "Time Format Test" + id: time_format_test + update_interval: 100ms + lambda: |- + auto now = ESPTime::from_epoch_local(1704067200); // 2024-01-01 00:00:00 UTC + char buf[ESPTime::STRFTIME_BUFFER_SIZE]; + size_t len = now.strftime_to(buf, "%Y-%m-%d %H:%M:%S"); + return std::string(buf, len); + + # Test strftime_to with a short format + - platform: template + name: "Time Short Format" + id: time_short_format + update_interval: 100ms + lambda: |- + auto now = ESPTime::from_epoch_local(1704067200); + char buf[ESPTime::STRFTIME_BUFFER_SIZE]; + size_t len = now.strftime_to(buf, "%H:%M"); + return std::string(buf, len); + + # Test strftime (std::string version) still works + - platform: template + name: "Time String Format" + id: time_string_format + update_interval: 100ms + lambda: |- + auto now = ESPTime::from_epoch_local(1704067200); + return now.strftime("%Y-%m-%d"); + + # Test strftime_to with empty/invalid format returns ERROR + - platform: template + name: "Time Error Format" + id: time_error_format + update_interval: 100ms + lambda: |- + auto now = ESPTime::from_epoch_local(1704067200); + char buf[ESPTime::STRFTIME_BUFFER_SIZE]; + // Empty format string causes strftime to return 0 + size_t len = now.strftime_to(buf, ""); + return std::string(buf, len); diff --git a/tests/integration/test_strftime_to.py b/tests/integration/test_strftime_to.py new file mode 100644 index 0000000000..9220da148b --- /dev/null +++ b/tests/integration/test_strftime_to.py @@ -0,0 +1,111 @@ +"""Integration test for ESPTime::strftime_to() method.""" + +from __future__ import annotations + +import asyncio + +from aioesphomeapi import EntityState, TextSensorState +import pytest + +from .state_utils import InitialStateHelper, require_entity +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_strftime_to( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test ESPTime::strftime_to() formats time correctly.""" + async with run_compiled(yaml_config), api_client_connected() as client: + device_info = await client.device_info() + assert device_info is not None + assert device_info.name == "strftime-to-test" + + # Get entities + entities, _ = await client.list_entities_services() + + # Find our text sensors + format_test = require_entity( + entities, "time_format_test", description="Time Format Test sensor" + ) + short_format = require_entity( + entities, "time_short_format", description="Time Short Format sensor" + ) + string_format = require_entity( + entities, "time_string_format", description="Time String Format sensor" + ) + error_format = require_entity( + entities, "time_error_format", description="Time Error Format sensor" + ) + + # Set up state tracking with InitialStateHelper + loop = asyncio.get_running_loop() + states: dict[int, TextSensorState] = {} + all_received = loop.create_future() + expected_keys = { + format_test.key, + short_format.key, + string_format.key, + error_format.key, + } + initial_state_helper = InitialStateHelper(entities) + + def on_state(state: EntityState) -> None: + if isinstance(state, TextSensorState) and not state.missing_state: + states[state.key] = state + if expected_keys <= states.keys() and not all_received.done(): + all_received.set_result(True) + + # Subscribe with the wrapper that filters initial states + client.subscribe_states(initial_state_helper.on_state_wrapper(on_state)) + + # Wait for initial states to be broadcast + try: + await initial_state_helper.wait_for_initial_states() + except TimeoutError: + pytest.fail("Timeout waiting for initial states") + + # Wait for all expected states + try: + await asyncio.wait_for(all_received, timeout=5.0) + except TimeoutError: + pytest.fail( + f"Timeout waiting for text sensor states. Got: {list(states.keys())}" + ) + + # Validate strftime_to with full format + # Note: The exact output depends on timezone, but should contain date components + format_test_state = states[format_test.key].state + assert "2024" in format_test_state or "2023" in format_test_state, ( + f"Expected year in format test output, got: {format_test_state}" + ) + # Should have format like "YYYY-MM-DD HH:MM:SS" + assert len(format_test_state) == 19, ( + f"Expected 19 chars for datetime format, got {len(format_test_state)}: {format_test_state}" + ) + + # Validate short format (HH:MM) + short_format_state = states[short_format.key].state + assert len(short_format_state) == 5, ( + f"Expected 5 chars for HH:MM format, got {len(short_format_state)}: {short_format_state}" + ) + assert ":" in short_format_state, ( + f"Expected colon in HH:MM format, got: {short_format_state}" + ) + + # Validate string format (the std::string returning version) + string_format_state = states[string_format.key].state + assert len(string_format_state) == 10, ( + f"Expected 10 chars for YYYY-MM-DD format, got {len(string_format_state)}: {string_format_state}" + ) + assert string_format_state.count("-") == 2, ( + f"Expected two dashes in YYYY-MM-DD format, got: {string_format_state}" + ) + + # Validate error format returns "ERROR" + error_format_state = states[error_format.key].state + assert error_format_state == "ERROR", ( + f"Expected 'ERROR' for empty format string, got: {error_format_state}" + )