From d8e586609fd8a8dd15001579d64f3d004702e5d7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 8 Apr 2026 20:32:31 -1000 Subject: [PATCH 01/18] [core] Optimize value_accuracy_to_buf to avoid snprintf Replace snprintf("%.*f") with integer-based formatting for finite float values with accuracy_decimals 0-3 (covers virtually all sensor usage). Falls back to snprintf for higher accuracy or NaN/Inf. Uses lrint() with double cast for the multiply to match snprintf's rounding behavior exactly. The fast path avoids snprintf's heavy float formatting machinery entirely. Also optimizes value_accuracy_with_uom_to_buf to append the UOM string directly instead of going through snprintf. Adds C++ unit tests that verify output matches snprintf for a range of values including edge cases. Benchmark: 92,961ns -> 6,484ns (14.3x faster, 2000 iterations). --- esphome/core/helpers.cpp | 81 ++++++++-- tests/components/core/test_value_accuracy.cpp | 151 ++++++++++++++++++ 2 files changed, 221 insertions(+), 11 deletions(-) create mode 100644 tests/components/core/test_value_accuracy.cpp diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index 5940f6ec98..48ed45477f 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -526,28 +526,87 @@ std::string value_accuracy_to_string(float value, int8_t accuracy_decimals) { return std::string(buf); } +// Fast float-to-string for accuracy_decimals 0-3 (covers virtually all sensor usage). +// Avoids snprintf("%.*f") which pulls in heavy float formatting machinery. +static size_t value_accuracy_to_buf_fast(char *buf, float value, int8_t accuracy_decimals) { + char *p = buf; + if (std::signbit(value)) { + *p++ = '-'; + value = -value; + } + uint32_t mult = 1; + if (accuracy_decimals == 1) + mult = 10; + else if (accuracy_decimals == 2) + mult = 100; + else if (accuracy_decimals == 3) + mult = 1000; + // Cast to double for the multiply to match snprintf's precision. + // float*int loses bits at exact-half boundaries (e.g. 23.45f*10 = 234.5 in float, + // but snprintf sees 234.500007... via double promotion and rounds differently). + uint32_t scaled = static_cast(lrint(static_cast(value) * mult)); + uint32_t int_part = scaled / mult; + // Write integer part in reverse, then flip + char *start = p; + if (int_part == 0) { + *p++ = '0'; + } else { + while (int_part > 0) { + *p++ = '0' + (int_part % 10); + int_part /= 10; + } + std::reverse(start, p); + } + if (accuracy_decimals > 0) { + *p++ = '.'; + uint32_t frac = scaled % mult; + uint32_t d = mult / 10; + while (d > 0) { + *p++ = '0' + static_cast(frac / d); + frac %= d; + d /= 10; + } + } + *p = '\0'; + return static_cast(p - buf); +} + size_t value_accuracy_to_buf(std::span buf, float value, int8_t accuracy_decimals) { normalize_accuracy_decimals(value, accuracy_decimals); - // snprintf returns chars that would be written (excluding null), or negative on error + + // Fast path for accuracy 0-3 and finite values + if (accuracy_decimals <= 3 && std::isfinite(value)) { + return value_accuracy_to_buf_fast(buf.data(), value, accuracy_decimals); + } + + // Fallback for NaN/Inf/high accuracy int len = snprintf(buf.data(), buf.size(), "%.*f", accuracy_decimals, value); if (len < 0) - return 0; // encoding error - // On truncation, snprintf returns would-be length; actual written is buf.size() - 1 + return 0; return static_cast(len) >= buf.size() ? buf.size() - 1 : static_cast(len); } size_t value_accuracy_with_uom_to_buf(std::span buf, float value, int8_t accuracy_decimals, StringRef unit_of_measurement) { + size_t len = value_accuracy_to_buf(buf, value, accuracy_decimals); if (unit_of_measurement.empty()) { - return value_accuracy_to_buf(buf, value, accuracy_decimals); + return len; } - normalize_accuracy_decimals(value, accuracy_decimals); - // snprintf returns chars that would be written (excluding null), or negative on error - int len = snprintf(buf.data(), buf.size(), "%.*f %s", accuracy_decimals, value, unit_of_measurement.c_str()); - if (len < 0) - return 0; // encoding error - // On truncation, snprintf returns would-be length; actual written is buf.size() - 1 - return static_cast(len) >= buf.size() ? buf.size() - 1 : static_cast(len); + // Append " " directly + char *p = buf.data() + len; + size_t remaining = buf.size() - len; + size_t uom_len = unit_of_measurement.size(); + // Need space for: ' ' + uom + '\0' + if (remaining < 2) { + return len; + } + *p++ = ' '; + remaining--; + size_t copy_len = std::min(uom_len, remaining - 1); + memcpy(p, unit_of_measurement.c_str(), copy_len); + p += copy_len; + *p = '\0'; + return static_cast(p - buf.data()); } int8_t step_to_accuracy_decimals(float step) { diff --git a/tests/components/core/test_value_accuracy.cpp b/tests/components/core/test_value_accuracy.cpp new file mode 100644 index 0000000000..a1fba90acf --- /dev/null +++ b/tests/components/core/test_value_accuracy.cpp @@ -0,0 +1,151 @@ +#include +#include +#include +#include +#include + +#include "esphome/core/helpers.h" + +namespace esphome::testing { + +// Helper to call value_accuracy_to_buf and return as string +static std::string va_to_string(float value, int8_t accuracy_decimals) { + char buf[VALUE_ACCURACY_MAX_LEN]; + std::span sp(buf); + size_t len = value_accuracy_to_buf(sp, value, accuracy_decimals); + return std::string(buf, len); +} + +// Helper: reference implementation using snprintf for comparison +static std::string va_reference(float value, int8_t accuracy_decimals) { + // Replicate normalize_accuracy_decimals logic + if (accuracy_decimals < 0) { + float divisor; + if (accuracy_decimals == -1) { + divisor = 10.0f; + } else if (accuracy_decimals == -2) { + divisor = 100.0f; + } else { + divisor = pow10_int(-accuracy_decimals); + } + value = roundf(value / divisor) * divisor; + accuracy_decimals = 0; + } + char buf[VALUE_ACCURACY_MAX_LEN]; + snprintf(buf, sizeof(buf), "%.*f", accuracy_decimals, value); + return std::string(buf); +} + +// --- Basic formatting --- + +TEST(ValueAccuracyToBuf, ZeroDecimals) { + EXPECT_EQ(va_to_string(23.456f, 0), "23"); + EXPECT_EQ(va_to_string(0.0f, 0), "0"); + EXPECT_EQ(va_to_string(100.0f, 0), "100"); + EXPECT_EQ(va_to_string(1.0f, 0), "1"); +} + +TEST(ValueAccuracyToBuf, OneDecimal) { + EXPECT_EQ(va_to_string(23.456f, 1), "23.5"); + EXPECT_EQ(va_to_string(0.0f, 1), "0.0"); + EXPECT_EQ(va_to_string(1.05f, 1), va_reference(1.05f, 1)); +} + +TEST(ValueAccuracyToBuf, TwoDecimals) { + EXPECT_EQ(va_to_string(23.456f, 2), "23.46"); + EXPECT_EQ(va_to_string(0.0f, 2), "0.00"); + EXPECT_EQ(va_to_string(1.005f, 2), va_reference(1.005f, 2)); +} + +TEST(ValueAccuracyToBuf, ThreeDecimals) { + EXPECT_EQ(va_to_string(23.456f, 3), "23.456"); + EXPECT_EQ(va_to_string(0.0f, 3), "0.000"); +} + +// --- Negative values --- + +TEST(ValueAccuracyToBuf, NegativeValues) { + EXPECT_EQ(va_to_string(-23.456f, 2), "-23.46"); + EXPECT_EQ(va_to_string(-0.5f, 1), "-0.5"); + EXPECT_EQ(va_to_string(-100.0f, 0), "-100"); +} + +// --- Negative accuracy_decimals (rounding to tens/hundreds) --- + +TEST(ValueAccuracyToBuf, NegativeAccuracy) { + EXPECT_EQ(va_to_string(1234.0f, -1), va_reference(1234.0f, -1)); + EXPECT_EQ(va_to_string(1234.0f, -2), va_reference(1234.0f, -2)); + EXPECT_EQ(va_to_string(56.0f, -1), va_reference(56.0f, -1)); +} + +// --- Special float values --- + +TEST(ValueAccuracyToBuf, NaN) { + std::string result = va_to_string(NAN, 2); + EXPECT_EQ(result, va_reference(NAN, 2)); +} + +TEST(ValueAccuracyToBuf, Infinity) { + std::string result = va_to_string(INFINITY, 2); + EXPECT_EQ(result, va_reference(INFINITY, 2)); +} + +TEST(ValueAccuracyToBuf, NegativeInfinity) { + std::string result = va_to_string(-INFINITY, 2); + EXPECT_EQ(result, va_reference(-INFINITY, 2)); +} + +// --- Edge cases --- + +TEST(ValueAccuracyToBuf, VerySmallValues) { + EXPECT_EQ(va_to_string(0.001f, 3), "0.001"); + EXPECT_EQ(va_to_string(0.001f, 2), "0.00"); + EXPECT_EQ(va_to_string(0.009f, 2), "0.01"); +} + +TEST(ValueAccuracyToBuf, LargeValues) { + EXPECT_EQ(va_to_string(999999.0f, 0), va_reference(999999.0f, 0)); + EXPECT_EQ(va_to_string(1013.25f, 2), "1013.25"); +} + +TEST(ValueAccuracyToBuf, Rounding) { + // 0.5 rounds up + EXPECT_EQ(va_to_string(23.5f, 0), "24"); + EXPECT_EQ(va_to_string(23.45f, 1), "23.5"); // float: 23.45 -> 23.4 or 23.5 + EXPECT_EQ(va_to_string(23.45f, 1), va_reference(23.45f, 1)); +} + +// --- Match snprintf for a range of typical sensor values --- + +TEST(ValueAccuracyToBuf, MatchesSnprintf) { + float test_values[] = {0.0f, 1.0f, -1.0f, 23.456f, -23.456f, 100.0f, 0.1f, 0.01f, 99.99f, 1013.25f, -40.0f}; + int8_t test_accuracies[] = {0, 1, 2, 3}; + + for (float value : test_values) { + for (int8_t acc : test_accuracies) { + EXPECT_EQ(va_to_string(value, acc), va_reference(value, acc)) + << "Mismatch for value=" << value << " accuracy=" << static_cast(acc); + } + } +} + +// --- Return value (length) --- + +TEST(ValueAccuracyToBuf, ReturnsCorrectLength) { + char buf[VALUE_ACCURACY_MAX_LEN]; + std::span sp(buf); + + size_t len = value_accuracy_to_buf(sp, 23.456f, 2); + EXPECT_EQ(len, 5u); // "23.46" + EXPECT_EQ(strlen(buf), len); + + len = value_accuracy_to_buf(sp, 0.0f, 0); + EXPECT_EQ(len, 1u); // "0" + EXPECT_EQ(strlen(buf), len); + + len = value_accuracy_to_buf(sp, -100.0f, 1); + EXPECT_EQ(len, 6u); // "-100.0" + EXPECT_EQ(strlen(buf), len); +} + +} // namespace esphome::testing From 5ac1f10b2873ff0798be71a542b091b11e079d8a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 8 Apr 2026 20:42:23 -1000 Subject: [PATCH 02/18] Fix clang-tidy braces --- esphome/core/helpers.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index 48ed45477f..30903e58ca 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -535,12 +535,13 @@ static size_t value_accuracy_to_buf_fast(char *buf, float value, int8_t accuracy value = -value; } uint32_t mult = 1; - if (accuracy_decimals == 1) + if (accuracy_decimals == 1) { mult = 10; - else if (accuracy_decimals == 2) + } else if (accuracy_decimals == 2) { mult = 100; - else if (accuracy_decimals == 3) + } else if (accuracy_decimals == 3) { mult = 1000; + } // Cast to double for the multiply to match snprintf's precision. // float*int loses bits at exact-half boundaries (e.g. 23.45f*10 = 234.5 in float, // but snprintf sees 234.500007... via double promotion and rounds differently). From ff1f0a0d46a98a85f7a798baa448fd454e8c8f0d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 8 Apr 2026 20:50:03 -1000 Subject: [PATCH 03/18] Extract inline helpers: small_pow10, uint32_to_str, frac_to_str --- esphome/core/helpers.cpp | 32 ++++---------------------------- esphome/core/helpers.h | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index 30903e58ca..244d307f8f 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -534,39 +534,15 @@ static size_t value_accuracy_to_buf_fast(char *buf, float value, int8_t accuracy *p++ = '-'; value = -value; } - uint32_t mult = 1; - if (accuracy_decimals == 1) { - mult = 10; - } else if (accuracy_decimals == 2) { - mult = 100; - } else if (accuracy_decimals == 3) { - mult = 1000; - } - // Cast to double for the multiply to match snprintf's precision. + uint32_t mult = small_pow10(accuracy_decimals); + // Cast to double for the multiply to match snprintf's rounding precision. // float*int loses bits at exact-half boundaries (e.g. 23.45f*10 = 234.5 in float, // but snprintf sees 234.500007... via double promotion and rounds differently). uint32_t scaled = static_cast(lrint(static_cast(value) * mult)); - uint32_t int_part = scaled / mult; - // Write integer part in reverse, then flip - char *start = p; - if (int_part == 0) { - *p++ = '0'; - } else { - while (int_part > 0) { - *p++ = '0' + (int_part % 10); - int_part /= 10; - } - std::reverse(start, p); - } + p = uint32_to_str(p, scaled / mult); if (accuracy_decimals > 0) { *p++ = '.'; - uint32_t frac = scaled % mult; - uint32_t d = mult / 10; - while (d > 0) { - *p++ = '0' + static_cast(frac / d); - frac %= d; - d /= 10; - } + p = frac_to_str(p, scaled % mult, mult / 10); } *p = '\0'; return static_cast(p - buf); diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index c26bbe17b7..ee6f76a09d 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -1295,6 +1295,46 @@ inline char *int8_to_str(char *buf, int8_t val) { return buf; } +/// Return 10^n for small non-negative n (0-3) as uint32_t, avoiding float. +inline uint32_t small_pow10(int8_t n) { + if (n == 1) { + return 10; + } else if (n == 2) { + return 100; + } else if (n == 3) { + return 1000; + } + return 1; +} + +/// Write unsigned 32-bit integer to buffer. Returns pointer past last char written. +/// Buffer must have at least 10 bytes free (max uint32 is 4294967295). +inline char *uint32_to_str(char *buf, uint32_t val) { + if (val == 0) { + *buf++ = '0'; + return buf; + } + char *start = buf; + while (val > 0) { + *buf++ = '0' + (val % 10); + val /= 10; + } + std::reverse(start, buf); + return buf; +} + +/// Write fractional digits with leading zeros to buffer. +/// frac is the fractional value, divisor is the highest place value (e.g. 100 for 3 digits). +/// Returns pointer past last char written. +inline char *frac_to_str(char *buf, uint32_t frac, uint32_t divisor) { + while (divisor > 0) { + *buf++ = '0' + static_cast(frac / divisor); + frac %= divisor; + divisor /= 10; + } + return buf; +} + /// Format byte array as lowercase hex to buffer (base implementation). char *format_hex_to(char *buffer, size_t buffer_size, const uint8_t *data, size_t length); From c8e636f25365640d38b67929ff7f5b4bd3cb2bdb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 8 Apr 2026 20:51:06 -1000 Subject: [PATCH 04/18] Add unit tests for small_pow10, uint32_to_str, frac_to_str --- tests/components/core/test_helpers.cpp | 119 +++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 tests/components/core/test_helpers.cpp diff --git a/tests/components/core/test_helpers.cpp b/tests/components/core/test_helpers.cpp new file mode 100644 index 0000000000..50df1492ad --- /dev/null +++ b/tests/components/core/test_helpers.cpp @@ -0,0 +1,119 @@ +#include +#include + +#include "esphome/core/helpers.h" + +namespace esphome::testing { + +// --- small_pow10() --- + +TEST(SmallPow10, Zero) { EXPECT_EQ(small_pow10(0), 1u); } +TEST(SmallPow10, One) { EXPECT_EQ(small_pow10(1), 10u); } +TEST(SmallPow10, Two) { EXPECT_EQ(small_pow10(2), 100u); } +TEST(SmallPow10, Three) { EXPECT_EQ(small_pow10(3), 1000u); } + +// --- uint32_to_str() --- + +TEST(Uint32ToStr, Zero) { + char buf[12]; + char *end = uint32_to_str(buf, 0); + *end = '\0'; + EXPECT_STREQ(buf, "0"); + EXPECT_EQ(end - buf, 1); +} + +TEST(Uint32ToStr, SingleDigit) { + char buf[12]; + char *end = uint32_to_str(buf, 7); + *end = '\0'; + EXPECT_STREQ(buf, "7"); +} + +TEST(Uint32ToStr, MultiDigit) { + char buf[12]; + char *end = uint32_to_str(buf, 12345); + *end = '\0'; + EXPECT_STREQ(buf, "12345"); + EXPECT_EQ(end - buf, 5); +} + +TEST(Uint32ToStr, Large) { + char buf[12]; + char *end = uint32_to_str(buf, 4294967295u); + *end = '\0'; + EXPECT_STREQ(buf, "4294967295"); + EXPECT_EQ(end - buf, 10); +} + +TEST(Uint32ToStr, PowersOfTen) { + char buf[12]; + char *end; + + end = uint32_to_str(buf, 10); + *end = '\0'; + EXPECT_STREQ(buf, "10"); + + end = uint32_to_str(buf, 100); + *end = '\0'; + EXPECT_STREQ(buf, "100"); + + end = uint32_to_str(buf, 1000); + *end = '\0'; + EXPECT_STREQ(buf, "1000"); +} + +// --- frac_to_str() --- + +TEST(FracToStr, OneDigit) { + char buf[8]; + char *end = frac_to_str(buf, 5, 1); + *end = '\0'; + EXPECT_STREQ(buf, "5"); + EXPECT_EQ(end - buf, 1); +} + +TEST(FracToStr, TwoDigits) { + char buf[8]; + char *end = frac_to_str(buf, 46, 10); + *end = '\0'; + EXPECT_STREQ(buf, "46"); +} + +TEST(FracToStr, ThreeDigits) { + char buf[8]; + char *end = frac_to_str(buf, 456, 100); + *end = '\0'; + EXPECT_STREQ(buf, "456"); + EXPECT_EQ(end - buf, 3); +} + +TEST(FracToStr, LeadingZeros) { + char buf[8]; + char *end = frac_to_str(buf, 1, 100); + *end = '\0'; + EXPECT_STREQ(buf, "001"); + + end = frac_to_str(buf, 5, 10); + *end = '\0'; + EXPECT_STREQ(buf, "05"); +} + +TEST(FracToStr, AllZeros) { + char buf[8]; + char *end = frac_to_str(buf, 0, 100); + *end = '\0'; + EXPECT_STREQ(buf, "000"); + + end = frac_to_str(buf, 0, 1); + *end = '\0'; + EXPECT_STREQ(buf, "0"); +} + +TEST(FracToStr, ZeroDivisor) { + char buf[8]; + buf[0] = 'X'; + char *end = frac_to_str(buf, 0, 0); + EXPECT_EQ(end, buf); // writes nothing +} + +} // namespace esphome::testing From e52c3b01e60e1f0c64be233aac64cddfc2f7fe76 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 8 Apr 2026 20:52:51 -1000 Subject: [PATCH 05/18] Extract buf_append_sep_str helper, add tests --- esphome/core/helpers.cpp | 18 +++---------- esphome/core/helpers.h | 15 +++++++++++ tests/components/core/test_helpers.cpp | 35 ++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 15 deletions(-) diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index 244d307f8f..b09e8ccbdf 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -569,21 +569,9 @@ size_t value_accuracy_with_uom_to_buf(std::span bu if (unit_of_measurement.empty()) { return len; } - // Append " " directly - char *p = buf.data() + len; - size_t remaining = buf.size() - len; - size_t uom_len = unit_of_measurement.size(); - // Need space for: ' ' + uom + '\0' - if (remaining < 2) { - return len; - } - *p++ = ' '; - remaining--; - size_t copy_len = std::min(uom_len, remaining - 1); - memcpy(p, unit_of_measurement.c_str(), copy_len); - p += copy_len; - *p = '\0'; - return static_cast(p - buf.data()); + char *end = buf_append_sep_str(buf.data() + len, buf.size() - len, ' ', unit_of_measurement.c_str(), + unit_of_measurement.size()); + return static_cast(end - buf.data()); } int8_t step_to_accuracy_decimals(float step) { diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index ee6f76a09d..3b93a2c476 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -1295,6 +1295,21 @@ inline char *int8_to_str(char *buf, int8_t val) { return buf; } +/// Append a separator char and a string to a buffer, respecting remaining space. +/// Returns pointer past last char written (null terminator is written). +inline char *buf_append_sep_str(char *buf, size_t remaining, char separator, const char *str, size_t str_len) { + if (remaining < 2) { + return buf; + } + *buf++ = separator; + remaining--; + size_t copy_len = std::min(str_len, remaining - 1); + memcpy(buf, str, copy_len); + buf += copy_len; + *buf = '\0'; + return buf; +} + /// Return 10^n for small non-negative n (0-3) as uint32_t, avoiding float. inline uint32_t small_pow10(int8_t n) { if (n == 1) { diff --git a/tests/components/core/test_helpers.cpp b/tests/components/core/test_helpers.cpp index 50df1492ad..261a4111b1 100644 --- a/tests/components/core/test_helpers.cpp +++ b/tests/components/core/test_helpers.cpp @@ -116,4 +116,39 @@ TEST(FracToStr, ZeroDivisor) { EXPECT_EQ(end, buf); // writes nothing } +// --- buf_append_sep_str() --- + +TEST(BufAppendSepStr, Basic) { + char buf[32] = "23.46"; + char *start = buf + 5; + char *end = buf_append_sep_str(start, sizeof(buf) - 5, ' ', "°C", 3); + EXPECT_STREQ(buf, "23.46 °C"); + EXPECT_EQ(end - buf, 9); // "°C" is 3 bytes (UTF-8) +} + +TEST(BufAppendSepStr, EmptyString) { + char buf[32] = "100"; + char *start = buf + 3; + char *end = buf_append_sep_str(start, sizeof(buf) - 3, ' ', "", 0); + EXPECT_STREQ(buf, "100 "); + EXPECT_EQ(end - start, 1); // just the separator +} + +TEST(BufAppendSepStr, NoRoom) { + char buf[8] = "1234567"; + char *start = buf + 7; + char *end = buf_append_sep_str(start, 1, ' ', "unit", 4); + EXPECT_EQ(end, start); // nothing written +} + +TEST(BufAppendSepStr, Truncation) { + char buf[8] = "val"; + char *start = buf + 3; + // remaining = 5, separator takes 1, so 3 chars of string fit + null + char *end = buf_append_sep_str(start, 5, ' ', "longunit", 8); + *end = '\0'; + EXPECT_STREQ(buf, "val lon"); + EXPECT_EQ(end - buf, 7); +} + } // namespace esphome::testing From 3b45179948e01e52ecfadd7c88c012773a20b7d0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 8 Apr 2026 21:01:32 -1000 Subject: [PATCH 06/18] Split uint32_to_str/frac_to_str into internal and public API - uint32_to_str_(): raw pointer, internal use - uint32_to_str(): template with compile-time buffer size check - frac_to_str_(): raw pointer, internal use - small_pow10(): simplify to ternary chain --- esphome/core/helpers.cpp | 4 +-- esphome/core/helpers.h | 33 ++++++++++--------- tests/components/core/test_helpers.cpp | 44 ++++++++++++++++---------- 3 files changed, 48 insertions(+), 33 deletions(-) diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index b09e8ccbdf..dc4560b5ef 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -539,10 +539,10 @@ static size_t value_accuracy_to_buf_fast(char *buf, float value, int8_t accuracy // float*int loses bits at exact-half boundaries (e.g. 23.45f*10 = 234.5 in float, // but snprintf sees 234.500007... via double promotion and rounds differently). uint32_t scaled = static_cast(lrint(static_cast(value) * mult)); - p = uint32_to_str(p, scaled / mult); + p = uint32_to_str_(p, scaled / mult); if (accuracy_decimals > 0) { *p++ = '.'; - p = frac_to_str(p, scaled % mult, mult / 10); + p = frac_to_str_(p, scaled % mult, mult / 10); } *p = '\0'; return static_cast(p - buf); diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 3b93a2c476..11696772be 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -1311,20 +1311,14 @@ inline char *buf_append_sep_str(char *buf, size_t remaining, char separator, con } /// Return 10^n for small non-negative n (0-3) as uint32_t, avoiding float. -inline uint32_t small_pow10(int8_t n) { - if (n == 1) { - return 10; - } else if (n == 2) { - return 100; - } else if (n == 3) { - return 1000; - } - return 1; -} +inline uint32_t small_pow10(int8_t n) { return n == 3 ? 1000 : n == 2 ? 100 : n == 1 ? 10 : 1; } -/// Write unsigned 32-bit integer to buffer. Returns pointer past last char written. -/// Buffer must have at least 10 bytes free (max uint32 is 4294967295). -inline char *uint32_to_str(char *buf, uint32_t val) { +/// Minimum buffer size for uint32_to_str: 10 digits + null terminator. +static constexpr size_t UINT32_MAX_STR_SIZE = 11; + +/// Write unsigned 32-bit integer to buffer (internal, no size check). +/// Buffer must have at least 10 bytes free. Returns pointer past last char written. +inline char *uint32_to_str_(char *buf, uint32_t val) { if (val == 0) { *buf++ = '0'; return buf; @@ -1338,10 +1332,19 @@ inline char *uint32_to_str(char *buf, uint32_t val) { return buf; } -/// Write fractional digits with leading zeros to buffer. +/// Write unsigned 32-bit integer to buffer with compile-time size check. +/// Null-terminates the output. Returns number of chars written (excluding null). +template inline size_t uint32_to_str(char (&buf)[N], uint32_t val) { + static_assert(N >= UINT32_MAX_STR_SIZE, "Buffer too small for uint32 (need 11 bytes)"); + char *end = uint32_to_str_(buf, val); + *end = '\0'; + return static_cast(end - buf); +} + +/// Write fractional digits with leading zeros to buffer (internal, no size check). /// frac is the fractional value, divisor is the highest place value (e.g. 100 for 3 digits). /// Returns pointer past last char written. -inline char *frac_to_str(char *buf, uint32_t frac, uint32_t divisor) { +inline char *frac_to_str_(char *buf, uint32_t frac, uint32_t divisor) { while (divisor > 0) { *buf++ = '0' + static_cast(frac / divisor); frac %= divisor; diff --git a/tests/components/core/test_helpers.cpp b/tests/components/core/test_helpers.cpp index 261a4111b1..6c0490b6c6 100644 --- a/tests/components/core/test_helpers.cpp +++ b/tests/components/core/test_helpers.cpp @@ -16,7 +16,7 @@ TEST(SmallPow10, Three) { EXPECT_EQ(small_pow10(3), 1000u); } TEST(Uint32ToStr, Zero) { char buf[12]; - char *end = uint32_to_str(buf, 0); + char *end = uint32_to_str_(buf, 0); *end = '\0'; EXPECT_STREQ(buf, "0"); EXPECT_EQ(end - buf, 1); @@ -24,14 +24,14 @@ TEST(Uint32ToStr, Zero) { TEST(Uint32ToStr, SingleDigit) { char buf[12]; - char *end = uint32_to_str(buf, 7); + char *end = uint32_to_str_(buf, 7); *end = '\0'; EXPECT_STREQ(buf, "7"); } TEST(Uint32ToStr, MultiDigit) { char buf[12]; - char *end = uint32_to_str(buf, 12345); + char *end = uint32_to_str_(buf, 12345); *end = '\0'; EXPECT_STREQ(buf, "12345"); EXPECT_EQ(end - buf, 5); @@ -39,7 +39,7 @@ TEST(Uint32ToStr, MultiDigit) { TEST(Uint32ToStr, Large) { char buf[12]; - char *end = uint32_to_str(buf, 4294967295u); + char *end = uint32_to_str_(buf, 4294967295u); *end = '\0'; EXPECT_STREQ(buf, "4294967295"); EXPECT_EQ(end - buf, 10); @@ -49,24 +49,36 @@ TEST(Uint32ToStr, PowersOfTen) { char buf[12]; char *end; - end = uint32_to_str(buf, 10); + end = uint32_to_str_(buf, 10); *end = '\0'; EXPECT_STREQ(buf, "10"); - end = uint32_to_str(buf, 100); + end = uint32_to_str_(buf, 100); *end = '\0'; EXPECT_STREQ(buf, "100"); - end = uint32_to_str(buf, 1000); + end = uint32_to_str_(buf, 1000); *end = '\0'; EXPECT_STREQ(buf, "1000"); } -// --- frac_to_str() --- +// --- uint32_to_str() (public, template with size check) --- + +TEST(Uint32ToStr, PublicApi) { + char buf[UINT32_MAX_STR_SIZE]; + EXPECT_EQ(uint32_to_str(buf, 0), 1u); + EXPECT_STREQ(buf, "0"); + EXPECT_EQ(uint32_to_str(buf, 12345), 5u); + EXPECT_STREQ(buf, "12345"); + EXPECT_EQ(uint32_to_str(buf, 4294967295u), 10u); + EXPECT_STREQ(buf, "4294967295"); +} + +// --- frac_to_str_() --- TEST(FracToStr, OneDigit) { char buf[8]; - char *end = frac_to_str(buf, 5, 1); + char *end = frac_to_str_(buf, 5, 1); *end = '\0'; EXPECT_STREQ(buf, "5"); EXPECT_EQ(end - buf, 1); @@ -74,14 +86,14 @@ TEST(FracToStr, OneDigit) { TEST(FracToStr, TwoDigits) { char buf[8]; - char *end = frac_to_str(buf, 46, 10); + char *end = frac_to_str_(buf, 46, 10); *end = '\0'; EXPECT_STREQ(buf, "46"); } TEST(FracToStr, ThreeDigits) { char buf[8]; - char *end = frac_to_str(buf, 456, 100); + char *end = frac_to_str_(buf, 456, 100); *end = '\0'; EXPECT_STREQ(buf, "456"); EXPECT_EQ(end - buf, 3); @@ -89,22 +101,22 @@ TEST(FracToStr, ThreeDigits) { TEST(FracToStr, LeadingZeros) { char buf[8]; - char *end = frac_to_str(buf, 1, 100); + char *end = frac_to_str_(buf, 1, 100); *end = '\0'; EXPECT_STREQ(buf, "001"); - end = frac_to_str(buf, 5, 10); + end = frac_to_str_(buf, 5, 10); *end = '\0'; EXPECT_STREQ(buf, "05"); } TEST(FracToStr, AllZeros) { char buf[8]; - char *end = frac_to_str(buf, 0, 100); + char *end = frac_to_str_(buf, 0, 100); *end = '\0'; EXPECT_STREQ(buf, "000"); - end = frac_to_str(buf, 0, 1); + end = frac_to_str_(buf, 0, 1); *end = '\0'; EXPECT_STREQ(buf, "0"); } @@ -112,7 +124,7 @@ TEST(FracToStr, AllZeros) { TEST(FracToStr, ZeroDivisor) { char buf[8]; buf[0] = 'X'; - char *end = frac_to_str(buf, 0, 0); + char *end = frac_to_str_(buf, 0, 0); EXPECT_EQ(end, buf); // writes nothing } From e0fa1340e2fc2b38d4f435162e6fc2baafa13f7b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 8 Apr 2026 21:02:20 -1000 Subject: [PATCH 07/18] Use std::span for uint32_to_str public API --- esphome/core/helpers.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 11696772be..009f08c323 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -1334,11 +1334,10 @@ inline char *uint32_to_str_(char *buf, uint32_t val) { /// Write unsigned 32-bit integer to buffer with compile-time size check. /// Null-terminates the output. Returns number of chars written (excluding null). -template inline size_t uint32_to_str(char (&buf)[N], uint32_t val) { - static_assert(N >= UINT32_MAX_STR_SIZE, "Buffer too small for uint32 (need 11 bytes)"); - char *end = uint32_to_str_(buf, val); +inline size_t uint32_to_str(std::span buf, uint32_t val) { + char *end = uint32_to_str_(buf.data(), val); *end = '\0'; - return static_cast(end - buf); + return static_cast(end - buf.data()); } /// Write fractional digits with leading zeros to buffer (internal, no size check). From 6b4b65346240c1cb93084141b28d07119fdc1d43 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 14 Apr 2026 03:18:38 -1000 Subject: [PATCH 08/18] [globals] Fix TemplatableFn deprecation warning for globals.set (#15733) --- esphome/components/globals/__init__.py | 7 ++++++- tests/components/globals/common.yaml | 8 ++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/esphome/components/globals/__init__.py b/esphome/components/globals/__init__.py index ec6730a41c..46725fe6dd 100644 --- a/esphome/components/globals/__init__.py +++ b/esphome/components/globals/__init__.py @@ -108,8 +108,13 @@ async def globals_set_to_code(config, action_id, template_arg, args): full_id, paren = await cg.get_variable_with_full_id(config[CONF_ID]) template_arg = cg.TemplateArguments(full_id.type, *template_arg) var = cg.new_Pvariable(action_id, template_arg, paren) + # Use the global's value_type alias as the lambda return type so + # TemplatableFn stores a direct function pointer instead of going through + # the deprecated converting trampoline when the value expression deduces + # to a different type (e.g. int literal assigned to a float global). + value_type = cg.RawExpression(f"{full_id.type}::value_type") templ = await cg.templatable( - config[CONF_VALUE], args, None, to_exp=cg.RawExpression, wrap_constant=True + config[CONF_VALUE], args, value_type, to_exp=cg.RawExpression ) cg.add(var.set_value(templ)) return var diff --git a/tests/components/globals/common.yaml b/tests/components/globals/common.yaml index 35dca0624f..6d5721d3be 100644 --- a/tests/components/globals/common.yaml +++ b/tests/components/globals/common.yaml @@ -4,6 +4,14 @@ esphome: - globals.set: id: glob_int value: "10" + # Set a float global with an integer literal - must emit the correct + # return type so TemplatableFn stores a direct function pointer. + - globals.set: + id: glob_float + value: "102" + - globals.set: + id: glob_float + value: !lambda "return 42;" globals: - id: glob_int From 2a530a4bf4618c08999db8b3fdfa52b1e2f09271 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 14 Apr 2026 07:48:33 -1000 Subject: [PATCH 09/18] [core] Optimize format_hex_internal by splitting separator loop (#15594) --- esphome/core/helpers.cpp | 17 ++-- esphome/core/helpers.h | 6 +- tests/components/core/test_helpers.cpp | 120 +++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 10 deletions(-) create mode 100644 tests/components/core/test_helpers.cpp diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index 5940f6ec98..cbe22dd09a 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -347,17 +347,18 @@ std::string format_mac_address_pretty(const uint8_t *mac) { return std::string(buf); } -// Internal helper for hex formatting - base is 'a' for lowercase or 'A' for uppercase +// Internal helper for hex formatting - base is 'a' for lowercase or 'A' for uppercase. +// When separator is set, it is written unconditionally after each byte and the last +// one is overwritten with '\0', eliminating the per-byte `i < length - 1` check. static char *format_hex_internal(char *buffer, size_t buffer_size, const uint8_t *data, size_t length, char separator, char base) { - if (length == 0) { - buffer[0] = '\0'; + if (length == 0 || buffer_size == 0) { + if (buffer_size > 0) + buffer[0] = '\0'; return buffer; } - // With separator: total length is 3*length (2*length hex chars, (length-1) separators, 1 null terminator) - // Without separator: total length is 2*length + 1 (2*length hex chars, 1 null terminator) uint8_t stride = separator ? 3 : 2; - size_t max_bytes = separator ? (buffer_size / stride) : ((buffer_size - 1) / stride); + size_t max_bytes = separator ? (buffer_size / 3) : ((buffer_size - 1) / 2); if (max_bytes == 0) { buffer[0] = '\0'; return buffer; @@ -369,10 +370,12 @@ static char *format_hex_internal(char *buffer, size_t buffer_size, const uint8_t size_t pos = i * stride; buffer[pos] = format_hex_char(data[i] >> 4, base); buffer[pos + 1] = format_hex_char(data[i] & 0x0F, base); - if (separator && i < length - 1) { + if (separator) { buffer[pos + 2] = separator; } } + // With separator: overwrite last separator with '\0' + // Without: write '\0' after last hex char buffer[length * stride - (separator ? 1 : 0)] = '\0'; return buffer; } diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index c26bbe17b7..3c42d7df07 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -1263,13 +1263,13 @@ constexpr uint8_t parse_hex_char(char c) { } /// Convert a nibble (0-15) to hex char with specified base ('a' for lowercase, 'A' for uppercase) -inline char format_hex_char(uint8_t v, char base) { return v >= 10 ? base + (v - 10) : '0' + v; } +ESPHOME_ALWAYS_INLINE inline char format_hex_char(uint8_t v, char base) { return v >= 10 ? base + (v - 10) : '0' + v; } /// Convert a nibble (0-15) to lowercase hex char -inline char format_hex_char(uint8_t v) { return format_hex_char(v, 'a'); } +ESPHOME_ALWAYS_INLINE inline char format_hex_char(uint8_t v) { return format_hex_char(v, 'a'); } /// Convert a nibble (0-15) to uppercase hex char (used for pretty printing) -inline char format_hex_pretty_char(uint8_t v) { return format_hex_char(v, 'A'); } +ESPHOME_ALWAYS_INLINE inline char format_hex_pretty_char(uint8_t v) { return format_hex_char(v, 'A'); } /// Write int8 value to buffer without modulo operations. /// Buffer must have at least 4 bytes free. Returns pointer past last char written. diff --git a/tests/components/core/test_helpers.cpp b/tests/components/core/test_helpers.cpp new file mode 100644 index 0000000000..00169621c3 --- /dev/null +++ b/tests/components/core/test_helpers.cpp @@ -0,0 +1,120 @@ +#include +#include + +#include "esphome/core/helpers.h" + +namespace esphome::core::testing { + +// --- format_hex_to() --- + +TEST(FormatHexTo, Basic) { + const uint8_t data[] = {0xAB, 0xCD, 0xEF}; + char buffer[7]; // 3 * 2 + 1 + format_hex_to(buffer, data, 3); + EXPECT_STREQ(buffer, "abcdef"); +} + +TEST(FormatHexTo, SingleByte) { + const uint8_t data[] = {0x0F}; + char buffer[3]; + format_hex_to(buffer, data, 1); + EXPECT_STREQ(buffer, "0f"); +} + +TEST(FormatHexTo, ZeroLength) { + char buffer[4] = "xxx"; + format_hex_to(buffer, static_cast(sizeof(buffer)), static_cast(nullptr), 0); + EXPECT_STREQ(buffer, ""); +} + +TEST(FormatHexTo, ZeroBufferSize) { + char buffer[4] = "xxx"; + const uint8_t data[] = {0xAB}; + format_hex_to(buffer, static_cast(0), data, 1); + // Should not crash, buffer unchanged + EXPECT_EQ(buffer[0], 'x'); +} + +TEST(FormatHexTo, BufferTooSmall) { + const uint8_t data[] = {0xAB, 0xCD, 0xEF}; + char buffer[5]; // only room for 2 bytes + format_hex_to(buffer, data, 3); + EXPECT_STREQ(buffer, "abcd"); +} + +TEST(FormatHexTo, MacAddress) { + const uint8_t mac[] = {0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}; + char buffer[13]; + format_hex_to(buffer, mac, 6); + EXPECT_STREQ(buffer, "aabbccddeeff"); +} + +// --- format_hex_pretty_to() --- + +TEST(FormatHexPrettyTo, BasicColon) { + const uint8_t data[] = {0xAB, 0xCD, 0xEF}; + char buffer[9]; // 3 * 3 + format_hex_pretty_to(buffer, data, 3); + EXPECT_STREQ(buffer, "AB:CD:EF"); +} + +TEST(FormatHexPrettyTo, SingleByte) { + const uint8_t data[] = {0x0F}; + char buffer[3]; + format_hex_pretty_to(buffer, data, 1); + EXPECT_STREQ(buffer, "0F"); +} + +TEST(FormatHexPrettyTo, ZeroLength) { + char buffer[4] = "xxx"; + format_hex_pretty_to(buffer, static_cast(sizeof(buffer)), static_cast(nullptr), 0); + EXPECT_STREQ(buffer, ""); +} + +TEST(FormatHexPrettyTo, ZeroBufferSize) { + char buffer[4] = "xxx"; + const uint8_t data[] = {0xAB}; + format_hex_pretty_to(buffer, static_cast(0), data, 1); + EXPECT_EQ(buffer[0], 'x'); +} + +TEST(FormatHexPrettyTo, CustomSeparator) { + const uint8_t data[] = {0xAA, 0xBB, 0xCC}; + char buffer[9]; + format_hex_pretty_to(buffer, data, 3, '-'); + EXPECT_STREQ(buffer, "AA-BB-CC"); +} + +// --- format_mac_addr_upper() --- + +TEST(FormatMacAddrUpper, Basic) { + const uint8_t mac[] = {0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}; + char buffer[MAC_ADDRESS_PRETTY_BUFFER_SIZE]; + format_mac_addr_upper(mac, buffer); + EXPECT_STREQ(buffer, "AA:BB:CC:DD:EE:FF"); +} + +TEST(FormatMacAddrUpper, AllZeros) { + const uint8_t mac[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + char buffer[MAC_ADDRESS_PRETTY_BUFFER_SIZE]; + format_mac_addr_upper(mac, buffer); + EXPECT_STREQ(buffer, "00:00:00:00:00:00"); +} + +// --- format_hex_char() --- + +TEST(FormatHexChar, LowercaseDigits) { + EXPECT_EQ(format_hex_char(0), '0'); + EXPECT_EQ(format_hex_char(9), '9'); + EXPECT_EQ(format_hex_char(10), 'a'); + EXPECT_EQ(format_hex_char(15), 'f'); +} + +TEST(FormatHexChar, UppercaseDigits) { + EXPECT_EQ(format_hex_pretty_char(0), '0'); + EXPECT_EQ(format_hex_pretty_char(9), '9'); + EXPECT_EQ(format_hex_pretty_char(10), 'A'); + EXPECT_EQ(format_hex_pretty_char(15), 'F'); +} + +} // namespace esphome::core::testing From c833ff4a8480338355011792ffea6aa581b774e2 Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Tue, 14 Apr 2026 13:49:18 -0400 Subject: [PATCH 10/18] [audio] Add/configure microDecoder library in preparation for use in future PRs (#15679) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- esphome/components/audio/__init__.py | 25 ++++++++++++++++++++++++- esphome/idf_component.yml | 2 ++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/esphome/components/audio/__init__.py b/esphome/components/audio/__init__.py index 8f2102de6a..fee582ca25 100644 --- a/esphome/components/audio/__init__.py +++ b/esphome/components/audio/__init__.py @@ -1,7 +1,11 @@ from dataclasses import dataclass import esphome.codegen as cg -from esphome.components.esp32 import add_idf_component, include_builtin_idf_component +from esphome.components.esp32 import ( + add_idf_component, + add_idf_sdkconfig_option, + include_builtin_idf_component, +) import esphome.config_validation as cv from esphome.const import CONF_BITS_PER_SAMPLE, CONF_NUM_CHANNELS, CONF_SAMPLE_RATE from esphome.core import CORE @@ -27,6 +31,7 @@ class AudioData: flac_support: bool = False mp3_support: bool = False opus_support: bool = False + micro_decoder_support: bool = False def _get_data() -> AudioData: @@ -50,6 +55,11 @@ def request_opus_support() -> None: _get_data().opus_support = True +def request_micro_decoder_support() -> None: + """Request micro-decoder library support for audio decoding.""" + _get_data().micro_decoder_support = True + + CONF_MIN_BITS_PER_SAMPLE = "min_bits_per_sample" CONF_MAX_BITS_PER_SAMPLE = "max_bits_per_sample" CONF_MIN_CHANNELS = "min_channels" @@ -208,6 +218,19 @@ async def to_code(config): ) data = _get_data() + + if data.micro_decoder_support: + add_idf_component(name="esphome/micro-decoder", ref="0.1.1") + + # All codecs are enabled by default in micro-decoder, so disable the ones that aren't requested to save flash + if not data.flac_support: + add_idf_sdkconfig_option("CONFIG_MICRO_DECODER_CODEC_FLAC", False) + if not data.mp3_support: + add_idf_sdkconfig_option("CONFIG_MICRO_DECODER_CODEC_MP3", False) + if not data.opus_support: + add_idf_sdkconfig_option("CONFIG_MICRO_DECODER_CODEC_OPUS", False) + + # Legacy audio_decoder.cpp support defines and components if data.flac_support: cg.add_define("USE_AUDIO_FLAC_SUPPORT") add_idf_component(name="esphome/micro-flac", ref="0.1.1") diff --git a/esphome/idf_component.yml b/esphome/idf_component.yml index bf42730e67..f4e3e751ec 100644 --- a/esphome/idf_component.yml +++ b/esphome/idf_component.yml @@ -3,6 +3,8 @@ dependencies: version: "7.4.2" esphome/esp-audio-libs: version: 2.0.4 + esphome/micro-decoder: + version: 0.1.1 esphome/micro-flac: version: 0.1.1 esphome/micro-opus: From 5ba8c644e4f0d47ffcd524be544323ee92d2b343 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 14 Apr 2026 07:49:27 -1000 Subject: [PATCH 11/18] [ld24xx] Replace heap-allocated SensorWithDedup with inline SensorWithDedup (#15676) --- esphome/components/ld2410/ld2410.cpp | 21 +++++++------ esphome/components/ld2410/ld2410.h | 4 +-- esphome/components/ld2412/ld2412.cpp | 29 +++++++++--------- esphome/components/ld2412/ld2412.h | 4 +-- esphome/components/ld2450/ld2450.cpp | 20 ++++++------- esphome/components/ld2450/ld2450.h | 18 ++++++------ esphome/components/ld24xx/ld24xx.h | 44 ++++++++++++++-------------- 7 files changed, 69 insertions(+), 71 deletions(-) diff --git a/esphome/components/ld2410/ld2410.cpp b/esphome/components/ld2410/ld2410.cpp index f10e7ec0aa..32e49c643f 100644 --- a/esphome/components/ld2410/ld2410.cpp +++ b/esphome/components/ld2410/ld2410.cpp @@ -360,8 +360,8 @@ void LD2410Component::handle_periodic_data_() { */ #ifdef USE_SENSOR SAFE_PUBLISH_SENSOR(this->moving_target_distance_sensor_, - encode_uint16(this->buffer_data_[MOVING_TARGET_HIGH], this->buffer_data_[MOVING_TARGET_LOW])) - SAFE_PUBLISH_SENSOR(this->moving_target_energy_sensor_, this->buffer_data_[MOVING_ENERGY]) + encode_uint16(this->buffer_data_[MOVING_TARGET_HIGH], this->buffer_data_[MOVING_TARGET_LOW])); + SAFE_PUBLISH_SENSOR(this->moving_target_energy_sensor_, this->buffer_data_[MOVING_ENERGY]); SAFE_PUBLISH_SENSOR(this->still_target_distance_sensor_, encode_uint16(this->buffer_data_[STILL_TARGET_HIGH], this->buffer_data_[STILL_TARGET_LOW])); SAFE_PUBLISH_SENSOR(this->still_target_energy_sensor_, this->buffer_data_[STILL_ENERGY]); @@ -375,26 +375,26 @@ void LD2410Component::handle_periodic_data_() { Moving energy: 20~28th bytes */ for (uint8_t i = 0; i < TOTAL_GATES; i++) { - SAFE_PUBLISH_SENSOR(this->gate_move_sensors_[i], this->buffer_data_[MOVING_SENSOR_START + i]) + SAFE_PUBLISH_SENSOR(this->gate_move_sensors_[i], this->buffer_data_[MOVING_SENSOR_START + i]); } /* Still energy: 29~37th bytes */ for (uint8_t i = 0; i < TOTAL_GATES; i++) { - SAFE_PUBLISH_SENSOR(this->gate_still_sensors_[i], this->buffer_data_[STILL_SENSOR_START + i]) + SAFE_PUBLISH_SENSOR(this->gate_still_sensors_[i], this->buffer_data_[STILL_SENSOR_START + i]); } /* Light sensor: 38th bytes */ - SAFE_PUBLISH_SENSOR(this->light_sensor_, this->buffer_data_[LIGHT_SENSOR]) + SAFE_PUBLISH_SENSOR(this->light_sensor_, this->buffer_data_[LIGHT_SENSOR]); } else { for (auto &gate_move_sensor : this->gate_move_sensors_) { - SAFE_PUBLISH_SENSOR_UNKNOWN(gate_move_sensor) + SAFE_PUBLISH_SENSOR_UNKNOWN(gate_move_sensor); } for (auto &gate_still_sensor : this->gate_still_sensors_) { - SAFE_PUBLISH_SENSOR_UNKNOWN(gate_still_sensor) + SAFE_PUBLISH_SENSOR_UNKNOWN(gate_still_sensor); } - SAFE_PUBLISH_SENSOR_UNKNOWN(this->light_sensor_) + SAFE_PUBLISH_SENSOR_UNKNOWN(this->light_sensor_); } #endif #ifdef USE_BINARY_SENSOR @@ -786,13 +786,12 @@ void LD2410Component::set_light_out_control() { } #ifdef USE_SENSOR -// These could leak memory, but they are only set once prior to 'setup()' and should never be used again. void LD2410Component::set_gate_move_sensor(uint8_t gate, sensor::Sensor *s) { - this->gate_move_sensors_[gate] = new SensorWithDedup(s); + this->gate_move_sensors_[gate].set_sensor(s); } void LD2410Component::set_gate_still_sensor(uint8_t gate, sensor::Sensor *s) { - this->gate_still_sensors_[gate] = new SensorWithDedup(s); + this->gate_still_sensors_[gate].set_sensor(s); } #endif diff --git a/esphome/components/ld2410/ld2410.h b/esphome/components/ld2410/ld2410.h index 687ed21d1d..31186b135f 100644 --- a/esphome/components/ld2410/ld2410.h +++ b/esphome/components/ld2410/ld2410.h @@ -129,8 +129,8 @@ class LD2410Component : public Component, public uart::UARTDevice { std::array gate_still_threshold_numbers_{}; #endif #ifdef USE_SENSOR - std::array *, TOTAL_GATES> gate_move_sensors_{}; - std::array *, TOTAL_GATES> gate_still_sensors_{}; + std::array, TOTAL_GATES> gate_move_sensors_{}; + std::array, TOTAL_GATES> gate_still_sensors_{}; #endif }; diff --git a/esphome/components/ld2412/ld2412.cpp b/esphome/components/ld2412/ld2412.cpp index 38e1a59aba..a502ae3c10 100644 --- a/esphome/components/ld2412/ld2412.cpp +++ b/esphome/components/ld2412/ld2412.cpp @@ -397,12 +397,12 @@ void LD2412Component::handle_periodic_data_() { */ #ifdef USE_SENSOR SAFE_PUBLISH_SENSOR(this->moving_target_distance_sensor_, - encode_uint16(this->buffer_data_[MOVING_TARGET_HIGH], this->buffer_data_[MOVING_TARGET_LOW])) - SAFE_PUBLISH_SENSOR(this->moving_target_energy_sensor_, this->buffer_data_[MOVING_ENERGY]) + encode_uint16(this->buffer_data_[MOVING_TARGET_HIGH], this->buffer_data_[MOVING_TARGET_LOW])); + SAFE_PUBLISH_SENSOR(this->moving_target_energy_sensor_, this->buffer_data_[MOVING_ENERGY]); SAFE_PUBLISH_SENSOR(this->still_target_distance_sensor_, - encode_uint16(this->buffer_data_[STILL_TARGET_HIGH], this->buffer_data_[STILL_TARGET_LOW])) - SAFE_PUBLISH_SENSOR(this->still_target_energy_sensor_, this->buffer_data_[STILL_ENERGY]) - if (this->detection_distance_sensor_ != nullptr) { + encode_uint16(this->buffer_data_[STILL_TARGET_HIGH], this->buffer_data_[STILL_TARGET_LOW])); + SAFE_PUBLISH_SENSOR(this->still_target_energy_sensor_, this->buffer_data_[STILL_ENERGY]); + if (this->detection_distance_sensor_.has_sensor()) { int new_detect_distance = 0; if (target_state != 0x00 && (target_state & MOVE_BITMASK)) { new_detect_distance = @@ -410,7 +410,7 @@ void LD2412Component::handle_periodic_data_() { } else if (target_state != 0x00) { new_detect_distance = encode_uint16(this->buffer_data_[STILL_TARGET_HIGH], this->buffer_data_[STILL_TARGET_LOW]); } - this->detection_distance_sensor_->publish_state_if_not_dup(new_detect_distance); + this->detection_distance_sensor_.publish_state_if_not_dup(new_detect_distance); } if (engineering_mode) { // Engineering mode needs at least LIGHT_SENSOR + 1 bytes @@ -423,27 +423,27 @@ void LD2412Component::handle_periodic_data_() { Moving energy: 20~28th bytes */ for (uint8_t i = 0; i < TOTAL_GATES; i++) { - SAFE_PUBLISH_SENSOR(this->gate_move_sensors_[i], this->buffer_data_[MOVING_SENSOR_START + i]) + SAFE_PUBLISH_SENSOR(this->gate_move_sensors_[i], this->buffer_data_[MOVING_SENSOR_START + i]); } /* Still energy: 29~37th bytes */ for (uint8_t i = 0; i < TOTAL_GATES; i++) { - SAFE_PUBLISH_SENSOR(this->gate_still_sensors_[i], this->buffer_data_[STILL_SENSOR_START + i]) + SAFE_PUBLISH_SENSOR(this->gate_still_sensors_[i], this->buffer_data_[STILL_SENSOR_START + i]); } /* Light sensor value */ - SAFE_PUBLISH_SENSOR(this->light_sensor_, this->buffer_data_[LIGHT_SENSOR]) + SAFE_PUBLISH_SENSOR(this->light_sensor_, this->buffer_data_[LIGHT_SENSOR]); } } else { for (auto &gate_move_sensor : this->gate_move_sensors_) { - SAFE_PUBLISH_SENSOR_UNKNOWN(gate_move_sensor) + SAFE_PUBLISH_SENSOR_UNKNOWN(gate_move_sensor); } for (auto &gate_still_sensor : this->gate_still_sensors_) { - SAFE_PUBLISH_SENSOR_UNKNOWN(gate_still_sensor) + SAFE_PUBLISH_SENSOR_UNKNOWN(gate_still_sensor); } - SAFE_PUBLISH_SENSOR_UNKNOWN(this->light_sensor_) + SAFE_PUBLISH_SENSOR_UNKNOWN(this->light_sensor_); } #endif // the radar module won't tell us when it's done, so we just have to keep polling... @@ -846,12 +846,11 @@ void LD2412Component::set_light_out_control() { } #ifdef USE_SENSOR -// These could leak memory, but they are only set once prior to 'setup()' and should never be used again. void LD2412Component::set_gate_move_sensor(uint8_t gate, sensor::Sensor *s) { - this->gate_move_sensors_[gate] = new SensorWithDedup(s); + this->gate_move_sensors_[gate].set_sensor(s); } void LD2412Component::set_gate_still_sensor(uint8_t gate, sensor::Sensor *s) { - this->gate_still_sensors_[gate] = new SensorWithDedup(s); + this->gate_still_sensors_[gate].set_sensor(s); } #endif diff --git a/esphome/components/ld2412/ld2412.h b/esphome/components/ld2412/ld2412.h index 7fd2245978..306e7ae31d 100644 --- a/esphome/components/ld2412/ld2412.h +++ b/esphome/components/ld2412/ld2412.h @@ -133,8 +133,8 @@ class LD2412Component : public Component, public uart::UARTDevice { std::array gate_still_threshold_numbers_{}; #endif #ifdef USE_SENSOR - std::array *, TOTAL_GATES> gate_move_sensors_{}; - std::array *, TOTAL_GATES> gate_still_sensors_{}; + std::array, TOTAL_GATES> gate_move_sensors_{}; + std::array, TOTAL_GATES> gate_still_sensors_{}; #endif }; diff --git a/esphome/components/ld2450/ld2450.cpp b/esphome/components/ld2450/ld2450.cpp index 58c3cac42d..0dc2638aad 100644 --- a/esphome/components/ld2450/ld2450.cpp +++ b/esphome/components/ld2450/ld2450.cpp @@ -565,6 +565,7 @@ void LD2450Component::handle_periodic_data_() { SAFE_PUBLISH_SENSOR(this->still_target_count_sensor_, still_target_count); // Moving Target Count SAFE_PUBLISH_SENSOR(this->moving_target_count_sensor_, moving_target_count); + #endif #ifdef USE_BINARY_SENSOR @@ -872,33 +873,32 @@ void LD2450Component::query_target_tracking_mode_() { this->send_command_(CMD_QU void LD2450Component::query_zone_() { this->send_command_(CMD_QUERY_ZONE, nullptr, 0); } #ifdef USE_SENSOR -// These could leak memory, but they are only set once prior to 'setup()' and should never be used again. void LD2450Component::set_move_x_sensor(uint8_t target, sensor::Sensor *s) { - this->move_x_sensors_[target] = new SensorWithDedup(s); + this->move_x_sensors_[target].set_sensor(s); } void LD2450Component::set_move_y_sensor(uint8_t target, sensor::Sensor *s) { - this->move_y_sensors_[target] = new SensorWithDedup(s); + this->move_y_sensors_[target].set_sensor(s); } void LD2450Component::set_move_speed_sensor(uint8_t target, sensor::Sensor *s) { - this->move_speed_sensors_[target] = new SensorWithDedup(s); + this->move_speed_sensors_[target].set_sensor(s); } void LD2450Component::set_move_angle_sensor(uint8_t target, sensor::Sensor *s) { - this->move_angle_sensors_[target] = new SensorWithDedup(s); + this->move_angle_sensors_[target].set_sensor(s); } void LD2450Component::set_move_distance_sensor(uint8_t target, sensor::Sensor *s) { - this->move_distance_sensors_[target] = new SensorWithDedup(s); + this->move_distance_sensors_[target].set_sensor(s); } void LD2450Component::set_move_resolution_sensor(uint8_t target, sensor::Sensor *s) { - this->move_resolution_sensors_[target] = new SensorWithDedup(s); + this->move_resolution_sensors_[target].set_sensor(s); } void LD2450Component::set_zone_target_count_sensor(uint8_t zone, sensor::Sensor *s) { - this->zone_target_count_sensors_[zone] = new SensorWithDedup(s); + this->zone_target_count_sensors_[zone].set_sensor(s); } void LD2450Component::set_zone_still_target_count_sensor(uint8_t zone, sensor::Sensor *s) { - this->zone_still_target_count_sensors_[zone] = new SensorWithDedup(s); + this->zone_still_target_count_sensors_[zone].set_sensor(s); } void LD2450Component::set_zone_moving_target_count_sensor(uint8_t zone, sensor::Sensor *s) { - this->zone_moving_target_count_sensors_[zone] = new SensorWithDedup(s); + this->zone_moving_target_count_sensors_[zone].set_sensor(s); } #endif #ifdef USE_TEXT_SENSOR diff --git a/esphome/components/ld2450/ld2450.h b/esphome/components/ld2450/ld2450.h index cbcdec10b3..10f9bb874a 100644 --- a/esphome/components/ld2450/ld2450.h +++ b/esphome/components/ld2450/ld2450.h @@ -182,15 +182,15 @@ class LD2450Component : public Component, public uart::UARTDevice { ZoneOfNumbers zone_numbers_[MAX_ZONES]; #endif #ifdef USE_SENSOR - std::array *, MAX_TARGETS> move_x_sensors_{}; - std::array *, MAX_TARGETS> move_y_sensors_{}; - std::array *, MAX_TARGETS> move_speed_sensors_{}; - std::array *, MAX_TARGETS> move_angle_sensors_{}; - std::array *, MAX_TARGETS> move_distance_sensors_{}; - std::array *, MAX_TARGETS> move_resolution_sensors_{}; - std::array *, MAX_ZONES> zone_target_count_sensors_{}; - std::array *, MAX_ZONES> zone_still_target_count_sensors_{}; - std::array *, MAX_ZONES> zone_moving_target_count_sensors_{}; + std::array, MAX_TARGETS> move_x_sensors_{}; + std::array, MAX_TARGETS> move_y_sensors_{}; + std::array, MAX_TARGETS> move_speed_sensors_{}; + std::array, MAX_TARGETS> move_angle_sensors_{}; + std::array, MAX_TARGETS> move_distance_sensors_{}; + std::array, MAX_TARGETS> move_resolution_sensors_{}; + std::array, MAX_ZONES> zone_target_count_sensors_{}; + std::array, MAX_ZONES> zone_still_target_count_sensors_{}; + std::array, MAX_ZONES> zone_moving_target_count_sensors_{}; #endif #ifdef USE_TEXT_SENSOR std::array direction_text_sensors_{}; diff --git a/esphome/components/ld24xx/ld24xx.h b/esphome/components/ld24xx/ld24xx.h index fd55167974..cba1b68a15 100644 --- a/esphome/components/ld24xx/ld24xx.h +++ b/esphome/components/ld24xx/ld24xx.h @@ -11,28 +11,20 @@ #define SUB_SENSOR_WITH_DEDUP(name, dedup_type) \ protected: \ - ld24xx::SensorWithDedup *name##_sensor_{nullptr}; \ + ld24xx::SensorWithDedup name##_sensor_{}; \ \ public: \ - void set_##name##_sensor(sensor::Sensor *sensor) { \ - this->name##_sensor_ = new ld24xx::SensorWithDedup(sensor); \ - } + void set_##name##_sensor(sensor::Sensor *sensor) { this->name##_sensor_.set_sensor(sensor); } #endif #define LOG_SENSOR_WITH_DEDUP_SAFE(tag, name, sensor) \ - if ((sensor) != nullptr) { \ - LOG_SENSOR(tag, name, (sensor)->sens); \ + if ((sensor).has_sensor()) { \ + LOG_SENSOR(tag, name, (sensor).get_sensor()); \ } -#define SAFE_PUBLISH_SENSOR(sensor, value) \ - if ((sensor) != nullptr) { \ - (sensor)->publish_state_if_not_dup(value); \ - } +#define SAFE_PUBLISH_SENSOR(sensor, value) (sensor).publish_state_if_not_dup(value) -#define SAFE_PUBLISH_SENSOR_UNKNOWN(sensor) \ - if ((sensor) != nullptr) { \ - (sensor)->publish_state_unknown(); \ - } +#define SAFE_PUBLISH_SENSOR_UNKNOWN(sensor) (sensor).publish_state_unknown() #define highbyte(val) (uint8_t)((val) >> 8) #define lowbyte(val) (uint8_t)((val) &0xff) @@ -70,25 +62,33 @@ inline void format_version_str(const uint8_t *version, std::span buffe } #ifdef USE_SENSOR -// Helper class to store a sensor with a deduplicator & publish state only when the value changes +/// Sensor with deduplication — sensor may be null, null check is internal. +/// Stored inline, no heap allocation. Does nothing when no sensor is set. template class SensorWithDedup { public: - SensorWithDedup(sensor::Sensor *sens) : sens(sens) {} + void set_sensor(sensor::Sensor *sens) { + this->sens_ = sens; + this->dedup_ = {}; + } void publish_state_if_not_dup(T state) { - if (this->publish_dedup.next(state)) { - this->sens->publish_state(static_cast(state)); + if (this->sens_ != nullptr && this->dedup_.next(state)) { + this->sens_->publish_state(static_cast(state)); } } void publish_state_unknown() { - if (this->publish_dedup.next_unknown()) { - this->sens->publish_state(NAN); + if (this->sens_ != nullptr && this->dedup_.next_unknown()) { + this->sens_->publish_state(NAN); } } - sensor::Sensor *sens; - Deduplicator publish_dedup; + bool has_sensor() const { return this->sens_ != nullptr; } + sensor::Sensor *get_sensor() const { return this->sens_; } + + protected: + sensor::Sensor *sens_{nullptr}; + Deduplicator dedup_; }; #endif } // namespace esphome::ld24xx From cf01163c8cdc6ad7c8ad68b37c30804349eac499 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 14 Apr 2026 07:49:44 -1000 Subject: [PATCH 12/18] [core] Add uint32_to_str helper and use in preferences (#15597) --- esphome/components/esp32/preferences.cpp | 12 +-- esphome/components/libretiny/preferences.cpp | 12 +-- esphome/core/helpers.cpp | 14 ++++ esphome/core/helpers.h | 15 ++++ tests/benchmarks/core/bench_helpers.cpp | 56 ++++++++++++++ tests/components/core/test_uint32_to_str.cpp | 77 ++++++++++++++++++++ 6 files changed, 170 insertions(+), 16 deletions(-) create mode 100644 tests/components/core/test_uint32_to_str.cpp diff --git a/esphome/components/esp32/preferences.cpp b/esphome/components/esp32/preferences.cpp index bc0a34ebe8..925c4e7662 100644 --- a/esphome/components/esp32/preferences.cpp +++ b/esphome/components/esp32/preferences.cpp @@ -4,7 +4,6 @@ #include "esphome/core/helpers.h" #include "esphome/core/log.h" #include -#include #include #include @@ -12,9 +11,6 @@ namespace esphome::esp32 { static const char *const TAG = "preferences"; -// Buffer size for converting uint32_t to string: max "4294967295" (10 chars) + null terminator + 1 padding -static constexpr size_t KEY_BUFFER_SIZE = 12; - struct NVSData { uint32_t key; SmallInlineBuffer<8> data; // Most prefs fit in 8 bytes (covers fan, cover, select, etc.) @@ -51,8 +47,8 @@ bool ESP32PreferenceBackend::load(uint8_t *data, size_t len) { } } - char key_str[KEY_BUFFER_SIZE]; - snprintf(key_str, sizeof(key_str), "%" PRIu32, this->key); + char key_str[UINT32_MAX_STR_SIZE]; + uint32_to_str(key_str, this->key); size_t actual_len; esp_err_t err = nvs_get_blob(this->nvs_handle, key_str, nullptr, &actual_len); if (err != 0) { @@ -108,8 +104,8 @@ bool ESP32Preferences::sync() { uint32_t last_key = 0; for (const auto &save : s_pending_save) { - char key_str[KEY_BUFFER_SIZE]; - snprintf(key_str, sizeof(key_str), "%" PRIu32, save.key); + char key_str[UINT32_MAX_STR_SIZE]; + uint32_to_str(key_str, save.key); ESP_LOGVV(TAG, "Checking if NVS data %s has changed", key_str); if (this->is_changed_(this->nvs_handle, save, key_str)) { esp_err_t err = nvs_set_blob(this->nvs_handle, key_str, save.data.data(), save.data.size()); diff --git a/esphome/components/libretiny/preferences.cpp b/esphome/components/libretiny/preferences.cpp index fba6717294..313b36d31e 100644 --- a/esphome/components/libretiny/preferences.cpp +++ b/esphome/components/libretiny/preferences.cpp @@ -3,7 +3,6 @@ #include "preferences.h" #include "esphome/core/helpers.h" #include "esphome/core/log.h" -#include #include #include @@ -11,9 +10,6 @@ namespace esphome::libretiny { static const char *const TAG = "preferences"; -// Buffer size for converting uint32_t to string: max "4294967295" (10 chars) + null terminator + 1 padding -static constexpr size_t KEY_BUFFER_SIZE = 12; - struct NVSData { uint32_t key; SmallInlineBuffer<8> data; // Most prefs fit in 8 bytes (covers fan, cover, select, etc.) @@ -50,8 +46,8 @@ bool LibreTinyPreferenceBackend::load(uint8_t *data, size_t len) { } } - char key_str[KEY_BUFFER_SIZE]; - snprintf(key_str, sizeof(key_str), "%" PRIu32, this->key); + char key_str[UINT32_MAX_STR_SIZE]; + uint32_to_str(key_str, this->key); fdb_blob_make(this->blob, data, len); size_t actual_len = fdb_kv_get_blob(this->db, key_str, this->blob); if (actual_len != len) { @@ -92,8 +88,8 @@ bool LibreTinyPreferences::sync() { uint32_t last_key = 0; for (const auto &save : s_pending_save) { - char key_str[KEY_BUFFER_SIZE]; - snprintf(key_str, sizeof(key_str), "%" PRIu32, save.key); + char key_str[UINT32_MAX_STR_SIZE]; + uint32_to_str(key_str, save.key); ESP_LOGVV(TAG, "Checking if FDB data %s has changed", key_str); if (this->is_changed_(&this->db, save, key_str)) { ESP_LOGV(TAG, "sync: key: %s, len: %zu", key_str, save.data.size()); diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index cbe22dd09a..34ecaf137f 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -380,6 +380,20 @@ static char *format_hex_internal(char *buffer, size_t buffer_size, const uint8_t return buffer; } +char *uint32_to_str_unchecked(char *buf, uint32_t val) { + if (val == 0) { + *buf++ = '0'; + return buf; + } + char *start = buf; + while (val > 0) { + *buf++ = '0' + (val % 10); + val /= 10; + } + std::reverse(start, buf); + return buf; +} + char *format_hex_to(char *buffer, size_t buffer_size, const uint8_t *data, size_t length) { return format_hex_internal(buffer, buffer_size, data, length, 0, 'a'); } diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 3c42d7df07..54bc32a5a5 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -1295,6 +1295,21 @@ inline char *int8_to_str(char *buf, int8_t val) { return buf; } +/// Minimum buffer size for uint32_to_str: 10 digits + null terminator. +static constexpr size_t UINT32_MAX_STR_SIZE = 11; + +/// Write unsigned 32-bit integer to buffer (internal, no size check). +/// Buffer must have at least 10 bytes free. Returns pointer past last char written. +char *uint32_to_str_unchecked(char *buf, uint32_t val); + +/// Write unsigned 32-bit integer to buffer with compile-time size check. +/// Null-terminates the output. Returns number of chars written (excluding null). +inline size_t uint32_to_str(std::span buf, uint32_t val) { + char *end = uint32_to_str_unchecked(buf.data(), val); + *end = '\0'; + return static_cast(end - buf.data()); +} + /// Format byte array as lowercase hex to buffer (base implementation). char *format_hex_to(char *buffer, size_t buffer_size, const uint8_t *data, size_t length); diff --git a/tests/benchmarks/core/bench_helpers.cpp b/tests/benchmarks/core/bench_helpers.cpp index d9a9d158a3..1ce9101ff6 100644 --- a/tests/benchmarks/core/bench_helpers.cpp +++ b/tests/benchmarks/core/bench_helpers.cpp @@ -1,4 +1,6 @@ #include +#include +#include #include "esphome/core/helpers.h" @@ -307,4 +309,58 @@ static void Base64Decode_32Bytes(benchmark::State &state) { } BENCHMARK(Base64Decode_32Bytes); +// --- uint32_to_str() vs snprintf --- + +static void Uint32ToStr_Small(benchmark::State &state) { + char buf[UINT32_MAX_STR_SIZE]; + for (auto _ : state) { + for (int i = 0; i < kInnerIterations; i++) { + uint32_to_str(buf, 12345); + benchmark::DoNotOptimize(buf); + benchmark::ClobberMemory(); + } + } + state.SetItemsProcessed(state.iterations() * kInnerIterations); +} +BENCHMARK(Uint32ToStr_Small); + +static void Snprintf_Uint32_Small(benchmark::State &state) { + char buf[UINT32_MAX_STR_SIZE]; + for (auto _ : state) { + for (int i = 0; i < kInnerIterations; i++) { + snprintf(buf, sizeof(buf), "%" PRIu32, static_cast(12345)); + benchmark::DoNotOptimize(buf); + benchmark::ClobberMemory(); + } + } + state.SetItemsProcessed(state.iterations() * kInnerIterations); +} +BENCHMARK(Snprintf_Uint32_Small); + +static void Uint32ToStr_Large(benchmark::State &state) { + char buf[UINT32_MAX_STR_SIZE]; + for (auto _ : state) { + for (int i = 0; i < kInnerIterations; i++) { + uint32_to_str(buf, 4294967295u); + benchmark::DoNotOptimize(buf); + benchmark::ClobberMemory(); + } + } + state.SetItemsProcessed(state.iterations() * kInnerIterations); +} +BENCHMARK(Uint32ToStr_Large); + +static void Snprintf_Uint32_Large(benchmark::State &state) { + char buf[UINT32_MAX_STR_SIZE]; + for (auto _ : state) { + for (int i = 0; i < kInnerIterations; i++) { + snprintf(buf, sizeof(buf), "%" PRIu32, static_cast(4294967295u)); + benchmark::DoNotOptimize(buf); + benchmark::ClobberMemory(); + } + } + state.SetItemsProcessed(state.iterations() * kInnerIterations); +} +BENCHMARK(Snprintf_Uint32_Large); + } // namespace esphome::benchmarks diff --git a/tests/components/core/test_uint32_to_str.cpp b/tests/components/core/test_uint32_to_str.cpp new file mode 100644 index 0000000000..fc754429ec --- /dev/null +++ b/tests/components/core/test_uint32_to_str.cpp @@ -0,0 +1,77 @@ +#include + +#include "esphome/core/helpers.h" + +namespace esphome::core::testing { + +// --- uint32_to_str_unchecked() (internal, raw pointer) --- + +TEST(Uint32ToStr, InternalZero) { + char buf[UINT32_MAX_STR_SIZE]; + char *end = uint32_to_str_unchecked(buf, 0); + *end = '\0'; + EXPECT_STREQ(buf, "0"); + EXPECT_EQ(end - buf, 1); +} + +TEST(Uint32ToStr, InternalSingleDigit) { + char buf[UINT32_MAX_STR_SIZE]; + char *end = uint32_to_str_unchecked(buf, 7); + *end = '\0'; + EXPECT_STREQ(buf, "7"); +} + +TEST(Uint32ToStr, InternalMultiDigit) { + char buf[UINT32_MAX_STR_SIZE]; + char *end = uint32_to_str_unchecked(buf, 12345); + *end = '\0'; + EXPECT_STREQ(buf, "12345"); + EXPECT_EQ(end - buf, 5); +} + +TEST(Uint32ToStr, InternalMaxValue) { + char buf[UINT32_MAX_STR_SIZE]; + char *end = uint32_to_str_unchecked(buf, 4294967295u); + *end = '\0'; + EXPECT_STREQ(buf, "4294967295"); + EXPECT_EQ(end - buf, 10); +} + +TEST(Uint32ToStr, InternalPowersOfTen) { + char buf[UINT32_MAX_STR_SIZE]; + char *end; + + end = uint32_to_str_unchecked(buf, 10); + *end = '\0'; + EXPECT_STREQ(buf, "10"); + + end = uint32_to_str_unchecked(buf, 100); + *end = '\0'; + EXPECT_STREQ(buf, "100"); + + end = uint32_to_str_unchecked(buf, 1000000); + *end = '\0'; + EXPECT_STREQ(buf, "1000000"); +} + +// --- uint32_to_str() (public, span API) --- + +TEST(Uint32ToStr, SpanZero) { + char buf[UINT32_MAX_STR_SIZE]; + EXPECT_EQ(uint32_to_str(buf, 0), 1u); + EXPECT_STREQ(buf, "0"); +} + +TEST(Uint32ToStr, SpanMultiDigit) { + char buf[UINT32_MAX_STR_SIZE]; + EXPECT_EQ(uint32_to_str(buf, 12345), 5u); + EXPECT_STREQ(buf, "12345"); +} + +TEST(Uint32ToStr, SpanMaxValue) { + char buf[UINT32_MAX_STR_SIZE]; + EXPECT_EQ(uint32_to_str(buf, 4294967295u), 10u); + EXPECT_STREQ(buf, "4294967295"); +} + +} // namespace esphome::core::testing From da9fbb8044c1766ceda1b48e68bcdfafc89c9ccc Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 14 Apr 2026 07:50:11 -1000 Subject: [PATCH 13/18] [core] Fix app_state_ status bits clobbered for non-looping components (#15658) --- esphome/core/application.cpp | 34 ++- esphome/core/application.h | 24 ++- esphome/core/component.cpp | 13 ++ esphome/core/component.h | 5 + tests/integration/fixtures/status_flags.yaml | 141 +++++++++++++ tests/integration/test_status_flags.py | 209 +++++++++++++++++++ 6 files changed, 416 insertions(+), 10 deletions(-) create mode 100644 tests/integration/fixtures/status_flags.yaml create mode 100644 tests/integration/test_status_flags.py diff --git a/esphome/core/application.cpp b/esphome/core/application.cpp index cd75859880..0c17c70161 100644 --- a/esphome/core/application.cpp +++ b/esphome/core/application.cpp @@ -85,8 +85,12 @@ void Application::setup() { if (component->can_proceed()) continue; + // Force the status LED to blink WARNING while we wait for a slow + // component to come up. Cleared after setup() finishes if no real + // component has warning set. + this->app_state_ |= STATUS_LED_WARNING; + do { - uint8_t new_app_state = STATUS_LED_WARNING; uint32_t now = millis(); // Process pending loop enables to handle GPIO interrupts during setup @@ -96,17 +100,26 @@ void Application::setup() { // Update loop_component_start_time_ right before calling each component this->loop_component_start_time_ = millis(); this->components_[j]->call(); - new_app_state |= this->components_[j]->get_component_state(); - this->app_state_ |= new_app_state; this->feed_wdt(); } this->after_loop_tasks_(); - this->app_state_ = new_app_state; yield(); } while (!component->can_proceed() && !component->is_failed()); } + // Setup is complete. Reconcile STATUS_LED_WARNING: the slow-setup path + // above may have forced it on, and any status_clear_warning() calls + // from components during setup were intentional no-ops (gated by + // APP_STATE_SETUP_COMPLETE). Walk components once here to pick up the + // real state. STATUS_LED_ERROR is never artificially forced, so its + // clear path always works and needs no reconciliation. Finally, set + // APP_STATE_SETUP_COMPLETE so subsequent warning clears go through + // the normal walk-and-clear path. + if (!this->any_component_has_status_flag_(STATUS_LED_WARNING)) + this->app_state_ &= ~STATUS_LED_WARNING; + this->app_state_ |= APP_STATE_SETUP_COMPLETE; + ESP_LOGI(TAG, "setup() finished successfully!"); #ifdef USE_SETUP_PRIORITY_OVERRIDE @@ -211,6 +224,19 @@ void HOT Application::feed_wdt(uint32_t time) { #endif } } +bool Application::any_component_has_status_flag_(uint8_t flag) const { + // Walk all components (not just looping ones) so non-looping components' + // status bits are respected. Only called from the slow-path clear helpers + // (status_clear_warning_slow_path_ / status_clear_error_slow_path_) on an + // actual set→clear transition, so walking O(N) here is paid once per + // transition — not once per loop iteration. + for (auto *component : this->components_) { + if ((component->get_component_state() & flag) != 0) + return true; + } + return false; +} + void Application::reboot() { ESP_LOGI(TAG, "Forcing a reboot"); for (auto &component : std::ranges::reverse_view(this->components_)) { diff --git a/esphome/core/application.h b/esphome/core/application.h index 6b2969b490..0150bb6646 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -401,7 +401,18 @@ class Application { */ void teardown_components(uint32_t timeout_ms); - uint8_t get_app_state() const { return this->app_state_; } + /// Return the public app state status bits (STATUS_LED_* only). + /// Internal bookkeeping bits like APP_STATE_SETUP_COMPLETE are masked + /// out so external readers (status_led components, etc.) never see them. + uint8_t get_app_state() const { return this->app_state_ & ~APP_STATE_SETUP_COMPLETE; } + + /// True once Application::setup() has finished walking all components + /// and finalized the initial status flags. Before this point, the + /// slow-setup busy-wait may be forcing STATUS_LED_WARNING on, and + /// status_clear_* intentionally skips its walk-and-clear step so the + /// forced bit doesn't get wiped. Stored as a free bit on app_state_ + /// (bit 6) to avoid costing additional RAM. + bool is_setup_complete() const { return (this->app_state_ & APP_STATE_SETUP_COMPLETE) != 0; } // Helper macro for entity getter method declarations #ifdef USE_DEVICES @@ -577,6 +588,12 @@ class Application { bool is_socket_ready_(int fd) const { return FD_ISSET(fd, &this->read_fds_); } #endif + /// Walk all registered components looking for any whose component_state_ + /// has the given flag set. Used by Component::status_clear_*_slow_path_() + /// (which is a friend) to decide whether to clear the corresponding bit on + /// this->app_state_ (the app-wide "any component has this status" indicator). + bool any_component_has_status_flag_(uint8_t flag) const; + /// Register a component, detecting loop() override at compile time. /// Uses HasLoopOverride which handles ambiguous &T::loop from multiple inheritance. template void register_component_(T *comp) { @@ -838,8 +855,6 @@ inline void ESPHOME_ALWAYS_INLINE Application::before_loop_tasks_(uint32_t loop_ } inline void ESPHOME_ALWAYS_INLINE Application::loop() { - uint8_t new_app_state = 0; - // Get the initial loop time at the start uint32_t last_op_end_time = millis(); @@ -859,13 +874,10 @@ inline void ESPHOME_ALWAYS_INLINE Application::loop() { // Use the finish method to get the current time as the end time last_op_end_time = guard.finish(); } - new_app_state |= component->get_component_state(); - this->app_state_ |= new_app_state; this->feed_wdt(last_op_end_time); } this->after_loop_tasks_(); - this->app_state_ = new_app_state; #ifdef USE_RUNTIME_STATS // Process any pending runtime stats printing after all components have run diff --git a/esphome/core/component.cpp b/esphome/core/component.cpp index deda42b0a7..8949b4b76d 100644 --- a/esphome/core/component.cpp +++ b/esphome/core/component.cpp @@ -411,10 +411,23 @@ void Component::status_set_error(const LogString *message) { } void Component::status_clear_warning_slow_path_() { this->component_state_ &= ~STATUS_LED_WARNING; + // Clear the app-wide STATUS_LED_WARNING bit only if setup has finished + // AND no other component still has it set. During setup the forced + // STATUS_LED_WARNING (from the slow-setup busy-wait) must not be wiped + // by a transient component clear — Application::setup() reconciles + // the warning bit once at the end before setting APP_STATE_SETUP_COMPLETE. + // The set path is unchanged (set_status_flag_ still writes directly). + if (App.is_setup_complete() && !App.any_component_has_status_flag_(STATUS_LED_WARNING)) + App.app_state_ &= ~STATUS_LED_WARNING; ESP_LOGW(TAG, "%s cleared Warning flag", LOG_STR_ARG(this->get_component_log_str())); } void Component::status_clear_error_slow_path_() { this->component_state_ &= ~STATUS_LED_ERROR; + // STATUS_LED_ERROR is never artificially forced — it only ever lands + // in app_state_ via a real set_status_flag_ call. So the walk-and-clear + // path is always safe, including during setup. + if (!App.any_component_has_status_flag_(STATUS_LED_ERROR)) + App.app_state_ &= ~STATUS_LED_ERROR; ESP_LOGE(TAG, "%s cleared Error flag", LOG_STR_ARG(this->get_component_log_str())); } void Component::status_momentary_warning(const char *name, uint32_t length) { diff --git a/esphome/core/component.h b/esphome/core/component.h index e2b7aa85d3..3307c5ae76 100644 --- a/esphome/core/component.h +++ b/esphome/core/component.h @@ -89,6 +89,11 @@ inline constexpr uint8_t STATUS_LED_WARNING = 0x08; inline constexpr uint8_t STATUS_LED_ERROR = 0x10; // Component loop override flag uses bit 5 (set at registration time) inline constexpr uint8_t COMPONENT_HAS_LOOP = 0x20; +// Bit 6 on Application::app_state_ (ONLY) — set at the end of +// Application::setup(). Component::status_clear_*_slow_path_() uses this to +// decide whether to propagate clears to App.app_state_. Never set on a +// Component's component_state_. +inline constexpr uint8_t APP_STATE_SETUP_COMPLETE = 0x40; // Remove before 2026.8.0 enum class RetryResult { DONE, RETRY }; diff --git a/tests/integration/fixtures/status_flags.yaml b/tests/integration/fixtures/status_flags.yaml new file mode 100644 index 0000000000..cb118dcc84 --- /dev/null +++ b/tests/integration/fixtures/status_flags.yaml @@ -0,0 +1,141 @@ +esphome: + name: status-flags-test + +host: +api: + actions: + # Warning flag services for sensor_a + - action: set_warning_a + then: + - lambda: "id(sensor_a)->status_set_warning();" + - component.update: app_warning_bit + - component.update: app_error_bit + - action: clear_warning_a + then: + - lambda: "id(sensor_a)->status_clear_warning();" + - component.update: app_warning_bit + - component.update: app_error_bit + + # Warning flag services for sensor_b + - action: set_warning_b + then: + - lambda: "id(sensor_b)->status_set_warning();" + - component.update: app_warning_bit + - component.update: app_error_bit + - action: clear_warning_b + then: + - lambda: "id(sensor_b)->status_clear_warning();" + - component.update: app_warning_bit + - component.update: app_error_bit + + # Error flag services for sensor_a + - action: set_error_a + then: + - lambda: "id(sensor_a)->status_set_error();" + - component.update: app_warning_bit + - component.update: app_error_bit + - action: clear_error_a + then: + - lambda: "id(sensor_a)->status_clear_error();" + - component.update: app_warning_bit + - component.update: app_error_bit + + # Error flag services for sensor_b + - action: set_error_b + then: + - lambda: "id(sensor_b)->status_set_error();" + - component.update: app_warning_bit + - component.update: app_error_bit + - action: clear_error_b + then: + - lambda: "id(sensor_b)->status_clear_error();" + - component.update: app_warning_bit + - component.update: app_error_bit + + # Snapshot of the status_led_light's output state for observation. + - action: snapshot_led + then: + - component.update: status_led_writes + - component.update: status_led_last_state + +logger: + +# Tracks each write to the fake status_led output. +globals: + - id: status_led_write_count + type: uint32_t + restore_value: no + initial_value: "0" + - id: status_led_last_write + type: bool + restore_value: no + initial_value: "false" + +# Fake binary output — status_led_light writes to this instead of a pin. +# Every write bumps a counter and records the last value, both of which +# are exposed below so the test can verify status_led_light's loop is +# actually reading App.get_app_state() and responding. +output: + - platform: template + id: fake_status_led + type: binary + write_action: + - globals.set: + id: status_led_write_count + value: !lambda "return id(status_led_write_count) + 1;" + - globals.set: + id: status_led_last_write + value: !lambda "return state;" + +# Actual status_led_light component under test. +light: + - platform: status_led + name: Status LED + id: status_led_light_id + output: fake_status_led + +sensor: + # Two components that the test will toggle warning/error flags on. + - platform: template + name: Sensor A + id: sensor_a + update_interval: 24h + lambda: return 1.0; + - platform: template + name: Sensor B + id: sensor_b + update_interval: 24h + lambda: return 2.0; + + # Expose App.app_state_'s STATUS_LED_WARNING / STATUS_LED_ERROR bits + # as 0.0 / 1.0. force_update ensures every manual component.update + # publishes even if the value is unchanged. + - platform: template + name: App Warning Bit + id: app_warning_bit + update_interval: 24h + force_update: true + lambda: |- + return (App.get_app_state() & STATUS_LED_WARNING) != 0 ? 1.0 : 0.0; + - platform: template + name: App Error Bit + id: app_error_bit + update_interval: 24h + force_update: true + lambda: |- + return (App.get_app_state() & STATUS_LED_ERROR) != 0 ? 1.0 : 0.0; + + # Observables for the fake status_led output. + - platform: template + name: Status LED Writes + id: status_led_writes + update_interval: 24h + force_update: true + lambda: return id(status_led_write_count); + - platform: template + name: Status LED Last State + id: status_led_last_state + update_interval: 24h + force_update: true + lambda: |- + return id(status_led_last_write) ? 1.0 : 0.0; diff --git a/tests/integration/test_status_flags.py b/tests/integration/test_status_flags.py new file mode 100644 index 0000000000..ffbc7c7f63 --- /dev/null +++ b/tests/integration/test_status_flags.py @@ -0,0 +1,209 @@ +"""Integration tests for Component::status_set/clear_warning/error propagation. + +Verifies that toggling STATUS_LED_WARNING / STATUS_LED_ERROR on individual +components correctly updates the app-wide bits on Application::app_state_, +AND that the status_led_light component actually responds to those bits +by writing to its output (the full chain from component.status_set_warning +→ App.app_state_ → status_led_light.loop() reading get_app_state()). + +Exercises the multi-component OR semantics (the app bit stays set while +any component still has the flag, and only clears when the last component +clears its bit), the independence of warning and error, and the actual +status_led_light read of the bits via a fake template output that counts +writes. +""" + +from __future__ import annotations + +import asyncio + +import pytest + +from .state_utils import InitialStateHelper, SensorTracker, build_key_to_entity_mapping +from .types import APIClientConnectedFactory, RunCompiledFunction + +# Time to let the host-mode main loop run so status_led_light.loop() can +# execute enough iterations to produce measurable write-count changes on +# the fake template output. 300 ms is well above the minimum needed. +STATUS_LED_SETTLE_S = 0.3 + + +@pytest.mark.asyncio +async def test_status_flags( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + async with run_compiled(yaml_config), api_client_connected() as client: + entities, services = await client.list_entities_services() + + # Map every custom API service by name for the test to execute. + svc = {s.name: s for s in services} + for name in ( + "set_warning_a", + "clear_warning_a", + "set_warning_b", + "clear_warning_b", + "set_error_a", + "clear_error_a", + "set_error_b", + "clear_error_b", + "snapshot_led", + ): + assert name in svc, f"service {name} not registered" + + # Track every sensor we care about. SensorTracker gives us + # expect(value) / expect_any() futures that resolve when a + # matching state arrives; much simpler than manual bookkeeping. + tracker = SensorTracker( + [ + "app_warning_bit", + "app_error_bit", + "status_led_writes", + "status_led_last_state", + ] + ) + tracker.key_to_sensor.update( + build_key_to_entity_mapping(entities, list(tracker.sensor_states.keys())) + ) + + # Swallow initial state broadcasts so the test only reacts to + # state changes triggered by our service calls. + initial_state_helper = InitialStateHelper(entities) + client.subscribe_states(initial_state_helper.on_state_wrapper(tracker.on_state)) + try: + await initial_state_helper.wait_for_initial_states() + except TimeoutError: + pytest.fail("Timeout waiting for initial states") + + async def call(name: str) -> None: + await client.execute_service(svc[name], {}) + + async def call_and_expect_bits( + service_name: str, *, warning: float, error: float + ) -> None: + """Execute a service and wait for both app bit sensors to match. + + Each bit-toggling service calls component.update on both + app_warning_bit and app_error_bit, so both sensors publish. + """ + futures = tracker.expect_all( + {"app_warning_bit": warning, "app_error_bit": error} + ) + await call(service_name) + await tracker.await_all(futures) + + async def snapshot_led_writes() -> int: + """Trigger a publish of the fake status_led output counter and return it.""" + future = tracker.expect_any("status_led_writes") + await call("snapshot_led") + await tracker.await_change(future, "status_led_writes") + return int(tracker.sensor_states["status_led_writes"][-1]) + + # ---- Baseline: everything clean ---- + await call_and_expect_bits("clear_warning_a", warning=0.0, error=0.0) + + # ================================================================ + # Part 1 — STATUS_LED_WARNING propagation to App.app_state_ + # ================================================================ + + # Single component set/clear + await call_and_expect_bits("set_warning_a", warning=1.0, error=0.0) + await call_and_expect_bits("clear_warning_a", warning=0.0, error=0.0) + + # Multi-component OR: both set, clear A, bit stays (B still has it), clear B, gone + await call_and_expect_bits("set_warning_a", warning=1.0, error=0.0) + await call_and_expect_bits("set_warning_b", warning=1.0, error=0.0) + await call_and_expect_bits("clear_warning_a", warning=1.0, error=0.0) + await call_and_expect_bits("clear_warning_b", warning=0.0, error=0.0) + + # Opposite clear order + await call_and_expect_bits("set_warning_a", warning=1.0, error=0.0) + await call_and_expect_bits("set_warning_b", warning=1.0, error=0.0) + await call_and_expect_bits("clear_warning_b", warning=1.0, error=0.0) + await call_and_expect_bits("clear_warning_a", warning=0.0, error=0.0) + + # ================================================================ + # Part 2 — STATUS_LED_ERROR propagation (same scenarios) + # ================================================================ + + await call_and_expect_bits("set_error_a", warning=0.0, error=1.0) + await call_and_expect_bits("clear_error_a", warning=0.0, error=0.0) + + await call_and_expect_bits("set_error_a", warning=0.0, error=1.0) + await call_and_expect_bits("set_error_b", warning=0.0, error=1.0) + await call_and_expect_bits("clear_error_a", warning=0.0, error=1.0) + await call_and_expect_bits("clear_error_b", warning=0.0, error=0.0) + + # ================================================================ + # Part 3 — warning and error are independent + # ================================================================ + + await call_and_expect_bits("set_warning_a", warning=1.0, error=0.0) + await call_and_expect_bits("set_error_b", warning=1.0, error=1.0) + await call_and_expect_bits("clear_warning_a", warning=0.0, error=1.0) + await call_and_expect_bits("clear_error_b", warning=0.0, error=0.0) + + # ================================================================ + # Part 4 — status_led_light actually reads App.app_state_ + # ================================================================ + # The fake status_led_light output increments status_led_write_count + # on every write. status_led_light::loop() writes its output on every + # iteration while an error/warning bit is set, so after holding a + # warning for ~300 ms we should see the counter move significantly. + # This is the end-to-end proof that the bits we set above actually + # reach status_led_light and drive its behavior. + + count_before_warning = await snapshot_led_writes() + await call_and_expect_bits("set_warning_a", warning=1.0, error=0.0) + # Let status_led_light's loop run long enough to toggle the pin + # several times (it reads get_app_state() every main loop iteration). + await asyncio.sleep(STATUS_LED_SETTLE_S) + count_after_warning = await snapshot_led_writes() + assert count_after_warning > count_before_warning, ( + "status_led_light did not respond to STATUS_LED_WARNING being set: " + f"write count stayed at {count_before_warning} → {count_after_warning}. " + "The full chain Component::status_set_warning → App.app_state_ → " + "status_led_light::loop reading get_app_state() is broken." + ) + await call_and_expect_bits("clear_warning_a", warning=0.0, error=0.0) + + # Same check for ERROR + count_before_error = await snapshot_led_writes() + await call_and_expect_bits("set_error_a", warning=0.0, error=1.0) + await asyncio.sleep(STATUS_LED_SETTLE_S) + count_after_error = await snapshot_led_writes() + assert count_after_error > count_before_error, ( + "status_led_light did not respond to STATUS_LED_ERROR being set: " + f"write count stayed at {count_before_error} → {count_after_error}. " + ) + await call_and_expect_bits("clear_error_a", warning=0.0, error=0.0) + + # ---- Set → clear → re-set round-trip ---- + # After clearing, status_led_light stops writing (steady state). + # Re-setting the flag must make it resume. This guards against a + # future idle optimization (e.g. #15642) where status_led disables + # its own loop when idle: if the re-enable path were broken, the + # second set would not produce writes. + # + # Snapshot AFTER the clear to avoid counting writes that were still + # in-flight from the error-set phase. + count_after_clear = await snapshot_led_writes() + await asyncio.sleep(STATUS_LED_SETTLE_S) + count_after_idle = await snapshot_led_writes() + assert count_after_idle - count_after_clear <= 5, ( + "status_led_light kept writing after warning/error was cleared: " + f"count grew from {count_after_clear} to {count_after_idle}. " + "Expected it to stop writing once all status bits were clear." + ) + # Re-set warning — writes must resume. + await call_and_expect_bits("set_warning_a", warning=1.0, error=0.0) + await asyncio.sleep(STATUS_LED_SETTLE_S) + count_after_reset = await snapshot_led_writes() + assert count_after_reset > count_after_idle + 5, ( + "status_led_light did not resume writing after re-setting " + f"STATUS_LED_WARNING: count went from {count_after_idle} to " + f"{count_after_reset}. If an idle optimization disabled the " + "loop, the re-enable path may be broken." + ) + await call_and_expect_bits("clear_warning_a", warning=0.0, error=0.0) From 4729efbd0478aa8ecaa0840b382f7703b8a933d1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 14 Apr 2026 07:50:28 -1000 Subject: [PATCH 14/18] [light] Deduplicate color_uncorrect channel math via shared helper (#15727) --- .../components/light/esp_color_correction.cpp | 16 +++++++++ .../components/light/esp_color_correction.h | 33 +++++-------------- 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/esphome/components/light/esp_color_correction.cpp b/esphome/components/light/esp_color_correction.cpp index 9d731a2bd5..e793226bb1 100644 --- a/esphome/components/light/esp_color_correction.cpp +++ b/esphome/components/light/esp_color_correction.cpp @@ -22,4 +22,20 @@ uint8_t ESPColorCorrection::gamma_uncorrect_(uint8_t value) const { return (target - a <= b - target) ? lo : lo + 1; } +Color ESPColorCorrection::color_uncorrect(Color color) const { + // uncorrected = corrected^(1/gamma) / (max_brightness * local_brightness) + return Color(this->color_uncorrect_red(color.red), this->color_uncorrect_green(color.green), + this->color_uncorrect_blue(color.blue), this->color_uncorrect_white(color.white)); +} + +uint8_t ESPColorCorrection::color_uncorrect_channel_(uint8_t value, uint8_t max_brightness) const { + if (max_brightness == 0 || this->local_brightness_ == 0) + return 0; + // Use 32-bit intermediates: when max_brightness and local_brightness_ are small but non-zero, + // (uncorrected / max_brightness) * 255 can exceed 65535 before the std::min(255) clamp runs. + uint32_t uncorrected = this->gamma_uncorrect_(value) * 255UL; + uint32_t res = ((uncorrected / max_brightness) * 255UL) / this->local_brightness_; + return static_cast(std::min(res, uint32_t(255))); +} + } // namespace esphome::light diff --git a/esphome/components/light/esp_color_correction.h b/esphome/components/light/esp_color_correction.h index 48ecc46364..4eb5208c96 100644 --- a/esphome/components/light/esp_color_correction.h +++ b/esphome/components/light/esp_color_correction.h @@ -46,38 +46,18 @@ class ESPColorCorrection { uint8_t res = esp_scale8_twice(white, this->max_brightness_.white, this->local_brightness_); return this->gamma_correct_(res); } - inline Color color_uncorrect(Color color) const ESPHOME_ALWAYS_INLINE { - // uncorrected = corrected^(1/gamma) / (max_brightness * local_brightness) - return Color(this->color_uncorrect_red(color.red), this->color_uncorrect_green(color.green), - this->color_uncorrect_blue(color.blue), this->color_uncorrect_white(color.white)); - } + Color color_uncorrect(Color color) const; inline uint8_t color_uncorrect_red(uint8_t red) const ESPHOME_ALWAYS_INLINE { - if (this->max_brightness_.red == 0 || this->local_brightness_ == 0) - return 0; - uint16_t uncorrected = this->gamma_uncorrect_(red) * 255UL; - uint16_t res = ((uncorrected / this->max_brightness_.red) * 255UL) / this->local_brightness_; - return (uint8_t) std::min(res, uint16_t(255)); + return this->color_uncorrect_channel_(red, this->max_brightness_.red); } inline uint8_t color_uncorrect_green(uint8_t green) const ESPHOME_ALWAYS_INLINE { - if (this->max_brightness_.green == 0 || this->local_brightness_ == 0) - return 0; - uint16_t uncorrected = this->gamma_uncorrect_(green) * 255UL; - uint16_t res = ((uncorrected / this->max_brightness_.green) * 255UL) / this->local_brightness_; - return (uint8_t) std::min(res, uint16_t(255)); + return this->color_uncorrect_channel_(green, this->max_brightness_.green); } inline uint8_t color_uncorrect_blue(uint8_t blue) const ESPHOME_ALWAYS_INLINE { - if (this->max_brightness_.blue == 0 || this->local_brightness_ == 0) - return 0; - uint16_t uncorrected = this->gamma_uncorrect_(blue) * 255UL; - uint16_t res = ((uncorrected / this->max_brightness_.blue) * 255UL) / this->local_brightness_; - return (uint8_t) std::min(res, uint16_t(255)); + return this->color_uncorrect_channel_(blue, this->max_brightness_.blue); } inline uint8_t color_uncorrect_white(uint8_t white) const ESPHOME_ALWAYS_INLINE { - if (this->max_brightness_.white == 0 || this->local_brightness_ == 0) - return 0; - uint16_t uncorrected = this->gamma_uncorrect_(white) * 255UL; - uint16_t res = ((uncorrected / this->max_brightness_.white) * 255UL) / this->local_brightness_; - return (uint8_t) std::min(res, uint16_t(255)); + return this->color_uncorrect_channel_(white, this->max_brightness_.white); } protected: @@ -85,6 +65,9 @@ class ESPColorCorrection { uint8_t gamma_correct_(uint8_t value) const; /// Reverse gamma: binary search the forward PROGMEM table uint8_t gamma_uncorrect_(uint8_t value) const; + /// Shared body of color_uncorrect_{red,green,blue,white}. Kept out-of-line + /// to avoid duplicating two 16-bit divides at every call site. + uint8_t color_uncorrect_channel_(uint8_t value, uint8_t max_brightness) const; const uint16_t *gamma_table_{nullptr}; Color max_brightness_{255, 255, 255, 255}; From 57d9e508ea1165fb13b7fcfa3a9ff0290480f2ff Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 14 Apr 2026 08:09:00 -1000 Subject: [PATCH 15/18] merge --- esphome/core/helpers.cpp | 2 +- esphome/core/helpers.h | 2 +- tests/components/core/test_helpers.cpp | 18 +++++++++--------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index d8b5a1e87f..e4ce672570 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -559,7 +559,7 @@ static size_t value_accuracy_to_buf_fast(char *buf, float value, int8_t accuracy p = uint32_to_str_unchecked(p, scaled / mult); if (accuracy_decimals > 0) { *p++ = '.'; - p = frac_to_str_(p, scaled % mult, mult / 10); + p = frac_to_str_unchecked(p, scaled % mult, mult / 10); } *p = '\0'; return static_cast(p - buf); diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index d41b8181a4..55f411fdec 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -1331,7 +1331,7 @@ inline size_t uint32_to_str(std::span buf, uint32_t v /// Write fractional digits with leading zeros to buffer (internal, no size check). /// frac is the fractional value, divisor is the highest place value (e.g. 100 for 3 digits). /// Returns pointer past last char written. -inline char *frac_to_str_(char *buf, uint32_t frac, uint32_t divisor) { +inline char *frac_to_str_unchecked(char *buf, uint32_t frac, uint32_t divisor) { while (divisor > 0) { *buf++ = '0' + static_cast(frac / divisor); frac %= divisor; diff --git a/tests/components/core/test_helpers.cpp b/tests/components/core/test_helpers.cpp index 68568982e4..5fb77ef753 100644 --- a/tests/components/core/test_helpers.cpp +++ b/tests/components/core/test_helpers.cpp @@ -124,11 +124,11 @@ TEST(SmallPow10, One) { EXPECT_EQ(small_pow10(1), 10u); } TEST(SmallPow10, Two) { EXPECT_EQ(small_pow10(2), 100u); } TEST(SmallPow10, Three) { EXPECT_EQ(small_pow10(3), 1000u); } -// --- frac_to_str_() --- +// --- frac_to_str_unchecked() --- TEST(FracToStr, OneDigit) { char buf[8]; - char *end = frac_to_str_(buf, 5, 1); + char *end = frac_to_str_unchecked(buf, 5, 1); *end = '\0'; EXPECT_STREQ(buf, "5"); EXPECT_EQ(end - buf, 1); @@ -136,14 +136,14 @@ TEST(FracToStr, OneDigit) { TEST(FracToStr, TwoDigits) { char buf[8]; - char *end = frac_to_str_(buf, 46, 10); + char *end = frac_to_str_unchecked(buf, 46, 10); *end = '\0'; EXPECT_STREQ(buf, "46"); } TEST(FracToStr, ThreeDigits) { char buf[8]; - char *end = frac_to_str_(buf, 456, 100); + char *end = frac_to_str_unchecked(buf, 456, 100); *end = '\0'; EXPECT_STREQ(buf, "456"); EXPECT_EQ(end - buf, 3); @@ -151,22 +151,22 @@ TEST(FracToStr, ThreeDigits) { TEST(FracToStr, LeadingZeros) { char buf[8]; - char *end = frac_to_str_(buf, 1, 100); + char *end = frac_to_str_unchecked(buf, 1, 100); *end = '\0'; EXPECT_STREQ(buf, "001"); - end = frac_to_str_(buf, 5, 10); + end = frac_to_str_unchecked(buf, 5, 10); *end = '\0'; EXPECT_STREQ(buf, "05"); } TEST(FracToStr, AllZeros) { char buf[8]; - char *end = frac_to_str_(buf, 0, 100); + char *end = frac_to_str_unchecked(buf, 0, 100); *end = '\0'; EXPECT_STREQ(buf, "000"); - end = frac_to_str_(buf, 0, 1); + end = frac_to_str_unchecked(buf, 0, 1); *end = '\0'; EXPECT_STREQ(buf, "0"); } @@ -174,7 +174,7 @@ TEST(FracToStr, AllZeros) { TEST(FracToStr, ZeroDivisor) { char buf[8]; buf[0] = 'X'; - char *end = frac_to_str_(buf, 0, 0); + char *end = frac_to_str_unchecked(buf, 0, 0); EXPECT_EQ(end, buf); // writes nothing } From 5066171a9daee4fbf0751a8cd012188d6cf6da54 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 14 Apr 2026 08:36:09 -1000 Subject: [PATCH 16/18] Address Copilot review: uint32 overflow guard, docstring, test namespace/include --- esphome/core/helpers.cpp | 18 ++++++++++++------ esphome/core/helpers.h | 5 ++++- tests/components/core/test_value_accuracy.cpp | 5 +++-- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index e4ce672570..ede1640af2 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -545,17 +545,19 @@ std::string value_accuracy_to_string(float value, int8_t accuracy_decimals) { // Fast float-to-string for accuracy_decimals 0-3 (covers virtually all sensor usage). // Avoids snprintf("%.*f") which pulls in heavy float formatting machinery. -static size_t value_accuracy_to_buf_fast(char *buf, float value, int8_t accuracy_decimals) { +// Caller must guarantee value is finite and |value| * mult fits in uint32_t. +static size_t value_accuracy_to_buf_fast(char *buf, float value, int8_t accuracy_decimals, uint32_t mult) { char *p = buf; if (std::signbit(value)) { *p++ = '-'; value = -value; } - uint32_t mult = small_pow10(accuracy_decimals); // Cast to double for the multiply to match snprintf's rounding precision. // float*int loses bits at exact-half boundaries (e.g. 23.45f*10 = 234.5 in float, // but snprintf sees 234.500007... via double promotion and rounds differently). - uint32_t scaled = static_cast(lrint(static_cast(value) * mult)); + // llrint returns long long so the result fits even on 32-bit targets where + // long is 32-bit; caller has already bounded |value * mult| to UINT32_MAX. + uint32_t scaled = static_cast(llrint(static_cast(value) * mult)); p = uint32_to_str_unchecked(p, scaled / mult); if (accuracy_decimals > 0) { *p++ = '.'; @@ -568,12 +570,16 @@ static size_t value_accuracy_to_buf_fast(char *buf, float value, int8_t accuracy size_t value_accuracy_to_buf(std::span buf, float value, int8_t accuracy_decimals) { normalize_accuracy_decimals(value, accuracy_decimals); - // Fast path for accuracy 0-3 and finite values + // Fast path for accuracy 0-3, finite values whose scaled magnitude fits in uint32_t. + // For 3 decimals that's |value| < ~4.29e6; larger totals fall through to snprintf. if (accuracy_decimals <= 3 && std::isfinite(value)) { - return value_accuracy_to_buf_fast(buf.data(), value, accuracy_decimals); + const uint32_t mult = small_pow10(accuracy_decimals); + if (std::fabs(value) < static_cast(UINT32_MAX) / mult) { + return value_accuracy_to_buf_fast(buf.data(), value, accuracy_decimals, mult); + } } - // Fallback for NaN/Inf/high accuracy + // Fallback for NaN/Inf/high accuracy/out-of-range int len = snprintf(buf.data(), buf.size(), "%.*f", accuracy_decimals, value); if (len < 0) return 0; diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 55f411fdec..5b4e034d8b 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -1296,7 +1296,10 @@ inline char *int8_to_str(char *buf, int8_t val) { } /// Append a separator char and a string to a buffer, respecting remaining space. -/// Returns pointer past last char written (null terminator is written). +/// Returns pointer past last char written. On success (remaining >= 2) a null +/// terminator is written after the copied string. If remaining < 2 nothing is +/// written (not even a terminator) and `buf` is returned unchanged — callers +/// needing a terminated buffer in that case must ensure one is already present. inline char *buf_append_sep_str(char *buf, size_t remaining, char separator, const char *str, size_t str_len) { if (remaining < 2) { return buf; diff --git a/tests/components/core/test_value_accuracy.cpp b/tests/components/core/test_value_accuracy.cpp index a1fba90acf..6bd8976acc 100644 --- a/tests/components/core/test_value_accuracy.cpp +++ b/tests/components/core/test_value_accuracy.cpp @@ -3,10 +3,11 @@ #include #include #include +#include #include "esphome/core/helpers.h" -namespace esphome::testing { +namespace esphome::core::testing { // Helper to call value_accuracy_to_buf and return as string static std::string va_to_string(float value, int8_t accuracy_decimals) { @@ -148,4 +149,4 @@ TEST(ValueAccuracyToBuf, ReturnsCorrectLength) { EXPECT_EQ(strlen(buf), len); } -} // namespace esphome::testing +} // namespace esphome::core::testing From 5ae9ddc46a66c5041605fd9663b736074de75c03 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 14 Apr 2026 08:41:43 -1000 Subject: [PATCH 17/18] buf_append_sep_str: always null-terminate when remaining >= 1 --- esphome/core/helpers.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 5b4e034d8b..4bb958c40e 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -1296,12 +1296,14 @@ inline char *int8_to_str(char *buf, int8_t val) { } /// Append a separator char and a string to a buffer, respecting remaining space. -/// Returns pointer past last char written. On success (remaining >= 2) a null -/// terminator is written after the copied string. If remaining < 2 nothing is -/// written (not even a terminator) and `buf` is returned unchanged — callers -/// needing a terminated buffer in that case must ensure one is already present. +/// Returns pointer past last char written. The buffer is always null-terminated +/// when remaining >= 1 (even on the no-room early-return), so callers always get +/// a valid C string. inline char *buf_append_sep_str(char *buf, size_t remaining, char separator, const char *str, size_t str_len) { if (remaining < 2) { + if (remaining >= 1) { + *buf = '\0'; + } return buf; } *buf++ = separator; From efba0f5fd6b0c42b0644a743da5b9b570a120623 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 14 Apr 2026 14:44:12 -1000 Subject: [PATCH 18/18] handle 0 --- esphome/core/helpers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index ede1640af2..f7bc6c5bee 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -589,7 +589,7 @@ size_t value_accuracy_to_buf(std::span buf, float size_t value_accuracy_with_uom_to_buf(std::span buf, float value, int8_t accuracy_decimals, StringRef unit_of_measurement) { size_t len = value_accuracy_to_buf(buf, value, accuracy_decimals); - if (unit_of_measurement.empty()) { + if (len == 0 || unit_of_measurement.empty()) { return len; } char *end = buf_append_sep_str(buf.data() + len, buf.size() - len, ' ', unit_of_measurement.c_str(),