From 47ee2f4ad904e9b5968593b479533bc9aa976a3b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 16:20:39 -1000 Subject: [PATCH 01/16] [wifi] Use StaticVector for WiFi listeners with per-type compile-time sizing (#13197) --- esphome/components/wifi/__init__.py | 56 +++++++++++++++---- esphome/components/wifi/wifi_component.h | 40 ++++++++++--- .../wifi/wifi_component_esp8266.cpp | 14 ++--- .../wifi/wifi_component_esp_idf.cpp | 16 +++--- .../wifi/wifi_component_libretiny.cpp | 16 +++--- .../components/wifi/wifi_component_pico_w.cpp | 14 ++--- esphome/components/wifi_info/text_sensor.py | 34 +++++------ .../wifi_info/wifi_info_text_sensor.cpp | 22 ++++++-- .../wifi_info/wifi_info_text_sensor.h | 10 +++- esphome/components/wifi_signal/sensor.py | 2 +- .../wifi_signal/wifi_signal_sensor.h | 6 +- esphome/core/defines.h | 9 ++- 12 files changed, 161 insertions(+), 78 deletions(-) diff --git a/esphome/components/wifi/__init__.py b/esphome/components/wifi/__init__.py index 26aec29b6d..98266eb589 100644 --- a/esphome/components/wifi/__init__.py +++ b/esphome/components/wifi/__init__.py @@ -624,7 +624,11 @@ async def wifi_disable_to_code(config, action_id, template_arg, args): KEEP_SCAN_RESULTS_KEY = "wifi_keep_scan_results" RUNTIME_POWER_SAVE_KEY = "wifi_runtime_power_save" -WIFI_LISTENERS_KEY = "wifi_listeners" +# Keys for listener counts +IP_STATE_LISTENERS_KEY = "wifi_ip_state_listeners" +SCAN_RESULTS_LISTENERS_KEY = "wifi_scan_results_listeners" +CONNECT_STATE_LISTENERS_KEY = "wifi_connect_state_listeners" +POWER_SAVE_LISTENERS_KEY = "wifi_power_save_listeners" def request_wifi_scan_results(): @@ -650,15 +654,28 @@ def enable_runtime_power_save_control(): CORE.data[RUNTIME_POWER_SAVE_KEY] = True -def request_wifi_listeners() -> None: - """Request that WiFi state listeners be compiled in. +def request_wifi_ip_state_listener() -> None: + """Request an IP state listener slot.""" + CORE.data[IP_STATE_LISTENERS_KEY] = CORE.data.get(IP_STATE_LISTENERS_KEY, 0) + 1 - Components that need to be notified about WiFi state changes (IP address changes, - scan results, connection state) should call this function during their code generation. - This enables the add_ip_state_listener(), add_scan_results_listener(), - and add_connect_state_listener() APIs. - """ - CORE.data[WIFI_LISTENERS_KEY] = True + +def request_wifi_scan_results_listener() -> None: + """Request a scan results listener slot.""" + CORE.data[SCAN_RESULTS_LISTENERS_KEY] = ( + CORE.data.get(SCAN_RESULTS_LISTENERS_KEY, 0) + 1 + ) + + +def request_wifi_connect_state_listener() -> None: + """Request a connect state listener slot.""" + CORE.data[CONNECT_STATE_LISTENERS_KEY] = ( + CORE.data.get(CONNECT_STATE_LISTENERS_KEY, 0) + 1 + ) + + +def request_wifi_power_save_listener() -> None: + """Request a power save listener slot.""" + CORE.data[POWER_SAVE_LISTENERS_KEY] = CORE.data.get(POWER_SAVE_LISTENERS_KEY, 0) + 1 @coroutine_with_priority(CoroPriority.FINAL) @@ -670,8 +687,25 @@ async def final_step(): ) if CORE.data.get(RUNTIME_POWER_SAVE_KEY, False): cg.add_define("USE_WIFI_RUNTIME_POWER_SAVE") - if CORE.data.get(WIFI_LISTENERS_KEY, False): - cg.add_define("USE_WIFI_LISTENERS") + + # Generate listener defines - each listener type has its own #ifdef + ip_state_count = CORE.data.get(IP_STATE_LISTENERS_KEY, 0) + scan_results_count = CORE.data.get(SCAN_RESULTS_LISTENERS_KEY, 0) + connect_state_count = CORE.data.get(CONNECT_STATE_LISTENERS_KEY, 0) + power_save_count = CORE.data.get(POWER_SAVE_LISTENERS_KEY, 0) + + if ip_state_count: + cg.add_define("USE_WIFI_IP_STATE_LISTENERS") + cg.add_define("ESPHOME_WIFI_IP_STATE_LISTENERS", ip_state_count) + if scan_results_count: + cg.add_define("USE_WIFI_SCAN_RESULTS_LISTENERS") + cg.add_define("ESPHOME_WIFI_SCAN_RESULTS_LISTENERS", scan_results_count) + if connect_state_count: + cg.add_define("USE_WIFI_CONNECT_STATE_LISTENERS") + cg.add_define("ESPHOME_WIFI_CONNECT_STATE_LISTENERS", connect_state_count) + if power_save_count: + cg.add_define("USE_WIFI_POWER_SAVE_LISTENERS") + cg.add_define("ESPHOME_WIFI_POWER_SAVE_LISTENERS", power_save_count) @automation.register_action( diff --git a/esphome/components/wifi/wifi_component.h b/esphome/components/wifi/wifi_component.h index b4c4a622d5..dfc91fb5da 100644 --- a/esphome/components/wifi/wifi_component.h +++ b/esphome/components/wifi/wifi_component.h @@ -275,6 +275,9 @@ struct LTWiFiEvent; * * Components can implement this interface to receive IP address updates * without the overhead of std::function callbacks. + * + * @note Components must call wifi.request_wifi_ip_state_listener() in their + * Python to_code() to register for this listener type. */ class WiFiIPStateListener { public: @@ -286,6 +289,9 @@ class WiFiIPStateListener { * * Components can implement this interface to receive scan results * without the overhead of std::function callbacks. + * + * @note Components must call wifi.request_wifi_scan_results_listener() in their + * Python to_code() to register for this listener type. */ class WiFiScanResultsListener { public: @@ -296,6 +302,9 @@ class WiFiScanResultsListener { * * Components can implement this interface to receive connection updates * without the overhead of std::function callbacks. + * + * @note Components must call wifi.request_wifi_connect_state_listener() in their + * Python to_code() to register for this listener type. */ class WiFiConnectStateListener { public: @@ -306,6 +315,9 @@ class WiFiConnectStateListener { * * Components can implement this interface to receive power save mode updates * without the overhead of std::function callbacks. + * + * @note Components must call wifi.request_wifi_power_save_listener() in their + * Python to_code() to register for this listener type. */ class WiFiPowerSaveListener { public: @@ -444,26 +456,32 @@ class WiFiComponent : public Component { int32_t get_wifi_channel(); -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_IP_STATE_LISTENERS /** Add a listener for IP state changes. * Listener receives: IP addresses, DNS address 1, DNS address 2 */ void add_ip_state_listener(WiFiIPStateListener *listener) { this->ip_state_listeners_.push_back(listener); } +#endif // USE_WIFI_IP_STATE_LISTENERS +#ifdef USE_WIFI_SCAN_RESULTS_LISTENERS /// Add a listener for WiFi scan results void add_scan_results_listener(WiFiScanResultsListener *listener) { this->scan_results_listeners_.push_back(listener); } +#endif // USE_WIFI_SCAN_RESULTS_LISTENERS +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS /** Add a listener for WiFi connection state changes. * Listener receives: SSID, BSSID */ void add_connect_state_listener(WiFiConnectStateListener *listener) { this->connect_state_listeners_.push_back(listener); } +#endif // USE_WIFI_CONNECT_STATE_LISTENERS +#ifdef USE_WIFI_POWER_SAVE_LISTENERS /** Add a listener for WiFi power save mode changes. * Listener receives: WiFiPowerSaveMode */ void add_power_save_listener(WiFiPowerSaveListener *listener) { this->power_save_listeners_.push_back(listener); } -#endif // USE_WIFI_LISTENERS +#endif // USE_WIFI_POWER_SAVE_LISTENERS #ifdef USE_WIFI_RUNTIME_POWER_SAVE /** Request high-performance mode (no power saving) for improved WiFi latency. @@ -628,12 +646,18 @@ class WiFiComponent : public Component { WiFiAP ap_; #endif float output_power_{NAN}; -#ifdef USE_WIFI_LISTENERS - std::vector ip_state_listeners_; - std::vector scan_results_listeners_; - std::vector connect_state_listeners_; - std::vector power_save_listeners_; -#endif // USE_WIFI_LISTENERS +#ifdef USE_WIFI_IP_STATE_LISTENERS + StaticVector ip_state_listeners_; +#endif +#ifdef USE_WIFI_SCAN_RESULTS_LISTENERS + StaticVector scan_results_listeners_; +#endif +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS + StaticVector connect_state_listeners_; +#endif +#ifdef USE_WIFI_POWER_SAVE_LISTENERS + StaticVector power_save_listeners_; +#endif ESPPreferenceObject pref_; #ifdef USE_WIFI_FAST_CONNECT ESPPreferenceObject fast_connect_pref_; diff --git a/esphome/components/wifi/wifi_component_esp8266.cpp b/esphome/components/wifi/wifi_component_esp8266.cpp index 61c4584d09..6fb5dd5769 100644 --- a/esphome/components/wifi/wifi_component_esp8266.cpp +++ b/esphome/components/wifi/wifi_component_esp8266.cpp @@ -105,7 +105,7 @@ bool WiFiComponent::wifi_apply_power_save_() { } wifi_fpm_auto_sleep_set_in_null_mode(1); bool success = wifi_set_sleep_type(power_save); -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_POWER_SAVE_LISTENERS if (success) { for (auto *listener : this->power_save_listeners_) { listener->on_wifi_power_save(this->power_save_); @@ -511,12 +511,13 @@ void WiFiComponent::wifi_event_callback(System_Event_t *event) { it.channel); #endif s_sta_connected = true; -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS for (auto *listener : global_wifi_component->connect_state_listeners_) { listener->on_wifi_connect_state(StringRef(it.ssid, it.ssid_len), it.bssid); } +#endif // For static IP configurations, GOT_IP event may not fire, so notify IP listeners here -#ifdef USE_WIFI_MANUAL_IP +#if defined(USE_WIFI_IP_STATE_LISTENERS) && defined(USE_WIFI_MANUAL_IP) if (const WiFiAP *config = global_wifi_component->get_selected_sta_(); config && config->get_manual_ip().has_value()) { for (auto *listener : global_wifi_component->ip_state_listeners_) { @@ -524,7 +525,6 @@ void WiFiComponent::wifi_event_callback(System_Event_t *event) { global_wifi_component->get_dns_address(0), global_wifi_component->get_dns_address(1)); } } -#endif #endif break; } @@ -547,7 +547,7 @@ void WiFiComponent::wifi_event_callback(System_Event_t *event) { // This ensures is_connected() returns false during listener callbacks, // which is critical for proper reconnection logic (e.g., roaming). global_wifi_component->error_from_callback_ = true; -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS // Notify listeners AFTER setting error flag so they see correct state static constexpr uint8_t EMPTY_BSSID[6] = {}; for (auto *listener : global_wifi_component->connect_state_listeners_) { @@ -578,7 +578,7 @@ void WiFiComponent::wifi_event_callback(System_Event_t *event) { ESP_LOGV(TAG, "static_ip=%s gateway=%s netmask=%s", network::IPAddress(&it.ip).str_to(ip_buf), network::IPAddress(&it.gw).str_to(gw_buf), network::IPAddress(&it.mask).str_to(mask_buf)); s_sta_got_ip = true; -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_IP_STATE_LISTENERS for (auto *listener : global_wifi_component->ip_state_listeners_) { listener->on_ip_state(global_wifi_component->wifi_sta_ip_addresses(), global_wifi_component->get_dns_address(0), global_wifi_component->get_dns_address(1)); @@ -771,7 +771,7 @@ void WiFiComponent::wifi_scan_done_callback_(void *arg, STATUS status) { it->is_hidden != 0); } this->scan_done_ = true; -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_SCAN_RESULTS_LISTENERS for (auto *listener : global_wifi_component->scan_results_listeners_) { listener->on_wifi_scan_results(global_wifi_component->scan_result_); } diff --git a/esphome/components/wifi/wifi_component_esp_idf.cpp b/esphome/components/wifi/wifi_component_esp_idf.cpp index 820725ed31..848ec3e11c 100644 --- a/esphome/components/wifi/wifi_component_esp_idf.cpp +++ b/esphome/components/wifi/wifi_component_esp_idf.cpp @@ -281,7 +281,7 @@ bool WiFiComponent::wifi_apply_power_save_() { break; } bool success = esp_wifi_set_ps(power_save) == ESP_OK; -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_POWER_SAVE_LISTENERS if (success) { for (auto *listener : this->power_save_listeners_) { listener->on_wifi_power_save(this->power_save_); @@ -741,18 +741,18 @@ void WiFiComponent::wifi_process_event_(IDFWiFiEvent *data) { (const char *) it.ssid, bssid_buf, it.channel, get_auth_mode_str(it.authmode)); #endif s_sta_connected = true; -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS for (auto *listener : this->connect_state_listeners_) { listener->on_wifi_connect_state(StringRef(it.ssid, it.ssid_len), it.bssid); } +#endif // For static IP configurations, GOT_IP event may not fire, so notify IP listeners here -#ifdef USE_WIFI_MANUAL_IP +#if defined(USE_WIFI_IP_STATE_LISTENERS) && defined(USE_WIFI_MANUAL_IP) if (const WiFiAP *config = this->get_selected_sta_(); config && config->get_manual_ip().has_value()) { for (auto *listener : this->ip_state_listeners_) { listener->on_ip_state(this->wifi_sta_ip_addresses(), this->get_dns_address(0), this->get_dns_address(1)); } } -#endif #endif } else if (data->event_base == WIFI_EVENT && data->event_id == WIFI_EVENT_STA_DISCONNECTED) { @@ -774,7 +774,7 @@ void WiFiComponent::wifi_process_event_(IDFWiFiEvent *data) { s_sta_connected = false; s_sta_connecting = false; error_from_callback_ = true; -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS static constexpr uint8_t EMPTY_BSSID[6] = {}; for (auto *listener : this->connect_state_listeners_) { listener->on_wifi_connect_state(StringRef(), EMPTY_BSSID); @@ -788,7 +788,7 @@ void WiFiComponent::wifi_process_event_(IDFWiFiEvent *data) { #endif /* USE_NETWORK_IPV6 */ ESP_LOGV(TAG, "static_ip=" IPSTR " gateway=" IPSTR, IP2STR(&it.ip_info.ip), IP2STR(&it.ip_info.gw)); this->got_ipv4_address_ = true; -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_IP_STATE_LISTENERS for (auto *listener : this->ip_state_listeners_) { listener->on_ip_state(this->wifi_sta_ip_addresses(), this->get_dns_address(0), this->get_dns_address(1)); } @@ -799,7 +799,7 @@ void WiFiComponent::wifi_process_event_(IDFWiFiEvent *data) { const auto &it = data->data.ip_got_ip6; ESP_LOGV(TAG, "IPv6 address=" IPV6STR, IPV62STR(it.ip6_info.ip)); this->num_ipv6_addresses_++; -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_IP_STATE_LISTENERS for (auto *listener : this->ip_state_listeners_) { listener->on_ip_state(this->wifi_sta_ip_addresses(), this->get_dns_address(0), this->get_dns_address(1)); } @@ -843,7 +843,7 @@ void WiFiComponent::wifi_process_event_(IDFWiFiEvent *data) { scan_result_.emplace_back(bssid, ssid, record.primary, record.rssi, record.authmode != WIFI_AUTH_OPEN, ssid.empty()); } -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_SCAN_RESULTS_LISTENERS for (auto *listener : this->scan_results_listeners_) { listener->on_wifi_scan_results(this->scan_result_); } diff --git a/esphome/components/wifi/wifi_component_libretiny.cpp b/esphome/components/wifi/wifi_component_libretiny.cpp index c5b6a8ad96..162ed4e835 100644 --- a/esphome/components/wifi/wifi_component_libretiny.cpp +++ b/esphome/components/wifi/wifi_component_libretiny.cpp @@ -144,7 +144,7 @@ bool WiFiComponent::wifi_sta_pre_setup_() { } bool WiFiComponent::wifi_apply_power_save_() { bool success = WiFi.setSleep(this->power_save_ != WIFI_POWER_SAVE_NONE); -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_POWER_SAVE_LISTENERS if (success) { for (auto *listener : this->power_save_listeners_) { listener->on_wifi_power_save(this->power_save_); @@ -455,19 +455,19 @@ void WiFiComponent::wifi_process_event_(LTWiFiEvent *event) { // Note: We don't set CONNECTED state here yet - wait for GOT_IP // This matches ESP32 IDF behavior where s_sta_connected is set but // wifi_sta_connect_status_() also checks got_ipv4_address_ -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS for (auto *listener : this->connect_state_listeners_) { listener->on_wifi_connect_state(StringRef(it.ssid, it.ssid_len), it.bssid); } +#endif // For static IP configurations, GOT_IP event may not fire, so notify IP listeners here -#ifdef USE_WIFI_MANUAL_IP +#if defined(USE_WIFI_IP_STATE_LISTENERS) && defined(USE_WIFI_MANUAL_IP) if (const WiFiAP *config = this->get_selected_sta_(); config && config->get_manual_ip().has_value()) { s_sta_state = LTWiFiSTAState::CONNECTED; for (auto *listener : this->ip_state_listeners_) { listener->on_ip_state(this->wifi_sta_ip_addresses(), this->get_dns_address(0), this->get_dns_address(1)); } } -#endif #endif break; } @@ -521,7 +521,7 @@ void WiFiComponent::wifi_process_event_(LTWiFiEvent *event) { this->error_from_callback_ = true; } -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS static constexpr uint8_t EMPTY_BSSID[6] = {}; for (auto *listener : this->connect_state_listeners_) { listener->on_wifi_connect_state(StringRef(), EMPTY_BSSID); @@ -547,7 +547,7 @@ void WiFiComponent::wifi_process_event_(LTWiFiEvent *event) { ESP_LOGV(TAG, "static_ip=%s gateway=%s", network::IPAddress(WiFi.localIP()).str_to(ip_buf), network::IPAddress(WiFi.gatewayIP()).str_to(gw_buf)); s_sta_state = LTWiFiSTAState::CONNECTED; -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_IP_STATE_LISTENERS for (auto *listener : this->ip_state_listeners_) { listener->on_ip_state(this->wifi_sta_ip_addresses(), this->get_dns_address(0), this->get_dns_address(1)); } @@ -556,7 +556,7 @@ void WiFiComponent::wifi_process_event_(LTWiFiEvent *event) { } case ESPHOME_EVENT_ID_WIFI_STA_GOT_IP6: { ESP_LOGV(TAG, "Got IPv6"); -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_IP_STATE_LISTENERS for (auto *listener : this->ip_state_listeners_) { listener->on_ip_state(this->wifi_sta_ip_addresses(), this->get_dns_address(0), this->get_dns_address(1)); } @@ -677,7 +677,7 @@ void WiFiComponent::wifi_scan_done_callback_() { ssid.length() == 0); } WiFi.scanDelete(); -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_SCAN_RESULTS_LISTENERS for (auto *listener : this->scan_results_listeners_) { listener->on_wifi_scan_results(this->scan_result_); } diff --git a/esphome/components/wifi/wifi_component_pico_w.cpp b/esphome/components/wifi/wifi_component_pico_w.cpp index 1aa737ff4a..29ac096d94 100644 --- a/esphome/components/wifi/wifi_component_pico_w.cpp +++ b/esphome/components/wifi/wifi_component_pico_w.cpp @@ -55,7 +55,7 @@ bool WiFiComponent::wifi_apply_power_save_() { } int ret = cyw43_wifi_pm(&cyw43_state, pm); bool success = ret == 0; -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_POWER_SAVE_LISTENERS if (success) { for (auto *listener : this->power_save_listeners_) { listener->on_wifi_power_save(this->power_save_); @@ -245,7 +245,7 @@ void WiFiComponent::wifi_loop_() { if (this->state_ == WIFI_COMPONENT_STATE_STA_SCANNING && !cyw43_wifi_scan_active(&cyw43_state)) { this->scan_done_ = true; ESP_LOGV(TAG, "Scan done"); -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_SCAN_RESULTS_LISTENERS for (auto *listener : this->scan_results_listeners_) { listener->on_wifi_scan_results(this->scan_result_); } @@ -263,28 +263,28 @@ void WiFiComponent::wifi_loop_() { // Just connected s_sta_was_connected = true; ESP_LOGV(TAG, "Connected"); -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS String ssid = WiFi.SSID(); bssid_t bssid = this->wifi_bssid(); for (auto *listener : this->connect_state_listeners_) { listener->on_wifi_connect_state(StringRef(ssid.c_str(), ssid.length()), bssid); } +#endif // For static IP configurations, notify IP listeners immediately as the IP is already configured -#ifdef USE_WIFI_MANUAL_IP +#if defined(USE_WIFI_IP_STATE_LISTENERS) && defined(USE_WIFI_MANUAL_IP) if (const WiFiAP *config = this->get_selected_sta_(); config && config->get_manual_ip().has_value()) { s_sta_had_ip = true; for (auto *listener : this->ip_state_listeners_) { listener->on_ip_state(this->wifi_sta_ip_addresses(), this->get_dns_address(0), this->get_dns_address(1)); } } -#endif #endif } else if (!is_connected && s_sta_was_connected) { // Just disconnected s_sta_was_connected = false; s_sta_had_ip = false; ESP_LOGV(TAG, "Disconnected"); -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS static constexpr uint8_t EMPTY_BSSID[6] = {}; for (auto *listener : this->connect_state_listeners_) { listener->on_wifi_connect_state(StringRef(), EMPTY_BSSID); @@ -305,7 +305,7 @@ void WiFiComponent::wifi_loop_() { // Just got IP address s_sta_had_ip = true; ESP_LOGV(TAG, "Got IP address"); -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_IP_STATE_LISTENERS for (auto *listener : this->ip_state_listeners_) { listener->on_ip_state(this->wifi_sta_ip_addresses(), this->get_dns_address(0), this->get_dns_address(1)); } diff --git a/esphome/components/wifi_info/text_sensor.py b/esphome/components/wifi_info/text_sensor.py index 8a7f192367..9ecb5b7490 100644 --- a/esphome/components/wifi_info/text_sensor.py +++ b/esphome/components/wifi_info/text_sensor.py @@ -69,16 +69,6 @@ CONFIG_SCHEMA = cv.Schema( } ) -# Keys that require WiFi listeners -_NETWORK_INFO_KEYS = { - CONF_SSID, - CONF_BSSID, - CONF_IP_ADDRESS, - CONF_DNS_ADDRESS, - CONF_SCAN_RESULTS, - CONF_POWER_SAVE_MODE, -} - async def setup_conf(config, key): if key in config: @@ -88,16 +78,28 @@ async def setup_conf(config, key): async def to_code(config): - # Request WiFi listeners for any sensor that needs them - if _NETWORK_INFO_KEYS.intersection(config): - wifi.request_wifi_listeners() + # Request specific WiFi listeners based on which sensors are configured + # SSID and BSSID use WiFiConnectStateListener + if CONF_SSID in config or CONF_BSSID in config: + wifi.request_wifi_connect_state_listener() + + # IP address and DNS use WiFiIPStateListener + if CONF_IP_ADDRESS in config or CONF_DNS_ADDRESS in config: + wifi.request_wifi_ip_state_listener() + + # Scan results use WiFiScanResultsListener + if CONF_SCAN_RESULTS in config: + wifi.request_wifi_scan_results_listener() + wifi.request_wifi_scan_results() + + # Power save mode uses WiFiPowerSaveListener + if CONF_POWER_SAVE_MODE in config: + wifi.request_wifi_power_save_listener() await setup_conf(config, CONF_SSID) await setup_conf(config, CONF_BSSID) await setup_conf(config, CONF_MAC_ADDRESS) - if CONF_SCAN_RESULTS in config: - await setup_conf(config, CONF_SCAN_RESULTS) - wifi.request_wifi_scan_results() + await setup_conf(config, CONF_SCAN_RESULTS) await setup_conf(config, CONF_DNS_ADDRESS) await setup_conf(config, CONF_POWER_SAVE_MODE) if conf := config.get(CONF_IP_ADDRESS): diff --git a/esphome/components/wifi_info/wifi_info_text_sensor.cpp b/esphome/components/wifi_info/wifi_info_text_sensor.cpp index 2c0e66eeaf..a63b30b892 100644 --- a/esphome/components/wifi_info/wifi_info_text_sensor.cpp +++ b/esphome/components/wifi_info/wifi_info_text_sensor.cpp @@ -10,9 +10,7 @@ namespace esphome::wifi_info { static const char *const TAG = "wifi_info"; -#ifdef USE_WIFI_LISTENERS - -static constexpr size_t MAX_STATE_LENGTH = 255; +#ifdef USE_WIFI_IP_STATE_LISTENERS /******************** * IPAddressWiFiInfo @@ -58,6 +56,10 @@ void DNSAddressWifiInfo::on_ip_state(const network::IPAddresses &ips, const netw this->publish_state(buf); } +#endif // USE_WIFI_IP_STATE_LISTENERS + +#ifdef USE_WIFI_SCAN_RESULTS_LISTENERS + /********************** * ScanResultsWiFiInfo *********************/ @@ -80,9 +82,9 @@ static char *format_scan_entry(char *buf, const char *ssid, size_t ssid_len, int } void ScanResultsWiFiInfo::on_wifi_scan_results(const wifi::wifi_scan_vector_t &results) { - char buf[MAX_STATE_LENGTH + 1]; + char buf[MAX_STATE_LEN + 1]; char *ptr = buf; - const char *end = buf + MAX_STATE_LENGTH; + const char *end = buf + MAX_STATE_LEN; for (const auto &scan : results) { if (scan.get_is_hidden()) @@ -98,6 +100,10 @@ void ScanResultsWiFiInfo::on_wifi_scan_results(const wifi::wifi_scan_vector_tpublish_state(buf); } +#endif // USE_WIFI_SCAN_RESULTS_LISTENERS + +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS + /*************** * SSIDWiFiInfo **************/ @@ -126,6 +132,10 @@ void BSSIDWiFiInfo::on_wifi_connect_state(StringRef ssid, std::spanpublish_state(buf); } +#endif // USE_WIFI_CONNECT_STATE_LISTENERS + +#ifdef USE_WIFI_POWER_SAVE_LISTENERS + /************************ * PowerSaveModeWiFiInfo ***********************/ @@ -182,7 +192,7 @@ void PowerSaveModeWiFiInfo::on_wifi_power_save(wifi::WiFiPowerSaveMode mode) { this->publish_state(mode_str); } -#endif +#endif // USE_WIFI_POWER_SAVE_LISTENERS /********************* * MacAddressWifiInfo diff --git a/esphome/components/wifi_info/wifi_info_text_sensor.h b/esphome/components/wifi_info/wifi_info_text_sensor.h index 6beb1372f5..8ef35a5f5d 100644 --- a/esphome/components/wifi_info/wifi_info_text_sensor.h +++ b/esphome/components/wifi_info/wifi_info_text_sensor.h @@ -11,7 +11,7 @@ namespace esphome::wifi_info { -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_IP_STATE_LISTENERS class IPAddressWiFiInfo final : public Component, public text_sensor::TextSensor, public wifi::WiFiIPStateListener { public: void setup() override; @@ -35,7 +35,9 @@ class DNSAddressWifiInfo final : public Component, public text_sensor::TextSenso void on_ip_state(const network::IPAddresses &ips, const network::IPAddress &dns1, const network::IPAddress &dns2) override; }; +#endif // USE_WIFI_IP_STATE_LISTENERS +#ifdef USE_WIFI_SCAN_RESULTS_LISTENERS class ScanResultsWiFiInfo final : public Component, public text_sensor::TextSensor, public wifi::WiFiScanResultsListener { @@ -47,7 +49,9 @@ class ScanResultsWiFiInfo final : public Component, // WiFiScanResultsListener interface void on_wifi_scan_results(const wifi::wifi_scan_vector_t &results) override; }; +#endif // USE_WIFI_SCAN_RESULTS_LISTENERS +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS class SSIDWiFiInfo final : public Component, public text_sensor::TextSensor, public wifi::WiFiConnectStateListener { public: void setup() override; @@ -65,7 +69,9 @@ class BSSIDWiFiInfo final : public Component, public text_sensor::TextSensor, pu // WiFiConnectStateListener interface void on_wifi_connect_state(StringRef ssid, std::span bssid) override; }; +#endif // USE_WIFI_CONNECT_STATE_LISTENERS +#ifdef USE_WIFI_POWER_SAVE_LISTENERS class PowerSaveModeWiFiInfo final : public Component, public text_sensor::TextSensor, public wifi::WiFiPowerSaveListener { @@ -76,7 +82,7 @@ class PowerSaveModeWiFiInfo final : public Component, // WiFiPowerSaveListener interface void on_wifi_power_save(wifi::WiFiPowerSaveMode mode) override; }; -#endif +#endif // USE_WIFI_POWER_SAVE_LISTENERS class MacAddressWifiInfo final : public Component, public text_sensor::TextSensor { public: diff --git a/esphome/components/wifi_signal/sensor.py b/esphome/components/wifi_signal/sensor.py index 82cb90c745..075cfd96c6 100644 --- a/esphome/components/wifi_signal/sensor.py +++ b/esphome/components/wifi_signal/sensor.py @@ -25,6 +25,6 @@ CONFIG_SCHEMA = sensor.sensor_schema( async def to_code(config): - wifi.request_wifi_listeners() + wifi.request_wifi_connect_state_listener() var = await sensor.new_sensor(config) await cg.register_component(var, config) diff --git a/esphome/components/wifi_signal/wifi_signal_sensor.h b/esphome/components/wifi_signal/wifi_signal_sensor.h index 2e1f8cbb2b..9ff4cc54a0 100644 --- a/esphome/components/wifi_signal/wifi_signal_sensor.h +++ b/esphome/components/wifi_signal/wifi_signal_sensor.h @@ -9,13 +9,13 @@ #include namespace esphome::wifi_signal { -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS class WiFiSignalSensor : public sensor::Sensor, public PollingComponent, public wifi::WiFiConnectStateListener { #else class WiFiSignalSensor : public sensor::Sensor, public PollingComponent { #endif public: -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS void setup() override { wifi::global_wifi_component->add_connect_state_listener(this); } #endif void update() override { @@ -28,7 +28,7 @@ class WiFiSignalSensor : public sensor::Sensor, public PollingComponent { float get_setup_priority() const override { return setup_priority::AFTER_WIFI; } -#ifdef USE_WIFI_LISTENERS +#ifdef USE_WIFI_CONNECT_STATE_LISTENERS // WiFiConnectStateListener interface - update RSSI immediately on connect void on_wifi_connect_state(StringRef ssid, std::span bssid) override { this->update(); } #endif diff --git a/esphome/core/defines.h b/esphome/core/defines.h index 3cc48c6008..673397fa31 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -222,7 +222,14 @@ #define USE_CAPTIVE_PORTAL_GZIP #define USE_WIFI_11KV_SUPPORT #define USE_WIFI_FAST_CONNECT -#define USE_WIFI_LISTENERS +#define USE_WIFI_IP_STATE_LISTENERS +#define USE_WIFI_SCAN_RESULTS_LISTENERS +#define USE_WIFI_CONNECT_STATE_LISTENERS +#define USE_WIFI_POWER_SAVE_LISTENERS +#define ESPHOME_WIFI_IP_STATE_LISTENERS 2 +#define ESPHOME_WIFI_SCAN_RESULTS_LISTENERS 2 +#define ESPHOME_WIFI_CONNECT_STATE_LISTENERS 2 +#define ESPHOME_WIFI_POWER_SAVE_LISTENERS 2 #define USE_WIFI_RUNTIME_POWER_SAVE #define USB_HOST_MAX_REQUESTS 16 From 8b49d465f80c51aa1c6be09064327a874bff569b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 17:44:43 -1000 Subject: [PATCH 02/16] [bh1750] Eliminate heap allocations by replacing callbacks with state machine (#11950) --- esphome/components/bh1750/bh1750.cpp | 306 ++++++++++++++++++--------- esphome/components/bh1750/bh1750.h | 34 ++- 2 files changed, 232 insertions(+), 108 deletions(-) diff --git a/esphome/components/bh1750/bh1750.cpp b/esphome/components/bh1750/bh1750.cpp index 2fc476c17d..bd7c667c25 100644 --- a/esphome/components/bh1750/bh1750.cpp +++ b/esphome/components/bh1750/bh1750.cpp @@ -1,8 +1,8 @@ #include "bh1750.h" #include "esphome/core/log.h" +#include "esphome/core/application.h" -namespace esphome { -namespace bh1750 { +namespace esphome::bh1750 { static const char *const TAG = "bh1750.sensor"; @@ -13,6 +13,31 @@ static const uint8_t BH1750_COMMAND_ONE_TIME_L = 0b00100011; static const uint8_t BH1750_COMMAND_ONE_TIME_H = 0b00100000; static const uint8_t BH1750_COMMAND_ONE_TIME_H2 = 0b00100001; +static constexpr uint32_t MEASUREMENT_TIMEOUT_MS = 2000; +static constexpr float HIGH_LIGHT_THRESHOLD_LX = 7000.0f; + +// Measurement time constants (datasheet values) +static constexpr uint16_t MTREG_DEFAULT = 69; +static constexpr uint16_t MTREG_MIN = 31; +static constexpr uint16_t MTREG_MAX = 254; +static constexpr uint16_t MEAS_TIME_L_MS = 24; // L-resolution max measurement time @ mtreg=69 +static constexpr uint16_t MEAS_TIME_H_MS = 180; // H/H2-resolution max measurement time @ mtreg=69 + +// Conversion constants (datasheet formulas) +static constexpr float RESOLUTION_DIVISOR = 1.2f; // counts to lux conversion divisor +static constexpr float MODE_H2_DIVISOR = 2.0f; // H2 mode has 2x higher resolution + +// MTreg calculation constants +static constexpr int COUNTS_TARGET = 50000; // Target counts for optimal range (avoid saturation) +static constexpr int COUNTS_NUMERATOR = 10; +static constexpr int COUNTS_DENOMINATOR = 12; + +// MTreg register bit manipulation constants +static constexpr uint8_t MTREG_HI_SHIFT = 5; // High 3 bits start at bit 5 +static constexpr uint8_t MTREG_HI_MASK = 0b111; // 3-bit mask for high bits +static constexpr uint8_t MTREG_LO_SHIFT = 0; // Low 5 bits start at bit 0 +static constexpr uint8_t MTREG_LO_MASK = 0b11111; // 5-bit mask for low bits + /* bh1750 properties: @@ -43,74 +68,7 @@ void BH1750Sensor::setup() { this->mark_failed(); return; } -} - -void BH1750Sensor::read_lx_(BH1750Mode mode, uint8_t mtreg, const std::function &f) { - // turn on (after one-shot sensor automatically powers down) - uint8_t turn_on = BH1750_COMMAND_POWER_ON; - if (this->write(&turn_on, 1) != i2c::ERROR_OK) { - ESP_LOGW(TAG, "Power on failed"); - f(NAN); - return; - } - - if (active_mtreg_ != mtreg) { - // set mtreg - uint8_t mtreg_hi = BH1750_COMMAND_MT_REG_HI | ((mtreg >> 5) & 0b111); - uint8_t mtreg_lo = BH1750_COMMAND_MT_REG_LO | ((mtreg >> 0) & 0b11111); - if (this->write(&mtreg_hi, 1) != i2c::ERROR_OK || this->write(&mtreg_lo, 1) != i2c::ERROR_OK) { - ESP_LOGW(TAG, "Set measurement time failed"); - active_mtreg_ = 0; - f(NAN); - return; - } - active_mtreg_ = mtreg; - } - - uint8_t cmd; - uint16_t meas_time; - switch (mode) { - case BH1750_MODE_L: - cmd = BH1750_COMMAND_ONE_TIME_L; - meas_time = 24 * mtreg / 69; - break; - case BH1750_MODE_H: - cmd = BH1750_COMMAND_ONE_TIME_H; - meas_time = 180 * mtreg / 69; - break; - case BH1750_MODE_H2: - cmd = BH1750_COMMAND_ONE_TIME_H2; - meas_time = 180 * mtreg / 69; - break; - default: - f(NAN); - return; - } - if (this->write(&cmd, 1) != i2c::ERROR_OK) { - ESP_LOGW(TAG, "Start measurement failed"); - f(NAN); - return; - } - - // probably not needed, but adjust for rounding - meas_time++; - - this->set_timeout("read", meas_time, [this, mode, mtreg, f]() { - uint16_t raw_value; - if (this->read(reinterpret_cast(&raw_value), 2) != i2c::ERROR_OK) { - ESP_LOGW(TAG, "Read data failed"); - f(NAN); - return; - } - raw_value = i2c::i2ctohs(raw_value); - - float lx = float(raw_value) / 1.2f; - lx *= 69.0f / mtreg; - if (mode == BH1750_MODE_H2) - lx /= 2.0f; - - f(lx); - }); + this->state_ = IDLE; } void BH1750Sensor::dump_config() { @@ -124,45 +82,189 @@ void BH1750Sensor::dump_config() { } void BH1750Sensor::update() { - // first do a quick measurement in L-mode with full range - // to find right range - this->read_lx_(BH1750_MODE_L, 31, [this](float val) { - if (std::isnan(val)) { - this->status_set_warning(); - this->publish_state(NAN); + const uint32_t now = millis(); + + // Start coarse measurement to determine optimal mode/mtreg + if (this->state_ != IDLE) { + // Safety timeout: reset if stuck + if (now - this->measurement_start_time_ > MEASUREMENT_TIMEOUT_MS) { + ESP_LOGW(TAG, "Measurement timeout, resetting state"); + this->state_ = IDLE; + } else { + ESP_LOGW(TAG, "Previous measurement not complete, skipping update"); return; } + } - BH1750Mode use_mode; - uint8_t use_mtreg; - if (val <= 7000) { - use_mode = BH1750_MODE_H2; - use_mtreg = 254; - } else { - use_mode = BH1750_MODE_H; - // lx = counts / 1.2 * (69 / mtreg) - // -> mtreg = counts / 1.2 * (69 / lx) - // calculate for counts=50000 (allow some range to not saturate, but maximize mtreg) - // -> mtreg = 50000*(10/12)*(69/lx) - int ideal_mtreg = 50000 * 10 * 69 / (12 * (int) val); - use_mtreg = std::min(254, std::max(31, ideal_mtreg)); - } - ESP_LOGV(TAG, "L result: %f -> Calculated mode=%d, mtreg=%d", val, (int) use_mode, use_mtreg); + if (!this->start_measurement_(BH1750_MODE_L, MTREG_MIN, now)) { + this->status_set_warning(); + this->publish_state(NAN); + return; + } - this->read_lx_(use_mode, use_mtreg, [this](float val) { - if (std::isnan(val)) { - this->status_set_warning(); - this->publish_state(NAN); - return; + this->state_ = WAITING_COARSE_MEASUREMENT; + this->enable_loop(); // Enable loop while measurement in progress +} + +void BH1750Sensor::loop() { + const uint32_t now = App.get_loop_component_start_time(); + + switch (this->state_) { + case IDLE: + // Disable loop when idle to save cycles + this->disable_loop(); + break; + + case WAITING_COARSE_MEASUREMENT: + if (now - this->measurement_start_time_ >= this->measurement_duration_) { + this->state_ = READING_COARSE_RESULT; } - ESP_LOGD(TAG, "'%s': Illuminance=%.1flx", this->get_name().c_str(), val); + break; + + case READING_COARSE_RESULT: { + float lx; + if (!this->read_measurement_(lx)) { + this->fail_and_reset_(); + break; + } + + this->process_coarse_result_(lx); + + // Start fine measurement with optimal settings + // fetch millis() again since the read can take a bit + if (!this->start_measurement_(this->fine_mode_, this->fine_mtreg_, millis())) { + this->fail_and_reset_(); + break; + } + + this->state_ = WAITING_FINE_MEASUREMENT; + break; + } + + case WAITING_FINE_MEASUREMENT: + if (now - this->measurement_start_time_ >= this->measurement_duration_) { + this->state_ = READING_FINE_RESULT; + } + break; + + case READING_FINE_RESULT: { + float lx; + if (!this->read_measurement_(lx)) { + this->fail_and_reset_(); + break; + } + + ESP_LOGD(TAG, "'%s': Illuminance=%.1flx", this->get_name().c_str(), lx); this->status_clear_warning(); - this->publish_state(val); - }); - }); + this->publish_state(lx); + this->state_ = IDLE; + break; + } + } +} + +bool BH1750Sensor::start_measurement_(BH1750Mode mode, uint8_t mtreg, uint32_t now) { + // Power on + uint8_t turn_on = BH1750_COMMAND_POWER_ON; + if (this->write(&turn_on, 1) != i2c::ERROR_OK) { + ESP_LOGW(TAG, "Power on failed"); + return false; + } + + // Set MTreg if changed + if (this->active_mtreg_ != mtreg) { + uint8_t mtreg_hi = BH1750_COMMAND_MT_REG_HI | ((mtreg >> MTREG_HI_SHIFT) & MTREG_HI_MASK); + uint8_t mtreg_lo = BH1750_COMMAND_MT_REG_LO | ((mtreg >> MTREG_LO_SHIFT) & MTREG_LO_MASK); + if (this->write(&mtreg_hi, 1) != i2c::ERROR_OK || this->write(&mtreg_lo, 1) != i2c::ERROR_OK) { + ESP_LOGW(TAG, "Set measurement time failed"); + this->active_mtreg_ = 0; + return false; + } + this->active_mtreg_ = mtreg; + } + + // Start measurement + uint8_t cmd; + uint16_t meas_time; + switch (mode) { + case BH1750_MODE_L: + cmd = BH1750_COMMAND_ONE_TIME_L; + meas_time = MEAS_TIME_L_MS * mtreg / MTREG_DEFAULT; + break; + case BH1750_MODE_H: + cmd = BH1750_COMMAND_ONE_TIME_H; + meas_time = MEAS_TIME_H_MS * mtreg / MTREG_DEFAULT; + break; + case BH1750_MODE_H2: + cmd = BH1750_COMMAND_ONE_TIME_H2; + meas_time = MEAS_TIME_H_MS * mtreg / MTREG_DEFAULT; + break; + default: + return false; + } + + if (this->write(&cmd, 1) != i2c::ERROR_OK) { + ESP_LOGW(TAG, "Start measurement failed"); + return false; + } + + // Store current measurement parameters + this->current_mode_ = mode; + this->current_mtreg_ = mtreg; + this->measurement_start_time_ = now; + this->measurement_duration_ = meas_time + 1; // Add 1ms for safety + + return true; +} + +bool BH1750Sensor::read_measurement_(float &lx_out) { + uint16_t raw_value; + if (this->read(reinterpret_cast(&raw_value), 2) != i2c::ERROR_OK) { + ESP_LOGW(TAG, "Read data failed"); + return false; + } + raw_value = i2c::i2ctohs(raw_value); + + float lx = float(raw_value) / RESOLUTION_DIVISOR; + lx *= float(MTREG_DEFAULT) / this->current_mtreg_; + if (this->current_mode_ == BH1750_MODE_H2) { + lx /= MODE_H2_DIVISOR; + } + + lx_out = lx; + return true; +} + +void BH1750Sensor::process_coarse_result_(float lx) { + if (std::isnan(lx)) { + // Use defaults if coarse measurement failed + this->fine_mode_ = BH1750_MODE_H2; + this->fine_mtreg_ = MTREG_MAX; + return; + } + + if (lx <= HIGH_LIGHT_THRESHOLD_LX) { + this->fine_mode_ = BH1750_MODE_H2; + this->fine_mtreg_ = MTREG_MAX; + } else { + this->fine_mode_ = BH1750_MODE_H; + // lx = counts / 1.2 * (69 / mtreg) + // -> mtreg = counts / 1.2 * (69 / lx) + // calculate for counts=50000 (allow some range to not saturate, but maximize mtreg) + // -> mtreg = 50000*(10/12)*(69/lx) + int ideal_mtreg = COUNTS_TARGET * COUNTS_NUMERATOR * MTREG_DEFAULT / (COUNTS_DENOMINATOR * (int) lx); + this->fine_mtreg_ = std::min((int) MTREG_MAX, std::max((int) MTREG_MIN, ideal_mtreg)); + } + + ESP_LOGV(TAG, "L result: %.1f -> Calculated mode=%d, mtreg=%d", lx, (int) this->fine_mode_, this->fine_mtreg_); +} + +void BH1750Sensor::fail_and_reset_() { + this->status_set_warning(); + this->publish_state(NAN); + this->state_ = IDLE; } float BH1750Sensor::get_setup_priority() const { return setup_priority::DATA; } -} // namespace bh1750 -} // namespace esphome +} // namespace esphome::bh1750 diff --git a/esphome/components/bh1750/bh1750.h b/esphome/components/bh1750/bh1750.h index a31eb33609..0460427954 100644 --- a/esphome/components/bh1750/bh1750.h +++ b/esphome/components/bh1750/bh1750.h @@ -4,10 +4,9 @@ #include "esphome/components/sensor/sensor.h" #include "esphome/components/i2c/i2c.h" -namespace esphome { -namespace bh1750 { +namespace esphome::bh1750 { -enum BH1750Mode { +enum BH1750Mode : uint8_t { BH1750_MODE_L, BH1750_MODE_H, BH1750_MODE_H2, @@ -21,13 +20,36 @@ class BH1750Sensor : public sensor::Sensor, public PollingComponent, public i2c: void setup() override; void dump_config() override; void update() override; + void loop() override; float get_setup_priority() const override; protected: - void read_lx_(BH1750Mode mode, uint8_t mtreg, const std::function &f); + // State machine states + enum State : uint8_t { + IDLE, + WAITING_COARSE_MEASUREMENT, + READING_COARSE_RESULT, + WAITING_FINE_MEASUREMENT, + READING_FINE_RESULT, + }; + // 4-byte aligned members + uint32_t measurement_start_time_{0}; + uint32_t measurement_duration_{0}; + + // 1-byte members grouped together to minimize padding + State state_{IDLE}; + BH1750Mode current_mode_{BH1750_MODE_L}; + uint8_t current_mtreg_{31}; + BH1750Mode fine_mode_{BH1750_MODE_H2}; + uint8_t fine_mtreg_{254}; uint8_t active_mtreg_{0}; + + // Helper methods + bool start_measurement_(BH1750Mode mode, uint8_t mtreg, uint32_t now); + bool read_measurement_(float &lx_out); + void process_coarse_result_(float lx); + void fail_and_reset_(); }; -} // namespace bh1750 -} // namespace esphome +} // namespace esphome::bh1750 From c8cc29a9913bef814fed5cb036eaca2f9c8879a2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 17:58:06 -1000 Subject: [PATCH 03/16] [api] Reduce batch RAM usage by 33% via switch dispatch (#13199) --- esphome/components/api/api_connection.cpp | 247 +++++++++++++++------- esphome/components/api/api_connection.h | 117 +++------- esphome/components/api/api_server.cpp | 7 +- esphome/components/api/list_entities.h | 5 +- esphome/components/event/event.h | 19 ++ 5 files changed, 229 insertions(+), 166 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index ea18d06511..0804985cc5 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -265,8 +265,7 @@ void APIConnection::loop() { // If we can't send the ping request directly (tx_buffer full), // schedule it at the front of the batch so it will be sent with priority ESP_LOGW(TAG, "Buffer full, ping queued"); - this->schedule_message_front_(nullptr, &APIConnection::try_send_ping_request, PingRequest::MESSAGE_TYPE, - PingRequest::ESTIMATED_SIZE); + this->schedule_message_front_(nullptr, PingRequest::MESSAGE_TYPE, PingRequest::ESTIMATED_SIZE); this->flags_.sent_ping = true; // Mark as sent to avoid scheduling multiple pings } } @@ -362,8 +361,8 @@ uint16_t APIConnection::encode_message_to_buffer(ProtoMessage &msg, uint8_t mess #ifdef USE_BINARY_SENSOR bool APIConnection::send_binary_sensor_state(binary_sensor::BinarySensor *binary_sensor) { - return this->send_message_smart_(binary_sensor, &APIConnection::try_send_binary_sensor_state, - BinarySensorStateResponse::MESSAGE_TYPE, BinarySensorStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(binary_sensor, BinarySensorStateResponse::MESSAGE_TYPE, + BinarySensorStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_binary_sensor_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, @@ -389,8 +388,7 @@ uint16_t APIConnection::try_send_binary_sensor_info(EntityBase *entity, APIConne #ifdef USE_COVER bool APIConnection::send_cover_state(cover::Cover *cover) { - return this->send_message_smart_(cover, &APIConnection::try_send_cover_state, CoverStateResponse::MESSAGE_TYPE, - CoverStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(cover, CoverStateResponse::MESSAGE_TYPE, CoverStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_cover_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, bool is_single) { @@ -430,8 +428,7 @@ void APIConnection::cover_command(const CoverCommandRequest &msg) { #ifdef USE_FAN bool APIConnection::send_fan_state(fan::Fan *fan) { - return this->send_message_smart_(fan, &APIConnection::try_send_fan_state, FanStateResponse::MESSAGE_TYPE, - FanStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(fan, FanStateResponse::MESSAGE_TYPE, FanStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_fan_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, bool is_single) { @@ -482,8 +479,7 @@ void APIConnection::fan_command(const FanCommandRequest &msg) { #ifdef USE_LIGHT bool APIConnection::send_light_state(light::LightState *light) { - return this->send_message_smart_(light, &APIConnection::try_send_light_state, LightStateResponse::MESSAGE_TYPE, - LightStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(light, LightStateResponse::MESSAGE_TYPE, LightStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_light_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, bool is_single) { @@ -569,8 +565,7 @@ void APIConnection::light_command(const LightCommandRequest &msg) { #ifdef USE_SENSOR bool APIConnection::send_sensor_state(sensor::Sensor *sensor) { - return this->send_message_smart_(sensor, &APIConnection::try_send_sensor_state, SensorStateResponse::MESSAGE_TYPE, - SensorStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(sensor, SensorStateResponse::MESSAGE_TYPE, SensorStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_sensor_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, @@ -598,8 +593,7 @@ uint16_t APIConnection::try_send_sensor_info(EntityBase *entity, APIConnection * #ifdef USE_SWITCH bool APIConnection::send_switch_state(switch_::Switch *a_switch) { - return this->send_message_smart_(a_switch, &APIConnection::try_send_switch_state, SwitchStateResponse::MESSAGE_TYPE, - SwitchStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(a_switch, SwitchStateResponse::MESSAGE_TYPE, SwitchStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_switch_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, @@ -633,8 +627,8 @@ void APIConnection::switch_command(const SwitchCommandRequest &msg) { #ifdef USE_TEXT_SENSOR bool APIConnection::send_text_sensor_state(text_sensor::TextSensor *text_sensor) { - return this->send_message_smart_(text_sensor, &APIConnection::try_send_text_sensor_state, - TextSensorStateResponse::MESSAGE_TYPE, TextSensorStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(text_sensor, TextSensorStateResponse::MESSAGE_TYPE, + TextSensorStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_text_sensor_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, @@ -658,8 +652,7 @@ uint16_t APIConnection::try_send_text_sensor_info(EntityBase *entity, APIConnect #ifdef USE_CLIMATE bool APIConnection::send_climate_state(climate::Climate *climate) { - return this->send_message_smart_(climate, &APIConnection::try_send_climate_state, ClimateStateResponse::MESSAGE_TYPE, - ClimateStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(climate, ClimateStateResponse::MESSAGE_TYPE, ClimateStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_climate_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, bool is_single) { @@ -754,8 +747,7 @@ void APIConnection::climate_command(const ClimateCommandRequest &msg) { #ifdef USE_NUMBER bool APIConnection::send_number_state(number::Number *number) { - return this->send_message_smart_(number, &APIConnection::try_send_number_state, NumberStateResponse::MESSAGE_TYPE, - NumberStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(number, NumberStateResponse::MESSAGE_TYPE, NumberStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_number_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, @@ -789,8 +781,7 @@ void APIConnection::number_command(const NumberCommandRequest &msg) { #ifdef USE_DATETIME_DATE bool APIConnection::send_date_state(datetime::DateEntity *date) { - return this->send_message_smart_(date, &APIConnection::try_send_date_state, DateStateResponse::MESSAGE_TYPE, - DateStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(date, DateStateResponse::MESSAGE_TYPE, DateStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_date_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, bool is_single) { @@ -818,8 +809,7 @@ void APIConnection::date_command(const DateCommandRequest &msg) { #ifdef USE_DATETIME_TIME bool APIConnection::send_time_state(datetime::TimeEntity *time) { - return this->send_message_smart_(time, &APIConnection::try_send_time_state, TimeStateResponse::MESSAGE_TYPE, - TimeStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(time, TimeStateResponse::MESSAGE_TYPE, TimeStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_time_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, bool is_single) { @@ -847,8 +837,8 @@ void APIConnection::time_command(const TimeCommandRequest &msg) { #ifdef USE_DATETIME_DATETIME bool APIConnection::send_datetime_state(datetime::DateTimeEntity *datetime) { - return this->send_message_smart_(datetime, &APIConnection::try_send_datetime_state, - DateTimeStateResponse::MESSAGE_TYPE, DateTimeStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(datetime, DateTimeStateResponse::MESSAGE_TYPE, + DateTimeStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_datetime_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, bool is_single) { @@ -878,8 +868,7 @@ void APIConnection::datetime_command(const DateTimeCommandRequest &msg) { #ifdef USE_TEXT bool APIConnection::send_text_state(text::Text *text) { - return this->send_message_smart_(text, &APIConnection::try_send_text_state, TextStateResponse::MESSAGE_TYPE, - TextStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(text, TextStateResponse::MESSAGE_TYPE, TextStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_text_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, @@ -911,8 +900,7 @@ void APIConnection::text_command(const TextCommandRequest &msg) { #ifdef USE_SELECT bool APIConnection::send_select_state(select::Select *select) { - return this->send_message_smart_(select, &APIConnection::try_send_select_state, SelectStateResponse::MESSAGE_TYPE, - SelectStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(select, SelectStateResponse::MESSAGE_TYPE, SelectStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_select_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, @@ -956,8 +944,7 @@ void esphome::api::APIConnection::button_command(const ButtonCommandRequest &msg #ifdef USE_LOCK bool APIConnection::send_lock_state(lock::Lock *a_lock) { - return this->send_message_smart_(a_lock, &APIConnection::try_send_lock_state, LockStateResponse::MESSAGE_TYPE, - LockStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(a_lock, LockStateResponse::MESSAGE_TYPE, LockStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_lock_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, @@ -997,8 +984,7 @@ void APIConnection::lock_command(const LockCommandRequest &msg) { #ifdef USE_VALVE bool APIConnection::send_valve_state(valve::Valve *valve) { - return this->send_message_smart_(valve, &APIConnection::try_send_valve_state, ValveStateResponse::MESSAGE_TYPE, - ValveStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(valve, ValveStateResponse::MESSAGE_TYPE, ValveStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_valve_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, bool is_single) { @@ -1032,8 +1018,8 @@ void APIConnection::valve_command(const ValveCommandRequest &msg) { #ifdef USE_MEDIA_PLAYER bool APIConnection::send_media_player_state(media_player::MediaPlayer *media_player) { - return this->send_message_smart_(media_player, &APIConnection::try_send_media_player_state, - MediaPlayerStateResponse::MESSAGE_TYPE, MediaPlayerStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(media_player, MediaPlayerStateResponse::MESSAGE_TYPE, + MediaPlayerStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_media_player_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, bool is_single) { @@ -1315,8 +1301,7 @@ void APIConnection::zwave_proxy_request(const ZWaveProxyRequest &msg) { #ifdef USE_ALARM_CONTROL_PANEL bool APIConnection::send_alarm_control_panel_state(alarm_control_panel::AlarmControlPanel *a_alarm_control_panel) { - return this->send_message_smart_(a_alarm_control_panel, &APIConnection::try_send_alarm_control_panel_state, - AlarmControlPanelStateResponse::MESSAGE_TYPE, + return this->send_message_smart_(a_alarm_control_panel, AlarmControlPanelStateResponse::MESSAGE_TYPE, AlarmControlPanelStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_alarm_control_panel_state(EntityBase *entity, APIConnection *conn, @@ -1369,8 +1354,8 @@ void APIConnection::alarm_control_panel_command(const AlarmControlPanelCommandRe #ifdef USE_WATER_HEATER bool APIConnection::send_water_heater_state(water_heater::WaterHeater *water_heater) { - return this->send_message_smart_(water_heater, &APIConnection::try_send_water_heater_state, - WaterHeaterStateResponse::MESSAGE_TYPE, WaterHeaterStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(water_heater, WaterHeaterStateResponse::MESSAGE_TYPE, + WaterHeaterStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_water_heater_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, bool is_single) { @@ -1419,10 +1404,11 @@ void APIConnection::on_water_heater_command_request(const WaterHeaterCommandRequ #endif #ifdef USE_EVENT -void APIConnection::send_event(event::Event *event, StringRef event_type) { - // get_last_event_type() returns StringRef pointing to null-terminated string literals from codegen - this->send_message_smart_(event, MessageCreator(event_type.c_str()), EventResponse::MESSAGE_TYPE, - EventResponse::ESTIMATED_SIZE); +// Event is a special case - unlike other entities with simple state fields, +// events store their state in a member accessed via obj->get_last_event_type() +void APIConnection::send_event(event::Event *event) { + this->send_message_smart_(event, EventResponse::MESSAGE_TYPE, EventResponse::ESTIMATED_SIZE, + event->get_last_event_type_index()); } uint16_t APIConnection::try_send_event_response(event::Event *event, StringRef event_type, APIConnection *conn, uint32_t remaining_size, bool is_single) { @@ -1473,8 +1459,7 @@ uint16_t APIConnection::try_send_infrared_info(EntityBase *entity, APIConnection #ifdef USE_UPDATE bool APIConnection::send_update_state(update::UpdateEntity *update) { - return this->send_message_smart_(update, &APIConnection::try_send_update_state, UpdateStateResponse::MESSAGE_TYPE, - UpdateStateResponse::ESTIMATED_SIZE); + return this->send_message_smart_(update, UpdateStateResponse::MESSAGE_TYPE, UpdateStateResponse::ESTIMATED_SIZE); } uint16_t APIConnection::try_send_update_state(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, bool is_single) { @@ -1897,30 +1882,31 @@ void APIConnection::on_fatal_error() { this->flags_.remove = true; } -void APIConnection::DeferredBatch::add_item(EntityBase *entity, MessageCreator creator, uint8_t message_type, - uint8_t estimated_size) { +void APIConnection::DeferredBatch::add_item(EntityBase *entity, uint8_t message_type, uint8_t estimated_size, + uint8_t aux_data_index) { // Check if we already have a message of this type for this entity // This provides deduplication per entity/message_type combination // O(n) but optimized for RAM and not performance. - for (auto &item : items) { - if (item.entity == entity && item.message_type == message_type) { - // Replace with new creator - item.creator = creator; - return; + // Skip deduplication for events - they are edge-triggered, every occurrence matters +#ifdef USE_EVENT + if (message_type != EventResponse::MESSAGE_TYPE) +#endif + { + for (const auto &item : items) { + if (item.entity == entity && item.message_type == message_type) + return; // Already queued } } - - // No existing item found, add new one - items.emplace_back(entity, creator, message_type, estimated_size); + // No existing item found (or event), add new one + items.push_back({entity, message_type, estimated_size, aux_data_index}); } -void APIConnection::DeferredBatch::add_item_front(EntityBase *entity, MessageCreator creator, uint8_t message_type, - uint8_t estimated_size) { +void APIConnection::DeferredBatch::add_item_front(EntityBase *entity, uint8_t message_type, uint8_t estimated_size) { // Add high priority message and swap to front // This avoids expensive vector::insert which shifts all elements // Note: We only ever have one high-priority message at a time (ping OR disconnect) // If we're disconnecting, pings are blocked, so this simple swap is sufficient - items.emplace_back(entity, creator, message_type, estimated_size); + items.push_back({entity, message_type, estimated_size, AUX_DATA_UNUSED}); if (items.size() > 1) { // Swap the new high-priority item to the front std::swap(items.front(), items.back()); @@ -1959,19 +1945,17 @@ void APIConnection::process_batch_() { if (num_items == 1) { const auto &item = this->deferred_batch_[0]; - // Let the creator calculate size and encode if it fits - uint16_t payload_size = - item.creator(item.entity, this, std::numeric_limits::max(), true, item.message_type); + // Let dispatch_message_ calculate size and encode if it fits + uint16_t payload_size = this->dispatch_message_(item, std::numeric_limits::max(), true); if (payload_size > 0 && this->send_buffer(ProtoWriteBuffer{&shared_buf}, item.message_type)) { #ifdef HAS_PROTO_MESSAGE_DUMP - // Log messages after send attempt for VV debugging - // It's safe to use the buffer for logging at this point regardless of send result + // Log message after send attempt for VV debugging this->log_batch_item_(item); #endif this->clear_batch_(); } else if (payload_size == 0) { - // Message too large + // Message too large to fit in available space ESP_LOGW(TAG, "Message too large to send: type=%u", item.message_type); this->clear_batch_(); } @@ -2016,9 +2000,9 @@ void APIConnection::process_batch_() { // Process items and encode directly to buffer (up to our limit) for (size_t i = 0; i < messages_to_process; i++) { const auto &item = this->deferred_batch_[i]; - // Try to encode message - // The creator will calculate overhead to determine if the message fits - uint16_t payload_size = item.creator(item.entity, this, remaining_size, false, item.message_type); + // Try to encode message via dispatch + // The dispatch function calculates overhead to determine if the message fits + uint16_t payload_size = this->dispatch_message_(item, remaining_size, false); if (payload_size == 0) { // Message won't fit, stop processing @@ -2084,18 +2068,129 @@ void APIConnection::process_batch_() { } } -uint16_t APIConnection::MessageCreator::operator()(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, - bool is_single, uint8_t message_type) const { +// Dispatch message encoding based on message_type +// Switch assigns function pointer, single call site for smaller code size +uint16_t APIConnection::dispatch_message_(const DeferredBatch::BatchItem &item, uint32_t remaining_size, + bool is_single) { #ifdef USE_EVENT - // Special case: EventResponse uses const char * pointer - if (message_type == EventResponse::MESSAGE_TYPE) { - auto *e = static_cast(entity); - return APIConnection::try_send_event_response(e, StringRef(data_.const_char_ptr), conn, remaining_size, is_single); + // Events need aux_data_index to look up event type from entity + if (item.message_type == EventResponse::MESSAGE_TYPE) { + // Skip if aux_data_index is invalid (should never happen in normal operation) + if (item.aux_data_index == DeferredBatch::AUX_DATA_UNUSED) + return 0; + auto *event = static_cast(item.entity); + return try_send_event_response(event, StringRef::from_maybe_nullptr(event->get_event_type(item.aux_data_index)), + this, remaining_size, is_single); } #endif - // All other message types use function pointers - return data_.function_ptr(entity, conn, remaining_size, is_single); + // All other message types use function pointer lookup via switch + MessageCreatorPtr func = nullptr; + +// Macros to reduce repetitive switch cases +#define CASE_STATE_INFO(entity_name, StateResp, InfoResp) \ + case StateResp::MESSAGE_TYPE: \ + func = &try_send_##entity_name##_state; \ + break; \ + case InfoResp::MESSAGE_TYPE: \ + func = &try_send_##entity_name##_info; \ + break; +#define CASE_INFO_ONLY(entity_name, InfoResp) \ + case InfoResp::MESSAGE_TYPE: \ + func = &try_send_##entity_name##_info; \ + break; + + switch (item.message_type) { +#ifdef USE_BINARY_SENSOR + CASE_STATE_INFO(binary_sensor, BinarySensorStateResponse, ListEntitiesBinarySensorResponse) +#endif +#ifdef USE_COVER + CASE_STATE_INFO(cover, CoverStateResponse, ListEntitiesCoverResponse) +#endif +#ifdef USE_FAN + CASE_STATE_INFO(fan, FanStateResponse, ListEntitiesFanResponse) +#endif +#ifdef USE_LIGHT + CASE_STATE_INFO(light, LightStateResponse, ListEntitiesLightResponse) +#endif +#ifdef USE_SENSOR + CASE_STATE_INFO(sensor, SensorStateResponse, ListEntitiesSensorResponse) +#endif +#ifdef USE_SWITCH + CASE_STATE_INFO(switch, SwitchStateResponse, ListEntitiesSwitchResponse) +#endif +#ifdef USE_BUTTON + CASE_INFO_ONLY(button, ListEntitiesButtonResponse) +#endif +#ifdef USE_TEXT_SENSOR + CASE_STATE_INFO(text_sensor, TextSensorStateResponse, ListEntitiesTextSensorResponse) +#endif +#ifdef USE_CLIMATE + CASE_STATE_INFO(climate, ClimateStateResponse, ListEntitiesClimateResponse) +#endif +#ifdef USE_NUMBER + CASE_STATE_INFO(number, NumberStateResponse, ListEntitiesNumberResponse) +#endif +#ifdef USE_DATETIME_DATE + CASE_STATE_INFO(date, DateStateResponse, ListEntitiesDateResponse) +#endif +#ifdef USE_DATETIME_TIME + CASE_STATE_INFO(time, TimeStateResponse, ListEntitiesTimeResponse) +#endif +#ifdef USE_DATETIME_DATETIME + CASE_STATE_INFO(datetime, DateTimeStateResponse, ListEntitiesDateTimeResponse) +#endif +#ifdef USE_TEXT + CASE_STATE_INFO(text, TextStateResponse, ListEntitiesTextResponse) +#endif +#ifdef USE_SELECT + CASE_STATE_INFO(select, SelectStateResponse, ListEntitiesSelectResponse) +#endif +#ifdef USE_LOCK + CASE_STATE_INFO(lock, LockStateResponse, ListEntitiesLockResponse) +#endif +#ifdef USE_VALVE + CASE_STATE_INFO(valve, ValveStateResponse, ListEntitiesValveResponse) +#endif +#ifdef USE_MEDIA_PLAYER + CASE_STATE_INFO(media_player, MediaPlayerStateResponse, ListEntitiesMediaPlayerResponse) +#endif +#ifdef USE_ALARM_CONTROL_PANEL + CASE_STATE_INFO(alarm_control_panel, AlarmControlPanelStateResponse, ListEntitiesAlarmControlPanelResponse) +#endif +#ifdef USE_WATER_HEATER + CASE_STATE_INFO(water_heater, WaterHeaterStateResponse, ListEntitiesWaterHeaterResponse) +#endif +#ifdef USE_CAMERA + CASE_INFO_ONLY(camera, ListEntitiesCameraResponse) +#endif +#ifdef USE_INFRARED + CASE_INFO_ONLY(infrared, ListEntitiesInfraredResponse) +#endif +#ifdef USE_EVENT + CASE_INFO_ONLY(event, ListEntitiesEventResponse) +#endif +#ifdef USE_UPDATE + CASE_STATE_INFO(update, UpdateStateResponse, ListEntitiesUpdateResponse) +#endif + // Special messages (not entity state/info) + case ListEntitiesDoneResponse::MESSAGE_TYPE: + func = &try_send_list_info_done; + break; + case DisconnectRequest::MESSAGE_TYPE: + func = &try_send_disconnect_request; + break; + case PingRequest::MESSAGE_TYPE: + func = &try_send_ping_request; + break; + default: + return 0; + } + +#undef CASE_STATE_INFO +#undef CASE_INFO_ONLY + + return func(item.entity, this, remaining_size, is_single); } uint16_t APIConnection::try_send_list_info_done(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, diff --git a/esphome/components/api/api_connection.h b/esphome/components/api/api_connection.h index b3d072ff69..21bf4c4073 100644 --- a/esphome/components/api/api_connection.h +++ b/esphome/components/api/api_connection.h @@ -12,6 +12,7 @@ #include "esphome/core/string_ref.h" #include +#include #include namespace esphome::api { @@ -38,8 +39,8 @@ class APIConnection final : public APIServerConnection { void loop(); bool send_list_info_done() { - return this->schedule_message_(nullptr, &APIConnection::try_send_list_info_done, - ListEntitiesDoneResponse::MESSAGE_TYPE, ListEntitiesDoneResponse::ESTIMATED_SIZE); + return this->schedule_message_(nullptr, ListEntitiesDoneResponse::MESSAGE_TYPE, + ListEntitiesDoneResponse::ESTIMATED_SIZE); } #ifdef USE_BINARY_SENSOR bool send_binary_sensor_state(binary_sensor::BinarySensor *binary_sensor); @@ -178,7 +179,7 @@ class APIConnection final : public APIServerConnection { #endif #ifdef USE_EVENT - void send_event(event::Event *event, StringRef event_type); + void send_event(event::Event *event); #endif #ifdef USE_UPDATE @@ -540,33 +541,17 @@ class APIConnection final : public APIServerConnection { // Function pointer type for message encoding using MessageCreatorPtr = uint16_t (*)(EntityBase *, APIConnection *, uint32_t remaining_size, bool is_single); - class MessageCreator { - public: - MessageCreator(MessageCreatorPtr ptr) { data_.function_ptr = ptr; } - explicit MessageCreator(const char *str_value) { data_.const_char_ptr = str_value; } - - // Call operator - uses message_type to determine union type - uint16_t operator()(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, bool is_single, - uint8_t message_type) const; - - private: - union Data { - MessageCreatorPtr function_ptr; - const char *const_char_ptr; - } data_; // 4 bytes on 32-bit, 8 bytes on 64-bit - }; - // Generic batching mechanism for both state updates and entity info struct DeferredBatch { - struct BatchItem { - EntityBase *entity; // Entity pointer - MessageCreator creator; // Function that creates the message when needed - uint8_t message_type; // Message type for overhead calculation (max 255) - uint8_t estimated_size; // Estimated message size (max 255 bytes) + // Sentinel value for unused aux_data_index + static constexpr uint8_t AUX_DATA_UNUSED = std::numeric_limits::max(); - // Constructor for creating BatchItem - BatchItem(EntityBase *entity, MessageCreator creator, uint8_t message_type, uint8_t estimated_size) - : entity(entity), creator(creator), message_type(message_type), estimated_size(estimated_size) {} + struct BatchItem { + EntityBase *entity; // 4 bytes - Entity pointer + uint8_t message_type; // 1 byte - Message type for protocol and dispatch + uint8_t estimated_size; // 1 byte - Estimated message size (max 255 bytes) + uint8_t aux_data_index{AUX_DATA_UNUSED}; // 1 byte - For events: index into entity's event_types + // 1 byte padding }; std::vector items; @@ -575,10 +560,11 @@ class APIConnection final : public APIServerConnection { // No pre-allocation - log connections never use batching, and for // connections that do, buffers are released after initial sync anyway - // Add item to the batch - void add_item(EntityBase *entity, MessageCreator creator, uint8_t message_type, uint8_t estimated_size); + // Add item to the batch (with deduplication) + void add_item(EntityBase *entity, uint8_t message_type, uint8_t estimated_size, + uint8_t aux_data_index = AUX_DATA_UNUSED); // Add item to the front of the batch (for high priority messages like ping) - void add_item_front(EntityBase *entity, MessageCreator creator, uint8_t message_type, uint8_t estimated_size); + void add_item_front(EntityBase *entity, uint8_t message_type, uint8_t estimated_size); // Clear all items void clear() { @@ -592,6 +578,7 @@ class APIConnection final : public APIServerConnection { bool empty() const { return items.empty(); } size_t size() const { return items.size(); } const BatchItem &operator[](size_t index) const { return items[index]; } + // Release excess capacity - only releases if items already empty void release_buffer() { // Safe to call: batch is processed before release_buffer is called, @@ -663,17 +650,15 @@ class APIConnection final : public APIServerConnection { this->flags_.batch_scheduled = false; } -#ifdef HAS_PROTO_MESSAGE_DUMP - // Helper to log a proto message from a MessageCreator object - void log_proto_message_(EntityBase *entity, const MessageCreator &creator, uint8_t message_type) { - this->flags_.log_only_mode = true; - creator(entity, this, MAX_BATCH_PACKET_SIZE, true, message_type); - this->flags_.log_only_mode = false; - } + // Dispatch message encoding based on message_type - replaces function pointer storage + // Switch assigns pointer, single call site for smaller code size + uint16_t dispatch_message_(const DeferredBatch::BatchItem &item, uint32_t remaining_size, bool is_single); +#ifdef HAS_PROTO_MESSAGE_DUMP void log_batch_item_(const DeferredBatch::BatchItem &item) { - // Use the helper to log the message - this->log_proto_message_(item.entity, item.creator, item.message_type); + this->flags_.log_only_mode = true; + this->dispatch_message_(item, MAX_BATCH_PACKET_SIZE, true); + this->flags_.log_only_mode = false; } #endif @@ -698,63 +683,31 @@ class APIConnection final : public APIServerConnection { // Helper method to send a message either immediately or via batching // Tries immediate send if should_send_immediately_() returns true and buffer has space // Falls back to batching if immediate send fails or isn't applicable - bool send_message_smart_(EntityBase *entity, MessageCreatorPtr creator, uint8_t message_type, - uint8_t estimated_size) { + bool send_message_smart_(EntityBase *entity, uint8_t message_type, uint8_t estimated_size, + uint8_t aux_data_index = DeferredBatch::AUX_DATA_UNUSED) { if (this->should_send_immediately_(message_type) && this->helper_->can_write_without_blocking()) { - // Now actually encode and send - if (creator(entity, this, MAX_BATCH_PACKET_SIZE, true) && + DeferredBatch::BatchItem item{entity, message_type, estimated_size, aux_data_index}; + if (this->dispatch_message_(item, MAX_BATCH_PACKET_SIZE, true) && this->send_buffer(ProtoWriteBuffer{&this->parent_->get_shared_buffer_ref()}, message_type)) { #ifdef HAS_PROTO_MESSAGE_DUMP - // Log the message in verbose mode - this->log_proto_message_(entity, MessageCreator(creator), message_type); + this->log_batch_item_(item); #endif return true; } - - // If immediate send failed, fall through to batching } - - // Fall back to scheduled batching - return this->schedule_message_(entity, creator, message_type, estimated_size); - } - - // Overload for MessageCreator (used by events which need to capture event_type) - bool send_message_smart_(EntityBase *entity, MessageCreator creator, uint8_t message_type, uint8_t estimated_size) { - // Try to send immediately if message type should bypass batching and buffer has space - if (this->should_send_immediately_(message_type) && this->helper_->can_write_without_blocking()) { - // Now actually encode and send - if (creator(entity, this, MAX_BATCH_PACKET_SIZE, true, message_type) && - this->send_buffer(ProtoWriteBuffer{&this->parent_->get_shared_buffer_ref()}, message_type)) { -#ifdef HAS_PROTO_MESSAGE_DUMP - // Log the message in verbose mode - this->log_proto_message_(entity, creator, message_type); -#endif - return true; - } - - // If immediate send failed, fall through to batching - } - - // Fall back to scheduled batching - return this->schedule_message_(entity, creator, message_type, estimated_size); + return this->schedule_message_(entity, message_type, estimated_size, aux_data_index); } // Helper function to schedule a deferred message with known message type - bool schedule_message_(EntityBase *entity, MessageCreator creator, uint8_t message_type, uint8_t estimated_size) { - this->deferred_batch_.add_item(entity, creator, message_type, estimated_size); + bool schedule_message_(EntityBase *entity, uint8_t message_type, uint8_t estimated_size, + uint8_t aux_data_index = DeferredBatch::AUX_DATA_UNUSED) { + this->deferred_batch_.add_item(entity, message_type, estimated_size, aux_data_index); return this->schedule_batch_(); } - // Overload for function pointers (for info messages and current state reads) - bool schedule_message_(EntityBase *entity, MessageCreatorPtr function_ptr, uint8_t message_type, - uint8_t estimated_size) { - return schedule_message_(entity, MessageCreator(function_ptr), message_type, estimated_size); - } - // Helper function to schedule a high priority message at the front of the batch - bool schedule_message_front_(EntityBase *entity, MessageCreatorPtr function_ptr, uint8_t message_type, - uint8_t estimated_size) { - this->deferred_batch_.add_item_front(entity, MessageCreator(function_ptr), message_type, estimated_size); + bool schedule_message_front_(EntityBase *entity, uint8_t message_type, uint8_t estimated_size) { + this->deferred_batch_.add_item_front(entity, message_type, estimated_size); return this->schedule_batch_(); } diff --git a/esphome/components/api/api_server.cpp b/esphome/components/api/api_server.cpp index 949262098f..a1fe33edb2 100644 --- a/esphome/components/api/api_server.cpp +++ b/esphome/components/api/api_server.cpp @@ -318,13 +318,11 @@ API_DISPATCH_UPDATE(water_heater::WaterHeater, water_heater) #endif #ifdef USE_EVENT -// Event is a special case - unlike other entities with simple state fields, -// events store their state in a member accessed via obj->get_last_event_type() void APIServer::on_event(event::Event *obj) { if (obj->is_internal()) return; for (auto &c : this->clients_) - c->send_event(obj, obj->get_last_event_type()); + c->send_event(obj); } #endif @@ -615,8 +613,7 @@ void APIServer::on_shutdown() { if (!c->send_message(req, DisconnectRequest::MESSAGE_TYPE)) { // If we can't send the disconnect request directly (tx_buffer full), // schedule it at the front of the batch so it will be sent with priority - c->schedule_message_front_(nullptr, &APIConnection::try_send_disconnect_request, DisconnectRequest::MESSAGE_TYPE, - DisconnectRequest::ESTIMATED_SIZE); + c->schedule_message_front_(nullptr, DisconnectRequest::MESSAGE_TYPE, DisconnectRequest::ESTIMATED_SIZE); } } } diff --git a/esphome/components/api/list_entities.h b/esphome/components/api/list_entities.h index 912aab72b2..bef36dd015 100644 --- a/esphome/components/api/list_entities.h +++ b/esphome/components/api/list_entities.h @@ -9,11 +9,10 @@ namespace esphome::api { class APIConnection; // Macro for generating ListEntitiesIterator handlers -// Calls schedule_message_ with try_send_*_info +// Calls schedule_message_ which dispatches to try_send_*_info #define LIST_ENTITIES_HANDLER(entity_type, EntityClass, ResponseType) \ bool ListEntitiesIterator::on_##entity_type(EntityClass *entity) { /* NOLINT(bugprone-macro-parentheses) */ \ - return this->client_->schedule_message_(entity, &APIConnection::try_send_##entity_type##_info, \ - ResponseType::MESSAGE_TYPE, ResponseType::ESTIMATED_SIZE); \ + return this->client_->schedule_message_(entity, ResponseType::MESSAGE_TYPE, ResponseType::ESTIMATED_SIZE); \ } class ListEntitiesIterator : public ComponentIterator { diff --git a/esphome/components/event/event.h b/esphome/components/event/event.h index 27700e32d8..f77ad326d9 100644 --- a/esphome/components/event/event.h +++ b/esphome/components/event/event.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -48,6 +49,24 @@ class Event : public EntityBase, public EntityBase_DeviceClass { /// Return the last triggered event type, or empty StringRef if no event triggered yet. StringRef get_last_event_type() const { return StringRef::from_maybe_nullptr(this->last_event_type_); } + /// Return event type by index, or nullptr if index is out of bounds. + const char *get_event_type(uint8_t index) const { + return index < this->types_.size() ? this->types_[index] : nullptr; + } + + /// Return index of last triggered event type, or max uint8_t if no event triggered yet. + uint8_t get_last_event_type_index() const { + if (this->last_event_type_ == nullptr) + return std::numeric_limits::max(); + // Most events have <3 types, uint8_t is sufficient for all reasonable scenarios + const uint8_t size = static_cast(this->types_.size()); + for (uint8_t i = 0; i < size; i++) { + if (this->types_[i] == this->last_event_type_) + return i; + } + return std::numeric_limits::max(); + } + /// Check if an event has been triggered. bool has_event() const { return this->last_event_type_ != nullptr; } From 42f98ebc80c7f391cf43e2b2f1a825b6a0380659 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 20:16:59 -1000 Subject: [PATCH 04/16] [scheduler] Eliminate heap allocations for std::string names and add uint32_t ID API --- esphome/components/api/api_server.cpp | 18 +- esphome/core/base_automation.h | 8 +- esphome/core/component.cpp | 23 ++ esphome/core/component.h | 47 +++ esphome/core/scheduler.cpp | 328 ++++++++++-------- esphome/core/scheduler.h | 207 ++++++----- .../fixtures/scheduler_numeric_id_test.yaml | 146 ++++++++ .../test_scheduler_numeric_id_test.py | 177 ++++++++++ 8 files changed, 709 insertions(+), 245 deletions(-) create mode 100644 tests/integration/fixtures/scheduler_numeric_id_test.yaml create mode 100644 tests/integration/test_scheduler_numeric_id_test.py diff --git a/esphome/components/api/api_server.cpp b/esphome/components/api/api_server.cpp index a1fe33edb2..a4eeb4dd5e 100644 --- a/esphome/components/api/api_server.cpp +++ b/esphome/components/api/api_server.cpp @@ -645,18 +645,18 @@ uint32_t APIServer::register_active_action_call(uint32_t client_call_id, APIConn this->active_action_calls_.push_back({action_call_id, client_call_id, conn}); // Schedule automatic cleanup after timeout (client will have given up by then) - this->set_timeout(str_sprintf("action_call_%u", action_call_id), USE_API_ACTION_CALL_TIMEOUT_MS, - [this, action_call_id]() { - ESP_LOGD(TAG, "Action call %u timed out", action_call_id); - this->unregister_active_action_call(action_call_id); - }); + // Uses numeric ID overload to avoid heap allocation from str_sprintf + this->set_timeout(action_call_id, USE_API_ACTION_CALL_TIMEOUT_MS, [this, action_call_id]() { + ESP_LOGD(TAG, "Action call %u timed out", action_call_id); + this->unregister_active_action_call(action_call_id); + }); return action_call_id; } void APIServer::unregister_active_action_call(uint32_t action_call_id) { - // Cancel the timeout for this action call - this->cancel_timeout(str_sprintf("action_call_%u", action_call_id)); + // Cancel the timeout for this action call (uses numeric ID overload) + this->cancel_timeout(action_call_id); // Swap-and-pop is more efficient than remove_if for unordered vectors for (size_t i = 0; i < this->active_action_calls_.size(); i++) { @@ -672,8 +672,8 @@ void APIServer::unregister_active_action_calls_for_connection(APIConnection *con // Remove all active action calls for disconnected connection using swap-and-pop for (size_t i = 0; i < this->active_action_calls_.size();) { if (this->active_action_calls_[i].connection == conn) { - // Cancel the timeout for this action call - this->cancel_timeout(str_sprintf("action_call_%u", this->active_action_calls_[i].action_call_id)); + // Cancel the timeout for this action call (uses numeric ID overload) + this->cancel_timeout(this->active_action_calls_[i].action_call_id); std::swap(this->active_action_calls_[i], this->active_action_calls_.back()); this->active_action_calls_.pop_back(); diff --git a/esphome/core/base_automation.h b/esphome/core/base_automation.h index e8878ac251..19d0ccf972 100644 --- a/esphome/core/base_automation.h +++ b/esphome/core/base_automation.h @@ -191,15 +191,15 @@ template class DelayAction : public Action, public Compon // instead of std::bind to avoid bind overhead (~16 bytes heap + faster execution) if constexpr (sizeof...(Ts) == 0) { App.scheduler.set_timer_common_( - this, Scheduler::SchedulerItem::TIMEOUT, - /* is_static_string= */ true, "delay", this->delay_.value(), [this]() { this->play_next_(); }, + this, Scheduler::SchedulerItem::TIMEOUT, Scheduler::NameType::STATIC_STRING, "delay", 0, this->delay_.value(), + [this]() { this->play_next_(); }, /* is_retry= */ false, /* skip_cancel= */ this->num_running_ > 1); } else { // For delays with arguments, use std::bind to preserve argument values // Arguments must be copied because original references may be invalid after delay auto f = std::bind(&DelayAction::play_next_, this, x...); - App.scheduler.set_timer_common_(this, Scheduler::SchedulerItem::TIMEOUT, - /* is_static_string= */ true, "delay", this->delay_.value(x...), std::move(f), + App.scheduler.set_timer_common_(this, Scheduler::SchedulerItem::TIMEOUT, Scheduler::NameType::STATIC_STRING, + "delay", 0, this->delay_.value(x...), std::move(f), /* is_retry= */ false, /* skip_cancel= */ this->num_running_ > 1); } } diff --git a/esphome/core/component.cpp b/esphome/core/component.cpp index 90be6cf646..decd080976 100644 --- a/esphome/core/component.cpp +++ b/esphome/core/component.cpp @@ -167,6 +167,26 @@ bool Component::cancel_timeout(const char *name) { // NOLINT return App.scheduler.cancel_timeout(this, name); } +// uint32_t (numeric ID) overloads - zero heap allocation +void Component::set_timeout(uint32_t id, uint32_t timeout, std::function &&f) { // NOLINT + App.scheduler.set_timeout(this, id, timeout, std::move(f)); +} + +bool Component::cancel_timeout(uint32_t id) { return App.scheduler.cancel_timeout(this, id); } + +void Component::set_interval(uint32_t id, uint32_t interval, std::function &&f) { // NOLINT + App.scheduler.set_interval(this, id, interval, std::move(f)); +} + +bool Component::cancel_interval(uint32_t id) { return App.scheduler.cancel_interval(this, id); } + +void Component::set_retry(uint32_t id, uint32_t initial_wait_time, uint8_t max_attempts, + std::function &&f, float backoff_increase_factor) { // NOLINT + App.scheduler.set_retry(this, id, initial_wait_time, max_attempts, std::move(f), backoff_increase_factor); +} + +bool Component::cancel_retry(uint32_t id) { return App.scheduler.cancel_retry(this, id); } + void Component::call_loop() { this->loop(); } void Component::call_setup() { this->setup(); } void Component::call_dump_config() { @@ -303,6 +323,9 @@ void Component::defer(std::function &&f) { // NOLINT bool Component::cancel_defer(const std::string &name) { // NOLINT return App.scheduler.cancel_timeout(this, name); } +bool Component::cancel_defer(const char *name) { // NOLINT + return App.scheduler.cancel_timeout(this, name); +} void Component::defer(const std::string &name, std::function &&f) { // NOLINT App.scheduler.set_timeout(this, name, 0, std::move(f)); } diff --git a/esphome/core/component.h b/esphome/core/component.h index 32f594d6f8..49349d4199 100644 --- a/esphome/core/component.h +++ b/esphome/core/component.h @@ -306,6 +306,8 @@ class Component { * * @see cancel_interval() */ + // Remove before 2026.7.0 + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") void set_interval(const std::string &name, uint32_t interval, std::function &&f); // NOLINT /** Set an interval function with a const char* name. @@ -324,6 +326,14 @@ class Component { */ void set_interval(const char *name, uint32_t interval, std::function &&f); // NOLINT + /** Set an interval function with a numeric ID (zero heap allocation). + * + * @param id The numeric identifier for this interval function + * @param interval The interval in ms + * @param f The function to call + */ + void set_interval(uint32_t id, uint32_t interval, std::function &&f); // NOLINT + void set_interval(uint32_t interval, std::function &&f); // NOLINT /** Cancel an interval function. @@ -331,8 +341,11 @@ class Component { * @param name The identifier for this interval function. * @return Whether an interval functions was deleted. */ + // Remove before 2026.7.0 + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") bool cancel_interval(const std::string &name); // NOLINT bool cancel_interval(const char *name); // NOLINT + bool cancel_interval(uint32_t id); // NOLINT /** Set an retry function with a unique name. Empty name means no cancelling possible. * @@ -364,12 +377,25 @@ class Component { * @param backoff_increase_factor time between retries is multiplied by this factor on every retry after the first * @see cancel_retry() */ + // Remove before 2026.7.0 + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") void set_retry(const std::string &name, uint32_t initial_wait_time, uint8_t max_attempts, // NOLINT std::function &&f, float backoff_increase_factor = 1.0f); // NOLINT void set_retry(const char *name, uint32_t initial_wait_time, uint8_t max_attempts, // NOLINT std::function &&f, float backoff_increase_factor = 1.0f); // NOLINT + /** Set a retry function with a numeric ID (zero heap allocation). + * + * @param id The numeric identifier for this retry function + * @param initial_wait_time The wait time after the first execution + * @param max_attempts The max number of attempts + * @param f The function to call + * @param backoff_increase_factor The factor to increase the retry interval by + */ + void set_retry(uint32_t id, uint32_t initial_wait_time, uint8_t max_attempts, // NOLINT + std::function &&f, float backoff_increase_factor = 1.0f); // NOLINT + void set_retry(uint32_t initial_wait_time, uint8_t max_attempts, std::function &&f, // NOLINT float backoff_increase_factor = 1.0f); // NOLINT @@ -378,8 +404,11 @@ class Component { * @param name The identifier for this retry function. * @return Whether a retry function was deleted. */ + // Remove before 2026.7.0 + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") bool cancel_retry(const std::string &name); // NOLINT bool cancel_retry(const char *name); // NOLINT + bool cancel_retry(uint32_t id); // NOLINT /** Set a timeout function with a unique name. * @@ -395,6 +424,8 @@ class Component { * * @see cancel_timeout() */ + // Remove before 2026.7.0 + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") void set_timeout(const std::string &name, uint32_t timeout, std::function &&f); // NOLINT /** Set a timeout function with a const char* name. @@ -413,6 +444,14 @@ class Component { */ void set_timeout(const char *name, uint32_t timeout, std::function &&f); // NOLINT + /** Set a timeout function with a numeric ID (zero heap allocation). + * + * @param id The numeric identifier for this timeout function + * @param timeout The timeout in ms + * @param f The function to call + */ + void set_timeout(uint32_t id, uint32_t timeout, std::function &&f); // NOLINT + void set_timeout(uint32_t timeout, std::function &&f); // NOLINT /** Cancel a timeout function. @@ -420,8 +459,11 @@ class Component { * @param name The identifier for this timeout function. * @return Whether a timeout functions was deleted. */ + // Remove before 2026.7.0 + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") bool cancel_timeout(const std::string &name); // NOLINT bool cancel_timeout(const char *name); // NOLINT + bool cancel_timeout(uint32_t id); // NOLINT /** Defer a callback to the next loop() call. * @@ -430,6 +472,8 @@ class Component { * @param name The name of the defer function. * @param f The callback. */ + // Remove before 2026.7.0 + ESPDEPRECATED("Use const char* overload instead. Removed in 2026.7.0", "2026.1.0") void defer(const std::string &name, std::function &&f); // NOLINT /** Defer a callback to the next loop() call with a const char* name. @@ -451,7 +495,10 @@ class Component { void defer(std::function &&f); // NOLINT /// Cancel a defer callback using the specified name, name must not be empty. + // Remove before 2026.7.0 + ESPDEPRECATED("Use const char* overload instead. Removed in 2026.7.0", "2026.1.0") bool cancel_defer(const std::string &name); // NOLINT + bool cancel_defer(const char *name); // NOLINT // Ordered for optimal packing on 32-bit systems const LogString *component_source_{nullptr}; diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index b28cb947c7..8a63b177ff 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -75,18 +75,35 @@ static void validate_static_string(const char *name) { // iterating over them from the loop task is fine; but iterating from any other context requires the lock to be held to // avoid the main thread modifying the list while it is being accessed. -// Common implementation for both timeout and interval -void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type type, bool is_static_string, - const void *name_ptr, uint32_t delay, std::function func, bool is_retry, - bool skip_cancel) { - // Get the name as const char* - const char *name_cstr = this->get_name_cstr_(is_static_string, name_ptr); +// Helper to get or create a scheduler item from the pool +// IMPORTANT: Caller must hold the scheduler lock before calling this function. +std::unique_ptr Scheduler::get_item_from_pool_locked_() { + std::unique_ptr item; + if (!this->scheduler_item_pool_.empty()) { + item = std::move(this->scheduler_item_pool_.back()); + this->scheduler_item_pool_.pop_back(); +#ifdef ESPHOME_DEBUG_SCHEDULER + ESP_LOGD(TAG, "Reused item from pool (pool size now: %zu)", this->scheduler_item_pool_.size()); +#endif + } else { + item = make_unique(); +#ifdef ESPHOME_DEBUG_SCHEDULER + ESP_LOGD(TAG, "Allocated new item (pool empty)"); +#endif + } + return item; +} +// Common implementation for both timeout and interval +// name_type determines storage type: STATIC_STRING uses static_name, others use hash_or_id +void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type type, NameType name_type, + const char *static_name, uint32_t hash_or_id, uint32_t delay, + std::function func, bool is_retry, bool skip_cancel) { if (delay == SCHEDULER_DONT_RUN) { - // Still need to cancel existing timer if name is not empty + // Still need to cancel existing timer if we have a name/id if (!skip_cancel) { LockGuard guard{this->lock_}; - this->cancel_item_locked_(component, name_cstr, type); + this->cancel_item_locked_(component, name_type, static_name, hash_or_id, type); } return; } @@ -98,23 +115,19 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type LockGuard guard{this->lock_}; // Create and populate the scheduler item - std::unique_ptr item; - if (!this->scheduler_item_pool_.empty()) { - // Reuse from pool - item = std::move(this->scheduler_item_pool_.back()); - this->scheduler_item_pool_.pop_back(); -#ifdef ESPHOME_DEBUG_SCHEDULER - ESP_LOGD(TAG, "Reused item from pool (pool size now: %zu)", this->scheduler_item_pool_.size()); -#endif - } else { - // Allocate new if pool is empty - item = make_unique(); -#ifdef ESPHOME_DEBUG_SCHEDULER - ESP_LOGD(TAG, "Allocated new item (pool empty)"); -#endif - } + auto item = this->get_item_from_pool_locked_(); item->component = component; - item->set_name(name_cstr, !is_static_string); + switch (name_type) { + case NameType::STATIC_STRING: + item->set_static_name(static_name); + break; + case NameType::HASHED_STRING: + item->set_hashed_name(hash_or_id); + break; + case NameType::NUMERIC_ID: + item->set_numeric_id(hash_or_id); + break; + } item->type = type; item->callback = std::move(func); // Reset remove flag - recycled items may have been cancelled (remove=true) in previous use @@ -127,7 +140,7 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type if (delay == 0 && type == SchedulerItem::TIMEOUT) { // Put in defer queue for guaranteed FIFO execution if (!skip_cancel) { - this->cancel_item_locked_(component, name_cstr, type); + this->cancel_item_locked_(component, name_type, static_name, hash_or_id, type); } this->defer_queue_.push_back(std::move(item)); return; @@ -141,66 +154,102 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type // Calculate random offset (0 to min(interval/2, 5s)) uint32_t offset = (uint32_t) (std::min(delay / 2, MAX_INTERVAL_DELAY) * random_float()); item->set_next_execution(now + offset); - ESP_LOGV(TAG, "Scheduler interval for %s is %" PRIu32 "ms, offset %" PRIu32 "ms", name_cstr ? name_cstr : "", delay, - offset); + ESP_LOGV(TAG, "Scheduler interval for %s is %" PRIu32 "ms, offset %" PRIu32 "ms", + name_type == NameType::STATIC_STRING ? static_name : "(id)", delay, offset); } else { item->interval = 0; item->set_next_execution(now + delay); } -#ifdef ESPHOME_DEBUG_SCHEDULER - this->debug_log_timer_(item.get(), is_static_string, name_cstr, type, delay, now); -#endif /* ESPHOME_DEBUG_SCHEDULER */ - // For retries, check if there's a cancelled timeout first - if (is_retry && name_cstr != nullptr && type == SchedulerItem::TIMEOUT && - (has_cancelled_timeout_in_container_locked_(this->items_, component, name_cstr, /* match_retry= */ true) || - has_cancelled_timeout_in_container_locked_(this->to_add_, component, name_cstr, /* match_retry= */ true))) { - // Skip scheduling - the retry was cancelled + if (is_retry && type == SchedulerItem::TIMEOUT) { + if (has_cancelled_timeout_in_container_locked_(this->items_, component, name_type, static_name, hash_or_id, + /* match_retry= */ true) || + has_cancelled_timeout_in_container_locked_(this->to_add_, component, name_type, static_name, hash_or_id, + /* match_retry= */ true)) { + // Skip scheduling - the retry was cancelled #ifdef ESPHOME_DEBUG_SCHEDULER - ESP_LOGD(TAG, "Skipping retry '%s' - found cancelled item", name_cstr); + ESP_LOGD(TAG, "Skipping retry - found cancelled item"); #endif - return; + return; + } } - // If name is provided, do atomic cancel-and-add (unless skip_cancel is true) - // Cancel existing items + // Cancel existing items with same name/id (unless skip_cancel is true) if (!skip_cancel) { - this->cancel_item_locked_(component, name_cstr, type); + this->cancel_item_locked_(component, name_type, static_name, hash_or_id, type); } - // Add new item directly to to_add_ - // since we have the lock held + + // Add new item directly to to_add_ since we have the lock held this->to_add_.push_back(std::move(item)); } +// Public API - const char* (static string) versions void HOT Scheduler::set_timeout(Component *component, const char *name, uint32_t timeout, std::function func) { - this->set_timer_common_(component, SchedulerItem::TIMEOUT, true, name, timeout, std::move(func)); -} - -void HOT Scheduler::set_timeout(Component *component, const std::string &name, uint32_t timeout, - std::function func) { - this->set_timer_common_(component, SchedulerItem::TIMEOUT, false, &name, timeout, std::move(func)); -} -bool HOT Scheduler::cancel_timeout(Component *component, const std::string &name) { - return this->cancel_item_(component, false, &name, SchedulerItem::TIMEOUT); -} -bool HOT Scheduler::cancel_timeout(Component *component, const char *name) { - return this->cancel_item_(component, true, name, SchedulerItem::TIMEOUT); -} -void HOT Scheduler::set_interval(Component *component, const std::string &name, uint32_t interval, - std::function func) { - this->set_timer_common_(component, SchedulerItem::INTERVAL, false, &name, interval, std::move(func)); + this->set_timer_common_(component, SchedulerItem::TIMEOUT, NameType::STATIC_STRING, name, 0, timeout, + std::move(func)); } void HOT Scheduler::set_interval(Component *component, const char *name, uint32_t interval, std::function func) { - this->set_timer_common_(component, SchedulerItem::INTERVAL, true, name, interval, std::move(func)); + this->set_timer_common_(component, SchedulerItem::INTERVAL, NameType::STATIC_STRING, name, 0, interval, + std::move(func)); } -bool HOT Scheduler::cancel_interval(Component *component, const std::string &name) { - return this->cancel_item_(component, false, &name, SchedulerItem::INTERVAL); + +bool HOT Scheduler::cancel_timeout(Component *component, const char *name) { + LockGuard guard{this->lock_}; + return this->cancel_item_locked_(component, NameType::STATIC_STRING, name, 0, SchedulerItem::TIMEOUT); } + bool HOT Scheduler::cancel_interval(Component *component, const char *name) { - return this->cancel_item_(component, true, name, SchedulerItem::INTERVAL); + LockGuard guard{this->lock_}; + return this->cancel_item_locked_(component, NameType::STATIC_STRING, name, 0, SchedulerItem::INTERVAL); +} + +// Public API - std::string (hashed) versions - computes FNV-1a hash internally +void HOT Scheduler::set_timeout(Component *component, const std::string &name, uint32_t timeout, + std::function func) { + this->set_timer_common_(component, SchedulerItem::TIMEOUT, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), + timeout, std::move(func)); +} + +void HOT Scheduler::set_interval(Component *component, const std::string &name, uint32_t interval, + std::function func) { + this->set_timer_common_(component, SchedulerItem::INTERVAL, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), + interval, std::move(func)); +} + +bool HOT Scheduler::cancel_timeout(Component *component, const std::string &name) { + LockGuard guard{this->lock_}; + return this->cancel_item_locked_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), + SchedulerItem::TIMEOUT); +} + +bool HOT Scheduler::cancel_interval(Component *component, const std::string &name) { + LockGuard guard{this->lock_}; + return this->cancel_item_locked_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), + SchedulerItem::INTERVAL); +} + +// Public API - uint32_t (numeric ID) versions +void HOT Scheduler::set_timeout(Component *component, uint32_t id, uint32_t timeout, std::function func) { + this->set_timer_common_(component, SchedulerItem::TIMEOUT, NameType::NUMERIC_ID, nullptr, id, timeout, + std::move(func)); +} + +void HOT Scheduler::set_interval(Component *component, uint32_t id, uint32_t interval, std::function func) { + this->set_timer_common_(component, SchedulerItem::INTERVAL, NameType::NUMERIC_ID, nullptr, id, interval, + std::move(func)); +} + +bool HOT Scheduler::cancel_timeout(Component *component, uint32_t id) { + LockGuard guard{this->lock_}; + return this->cancel_item_locked_(component, NameType::NUMERIC_ID, nullptr, id, SchedulerItem::TIMEOUT); +} + +bool HOT Scheduler::cancel_interval(Component *component, uint32_t id) { + LockGuard guard{this->lock_}; + return this->cancel_item_locked_(component, NameType::NUMERIC_ID, nullptr, id, SchedulerItem::INTERVAL); } struct RetryArgs { @@ -208,49 +257,54 @@ struct RetryArgs { std::function func; Component *component; Scheduler *scheduler; - const char *name; // Points to static string or owned copy + // Union for name storage - only one is used based on name_type + union { + const char *static_name; // For STATIC_STRING + uint32_t hash_or_id; // For HASHED_STRING or NUMERIC_ID + } name_; uint32_t current_interval; float backoff_increase_factor; + Scheduler::NameType name_type; // Discriminator for name_ union uint8_t retry_countdown; - bool name_is_dynamic; // True if name needs delete[] - - ~RetryArgs() { - if (this->name_is_dynamic && this->name) { - delete[] this->name; - } - } }; void retry_handler(const std::shared_ptr &args) { RetryResult const retry_result = args->func(--args->retry_countdown); if (retry_result == RetryResult::DONE || args->retry_countdown <= 0) return; - // second execution of `func` happens after `initial_wait_time` - // Pass is_static_string=true because args->name is owned by the shared_ptr - // which is captured in the lambda and outlives the SchedulerItem + // Second execution of `func` happens after `initial_wait_time` + // static_name is owned by the shared_ptr which is captured in the lambda + const char *static_name = (args->name_type == Scheduler::NameType::STATIC_STRING) ? args->name_.static_name : nullptr; + uint32_t hash_or_id = (args->name_type != Scheduler::NameType::STATIC_STRING) ? args->name_.hash_or_id : 0; args->scheduler->set_timer_common_( - args->component, Scheduler::SchedulerItem::TIMEOUT, true, args->name, args->current_interval, - [args]() { retry_handler(args); }, /* is_retry= */ true); + args->component, Scheduler::SchedulerItem::TIMEOUT, args->name_type, static_name, hash_or_id, + args->current_interval, [args]() { retry_handler(args); }, + /* is_retry= */ true); // backoff_increase_factor applied to third & later executions args->current_interval *= args->backoff_increase_factor; } -void HOT Scheduler::set_retry_common_(Component *component, bool is_static_string, const void *name_ptr, - uint32_t initial_wait_time, uint8_t max_attempts, +// Common implementation for retry +// name_type determines storage type: STATIC_STRING uses static_name, others use hash_or_id +void HOT Scheduler::set_retry_common_(Component *component, NameType name_type, const char *static_name, + uint32_t hash_or_id, uint32_t initial_wait_time, uint8_t max_attempts, std::function func, float backoff_increase_factor) { - const char *name_cstr = this->get_name_cstr_(is_static_string, name_ptr); - - if (name_cstr != nullptr) - this->cancel_retry(component, name_cstr); + // Cancel existing retry with same name/id + { + LockGuard guard{this->lock_}; + this->cancel_item_locked_(component, name_type, static_name, hash_or_id, SchedulerItem::TIMEOUT, + /* match_retry= */ true); + } if (initial_wait_time == SCHEDULER_DONT_RUN) return; ESP_LOGVV(TAG, "set_retry(name='%s', initial_wait_time=%" PRIu32 ", max_attempts=%u, backoff_factor=%0.1f)", - name_cstr ? name_cstr : "", initial_wait_time, max_attempts, backoff_increase_factor); + name_type == NameType::STATIC_STRING ? static_name : "(id)", initial_wait_time, max_attempts, + backoff_increase_factor); if (backoff_increase_factor < 0.0001) { - ESP_LOGE(TAG, "backoff_factor %0.1f too small, using 1.0: %s", backoff_increase_factor, name_cstr ? name_cstr : ""); + ESP_LOGE(TAG, "backoff_factor %0.1f too small, using 1.0", backoff_increase_factor); backoff_increase_factor = 1; } @@ -258,56 +312,60 @@ void HOT Scheduler::set_retry_common_(Component *component, bool is_static_strin args->func = std::move(func); args->component = component; args->scheduler = this; + args->name_type = name_type; + if (name_type == NameType::STATIC_STRING) { + args->name_.static_name = static_name; + } else { + args->name_.hash_or_id = hash_or_id; + } args->current_interval = initial_wait_time; args->backoff_increase_factor = backoff_increase_factor; args->retry_countdown = max_attempts; - // Store name - either as static pointer or owned copy - if (name_cstr == nullptr || name_cstr[0] == '\0') { - // Empty or null name - use empty string literal - args->name = ""; - args->name_is_dynamic = false; - } else if (is_static_string) { - // Static string - just store the pointer - args->name = name_cstr; - args->name_is_dynamic = false; - } else { - // Dynamic string - make a copy - size_t len = strlen(name_cstr); - char *copy = new char[len + 1]; - memcpy(copy, name_cstr, len + 1); - args->name = copy; - args->name_is_dynamic = true; - } - // First execution of `func` immediately - use set_timer_common_ with is_retry=true - // Pass is_static_string=true because args->name is owned by the shared_ptr - // which is captured in the lambda and outlives the SchedulerItem this->set_timer_common_( - component, SchedulerItem::TIMEOUT, true, args->name, 0, [args]() { retry_handler(args); }, + component, SchedulerItem::TIMEOUT, name_type, static_name, hash_or_id, 0, [args]() { retry_handler(args); }, /* is_retry= */ true); } -void HOT Scheduler::set_retry(Component *component, const std::string &name, uint32_t initial_wait_time, - uint8_t max_attempts, std::function func, - float backoff_increase_factor) { - this->set_retry_common_(component, false, &name, initial_wait_time, max_attempts, std::move(func), - backoff_increase_factor); -} - +// Public API - const char* (static string) versions void HOT Scheduler::set_retry(Component *component, const char *name, uint32_t initial_wait_time, uint8_t max_attempts, std::function func, float backoff_increase_factor) { - this->set_retry_common_(component, true, name, initial_wait_time, max_attempts, std::move(func), + this->set_retry_common_(component, NameType::STATIC_STRING, name, 0, initial_wait_time, max_attempts, std::move(func), backoff_increase_factor); } -bool HOT Scheduler::cancel_retry(Component *component, const std::string &name) { - return this->cancel_retry(component, name.c_str()); -} bool HOT Scheduler::cancel_retry(Component *component, const char *name) { - // Cancel timeouts that have is_retry flag set LockGuard guard{this->lock_}; - return this->cancel_item_locked_(component, name, SchedulerItem::TIMEOUT, /* match_retry= */ true); + return this->cancel_item_locked_(component, NameType::STATIC_STRING, name, 0, SchedulerItem::TIMEOUT, + /* match_retry= */ true); +} + +// Public API - std::string (hashed) versions +void HOT Scheduler::set_retry(Component *component, const std::string &name, uint32_t initial_wait_time, + uint8_t max_attempts, std::function func, + float backoff_increase_factor) { + this->set_retry_common_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), initial_wait_time, + max_attempts, std::move(func), backoff_increase_factor); +} + +bool HOT Scheduler::cancel_retry(Component *component, const std::string &name) { + LockGuard guard{this->lock_}; + return this->cancel_item_locked_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), + SchedulerItem::TIMEOUT, /* match_retry= */ true); +} + +// Public API - uint32_t (numeric ID) versions +void HOT Scheduler::set_retry(Component *component, uint32_t id, uint32_t initial_wait_time, uint8_t max_attempts, + std::function func, float backoff_increase_factor) { + this->set_retry_common_(component, NameType::NUMERIC_ID, nullptr, id, initial_wait_time, max_attempts, + std::move(func), backoff_increase_factor); +} + +bool HOT Scheduler::cancel_retry(Component *component, uint32_t id) { + LockGuard guard{this->lock_}; + return this->cancel_item_locked_(component, NameType::NUMERIC_ID, nullptr, id, SchedulerItem::TIMEOUT, + /* match_retry= */ true); } optional HOT Scheduler::next_schedule_in(uint32_t now) { @@ -560,33 +618,22 @@ uint32_t HOT Scheduler::execute_item_(SchedulerItem *item, uint32_t now) { return guard.finish(); } -// Common implementation for cancel operations -bool HOT Scheduler::cancel_item_(Component *component, bool is_static_string, const void *name_ptr, - SchedulerItem::Type type) { - // Get the name as const char* - const char *name_cstr = this->get_name_cstr_(is_static_string, name_ptr); - - // obtain lock because this function iterates and can be called from non-loop task context - LockGuard guard{this->lock_}; - return this->cancel_item_locked_(component, name_cstr, type); -} - -// Helper to cancel items by name - must be called with lock held -bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_cstr, SchedulerItem::Type type, - bool match_retry) { - // Early return if name is invalid - no items to cancel - if (name_cstr == nullptr) { +// Helper to cancel items - must be called with lock held +// name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id +bool HOT Scheduler::cancel_item_locked_(Component *component, NameType name_type, const char *static_name, + uint32_t hash_or_id, SchedulerItem::Type type, bool match_retry) { + // Early return if static string name is invalid + if (name_type == NameType::STATIC_STRING && static_name == nullptr) { return false; } size_t total_cancelled = 0; - // Check all containers for matching items #ifndef ESPHOME_THREAD_SINGLE // Mark items in defer queue as cancelled (they'll be skipped when processed) if (type == SchedulerItem::TIMEOUT) { - total_cancelled += - this->mark_matching_items_removed_locked_(this->defer_queue_, component, name_cstr, type, match_retry); + total_cancelled += this->mark_matching_items_removed_locked_(this->defer_queue_, component, name_type, static_name, + hash_or_id, type, match_retry); } #endif /* not ESPHOME_THREAD_SINGLE */ @@ -596,14 +643,15 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c // would destroy the callback while it's running (use-after-free). // Only the main loop in call() should recycle items after execution completes. if (!this->items_.empty()) { - size_t heap_cancelled = - this->mark_matching_items_removed_locked_(this->items_, component, name_cstr, type, match_retry); + size_t heap_cancelled = this->mark_matching_items_removed_locked_(this->items_, component, name_type, static_name, + hash_or_id, type, match_retry); total_cancelled += heap_cancelled; this->to_remove_ += heap_cancelled; } // Cancel items in to_add_ - total_cancelled += this->mark_matching_items_removed_locked_(this->to_add_, component, name_cstr, type, match_retry); + total_cancelled += this->mark_matching_items_removed_locked_(this->to_add_, component, name_type, static_name, + hash_or_id, type, match_retry); return total_cancelled > 0; } @@ -785,8 +833,6 @@ void Scheduler::recycle_item_main_loop_(std::unique_ptr item) { if (this->scheduler_item_pool_.size() < MAX_POOL_SIZE) { // Clear callback to release captured resources item->callback = nullptr; - // Clear dynamic name if any - item->clear_dynamic_name(); this->scheduler_item_pool_.push_back(std::move(item)); #ifdef ESPHOME_DEBUG_SCHEDULER ESP_LOGD(TAG, "Recycled item to pool (pool size now: %zu)", this->scheduler_item_pool_.size()); diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 5bf3d19adb..116b79b75a 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -1,9 +1,10 @@ #pragma once #include "esphome/core/defines.h" -#include -#include #include +#include +#include +#include #ifdef ESPHOME_THREAD_MULTI_ATOMICS #include #endif @@ -29,8 +30,21 @@ class Scheduler { template friend class DelayAction; public: - // Public API - accepts std::string for backward compatibility + // std::string overloads - deprecated, use const char* or uint32_t instead + // Remove before 2026.7.0 + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") void set_timeout(Component *component, const std::string &name, uint32_t timeout, std::function func); + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") + bool cancel_timeout(Component *component, const std::string &name); + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") + void set_interval(Component *component, const std::string &name, uint32_t interval, std::function func); + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") + bool cancel_interval(Component *component, const std::string &name); + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") + void set_retry(Component *component, const std::string &name, uint32_t initial_wait_time, uint8_t max_attempts, + std::function func, float backoff_increase_factor = 1.0f); + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") + bool cancel_retry(Component *component, const std::string &name); /** Set a timeout with a const char* name. * @@ -39,15 +53,13 @@ class Scheduler { * - A string literal (e.g., "update") * - A static const char* variable * - A pointer with lifetime >= the scheduled task - * - * For dynamic strings, use the std::string overload instead. */ void set_timeout(Component *component, const char *name, uint32_t timeout, std::function func); - - bool cancel_timeout(Component *component, const std::string &name); bool cancel_timeout(Component *component, const char *name); - void set_interval(Component *component, const std::string &name, uint32_t interval, std::function func); + /// Set a timeout with a numeric ID (zero heap allocation) + void set_timeout(Component *component, uint32_t id, uint32_t timeout, std::function func); + bool cancel_timeout(Component *component, uint32_t id); /** Set an interval with a const char* name. * @@ -56,20 +68,23 @@ class Scheduler { * - A string literal (e.g., "update") * - A static const char* variable * - A pointer with lifetime >= the scheduled task - * - * For dynamic strings, use the std::string overload instead. */ void set_interval(Component *component, const char *name, uint32_t interval, std::function func); - - bool cancel_interval(Component *component, const std::string &name); bool cancel_interval(Component *component, const char *name); - void set_retry(Component *component, const std::string &name, uint32_t initial_wait_time, uint8_t max_attempts, - std::function func, float backoff_increase_factor = 1.0f); + + /// Set an interval with a numeric ID (zero heap allocation) + void set_interval(Component *component, uint32_t id, uint32_t interval, std::function func); + bool cancel_interval(Component *component, uint32_t id); + void set_retry(Component *component, const char *name, uint32_t initial_wait_time, uint8_t max_attempts, std::function func, float backoff_increase_factor = 1.0f); - bool cancel_retry(Component *component, const std::string &name); bool cancel_retry(Component *component, const char *name); + /// Set a retry with a numeric ID (zero heap allocation) + void set_retry(Component *component, uint32_t id, uint32_t initial_wait_time, uint8_t max_attempts, + std::function func, float backoff_increase_factor = 1.0f); + bool cancel_retry(Component *component, uint32_t id); + // Calculate when the next scheduled item should run // @param now Fresh timestamp from millis() - must not be stale/cached // Returns the time in milliseconds until the next scheduled item, or nullopt if no items @@ -83,14 +98,22 @@ class Scheduler { void process_to_add(); + // Name storage type discriminator for SchedulerItem + // Used to distinguish between static strings, hashed strings, and numeric IDs + enum class NameType : uint8_t { + STATIC_STRING = 0, // const char* pointer to static/flash storage + HASHED_STRING = 1, // uint32_t FNV-1a hash of a runtime string + NUMERIC_ID = 2 // uint32_t numeric identifier + }; + protected: struct SchedulerItem { // Ordered by size to minimize padding Component *component; - // Optimized name storage using tagged union + // Optimized name storage using tagged union - zero heap allocation union { - const char *static_name; // For string literals (no allocation) - char *dynamic_name; // For allocated strings + const char *static_name; // For STATIC_STRING (string literals, no allocation) + uint32_t hash_or_id; // For HASHED_STRING or NUMERIC_ID } name_; uint32_t interval; // Split time to handle millis() rollover. The scheduler combines the 32-bit millis() @@ -109,19 +132,19 @@ class Scheduler { // Place atomic separately since it can't be packed with bit fields std::atomic remove{false}; - // Bit-packed fields (3 bits used, 5 bits padding in 1 byte) - enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1; - bool name_is_dynamic : 1; // True if name was dynamically allocated (needs delete[]) - bool is_retry : 1; // True if this is a retry timeout - // 5 bits padding -#else - // Single-threaded or multi-threaded without atomics: can pack all fields together // Bit-packed fields (4 bits used, 4 bits padding in 1 byte) enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1; + NameType name_type_ : 2; // Discriminator for name_ union (STATIC_STRING, HASHED_STRING, NUMERIC_ID) + bool is_retry : 1; // True if this is a retry timeout + // 4 bits padding +#else + // Single-threaded or multi-threaded without atomics: can pack all fields together + // Bit-packed fields (5 bits used, 3 bits padding in 1 byte) + enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1; bool remove : 1; - bool name_is_dynamic : 1; // True if name was dynamically allocated (needs delete[]) - bool is_retry : 1; // True if this is a retry timeout - // 4 bits padding + NameType name_type_ : 2; // Discriminator for name_ union (STATIC_STRING, HASHED_STRING, NUMERIC_ID) + bool is_retry : 1; // True if this is a retry timeout + // 3 bits padding #endif // Constructor @@ -133,19 +156,19 @@ class Scheduler { #ifdef ESPHOME_THREAD_MULTI_ATOMICS // remove is initialized in the member declaration as std::atomic{false} type(TIMEOUT), - name_is_dynamic(false), + name_type_(NameType::STATIC_STRING), is_retry(false) { #else type(TIMEOUT), remove(false), - name_is_dynamic(false), + name_type_(NameType::STATIC_STRING), is_retry(false) { #endif name_.static_name = nullptr; } - // Destructor to clean up dynamic names - ~SchedulerItem() { clear_dynamic_name(); } + // Destructor - no dynamic memory to clean up + ~SchedulerItem() = default; // Delete copy operations to prevent accidental copies SchedulerItem(const SchedulerItem &) = delete; @@ -155,36 +178,31 @@ class Scheduler { SchedulerItem(SchedulerItem &&) = delete; SchedulerItem &operator=(SchedulerItem &&) = delete; - // Helper to get the name regardless of storage type - const char *get_name() const { return name_is_dynamic ? name_.dynamic_name : name_.static_name; } + // Helper to get the static name (only valid for STATIC_STRING type) + const char *get_name() const { return (name_type_ == NameType::STATIC_STRING) ? name_.static_name : nullptr; } - // Helper to clear dynamic name if allocated - void clear_dynamic_name() { - if (name_is_dynamic && name_.dynamic_name) { - delete[] name_.dynamic_name; - name_.dynamic_name = nullptr; - name_is_dynamic = false; - } + // Helper to get the hash or numeric ID (only valid for HASHED_STRING or NUMERIC_ID types) + uint32_t get_name_hash_or_id() const { return (name_type_ != NameType::STATIC_STRING) ? name_.hash_or_id : 0; } + + // Helper to get the name type + NameType get_name_type() const { return name_type_; } + + // Helper to set a static string name (no allocation) + void set_static_name(const char *name) { + name_.static_name = name; + name_type_ = NameType::STATIC_STRING; } - // Helper to set name with proper ownership - void set_name(const char *name, bool make_copy = false) { - // Clean up old dynamic name if any - clear_dynamic_name(); + // Helper to set a hashed string name (hash computed from std::string) + void set_hashed_name(uint32_t hash) { + name_.hash_or_id = hash; + name_type_ = NameType::HASHED_STRING; + } - if (!name) { - // nullptr case - no name provided - name_.static_name = nullptr; - } else if (make_copy) { - // Make a copy for dynamic strings (including empty strings) - size_t len = strlen(name); - name_.dynamic_name = new char[len + 1]; - memcpy(name_.dynamic_name, name, len + 1); - name_is_dynamic = true; - } else { - // Use static string directly (including empty strings) - name_.static_name = name; - } + // Helper to set a numeric ID name + void set_numeric_id(uint32_t id) { + name_.hash_or_id = id; + name_type_ = NameType::NUMERIC_ID; } static bool cmp(const std::unique_ptr &a, const std::unique_ptr &b); @@ -207,12 +225,16 @@ class Scheduler { }; // Common implementation for both timeout and interval - void set_timer_common_(Component *component, SchedulerItem::Type type, bool is_static_string, const void *name_ptr, - uint32_t delay, std::function func, bool is_retry = false, bool skip_cancel = false); + // name_type determines storage type: STATIC_STRING uses static_name, others use hash_or_id + void set_timer_common_(Component *component, SchedulerItem::Type type, NameType name_type, const char *static_name, + uint32_t hash_or_id, uint32_t delay, std::function func, bool is_retry = false, + bool skip_cancel = false); // Common implementation for retry - void set_retry_common_(Component *component, bool is_static_string, const void *name_ptr, uint32_t initial_wait_time, - uint8_t max_attempts, std::function func, float backoff_increase_factor); + // name_type determines storage type: STATIC_STRING uses static_name, others use hash_or_id + void set_retry_common_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, + uint32_t initial_wait_time, uint8_t max_attempts, std::function func, + float backoff_increase_factor); uint64_t millis_64_(uint32_t now); // Cleanup logically deleted items from the scheduler @@ -222,38 +244,31 @@ class Scheduler { // Remove and return the front item from the heap // IMPORTANT: Caller must hold the scheduler lock before calling this function. std::unique_ptr pop_raw_locked_(); + // Get or create a scheduler item from the pool + // IMPORTANT: Caller must hold the scheduler lock before calling this function. + std::unique_ptr get_item_from_pool_locked_(); private: - // Helper to cancel items by name - must be called with lock held - bool cancel_item_locked_(Component *component, const char *name, SchedulerItem::Type type, bool match_retry = false); + // Helper to cancel items - must be called with lock held + // name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id + bool cancel_item_locked_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, + SchedulerItem::Type type, bool match_retry = false); - // Helper to extract name as const char* from either static string or std::string - inline const char *get_name_cstr_(bool is_static_string, const void *name_ptr) { - return is_static_string ? static_cast(name_ptr) : static_cast(name_ptr)->c_str(); - } - - // Common implementation for cancel operations - bool cancel_item_(Component *component, bool is_static_string, const void *name_ptr, SchedulerItem::Type type); - - // Helper to check if two scheduler item names match - inline bool HOT names_match_(const char *name1, const char *name2) const { + // Helper to check if two static string names match + inline bool HOT names_match_static_(const char *name1, const char *name2) const { // Check pointer equality first (common for static strings), then string contents - // The core ESPHome codebase uses static strings (const char*) for component names, - // making pointer comparison effective. The std::string overloads exist only for - // compatibility with external components but are rarely used in practice. return (name1 != nullptr && name2 != nullptr) && ((name1 == name2) || (strcmp(name1, name2) == 0)); } // Helper function to check if item matches criteria for cancellation + // name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id // IMPORTANT: Must be called with scheduler lock held inline bool HOT matches_item_locked_(const std::unique_ptr &item, Component *component, - const char *name_cstr, SchedulerItem::Type type, bool match_retry, - bool skip_removed = true) const { + NameType name_type, const char *static_name, uint32_t hash_or_id, + SchedulerItem::Type type, bool match_retry, bool skip_removed = true) const { // THREAD SAFETY: Check for nullptr first to prevent LoadProhibited crashes. On multi-threaded // platforms, items can be moved out of defer_queue_ during processing, leaving nullptr entries. - // PR #11305 added nullptr checks in callers (mark_matching_items_removed_locked_() and - // has_cancelled_timeout_in_container_locked_()), but this check provides defense-in-depth: helper - // functions should be safe regardless of caller behavior. + // This check provides defense-in-depth: helper functions should be safe regardless of caller behavior. // Fixes: https://github.com/esphome/esphome/issues/11940 if (!item) return false; @@ -261,7 +276,14 @@ class Scheduler { (match_retry && !item->is_retry)) { return false; } - return this->names_match_(item->get_name(), name_cstr); + // Name type must match + if (item->get_name_type() != name_type) + return false; + // For static strings, compare the string content; for hash/ID, compare the value + if (name_type == NameType::STATIC_STRING) { + return this->names_match_static_(item->get_name(), static_name); + } + return item->get_name_hash_or_id() == hash_or_id; } // Helper to execute a scheduler item @@ -410,11 +432,13 @@ class Scheduler { } // Helper to mark matching items in a container as removed + // name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id // Returns the number of items marked for removal // IMPORTANT: Must be called with scheduler lock held template - size_t mark_matching_items_removed_locked_(Container &container, Component *component, const char *name_cstr, - SchedulerItem::Type type, bool match_retry) { + size_t mark_matching_items_removed_locked_(Container &container, Component *component, NameType name_type, + const char *static_name, uint32_t hash_or_id, SchedulerItem::Type type, + bool match_retry) { size_t count = 0; for (auto &item : container) { // Skip nullptr items (can happen in defer_queue_ when items are being processed) @@ -423,8 +447,7 @@ class Scheduler { // the vector can still contain nullptr items from the processing loop. This check prevents crashes. if (!item) continue; - if (this->matches_item_locked_(item, component, name_cstr, type, match_retry)) { - // Mark item for removal (platform-specific) + if (this->matches_item_locked_(item, component, name_type, static_name, hash_or_id, type, match_retry)) { this->set_item_removed_(item.get(), true); count++; } @@ -433,10 +456,12 @@ class Scheduler { } // Template helper to check if any item in a container matches our criteria + // name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id // IMPORTANT: Must be called with scheduler lock held template - bool has_cancelled_timeout_in_container_locked_(const Container &container, Component *component, - const char *name_cstr, bool match_retry) const { + bool has_cancelled_timeout_in_container_locked_(const Container &container, Component *component, NameType name_type, + const char *static_name, uint32_t hash_or_id, + bool match_retry) const { for (const auto &item : container) { // Skip nullptr items (can happen in defer_queue_ when items are being processed) // The defer_queue_ uses index-based processing: items are std::moved out but left in the @@ -445,8 +470,8 @@ class Scheduler { if (!item) continue; if (is_item_removed_(item.get()) && - this->matches_item_locked_(item, component, name_cstr, SchedulerItem::TIMEOUT, match_retry, - /* skip_removed= */ false)) { + this->matches_item_locked_(item, component, name_type, static_name, hash_or_id, SchedulerItem::TIMEOUT, + match_retry, /* skip_removed= */ false)) { return true; } } diff --git a/tests/integration/fixtures/scheduler_numeric_id_test.yaml b/tests/integration/fixtures/scheduler_numeric_id_test.yaml new file mode 100644 index 0000000000..f8265a7832 --- /dev/null +++ b/tests/integration/fixtures/scheduler_numeric_id_test.yaml @@ -0,0 +1,146 @@ +esphome: + name: scheduler-numeric-id-test + on_boot: + priority: -100 + then: + - logger.log: "Starting scheduler numeric ID tests" + +host: +api: +logger: + level: VERBOSE + +globals: + - id: timeout_counter + type: int + initial_value: '0' + - id: interval_counter + type: int + initial_value: '0' + - id: tests_done + type: bool + initial_value: 'false' + - id: results_reported + type: bool + initial_value: 'false' + +script: + - id: test_numeric_ids + then: + - logger.log: "Testing numeric ID timeouts and intervals" + - lambda: |- + auto *component1 = id(test_sensor1); + + // Test 1: Numeric ID with set_timeout (uint32_t) + App.scheduler.set_timeout(component1, 1001U, 50, []() { + ESP_LOGI("test", "Numeric timeout 1001 fired"); + id(timeout_counter) += 1; + }); + + // Test 2: Another numeric ID timeout + App.scheduler.set_timeout(component1, 1002U, 100, []() { + ESP_LOGI("test", "Numeric timeout 1002 fired"); + id(timeout_counter) += 1; + }); + + // Test 3: Numeric ID with set_interval + App.scheduler.set_interval(component1, 2001U, 200, []() { + ESP_LOGI("test", "Numeric interval 2001 fired, count: %d", id(interval_counter)); + id(interval_counter) += 1; + if (id(interval_counter) >= 3) { + App.scheduler.cancel_interval(id(test_sensor1), 2001U); + ESP_LOGI("test", "Cancelled numeric interval 2001"); + } + }); + + // Test 4: Cancel timeout with numeric ID + App.scheduler.set_timeout(component1, 3001U, 5000, []() { + ESP_LOGE("test", "ERROR: Timeout 3001 should have been cancelled"); + }); + App.scheduler.cancel_timeout(component1, 3001U); + ESP_LOGI("test", "Cancelled numeric timeout 3001"); + + // Test 5: Multiple timeouts with same numeric ID - only last should execute + for (int i = 0; i < 5; i++) { + App.scheduler.set_timeout(component1, 4001U, 300 + i*10, [i]() { + ESP_LOGI("test", "Duplicate numeric timeout %d fired", i); + id(timeout_counter) += 1; + }); + } + ESP_LOGI("test", "Created 5 timeouts with same numeric ID 4001"); + + // Test 6: Cancel non-existent numeric ID + bool cancelled_nonexistent = App.scheduler.cancel_timeout(component1, 9999U); + ESP_LOGI("test", "Cancel non-existent numeric ID result: %s", + cancelled_nonexistent ? "true (unexpected!)" : "false (expected)"); + + // Test 7: Component method uint32_t overloads + class TestNumericComponent : public Component { + public: + void test_numeric_methods() { + // Test set_timeout with uint32_t ID + this->set_timeout(5001U, 150, []() { + ESP_LOGI("test", "Component numeric timeout 5001 fired"); + id(timeout_counter) += 1; + }); + + // Test set_interval with uint32_t ID + this->set_interval(5002U, 400, []() { + ESP_LOGI("test", "Component numeric interval 5002 fired"); + id(interval_counter) += 1; + // Cancel after first fire + App.scheduler.cancel_interval(nullptr, 5002U); + }); + } + }; + + static TestNumericComponent test_component; + test_component.test_numeric_methods(); + + // Test 8: Zero ID (edge case) + App.scheduler.set_timeout(component1, 0U, 200, []() { + ESP_LOGI("test", "Numeric timeout with ID 0 fired"); + id(timeout_counter) += 1; + }); + + // Test 9: Max uint32_t ID (edge case) + App.scheduler.set_timeout(component1, 0xFFFFFFFFU, 250, []() { + ESP_LOGI("test", "Numeric timeout with max ID fired"); + id(timeout_counter) += 1; + }); + + - id: report_results + then: + - lambda: |- + ESP_LOGI("test", "Final results - Timeouts: %d, Intervals: %d", + id(timeout_counter), id(interval_counter)); + +sensor: + - platform: template + name: Test Sensor 1 + id: test_sensor1 + lambda: return 1.0; + update_interval: never + +interval: + # Run numeric ID tests after boot + - interval: 0.1s + then: + - if: + condition: + lambda: 'return id(tests_done) == false;' + then: + - lambda: 'id(tests_done) = true;' + - script.execute: test_numeric_ids + - logger.log: "Started numeric ID tests" + + # Report results after tests complete + - interval: 0.2s + then: + - if: + condition: + lambda: 'return id(tests_done) && !id(results_reported);' + then: + - lambda: 'id(results_reported) = true;' + - delay: 1.5s + - script.execute: report_results diff --git a/tests/integration/test_scheduler_numeric_id_test.py b/tests/integration/test_scheduler_numeric_id_test.py new file mode 100644 index 0000000000..e56d889cd1 --- /dev/null +++ b/tests/integration/test_scheduler_numeric_id_test.py @@ -0,0 +1,177 @@ +"""Test scheduler numeric ID (uint32_t) overloads.""" + +import asyncio +import re + +import pytest + +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_scheduler_numeric_id_test( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test that scheduler handles numeric IDs (uint32_t) correctly.""" + # Track counts + timeout_count = 0 + interval_count = 0 + + # Events for each test completion + numeric_timeout_1001_fired = asyncio.Event() + numeric_timeout_1002_fired = asyncio.Event() + numeric_interval_2001_fired = asyncio.Event() + numeric_interval_cancelled = asyncio.Event() + numeric_timeout_cancelled = asyncio.Event() + duplicate_timeout_fired = asyncio.Event() + component_timeout_fired = asyncio.Event() + component_interval_fired = asyncio.Event() + zero_id_timeout_fired = asyncio.Event() + max_id_timeout_fired = asyncio.Event() + final_results_logged = asyncio.Event() + + # Track interval counts + numeric_interval_count = 0 + + def on_log_line(line: str) -> None: + nonlocal timeout_count, interval_count, numeric_interval_count + + # Strip ANSI color codes + clean_line = re.sub(r"\x1b\[[0-9;]*m", "", line) + + # Check for numeric timeout completions + if "Numeric timeout 1001 fired" in clean_line: + numeric_timeout_1001_fired.set() + timeout_count += 1 + + elif "Numeric timeout 1002 fired" in clean_line: + numeric_timeout_1002_fired.set() + timeout_count += 1 + + # Check for numeric interval + elif "Numeric interval 2001 fired" in clean_line: + match = re.search(r"count: (\d+)", clean_line) + if match: + numeric_interval_count = int(match.group(1)) + numeric_interval_2001_fired.set() + + elif "Cancelled numeric interval 2001" in clean_line: + numeric_interval_cancelled.set() + + elif "Cancelled numeric timeout 3001" in clean_line: + numeric_timeout_cancelled.set() + + # Check for duplicate timeout (only last should fire) + elif "Duplicate numeric timeout" in clean_line: + match = re.search(r"timeout (\d+) fired", clean_line) + if match and match.group(1) == "4": + duplicate_timeout_fired.set() + timeout_count += 1 + + # Check for component method tests + elif "Component numeric timeout 5001 fired" in clean_line: + component_timeout_fired.set() + timeout_count += 1 + + elif "Component numeric interval 5002 fired" in clean_line: + component_interval_fired.set() + interval_count += 1 + + # Check for edge case tests + elif "Numeric timeout with ID 0 fired" in clean_line: + zero_id_timeout_fired.set() + timeout_count += 1 + + elif "Numeric timeout with max ID fired" in clean_line: + max_id_timeout_fired.set() + timeout_count += 1 + + # Check for final results + elif "Final results" in clean_line: + match = re.search(r"Timeouts: (\d+), Intervals: (\d+)", clean_line) + if match: + timeout_count = int(match.group(1)) + interval_count = int(match.group(2)) + final_results_logged.set() + + async with ( + run_compiled(yaml_config, line_callback=on_log_line), + api_client_connected() as client, + ): + # Verify we can connect + device_info = await client.device_info() + assert device_info is not None + assert device_info.name == "scheduler-numeric-id-test" + + # Wait for numeric timeout tests + try: + await asyncio.wait_for(numeric_timeout_1001_fired.wait(), timeout=0.5) + except TimeoutError: + pytest.fail("Numeric timeout 1001 did not fire within 0.5 seconds") + + try: + await asyncio.wait_for(numeric_timeout_1002_fired.wait(), timeout=0.5) + except TimeoutError: + pytest.fail("Numeric timeout 1002 did not fire within 0.5 seconds") + + try: + await asyncio.wait_for(numeric_interval_2001_fired.wait(), timeout=1.0) + except TimeoutError: + pytest.fail("Numeric interval 2001 did not fire within 1 second") + + try: + await asyncio.wait_for(numeric_interval_cancelled.wait(), timeout=2.0) + except TimeoutError: + pytest.fail("Numeric interval 2001 was not cancelled within 2 seconds") + + # Verify numeric interval ran at least twice + assert numeric_interval_count >= 2, ( + f"Expected numeric interval to run at least 2 times, got {numeric_interval_count}" + ) + + # Verify numeric timeout was cancelled + assert numeric_timeout_cancelled.is_set(), ( + "Numeric timeout 3001 should have been cancelled" + ) + + # Wait for duplicate timeout (only last one should fire) + try: + await asyncio.wait_for(duplicate_timeout_fired.wait(), timeout=1.0) + except TimeoutError: + pytest.fail("Duplicate numeric timeout did not fire within 1 second") + + # Wait for component method tests + try: + await asyncio.wait_for(component_timeout_fired.wait(), timeout=0.5) + except TimeoutError: + pytest.fail("Component numeric timeout did not fire within 0.5 seconds") + + try: + await asyncio.wait_for(component_interval_fired.wait(), timeout=1.0) + except TimeoutError: + pytest.fail("Component numeric interval did not fire within 1 second") + + # Wait for edge case tests + try: + await asyncio.wait_for(zero_id_timeout_fired.wait(), timeout=0.5) + except TimeoutError: + pytest.fail("Zero ID timeout did not fire within 0.5 seconds") + + try: + await asyncio.wait_for(max_id_timeout_fired.wait(), timeout=0.5) + except TimeoutError: + pytest.fail("Max ID timeout did not fire within 0.5 seconds") + + # Wait for final results + try: + await asyncio.wait_for(final_results_logged.wait(), timeout=3.0) + except TimeoutError: + pytest.fail("Final results were not logged within 3 seconds") + + # Verify results + assert timeout_count >= 6, f"Expected at least 6 timeouts, got {timeout_count}" + assert interval_count >= 3, ( + f"Expected at least 3 interval fires, got {interval_count}" + ) From c8fcc258c35ef05e6c22278c6db964126582834b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 21:32:24 -1000 Subject: [PATCH 05/16] cleanup --- .../fixtures/scheduler_numeric_id_test.yaml | 29 +++++++++++- .../fixtures/scheduler_retry_test.yaml | 21 --------- .../test_scheduler_numeric_id_test.py | 44 ++++++++++++++++++- .../integration/test_scheduler_retry_test.py | 19 +------- 4 files changed, 70 insertions(+), 43 deletions(-) diff --git a/tests/integration/fixtures/scheduler_numeric_id_test.yaml b/tests/integration/fixtures/scheduler_numeric_id_test.yaml index f8265a7832..29b547d66d 100644 --- a/tests/integration/fixtures/scheduler_numeric_id_test.yaml +++ b/tests/integration/fixtures/scheduler_numeric_id_test.yaml @@ -17,6 +17,9 @@ globals: - id: interval_counter type: int initial_value: '0' + - id: retry_counter + type: int + initial_value: '0' - id: tests_done type: bool initial_value: 'false' @@ -109,11 +112,33 @@ script: id(timeout_counter) += 1; }); + // Test 10: set_retry with numeric ID + App.scheduler.set_retry(component1, 6001U, 50, 3, + [](uint8_t retry_countdown) { + id(retry_counter)++; + ESP_LOGI("test", "Numeric retry 6001 attempt %d (countdown=%d)", + id(retry_counter), retry_countdown); + if (id(retry_counter) >= 2) { + ESP_LOGI("test", "Numeric retry 6001 done"); + return RetryResult::DONE; + } + return RetryResult::RETRY; + }); + + // Test 11: cancel_retry with numeric ID + App.scheduler.set_retry(component1, 6002U, 100, 5, + [](uint8_t retry_countdown) { + ESP_LOGE("test", "ERROR: Numeric retry 6002 should have been cancelled"); + return RetryResult::RETRY; + }); + App.scheduler.cancel_retry(component1, 6002U); + ESP_LOGI("test", "Cancelled numeric retry 6002"); + - id: report_results then: - lambda: |- - ESP_LOGI("test", "Final results - Timeouts: %d, Intervals: %d", - id(timeout_counter), id(interval_counter)); + ESP_LOGI("test", "Final results - Timeouts: %d, Intervals: %d, Retries: %d", + id(timeout_counter), id(interval_counter), id(retry_counter)); sensor: - platform: template diff --git a/tests/integration/fixtures/scheduler_retry_test.yaml b/tests/integration/fixtures/scheduler_retry_test.yaml index 11fff6c395..ffe9082a69 100644 --- a/tests/integration/fixtures/scheduler_retry_test.yaml +++ b/tests/integration/fixtures/scheduler_retry_test.yaml @@ -43,9 +43,6 @@ globals: - id: static_char_retry_counter type: int initial_value: '0' - - id: mixed_cancel_result - type: bool - initial_value: 'false' # Using different component types for each test to ensure isolation sensor: @@ -271,23 +268,6 @@ script: ESP_LOGI("test", "Static cancel result: %s", result ? "true" : "false"); }); - # Test 10: Mix string and const char* cancel - - logger.log: "=== Test 10: Mixed string/const char* ===" - - lambda: |- - auto *component = id(immediate_done_sensor); - - // Set with std::string - std::string str_name = "mixed_retry"; - App.scheduler.set_retry(component, str_name, 40, 3, - [](uint8_t retry_countdown) { - ESP_LOGI("test", "Mixed retry - should be cancelled"); - return RetryResult::RETRY; - }); - - // Cancel with const char* - id(mixed_cancel_result) = App.scheduler.cancel_retry(component, "mixed_retry"); - ESP_LOGI("test", "Mixed cancel result: %s", id(mixed_cancel_result) ? "true" : "false"); - # Wait for all tests to complete before reporting - delay: 500ms @@ -303,5 +283,4 @@ script: ESP_LOGI("test", "Multiple same name counter: %d (expected 20+)", id(multiple_same_name_counter)); ESP_LOGI("test", "Const char retry counter: %d (expected 1)", id(const_char_retry_counter)); ESP_LOGI("test", "Static char retry counter: %d (expected 1)", id(static_char_retry_counter)); - ESP_LOGI("test", "Mixed cancel result: %s (expected true)", id(mixed_cancel_result) ? "true" : "false"); ESP_LOGI("test", "All retry tests completed"); diff --git a/tests/integration/test_scheduler_numeric_id_test.py b/tests/integration/test_scheduler_numeric_id_test.py index e56d889cd1..510256b9a4 100644 --- a/tests/integration/test_scheduler_numeric_id_test.py +++ b/tests/integration/test_scheduler_numeric_id_test.py @@ -18,6 +18,7 @@ async def test_scheduler_numeric_id_test( # Track counts timeout_count = 0 interval_count = 0 + retry_count = 0 # Events for each test completion numeric_timeout_1001_fired = asyncio.Event() @@ -30,13 +31,17 @@ async def test_scheduler_numeric_id_test( component_interval_fired = asyncio.Event() zero_id_timeout_fired = asyncio.Event() max_id_timeout_fired = asyncio.Event() + numeric_retry_done = asyncio.Event() + numeric_retry_cancelled = asyncio.Event() final_results_logged = asyncio.Event() # Track interval counts numeric_interval_count = 0 + numeric_retry_count = 0 def on_log_line(line: str) -> None: - nonlocal timeout_count, interval_count, numeric_interval_count + nonlocal timeout_count, interval_count, retry_count + nonlocal numeric_interval_count, numeric_retry_count # Strip ANSI color codes clean_line = re.sub(r"\x1b\[[0-9;]*m", "", line) @@ -88,12 +93,27 @@ async def test_scheduler_numeric_id_test( max_id_timeout_fired.set() timeout_count += 1 + # Check for numeric retry tests + elif "Numeric retry 6001 attempt" in clean_line: + match = re.search(r"attempt (\d+)", clean_line) + if match: + numeric_retry_count = int(match.group(1)) + + elif "Numeric retry 6001 done" in clean_line: + numeric_retry_done.set() + + elif "Cancelled numeric retry 6002" in clean_line: + numeric_retry_cancelled.set() + # Check for final results elif "Final results" in clean_line: - match = re.search(r"Timeouts: (\d+), Intervals: (\d+)", clean_line) + match = re.search( + r"Timeouts: (\d+), Intervals: (\d+), Retries: (\d+)", clean_line + ) if match: timeout_count = int(match.group(1)) interval_count = int(match.group(2)) + retry_count = int(match.group(3)) final_results_logged.set() async with ( @@ -164,6 +184,23 @@ async def test_scheduler_numeric_id_test( except TimeoutError: pytest.fail("Max ID timeout did not fire within 0.5 seconds") + # Wait for numeric retry tests + try: + await asyncio.wait_for(numeric_retry_done.wait(), timeout=1.0) + except TimeoutError: + pytest.fail( + f"Numeric retry 6001 did not complete. Count: {numeric_retry_count}" + ) + + assert numeric_retry_count >= 2, ( + f"Expected at least 2 numeric retry attempts, got {numeric_retry_count}" + ) + + # Verify numeric retry was cancelled + assert numeric_retry_cancelled.is_set(), ( + "Numeric retry 6002 should have been cancelled" + ) + # Wait for final results try: await asyncio.wait_for(final_results_logged.wait(), timeout=3.0) @@ -175,3 +212,6 @@ async def test_scheduler_numeric_id_test( assert interval_count >= 3, ( f"Expected at least 3 interval fires, got {interval_count}" ) + assert retry_count >= 2, ( + f"Expected at least 2 retry attempts, got {retry_count}" + ) diff --git a/tests/integration/test_scheduler_retry_test.py b/tests/integration/test_scheduler_retry_test.py index c04b7197c9..910034e5bb 100644 --- a/tests/integration/test_scheduler_retry_test.py +++ b/tests/integration/test_scheduler_retry_test.py @@ -25,7 +25,6 @@ async def test_scheduler_retry_test( multiple_name_done = asyncio.Event() const_char_done = asyncio.Event() static_char_done = asyncio.Event() - mixed_cancel_done = asyncio.Event() test_complete = asyncio.Event() # Track retry counts @@ -42,14 +41,13 @@ async def test_scheduler_retry_test( # Track specific test results cancel_result = None empty_cancel_result = None - mixed_cancel_result = None backoff_intervals = [] def on_log_line(line: str) -> None: nonlocal simple_retry_count, backoff_retry_count, immediate_done_count nonlocal cancel_retry_count, empty_name_retry_count, component_retry_count nonlocal multiple_name_count, const_char_retry_count, static_char_retry_count - nonlocal cancel_result, empty_cancel_result, mixed_cancel_result + nonlocal cancel_result, empty_cancel_result # Strip ANSI color codes clean_line = re.sub(r"\x1b\[[0-9;]*m", "", line) @@ -129,11 +127,6 @@ async def test_scheduler_retry_test( # This is part of test 9, but we don't track it separately pass - # Mixed cancel test - elif "Mixed cancel result:" in clean_line: - mixed_cancel_result = "true" in clean_line - mixed_cancel_done.set() - # Test completion elif "All retry tests completed" in clean_line: test_complete.set() @@ -279,16 +272,6 @@ async def test_scheduler_retry_test( f"Expected 1 static char retry call, got {static_char_retry_count}" ) - # Wait for mixed cancel test - try: - await asyncio.wait_for(mixed_cancel_done.wait(), timeout=1.0) - except TimeoutError: - pytest.fail("Mixed cancel test did not complete") - - assert mixed_cancel_result is True, ( - "Mixed string/const char cancel should have succeeded" - ) - # Wait for test completion try: await asyncio.wait_for(test_complete.wait(), timeout=1.0) From 16d734277270df0b6326ac3b381af240165143df Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 21:37:16 -1000 Subject: [PATCH 06/16] cleanup --- esphome/core/scheduler.cpp | 42 +++++++++++++++++--------------------- esphome/core/scheduler.h | 3 +++ 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 8a63b177ff..2c5e6b593a 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -196,14 +196,19 @@ void HOT Scheduler::set_interval(Component *component, const char *name, uint32_ std::move(func)); } -bool HOT Scheduler::cancel_timeout(Component *component, const char *name) { +// Common implementation for cancel operations - handles locking +bool HOT Scheduler::cancel_item_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, + SchedulerItem::Type type, bool match_retry) { LockGuard guard{this->lock_}; - return this->cancel_item_locked_(component, NameType::STATIC_STRING, name, 0, SchedulerItem::TIMEOUT); + return this->cancel_item_locked_(component, name_type, static_name, hash_or_id, type, match_retry); +} + +bool HOT Scheduler::cancel_timeout(Component *component, const char *name) { + return this->cancel_item_(component, NameType::STATIC_STRING, name, 0, SchedulerItem::TIMEOUT); } bool HOT Scheduler::cancel_interval(Component *component, const char *name) { - LockGuard guard{this->lock_}; - return this->cancel_item_locked_(component, NameType::STATIC_STRING, name, 0, SchedulerItem::INTERVAL); + return this->cancel_item_(component, NameType::STATIC_STRING, name, 0, SchedulerItem::INTERVAL); } // Public API - std::string (hashed) versions - computes FNV-1a hash internally @@ -220,15 +225,11 @@ void HOT Scheduler::set_interval(Component *component, const std::string &name, } bool HOT Scheduler::cancel_timeout(Component *component, const std::string &name) { - LockGuard guard{this->lock_}; - return this->cancel_item_locked_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), - SchedulerItem::TIMEOUT); + return this->cancel_item_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), SchedulerItem::TIMEOUT); } bool HOT Scheduler::cancel_interval(Component *component, const std::string &name) { - LockGuard guard{this->lock_}; - return this->cancel_item_locked_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), - SchedulerItem::INTERVAL); + return this->cancel_item_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), SchedulerItem::INTERVAL); } // Public API - uint32_t (numeric ID) versions @@ -243,13 +244,11 @@ void HOT Scheduler::set_interval(Component *component, uint32_t id, uint32_t int } bool HOT Scheduler::cancel_timeout(Component *component, uint32_t id) { - LockGuard guard{this->lock_}; - return this->cancel_item_locked_(component, NameType::NUMERIC_ID, nullptr, id, SchedulerItem::TIMEOUT); + return this->cancel_item_(component, NameType::NUMERIC_ID, nullptr, id, SchedulerItem::TIMEOUT); } bool HOT Scheduler::cancel_interval(Component *component, uint32_t id) { - LockGuard guard{this->lock_}; - return this->cancel_item_locked_(component, NameType::NUMERIC_ID, nullptr, id, SchedulerItem::INTERVAL); + return this->cancel_item_(component, NameType::NUMERIC_ID, nullptr, id, SchedulerItem::INTERVAL); } struct RetryArgs { @@ -336,9 +335,8 @@ void HOT Scheduler::set_retry(Component *component, const char *name, uint32_t i } bool HOT Scheduler::cancel_retry(Component *component, const char *name) { - LockGuard guard{this->lock_}; - return this->cancel_item_locked_(component, NameType::STATIC_STRING, name, 0, SchedulerItem::TIMEOUT, - /* match_retry= */ true); + return this->cancel_item_(component, NameType::STATIC_STRING, name, 0, SchedulerItem::TIMEOUT, + /* match_retry= */ true); } // Public API - std::string (hashed) versions @@ -350,9 +348,8 @@ void HOT Scheduler::set_retry(Component *component, const std::string &name, uin } bool HOT Scheduler::cancel_retry(Component *component, const std::string &name) { - LockGuard guard{this->lock_}; - return this->cancel_item_locked_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), - SchedulerItem::TIMEOUT, /* match_retry= */ true); + return this->cancel_item_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), SchedulerItem::TIMEOUT, + /* match_retry= */ true); } // Public API - uint32_t (numeric ID) versions @@ -363,9 +360,8 @@ void HOT Scheduler::set_retry(Component *component, uint32_t id, uint32_t initia } bool HOT Scheduler::cancel_retry(Component *component, uint32_t id) { - LockGuard guard{this->lock_}; - return this->cancel_item_locked_(component, NameType::NUMERIC_ID, nullptr, id, SchedulerItem::TIMEOUT, - /* match_retry= */ true); + return this->cancel_item_(component, NameType::NUMERIC_ID, nullptr, id, SchedulerItem::TIMEOUT, + /* match_retry= */ true); } optional HOT Scheduler::next_schedule_in(uint32_t now) { diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 116b79b75a..2ba17e805e 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -249,6 +249,9 @@ class Scheduler { std::unique_ptr get_item_from_pool_locked_(); private: + // Common implementation for cancel operations - handles locking + bool cancel_item_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, + SchedulerItem::Type type, bool match_retry = false); // Helper to cancel items - must be called with lock held // name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id bool cancel_item_locked_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, From ba36934f91a47b38ef4e9fae910c4278c6128042 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 21:46:19 -1000 Subject: [PATCH 07/16] minimize diff --- esphome/core/scheduler.cpp | 127 +++++++++++++++++-------------------- esphome/core/scheduler.h | 9 ++- 2 files changed, 63 insertions(+), 73 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 2c5e6b593a..49e1d3c629 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -161,6 +161,11 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type item->set_next_execution(now + delay); } +#ifdef ESPHOME_DEBUG_SCHEDULER + this->debug_log_timer_(item.get(), name_type == NameType::STATIC_STRING, + name_type == NameType::STATIC_STRING ? static_name : nullptr, type, delay, now); +#endif /* ESPHOME_DEBUG_SCHEDULER */ + // For retries, check if there's a cancelled timeout first if (is_retry && type == SchedulerItem::TIMEOUT) { if (has_cancelled_timeout_in_container_locked_(this->items_, component, name_type, static_name, hash_or_id, @@ -175,78 +180,60 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type } } - // Cancel existing items with same name/id (unless skip_cancel is true) + // If name is provided, do atomic cancel-and-add (unless skip_cancel is true) + // Cancel existing items if (!skip_cancel) { this->cancel_item_locked_(component, name_type, static_name, hash_or_id, type); } - - // Add new item directly to to_add_ since we have the lock held + // Add new item directly to to_add_ + // since we have the lock held this->to_add_.push_back(std::move(item)); } -// Public API - const char* (static string) versions void HOT Scheduler::set_timeout(Component *component, const char *name, uint32_t timeout, std::function func) { this->set_timer_common_(component, SchedulerItem::TIMEOUT, NameType::STATIC_STRING, name, 0, timeout, std::move(func)); } +void HOT Scheduler::set_timeout(Component *component, const std::string &name, uint32_t timeout, + std::function func) { + this->set_timer_common_(component, SchedulerItem::TIMEOUT, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), + timeout, std::move(func)); +} +void HOT Scheduler::set_timeout(Component *component, uint32_t id, uint32_t timeout, std::function func) { + this->set_timer_common_(component, SchedulerItem::TIMEOUT, NameType::NUMERIC_ID, nullptr, id, timeout, + std::move(func)); +} +bool HOT Scheduler::cancel_timeout(Component *component, const std::string &name) { + return this->cancel_item_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), SchedulerItem::TIMEOUT); +} +bool HOT Scheduler::cancel_timeout(Component *component, const char *name) { + return this->cancel_item_(component, NameType::STATIC_STRING, name, 0, SchedulerItem::TIMEOUT); +} +bool HOT Scheduler::cancel_timeout(Component *component, uint32_t id) { + return this->cancel_item_(component, NameType::NUMERIC_ID, nullptr, id, SchedulerItem::TIMEOUT); +} +void HOT Scheduler::set_interval(Component *component, const std::string &name, uint32_t interval, + std::function func) { + this->set_timer_common_(component, SchedulerItem::INTERVAL, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), + interval, std::move(func)); +} + void HOT Scheduler::set_interval(Component *component, const char *name, uint32_t interval, std::function func) { this->set_timer_common_(component, SchedulerItem::INTERVAL, NameType::STATIC_STRING, name, 0, interval, std::move(func)); } - -// Common implementation for cancel operations - handles locking -bool HOT Scheduler::cancel_item_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, - SchedulerItem::Type type, bool match_retry) { - LockGuard guard{this->lock_}; - return this->cancel_item_locked_(component, name_type, static_name, hash_or_id, type, match_retry); -} - -bool HOT Scheduler::cancel_timeout(Component *component, const char *name) { - return this->cancel_item_(component, NameType::STATIC_STRING, name, 0, SchedulerItem::TIMEOUT); -} - -bool HOT Scheduler::cancel_interval(Component *component, const char *name) { - return this->cancel_item_(component, NameType::STATIC_STRING, name, 0, SchedulerItem::INTERVAL); -} - -// Public API - std::string (hashed) versions - computes FNV-1a hash internally -void HOT Scheduler::set_timeout(Component *component, const std::string &name, uint32_t timeout, - std::function func) { - this->set_timer_common_(component, SchedulerItem::TIMEOUT, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), - timeout, std::move(func)); -} - -void HOT Scheduler::set_interval(Component *component, const std::string &name, uint32_t interval, - std::function func) { - this->set_timer_common_(component, SchedulerItem::INTERVAL, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), - interval, std::move(func)); -} - -bool HOT Scheduler::cancel_timeout(Component *component, const std::string &name) { - return this->cancel_item_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), SchedulerItem::TIMEOUT); -} - -bool HOT Scheduler::cancel_interval(Component *component, const std::string &name) { - return this->cancel_item_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), SchedulerItem::INTERVAL); -} - -// Public API - uint32_t (numeric ID) versions -void HOT Scheduler::set_timeout(Component *component, uint32_t id, uint32_t timeout, std::function func) { - this->set_timer_common_(component, SchedulerItem::TIMEOUT, NameType::NUMERIC_ID, nullptr, id, timeout, - std::move(func)); -} - void HOT Scheduler::set_interval(Component *component, uint32_t id, uint32_t interval, std::function func) { this->set_timer_common_(component, SchedulerItem::INTERVAL, NameType::NUMERIC_ID, nullptr, id, interval, std::move(func)); } - -bool HOT Scheduler::cancel_timeout(Component *component, uint32_t id) { - return this->cancel_item_(component, NameType::NUMERIC_ID, nullptr, id, SchedulerItem::TIMEOUT); +bool HOT Scheduler::cancel_interval(Component *component, const std::string &name) { + return this->cancel_item_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), SchedulerItem::INTERVAL); +} +bool HOT Scheduler::cancel_interval(Component *component, const char *name) { + return this->cancel_item_(component, NameType::STATIC_STRING, name, 0, SchedulerItem::INTERVAL); } - bool HOT Scheduler::cancel_interval(Component *component, uint32_t id) { return this->cancel_item_(component, NameType::NUMERIC_ID, nullptr, id, SchedulerItem::INTERVAL); } @@ -271,8 +258,9 @@ void retry_handler(const std::shared_ptr &args) { RetryResult const retry_result = args->func(--args->retry_countdown); if (retry_result == RetryResult::DONE || args->retry_countdown <= 0) return; - // Second execution of `func` happens after `initial_wait_time` - // static_name is owned by the shared_ptr which is captured in the lambda + // second execution of `func` happens after `initial_wait_time` + // args->name_ is owned by the shared_ptr + // which is captured in the lambda and outlives the SchedulerItem const char *static_name = (args->name_type == Scheduler::NameType::STATIC_STRING) ? args->name_.static_name : nullptr; uint32_t hash_or_id = (args->name_type != Scheduler::NameType::STATIC_STRING) ? args->name_.hash_or_id : 0; args->scheduler->set_timer_common_( @@ -283,17 +271,10 @@ void retry_handler(const std::shared_ptr &args) { args->current_interval *= args->backoff_increase_factor; } -// Common implementation for retry -// name_type determines storage type: STATIC_STRING uses static_name, others use hash_or_id void HOT Scheduler::set_retry_common_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, uint32_t initial_wait_time, uint8_t max_attempts, std::function func, float backoff_increase_factor) { - // Cancel existing retry with same name/id - { - LockGuard guard{this->lock_}; - this->cancel_item_locked_(component, name_type, static_name, hash_or_id, SchedulerItem::TIMEOUT, - /* match_retry= */ true); - } + this->cancel_retry(component, name_type, static_name, hash_or_id); if (initial_wait_time == SCHEDULER_DONT_RUN) return; @@ -327,19 +308,21 @@ void HOT Scheduler::set_retry_common_(Component *component, NameType name_type, /* is_retry= */ true); } -// Public API - const char* (static string) versions void HOT Scheduler::set_retry(Component *component, const char *name, uint32_t initial_wait_time, uint8_t max_attempts, std::function func, float backoff_increase_factor) { this->set_retry_common_(component, NameType::STATIC_STRING, name, 0, initial_wait_time, max_attempts, std::move(func), backoff_increase_factor); } -bool HOT Scheduler::cancel_retry(Component *component, const char *name) { - return this->cancel_item_(component, NameType::STATIC_STRING, name, 0, SchedulerItem::TIMEOUT, +bool HOT Scheduler::cancel_retry(Component *component, NameType name_type, const char *static_name, + uint32_t hash_or_id) { + return this->cancel_item_(component, name_type, static_name, hash_or_id, SchedulerItem::TIMEOUT, /* match_retry= */ true); } +bool HOT Scheduler::cancel_retry(Component *component, const char *name) { + return this->cancel_retry(component, NameType::STATIC_STRING, name, 0); +} -// Public API - std::string (hashed) versions void HOT Scheduler::set_retry(Component *component, const std::string &name, uint32_t initial_wait_time, uint8_t max_attempts, std::function func, float backoff_increase_factor) { @@ -348,11 +331,9 @@ void HOT Scheduler::set_retry(Component *component, const std::string &name, uin } bool HOT Scheduler::cancel_retry(Component *component, const std::string &name) { - return this->cancel_item_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name), SchedulerItem::TIMEOUT, - /* match_retry= */ true); + return this->cancel_retry(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name)); } -// Public API - uint32_t (numeric ID) versions void HOT Scheduler::set_retry(Component *component, uint32_t id, uint32_t initial_wait_time, uint8_t max_attempts, std::function func, float backoff_increase_factor) { this->set_retry_common_(component, NameType::NUMERIC_ID, nullptr, id, initial_wait_time, max_attempts, @@ -360,8 +341,7 @@ void HOT Scheduler::set_retry(Component *component, uint32_t id, uint32_t initia } bool HOT Scheduler::cancel_retry(Component *component, uint32_t id) { - return this->cancel_item_(component, NameType::NUMERIC_ID, nullptr, id, SchedulerItem::TIMEOUT, - /* match_retry= */ true); + return this->cancel_retry(component, NameType::NUMERIC_ID, nullptr, id); } optional HOT Scheduler::next_schedule_in(uint32_t now) { @@ -614,6 +594,13 @@ uint32_t HOT Scheduler::execute_item_(SchedulerItem *item, uint32_t now) { return guard.finish(); } +// Common implementation for cancel operations - handles locking +bool HOT Scheduler::cancel_item_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, + SchedulerItem::Type type, bool match_retry) { + LockGuard guard{this->lock_}; + return this->cancel_item_locked_(component, name_type, static_name, hash_or_id, type, match_retry); +} + // Helper to cancel items - must be called with lock held // name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id bool HOT Scheduler::cancel_item_locked_(Component *component, NameType name_type, const char *static_name, diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 2ba17e805e..256808cf92 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -235,6 +235,8 @@ class Scheduler { void set_retry_common_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, uint32_t initial_wait_time, uint8_t max_attempts, std::function func, float backoff_increase_factor); + // Common implementation for cancel_retry + bool cancel_retry(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id); uint64_t millis_64_(uint32_t now); // Cleanup logically deleted items from the scheduler @@ -249,14 +251,15 @@ class Scheduler { std::unique_ptr get_item_from_pool_locked_(); private: - // Common implementation for cancel operations - handles locking - bool cancel_item_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, - SchedulerItem::Type type, bool match_retry = false); // Helper to cancel items - must be called with lock held // name_type determines matching: STATIC_STRING uses static_name, others use hash_or_id bool cancel_item_locked_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, SchedulerItem::Type type, bool match_retry = false); + // Common implementation for cancel operations - handles locking + bool cancel_item_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, + SchedulerItem::Type type, bool match_retry = false); + // Helper to check if two static string names match inline bool HOT names_match_static_(const char *name1, const char *name2) const { // Check pointer equality first (common for static strings), then string contents From 4520f7f646f631b336f0715682b2734549745570 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 21:47:27 -1000 Subject: [PATCH 08/16] minimize diff --- esphome/core/scheduler.h | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 256808cf92..2bde1fbe8a 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -30,21 +30,10 @@ class Scheduler { template friend class DelayAction; public: - // std::string overloads - deprecated, use const char* or uint32_t instead + // std::string overload - deprecated, use const char* or uint32_t instead // Remove before 2026.7.0 ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") void set_timeout(Component *component, const std::string &name, uint32_t timeout, std::function func); - ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") - bool cancel_timeout(Component *component, const std::string &name); - ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") - void set_interval(Component *component, const std::string &name, uint32_t interval, std::function func); - ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") - bool cancel_interval(Component *component, const std::string &name); - ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") - void set_retry(Component *component, const std::string &name, uint32_t initial_wait_time, uint8_t max_attempts, - std::function func, float backoff_increase_factor = 1.0f); - ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") - bool cancel_retry(Component *component, const std::string &name); /** Set a timeout with a const char* name. * @@ -55,12 +44,17 @@ class Scheduler { * - A pointer with lifetime >= the scheduled task */ void set_timeout(Component *component, const char *name, uint32_t timeout, std::function func); - bool cancel_timeout(Component *component, const char *name); - /// Set a timeout with a numeric ID (zero heap allocation) void set_timeout(Component *component, uint32_t id, uint32_t timeout, std::function func); + + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") + bool cancel_timeout(Component *component, const std::string &name); + bool cancel_timeout(Component *component, const char *name); bool cancel_timeout(Component *component, uint32_t id); + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") + void set_interval(Component *component, const std::string &name, uint32_t interval, std::function func); + /** Set an interval with a const char* name. * * IMPORTANT: The provided name pointer must remain valid for the lifetime of the scheduler item. @@ -70,19 +64,26 @@ class Scheduler { * - A pointer with lifetime >= the scheduled task */ void set_interval(Component *component, const char *name, uint32_t interval, std::function func); - bool cancel_interval(Component *component, const char *name); - /// Set an interval with a numeric ID (zero heap allocation) void set_interval(Component *component, uint32_t id, uint32_t interval, std::function func); + + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") + bool cancel_interval(Component *component, const std::string &name); + bool cancel_interval(Component *component, const char *name); bool cancel_interval(Component *component, uint32_t id); + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") + void set_retry(Component *component, const std::string &name, uint32_t initial_wait_time, uint8_t max_attempts, + std::function func, float backoff_increase_factor = 1.0f); void set_retry(Component *component, const char *name, uint32_t initial_wait_time, uint8_t max_attempts, std::function func, float backoff_increase_factor = 1.0f); - bool cancel_retry(Component *component, const char *name); - /// Set a retry with a numeric ID (zero heap allocation) void set_retry(Component *component, uint32_t id, uint32_t initial_wait_time, uint8_t max_attempts, std::function func, float backoff_increase_factor = 1.0f); + + ESPDEPRECATED("Use const char* or uint32_t overload instead. Removed in 2026.7.0", "2026.1.0") + bool cancel_retry(Component *component, const std::string &name); + bool cancel_retry(Component *component, const char *name); bool cancel_retry(Component *component, uint32_t id); // Calculate when the next scheduled item should run From 25b7d1ea1560ef7c17297362ba33a2ab1d4ac85d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 21:50:03 -1000 Subject: [PATCH 09/16] minimize diff --- esphome/core/scheduler.cpp | 56 +++++++++++++++++++------------------- esphome/core/scheduler.h | 7 ++++- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 49e1d3c629..4e7c4e0c2a 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -75,25 +75,6 @@ static void validate_static_string(const char *name) { // iterating over them from the loop task is fine; but iterating from any other context requires the lock to be held to // avoid the main thread modifying the list while it is being accessed. -// Helper to get or create a scheduler item from the pool -// IMPORTANT: Caller must hold the scheduler lock before calling this function. -std::unique_ptr Scheduler::get_item_from_pool_locked_() { - std::unique_ptr item; - if (!this->scheduler_item_pool_.empty()) { - item = std::move(this->scheduler_item_pool_.back()); - this->scheduler_item_pool_.pop_back(); -#ifdef ESPHOME_DEBUG_SCHEDULER - ESP_LOGD(TAG, "Reused item from pool (pool size now: %zu)", this->scheduler_item_pool_.size()); -#endif - } else { - item = make_unique(); -#ifdef ESPHOME_DEBUG_SCHEDULER - ESP_LOGD(TAG, "Allocated new item (pool empty)"); -#endif - } - return item; -} - // Common implementation for both timeout and interval // name_type determines storage type: STATIC_STRING uses static_name, others use hash_or_id void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type type, NameType name_type, @@ -167,17 +148,17 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type #endif /* ESPHOME_DEBUG_SCHEDULER */ // For retries, check if there's a cancelled timeout first - if (is_retry && type == SchedulerItem::TIMEOUT) { - if (has_cancelled_timeout_in_container_locked_(this->items_, component, name_type, static_name, hash_or_id, - /* match_retry= */ true) || - has_cancelled_timeout_in_container_locked_(this->to_add_, component, name_type, static_name, hash_or_id, - /* match_retry= */ true)) { - // Skip scheduling - the retry was cancelled + // Skip check for anonymous retries (STATIC_STRING with nullptr) - they can't be cancelled by name + if (is_retry && (name_type != NameType::STATIC_STRING || static_name != nullptr) && type == SchedulerItem::TIMEOUT && + (has_cancelled_timeout_in_container_locked_(this->items_, component, name_type, static_name, hash_or_id, + /* match_retry= */ true) || + has_cancelled_timeout_in_container_locked_(this->to_add_, component, name_type, static_name, hash_or_id, + /* match_retry= */ true))) { + // Skip scheduling - the retry was cancelled #ifdef ESPHOME_DEBUG_SCHEDULER - ESP_LOGD(TAG, "Skipping retry - found cancelled item"); + ESP_LOGD(TAG, "Skipping retry - found cancelled item"); #endif - return; - } + return; } // If name is provided, do atomic cancel-and-add (unless skip_cancel is true) @@ -849,4 +830,23 @@ void Scheduler::debug_log_timer_(const SchedulerItem *item, bool is_static_strin } #endif /* ESPHOME_DEBUG_SCHEDULER */ +// Helper to get or create a scheduler item from the pool +// IMPORTANT: Caller must hold the scheduler lock before calling this function. +std::unique_ptr Scheduler::get_item_from_pool_locked_() { + std::unique_ptr item; + if (!this->scheduler_item_pool_.empty()) { + item = std::move(this->scheduler_item_pool_.back()); + this->scheduler_item_pool_.pop_back(); +#ifdef ESPHOME_DEBUG_SCHEDULER + ESP_LOGD(TAG, "Reused item from pool (pool size now: %zu)", this->scheduler_item_pool_.size()); +#endif + } else { + item = make_unique(); +#ifdef ESPHOME_DEBUG_SCHEDULER + ESP_LOGD(TAG, "Allocated new item (pool empty)"); +#endif + } + return item; +} + } // namespace esphome diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 2bde1fbe8a..4333f74f7d 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -264,6 +264,9 @@ class Scheduler { // Helper to check if two static string names match inline bool HOT names_match_static_(const char *name1, const char *name2) const { // Check pointer equality first (common for static strings), then string contents + // The core ESPHome codebase uses static strings (const char*) for component names, + // making pointer comparison effective. The std::string overloads exist only for + // compatibility with external components but are rarely used in practice. return (name1 != nullptr && name2 != nullptr) && ((name1 == name2) || (strcmp(name1, name2) == 0)); } @@ -275,7 +278,9 @@ class Scheduler { SchedulerItem::Type type, bool match_retry, bool skip_removed = true) const { // THREAD SAFETY: Check for nullptr first to prevent LoadProhibited crashes. On multi-threaded // platforms, items can be moved out of defer_queue_ during processing, leaving nullptr entries. - // This check provides defense-in-depth: helper functions should be safe regardless of caller behavior. + // PR #11305 added nullptr checks in callers (mark_matching_items_removed_locked_() and + // has_cancelled_timeout_in_container_locked_()), but this check provides defense-in-depth: helper + // functions should be safe regardless of caller behavior. // Fixes: https://github.com/esphome/esphome/issues/11940 if (!item) return false; From 38c5421d54778ab5a8c56fd933eaca9ac0d09383 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 21:56:06 -1000 Subject: [PATCH 10/16] name log --- esphome/core/scheduler.cpp | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 4e7c4e0c2a..5ef959a36c 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -32,6 +32,27 @@ static constexpr uint32_t HALF_MAX_UINT32 = std::numeric_limits::max() // max delay to start an interval sequence static constexpr uint32_t MAX_INTERVAL_DELAY = 5000; +// Helper struct for formatting scheduler item names consistently in logs +// Uses a stack buffer to avoid heap allocation +struct SchedulerNameLog { + char buffer[20]; // Enough for "id:4294967295" or "hash:0xFFFFFFFF" + + // Format a scheduler item name for logging + // Returns pointer to formatted string (either static_name or internal buffer) + const char *format(Scheduler::NameType name_type, const char *static_name, uint32_t hash_or_id) { + using NameType = Scheduler::NameType; + if (name_type == NameType::STATIC_STRING) { + return static_name ? static_name : "(null)"; + } else if (name_type == NameType::HASHED_STRING) { + snprintf(buffer, sizeof(buffer), "hash:0x%08" PRIX32, hash_or_id); + return buffer; + } else { // NUMERIC_ID + snprintf(buffer, sizeof(buffer), "id:%" PRIu32, hash_or_id); + return buffer; + } + } +}; + // Uncomment to debug scheduler // #define ESPHOME_DEBUG_SCHEDULER @@ -135,8 +156,9 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type // Calculate random offset (0 to min(interval/2, 5s)) uint32_t offset = (uint32_t) (std::min(delay / 2, MAX_INTERVAL_DELAY) * random_float()); item->set_next_execution(now + offset); + SchedulerNameLog name_log; ESP_LOGV(TAG, "Scheduler interval for %s is %" PRIu32 "ms, offset %" PRIu32 "ms", - name_type == NameType::STATIC_STRING ? static_name : "(id)", delay, offset); + name_log.format(name_type, static_name, hash_or_id), delay, offset); } else { item->interval = 0; item->set_next_execution(now + delay); @@ -156,7 +178,9 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type /* match_retry= */ true))) { // Skip scheduling - the retry was cancelled #ifdef ESPHOME_DEBUG_SCHEDULER - ESP_LOGD(TAG, "Skipping retry - found cancelled item"); + SchedulerNameLog skip_name_log; + ESP_LOGD(TAG, "Skipping retry '%s' - found cancelled item", + skip_name_log.format(name_type, static_name, hash_or_id)); #endif return; } @@ -260,12 +284,14 @@ void HOT Scheduler::set_retry_common_(Component *component, NameType name_type, if (initial_wait_time == SCHEDULER_DONT_RUN) return; + SchedulerNameLog name_log; ESP_LOGVV(TAG, "set_retry(name='%s', initial_wait_time=%" PRIu32 ", max_attempts=%u, backoff_factor=%0.1f)", - name_type == NameType::STATIC_STRING ? static_name : "(id)", initial_wait_time, max_attempts, + name_log.format(name_type, static_name, hash_or_id), initial_wait_time, max_attempts, backoff_increase_factor); if (backoff_increase_factor < 0.0001) { - ESP_LOGE(TAG, "backoff_factor %0.1f too small, using 1.0", backoff_increase_factor); + ESP_LOGE(TAG, "backoff_factor %0.1f too small, using 1.0: %s", backoff_increase_factor, + name_log.format(name_type, static_name, hash_or_id)); backoff_increase_factor = 1; } From bf6d75fd5e62fd4d6b90de8199dff4b43fbc7564 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 22:08:57 -1000 Subject: [PATCH 11/16] fix --- esphome/core/scheduler.cpp | 12 ++++++------ esphome/core/scheduler.h | 2 +- .../fixtures/scheduler_numeric_id_test.yaml | 8 +++++--- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 5ef959a36c..6dac2a36d3 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -279,7 +279,7 @@ void retry_handler(const std::shared_ptr &args) { void HOT Scheduler::set_retry_common_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id, uint32_t initial_wait_time, uint8_t max_attempts, std::function func, float backoff_increase_factor) { - this->cancel_retry(component, name_type, static_name, hash_or_id); + this->cancel_retry_(component, name_type, static_name, hash_or_id); if (initial_wait_time == SCHEDULER_DONT_RUN) return; @@ -321,13 +321,13 @@ void HOT Scheduler::set_retry(Component *component, const char *name, uint32_t i backoff_increase_factor); } -bool HOT Scheduler::cancel_retry(Component *component, NameType name_type, const char *static_name, - uint32_t hash_or_id) { +bool HOT Scheduler::cancel_retry_(Component *component, NameType name_type, const char *static_name, + uint32_t hash_or_id) { return this->cancel_item_(component, name_type, static_name, hash_or_id, SchedulerItem::TIMEOUT, /* match_retry= */ true); } bool HOT Scheduler::cancel_retry(Component *component, const char *name) { - return this->cancel_retry(component, NameType::STATIC_STRING, name, 0); + return this->cancel_retry_(component, NameType::STATIC_STRING, name, 0); } void HOT Scheduler::set_retry(Component *component, const std::string &name, uint32_t initial_wait_time, @@ -338,7 +338,7 @@ void HOT Scheduler::set_retry(Component *component, const std::string &name, uin } bool HOT Scheduler::cancel_retry(Component *component, const std::string &name) { - return this->cancel_retry(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name)); + return this->cancel_retry_(component, NameType::HASHED_STRING, nullptr, fnv1a_hash(name)); } void HOT Scheduler::set_retry(Component *component, uint32_t id, uint32_t initial_wait_time, uint8_t max_attempts, @@ -348,7 +348,7 @@ void HOT Scheduler::set_retry(Component *component, uint32_t id, uint32_t initia } bool HOT Scheduler::cancel_retry(Component *component, uint32_t id) { - return this->cancel_retry(component, NameType::NUMERIC_ID, nullptr, id); + return this->cancel_retry_(component, NameType::NUMERIC_ID, nullptr, id); } optional HOT Scheduler::next_schedule_in(uint32_t now) { diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 4333f74f7d..92ff93879a 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -237,7 +237,7 @@ class Scheduler { uint32_t initial_wait_time, uint8_t max_attempts, std::function func, float backoff_increase_factor); // Common implementation for cancel_retry - bool cancel_retry(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id); + bool cancel_retry_(Component *component, NameType name_type, const char *static_name, uint32_t hash_or_id); uint64_t millis_64_(uint32_t now); // Cleanup logically deleted items from the scheduler diff --git a/tests/integration/fixtures/scheduler_numeric_id_test.yaml b/tests/integration/fixtures/scheduler_numeric_id_test.yaml index 29b547d66d..bf60f2fda9 100644 --- a/tests/integration/fixtures/scheduler_numeric_id_test.yaml +++ b/tests/integration/fixtures/scheduler_numeric_id_test.yaml @@ -88,11 +88,13 @@ script: }); // Test set_interval with uint32_t ID - this->set_interval(5002U, 400, []() { + // Capture 'this' pointer so we can cancel with correct component + auto *self = this; + this->set_interval(5002U, 400, [self]() { ESP_LOGI("test", "Component numeric interval 5002 fired"); id(interval_counter) += 1; - // Cancel after first fire - App.scheduler.cancel_interval(nullptr, 5002U); + // Cancel after first fire - must use same component pointer + App.scheduler.cancel_interval(self, 5002U); }); } }; From edde7194c9286b7a7864634e0faeb4a7bf6ed453 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 22:19:40 -1000 Subject: [PATCH 12/16] no ram increase --- esphome/core/progmem.h | 4 ++++ esphome/core/scheduler.cpp | 36 +++++++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/esphome/core/progmem.h b/esphome/core/progmem.h index d1594f47e7..fe9c9b5a75 100644 --- a/esphome/core/progmem.h +++ b/esphome/core/progmem.h @@ -8,11 +8,15 @@ // ESP8266 uses Arduino macros #define ESPHOME_F(string_literal) F(string_literal) #define ESPHOME_PGM_P PGM_P +#define ESPHOME_PSTR(s) PSTR(s) #define ESPHOME_strncpy_P strncpy_P #define ESPHOME_strncat_P strncat_P +#define ESPHOME_snprintf_P snprintf_P #else #define ESPHOME_F(string_literal) (string_literal) #define ESPHOME_PGM_P const char * +#define ESPHOME_PSTR(s) (s) #define ESPHOME_strncpy_P strncpy #define ESPHOME_strncat_P strncat +#define ESPHOME_snprintf_P snprintf #endif diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 6dac2a36d3..39fa101be8 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -5,6 +5,7 @@ #include "esphome/core/hal.h" #include "esphome/core/helpers.h" #include "esphome/core/log.h" +#include "esphome/core/progmem.h" #include #include #include @@ -32,26 +33,33 @@ static constexpr uint32_t HALF_MAX_UINT32 = std::numeric_limits::max() // max delay to start an interval sequence static constexpr uint32_t MAX_INTERVAL_DELAY = 5000; +#if defined(ESPHOME_LOG_HAS_VERBOSE) || defined(ESPHOME_DEBUG_SCHEDULER) // Helper struct for formatting scheduler item names consistently in logs // Uses a stack buffer to avoid heap allocation +// Uses ESPHOME_snprintf_P/ESPHOME_PSTR for ESP8266 to keep format strings in flash struct SchedulerNameLog { - char buffer[20]; // Enough for "id:4294967295" or "hash:0xFFFFFFFF" + char buffer[20]; // Enough for "id:4294967295" or "hash:0xFFFFFFFF" or "(null)" // Format a scheduler item name for logging // Returns pointer to formatted string (either static_name or internal buffer) const char *format(Scheduler::NameType name_type, const char *static_name, uint32_t hash_or_id) { using NameType = Scheduler::NameType; if (name_type == NameType::STATIC_STRING) { - return static_name ? static_name : "(null)"; + if (static_name) + return static_name; + // Copy "(null)" to buffer to keep it in flash on ESP8266 + ESPHOME_strncpy_P(buffer, ESPHOME_PSTR("(null)"), sizeof(buffer)); + return buffer; } else if (name_type == NameType::HASHED_STRING) { - snprintf(buffer, sizeof(buffer), "hash:0x%08" PRIX32, hash_or_id); + ESPHOME_snprintf_P(buffer, sizeof(buffer), ESPHOME_PSTR("hash:0x%08" PRIX32), hash_or_id); return buffer; } else { // NUMERIC_ID - snprintf(buffer, sizeof(buffer), "id:%" PRIu32, hash_or_id); + ESPHOME_snprintf_P(buffer, sizeof(buffer), ESPHOME_PSTR("id:%" PRIu32), hash_or_id); return buffer; } } }; +#endif // Uncomment to debug scheduler // #define ESPHOME_DEBUG_SCHEDULER @@ -156,9 +164,11 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type // Calculate random offset (0 to min(interval/2, 5s)) uint32_t offset = (uint32_t) (std::min(delay / 2, MAX_INTERVAL_DELAY) * random_float()); item->set_next_execution(now + offset); +#ifdef ESPHOME_LOG_HAS_VERBOSE SchedulerNameLog name_log; ESP_LOGV(TAG, "Scheduler interval for %s is %" PRIu32 "ms, offset %" PRIu32 "ms", name_log.format(name_type, static_name, hash_or_id), delay, offset); +#endif } else { item->interval = 0; item->set_next_execution(now + delay); @@ -284,17 +294,21 @@ void HOT Scheduler::set_retry_common_(Component *component, NameType name_type, if (initial_wait_time == SCHEDULER_DONT_RUN) return; - SchedulerNameLog name_log; - ESP_LOGVV(TAG, "set_retry(name='%s', initial_wait_time=%" PRIu32 ", max_attempts=%u, backoff_factor=%0.1f)", - name_log.format(name_type, static_name, hash_or_id), initial_wait_time, max_attempts, - backoff_increase_factor); - if (backoff_increase_factor < 0.0001) { - ESP_LOGE(TAG, "backoff_factor %0.1f too small, using 1.0: %s", backoff_increase_factor, - name_log.format(name_type, static_name, hash_or_id)); + ESP_LOGE(TAG, "set_retry: backoff_factor %0.1f too small, using 1.0: %s", backoff_increase_factor, + (name_type == NameType::STATIC_STRING && static_name) ? static_name : ""); backoff_increase_factor = 1; } +#ifdef ESPHOME_LOG_HAS_VERY_VERBOSE + { + SchedulerNameLog name_log; + ESP_LOGVV(TAG, "set_retry(name='%s', initial_wait_time=%" PRIu32 ", max_attempts=%u, backoff_factor=%0.1f)", + name_log.format(name_type, static_name, hash_or_id), initial_wait_time, max_attempts, + backoff_increase_factor); + } +#endif + auto args = std::make_shared(); args->func = std::move(func); args->component = component; From 4e2c635d14d9ffb5884d0acd4059568470555e4a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 22:21:29 -1000 Subject: [PATCH 13/16] no ram increase --- esphome/core/scheduler.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 39fa101be8..3052487d5c 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -294,12 +294,6 @@ void HOT Scheduler::set_retry_common_(Component *component, NameType name_type, if (initial_wait_time == SCHEDULER_DONT_RUN) return; - if (backoff_increase_factor < 0.0001) { - ESP_LOGE(TAG, "set_retry: backoff_factor %0.1f too small, using 1.0: %s", backoff_increase_factor, - (name_type == NameType::STATIC_STRING && static_name) ? static_name : ""); - backoff_increase_factor = 1; - } - #ifdef ESPHOME_LOG_HAS_VERY_VERBOSE { SchedulerNameLog name_log; @@ -309,6 +303,12 @@ void HOT Scheduler::set_retry_common_(Component *component, NameType name_type, } #endif + if (backoff_increase_factor < 0.0001) { + ESP_LOGE(TAG, "set_retry: backoff_factor %0.1f too small, using 1.0: %s", backoff_increase_factor, + (name_type == NameType::STATIC_STRING && static_name) ? static_name : ""); + backoff_increase_factor = 1; + } + auto args = std::make_shared(); args->func = std::move(func); args->component = component; From c73a4125371eb3a787dddf44a3bcd9c6a5886d25 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 22:30:17 -1000 Subject: [PATCH 14/16] tweaks --- esphome/core/scheduler.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 3052487d5c..902feeb115 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -446,10 +446,11 @@ void HOT Scheduler::call(uint32_t now) { item = this->pop_raw_locked_(); } - const char *name = item->get_name(); + SchedulerNameLog name_log; bool is_cancelled = is_item_removed_(item.get()); ESP_LOGD(TAG, " %s '%s/%s' interval=%" PRIu32 " next_execution in %" PRIu64 "ms at %" PRIu64 "%s", - item->get_type_str(), LOG_STR_ARG(item->get_source()), name ? name : "(null)", item->interval, + item->get_type_str(), LOG_STR_ARG(item->get_source()), + name_log.format(item->get_name_type(), item->get_name(), item->get_name_hash_or_id()), item->interval, item->get_next_execution() - now_64, item->get_next_execution(), is_cancelled ? " [CANCELLED]" : ""); old_items.push_back(std::move(item)); @@ -513,10 +514,13 @@ void HOT Scheduler::call(uint32_t now) { #endif #ifdef ESPHOME_DEBUG_SCHEDULER - const char *item_name = item->get_name(); - ESP_LOGV(TAG, "Running %s '%s/%s' with interval=%" PRIu32 " next_execution=%" PRIu64 " (now=%" PRIu64 ")", - item->get_type_str(), LOG_STR_ARG(item->get_source()), item_name ? item_name : "(null)", item->interval, - item->get_next_execution(), now_64); + { + SchedulerNameLog name_log; + ESP_LOGV(TAG, "Running %s '%s/%s' with interval=%" PRIu32 " next_execution=%" PRIu64 " (now=%" PRIu64 ")", + item->get_type_str(), LOG_STR_ARG(item->get_source()), + name_log.format(item->get_name_type(), item->get_name(), item->get_name_hash_or_id()), item->interval, + item->get_next_execution(), now_64); + } #endif /* ESPHOME_DEBUG_SCHEDULER */ // Warning: During callback(), a lot of stuff can happen, including: From 121051228680fbc6d77d8ae741e65c64c3691b2e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 22:33:32 -1000 Subject: [PATCH 15/16] fix double dep warning --- esphome/core/component.cpp | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/esphome/core/component.cpp b/esphome/core/component.cpp index decd080976..2f61f7d195 100644 --- a/esphome/core/component.cpp +++ b/esphome/core/component.cpp @@ -118,7 +118,10 @@ void Component::setup() {} void Component::loop() {} void Component::set_interval(const std::string &name, uint32_t interval, std::function &&f) { // NOLINT +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" App.scheduler.set_interval(this, name, interval, std::move(f)); +#pragma GCC diagnostic pop } void Component::set_interval(const char *name, uint32_t interval, std::function &&f) { // NOLINT @@ -126,7 +129,10 @@ void Component::set_interval(const char *name, uint32_t interval, std::function< } bool Component::cancel_interval(const std::string &name) { // NOLINT +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" return App.scheduler.cancel_interval(this, name); +#pragma GCC diagnostic pop } bool Component::cancel_interval(const char *name) { // NOLINT @@ -135,7 +141,10 @@ bool Component::cancel_interval(const char *name) { // NOLINT void Component::set_retry(const std::string &name, uint32_t initial_wait_time, uint8_t max_attempts, std::function &&f, float backoff_increase_factor) { // NOLINT +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" App.scheduler.set_retry(this, name, initial_wait_time, max_attempts, std::move(f), backoff_increase_factor); +#pragma GCC diagnostic pop } void Component::set_retry(const char *name, uint32_t initial_wait_time, uint8_t max_attempts, @@ -144,7 +153,10 @@ void Component::set_retry(const char *name, uint32_t initial_wait_time, uint8_t } bool Component::cancel_retry(const std::string &name) { // NOLINT +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" return App.scheduler.cancel_retry(this, name); +#pragma GCC diagnostic pop } bool Component::cancel_retry(const char *name) { // NOLINT @@ -152,7 +164,10 @@ bool Component::cancel_retry(const char *name) { // NOLINT } void Component::set_timeout(const std::string &name, uint32_t timeout, std::function &&f) { // NOLINT +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" App.scheduler.set_timeout(this, name, timeout, std::move(f)); +#pragma GCC diagnostic pop } void Component::set_timeout(const char *name, uint32_t timeout, std::function &&f) { // NOLINT @@ -160,7 +175,10 @@ void Component::set_timeout(const char *name, uint32_t timeout, std::function &&f) { // NOLINT App.scheduler.set_timeout(this, static_cast(nullptr), 0, std::move(f)); } bool Component::cancel_defer(const std::string &name) { // NOLINT +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" return App.scheduler.cancel_timeout(this, name); +#pragma GCC diagnostic pop } bool Component::cancel_defer(const char *name) { // NOLINT return App.scheduler.cancel_timeout(this, name); } void Component::defer(const std::string &name, std::function &&f) { // NOLINT +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" App.scheduler.set_timeout(this, name, 0, std::move(f)); +#pragma GCC diagnostic pop } void Component::defer(const char *name, std::function &&f) { // NOLINT App.scheduler.set_timeout(this, name, 0, std::move(f)); From 5541a7f0433e983036ead2449db22a9d4004290b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 13 Jan 2026 22:36:37 -1000 Subject: [PATCH 16/16] one more place to log --- esphome/core/scheduler.cpp | 16 ++++++++-------- esphome/core/scheduler.h | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 902feeb115..047bf4ef17 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -175,8 +175,7 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type } #ifdef ESPHOME_DEBUG_SCHEDULER - this->debug_log_timer_(item.get(), name_type == NameType::STATIC_STRING, - name_type == NameType::STATIC_STRING ? static_name : nullptr, type, delay, now); + this->debug_log_timer_(item.get(), name_type, static_name, hash_or_id, type, delay, now); #endif /* ESPHOME_DEBUG_SCHEDULER */ // For retries, check if there's a cancelled timeout first @@ -854,21 +853,22 @@ void Scheduler::recycle_item_main_loop_(std::unique_ptr item) { } #ifdef ESPHOME_DEBUG_SCHEDULER -void Scheduler::debug_log_timer_(const SchedulerItem *item, bool is_static_string, const char *name_cstr, - SchedulerItem::Type type, uint32_t delay, uint64_t now) { +void Scheduler::debug_log_timer_(const SchedulerItem *item, NameType name_type, const char *static_name, + uint32_t hash_or_id, SchedulerItem::Type type, uint32_t delay, uint64_t now) { // Validate static strings in debug mode - if (is_static_string && name_cstr != nullptr) { - validate_static_string(name_cstr); + if (name_type == NameType::STATIC_STRING && static_name != nullptr) { + validate_static_string(static_name); } // Debug logging + SchedulerNameLog name_log; const char *type_str = (type == SchedulerItem::TIMEOUT) ? "timeout" : "interval"; if (type == SchedulerItem::TIMEOUT) { ESP_LOGD(TAG, "set_%s(name='%s/%s', %s=%" PRIu32 ")", type_str, LOG_STR_ARG(item->get_source()), - name_cstr ? name_cstr : "(null)", type_str, delay); + name_log.format(name_type, static_name, hash_or_id), type_str, delay); } else { ESP_LOGD(TAG, "set_%s(name='%s/%s', %s=%" PRIu32 ", offset=%" PRIu32 ")", type_str, LOG_STR_ARG(item->get_source()), - name_cstr ? name_cstr : "(null)", type_str, delay, + name_log.format(name_type, static_name, hash_or_id), type_str, delay, static_cast(item->get_next_execution() - now)); } } diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 92ff93879a..8c2e349180 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -317,7 +317,7 @@ class Scheduler { #ifdef ESPHOME_DEBUG_SCHEDULER // Helper for debug logging in set_timer_common_ - extracted to reduce code size - void debug_log_timer_(const SchedulerItem *item, bool is_static_string, const char *name_cstr, + void debug_log_timer_(const SchedulerItem *item, NameType name_type, const char *static_name, uint32_t hash_or_id, SchedulerItem::Type type, uint32_t delay, uint64_t now); #endif /* ESPHOME_DEBUG_SCHEDULER */