From 0c9d443a5c041910f4df5cbba3a136d151c92c8e Mon Sep 17 00:00:00 2001 From: Edward Firmo <94725493+edwardtfn@users.noreply.github.com> Date: Tue, 21 Apr 2026 15:18:46 +0200 Subject: [PATCH 01/15] [esp32_ble] Add `use_psram` option to offload BT memory allocation to SPIRAM (#15644) --- esphome/components/esp32_ble/__init__.py | 20 +++++++++++++++++++ esphome/components/esp32_ble/ble.cpp | 3 +++ esphome/core/defines.h | 1 + .../esp32_ble/common_use_psram.yaml | 4 ++++ .../components/esp32_ble/test.esp32-ard.yaml | 1 + .../components/esp32_ble/test.esp32-idf.yaml | 1 + .../esp32_ble/test.esp32-p4-idf.yaml | 1 + 7 files changed, 31 insertions(+) create mode 100644 tests/components/esp32_ble/common_use_psram.yaml diff --git a/esphome/components/esp32_ble/__init__.py b/esphome/components/esp32_ble/__init__.py index 79d05049bf..c7b6b40394 100644 --- a/esphome/components/esp32_ble/__init__.py +++ b/esphome/components/esp32_ble/__init__.py @@ -7,6 +7,7 @@ from typing import Any from esphome import automation import esphome.codegen as cg +from esphome.components.const import CONF_USE_PSRAM from esphome.components.esp32 import add_idf_sdkconfig_option, const, get_esp32_variant from esphome.components.esp32.const import VARIANT_ESP32C2 import esphome.config_validation as cv @@ -342,6 +343,9 @@ CONFIG_SCHEMA = cv.Schema( cv.Optional(CONF_MAX_CONNECTIONS, default=DEFAULT_MAX_CONNECTIONS): cv.All( cv.positive_int, cv.Range(min=1, max=IDF_MAX_CONNECTIONS) ), + cv.Optional(CONF_USE_PSRAM): cv.All( + cv.only_on_esp32, cv.requires_component("psram"), cv.boolean + ), } ).extend(cv.COMPONENT_SCHEMA) @@ -598,6 +602,22 @@ async def to_code(config): add_idf_sdkconfig_option("CONFIG_BT_ENABLED", True) add_idf_sdkconfig_option("CONFIG_BT_BLE_42_FEATURES_SUPPORTED", True) + # When PSRAM and BT are used together, Bluedroid should prefer SPIRAM for + # heap allocations and use dynamic (heap-based) environment memory tables + # instead of large static DRAM arrays. This frees ~40 kB of internal RAM. + # Reference: Espressif ADF Design Considerations + # https://espressif-docs.readthedocs-hosted.com/projects/esp-adf/en/latest/ + # design-guide/design-considerations.html + if config.get(CONF_USE_PSRAM, False): + cg.add_define("USE_ESP32_BLE_PSRAM") + # CONFIG_BT_ALLOCATION_FROM_SPIRAM_FIRST is only available on ESP32 + # (BTDM dual-mode controller). BLE-only SoCs (C3, S3, C2, H2) do not + # expose this Kconfig symbol; applying it there would cause a build error. + if get_esp32_variant() == const.VARIANT_ESP32: + add_idf_sdkconfig_option("CONFIG_BT_ALLOCATION_FROM_SPIRAM_FIRST", True) + # CONFIG_BT_BLE_DYNAMIC_ENV_MEMORY applies to all Bluedroid-enabled variants. + add_idf_sdkconfig_option("CONFIG_BT_BLE_DYNAMIC_ENV_MEMORY", True) + # Register the core BLE loggers that are always needed register_bt_logger(BTLoggers.GAP, BTLoggers.BTM, BTLoggers.HCI) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 0280439731..ebb44c7d91 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -667,6 +667,9 @@ void ESP32BLE::dump_config() { " MAC address: %s\n" " IO Capability: %s", mac_s, io_capability_s); +#ifdef USE_ESP32_BLE_PSRAM + ESP_LOGCONFIG(TAG, " PSRAM BLE allocation: enabled"); +#endif #ifdef ESPHOME_ESP32_BLE_EXTENDED_AUTH_PARAMS const char *auth_req_mode_s = ""; diff --git a/esphome/core/defines.h b/esphome/core/defines.h index 0fb7221571..07cac97e17 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -48,6 +48,7 @@ #define USE_ENTITY_DEVICE_CLASS #define USE_ENTITY_ICON #define USE_ENTITY_UNIT_OF_MEASUREMENT +#define USE_ESP32_BLE_PSRAM #define USE_ESP32_CAMERA_JPEG_CONVERSION #define USE_ESP32_HOSTED #define USE_ESP32_IMPROV_STATE_CALLBACK diff --git a/tests/components/esp32_ble/common_use_psram.yaml b/tests/components/esp32_ble/common_use_psram.yaml new file mode 100644 index 0000000000..cce6cf547f --- /dev/null +++ b/tests/components/esp32_ble/common_use_psram.yaml @@ -0,0 +1,4 @@ +esp32_ble: + use_psram: true + +psram: diff --git a/tests/components/esp32_ble/test.esp32-ard.yaml b/tests/components/esp32_ble/test.esp32-ard.yaml index dade44d145..fa7b9befc7 100644 --- a/tests/components/esp32_ble/test.esp32-ard.yaml +++ b/tests/components/esp32_ble/test.esp32-ard.yaml @@ -1 +1,2 @@ <<: !include common.yaml +<<: !include common_use_psram.yaml diff --git a/tests/components/esp32_ble/test.esp32-idf.yaml b/tests/components/esp32_ble/test.esp32-idf.yaml index f8defaf28f..0b2a920c60 100644 --- a/tests/components/esp32_ble/test.esp32-idf.yaml +++ b/tests/components/esp32_ble/test.esp32-idf.yaml @@ -1,4 +1,5 @@ <<: !include common.yaml +<<: !include common_use_psram.yaml esp32_ble: io_capability: keyboard_only diff --git a/tests/components/esp32_ble/test.esp32-p4-idf.yaml b/tests/components/esp32_ble/test.esp32-p4-idf.yaml index 4eeb7c2f18..170220bf48 100644 --- a/tests/components/esp32_ble/test.esp32-p4-idf.yaml +++ b/tests/components/esp32_ble/test.esp32-p4-idf.yaml @@ -2,6 +2,7 @@ packages: ble: !include ../../test_build_components/common/ble/esp32-p4-idf.yaml <<: !include common.yaml +<<: !include common_use_psram.yaml esp32_ble: io_capability: keyboard_only From 43c6b839cd70e3c2430964ec441c3815ac203d91 Mon Sep 17 00:00:00 2001 From: Geoff Date: Tue, 21 Apr 2026 07:00:03 -0700 Subject: [PATCH 02/15] [sensor] Filter to round to significant digits (#11157) Co-authored-by: Clyde Stubbs <2366188+clydebarrow@users.noreply.github.com> Co-authored-by: J. Nick Koston Co-authored-by: J. Nick Koston Co-authored-by: J. Nick Koston --- esphome/components/sensor/__init__.py | 14 ++++++ esphome/components/sensor/filter.h | 13 +++++ esphome/core/helpers.cpp | 17 +++++++ esphome/core/helpers.h | 5 ++ tests/components/core/helpers_test.cpp | 58 ++++++++++++++++++++++ tests/components/template/common-base.yaml | 1 + 6 files changed, 108 insertions(+) create mode 100644 tests/components/core/helpers_test.cpp diff --git a/esphome/components/sensor/__init__.py b/esphome/components/sensor/__init__.py index 8dcb7165e3..43fbc98953 100644 --- a/esphome/components/sensor/__init__.py +++ b/esphome/components/sensor/__init__.py @@ -118,6 +118,7 @@ from esphome.schema_extractors import SCHEMA_EXTRACT, schema_extractor from esphome.util import Registry CODEOWNERS = ["@esphome/core"] + DEVICE_CLASSES = [ DEVICE_CLASS_ABSOLUTE_HUMIDITY, DEVICE_CLASS_APPARENT_POWER, @@ -293,6 +294,7 @@ SensorInRangeCondition = sensor_ns.class_("SensorInRangeCondition", Filter) ClampFilter = sensor_ns.class_("ClampFilter", Filter) RoundFilter = sensor_ns.class_("RoundFilter", Filter) RoundMultipleFilter = sensor_ns.class_("RoundMultipleFilter", Filter) +RoundSignificantDigitsFilter = sensor_ns.class_("RoundSignificantDigitsFilter", Filter) validate_unit_of_measurement = cv.All( cv.string_strict, @@ -900,6 +902,18 @@ async def round_multiple_filter_to_code(config, filter_id): ) +@FILTER_REGISTRY.register( + "round_to_significant_digits", + RoundSignificantDigitsFilter, + cv.int_range(min=1, max=6), +) +async def round_significant_digits_filter_to_code(config, filter_id): + return cg.new_Pvariable( + filter_id, + cg.TemplateArguments(config), + ) + + async def build_filters(config): return await cg.build_registry_list(FILTER_REGISTRY, config) diff --git a/esphome/components/sensor/filter.h b/esphome/components/sensor/filter.h index a91d66a8fb..917a1ce7d5 100644 --- a/esphome/components/sensor/filter.h +++ b/esphome/components/sensor/filter.h @@ -604,6 +604,19 @@ class RoundMultipleFilter : public Filter { float multiple_; }; +template class RoundSignificantDigitsFilter : public Filter { + public: + optional new_value(float value) override { + if (std::isfinite(value)) { + if (value == 0.0f) + return 0.0f; + float factor = pow10_int(Digits - 1 - ilog10(value)); + return roundf(value * factor) / factor; + } + return value; + } +}; + class ToNTCResistanceFilter : public Filter { public: ToNTCResistanceFilter(double a, double b, double c) : a_(a), b_(b), c_(c) {} diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index 1d0efd01ce..113b6f6187 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -413,6 +413,23 @@ ParseOnOffState parse_on_off(const char *str, const char *on, const char *off) { return PARSE_NONE; } +int8_t ilog10(float value) { + float abs_val = fabsf(value); + int8_t exp = 0; + if (abs_val >= 10.0f) { + while (abs_val >= 10.0f) { + abs_val /= 10.0f; + exp++; + } + } else if (abs_val < 1.0f) { + while (abs_val < 1.0f) { + abs_val *= 10.0f; + exp--; + } + } + return exp; +} + static inline void normalize_accuracy_decimals(float &value, int8_t &accuracy_decimals) { if (accuracy_decimals < 0) { float divisor; diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 6b71916cd2..939852bfcb 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -740,6 +740,11 @@ template class SmallBufferWithHeapFallb /// @name Mathematics ///@{ +/// Compute floor(log10(fabs(value))) using iterative comparison. +/// Avoids pulling in __ieee754_logf/log10f (~1KB flash). +/// Only valid for finite, non-zero values. +int8_t ilog10(float value); + /// Compute 10^exp using iterative multiplication/division. /// Avoids pulling in powf/__ieee754_powf (~2.3KB flash) for small integer exponents. // NOLINT /// Matches powf(10, exp) for the int8_t exponent range used by sensor accuracy_decimals. // NOLINT diff --git a/tests/components/core/helpers_test.cpp b/tests/components/core/helpers_test.cpp new file mode 100644 index 0000000000..468185787f --- /dev/null +++ b/tests/components/core/helpers_test.cpp @@ -0,0 +1,58 @@ +#include +#include +#include "esphome/core/helpers.h" + +namespace esphome { + +TEST(HelpersTest, Ilog10PowersOfTen) { + EXPECT_EQ(ilog10(1.0f), 0); + EXPECT_EQ(ilog10(10.0f), 1); + EXPECT_EQ(ilog10(100.0f), 2); + EXPECT_EQ(ilog10(1000.0f), 3); + EXPECT_EQ(ilog10(10000.0f), 4); + EXPECT_EQ(ilog10(100000.0f), 5); + EXPECT_EQ(ilog10(0.1f), -1); + EXPECT_EQ(ilog10(0.001f), -3); +} + +TEST(HelpersTest, Ilog10General) { + EXPECT_EQ(ilog10(5.0f), 0); + EXPECT_EQ(ilog10(9.99f), 0); + EXPECT_EQ(ilog10(50.0f), 1); + EXPECT_EQ(ilog10(99.0f), 1); + EXPECT_EQ(ilog10(999.0f), 2); + EXPECT_EQ(ilog10(0.5f), -1); + EXPECT_EQ(ilog10(0.0072f), -3); + EXPECT_EQ(ilog10(120000.0f), 5); + EXPECT_EQ(ilog10(123456.789f), 5); +} + +TEST(HelpersTest, Ilog10Negative) { + EXPECT_EQ(ilog10(-1.0f), 0); + EXPECT_EQ(ilog10(-10.0f), 1); + EXPECT_EQ(ilog10(-0.1f), -1); + EXPECT_EQ(ilog10(-123.456f), 2); +} + +// Verify that ilog10 + pow10_int produces the same rounding result as log10/pow. +// ilog10 may differ from floor(log10f()) for values not exactly representable in float +// (e.g. 0.01f is 0.00999...), but the full round-trip must match. +TEST(HelpersTest, Ilog10RoundTripMatchesLog10) { + float values[] = {0.0072f, 0.05f, 0.1f, 0.5f, 1.0f, 3.14f, 9.99f, 10.0f, 42.0f, 100.0f, + 1234.5f, 9999.0f, 10000.0f, 99999.0f, 120000.0f, 999999.0f, -1.0f, -0.1f, -123.456f, -10000.0f}; + for (uint8_t digits = 1; digits <= 6; digits++) { + for (float v : values) { + // New implementation using ilog10 + pow10_int + float factor_new = pow10_int(digits - 1 - ilog10(v)); + float result_new = roundf(v * factor_new) / factor_new; + + // Reference using log10/pow + double factor_ref = pow(10.0, digits - std::ceil(std::log10(std::fabs(v)))); + float result_ref = static_cast(round(v * factor_ref) / factor_ref); + + EXPECT_FLOAT_EQ(result_new, result_ref) << "mismatch for value=" << v << " digits=" << (int) digits; + } + } +} + +} // namespace esphome diff --git a/tests/components/template/common-base.yaml b/tests/components/template/common-base.yaml index ed398b0abd..ecc65de66c 100644 --- a/tests/components/template/common-base.yaml +++ b/tests/components/template/common-base.yaml @@ -171,6 +171,7 @@ sensor: quantile: .9 - round: 1 - round_to_multiple_of: 0.25 + - round_to_significant_digits: 3 - skip_initial: 3 - sliding_window_moving_average: window_size: 15 From 7560112144bf9e586d6ab54854294442ab7b6352 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 21 Apr 2026 22:08:41 +0200 Subject: [PATCH 03/15] Bump aioesphomeapi from 44.16.1 to 44.17.0 (#15906) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 1623876cb5..95d7c8c032 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,7 +12,7 @@ platformio==6.1.19 esptool==5.2.0 click==8.3.2 esphome-dashboard==20260408.1 -aioesphomeapi==44.16.1 +aioesphomeapi==44.17.0 zeroconf==0.148.0 puremagic==1.30 ruamel.yaml==0.19.1 # dashboard_import From ee91ad8f068391a213cd1aa04b13aeed7e9a5dee Mon Sep 17 00:00:00 2001 From: Keith Burzinski Date: Tue, 21 Apr 2026 18:25:05 -0500 Subject: [PATCH 04/15] [esp32] Add Secure Boot V1 ECDSA signing scheme for pre-rev-3.0 ESP32 (#15882) --- esphome/components/esp32/__init__.py | 95 +++++++++++--- esphome/components/esp32/post_build.py.script | 123 +++++++++++++++++- .../esp32/dummy_signing_key_v1_ecdsa.pem | 7 + .../esp32/test-signed_ota_v1.esp32-idf.yaml | 10 ++ 4 files changed, 212 insertions(+), 23 deletions(-) create mode 100644 tests/components/esp32/dummy_signing_key_v1_ecdsa.pem create mode 100644 tests/components/esp32/test-signed_ota_v1.esp32-idf.yaml diff --git a/esphome/components/esp32/__init__.py b/esphome/components/esp32/__init__.py index a68614cb43..77b405a449 100644 --- a/esphome/components/esp32/__init__.py +++ b/esphome/components/esp32/__init__.py @@ -128,23 +128,30 @@ ASSERTION_LEVELS = { SIGNING_SCHEMES = { "rsa3072": "CONFIG_SECURE_SIGNED_APPS_RSA_SCHEME", "ecdsa256": "CONFIG_SECURE_SIGNED_APPS_ECDSA_V2_SCHEME", + "ecdsa_v1": "CONFIG_SECURE_SIGNED_APPS_ECDSA_SCHEME", } -# Chip variants that only support one signing scheme for Secure Boot V2. +# Chip variants that only support one V2 signing scheme. # Based on SOC_SECURE_BOOT_V2_RSA / SOC_SECURE_BOOT_V2_ECC in soc_caps.h. -# Variants not listed in either set support both RSA and ECDSA +# Variants not listed in either set support both RSA and ECDSA V2 # (e.g. C5, C6, H2, P4). New variants should be added to the # appropriate set if they only support one scheme. -SIGNED_OTA_RSA_ONLY_VARIANTS = { - VARIANT_ESP32, +# Note: VARIANT_ESP32 is not listed here because it supports V2 RSA only +# when minimum_chip_revision >= 3.0, which requires special handling. +SIGNED_OTA_V2_RSA_ONLY_VARIANTS = { VARIANT_ESP32S2, VARIANT_ESP32S3, VARIANT_ESP32C3, } -SIGNED_OTA_ECC_ONLY_VARIANTS = { +SIGNED_OTA_V2_ECC_ONLY_VARIANTS = { VARIANT_ESP32C2, VARIANT_ESP32C61, } +# V1 ECDSA (Secure Boot V1) is only supported on the original ESP32. +# Based on SOC_SECURE_BOOT_V1 in soc_caps.h. +SIGNED_OTA_V1_ECDSA_VARIANTS = { + VARIANT_ESP32, +} COMPILER_OPTIMIZATIONS = { "DEBUG": "CONFIG_COMPILER_OPTIMIZATION_DEBUG", @@ -991,25 +998,73 @@ def final_validate(config): if signed_ota := advanced.get(CONF_SIGNED_OTA_VERIFICATION): scheme = signed_ota[CONF_SIGNING_SCHEME] variant = config[CONF_VARIANT] - scheme_variant_conflicts = { - "ecdsa256": (SIGNED_OTA_RSA_ONLY_VARIANTS, "rsa3072"), - "rsa3072": (SIGNED_OTA_ECC_ONLY_VARIANTS, "ecdsa256"), - } - if (conflict := scheme_variant_conflicts.get(scheme)) and variant in conflict[ - 0 - ]: + min_rev = advanced.get(CONF_MINIMUM_CHIP_REVISION) + scheme_path = [ + CONF_FRAMEWORK, + CONF_ADVANCED, + CONF_SIGNED_OTA_VERIFICATION, + CONF_SIGNING_SCHEME, + ] + + # V1 ECDSA is only available on the original ESP32 + if scheme == "ecdsa_v1" and variant not in SIGNED_OTA_V1_ECDSA_VARIANTS: errs.append( cv.Invalid( - f"Signing scheme '{scheme}' is not supported on " - f"{VARIANT_FRIENDLY[variant]}. Use '{conflict[1]}' instead.", - path=[ - CONF_FRAMEWORK, - CONF_ADVANCED, - CONF_SIGNED_OTA_VERIFICATION, - CONF_SIGNING_SCHEME, - ], + f"Signing scheme 'ecdsa_v1' is only supported on " + f"{VARIANT_FRIENDLY[VARIANT_ESP32]}. " + f"Use 'rsa3072' or 'ecdsa256' instead.", + path=scheme_path, ) ) + elif variant == VARIANT_ESP32: + # On ESP32, V2 RSA requires minimum_chip_revision >= 3.0 + # Note: string comparison works here because cv.one_of constrains + # min_rev to known ESP32_CHIP_REVISIONS values ("0.0".."3.1"). + if scheme == "rsa3072" and (min_rev is None or min_rev < "3.0"): + errs.append( + cv.Invalid( + f"Signing scheme 'rsa3072' on {VARIANT_FRIENDLY[variant]} " + f"requires minimum_chip_revision: '3.0' or higher " + f"(Secure Boot V2 RSA needs chip revision 3.0+). " + f"For older chip revisions, use 'ecdsa_v1' instead.", + path=scheme_path, + ) + ) + # ESP32 does not support V2 ECDSA (no SOC_SECURE_BOOT_V2_ECC) + elif scheme == "ecdsa256": + errs.append( + cv.Invalid( + f"Signing scheme 'ecdsa256' is not supported on " + f"{VARIANT_FRIENDLY[variant]}. Use 'rsa3072' (with " + f"minimum_chip_revision: '3.0') or 'ecdsa_v1' instead.", + path=scheme_path, + ) + ) + # V1 on rev 3.0+ -- suggest V2 RSA for stronger security + elif scheme == "ecdsa_v1" and min_rev is not None and min_rev >= "3.0": + _LOGGER.info( + "Using Secure Boot V1 ECDSA on %s rev %s. " + "Consider using 'rsa3072' (Secure Boot V2 RSA) for " + "stronger security on chip revision 3.0+.", + VARIANT_FRIENDLY[variant], + min_rev, + ) + else: + # Non-ESP32 variants: check V2 scheme-variant compatibility + scheme_variant_conflicts = { + "ecdsa256": (SIGNED_OTA_V2_RSA_ONLY_VARIANTS, "rsa3072"), + "rsa3072": (SIGNED_OTA_V2_ECC_ONLY_VARIANTS, "ecdsa256"), + } + if ( + conflict := scheme_variant_conflicts.get(scheme) + ) and variant in conflict[0]: + errs.append( + cv.Invalid( + f"Signing scheme '{scheme}' is not supported on " + f"{VARIANT_FRIENDLY[variant]}. Use '{conflict[1]}' instead.", + path=scheme_path, + ) + ) if CONF_OTA not in full_config: _LOGGER.warning( "Signed OTA verification is enabled but no OTA component is configured. " diff --git a/esphome/components/esp32/post_build.py.script b/esphome/components/esp32/post_build.py.script index 8d13214259..b329f6b82b 100644 --- a/esphome/components/esp32/post_build.py.script +++ b/esphome/components/esp32/post_build.py.script @@ -5,6 +5,7 @@ import json # noqa: E402 import os # noqa: E402 import pathlib # noqa: E402 import shutil # noqa: E402 +import subprocess # noqa: E402 from glob import glob # noqa: E402 @@ -25,6 +26,114 @@ def _parse_sdkconfig(sdkconfig_path): return options +def _generate_v1_verification_key(env): + """Generate the V1 ECDSA verification key binary and assembly source file. + + Secure Boot V1 embeds the public verification key directly in the app binary + as a compiled object (via a .S assembly file). The ESP-IDF CMake build generates + these files via custom commands, but PlatformIO's SCons bridge does not execute + them. This function replicates that logic: + 1. Extracts the raw public key from the PEM signing key using espsecure. + 2. Generates the .S assembly source that embeds the key bytes. + """ + build_dir = pathlib.Path(env.subst("$BUILD_DIR")) + project_dir = pathlib.Path(env.subst("$PROJECT_DIR")) + pioenv = env.subst("$PIOENV") + sdkconfig = _parse_sdkconfig(project_dir / f"sdkconfig.{pioenv}") + + if sdkconfig.get("CONFIG_SECURE_SIGNED_APPS_ECDSA_SCHEME") != "y": + return + + bin_path = build_dir / "signature_verification_key.bin" + asm_path = build_dir / "signature_verification_key.bin.S" + + # Determine the source of the verification key + if sdkconfig.get("CONFIG_SECURE_BOOT_BUILD_SIGNED_BINARIES") == "y": + # Extract public key from the signing key + signing_key = sdkconfig.get("CONFIG_SECURE_BOOT_SIGNING_KEY") + if not signing_key: + return + signing_key_path = pathlib.Path(signing_key) + if not signing_key_path.exists(): + print(f"Error: V1 ECDSA signing key not found: {signing_key_path}") + env.Exit(1) + return + + if not bin_path.exists() or bin_path.stat().st_mtime < signing_key_path.stat().st_mtime: + python_exe = env.subst("$PYTHONEXE") + result = subprocess.run( + [python_exe, "-m", "espsecure", "extract_public_key", + "--keyfile", str(signing_key_path), str(bin_path)], + capture_output=True, text=True, + ) + if result.returncode != 0: + print(f"Error extracting V1 verification key: {result.stderr}") + env.Exit(1) + return + print(f"Extracted V1 ECDSA verification key from {signing_key_path.name}") + else: + # User-provided verification key -- should already be a raw binary file + verification_key = sdkconfig.get("CONFIG_SECURE_BOOT_VERIFICATION_KEY") + if not verification_key: + return + verification_key_path = pathlib.Path(verification_key) + if not verification_key_path.exists(): + print(f"Error: Verification key not found: {verification_key_path}") + env.Exit(1) + return + shutil.copyfile(str(verification_key_path), str(bin_path)) + + if not bin_path.exists(): + return + + # Generate the .S assembly file from the binary key data. + # Replicates ESP-IDF's data_file_embed_asm.cmake with RENAME_TO=signature_verification_key_bin. + # The file is needed in both the app build dir and the bootloader build dir, since + # the bootloader also embeds the verification key when CONFIG_SECURE_SIGNED_ON_BOOT_NO_SECURE_BOOT + # is enabled. PlatformIO's SCons bridge does not execute the CMake custom commands that + # normally generate these files. + data = bin_path.read_bytes() + varname = "signature_verification_key_bin" + + lines = [] + lines.append(f"/* Data converted from {bin_path.name} */") + lines.append(".data") + lines.append("#if !defined (__APPLE__) && !defined (__linux__)") + lines.append(".section .rodata.embedded") + lines.append("#endif") + lines.append(f"\n.global {varname}") + lines.append(f"{varname}:") + lines.append(f"\n.global _binary_{varname}_start") + lines.append(f"_binary_{varname}_start: /* for objcopy compatibility */") + + # Format binary data as .byte lines (16 bytes per line) + for i in range(0, len(data), 16): + chunk = data[i:i + 16] + hex_bytes = ", ".join(f"0x{b:02x}" for b in chunk) + lines.append(f".byte {hex_bytes}") + + lines.append(f"\n.global _binary_{varname}_end") + lines.append(f"_binary_{varname}_end: /* for objcopy compatibility */") + lines.append(f"\n.global {varname}_length") + lines.append(f"{varname}_length:") + lines.append(f".long {len(data)}") + lines.append("") + lines.append('#if defined (__linux__)') + lines.append('.section .note.GNU-stack,"",@progbits') + lines.append("#endif") + + asm_content = "\n".join(lines) + "\n" + + # Write to app build dir and bootloader build dir + asm_path.write_text(asm_content) + bootloader_dir = build_dir / "bootloader" + if bootloader_dir.is_dir(): + bootloader_bin = bootloader_dir / "signature_verification_key.bin" + bootloader_asm = bootloader_dir / "signature_verification_key.bin.S" + shutil.copyfile(str(bin_path), str(bootloader_bin)) + bootloader_asm.write_text(asm_content) + + def sign_firmware(source, target, env): """ Sign the firmware binary using espsecure.py if signed OTA verification is enabled. @@ -55,9 +164,12 @@ def sign_firmware(source, target, env): env.Exit(1) return - # ESPHome only exposes RSA3072 and ECDSA256 (both Secure Boot V2 schemes), - # so the espsecure signature version is always 2. - sign_version = "2" + # Determine espsecure signature version from the signing scheme: + # V1 ECDSA (Secure Boot V1) uses --version 1, V2 RSA/ECDSA use --version 2. + if sdkconfig.get("CONFIG_SECURE_SIGNED_APPS_ECDSA_SCHEME") == "y": + sign_version = "1" + else: + sign_version = "2" firmware_name = os.path.basename(env.subst("$PROGNAME")) + ".bin" firmware_path = build_dir / firmware_name @@ -217,6 +329,11 @@ def esp32_copy_ota_bin(source, target, env): print(f"Copied firmware to {new_file_name}") +# Generate V1 ECDSA verification key files before build starts. +# Workaround for PlatformIO not executing CMake custom commands that extract +# the public key and generate the .S assembly file for Secure Boot V1. +_generate_v1_verification_key(env) # noqa: F821 + # Run signing first, then merge, then ota copy env.AddPostAction("$BUILD_DIR/${PROGNAME}.bin", sign_firmware) # noqa: F821 env.AddPostAction("$BUILD_DIR/${PROGNAME}.bin", merge_factory_bin) # noqa: F821 diff --git a/tests/components/esp32/dummy_signing_key_v1_ecdsa.pem b/tests/components/esp32/dummy_signing_key_v1_ecdsa.pem new file mode 100644 index 0000000000..bd09205606 --- /dev/null +++ b/tests/components/esp32/dummy_signing_key_v1_ecdsa.pem @@ -0,0 +1,7 @@ +*** DO NOT USE THIS KEY...EVER *** +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIEZIp96p7Z7QN6vxOFE5FdRNm535vW81Ax07KnGxVjiMoAoGCCqGSM49 +AwEHoUQDQgAEK+fBQDn1Q+r5lGwcDoMUgeg2Aq16LLrLUz7xWI6mS0PUClzolDIo +eaV/Pfjl7zAvkbQQsZq3rTNnr1eGAk5P+A== +-----END EC PRIVATE KEY----- +*** DO NOT USE THIS KEY...EVER *** diff --git a/tests/components/esp32/test-signed_ota_v1.esp32-idf.yaml b/tests/components/esp32/test-signed_ota_v1.esp32-idf.yaml new file mode 100644 index 0000000000..b32e157daf --- /dev/null +++ b/tests/components/esp32/test-signed_ota_v1.esp32-idf.yaml @@ -0,0 +1,10 @@ +esp32: + variant: esp32 + framework: + type: esp-idf + advanced: + signed_ota_verification: + signing_key: ../../components/esp32/dummy_signing_key_v1_ecdsa.pem + signing_scheme: ecdsa_v1 + +<<: !include common.yaml From b20fedd806d44ad3b3241347e39595cdea4b9089 Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Wed, 22 Apr 2026 13:18:21 +1200 Subject: [PATCH 05/15] [bl0906] Disable loop when idle and introduce BL0906Stage enum (#15884) Co-authored-by: J. Nick Koston --- esphome/components/bl0906/bl0906.cpp | 67 +++++++++++++++++++--------- esphome/components/bl0906/bl0906.h | 19 +++++++- 2 files changed, 65 insertions(+), 21 deletions(-) diff --git a/esphome/components/bl0906/bl0906.cpp b/esphome/components/bl0906/bl0906.cpp index 70db235a37..d554057f7b 100644 --- a/esphome/components/bl0906/bl0906.cpp +++ b/esphome/components/bl0906/bl0906.cpp @@ -20,58 +20,77 @@ constexpr uint8_t bl0906_checksum(const uint8_t address, const DataPacket *data) } void BL0906::loop() { - if (this->current_channel_ == UINT8_MAX) { - return; - } - while (this->available()) this->flush(); - if (this->current_channel_ == 0) { + if (this->current_stage_ == STAGE_IDLE) { + // Woken up between cycles to drain the action queue. Go back to sleep. + this->handle_actions_(); + this->disable_loop(); + return; + } + + if (this->current_stage_ == STAGE_TEMP) { // Temperature this->read_data_(BL0906_TEMPERATURE, BL0906_TREF, this->temperature_sensor_); - } else if (this->current_channel_ == 1) { + } else if (this->current_stage_ == STAGE_CHANNEL_1) { this->read_data_(BL0906_I_1_RMS, BL0906_IREF, this->current_1_sensor_); this->read_data_(BL0906_WATT_1, BL0906_PREF, this->power_1_sensor_); this->read_data_(BL0906_CF_1_CNT, BL0906_EREF, this->energy_1_sensor_); - } else if (this->current_channel_ == 2) { + } else if (this->current_stage_ == STAGE_CHANNEL_2) { this->read_data_(BL0906_I_2_RMS, BL0906_IREF, this->current_2_sensor_); this->read_data_(BL0906_WATT_2, BL0906_PREF, this->power_2_sensor_); this->read_data_(BL0906_CF_2_CNT, BL0906_EREF, this->energy_2_sensor_); - } else if (this->current_channel_ == 3) { + } else if (this->current_stage_ == STAGE_CHANNEL_3) { this->read_data_(BL0906_I_3_RMS, BL0906_IREF, this->current_3_sensor_); this->read_data_(BL0906_WATT_3, BL0906_PREF, this->power_3_sensor_); this->read_data_(BL0906_CF_3_CNT, BL0906_EREF, this->energy_3_sensor_); - } else if (this->current_channel_ == 4) { + } else if (this->current_stage_ == STAGE_CHANNEL_4) { this->read_data_(BL0906_I_4_RMS, BL0906_IREF, this->current_4_sensor_); this->read_data_(BL0906_WATT_4, BL0906_PREF, this->power_4_sensor_); this->read_data_(BL0906_CF_4_CNT, BL0906_EREF, this->energy_4_sensor_); - } else if (this->current_channel_ == 5) { + } else if (this->current_stage_ == STAGE_CHANNEL_5) { this->read_data_(BL0906_I_5_RMS, BL0906_IREF, this->current_5_sensor_); this->read_data_(BL0906_WATT_5, BL0906_PREF, this->power_5_sensor_); this->read_data_(BL0906_CF_5_CNT, BL0906_EREF, this->energy_5_sensor_); - } else if (this->current_channel_ == 6) { + } else if (this->current_stage_ == STAGE_CHANNEL_6) { this->read_data_(BL0906_I_6_RMS, BL0906_IREF, this->current_6_sensor_); this->read_data_(BL0906_WATT_6, BL0906_PREF, this->power_6_sensor_); this->read_data_(BL0906_CF_6_CNT, BL0906_EREF, this->energy_6_sensor_); - } else if (this->current_channel_ == UINT8_MAX - 2) { + } else if (this->current_stage_ == STAGE_FREQ) { // Frequency - this->read_data_(BL0906_FREQUENCY, BL0906_FREF, frequency_sensor_); + this->read_data_(BL0906_FREQUENCY, BL0906_FREF, this->frequency_sensor_); // Voltage - this->read_data_(BL0906_V_RMS, BL0906_UREF, voltage_sensor_); - } else if (this->current_channel_ == UINT8_MAX - 1) { + this->read_data_(BL0906_V_RMS, BL0906_UREF, this->voltage_sensor_); + } else if (this->current_stage_ == STAGE_POWER) { // Total power this->read_data_(BL0906_WATT_SUM, BL0906_WATT, this->total_power_sensor_); // Total Energy this->read_data_(BL0906_CF_SUM_CNT, BL0906_CF, this->total_energy_sensor_); - } else { - this->current_channel_ = UINT8_MAX - 2; // Go to frequency and voltage - return; } - this->current_channel_++; + this->advance_stage_(); this->handle_actions_(); } +void BL0906::advance_stage_() { + switch (this->current_stage_) { + case STAGE_CHANNEL_6: + this->current_stage_ = STAGE_FREQ; + break; + case STAGE_FREQ: + this->current_stage_ = STAGE_POWER; + break; + case STAGE_POWER: + // Cycle complete; sleep until the next update(). + this->current_stage_ = STAGE_IDLE; + this->disable_loop(); + break; + default: + this->current_stage_ = static_cast(this->current_stage_ + 1); + break; + } +} + void BL0906::setup() { while (this->available()) this->flush(); @@ -85,12 +104,20 @@ void BL0906::setup() { this->bias_correction_(BL0906_RMSOS_6, 0.01200, 0); // Calibration current_6 this->write_array(USR_WRPROT_ONLYREAD, sizeof(USR_WRPROT_ONLYREAD)); + + // Loop stays idle until the first update() or enqueued action. + this->disable_loop(); } -void BL0906::update() { this->current_channel_ = 0; } +void BL0906::update() { + this->current_stage_ = STAGE_TEMP; + this->enable_loop(); +} size_t BL0906::enqueue_action_(ActionCallbackFuncPtr function) { this->action_queue_.push_back(function); + // Ensure the queue is serviced even if the read cycle has already completed. + this->enable_loop(); return this->action_queue_.size(); } diff --git a/esphome/components/bl0906/bl0906.h b/esphome/components/bl0906/bl0906.h index 493b645c89..f7ba5423d2 100644 --- a/esphome/components/bl0906/bl0906.h +++ b/esphome/components/bl0906/bl0906.h @@ -12,6 +12,22 @@ namespace esphome { namespace bl0906 { +// Stage values for the read state machine. After STAGE_CHANNEL_6 the state machine +// jumps to the two sentinel stages below, then to STAGE_IDLE which marks the cycle +// as complete and disables the loop. +enum BL0906Stage : uint8_t { + STAGE_TEMP = 0, // chip temperature + STAGE_CHANNEL_1 = 1, // per-phase current + power + energy + STAGE_CHANNEL_2 = 2, + STAGE_CHANNEL_3 = 3, + STAGE_CHANNEL_4 = 4, + STAGE_CHANNEL_5 = 5, + STAGE_CHANNEL_6 = 6, + STAGE_FREQ = UINT8_MAX - 2, // frequency + voltage + STAGE_POWER = UINT8_MAX - 1, // total power + total energy + STAGE_IDLE = UINT8_MAX, // cycle complete +}; + struct DataPacket { // NOLINT(altera-struct-pack-align) uint8_t l{0}; uint8_t m{0}; @@ -79,7 +95,8 @@ class BL0906 : public PollingComponent, public uart::UARTDevice { void bias_correction_(uint8_t address, float measurements, float correction); - uint8_t current_channel_{0}; + BL0906Stage current_stage_{STAGE_IDLE}; + void advance_stage_(); size_t enqueue_action_(ActionCallbackFuncPtr function); void handle_actions_(); From 9cebce1b6ecefcdb6d2c8fa3b6b854f08c24484e Mon Sep 17 00:00:00 2001 From: Javier Peletier Date: Wed, 22 Apr 2026 03:19:01 +0200 Subject: [PATCH 06/15] [substitutions] Improve error messages with include stack trace (#15874) Co-authored-by: J. Nick Koston --- esphome/components/packages/__init__.py | 64 +++++-- esphome/components/substitutions/__init__.py | 83 +++----- esphome/yaml_util.py | 119 ++++++++++++ .../component_tests/packages/test_packages.py | 24 ++- tests/unit_tests/test_substitutions.py | 47 ++++- tests/unit_tests/test_yaml_util.py | 181 +++++++++++++++++- 6 files changed, 432 insertions(+), 86 deletions(-) diff --git a/esphome/components/packages/__init__.py b/esphome/components/packages/__init__.py index 97a5309480..b6ec0067c9 100644 --- a/esphome/components/packages/__init__.py +++ b/esphome/components/packages/__init__.py @@ -42,6 +42,11 @@ DOMAIN = CONF_PACKAGES # Guard against infinite include chains (e.g. A includes B includes A). MAX_INCLUDE_DEPTH = 20 +PackageCallback = Callable[ + [dict | str | yaml_util.IncludeFile, ContextVars | None, yaml_util.DocumentPath], + dict, +] + def is_remote_package(package_config: dict) -> bool: """Returns True if the package_config is a remote package definition.""" @@ -281,8 +286,9 @@ def _process_remote_package(config: dict, skip_update: bool = False) -> dict: def _walk_package_dict( packages: dict, - callback: Callable[[dict, ContextVars | None], dict], + callback: PackageCallback, context: ContextVars | None, + path: yaml_util.DocumentPath, ) -> cv.Invalid | None: """Iterate a packages dict in reverse priority order, invoking callback on each entry. @@ -291,7 +297,9 @@ def _walk_package_dict( for package_name, package_config in reversed(packages.items()): with cv.prepend_path(package_name): try: - packages[package_name] = callback(package_config, context) + packages[package_name] = callback( + package_config, context, path + [package_name] + ) except cv.Invalid as err: return err return None @@ -299,20 +307,22 @@ def _walk_package_dict( def _walk_package_list( packages: list, - callback: Callable[[dict, ContextVars | None], dict], + callback: PackageCallback, context: ContextVars | None, + path: yaml_util.DocumentPath, ) -> None: """Iterate a packages list in reverse priority order, invoking callback on each entry.""" for idx in reversed(range(len(packages))): with cv.prepend_path(idx): - packages[idx] = callback(packages[idx], context) + packages[idx] = callback(packages[idx], context, path + [idx]) def _walk_packages( config: dict, - callback: Callable[[dict, ContextVars | None], dict], + callback: PackageCallback, context: ContextVars | None = None, validate_deprecated: bool = True, + path: yaml_util.DocumentPath | None = None, ) -> dict: """Walks the packages structure in priority order, invoking ``callback`` on each package definition found. @@ -323,19 +333,24 @@ def _walk_packages( if CONF_PACKAGES not in config: return config packages = config[CONF_PACKAGES] + packages_path = (path or []) + [CONF_PACKAGES] with cv.prepend_path(CONF_PACKAGES): if isinstance(packages, yaml_util.IncludeFile): # If the packages key is an IncludeFile, resolve it first before processing. - packages, _ = resolve_include(packages, [], context, strict_undefined=False) + packages = resolve_include( + packages, packages_path, context, strict_undefined=False + ) if not isinstance(packages, (dict, list)): raise cv.Invalid( f"Packages must be a key to value mapping or list, got {type(packages)} instead" ) if not isinstance(packages, dict): - _walk_package_list(packages, callback, context) - elif (result := _walk_package_dict(packages, callback, context)) is not None: + _walk_package_list(packages, callback, context, packages_path) + elif ( + result := _walk_package_dict(packages, callback, context, packages_path) + ) is not None: if not validate_deprecated or any( is_package_definition(v) for v in packages.values() ): @@ -344,14 +359,18 @@ def _walk_packages( # This block can be removed once the single-package # deprecation period (2026.7.0) is over. config[CONF_PACKAGES] = [packages] - return _walk_packages(deprecate_single_package(config), callback, context) + return _walk_packages( + deprecate_single_package(config), callback, context, path=path + ) config[CONF_PACKAGES] = packages return config def _substitute_package_definition( - package_config: dict | str, context_vars: ContextVars | None + package_config: dict | str, + context_vars: ContextVars | None, + path: yaml_util.DocumentPath | None = None, ) -> dict | str: """Substitute variables in a package definition string or remote package dict. @@ -369,12 +388,12 @@ def _substitute_package_definition( errors: ErrList = [] package_config = substitute( item=package_config, - path=[], + path=path or [], parent_context=context_vars or ContextVars(), strict_undefined=False, errors=errors, ) - raise_first_undefined(errors, package_config, "package definition") + raise_first_undefined(errors, "package definition") return package_config @@ -432,6 +451,7 @@ class _PackageProcessor: self, package_config: dict | str | yaml_util.IncludeFile, context_vars: ContextVars | None, + path: yaml_util.DocumentPath, ) -> dict: """Resolve a package definition to a concrete ``dict`` and fetch remote packages. @@ -454,15 +474,15 @@ class _PackageProcessor: """ for _ in range(MAX_INCLUDE_DEPTH): if isinstance(package_config, yaml_util.IncludeFile): - package_config, _ = resolve_include( + package_config = resolve_include( package_config, - [], + path, context_vars or ContextVars(), strict_undefined=False, ) package_config = _substitute_package_definition( - package_config, context_vars + package_config, context_vars, path ) package_config = PACKAGE_SCHEMA(package_config) if isinstance(package_config, dict): @@ -483,13 +503,16 @@ class _PackageProcessor: _update_substitutions_context(self.parent_context, subs) def process_package( - self, package_config: dict | str, context_vars: ContextVars | None + self, + package_config: dict | str, + context_vars: ContextVars | None, + path: yaml_util.DocumentPath, ) -> dict: """Resolve a single package and recurse into any nested packages.""" from_remote = isinstance(package_config, dict) and is_remote_package( package_config ) - package_config = self.resolve_package(package_config, context_vars) + package_config = self.resolve_package(package_config, context_vars, path) self.collect_substitutions(package_config) if CONF_PACKAGES not in package_config: @@ -509,6 +532,7 @@ class _PackageProcessor: self.process_package, context_vars, validate_deprecated=not from_remote, + path=path, ) @@ -565,11 +589,13 @@ def merge_packages(config: dict) -> dict: merge_list: list[dict] = [] def process_package_callback( - package_config: dict, context: ContextVars | None + package_config: dict, + context: ContextVars | None, + path: yaml_util.DocumentPath | None = None, ) -> dict: """This will be called for each package found in the config.""" merge_list.append(package_config) - return _walk_packages(package_config, process_package_callback) + return _walk_packages(package_config, process_package_callback, path=path) _walk_packages(config, process_package_callback, validate_deprecated=False) # Merge all packages into the main config: diff --git a/esphome/components/substitutions/__init__.py b/esphome/components/substitutions/__init__.py index 8bbccffca1..fb7cd7c51b 100644 --- a/esphome/components/substitutions/__init__.py +++ b/esphome/components/substitutions/__init__.py @@ -11,9 +11,11 @@ from esphome.types import ConfigType from esphome.util import OrderedDict from esphome.yaml_util import ( ConfigContext, + DocumentPath, ESPHomeDataBase, ESPLiteralValue, IncludeFile, + format_path, make_data_base, ) @@ -23,8 +25,8 @@ CODEOWNERS = ["@esphome/core"] _LOGGER = logging.getLogger(__name__) ContextVars = ChainMap[str, Any] -SubstitutionPath = list[int | str] -ErrList = list[tuple[UndefinedError, SubstitutionPath, Any]] +ErrList = list[tuple[UndefinedError, DocumentPath, Any]] + # Module-level instance is safe: context_vars is passed per-call, and context_trace # is stack-saved/restored within expand(). Not thread-safe — only use from one thread. jinja = Jinja() @@ -32,16 +34,13 @@ jinja = Jinja() def raise_first_undefined( errors: ErrList, - source: Any, context_label: str, ) -> None: """If *errors* is non-empty, raise ``cv.Invalid`` for the first undefined variable. - The raised error names the missing variable, the path walked into *source* - (for nested dicts, e.g. ``url`` or ``ref``), and the YAML source location - when *source* carries one. Only the first error is surfaced; the user will - re-run after fixing it and any remaining undefined variables will be - reported then. + The raised error names the missing variable and its location in the include + stack. Only the first error is surfaced; the user will re-run after fixing it + and any remaining undefined variables will be reported then. ``context_label`` is the noun describing where the undefined variable appeared (e.g. ``"package definition"``). @@ -57,26 +56,8 @@ def raise_first_undefined( for e, p_path, _ in errors[1:] ) _LOGGER.debug("Additional undefined variables in %s: %s", context_label, extras) - # Prefer the location of the offending scalar (e.g. the `url:` value) over - # the enclosing package-definition dict so the message points at the exact - # line/column that carries the undefined variable. - location_node = ( - err_value - if isinstance(err_value, ESPHomeDataBase) and err_value.esp_range is not None - else source - ) - location = "" - if ( - isinstance(location_node, ESPHomeDataBase) - and location_node.esp_range is not None - ): - mark = location_node.esp_range.start_mark - # DocumentLocation.line/column are 0-based (from the YAML Mark). Render - # as 1-based to match config.line_info() and editor line numbering. - location = f" (in {mark.document} {mark.line + 1}:{mark.column + 1})" - field = f" at '{'->'.join(str(p) for p in err_path)}'" if err_path else "" raise cv.Invalid( - f"Undefined variable in {context_label}{field}: {err.message}{location}" + f"Undefined variable in {context_label}: {err.message}\n{format_path(err_path, err_value)}" ) @@ -145,7 +126,7 @@ def _resolve_var(name: str, context_vars: ContextVars) -> Any: def _handle_undefined( err: UndefinedError, - path: SubstitutionPath, + path: DocumentPath, value: Any, strict_undefined: bool, errors: ErrList | None, @@ -163,7 +144,7 @@ def _handle_undefined( def _expand_substitutions( value: str, - path: SubstitutionPath, + path: DocumentPath, context_vars: ContextVars, strict_undefined: bool, errors: ErrList | None, @@ -236,7 +217,7 @@ def _expand_substitutions( f"\nEvaluation stack: (most recent evaluation last)" f"\n{err.stack_trace_str()}" f"\nRelevant context:\n{err.context_trace_str()}" - f"\nSee {'->'.join(str(x) for x in path)}", + f"\n{format_path(path, orig_value)}", path, ) from err else: @@ -345,15 +326,13 @@ def push_context( def resolve_include( include: IncludeFile, - path: list[int | str], + path: DocumentPath, context_vars: ContextVars, strict_undefined: bool = True, errors: ErrList | None = None, -) -> tuple[Any, str]: +) -> Any: """Resolve an include, substituting the filename if needed. - Returns the loaded content and the resolved filename. - Note: no path-traversal validation is performed on the resolved filename. A substitution that resolves to an absolute path will bypass the parent directory (Path.__truediv__ ignores the left operand for absolute paths). @@ -361,44 +340,44 @@ def resolve_include( values (including command-line substitutions), so path restrictions are an explicit non-goal here. """ - original = str(include.file) + original = include.file + original_str = str(original) filename = str( _expand_substitutions( - original, path + ["file"], context_vars, strict_undefined, errors + original_str, path + ["file"], context_vars, strict_undefined, errors ) ) - if filename != original: + substituted = filename != original_str + if substituted: include = IncludeFile( include.parent_file, filename, include.vars, include.yaml_loader ) try: - return include.load(), filename + return include.load() except esphome.core.EsphomeError as err: + resolved = f" (expanded from '{original}')" if substituted else "" raise cv.Invalid( - f"Error including file '{filename}': {err}", + f"Error including file '{filename}'{resolved}: {err}" + f"\n{format_path(path, original)}", path + [f"<{filename}>"], ) from err def _substitute_include( include: IncludeFile, - path: list[int | str], + path: DocumentPath, context_vars: ContextVars, strict_undefined: bool, errors: ErrList | None, ) -> Any: """Resolve an include and substitute its content.""" - content, filename = resolve_include( - include, path, context_vars, strict_undefined, errors - ) - return substitute( - content, path + [f"<{filename}>"], context_vars, strict_undefined, errors - ) + content = resolve_include(include, path, context_vars, strict_undefined, errors) + return substitute(content, path, context_vars, strict_undefined, errors) def substitute( item: Any, - path: SubstitutionPath, + path: DocumentPath, parent_context: ContextVars, strict_undefined: bool, errors: ErrList | None = None, @@ -451,16 +430,12 @@ def _warn_unresolved_variables(errors: ErrList) -> None: for err, path, expression in errors: if "password" in path: continue - location: str = "->".join(str(x) for x in path) - if isinstance(expression, ESPHomeDataBase) and expression.esp_range is not None: - location += f" in {str(expression.esp_range.start_mark)}" - _LOGGER.warning( "The string '%s' looks like an expression," - " but could not resolve all the variables: %s (see %s)", + " but could not resolve all the variables: %s\n%s", expression, err.message, - location, + format_path(path, expression), ) @@ -479,7 +454,7 @@ def resolve_substitutions_block( # Single-shot resolution — matches ``_walk_packages`` for the # ``packages: !include`` entry point. Chained includes (an include that # itself loads another ``!include`` at the top level) are not supported. - substitutions, _ = resolve_include( + substitutions = resolve_include( substitutions, [], ContextVars(command_line_substitutions or {}), diff --git a/esphome/yaml_util.py b/esphome/yaml_util.py index e15adff935..42da27ec14 100644 --- a/esphome/yaml_util.py +++ b/esphome/yaml_util.py @@ -48,6 +48,8 @@ _SECRET_VALUES = {} # Not thread-safe — config processing is single-threaded today. _load_listeners: list[Callable[[Path], None]] = [] +DocumentPath = list[str | int] + @contextmanager def track_yaml_loads() -> Generator[list[Path]]: @@ -679,6 +681,123 @@ def is_secret(value): return None +def _path_doc(item: Any) -> str | None: + """Return the source document name if *item* carries location info.""" + if isinstance(item, ESPHomeDataBase) and (r := item.esp_range) is not None: + return r.start_mark.document + return None + + +def _fmt_mark(loc: Any) -> str: + """Render a DocumentLocation as a 1-based 'file line:col' string.""" + return f"{loc.document} {loc.line + 1}:{loc.column + 1}" + + +def _obj_loc(obj: Any) -> str: + """Return formatted source location for *obj*, or '' if it has none.""" + if isinstance(obj, ESPHomeDataBase) and (r := obj.esp_range) is not None: + return _fmt_mark(r.start_mark) + return "" + + +def _fmt_segment(seg: list) -> str: + """Format a path segment, rendering integers as [n] subscripts.""" + parts: list[str] = [] + for item in seg: + if isinstance(item, int): + if parts: + parts[-1] = f"{parts[-1]}[{item}]" + else: + parts.append(f"[{item}]") + else: + parts.append(str(item)) + return "->".join(parts) + + +def _split_into_frames( + path: DocumentPath, +) -> list[tuple[list, str]]: + """Group *path* into per-file frames at include boundaries. + + A "frame" is the slice of the path that belongs to one source document. + Each path item is either: + + * a **located key** — has an ``ESPHomeDataBase`` source mark; this is + what tells us which document owns the surrounding keys. + * an **integer** — a list subscript; always attaches to the open frame + (renders as ``foo[3]`` on the previous name). + * an **unlocated string** — a key with no source mark (e.g. constants + like ``CONF_PACKAGES``); it describes the parent of the *next* file, + so it migrates to the next frame when the document changes. + + Returns a list of ``(items, "file line:col")`` tuples in walk order + (outermost frame first). + """ + frames: list[tuple[list, str]] = [] + open_frame: list = [] + next_frame_keys: list = [] # unlocated strings buffered for the next frame + open_doc: str | None = None + open_loc = "" + + for item in path: + doc = _path_doc(item) + if doc is None: + # Ints subscript the open frame's last name; everything else + # (strings, or leading ints with no open frame) is buffered for + # the next frame. + if isinstance(item, int) and open_doc is not None: + open_frame.append(item) + else: + next_frame_keys.append(item) + continue + if open_doc is not None and doc != open_doc: + # Crossed an include boundary: close the open frame. + frames.append((open_frame, open_loc)) + open_frame = [] + open_frame.extend(next_frame_keys) + next_frame_keys.clear() + open_frame.append(item) + open_doc = doc + open_loc = _fmt_mark(item.esp_range.start_mark) + + if open_doc is not None: + # Trailing buffered keys belong to the innermost (last) frame. + open_frame.extend(next_frame_keys) + frames.append((open_frame, open_loc)) + return frames + + +def format_path(path: DocumentPath, current_obj: Any) -> str: + """Build a human-readable include stack from a config path. + + Each YAML key in *path* that carries an ``ESPHomeDataBase`` ``esp_range`` + reveals which file it came from. When the source document changes between + consecutive such keys, that is an include boundary. The path is split + into per-file frames and formatted innermost-first, e.g.:: + + In: packages->roam in common/package/wifi.yaml 26:10 + Included from packages->net in common/hardware.yaml 44:2 + Included from packages->device in my_project.yaml 11:2 + + The innermost ``In:`` line uses the location from *current_obj* when + available (the value that triggered the error) for extra precision. + """ + frames = _split_into_frames(path) + obj_loc = _obj_loc(current_obj) + + if not frames: + # No source info anywhere in the path: render as a flat path, + # using current_obj's location if it happens to have one. + suffix = f" in {obj_loc}" if obj_loc else "" + return f"In: {_fmt_segment(path)}{suffix}" + + inner_seg, inner_loc = frames[-1] + lines = [f"In: {_fmt_segment(inner_seg)} in {obj_loc or inner_loc}"] + for seg, loc in reversed(frames[:-1]): + lines.append(f" Included from {_fmt_segment(seg)} in {loc}") + return "\n".join(lines) + + class ESPHomeDumper(yaml.SafeDumper): def represent_mapping(self, tag, mapping, flow_style=None): value = [] diff --git a/tests/component_tests/packages/test_packages.py b/tests/component_tests/packages/test_packages.py index 0bd339efa9..af4b6db796 100644 --- a/tests/component_tests/packages/test_packages.py +++ b/tests/component_tests/packages/test_packages.py @@ -46,7 +46,7 @@ from esphome.const import ( ) from esphome.core import CORE from esphome.util import OrderedDict -from esphome.yaml_util import IncludeFile, add_context, load_yaml +from esphome.yaml_util import DocumentPath, IncludeFile, add_context, load_yaml # Test strings TEST_DEVICE_NAME = "test_device_name" @@ -1113,7 +1113,7 @@ def test_packages_include_file_resolves_to_list(mock_resolve_include) -> None: """When packages: is an IncludeFile that resolves to a list, it is processed correctly.""" include_file = MagicMock(spec=IncludeFile) package_content = {CONF_WIFI: {CONF_SSID: TEST_PACKAGE_WIFI_SSID}} - mock_resolve_include.return_value = ([package_content], None) + mock_resolve_include.return_value = [package_content] config = {CONF_PACKAGES: include_file} result = do_packages_pass(config) @@ -1127,7 +1127,7 @@ def test_packages_include_file_resolves_to_dict(mock_resolve_include) -> None: """When packages: is an IncludeFile that resolves to a dict, it is processed correctly.""" include_file = MagicMock(spec=IncludeFile) package_content = {CONF_WIFI: {CONF_SSID: TEST_PACKAGE_WIFI_SSID}} - mock_resolve_include.return_value = ({"network": package_content}, None) + mock_resolve_include.return_value = {"network": package_content} config = {CONF_PACKAGES: include_file} result = do_packages_pass(config) @@ -1142,7 +1142,7 @@ def test_packages_include_file_resolves_to_invalid_type_raises( ) -> None: """When packages: is an IncludeFile that resolves to an invalid type, cv.Invalid is raised.""" include_file = MagicMock(spec=IncludeFile) - mock_resolve_include.return_value = ("not_a_dict_or_list", None) + mock_resolve_include.return_value = "not_a_dict_or_list" config = {CONF_PACKAGES: include_file} with pytest.raises( @@ -1215,7 +1215,9 @@ def test_named_dict_with_include_files_no_false_deprecation_warning( call_count = 0 - def failing_callback(package_config: dict, context: object) -> dict: + def failing_callback( + package_config: dict, context: object, path: DocumentPath | None = None + ) -> dict: nonlocal call_count call_count += 1 if call_count == 1: @@ -1251,7 +1253,9 @@ def test_validate_deprecated_false_raises_directly( call_count = 0 - def failing_callback(package_config: dict, context: object) -> dict: + def failing_callback( + package_config: dict, context: object, path: DocumentPath | None = None + ) -> dict: nonlocal call_count call_count += 1 if call_count == 1: @@ -1283,7 +1287,9 @@ def test_error_on_first_declared_package_still_detected() -> None: call_count = 0 - def fail_on_last(package_config: dict, context: object) -> dict: + def fail_on_last( + package_config: dict, context: object, path: DocumentPath | None = None + ) -> dict: nonlocal call_count call_count += 1 # Reverse iteration: third_pkg (1), second_pkg (2), first_pkg (3) @@ -1312,7 +1318,9 @@ def test_deprecated_single_package_fallback_still_works( attempt = 0 - def fail_then_succeed(package_config: dict, context: object) -> dict: + def fail_then_succeed( + package_config: dict, context: object, path: DocumentPath | None = None + ) -> dict: nonlocal attempt attempt += 1 if attempt == 1: diff --git a/tests/unit_tests/test_substitutions.py b/tests/unit_tests/test_substitutions.py index 3599e703d9..215ec291f9 100644 --- a/tests/unit_tests/test_substitutions.py +++ b/tests/unit_tests/test_substitutions.py @@ -659,7 +659,7 @@ def test_resolve_package_max_depth_exceeded(tmp_path: Path) -> None: cv.Invalid, match=f"Maximum include nesting depth \\({MAX_INCLUDE_DEPTH}\\) exceeded", ): - processor.resolve_package(package_config, substitutions.ContextVars()) + processor.resolve_package(package_config, substitutions.ContextVars(), []) def test_include_filename_substitution_undefined_var(tmp_path: Path) -> None: @@ -690,7 +690,7 @@ def test_raise_first_undefined_logs_extras_at_debug( caplog.at_level(logging.DEBUG, logger="esphome.components.substitutions"), pytest.raises(cv.Invalid) as exc_info, ): - substitutions.raise_first_undefined(errors, None, "package definition") + substitutions.raise_first_undefined(errors, "package definition") # First error is surfaced as the cv.Invalid message. raised = str(exc_info.value) @@ -706,7 +706,7 @@ def test_raise_first_undefined_logs_extras_at_debug( def test_raise_first_undefined_noop_on_empty() -> None: """An empty errors list is a no-op — no exception, no log.""" - substitutions.raise_first_undefined([], None, "package definition") + substitutions.raise_first_undefined([], "package definition") def test_do_substitution_pass_included_substitutions_must_be_mapping( @@ -778,4 +778,43 @@ def test_resolve_package_undefined_var_in_include_filename(tmp_path: Path) -> No ) processor = _PackageProcessor({}, None, False) with pytest.raises(cv.Invalid, match="unresolved substitutions"): - processor.resolve_package(package_config, substitutions.ContextVars()) + processor.resolve_package(package_config, substitutions.ContextVars(), []) + + +def test_resolve_include_error_shows_expanded_from_when_substituted( + tmp_path: Path, +) -> None: + """When a substituted filename fails to load, the error includes '(expanded from ...)'.""" + parent = tmp_path / "main.yaml" + parent.write_text("") + + def failing_loader(_path: Path) -> None: + raise EsphomeError("File not found") + + include = yaml_util.IncludeFile(parent, "${device}.yaml", None, failing_loader) + context = substitutions.ContextVars({"device": "my_device"}) + + with pytest.raises(cv.Invalid) as exc_info: + substitutions.resolve_include(include, [], context) + + msg = str(exc_info.value) + assert "my_device.yaml" in msg + assert "expanded from '${device}.yaml'" in msg + + +def test_resolve_include_error_no_expanded_from_for_literal_filename( + tmp_path: Path, +) -> None: + """When a literal filename fails to load, the error has no 'expanded from' clause.""" + parent = tmp_path / "main.yaml" + parent.write_text("") + + def failing_loader(_path: Path) -> None: + raise EsphomeError("File not found") + + include = yaml_util.IncludeFile(parent, "literal.yaml", None, failing_loader) + + with pytest.raises(cv.Invalid) as exc_info: + substitutions.resolve_include(include, [], substitutions.ContextVars()) + + assert "expanded from" not in str(exc_info.value) diff --git a/tests/unit_tests/test_yaml_util.py b/tests/unit_tests/test_yaml_util.py index bfd60de44d..e3aa2a16f5 100644 --- a/tests/unit_tests/test_yaml_util.py +++ b/tests/unit_tests/test_yaml_util.py @@ -9,8 +9,9 @@ from esphome import core, yaml_util from esphome.components import substitutions from esphome.config_helpers import Extend, Remove import esphome.config_validation as cv -from esphome.core import EsphomeError +from esphome.core import DocumentLocation, DocumentRange, EsphomeError from esphome.util import OrderedDict +from esphome.yaml_util import ESPHomeDataBase, format_path, make_data_base @pytest.fixture(autouse=True) @@ -712,3 +713,181 @@ def test_yaml_merge_chain_include_depth_exceeded() -> None: yaml_text = "base:\n <<: !include loop.yaml\n" with pytest.raises(EsphomeError, match="Maximum include chain depth"): yaml_util.parse_yaml(parent, io.StringIO(yaml_text), self_referencing_loader) + + +def _located(value, doc: str, line: int, col: int): + """Return *value* wrapped with a fake ESPHomeDataBase source location.""" + loc = DocumentLocation(doc, line, col) + obj = make_data_base(value) + if isinstance(obj, ESPHomeDataBase): + obj._esp_range = DocumentRange(loc, loc) + return obj + + +def test_format_path_no_location_info_returns_flat_path(): + """Plain path items with no esp_range produce a simple flat 'In:' line.""" + result = format_path(["wifi", "ssid"], None) + assert result == "In: wifi->ssid" + + +def test_format_path_no_location_info_current_obj_adds_file(): + """When path has no location but current_obj does, its location is shown.""" + obj = _located("${var}", "main.yaml", 5, 10) + result = format_path(["wifi", "ssid"], obj) + assert result == "In: wifi->ssid in main.yaml 6:11" + + +def test_format_path_single_frame_no_include_boundary(): + """All located keys from the same document → single 'In:' line, no 'Included from'.""" + path = ["packages", _located("pkg1", "root.yaml", 5, 2)] + result = format_path(path, None) + assert result.startswith("In: packages->pkg1 in root.yaml 6:3") + assert "Included from" not in result + + +def test_format_path_two_frames_shows_included_from(): + """Keys from two different documents produce 'In:' + one 'Included from' line.""" + path = [ + "packages", + _located("device", "root.yaml", 10, 2), + "packages", + _located("inner", "hardware.yaml", 3, 2), + ] + result = format_path(path, None) + assert "In: packages->inner in hardware.yaml 4:3" in result + assert "Included from packages->device in root.yaml 11:3" in result + + +def test_format_path_three_frames_full_include_stack(): + """Three document levels produce two 'Included from' lines in correct order.""" + path = [ + "packages", + _located("device", "root.yaml", 10, 2), + "packages", + _located("_wifi_", "hardware.yaml", 43, 2), + "packages", + _located("_roam_", "wifi.yaml", 25, 2), + ] + result = format_path(path, None) + lines = result.splitlines() + assert lines[0].startswith("In: packages->_roam_ in wifi.yaml") + assert lines[1].startswith(" Included from packages->_wifi_ in hardware.yaml") + assert lines[2].startswith(" Included from packages->device in root.yaml") + + +def test_format_path_current_obj_overrides_innermost_location(): + """current_obj's esp_range replaces the key's column for the 'In:' line.""" + path = ["packages", _located("pkg1", "root.yaml", 5, 2)] + # Value (the expression) sits at column 10, not column 2 like the key + value = _located("${undefined}", "root.yaml", 5, 10) + result = format_path(path, value) + assert "6:11" in result + assert "6:3" not in result + + +def test_format_path_empty_path_with_no_location(): + """Empty path with no location info returns 'In: '.""" + result = format_path([], None) + assert result == "In: " + + +def test_format_path_integer_path_items_formatted_as_subscript(): + """Integer indices are rendered as [n] subscripts in the flat fallback.""" + result = format_path(["packages", 0], None) + assert result == "In: packages[0]" + + +def test_format_path_integer_list_index_attached_to_previous_frame(): + """A list index between two include boundaries attaches to the outer frame.""" + path = [ + "packages", + _located("packages", "main.yaml", 5, 0), + 0, + _located("packages", "level1.yaml", 2, 0), + 0, + _located("esphome", "level2.yaml", 0, 0), + _located("name", "level2.yaml", 1, 8), + ] + result = format_path(path, None) + lines = result.splitlines() + assert lines[0].startswith("In: esphome->name in level2.yaml") + assert "packages[0]" in lines[1] and "level1.yaml" in lines[1] + assert "packages[0]" in lines[2] and "main.yaml" in lines[2] + + +def test_format_path_trailing_unlocated_string_after_located_key(): + """Plain string keys after the last located key must still appear in output.""" + path = [_located("packages", "main.yaml", 5, 0), "sub", "key"] + result = format_path(path, None) + assert result == "In: packages->sub->key in main.yaml 6:1" + + +def test_format_path_trailing_unlocated_int_attaches_to_current_frame(): + """Trailing ints attach to the open frame's last key (subscript), strings + buffer until end-of-path and then flush behind.""" + path = [_located("packages", "main.yaml", 5, 0), 0, "sub"] + result = format_path(path, None) + # Int attaches to 'packages' as [0] subscript; trailing 'sub' is flushed + # at end and appears after. + assert result == "In: packages[0]->sub in main.yaml 6:1" + + +def test_format_path_only_trailing_unlocated_strings_are_preserved(): + """Trailing pending items must not be silently dropped after the last frame.""" + path = [ + _located("packages", "main.yaml", 5, 0), + _located("inner", "hardware.yaml", 3, 0), + "tail1", + "tail2", + ] + result = format_path(path, None) + lines = result.splitlines() + assert lines[0] == "In: inner->tail1->tail2 in hardware.yaml 4:1" + assert lines[1] == " Included from packages in main.yaml 6:1" + + +def test_format_path_leading_int_with_no_current_doc_goes_to_pending(): + """An int before any located key is buffered and shown in the first frame.""" + path = [0, _located("name", "main.yaml", 1, 0)] + result = format_path(path, None) + # Leading ints have no preceding name to subscript onto, so they render + # as bare [n] in the formatted segment. + assert result == "In: [0]->name in main.yaml 2:1" + + +def test_format_path_only_unlocated_int_returns_flat_fallback(): + """Path with only an int and no location info renders via the flat fallback.""" + result = format_path([0], None) + assert result == "In: [0]" + + +def test_format_path_current_obj_in_different_doc_than_innermost_frame(): + """current_obj's location is preferred even when its document differs from the frame's.""" + path = [_located("packages", "root.yaml", 1, 0)] + value = _located("${var}", "other.yaml", 9, 4) + result = format_path(path, value) + # Innermost line uses current_obj's mark (other.yaml 10:5), not the key's. + assert result == "In: packages in other.yaml 10:5" + + +def test_format_path_current_obj_without_location_falls_back_to_key(): + """An ESPHomeDataBase current_obj with no esp_range falls back to the key's location.""" + + class _NoRange(ESPHomeDataBase, str): + pass + + obj = _NoRange.__new__(_NoRange, "value") + str.__init__(obj) + # No _esp_range set on this instance. + assert obj.esp_range is None + + path = [_located("packages", "main.yaml", 5, 2)] + result = format_path(path, obj) + assert result == "In: packages in main.yaml 6:3" + + +def test_format_path_empty_path_with_located_current_obj(): + """An empty path with a located current_obj still surfaces the location.""" + obj = _located("${var}", "main.yaml", 0, 0) + result = format_path([], obj) + assert result == "In: in main.yaml 1:1" From da44d43981cff7b79765652dbcfdc39fd481935a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 22 Apr 2026 05:07:48 +0200 Subject: [PATCH 07/15] Update pyparsing requirement from >=3.0 to >=3.3.2 (#15910) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 95d7c8c032..482ea92da7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -27,7 +27,7 @@ smpclient==6.0.0 requests==2.33.1 # esp-idf >= 5.0 requires this -pyparsing >= 3.0 +pyparsing >= 3.3.2 # For autocompletion argcomplete>=2.0.0 From 78f1467be46956a0a5f621a04f0ad0967cf92f44 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 22 Apr 2026 03:08:42 +0000 Subject: [PATCH 08/15] Bump aioesphomeapi from 44.17.0 to 44.18.0 (#15912) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 482ea92da7..b49777beaa 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,7 +12,7 @@ platformio==6.1.19 esptool==5.2.0 click==8.3.2 esphome-dashboard==20260408.1 -aioesphomeapi==44.17.0 +aioesphomeapi==44.18.0 zeroconf==0.148.0 puremagic==1.30 ruamel.yaml==0.19.1 # dashboard_import From bb81c91d0c9ed21b367fba7cabbb05ef5b3bab26 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 22 Apr 2026 03:08:58 +0000 Subject: [PATCH 09/15] Update tzdata requirement from >=2021.1 to >=2026.1 (#15911) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index b49777beaa..90f06eff98 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,7 +6,7 @@ colorama==0.4.6 icmplib==3.0.4 tornado==6.5.5 tzlocal==5.3.1 # from time -tzdata>=2021.1 # from time +tzdata>=2026.1 # from time pyserial==3.5 platformio==6.1.19 esptool==5.2.0 From edcf96d0575232c153e8d4d7bb6de2aab0410c9a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Apr 2026 06:24:09 +0200 Subject: [PATCH 10/15] [wifi] Use queue abstraction for LibreTiny WiFi events (#15343) --- esphome/components/libretiny/__init__.py | 7 ++ .../libretiny/freertos_static_alloc.c | 52 ++++++++++ esphome/components/wifi/wifi_component.cpp | 11 ++- esphome/components/wifi/wifi_component.h | 20 +++- .../wifi/wifi_component_esp8266.cpp | 5 +- .../wifi/wifi_component_esp_idf.cpp | 22 +++-- .../wifi/wifi_component_libretiny.cpp | 61 ++++-------- .../components/wifi/wifi_component_pico_w.cpp | 3 +- esphome/core/freertos_queue.h | 99 +++++++++++++++++++ 9 files changed, 227 insertions(+), 53 deletions(-) create mode 100644 esphome/components/libretiny/freertos_static_alloc.c create mode 100644 esphome/core/freertos_queue.h diff --git a/esphome/components/libretiny/__init__.py b/esphome/components/libretiny/__init__.py index 4f42f40478..40b8c8dc6c 100644 --- a/esphome/components/libretiny/__init__.py +++ b/esphome/components/libretiny/__init__.py @@ -443,6 +443,13 @@ async def component_to_code(config): # 4-8KB flash). Even if linked, it would use locks, so explicit FreeRTOS # mutexes are simpler and equivalent. cg.add_define(ThreadModel.MULTI_NO_ATOMICS) + # Enable FreeRTOS static allocation so FreeRTOSQueue can use + # xQueueCreateStatic (queue storage in BSS, no heap allocation). + # Also moves FreeRTOS internal structures (timer command queue) to BSS. + # BK72xx's FreeRTOSConfig.h doesn't define this, defaulting to 0. + # The -D wins over the #ifndef default in FreeRTOS.h. + # Not enabled on RTL87xx/LN882x — costs more heap than it saves there. + cg.add_build_flag("-DconfigSUPPORT_STATIC_ALLOCATION=1") # RTL8710B needs FreeRTOS 8.2.3+ for xTaskNotifyGive/ulTaskNotifyTake # required by AsyncTCP 3.4.3+ (https://github.com/esphome/esphome/issues/10220) diff --git a/esphome/components/libretiny/freertos_static_alloc.c b/esphome/components/libretiny/freertos_static_alloc.c new file mode 100644 index 0000000000..62b0524230 --- /dev/null +++ b/esphome/components/libretiny/freertos_static_alloc.c @@ -0,0 +1,52 @@ +/* + * FreeRTOS static allocation callbacks for LibreTiny platforms. + * + * Required when configSUPPORT_STATIC_ALLOCATION is enabled. These callbacks + * provide memory for the idle and timer tasks. Following ESP-IDF's approach, + * we allocate from the FreeRTOS heap (pvPortMalloc) rather than using truly + * static buffers, to avoid assumptions about memory layout. + * + * This enables xQueueCreateStatic, xTaskCreateStatic, etc. throughout ESPHome, + * allowing queue storage to live in BSS with zero runtime heap allocation. + */ + +#ifdef USE_BK72XX + +#include +#include + +#if (configSUPPORT_STATIC_ALLOCATION == 1) + +void vApplicationGetIdleTaskMemory(StaticTask_t **ppxIdleTaskTCBBuffer, StackType_t **ppxIdleTaskStackBuffer, + uint32_t *pulIdleTaskStackSize) { + /* Stack grows down on ARM — allocate stack first, then TCB, + * so the stack does not grow into the TCB. */ + StackType_t *stack = (StackType_t *) pvPortMalloc(configMINIMAL_STACK_SIZE * sizeof(StackType_t)); + StaticTask_t *tcb = (StaticTask_t *) pvPortMalloc(sizeof(StaticTask_t)); + configASSERT(stack != NULL); + configASSERT(tcb != NULL); + + *ppxIdleTaskTCBBuffer = tcb; + *ppxIdleTaskStackBuffer = stack; + *pulIdleTaskStackSize = configMINIMAL_STACK_SIZE; +} + +#if (configUSE_TIMERS == 1) + +void vApplicationGetTimerTaskMemory(StaticTask_t **ppxTimerTaskTCBBuffer, StackType_t **ppxTimerTaskStackBuffer, + uint32_t *pulTimerTaskStackSize) { + StackType_t *stack = (StackType_t *) pvPortMalloc(configTIMER_TASK_STACK_DEPTH * sizeof(StackType_t)); + StaticTask_t *tcb = (StaticTask_t *) pvPortMalloc(sizeof(StaticTask_t)); + configASSERT(stack != NULL); + configASSERT(tcb != NULL); + + *ppxTimerTaskTCBBuffer = tcb; + *ppxTimerTaskStackBuffer = stack; + *pulTimerTaskStackSize = configTIMER_TASK_STACK_DEPTH; +} + +#endif /* configUSE_TIMERS */ + +#endif /* configSUPPORT_STATIC_ALLOCATION */ + +#endif /* USE_BK72XX */ diff --git a/esphome/components/wifi/wifi_component.cpp b/esphome/components/wifi/wifi_component.cpp index 598aee8f66..481846085c 100644 --- a/esphome/components/wifi/wifi_component.cpp +++ b/esphome/components/wifi/wifi_component.cpp @@ -732,9 +732,16 @@ void WiFiComponent::restart_adapter() { } void WiFiComponent::loop() { - this->wifi_loop_(); + bool events_processed = this->wifi_loop_(); const uint32_t now = App.get_loop_component_start_time(); - this->update_connected_state_(); + // Connection state can only change when events are processed (ESP-IDF/LibreTiny) + // or polled (ESP8266/Pico W). Skip the expensive wifi_sta_connect_status_() call + // when no events arrived and we're already in steady state. + // Must also run when connected_ is false — after state transitions to STA_CONNECTED, + // connected_ won't be set until update_connected_state_() runs. + if (events_processed || !this->connected_) { + this->update_connected_state_(); + } if (this->has_sta()) { #if defined(USE_WIFI_CONNECT_TRIGGER) || defined(USE_WIFI_DISCONNECT_TRIGGER) diff --git a/esphome/components/wifi/wifi_component.h b/esphome/components/wifi/wifi_component.h index 665dec37d5..53fb0728fb 100644 --- a/esphome/components/wifi/wifi_component.h +++ b/esphome/components/wifi/wifi_component.h @@ -9,6 +9,11 @@ #ifdef USE_ESP32 #include "esphome/core/lock_free_queue.h" #endif +#if defined(USE_LIBRETINY) && defined(ESPHOME_THREAD_MULTI_ATOMICS) +#include "esphome/core/lock_free_queue.h" +#elif defined(USE_LIBRETINY) && defined(ESPHOME_THREAD_MULTI_NO_ATOMICS) +#include "esphome/core/freertos_queue.h" +#endif #include "esphome/core/string_ref.h" #include @@ -657,7 +662,7 @@ class WiFiComponent final : public Component { void connect_soon_(); - void wifi_loop_(); + bool wifi_loop_(); #ifdef USE_ESP8266 void process_pending_callbacks_(); #endif @@ -882,6 +887,19 @@ class WiFiComponent final : public Component { LockFreeQueue event_queue_; #endif +#ifdef USE_LIBRETINY + // Thread-safe queue for WiFi events from LibreTiny callback thread. + // LockFreeQueue on platforms with hardware atomics (RTL87xx, LN882x), + // FreeRTOSQueue on platforms without (BK72xx). + static constexpr uint8_t LT_EVENT_QUEUE_SIZE = 16; +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + // Ring buffer reserves one slot, so +1 for 16 usable slots + LockFreeQueue event_queue_; +#else + FreeRTOSQueue event_queue_; +#endif +#endif + private: // Stores a pointer to a string literal (static storage duration). // ONLY set from Python-generated code with string literals - never dynamic strings. diff --git a/esphome/components/wifi/wifi_component_esp8266.cpp b/esphome/components/wifi/wifi_component_esp8266.cpp index cb53d3ac1b..e56a8df350 100644 --- a/esphome/components/wifi/wifi_component_esp8266.cpp +++ b/esphome/components/wifi/wifi_component_esp8266.cpp @@ -938,7 +938,10 @@ network::IPAddress WiFiComponent::wifi_gateway_ip_() { return network::IPAddress(&ip.gw); } network::IPAddress WiFiComponent::wifi_dns_ip_(int num) { return network::IPAddress(dns_getserver(num)); } -void WiFiComponent::wifi_loop_() { this->process_pending_callbacks_(); } +bool WiFiComponent::wifi_loop_() { + this->process_pending_callbacks_(); + return true; +} void WiFiComponent::process_pending_callbacks_() { // Process callbacks deferred from ESP8266 SDK system context (~2KB stack) diff --git a/esphome/components/wifi/wifi_component_esp_idf.cpp b/esphome/components/wifi/wifi_component_esp_idf.cpp index 4097df80af..c790742c79 100644 --- a/esphome/components/wifi/wifi_component_esp_idf.cpp +++ b/esphome/components/wifi/wifi_component_esp_idf.cpp @@ -715,17 +715,25 @@ const char *get_disconnect_reason_str(uint8_t reason) { } } -void WiFiComponent::wifi_loop_() { +bool WiFiComponent::wifi_loop_() { + // Use pop() directly instead of empty() — pop() costs 1 memw (acquire on tail_), + // while empty() costs 2 memw (acquire on both head_ and tail_) on Xtensa. + IDFWiFiEvent *data = this->event_queue_.pop(); + if (data == nullptr) + return false; + + do { + wifi_process_event_(data); + delete data; // NOLINT(cppcoreguidelines-owning-memory) + } while ((data = this->event_queue_.pop()) != nullptr); + + // Drops only occur when the queue is full, and only this loop drains it, + // so if pop() returned nullptr above we can skip this check. uint16_t dropped = this->event_queue_.get_and_reset_dropped_count(); if (dropped > 0) { ESP_LOGW(TAG, "Dropped %u WiFi events due to buffer overflow", dropped); } - - IDFWiFiEvent *data; - while ((data = this->event_queue_.pop()) != nullptr) { - wifi_process_event_(data); - delete data; // NOLINT(cppcoreguidelines-owning-memory) - } + return true; } // Events are processed from queue in main loop context, but listener notifications // must be deferred until after the state machine transitions (in check_connecting_finished) diff --git a/esphome/components/wifi/wifi_component_libretiny.cpp b/esphome/components/wifi/wifi_component_libretiny.cpp index 9565ffa747..cdd11ceaef 100644 --- a/esphome/components/wifi/wifi_component_libretiny.cpp +++ b/esphome/components/wifi/wifi_component_libretiny.cpp @@ -10,9 +10,6 @@ #include "lwip/err.h" #include "lwip/dns.h" -#include -#include - #ifdef USE_BK72XX extern "C" { #include @@ -43,16 +40,13 @@ static const char *const TAG = "wifi_lt"; // (like connection status flags) from the callback causes race conditions: // - The main loop may never see state changes (values cached in registers) // - State changes may be visible in inconsistent order -// - LibreTiny targets (BK7231, RTL8720) lack atomic instructions (no LDREX/STREX) // // Solution: Queue events in the callback and process them in the main loop. // This is the same approach used by ESP32 IDF's wifi_process_event_(). // All state modifications happen in the main loop context, eliminating races. - -static constexpr size_t EVENT_QUEUE_SIZE = 16; // Max pending WiFi events before overflow -static QueueHandle_t s_event_queue = nullptr; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -static volatile uint32_t s_event_queue_overflow_count = - 0; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) +// +// On platforms with hardware atomics (RTL87xx, LN882x): LockFreeQueue (SPSC ring buffer) +// On platforms without (BK72xx): FreeRTOSQueue (xQueue wrapper with critical sections) // Event structure for queued WiFi events - contains a copy of event data // to avoid lifetime issues with the original event data from the callback @@ -352,10 +346,6 @@ using esphome_wifi_event_info_t = arduino_event_info_t; // Event callback - runs in WiFi driver thread context // Only queues events for processing in main loop, no logging or state changes here void WiFiComponent::wifi_event_callback_(esphome_wifi_event_id_t event, esphome_wifi_event_info_t info) { - if (s_event_queue == nullptr) { - return; - } - // Allocate on heap and fill directly to avoid extra memcpy auto *to_send = new LTWiFiEvent{}; // NOLINT(cppcoreguidelines-owning-memory) to_send->event_id = event; @@ -428,9 +418,8 @@ void WiFiComponent::wifi_event_callback_(esphome_wifi_event_id_t event, esphome_ } // Queue event (don't block if queue is full) - if (xQueueSend(s_event_queue, &to_send, 0) != pdPASS) { + if (!this->event_queue_.push(to_send)) { delete to_send; // NOLINT(cppcoreguidelines-owning-memory) - s_event_queue_overflow_count++; } } @@ -620,14 +609,6 @@ void WiFiComponent::wifi_process_event_(LTWiFiEvent *event) { } } void WiFiComponent::wifi_pre_setup_() { - // Create event queue for thread-safe event handling - // Events are pushed from WiFi callback thread and processed in main loop - s_event_queue = xQueueCreate(EVENT_QUEUE_SIZE, sizeof(LTWiFiEvent *)); - if (s_event_queue == nullptr) { - ESP_LOGE(TAG, "Failed to create event queue"); - return; - } - WiFi.onEvent( [this](arduino_event_id_t event, arduino_event_info_t info) { this->wifi_event_callback_(event, info); }); // Make sure WiFi is in clean state before anything starts @@ -796,28 +777,26 @@ int32_t WiFiComponent::get_wifi_channel() { return WiFi.channel(); } network::IPAddress WiFiComponent::wifi_subnet_mask_() { return {WiFi.subnetMask()}; } network::IPAddress WiFiComponent::wifi_gateway_ip_() { return {WiFi.gatewayIP()}; } network::IPAddress WiFiComponent::wifi_dns_ip_(int num) { return {WiFi.dnsIP(num)}; } -void WiFiComponent::wifi_loop_() { - // Process all pending events from the queue - if (s_event_queue == nullptr) { - return; - } - - // Check for dropped events due to queue overflow - if (s_event_queue_overflow_count > 0) { - ESP_LOGW(TAG, "Event queue overflow, %" PRIu32 " events dropped", s_event_queue_overflow_count); - s_event_queue_overflow_count = 0; - } - - while (true) { - LTWiFiEvent *event; - if (xQueueReceive(s_event_queue, &event, 0) != pdTRUE) { - // No more events - break; - } +bool WiFiComponent::wifi_loop_() { + // Use pop() directly instead of empty() — avoids redundant synchronization. + // LockFreeQueue: pop() costs 1 memw vs empty()'s 2 memw on Xtensa. + // FreeRTOSQueue: pop() is 1 critical section vs empty() + pop() = 2. + LTWiFiEvent *event = this->event_queue_.pop(); + if (event == nullptr) + return false; + do { wifi_process_event_(event); delete event; // NOLINT(cppcoreguidelines-owning-memory) + } while ((event = this->event_queue_.pop()) != nullptr); + + // Drops only occur when the queue is full, and only this loop drains it, + // so if pop() returned nullptr above we can skip this check. + uint16_t dropped = this->event_queue_.get_and_reset_dropped_count(); + if (dropped > 0) { + ESP_LOGW(TAG, "Dropped %" PRIu16 " WiFi events due to buffer overflow", dropped); } + return true; } } // namespace esphome::wifi diff --git a/esphome/components/wifi/wifi_component_pico_w.cpp b/esphome/components/wifi/wifi_component_pico_w.cpp index 1cfeee3c1b..4e1e0395c0 100644 --- a/esphome/components/wifi/wifi_component_pico_w.cpp +++ b/esphome/components/wifi/wifi_component_pico_w.cpp @@ -303,7 +303,7 @@ network::IPAddress WiFiComponent::wifi_dns_ip_(int num) { // Connect state listener notifications are deferred until after the state machine // transitions (in check_connecting_finished) so that conditions like wifi.connected // return correct values in automations. -void WiFiComponent::wifi_loop_() { +bool WiFiComponent::wifi_loop_() { // Handle scan completion if (this->state_ == WIFI_COMPONENT_STATE_STA_SCANNING && !cyw43_wifi_scan_active(&cyw43_state)) { this->scan_done_ = true; @@ -365,6 +365,7 @@ void WiFiComponent::wifi_loop_() { #endif } } + return true; } void WiFiComponent::wifi_pre_setup_() {} diff --git a/esphome/core/freertos_queue.h b/esphome/core/freertos_queue.h new file mode 100644 index 0000000000..2f3faf818a --- /dev/null +++ b/esphome/core/freertos_queue.h @@ -0,0 +1,99 @@ +#pragma once + +#include "esphome/core/defines.h" + +#ifdef ESPHOME_THREAD_MULTI_NO_ATOMICS + +#include +#include + +#include +#include + +/* + * FreeRTOS queue wrapper for single-producer single-consumer scenarios on + * platforms without hardware atomic support (e.g. BK72xx ARM968E-S). + * + * Provides the same API as LockFreeQueue (push, pop, get_and_reset_dropped_count, + * empty, full, size) but uses xQueue internally, which synchronizes via + * FreeRTOS critical sections. Uses xQueueCreateStatic so the queue storage + * lives in BSS with zero runtime heap allocation. + * + * @tparam T The type of elements stored in the queue (stored as pointers) + * @tparam SIZE The maximum number of elements + */ + +namespace esphome { + +template class FreeRTOSQueue { + public: + FreeRTOSQueue() : dropped_count_(0) { + this->handle_ = xQueueCreateStatic(SIZE, sizeof(T *), this->storage_, &this->queue_buf_); + } + + // No destructor — ESPHome components are never destroyed. Intentionally + // omitted to avoid pulling in vQueueDelete code on resource-constrained targets. + + // Non-copyable, non-movable — queue handle is not transferable + FreeRTOSQueue(const FreeRTOSQueue &) = delete; + FreeRTOSQueue &operator=(const FreeRTOSQueue &) = delete; + FreeRTOSQueue(FreeRTOSQueue &&) = delete; + FreeRTOSQueue &operator=(FreeRTOSQueue &&) = delete; + + bool push(T *element) { + if (element == nullptr) + return false; + + if (xQueueSend(this->handle_, &element, 0) != pdPASS) { + this->increment_dropped_count(); + return false; + } + return true; + } + + T *pop() { + T *element; + if (xQueueReceive(this->handle_, &element, 0) != pdTRUE) { + return nullptr; + } + return element; + } + + uint16_t get_and_reset_dropped_count() { + // Fast path: plain read of aligned uint16_t is a single ARM load instruction. + // Worst case is reading a stale zero and reporting drops one iteration later. + // Avoids critical section overhead on every loop() call since drops are rare. + if (this->dropped_count_ == 0) + return 0; + // Declare outside critical section — BK72xx portENTER_CRITICAL may introduce a scope + uint16_t count; + portENTER_CRITICAL(); + count = this->dropped_count_; + this->dropped_count_ = 0; + portEXIT_CRITICAL(); + return count; + } + + void increment_dropped_count() { + portENTER_CRITICAL(); + this->dropped_count_++; + portEXIT_CRITICAL(); + } + + bool empty() const { return uxQueueMessagesWaiting(this->handle_) == 0; } + + bool full() const { return uxQueueSpacesAvailable(this->handle_) == 0; } + + size_t size() const { return uxQueueMessagesWaiting(this->handle_); } + + protected: + // Static storage for the queue — lives in BSS, no heap allocation + uint8_t storage_[SIZE * sizeof(T *)]; + StaticQueue_t queue_buf_; + QueueHandle_t handle_; + uint16_t dropped_count_; +}; + +} // namespace esphome + +#endif // ESPHOME_THREAD_MULTI_NO_ATOMICS From 67576d4879e252d4b765cef4ca92c580377d68b5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Apr 2026 06:29:13 +0200 Subject: [PATCH 11/15] [rp2040] Tune oversized lwIP defaults for ESPHome (#14843) --- MANIFEST.in | 1 + esphome/components/rp2040/__init__.py | 162 +++++++++++++++++- esphome/components/rp2040/const.py | 1 + .../rp2040/inject_lwip_include.py.script | 18 ++ esphome/components/rp2040/lwipopts.h.jinja | 46 +++++ esphome/components/wifi/__init__.py | 10 +- script/stress_test_connect.py | 84 +++++++++ 7 files changed, 316 insertions(+), 6 deletions(-) create mode 100644 esphome/components/rp2040/inject_lwip_include.py.script create mode 100644 esphome/components/rp2040/lwipopts.h.jinja create mode 100644 script/stress_test_connect.py diff --git a/MANIFEST.in b/MANIFEST.in index ed65edc656..e426627e8d 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -4,4 +4,5 @@ include requirements.txt recursive-include esphome *.yaml recursive-include esphome *.cpp *.h *.tcc *.c recursive-include esphome *.py.script +recursive-include esphome *.jinja recursive-include esphome LICENSE.txt diff --git a/esphome/components/rp2040/__init__.py b/esphome/components/rp2040/__init__.py index e452780d41..ed246416c9 100644 --- a/esphome/components/rp2040/__init__.py +++ b/esphome/components/rp2040/__init__.py @@ -26,7 +26,7 @@ from esphome.core.config import BOARD_MAX_LENGTH from esphome.helpers import copy_file_if_changed, read_file, write_file_if_changed from . import boards -from .const import KEY_BOARD, KEY_PIO_FILES, KEY_RP2040, rp2040_ns +from .const import KEY_BOARD, KEY_LWIP_OPTS, KEY_PIO_FILES, KEY_RP2040, rp2040_ns # force import gpio to register pin schema from .gpio import rp2040_pin_to_code # noqa @@ -240,6 +240,160 @@ async def to_code(config): cg.add_define("USE_RP2040_WATCHDOG_TIMEOUT", config[CONF_WATCHDOG_TIMEOUT]) cg.add_define("USE_RP2040_CRASH_HANDLER") + _configure_lwip() + + +def _configure_lwip() -> None: + """Configure lwIP options for RP2040 by generating a custom lwipopts.h. + + Arduino-pico's lwipopts.h has no #ifndef guards, so -D flags cannot override + its settings. Instead, we generate a replacement lwipopts.h and place it in an + include directory that shadows the framework's version. + + lwIP is compiled from source on RP2040 (not pre-built), so our replacement + header fully controls the compiled lwIP behavior. + + RP2040 uses NO_SYS=1 (polling, no RTOS thread), LWIP_SOCKET=0, LWIP_NETCONN=0. + DHCP/DNS use raw udp_new() which allocates from MEMP_NUM_UDP_PCB. + + Comparison of arduino-pico defaults vs ESPHome targets (TCP_MSS=1460): + + Setting ESP8266 ESP32 arduino-pico New + ──────────────────────────────────────────────────────────────── + TCP_SND_BUF 2×MSS 4×MSS 8×MSS 4×MSS + TCP_WND 4×MSS 4×MSS 8×MSS 4×MSS + MEM_LIBC_MALLOC 1 1 0 0* + MEMP_MEM_MALLOC 1 1 0 0** + MEM_SIZE N/A*** N/A*** 16KB 16KB + PBUF_POOL_SIZE 10 16 24 16 + MEMP_NUM_TCP_SEG 10 16 32 17 + MEMP_NUM_TCP_PCB 5 16 5 dynamic + MEMP_NUM_TCP_PCB_LISTEN 4 16 8**** dynamic + MEMP_NUM_UDP_PCB 4 16 7 dynamic + TCP_SND_QUEUELEN ~8 17 32 17 + + * MEM_LIBC_MALLOC must stay 0: arduino-pico uses + PICO_CYW43_ARCH_THREADSAFE_BACKGROUND which runs lwIP callbacks from + a low-priority pendsv IRQ. The pico-sdk explicitly blocks + MEM_LIBC_MALLOC=1 because libc malloc uses mutexes (unsafe in IRQ). + ** MEMP_MEM_MALLOC must stay 0: the dedicated lwIP heap (MEM_SIZE=16KB) + is too small to hold all pools dynamically. The PBUF_POOL alone needs + ~24KB (16 × 1524 bytes). Increasing MEM_SIZE would negate BSS savings. + *** ESP8266/ESP32 use MEM_LIBC_MALLOC=1 (system heap, no dedicated pool). + **** opt.h default; arduino-pico doesn't override MEMP_NUM_TCP_PCB_LISTEN. + "dynamic" = auto-calculated from component socket registrations via + socket.get_socket_counts() with minimums of 8 TCP / 6 UDP / 2 TCP_LISTEN. + """ + from esphome.components.socket import ( + MIN_TCP_LISTEN_SOCKETS, + MIN_TCP_SOCKETS, + MIN_UDP_SOCKETS, + get_socket_counts, + ) + + sc = get_socket_counts() + # Apply platform minimums — ensure headroom for ESPHome's needs + tcp_sockets = max(MIN_TCP_SOCKETS, sc.tcp) + udp_sockets = max(MIN_UDP_SOCKETS, sc.udp) + # RP2040 has more RAM (264KB) than most LibreTiny boards, so DHCP/DNS + # UDP PCBs (2) are absorbed by the generous minimum of 6. + listening_tcp = max(MIN_TCP_LISTEN_SOCKETS, sc.tcp_listen) + + # TCP_SND_BUF: 4×MSS=5,840 matches ESP32. Down from arduino-pico's 8×MSS. + # ESPAsyncWebServer allocates malloc(tcp_sndbuf()) per response chunk. + tcp_snd_buf = "(4*TCP_MSS)" + + # TCP_WND: receive window. 4×MSS matches ESP32. Down from arduino-pico's 8×MSS. + tcp_wnd = "(4*TCP_MSS)" + + # TCP_SND_QUEUELEN: max pbufs queued for send buffer + # ESP-IDF formula: (4 * TCP_SND_BUF + (TCP_MSS - 1)) / TCP_MSS + # With 4×MSS: (4*5840 + 1459) / 1460 = 17 — match ESP32 + tcp_snd_queuelen = 17 + # MEMP_NUM_TCP_SEG: segment pool, must be >= TCP_SND_QUEUELEN (lwIP sanity check) + memp_num_tcp_seg = tcp_snd_queuelen + + # PBUF_POOL_SIZE: RP2040 has 264KB RAM, more generous than LibreTiny. + # 16 matches ESP32 (vs arduino-pico's 24). With MEMP_MEM_MALLOC=1, + # this is a max count (allocated on demand from heap). + pbuf_pool_size = 16 + + # Build the lwIP override defines for the Jinja2 template. + # The template uses #include_next to chain to the framework's original + # lwipopts.h, then #undef/#define only the values we need to change. + # + # Note: MEMP_MEM_MALLOC stays 0 (framework default). While the memp + # allocations use the dedicated lwIP heap (IRQ-safe), the 16KB MEM_SIZE + # is too small to hold all pools dynamically under stress. The PBUF_POOL + # alone needs ~24KB (16 × 1524 bytes). Increasing MEM_SIZE would negate + # the BSS savings. + # + # MEM_LIBC_MALLOC stays 0 (framework default): arduino-pico uses + # PICO_CYW43_ARCH_THREADSAFE_BACKGROUND which runs lwIP callbacks from + # a low-priority pendsv IRQ where libc malloc (mutex-based) is unsafe. + lwip_defines: dict[str, str] = { + "TCP_SND_BUF": tcp_snd_buf, + "TCP_WND": tcp_wnd, + "TCP_SND_QUEUELEN": str(tcp_snd_queuelen), + "MEMP_NUM_TCP_SEG": str(memp_num_tcp_seg), + "PBUF_POOL_SIZE": str(pbuf_pool_size), + "MEMP_NUM_TCP_PCB": str(tcp_sockets), + "MEMP_NUM_TCP_PCB_LISTEN": str(listening_tcp), + "MEMP_NUM_UDP_PCB": str(udp_sockets), + } + + # Store for copy_files() to generate the header + CORE.data[KEY_RP2040][KEY_LWIP_OPTS] = lwip_defines + + # Add a pre-build extra script that injects our lwip_override directory + # into CCFLAGS so our lwipopts.h shadows the framework's version. + # Regular build_flags (-I/-isystem) come after -iwithprefixbefore in GCC's + # search order, so we must prepend via an extra_scripts hook. + cg.add_platformio_option("extra_scripts", ["pre:inject_lwip_include.py"]) + + tcp_min = " (min)" if tcp_sockets > sc.tcp else "" + udp_min = " (min)" if udp_sockets > sc.udp else "" + listen_min = " (min)" if listening_tcp > sc.tcp_listen else "" + _LOGGER.info( + "Configuring lwIP: TCP=%d%s [%s], UDP=%d%s [%s], TCP_LISTEN=%d%s [%s]", + tcp_sockets, + tcp_min, + sc.tcp_details, + udp_sockets, + udp_min, + sc.udp_details, + listening_tcp, + listen_min, + sc.tcp_listen_details, + ) + + +def _generate_lwipopts_h() -> None: + """Generate a custom lwipopts.h that shadows the framework's version. + + Uses Jinja2 to render the template with the lwIP defines calculated + during code generation. The generated header is placed in lwip_override/ + in the build directory, and a pre-build script injects this directory + into the compiler include path before the framework's own include dir. + """ + from jinja2 import Environment, FileSystemLoader + + lwip_defines = CORE.data[KEY_RP2040].get(KEY_LWIP_OPTS) + if not lwip_defines: + return + + template_dir = Path(__file__).parent + jinja_env = Environment( + loader=FileSystemLoader(str(template_dir)), + keep_trailing_newline=True, + ) + template = jinja_env.get_template("lwipopts.h.jinja") + content = template.render(**lwip_defines) + + lwip_dir = CORE.relative_build_path("lwip_override") + lwip_dir.mkdir(parents=True, exist_ok=True) + write_file_if_changed(lwip_dir / "lwipopts.h", content) + def add_pio_file(component: str, key: str, data: str): try: @@ -289,6 +443,12 @@ def copy_files(): post_build_file, CORE.relative_build_path("post_build.py"), ) + inject_lwip_file = dir / "inject_lwip_include.py.script" + copy_file_if_changed( + inject_lwip_file, + CORE.relative_build_path("inject_lwip_include.py"), + ) + _generate_lwipopts_h() if generate_pio_files(): path = CORE.relative_src_path("esphome.h") content = read_file(path).rstrip("\n") diff --git a/esphome/components/rp2040/const.py b/esphome/components/rp2040/const.py index ab5f42d757..e381d0482d 100644 --- a/esphome/components/rp2040/const.py +++ b/esphome/components/rp2040/const.py @@ -1,6 +1,7 @@ import esphome.codegen as cg KEY_BOARD = "board" +KEY_LWIP_OPTS = "lwip_opts" KEY_RP2040 = "rp2040" KEY_PIO_FILES = "pio_files" diff --git a/esphome/components/rp2040/inject_lwip_include.py.script b/esphome/components/rp2040/inject_lwip_include.py.script new file mode 100644 index 0000000000..4ae9863e37 --- /dev/null +++ b/esphome/components/rp2040/inject_lwip_include.py.script @@ -0,0 +1,18 @@ +# pylint: disable=E0602 +Import("env") # noqa + +import os + +# PlatformIO pre-build script: inject lwip_override include path so our +# lwipopts.h shadows the framework's version during lwIP compilation. +# +# The arduino-pico builder uses -iprefix + -iwithprefixbefore for includes, +# which takes priority over CPPPATH (-I). We must inject our path into the +# CCFLAGS BEFORE the -iprefix flag to ensure our lwipopts.h is found first. + +lwip_dir = os.path.join(env["PROJECT_DIR"], "lwip_override") + +if os.path.isdir(lwip_dir): + # Insert -I at the beginning of CCFLAGS, before the framework's + # -iprefix/-iwithprefixbefore flags which would otherwise take priority. + env.Prepend(CCFLAGS=["-I", lwip_dir]) diff --git a/esphome/components/rp2040/lwipopts.h.jinja b/esphome/components/rp2040/lwipopts.h.jinja new file mode 100644 index 0000000000..36d7d4da14 --- /dev/null +++ b/esphome/components/rp2040/lwipopts.h.jinja @@ -0,0 +1,46 @@ +// ESPHome lwIP configuration override for RP2040. +// Includes the framework's original lwipopts.h, then overrides specific +// settings to tune lwIP for ESPHome's IoT use case. +// +// This file is found first via -I injection (see inject_lwip_include.py.script). +// #include_next chains to the framework's original in include/lwipopts.h. +// Since the original uses #pragma once, it won't be included again later +// (e.g. via tusb_config.h), avoiding duplicate definition warnings. + +// Include the framework's original lwipopts.h first +#include_next "lwipopts.h" + +// --- ESPHome overrides below --- +// Only #undef and redefine values that differ from the framework defaults. + +// TCP send/receive buffers: 4xMSS matches ESP32 (down from 8xMSS) +#undef TCP_SND_BUF +#define TCP_SND_BUF {{ TCP_SND_BUF }} + +#undef TCP_WND +#define TCP_WND {{ TCP_WND }} + +// Queued segment limits: derived from 4xMSS buffer size, matching ESP32 +#undef TCP_SND_QUEUELEN +#define TCP_SND_QUEUELEN {{ TCP_SND_QUEUELEN }} + +#undef MEMP_NUM_TCP_SEG +#define MEMP_NUM_TCP_SEG {{ MEMP_NUM_TCP_SEG }} + +// Packet buffer pool: 16 matches ESP32 (down from 24) +#undef PBUF_POOL_SIZE +#define PBUF_POOL_SIZE {{ PBUF_POOL_SIZE }} + +// PCB pools: sized to actual component needs via socket.get_socket_counts() +#undef MEMP_NUM_TCP_PCB +#define MEMP_NUM_TCP_PCB {{ MEMP_NUM_TCP_PCB }} + +#undef MEMP_NUM_TCP_PCB_LISTEN +#define MEMP_NUM_TCP_PCB_LISTEN {{ MEMP_NUM_TCP_PCB_LISTEN }} + +#undef MEMP_NUM_UDP_PCB +#define MEMP_NUM_UDP_PCB {{ MEMP_NUM_UDP_PCB }} + +// Listen backlog: match component needs +#undef TCP_DEFAULT_LISTEN_BACKLOG +#define TCP_DEFAULT_LISTEN_BACKLOG {{ MEMP_NUM_TCP_PCB_LISTEN }} diff --git a/esphome/components/wifi/__init__.py b/esphome/components/wifi/__init__.py index 33557f03c7..bc4e177219 100644 --- a/esphome/components/wifi/__init__.py +++ b/esphome/components/wifi/__init__.py @@ -289,12 +289,12 @@ def final_validate(config): def _consume_wifi_sockets(config: ConfigType) -> ConfigType: """Register UDP PCBs used internally by lwIP for DHCP and DNS. - Only needed on LibreTiny where we directly set MEMP_NUM_UDP_PCB (the raw - PCB pool shared by both application sockets and lwIP internals like DHCP/DNS). - On ESP32, CONFIG_LWIP_MAX_SOCKETS only controls the POSIX socket layer — - DHCP/DNS use raw udp_new() which bypasses it entirely. + Needed on LibreTiny and RP2040 where we directly set MEMP_NUM_UDP_PCB (the + raw PCB pool shared by both application sockets and lwIP internals like + DHCP/DNS). On ESP32, CONFIG_LWIP_MAX_SOCKETS only controls the POSIX socket + layer — DHCP/DNS use raw udp_new() which bypasses it entirely. """ - if not (CORE.is_bk72xx or CORE.is_rtl87xx or CORE.is_ln882x): + if not (CORE.is_bk72xx or CORE.is_rtl87xx or CORE.is_ln882x or CORE.is_rp2040): return config from esphome.components import socket diff --git a/script/stress_test_connect.py b/script/stress_test_connect.py new file mode 100644 index 0000000000..f91a7e8f99 --- /dev/null +++ b/script/stress_test_connect.py @@ -0,0 +1,84 @@ +"""Rapid connect/disconnect stress test for ESPHome native API.""" + +import asyncio +import sys +import time + +from aioesphomeapi import APIClient + +HOST = "192.168.1.100" +PORT = 6053 +PASSWORD = "" +NOISE_PSK = None +ITERATIONS = 500 +CONCURRENCY = 4 # simultaneous connection attempts + + +async def connect_disconnect(client_id: int, iteration: int) -> tuple[int, bool, str]: + """Connect and immediately disconnect.""" + cli = APIClient(HOST, PORT, PASSWORD, noise_psk=NOISE_PSK) + try: + await asyncio.wait_for(cli.connect(login=True), timeout=10) + await cli.disconnect() + return iteration, True, "" + except Exception as e: + return ( + iteration, + False, + f"client{client_id} iter{iteration}: {type(e).__name__}: {e}", + ) + finally: + await cli.disconnect(force=True) + + +async def main() -> None: + iterations = int(sys.argv[1]) if len(sys.argv) > 1 else ITERATIONS + concurrency = int(sys.argv[2]) if len(sys.argv) > 2 else CONCURRENCY + + print(f"Stress testing {HOST}:{PORT}") + print(f"Iterations: {iterations}, Concurrency: {concurrency}") + print() + + success = 0 + fail = 0 + errors: list[str] = [] + start = time.monotonic() + + sem = asyncio.Semaphore(concurrency) + + async def run(client_id: int, iteration: int) -> tuple[int, bool, str]: + async with sem: + return await connect_disconnect(client_id, iteration) + + tasks = [asyncio.create_task(run(i % concurrency, i)) for i in range(iterations)] + + for coro in asyncio.as_completed(tasks): + iteration, ok, err = await coro + if ok: + success += 1 + else: + fail += 1 + errors.append(err) + total = success + fail + if total % 10 == 0 or not ok: + elapsed = time.monotonic() - start + rate = total / elapsed if elapsed > 0 else 0 + print(f"[{total}/{iterations}] ok={success} fail={fail} ({rate:.1f}/s)") + if err: + print(f" ERROR: {err}") + + elapsed = time.monotonic() - start + print() + print(f"Done in {elapsed:.1f}s") + print(f"Success: {success}, Failed: {fail}, Rate: {iterations / elapsed:.1f}/s") + + if errors: + print("\nLast 10 errors:") + for e in errors[-10:]: + print(f" {e}") + + sys.exit(1 if fail > 0 else 0) + + +if __name__ == "__main__": + asyncio.run(main()) From 699cf9690ab32d374e63d8828440893bb62bc96a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Apr 2026 06:31:34 +0200 Subject: [PATCH 12/15] [core] Optimize value_accuracy_to_buf to avoid snprintf (#15596) --- esphome/core/helpers.cpp | 54 +++- esphome/core/helpers.h | 35 +++ tests/components/core/test_helpers.cpp | 96 +++++++ tests/components/core/test_value_accuracy.cpp | 237 ++++++++++++++++++ 4 files changed, 410 insertions(+), 12 deletions(-) create mode 100644 tests/components/core/test_value_accuracy.cpp diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index 113b6f6187..e71da95e6b 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -447,28 +447,58 @@ static inline void normalize_accuracy_decimals(float &value, int8_t &accuracy_de // value_accuracy_to_string moved to alloc_helpers.cpp +// Fast float-to-string for accuracy_decimals 0-3 (covers virtually all sensor usage). +// Avoids snprintf("%.*f") which pulls in heavy float formatting machinery. +// 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; + } + // 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). + // 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++ = '.'; + p = frac_to_str_unchecked(p, scaled % mult, mult / 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, 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)) { + 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/out-of-range 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) { - if (unit_of_measurement.empty()) { - return value_accuracy_to_buf(buf, value, accuracy_decimals); + size_t len = value_accuracy_to_buf(buf, value, accuracy_decimals); + if (len == 0 || unit_of_measurement.empty()) { + 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); + 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 939852bfcb..4a91c46074 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -1311,6 +1311,29 @@ 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. 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; + 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) { return n == 3 ? 1000 : n == 2 ? 100 : n == 1 ? 10 : 1; } + /// Minimum buffer size for uint32_to_str: 10 digits + null terminator. static constexpr size_t UINT32_MAX_STR_SIZE = 11; @@ -1326,6 +1349,18 @@ inline size_t uint32_to_str(std::span buf, uint32_t v return static_cast(end - buf.data()); } +/// 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_unchecked(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); diff --git a/tests/components/core/test_helpers.cpp b/tests/components/core/test_helpers.cpp index 00169621c3..5fb77ef753 100644 --- a/tests/components/core/test_helpers.cpp +++ b/tests/components/core/test_helpers.cpp @@ -117,4 +117,100 @@ TEST(FormatHexChar, UppercaseDigits) { EXPECT_EQ(format_hex_pretty_char(15), 'F'); } +// --- 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); } + +// --- frac_to_str_unchecked() --- + +TEST(FracToStr, OneDigit) { + char buf[8]; + char *end = frac_to_str_unchecked(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_unchecked(buf, 46, 10); + *end = '\0'; + EXPECT_STREQ(buf, "46"); +} + +TEST(FracToStr, ThreeDigits) { + char buf[8]; + char *end = frac_to_str_unchecked(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_unchecked(buf, 1, 100); + *end = '\0'; + EXPECT_STREQ(buf, "001"); + + 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_unchecked(buf, 0, 100); + *end = '\0'; + EXPECT_STREQ(buf, "000"); + + end = frac_to_str_unchecked(buf, 0, 1); + *end = '\0'; + EXPECT_STREQ(buf, "0"); +} + +TEST(FracToStr, ZeroDivisor) { + char buf[8]; + buf[0] = 'X'; + char *end = frac_to_str_unchecked(buf, 0, 0); + 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::core::testing diff --git a/tests/components/core/test_value_accuracy.cpp b/tests/components/core/test_value_accuracy.cpp new file mode 100644 index 0000000000..381a742a9c --- /dev/null +++ b/tests/components/core/test_value_accuracy.cpp @@ -0,0 +1,237 @@ +#include +#include +#include +#include +#include +#include + +#include "esphome/core/helpers.h" +#include "esphome/core/string_ref.h" + +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) { + 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); +} + +TEST(ValueAccuracyToBuf, NegativeZero) { + // Hand-rolled formatter must preserve snprintf's sign-of-zero behavior. + EXPECT_EQ(va_to_string(-0.0f, 2), va_reference(-0.0f, 2)); + EXPECT_EQ(va_to_string(-0.0f, 0), va_reference(-0.0f, 0)); + // Tiny negative that rounds to zero at this precision must still render as "-0.00". + EXPECT_EQ(va_to_string(-0.001f, 2), va_reference(-0.001f, 2)); +} + +TEST(ValueAccuracyToBuf, OverflowFallsBackToSnprintf) { + // |value| * 10^acc must exceed UINT32_MAX to exercise the snprintf fallback path. + EXPECT_EQ(va_to_string(1.0e7f, 3), va_reference(1.0e7f, 3)); + EXPECT_EQ(va_to_string(-1.0e7f, 3), va_reference(-1.0e7f, 3)); + EXPECT_EQ(va_to_string(5.0e9f, 0), va_reference(5.0e9f, 0)); +} + +// --- value_accuracy_with_uom_to_buf --- + +static std::string va_uom_to_string(float value, int8_t accuracy_decimals, const char *uom) { + char buf[VALUE_ACCURACY_MAX_LEN]; + std::span sp(buf); + StringRef ref(uom); + size_t len = value_accuracy_with_uom_to_buf(sp, value, accuracy_decimals, ref); + return std::string(buf, len); +} + +static std::string va_uom_reference(float value, int8_t accuracy_decimals, const char *uom) { + char buf[VALUE_ACCURACY_MAX_LEN]; + if (!uom || *uom == '\0') { + snprintf(buf, sizeof(buf), "%.*f", accuracy_decimals, value); + } else { + snprintf(buf, sizeof(buf), "%.*f %s", accuracy_decimals, value, uom); + } + return std::string(buf); +} + +TEST(ValueAccuracyWithUomToBuf, BasicWithUnit) { + EXPECT_EQ(va_uom_to_string(23.456f, 2, "°C"), va_uom_reference(23.456f, 2, "°C")); + EXPECT_EQ(va_uom_to_string(1013.25f, 2, "hPa"), va_uom_reference(1013.25f, 2, "hPa")); + EXPECT_EQ(va_uom_to_string(-40.0f, 1, "°F"), va_uom_reference(-40.0f, 1, "°F")); + EXPECT_EQ(va_uom_to_string(100.0f, 0, "%"), va_uom_reference(100.0f, 0, "%")); +} + +TEST(ValueAccuracyWithUomToBuf, EmptyUnit) { + EXPECT_EQ(va_uom_to_string(23.456f, 2, ""), "23.46"); + EXPECT_EQ(va_uom_to_string(0.0f, 1, ""), "0.0"); +} + +TEST(ValueAccuracyWithUomToBuf, ReturnsCorrectLength) { + char buf[VALUE_ACCURACY_MAX_LEN]; + std::span sp(buf); + StringRef ref("°C"); + size_t len = value_accuracy_with_uom_to_buf(sp, 23.46f, 2, ref); + EXPECT_EQ(strlen(buf), len); + EXPECT_EQ(len, strlen("23.46 °C")); +} + +TEST(ValueAccuracyWithUomToBuf, NearBufferLimitTruncates) { + // Build a unit long enough that value + " " + unit exceeds VALUE_ACCURACY_MAX_LEN. + // "23.46" (5) + " " (1) + unit -> must cap at buf.size()-1 and stay null-terminated. + std::string long_unit(VALUE_ACCURACY_MAX_LEN, 'U'); + char buf[VALUE_ACCURACY_MAX_LEN]; + std::span sp(buf); + StringRef ref(long_unit.c_str()); + size_t len = value_accuracy_with_uom_to_buf(sp, 23.46f, 2, ref); + EXPECT_LT(len, VALUE_ACCURACY_MAX_LEN); + EXPECT_EQ(strlen(buf), len); + // Should begin with the formatted value and a separator. + EXPECT_EQ(std::string(buf, 6), "23.46 "); +} + +TEST(ValueAccuracyWithUomToBuf, MatchesSnprintf) { + const char *units[] = {"°C", "hPa", "%", "W", "kWh", "m/s"}; + float values[] = {0.0f, 23.456f, -40.0f, 1013.25f, 100.0f}; + int8_t accs[] = {0, 1, 2, 3}; + for (const char *u : units) { + for (float v : values) { + for (int8_t a : accs) { + EXPECT_EQ(va_uom_to_string(v, a, u), va_uom_reference(v, a, u)) + << "value=" << v << " acc=" << static_cast(a) << " uom=" << u; + } + } + } +} + +} // namespace esphome::core::testing From 9c80cbf19c6d604d1906d8e83bced3d67dd5f6dd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Apr 2026 06:34:26 +0200 Subject: [PATCH 13/15] [light] Reduce validate_ clamp code size and speed up unit-range clamps (#15728) --- esphome/components/light/light_call.cpp | 63 +++++++++------ esphome/components/light/light_call.h | 39 ++++----- esphome/components/light/light_color_values.h | 80 ++++++++++++++----- 3 files changed, 117 insertions(+), 65 deletions(-) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index a749cd7305..7b28065e4e 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -10,13 +10,10 @@ namespace esphome::light { static const char *const TAG = "light"; -// Helper functions to reduce code size for logging -static void clamp_and_log_if_invalid(const char *name, float &value, const LogString *param_name, float min = 0.0f, - float max = 1.0f) { - if (value < min || value > max) { - ESP_LOGW(TAG, "'%s': %s value %.2f is out of range [%.1f - %.1f]", name, LOG_STR_ARG(param_name), value, min, max); - value = clamp(value, min, max); - } +// Cold-path logger; caller handles the clamp so the in-range hot path avoids +// the spill/reload around the call. +static void log_value_out_of_range(const char *name, float value, const LogString *param_name, float min, float max) { + ESP_LOGW(TAG, "'%s': %s value %.2f is out of range [%.1f - %.1f]", name, LOG_STR_ARG(param_name), value, min, max); } #if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_WARN @@ -57,6 +54,12 @@ static void log_invalid_parameter(const char *name, const LogString *message) { PROGMEM_STRING_TABLE(ColorModeHumanStrings, "Unknown", "On/Off", "Brightness", "White", "Color temperature", "Cold/warm white", "RGB", "RGBW", "RGB + color temperature", "RGB + cold/warm white"); +// Indices 0-7 match FieldFlags bits 0-7; index 8 is color_temperature. +// PROGMEM_STRING_TABLE is constexpr-init (no RAM guard variable). +PROGMEM_STRING_TABLE(ValidateFieldNames, "Brightness", "Color brightness", "Red", "Green", "Blue", "White", + "Cold white", "Warm white", "Color temperature"); +static constexpr uint8_t VALIDATE_CT_INDEX = 8; + static const LogString *color_mode_to_human(ColorMode color_mode) { return ColorModeHumanStrings::get_log_str(ColorModeBitPolicy::to_bit(color_mode), 0); } @@ -277,25 +280,37 @@ LightColorValues LightCall::validate_() { if (this->has_state()) v.set_state(this->state_); - // clamp_and_log_if_invalid already clamps in-place, so assign directly - // to avoid redundant clamp code from the setter being inlined. -#define VALIDATE_AND_APPLY(field, name_str, ...) \ - if (this->has_##field()) { \ - clamp_and_log_if_invalid(name, this->field##_, LOG_STR(name_str), ##__VA_ARGS__); \ - v.field##_ = this->field##_; \ + // FieldFlags bits 0-7 must match unit_fields_ array indices. + static_assert(FLAG_HAS_BRIGHTNESS == 1u << 0 && FLAG_HAS_COLOR_BRIGHTNESS == 1u << 1 && FLAG_HAS_RED == 1u << 2 && + FLAG_HAS_GREEN == 1u << 3 && FLAG_HAS_BLUE == 1u << 4 && FLAG_HAS_WHITE == 1u << 5 && + FLAG_HAS_COLD_WHITE == 1u << 6 && FLAG_HAS_WARM_WHITE == 1u << 7, + "FieldFlags bits 0-7 must match unit_fields_ indices"); + + // Iterate set bits only (ctz + clear-lowest) — HA can drive perform() + // at high frequency so the hot path is O(popcount). + unsigned active = this->flags_ & CLAMP_FLAGS_MASK; + while (active != 0) { + unsigned bit = __builtin_ctz(active); + active &= active - 1; // clear lowest set bit + float &value = this->unit_fields_[bit]; + if (float_out_of_unit_range(value)) { + log_value_out_of_range(name, value, ValidateFieldNames::get_log_str(bit, 0), 0.0f, 1.0f); + value = clamp_unit_float(value); + } + v.unit_fields_[bit] = value; } - VALIDATE_AND_APPLY(brightness, "Brightness") - VALIDATE_AND_APPLY(color_brightness, "Color brightness") - VALIDATE_AND_APPLY(red, "Red") - VALIDATE_AND_APPLY(green, "Green") - VALIDATE_AND_APPLY(blue, "Blue") - VALIDATE_AND_APPLY(white, "White") - VALIDATE_AND_APPLY(cold_white, "Cold white") - VALIDATE_AND_APPLY(warm_white, "Warm white") - VALIDATE_AND_APPLY(color_temperature, "Color temperature", traits.get_min_mireds(), traits.get_max_mireds()) - -#undef VALIDATE_AND_APPLY + // color_temperature: runtime range from traits. + if (this->has_color_temperature()) { + const float ct_min = traits.get_min_mireds(); + const float ct_max = traits.get_max_mireds(); + if (this->color_temperature_ < ct_min || this->color_temperature_ > ct_max) { + log_value_out_of_range(name, this->color_temperature_, ValidateFieldNames::get_log_str(VALIDATE_CT_INDEX, 0), + ct_min, ct_max); + this->color_temperature_ = clamp(this->color_temperature_, ct_min, ct_max); + } + v.color_temperature_ = this->color_temperature_; + } v.normalize_color(); diff --git a/esphome/components/light/light_call.h b/esphome/components/light/light_call.h index 39953d0d20..e3352de727 100644 --- a/esphome/components/light/light_call.h +++ b/esphome/components/light/light_call.h @@ -195,25 +195,26 @@ class LightCall { /// Some color modes also can be set using non-native parameters, transform those calls. void transform_parameters_(const LightTraits &traits); - // Bitfield flags - each flag indicates whether a corresponding value has been set. + // Bits 0-7 index unit_fields_[] in validate_(); don't reorder (asserts in light_call.cpp). enum FieldFlags : uint16_t { - FLAG_HAS_STATE = 1 << 0, - FLAG_HAS_TRANSITION = 1 << 1, - FLAG_HAS_FLASH = 1 << 2, - FLAG_HAS_EFFECT = 1 << 3, - FLAG_HAS_BRIGHTNESS = 1 << 4, - FLAG_HAS_COLOR_BRIGHTNESS = 1 << 5, - FLAG_HAS_RED = 1 << 6, - FLAG_HAS_GREEN = 1 << 7, - FLAG_HAS_BLUE = 1 << 8, - FLAG_HAS_WHITE = 1 << 9, - FLAG_HAS_COLOR_TEMPERATURE = 1 << 10, - FLAG_HAS_COLD_WHITE = 1 << 11, - FLAG_HAS_WARM_WHITE = 1 << 12, + FLAG_HAS_BRIGHTNESS = 1 << 0, + FLAG_HAS_COLOR_BRIGHTNESS = 1 << 1, + FLAG_HAS_RED = 1 << 2, + FLAG_HAS_GREEN = 1 << 3, + FLAG_HAS_BLUE = 1 << 4, + FLAG_HAS_WHITE = 1 << 5, + FLAG_HAS_COLD_WHITE = 1 << 6, + FLAG_HAS_WARM_WHITE = 1 << 7, + FLAG_HAS_COLOR_TEMPERATURE = 1 << 8, + FLAG_HAS_STATE = 1 << 9, + FLAG_HAS_TRANSITION = 1 << 10, + FLAG_HAS_FLASH = 1 << 11, + FLAG_HAS_EFFECT = 1 << 12, FLAG_HAS_COLOR_MODE = 1 << 13, FLAG_PUBLISH = 1 << 14, FLAG_SAVE = 1 << 15, }; + static constexpr uint16_t CLAMP_FLAGS_MASK = 0x00FFu; // bits 0-7 inline bool has_transition_() { return (this->flags_ & FLAG_HAS_TRANSITION) != 0; } inline bool has_flash_() { return (this->flags_ & FLAG_HAS_FLASH) != 0; } @@ -239,19 +240,11 @@ class LightCall { LightState *parent_; // Light state values - use flags_ to check if a value has been set. - // Group 4-byte aligned members first uint32_t transition_length_; uint32_t flash_length_; uint32_t effect_; - float brightness_; - float color_brightness_; - float red_; - float green_; - float blue_; - float white_; + ESPHOME_LIGHT_UNIT_FIELDS_UNION(); float color_temperature_; - float cold_white_; - float warm_white_; // Smaller members at the end for better packing uint16_t flags_{FLAG_PUBLISH | FLAG_SAVE}; // Tracks which values are set diff --git a/esphome/components/light/light_color_values.h b/esphome/components/light/light_color_values.h index fa286a3941..5cafa9fe82 100644 --- a/esphome/components/light/light_color_values.h +++ b/esphome/components/light/light_color_values.h @@ -3,11 +3,62 @@ #include "esphome/core/helpers.h" #include "color_mode.h" #include +#include +#include namespace esphome::light { inline static uint8_t to_uint8_scale(float x) { return static_cast(roundf(x * 255.0f)); } +// IEEE 754 bit patterns. Values in [0.0f, 1.0f] have bits <= ONE_F_BITS; +// negatives have the sign bit set (→ huge unsigned). A single unsigned compare +// replaces two soft-float __ltsf2/__gtsf2 calls on ESP8266. +static constexpr uint32_t ONE_F_BITS = 0x3F800000u; // 1.0f +static constexpr uint32_t NEG_ZERO_F_BITS = 0x80000000u; // -0.0f / sign-bit mask +static_assert(sizeof(float) == sizeof(uint32_t), "float must be 32-bit"); +static_assert(std::numeric_limits::is_iec559, "IEEE 754 float required"); + +// Union pun — memcpy/bit_cast don't fold on xtensa-gcc (see api/proto.h). +// -0.0f is numerically zero so it's reported in range (no warning, no clamp). +inline bool float_out_of_unit_range(float x) { + union { + float f; + uint32_t u; + } pun; + pun.f = x; + return pun.u > ONE_F_BITS && pun.u != NEG_ZERO_F_BITS; +} + +// Clamps to [0.0f, 1.0f] without float compares. Out of range: sign bit set +// (negatives, -NaN, -Inf) → 0.0f; sign bit clear (>1, +NaN, +Inf) → 1.0f. +inline float clamp_unit_float(float x) { + union { + float f; + uint32_t u; + } pun; + pun.f = x; + if (pun.u <= ONE_F_BITS) + return x; + return (pun.u & NEG_ZERO_F_BITS) ? 0.0f : 1.0f; // sign bit → negative → clamp to 0 +} + +// Shared anonymous union: eight unit-range floats alias unit_fields_[8] so +// LightCall::validate_() can iterate them as a real array. GCC/Clang ext. +#define ESPHOME_LIGHT_UNIT_FIELDS_UNION() \ + union { \ + struct { \ + float brightness_; \ + float color_brightness_; \ + float red_; \ + float green_; \ + float blue_; \ + float white_; \ + float cold_white_; \ + float warm_white_; \ + }; \ + float unit_fields_[8]; \ + } + /** This class represents the color state for a light object. * * The representation of the color state is dependent on the active color mode. A color mode consists of multiple @@ -52,9 +103,9 @@ class LightColorValues { green_(1.0f), blue_(1.0f), white_(1.0f), - color_temperature_{0.0f}, cold_white_{1.0f}, warm_white_{1.0f}, + color_temperature_{0.0f}, color_mode_(ColorMode::UNKNOWN) {} LightColorValues(ColorMode color_mode, float state, float brightness, float color_brightness, float red, float green, @@ -220,39 +271,39 @@ class LightColorValues { /// Get the binary true/false state of these light color values. bool is_on() const { return this->get_state() != 0.0f; } /// Set the state of these light color values. In range from 0.0 (off) to 1.0 (on) - void set_state(float state) { this->state_ = clamp(state, 0.0f, 1.0f); } + void set_state(float state) { this->state_ = clamp_unit_float(state); } /// Set the state of these light color values as a binary true/false. void set_state(bool state) { this->state_ = state ? 1.0f : 0.0f; } /// Get the brightness property of these light color values. In range 0.0 to 1.0 float get_brightness() const { return this->brightness_; } /// Set the brightness property of these light color values. In range 0.0 to 1.0 - void set_brightness(float brightness) { this->brightness_ = clamp(brightness, 0.0f, 1.0f); } + void set_brightness(float brightness) { this->brightness_ = clamp_unit_float(brightness); } /// Get the color brightness property of these light color values. In range 0.0 to 1.0 float get_color_brightness() const { return this->color_brightness_; } /// Set the color brightness property of these light color values. In range 0.0 to 1.0 - void set_color_brightness(float brightness) { this->color_brightness_ = clamp(brightness, 0.0f, 1.0f); } + void set_color_brightness(float brightness) { this->color_brightness_ = clamp_unit_float(brightness); } /// Get the red property of these light color values. In range 0.0 to 1.0 float get_red() const { return this->red_; } /// Set the red property of these light color values. In range 0.0 to 1.0 - void set_red(float red) { this->red_ = clamp(red, 0.0f, 1.0f); } + void set_red(float red) { this->red_ = clamp_unit_float(red); } /// Get the green property of these light color values. In range 0.0 to 1.0 float get_green() const { return this->green_; } /// Set the green property of these light color values. In range 0.0 to 1.0 - void set_green(float green) { this->green_ = clamp(green, 0.0f, 1.0f); } + void set_green(float green) { this->green_ = clamp_unit_float(green); } /// Get the blue property of these light color values. In range 0.0 to 1.0 float get_blue() const { return this->blue_; } /// Set the blue property of these light color values. In range 0.0 to 1.0 - void set_blue(float blue) { this->blue_ = clamp(blue, 0.0f, 1.0f); } + void set_blue(float blue) { this->blue_ = clamp_unit_float(blue); } /// Get the white property of these light color values. In range 0.0 to 1.0 float get_white() const { return white_; } /// Set the white property of these light color values. In range 0.0 to 1.0 - void set_white(float white) { this->white_ = clamp(white, 0.0f, 1.0f); } + void set_white(float white) { this->white_ = clamp_unit_float(white); } /// Get the color temperature property of these light color values in mired. float get_color_temperature() const { return this->color_temperature_; } @@ -277,26 +328,19 @@ class LightColorValues { /// Get the cold white property of these light color values. In range 0.0 to 1.0. float get_cold_white() const { return this->cold_white_; } /// Set the cold white property of these light color values. In range 0.0 to 1.0. - void set_cold_white(float cold_white) { this->cold_white_ = clamp(cold_white, 0.0f, 1.0f); } + void set_cold_white(float cold_white) { this->cold_white_ = clamp_unit_float(cold_white); } /// Get the warm white property of these light color values. In range 0.0 to 1.0. float get_warm_white() const { return this->warm_white_; } /// Set the warm white property of these light color values. In range 0.0 to 1.0. - void set_warm_white(float warm_white) { this->warm_white_ = clamp(warm_white, 0.0f, 1.0f); } + void set_warm_white(float warm_white) { this->warm_white_ = clamp_unit_float(warm_white); } friend class LightCall; protected: float state_; ///< ON / OFF, float for transition - float brightness_; - float color_brightness_; - float red_; - float green_; - float blue_; - float white_; + ESPHOME_LIGHT_UNIT_FIELDS_UNION(); float color_temperature_; ///< Color Temperature in Mired - float cold_white_; - float warm_white_; ColorMode color_mode_; }; From a3b49d1ed9f2ebdffce9ac1af73b5f6a67660a43 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Apr 2026 06:43:33 +0200 Subject: [PATCH 14/15] [core] Use MAC_ADDRESS_BUFFER_SIZE constant instead of duplicated literal (#15913) --- esphome/components/esp32_ble/ble.cpp | 4 +--- esphome/core/application.h | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index ebb44c7d91..6bbf0d6a26 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -257,11 +257,9 @@ bool ESP32BLE::ble_setup_() { if (this->name_ != nullptr) { if (App.is_name_add_mac_suffix_enabled()) { - // MAC address length: 12 hex chars + null terminator - constexpr size_t mac_address_len = 13; // MAC address suffix length (last 6 characters of 12-char MAC address string) constexpr size_t mac_address_suffix_len = 6; - char mac_addr[mac_address_len]; + char mac_addr[MAC_ADDRESS_BUFFER_SIZE]; get_mac_address_into_buffer(mac_addr); const char *mac_suffix_ptr = mac_addr + mac_address_suffix_len; make_name_with_suffix_to(name_buffer, sizeof(name_buffer), this->name_, strlen(this->name_), '-', mac_suffix_ptr, diff --git a/esphome/core/application.h b/esphome/core/application.h index d3851a32da..e579080c97 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -82,11 +82,9 @@ class Application { void pre_setup(char *name, size_t name_len, char *friendly_name, size_t friendly_name_len) { arch_init(); this->name_add_mac_suffix_ = true; - // MAC address length: 12 hex chars + null terminator - constexpr size_t mac_address_len = 13; // MAC address suffix length (last 6 characters of 12-char MAC address string) constexpr size_t mac_address_suffix_len = 6; - char mac_addr[mac_address_len]; + char mac_addr[MAC_ADDRESS_BUFFER_SIZE]; get_mac_address_into_buffer(mac_addr); // Overwrite the placeholder suffix in the mutable static buffers with actual MAC // name is always non-empty (validated by validate_hostname in Python config) From 23ad30cb4cbb81aca313cf49b32473a375e5cc56 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Apr 2026 06:44:53 +0200 Subject: [PATCH 15/15] [esp32] Use xTaskGetTickCount() for millis() when tick rate is 1kHz (#15661) --- esphome/components/esp32/core.cpp | 21 +++++++++++++++- esphome/core/application.cpp | 16 ++++++------ esphome/core/application.h | 2 +- esphome/core/component.h | 3 ++- esphome/core/millis_internal.h | 42 +++++++++++++++++++++++++++++++ esphome/core/scheduler.h | 10 ++++++-- 6 files changed, 80 insertions(+), 14 deletions(-) create mode 100644 esphome/core/millis_internal.h diff --git a/esphome/components/esp32/core.cpp b/esphome/components/esp32/core.cpp index add50dcf4d..1c63137183 100644 --- a/esphome/components/esp32/core.cpp +++ b/esphome/components/esp32/core.cpp @@ -23,7 +23,26 @@ extern "C" __attribute__((weak)) void initArduino() {} namespace esphome { void HOT yield() { vPortYield(); } -uint32_t IRAM_ATTR HOT millis() { return micros_to_millis(static_cast(esp_timer_get_time())); } +// Use xTaskGetTickCount() when tick rate is 1 kHz (ESPHome's default via sdkconfig), +// falling back to esp_timer for non-standard rates. IRAM_ATTR is required because +// Wiegand and ZyAura call millis() from IRAM_ATTR ISR handlers on ESP32. +// xTaskGetTickCountFromISR() is used in ISR context to satisfy the FreeRTOS API contract. +uint32_t IRAM_ATTR HOT millis() { +#if CONFIG_FREERTOS_HZ == 1000 + if (xPortInIsrContext()) [[unlikely]] { + return xTaskGetTickCountFromISR(); + } + return xTaskGetTickCount(); +#else + return micros_to_millis(static_cast(esp_timer_get_time())); +#endif +} +// millis_64() stays on esp_timer — a different clock from xTaskGetTickCount(). This is +// safe because the two are never cross-compared: millis() values are only used for +// millis()-vs-millis() deltas (feed_wdt, warn_blocking, component start time), while +// millis_64() is used by the Scheduler and uptime sensors. On ESP32 (USE_NATIVE_64BIT_TIME), +// Scheduler::millis_64_from_(now) discards the 32-bit now and calls millis_64() directly, +// so the Scheduler is internally consistent on the esp_timer clock. uint64_t HOT millis_64() { return micros_to_millis(static_cast(esp_timer_get_time())); } void HOT delay(uint32_t ms) { vTaskDelay(ms / portTICK_PERIOD_MS); } uint32_t IRAM_ATTR HOT micros() { return (uint32_t) esp_timer_get_time(); } diff --git a/esphome/core/application.cpp b/esphome/core/application.cpp index b626eb1de6..ea1912d645 100644 --- a/esphome/core/application.cpp +++ b/esphome/core/application.cpp @@ -78,7 +78,7 @@ void Application::setup() { Component *component = this->components_[i]; // Update loop_component_start_time_ before calling each component during setup - this->loop_component_start_time_ = millis(); + this->loop_component_start_time_ = MillisInternal::get(); component->call(); this->scheduler.process_to_add(); this->feed_wdt(); @@ -91,17 +91,15 @@ void Application::setup() { this->app_state_ |= STATUS_LED_WARNING; do { - uint32_t now = millis(); - // Service scheduler and process pending loop enables to handle GPIO // interrupts during setup. During setup we always run the component // phase (no loop_interval_ gate), so call both helpers unconditionally. - this->scheduler_tick_(now); + this->scheduler_tick_(MillisInternal::get()); this->before_component_phase_(); for (uint32_t j = 0; j <= i; j++) { // Update loop_component_start_time_ right before calling each component - this->loop_component_start_time_ = millis(); + this->loop_component_start_time_ = MillisInternal::get(); this->components_[j]->call(); this->feed_wdt(); } @@ -215,7 +213,7 @@ void Application::process_dump_config_() { void Application::feed_wdt() { // Cold entry: callers without a millis() timestamp in hand. Fetches the // time and takes the same rate-limit paths as feed_wdt_with_time(). - uint32_t now = millis(); + uint32_t now = MillisInternal::get(); if (now - this->last_wdt_feed_ > WDT_FEED_INTERVAL_MS) { this->feed_wdt_slow_(now); } @@ -305,7 +303,7 @@ void Application::run_powerdown_hooks() { } void Application::teardown_components(uint32_t timeout_ms) { - uint32_t start_time = millis(); + uint32_t start_time = MillisInternal::get(); // Use a StaticVector instead of std::vector to avoid heap allocation // since we know the actual size at compile time @@ -384,7 +382,7 @@ void Application::teardown_components(uint32_t timeout_ms) { } // Update time for next iteration - now = millis(); + now = MillisInternal::get(); } if (pending_count > 0) { @@ -427,7 +425,7 @@ void Application::disable_component_loop_(Component *component) { // This prevents integer underflow in timing calculations by ensuring // the swapped component starts with a fresh timing reference, avoiding // errors caused by stale or wrapped timing values. - this->loop_component_start_time_ = millis(); + this->loop_component_start_time_ = MillisInternal::get(); } } return; diff --git a/esphome/core/application.h b/esphome/core/application.h index e579080c97..b480e52b2d 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -637,7 +637,7 @@ inline void ESPHOME_ALWAYS_INLINE Application::loop() { // (advanced by its per-item feeds) or `now` unchanged. We adopt it as `now` // so the gate check and WDT feed both reflect actual elapsed time after // scheduler dispatch, without an extra millis() call. - uint32_t now = this->scheduler_tick_(millis()); + uint32_t now = this->scheduler_tick_(MillisInternal::get()); // Guarantee one WDT feed per tick even when the scheduler had nothing to // dispatch and the component phase is gated out — covers configs with no // looping components and no scheduler work (setup() has its own diff --git a/esphome/core/component.h b/esphome/core/component.h index 67db5423af..6afcfda41d 100644 --- a/esphome/core/component.h +++ b/esphome/core/component.h @@ -9,6 +9,7 @@ #include "esphome/core/hal.h" #include "esphome/core/helpers.h" #include "esphome/core/log.h" +#include "esphome/core/millis_internal.h" #include "esphome/core/optional.h" // Forward declarations for friend access from codegen-generated setup() @@ -656,7 +657,7 @@ class WarnIfComponentBlockingGuard { #ifdef USE_RUNTIME_STATS this->component_->runtime_stats_.record_time(micros() - this->started_us_); #endif - uint32_t curr_time = millis(); + uint32_t curr_time = MillisInternal::get(); #ifndef USE_BENCHMARK // Fast path: compare against constant threshold in ms (computed at compile time from centiseconds) static constexpr uint32_t WARN_IF_BLOCKING_OVER_MS = static_cast(WARN_IF_BLOCKING_OVER_CS) * 10U; diff --git a/esphome/core/millis_internal.h b/esphome/core/millis_internal.h new file mode 100644 index 0000000000..6b73476680 --- /dev/null +++ b/esphome/core/millis_internal.h @@ -0,0 +1,42 @@ +#pragma once + +#include "esphome/core/hal.h" +#include "esphome/core/helpers.h" + +#if defined(USE_ESP32) +#include +#include +#include +#endif + +namespace esphome { + +// Friend-gated accessor for a fast millis() variant intended only for +// known task-context callers on the main loop hot path (Application::loop() +// and WarnIfComponentBlockingGuard::finish()). It skips the ISR-context +// dispatch that the public esphome::millis() pays on ESP32. +// +// MUST NOT be called from ISR context: on ESP32 it calls the non-FromISR +// FreeRTOS API directly, which is undefined behavior in ISR context. +// +// Adding new callers requires adding a friend declaration here — that +// is the review point. Do not relax the access (e.g. by making get() +// public) without considering the ISR-safety contract. +// +// Other platforms currently delegate to the public millis(); the friend +// gate still enforces the intent so platform-specific fast paths can be +// added later without changing call sites. +class MillisInternal { + private: + static ESPHOME_ALWAYS_INLINE uint32_t get() { +#if defined(USE_ESP32) && CONFIG_FREERTOS_HZ == 1000 + return xTaskGetTickCount(); +#else + return millis(); +#endif + } + friend class Application; + friend class WarnIfComponentBlockingGuard; +}; + +} // namespace esphome diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index b0ce365a6f..b7e99d4603 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -285,8 +285,14 @@ class Scheduler { bool cancel_retry_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id); // Extend a 32-bit millis() value to 64-bit. Use when the caller already has a fresh now. - // On platforms with native 64-bit time, ignores now and uses millis_64() directly. - // On other platforms, extends now to 64-bit using rollover tracking. + // On platforms with native 64-bit time (ESP32, Host, Zephyr, RP2040 — see + // USE_NATIVE_64BIT_TIME in defines.h), ignores now and uses millis_64() directly, so the + // Scheduler always works in 64-bit time regardless of what the caller's 32-bit now came + // from. On ESP32 specifically, millis() comes from xTaskGetTickCount while millis_64() + // comes from esp_timer — two different clocks — but that is safe because scheduling + // compares millis_64 values against millis_64 only, never against millis(). + // On platforms without native 64-bit time (e.g. ESP8266), extends now to 64-bit using + // rollover tracking, so both millis() and scheduling use the same underlying clock. uint64_t ESPHOME_ALWAYS_INLINE millis_64_from_(uint32_t now) { #ifdef USE_NATIVE_64BIT_TIME (void) now;