From 140ca070a20c9e1b3d3d380f467c0553c2f8a9b6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 29 Jun 2025 22:40:36 -0500 Subject: [PATCH] Optimize scheduler string storage to eliminate heap allocations (#9251) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- esphome/core/component.cpp | 18 +- esphome/core/component.h | 34 ++++ esphome/core/scheduler.cpp | 175 +++++++++++++----- esphome/core/scheduler.h | 119 ++++++++++-- .../fixtures/scheduler_string_test.yaml | 164 ++++++++++++++++ .../integration/test_scheduler_string_test.py | 166 +++++++++++++++++ 6 files changed, 614 insertions(+), 62 deletions(-) create mode 100644 tests/integration/fixtures/scheduler_string_test.yaml create mode 100644 tests/integration/test_scheduler_string_test.py diff --git a/esphome/core/component.cpp b/esphome/core/component.cpp index 8fa63de84e..6661223e35 100644 --- a/esphome/core/component.cpp +++ b/esphome/core/component.cpp @@ -60,10 +60,18 @@ void Component::set_interval(const std::string &name, uint32_t interval, std::fu App.scheduler.set_interval(this, name, interval, std::move(f)); } +void Component::set_interval(const char *name, uint32_t interval, std::function &&f) { // NOLINT + App.scheduler.set_interval(this, name, interval, std::move(f)); +} + bool Component::cancel_interval(const std::string &name) { // NOLINT return App.scheduler.cancel_interval(this, name); } +bool Component::cancel_interval(const char *name) { // NOLINT + return App.scheduler.cancel_interval(this, name); +} + 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 App.scheduler.set_retry(this, name, initial_wait_time, max_attempts, std::move(f), backoff_increase_factor); @@ -77,10 +85,18 @@ void Component::set_timeout(const std::string &name, uint32_t timeout, std::func App.scheduler.set_timeout(this, name, timeout, std::move(f)); } +void Component::set_timeout(const char *name, uint32_t timeout, std::function &&f) { // NOLINT + App.scheduler.set_timeout(this, name, timeout, std::move(f)); +} + bool Component::cancel_timeout(const std::string &name) { // NOLINT return App.scheduler.cancel_timeout(this, name); } +bool Component::cancel_timeout(const char *name) { // NOLINT + return App.scheduler.cancel_timeout(this, name); +} + void Component::call_loop() { this->loop(); } void Component::call_setup() { this->setup(); } void Component::call_dump_config() { @@ -189,7 +205,7 @@ bool Component::is_in_loop_state() const { return (this->component_state_ & COMPONENT_STATE_MASK) == COMPONENT_STATE_LOOP; } void Component::defer(std::function &&f) { // NOLINT - App.scheduler.set_timeout(this, "", 0, std::move(f)); + App.scheduler.set_timeout(this, static_cast(nullptr), 0, std::move(f)); } bool Component::cancel_defer(const std::string &name) { // NOLINT return App.scheduler.cancel_timeout(this, name); diff --git a/esphome/core/component.h b/esphome/core/component.h index 7f2bdd8414..5b37deeb68 100644 --- a/esphome/core/component.h +++ b/esphome/core/component.h @@ -260,6 +260,22 @@ class Component { */ void set_interval(const std::string &name, uint32_t interval, std::function &&f); // NOLINT + /** Set an interval function with a const char* name. + * + * IMPORTANT: The provided name pointer must remain valid for the lifetime of the scheduler item. + * This means the name should be: + * - 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. + * + * @param name The identifier for this interval function (must have static lifetime) + * @param interval The interval in ms + * @param f The function to call + */ + void set_interval(const char *name, uint32_t interval, std::function &&f); // NOLINT + void set_interval(uint32_t interval, std::function &&f); // NOLINT /** Cancel an interval function. @@ -268,6 +284,7 @@ class Component { * @return Whether an interval functions was deleted. */ bool cancel_interval(const std::string &name); // NOLINT + bool cancel_interval(const char *name); // NOLINT /** Set an retry function with a unique name. Empty name means no cancelling possible. * @@ -328,6 +345,22 @@ class Component { */ void set_timeout(const std::string &name, uint32_t timeout, std::function &&f); // NOLINT + /** Set a timeout function with a const char* name. + * + * IMPORTANT: The provided name pointer must remain valid for the lifetime of the scheduler item. + * This means the name should be: + * - A string literal (e.g., "init") + * - A static const char* variable + * - A pointer with lifetime >= the timeout duration + * + * For dynamic strings, use the std::string overload instead. + * + * @param name The identifier for this timeout function (must have static lifetime) + * @param timeout The timeout in ms + * @param f The function to call + */ + void set_timeout(const char *name, uint32_t timeout, std::function &&f); // NOLINT + void set_timeout(uint32_t timeout, std::function &&f); // NOLINT /** Cancel a timeout function. @@ -336,6 +369,7 @@ class Component { * @return Whether a timeout functions was deleted. */ bool cancel_timeout(const std::string &name); // NOLINT + bool cancel_timeout(const char *name); // NOLINT /** Defer a callback to the next loop() call. * diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 8144435163..5c01b4f3f4 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -7,6 +7,7 @@ #include "esphome/core/log.h" #include #include +#include namespace esphome { @@ -17,75 +18,138 @@ static const uint32_t MAX_LOGICALLY_DELETED_ITEMS = 10; // Uncomment to debug scheduler // #define ESPHOME_DEBUG_SCHEDULER +#ifdef ESPHOME_DEBUG_SCHEDULER +// Helper to validate that a pointer looks like it's in static memory +static void validate_static_string(const char *name) { + if (name == nullptr) + return; + + // This is a heuristic check - stack and heap pointers are typically + // much higher in memory than static data + uintptr_t addr = reinterpret_cast(name); + + // Create a stack variable to compare against + int stack_var; + uintptr_t stack_addr = reinterpret_cast(&stack_var); + + // If the string pointer is near our stack variable, it's likely on the stack + // Using 8KB range as ESP32 main task stack is typically 8192 bytes + if (addr > (stack_addr - 0x2000) && addr < (stack_addr + 0x2000)) { + ESP_LOGW(TAG, + "WARNING: Scheduler name '%s' at %p appears to be on the stack - this is unsafe!\n" + " Stack reference at %p", + name, name, &stack_var); + } + + // Also check if it might be on the heap by seeing if it's in a very different range + // This is platform-specific but generally heap is allocated far from static memory + static const char *static_str = "test"; + uintptr_t static_addr = reinterpret_cast(static_str); + + // If the address is very far from known static memory, it might be heap + if (addr > static_addr + 0x100000 || (static_addr > 0x100000 && addr < static_addr - 0x100000)) { + ESP_LOGW(TAG, "WARNING: Scheduler name '%s' at %p might be on heap (static ref at %p)", name, name, static_str); + } +} +#endif + // A note on locking: the `lock_` lock protects the `items_` and `to_add_` containers. It must be taken when writing to // them (i.e. when adding/removing items, but not when changing items). As items are only deleted from the loop task, // 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. -void HOT Scheduler::set_timeout(Component *component, const std::string &name, uint32_t timeout, - std::function func) { - const auto now = this->millis_(); +// 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) { + // Get the name as const char* + const char *name_cstr = + is_static_string ? static_cast(name_ptr) : static_cast(name_ptr)->c_str(); - if (!name.empty()) - this->cancel_timeout(component, name); + // Cancel existing timer if name is not empty + if (name_cstr != nullptr && name_cstr[0] != '\0') { + this->cancel_item_(component, name_cstr, type); + } - if (timeout == SCHEDULER_DONT_RUN) + if (delay == SCHEDULER_DONT_RUN) return; + const auto now = this->millis_(); + + // Create and populate the scheduler item auto item = make_unique(); item->component = component; - item->name = name; - item->type = SchedulerItem::TIMEOUT; - item->next_execution_ = now + timeout; + item->set_name(name_cstr, !is_static_string); + item->type = type; item->callback = std::move(func); item->remove = false; + + // Type-specific setup + if (type == SchedulerItem::INTERVAL) { + item->interval = delay; + // Calculate random offset (0 to interval/2) + uint32_t offset = (delay != 0) ? (random_uint32() % delay) / 2 : 0; + item->next_execution_ = now + offset; + } else { + item->interval = 0; + item->next_execution_ = now + delay; + } + #ifdef ESPHOME_DEBUG_SCHEDULER - ESP_LOGD(TAG, "set_timeout(name='%s/%s', timeout=%" PRIu32 ")", item->get_source(), name.c_str(), timeout); + // Validate static strings in debug mode + if (is_static_string && name_cstr != nullptr) { + validate_static_string(name_cstr); + } + + // Debug logging + 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, item->get_source(), + name_cstr ? name_cstr : "(null)", type_str, delay); + } else { + ESP_LOGD(TAG, "set_%s(name='%s/%s', %s=%" PRIu32 ", offset=%" PRIu32 ")", type_str, item->get_source(), + name_cstr ? name_cstr : "(null)", type_str, delay, static_cast(item->next_execution_ - now)); + } #endif + this->push_(std::move(item)); } + +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, name, SchedulerItem::TIMEOUT); } +bool HOT Scheduler::cancel_timeout(Component *component, const char *name) { + return this->cancel_item_(component, name, SchedulerItem::TIMEOUT); +} void HOT Scheduler::set_interval(Component *component, const std::string &name, uint32_t interval, std::function func) { - const auto now = this->millis_(); + this->set_timer_common_(component, SchedulerItem::INTERVAL, false, &name, interval, std::move(func)); +} - if (!name.empty()) - this->cancel_interval(component, name); - - if (interval == SCHEDULER_DONT_RUN) - return; - - // only put offset in lower half - uint32_t offset = 0; - if (interval != 0) - offset = (random_uint32() % interval) / 2; - - auto item = make_unique(); - item->component = component; - item->name = name; - item->type = SchedulerItem::INTERVAL; - item->interval = interval; - item->next_execution_ = now + offset; - item->callback = std::move(func); - item->remove = false; -#ifdef ESPHOME_DEBUG_SCHEDULER - ESP_LOGD(TAG, "set_interval(name='%s/%s', interval=%" PRIu32 ", offset=%" PRIu32 ")", item->get_source(), - name.c_str(), interval, offset); -#endif - this->push_(std::move(item)); +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)); } bool HOT Scheduler::cancel_interval(Component *component, const std::string &name) { return this->cancel_item_(component, name, SchedulerItem::INTERVAL); } +bool HOT Scheduler::cancel_interval(Component *component, const char *name) { + return this->cancel_item_(component, name, SchedulerItem::INTERVAL); +} struct RetryArgs { std::function func; uint8_t retry_countdown; uint32_t current_interval; Component *component; - std::string name; + std::string name; // Keep as std::string since retry uses it dynamically float backoff_increase_factor; Scheduler *scheduler; }; @@ -154,7 +218,7 @@ void HOT Scheduler::call() { if (now - last_print > 2000) { last_print = now; std::vector> old_items; - ESP_LOGD(TAG, "Items: count=%u, now=%" PRIu64 " (%u, %" PRIu32 ")", this->items_.size(), now, this->millis_major_, + ESP_LOGD(TAG, "Items: count=%zu, now=%" PRIu64 " (%u, %" PRIu32 ")", this->items_.size(), now, this->millis_major_, this->last_millis_); while (!this->empty_()) { this->lock_.lock(); @@ -162,8 +226,9 @@ void HOT Scheduler::call() { this->pop_raw_(); this->lock_.unlock(); + const char *name = item->get_name(); ESP_LOGD(TAG, " %s '%s/%s' interval=%" PRIu32 " next_execution in %" PRIu64 "ms at %" PRIu64, - item->get_type_str(), item->get_source(), item->name.c_str(), item->interval, + item->get_type_str(), item->get_source(), name ? name : "(null)", item->interval, item->next_execution_ - now, item->next_execution_); old_items.push_back(std::move(item)); @@ -220,9 +285,10 @@ void HOT Scheduler::call() { App.set_current_component(item->component); #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(), item->get_source(), item->name.c_str(), item->interval, item->next_execution_, - now); + item->get_type_str(), item->get_source(), item_name ? item_name : "(null)", item->interval, + item->next_execution_, now); #endif // Warning: During callback(), a lot of stuff can happen, including: @@ -298,19 +364,33 @@ void HOT Scheduler::push_(std::unique_ptr item) { LockGuard guard{this->lock_}; this->to_add_.push_back(std::move(item)); } -bool HOT Scheduler::cancel_item_(Component *component, const std::string &name, Scheduler::SchedulerItem::Type type) { +// Common implementation for cancel operations +bool HOT Scheduler::cancel_item_common_(Component *component, bool is_static_string, const void *name_ptr, + SchedulerItem::Type type) { + // Get the name as const char* + const char *name_cstr = + is_static_string ? static_cast(name_ptr) : static_cast(name_ptr)->c_str(); + + // Handle null or empty names + if (name_cstr == nullptr) + return false; + // obtain lock because this function iterates and can be called from non-loop task context LockGuard guard{this->lock_}; bool ret = false; + for (auto &it : this->items_) { - if (it->component == component && it->name == name && it->type == type && !it->remove) { + const char *item_name = it->get_name(); + if (it->component == component && item_name != nullptr && strcmp(name_cstr, item_name) == 0 && it->type == type && + !it->remove) { to_remove_++; it->remove = true; ret = true; } } for (auto &it : this->to_add_) { - if (it->component == component && it->name == name && it->type == type) { + const char *item_name = it->get_name(); + if (it->component == component && item_name != nullptr && strcmp(name_cstr, item_name) == 0 && it->type == type) { it->remove = true; ret = true; } @@ -318,6 +398,15 @@ bool HOT Scheduler::cancel_item_(Component *component, const std::string &name, return ret; } + +bool HOT Scheduler::cancel_item_(Component *component, const std::string &name, Scheduler::SchedulerItem::Type type) { + return this->cancel_item_common_(component, false, &name, type); +} + +bool HOT Scheduler::cancel_item_(Component *component, const char *name, SchedulerItem::Type type) { + return this->cancel_item_common_(component, true, name, type); +} + uint64_t Scheduler::millis_() { // Get the current 32-bit millis value const uint32_t now = millis(); diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 1284bcd4a7..a64968932e 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -12,11 +12,40 @@ class Component; class Scheduler { public: + // Public API - accepts std::string for backward compatibility void set_timeout(Component *component, const std::string &name, uint32_t timeout, std::function func); - bool cancel_timeout(Component *component, const std::string &name); - void set_interval(Component *component, const std::string &name, uint32_t interval, std::function func); - bool cancel_interval(Component *component, const std::string &name); + /** Set a timeout with a const char* name. + * + * IMPORTANT: The provided name pointer must remain valid for the lifetime of the scheduler item. + * This means the name should be: + * - 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 an interval with a const char* name. + * + * IMPORTANT: The provided name pointer must remain valid for the lifetime of the scheduler item. + * This means the name should be: + * - 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); bool cancel_retry(Component *component, const std::string &name); @@ -36,32 +65,86 @@ class Scheduler { // with a 16-bit rollover counter to create a 64-bit time that won't roll over for // billions of years. This ensures correct scheduling even when devices run for months. uint64_t next_execution_; - std::string name; - std::function callback; - enum Type : uint8_t { TIMEOUT, INTERVAL } type; - bool remove; - static bool cmp(const std::unique_ptr &a, const std::unique_ptr &b); - const char *get_type_str() { - switch (this->type) { - case SchedulerItem::INTERVAL: - return "interval"; - case SchedulerItem::TIMEOUT: - return "timeout"; - default: - return ""; + // Optimized name storage using tagged union + union { + const char *static_name; // For string literals (no allocation) + char *dynamic_name; // For allocated strings + } name_; + + std::function callback; + + // Bit-packed fields to minimize padding + enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1; + bool remove : 1; + bool name_is_dynamic : 1; // True if name was dynamically allocated (needs delete[]) + // 5 bits padding + + // Constructor + SchedulerItem() + : component(nullptr), interval(0), next_execution_(0), type(TIMEOUT), remove(false), name_is_dynamic(false) { + name_.static_name = nullptr; + } + + // Destructor to clean up dynamic names + ~SchedulerItem() { + if (name_is_dynamic) { + delete[] name_.dynamic_name; } } - const char *get_source() { - return this->component != nullptr ? this->component->get_component_source() : "unknown"; + + // Delete copy operations to prevent accidental copies + SchedulerItem(const SchedulerItem &) = delete; + SchedulerItem &operator=(const SchedulerItem &) = delete; + + // Default move operations + SchedulerItem(SchedulerItem &&) = default; + SchedulerItem &operator=(SchedulerItem &&) = default; + + // 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 set name with proper ownership + void set_name(const char *name, bool make_copy = false) { + // Clean up old dynamic name if any + if (name_is_dynamic && name_.dynamic_name) { + delete[] name_.dynamic_name; + name_is_dynamic = false; + } + + if (!name || !name[0]) { + name_.static_name = nullptr; + } else if (make_copy) { + // Make a copy for dynamic 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 + name_.static_name = name; + } } + + static bool cmp(const std::unique_ptr &a, const std::unique_ptr &b); + const char *get_type_str() const { return (type == TIMEOUT) ? "timeout" : "interval"; } + const char *get_source() const { return component ? component->get_component_source() : "unknown"; } }; + // 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); + uint64_t millis_(); void cleanup_(); void pop_raw_(); void push_(std::unique_ptr item); + // Common implementation for cancel operations + bool cancel_item_common_(Component *component, bool is_static_string, const void *name_ptr, SchedulerItem::Type type); + bool cancel_item_(Component *component, const std::string &name, SchedulerItem::Type type); + bool cancel_item_(Component *component, const char *name, SchedulerItem::Type type); + bool empty_() { this->cleanup_(); return this->items_.empty(); diff --git a/tests/integration/fixtures/scheduler_string_test.yaml b/tests/integration/fixtures/scheduler_string_test.yaml new file mode 100644 index 0000000000..1188577e15 --- /dev/null +++ b/tests/integration/fixtures/scheduler_string_test.yaml @@ -0,0 +1,164 @@ +esphome: + name: scheduler-string-test + on_boot: + priority: -100 + then: + - logger.log: "Starting scheduler string tests" + platformio_options: + build_flags: + - "-DESPHOME_DEBUG_SCHEDULER" # Enable scheduler debug logging + +host: +api: +logger: + level: VERBOSE + +globals: + - id: timeout_counter + type: int + initial_value: '0' + - id: interval_counter + type: int + initial_value: '0' + - id: dynamic_counter + type: int + initial_value: '0' + - id: static_tests_done + type: bool + initial_value: 'false' + - id: dynamic_tests_done + type: bool + initial_value: 'false' + - id: results_reported + type: bool + initial_value: 'false' + +script: + - id: test_static_strings + then: + - logger.log: "Testing static string timeouts and intervals" + - lambda: |- + auto *component1 = id(test_sensor1); + // Test 1: Static string literals with set_timeout + App.scheduler.set_timeout(component1, "static_timeout_1", 50, []() { + ESP_LOGI("test", "Static timeout 1 fired"); + id(timeout_counter) += 1; + }); + + // Test 2: Static const char* with set_timeout + static const char* TIMEOUT_NAME = "static_timeout_2"; + App.scheduler.set_timeout(component1, TIMEOUT_NAME, 100, []() { + ESP_LOGI("test", "Static timeout 2 fired"); + id(timeout_counter) += 1; + }); + + // Test 3: Static string literal with set_interval + App.scheduler.set_interval(component1, "static_interval_1", 200, []() { + ESP_LOGI("test", "Static interval 1 fired, count: %d", id(interval_counter)); + id(interval_counter) += 1; + if (id(interval_counter) >= 3) { + App.scheduler.cancel_interval(id(test_sensor1), "static_interval_1"); + ESP_LOGI("test", "Cancelled static interval 1"); + } + }); + + // Test 4: Empty string (should be handled safely) + App.scheduler.set_timeout(component1, "", 150, []() { + ESP_LOGI("test", "Empty string timeout fired"); + }); + + // Test 5: Cancel timeout with const char* literal + App.scheduler.set_timeout(component1, "cancel_static_timeout", 5000, []() { + ESP_LOGI("test", "This static timeout should be cancelled"); + }); + // Cancel using const char* directly + App.scheduler.cancel_timeout(component1, "cancel_static_timeout"); + ESP_LOGI("test", "Cancelled static timeout using const char*"); + + - id: test_dynamic_strings + then: + - logger.log: "Testing dynamic string timeouts and intervals" + - lambda: |- + auto *component2 = id(test_sensor2); + + // Test 6: Dynamic string with set_timeout (std::string) + std::string dynamic_name = "dynamic_timeout_" + std::to_string(id(dynamic_counter)++); + App.scheduler.set_timeout(component2, dynamic_name, 100, []() { + ESP_LOGI("test", "Dynamic timeout fired"); + id(timeout_counter) += 1; + }); + + // Test 7: Dynamic string with set_interval + std::string interval_name = "dynamic_interval_" + std::to_string(id(dynamic_counter)++); + App.scheduler.set_interval(component2, interval_name, 250, [interval_name]() { + ESP_LOGI("test", "Dynamic interval fired: %s", interval_name.c_str()); + id(interval_counter) += 1; + if (id(interval_counter) >= 6) { + App.scheduler.cancel_interval(id(test_sensor2), interval_name); + ESP_LOGI("test", "Cancelled dynamic interval"); + } + }); + + // Test 8: Cancel with different string object but same content + std::string cancel_name = "cancel_test"; + App.scheduler.set_timeout(component2, cancel_name, 2000, []() { + ESP_LOGI("test", "This should be cancelled"); + }); + + // Cancel using a different string object + std::string cancel_name_2 = "cancel_test"; + App.scheduler.cancel_timeout(component2, cancel_name_2); + ESP_LOGI("test", "Cancelled timeout using different string object"); + + - 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 + + - platform: template + name: Test Sensor 2 + id: test_sensor2 + lambda: return 2.0; + update_interval: never + +interval: + # Run static string tests after boot - using script to run once + - interval: 0.1s + then: + - if: + condition: + lambda: 'return id(static_tests_done) == false;' + then: + - lambda: 'id(static_tests_done) = true;' + - script.execute: test_static_strings + - logger.log: "Started static string tests" + + # Run dynamic string tests after static tests + - interval: 0.2s + then: + - if: + condition: + lambda: 'return id(static_tests_done) && !id(dynamic_tests_done);' + then: + - lambda: 'id(dynamic_tests_done) = true;' + - delay: 0.2s + - script.execute: test_dynamic_strings + + # Report results after all tests + - interval: 0.2s + then: + - if: + condition: + lambda: 'return id(dynamic_tests_done) && !id(results_reported);' + then: + - lambda: 'id(results_reported) = true;' + - delay: 1s + - script.execute: report_results diff --git a/tests/integration/test_scheduler_string_test.py b/tests/integration/test_scheduler_string_test.py new file mode 100644 index 0000000000..b5ca07f9db --- /dev/null +++ b/tests/integration/test_scheduler_string_test.py @@ -0,0 +1,166 @@ +"""Test scheduler string optimization with static and dynamic strings.""" + +import asyncio +import re + +import pytest + +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_scheduler_string_test( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test that scheduler handles both static and dynamic strings correctly.""" + # Track counts + timeout_count = 0 + interval_count = 0 + + # Events for each test completion + static_timeout_1_fired = asyncio.Event() + static_timeout_2_fired = asyncio.Event() + static_interval_fired = asyncio.Event() + static_interval_cancelled = asyncio.Event() + empty_string_timeout_fired = asyncio.Event() + static_timeout_cancelled = asyncio.Event() + dynamic_timeout_fired = asyncio.Event() + dynamic_interval_fired = asyncio.Event() + cancel_test_done = asyncio.Event() + final_results_logged = asyncio.Event() + + # Track interval counts + static_interval_count = 0 + dynamic_interval_count = 0 + + def on_log_line(line: str) -> None: + nonlocal \ + timeout_count, \ + interval_count, \ + static_interval_count, \ + dynamic_interval_count + + # Strip ANSI color codes + clean_line = re.sub(r"\x1b\[[0-9;]*m", "", line) + + # Check for static timeout completions + if "Static timeout 1 fired" in clean_line: + static_timeout_1_fired.set() + timeout_count += 1 + + elif "Static timeout 2 fired" in clean_line: + static_timeout_2_fired.set() + timeout_count += 1 + + # Check for static interval + elif "Static interval 1 fired" in clean_line: + match = re.search(r"count: (\d+)", clean_line) + if match: + static_interval_count = int(match.group(1)) + static_interval_fired.set() + + elif "Cancelled static interval 1" in clean_line: + static_interval_cancelled.set() + + # Check for empty string timeout + elif "Empty string timeout fired" in clean_line: + empty_string_timeout_fired.set() + + # Check for static timeout cancellation + elif "Cancelled static timeout using const char*" in clean_line: + static_timeout_cancelled.set() + + # Check for dynamic string tests + elif "Dynamic timeout fired" in clean_line: + dynamic_timeout_fired.set() + timeout_count += 1 + + elif "Dynamic interval fired" in clean_line: + dynamic_interval_count += 1 + dynamic_interval_fired.set() + + # Check for cancel test + elif "Cancelled timeout using different string object" in clean_line: + cancel_test_done.set() + + # 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-string-test" + + # Wait for static string tests + try: + await asyncio.wait_for(static_timeout_1_fired.wait(), timeout=0.5) + except asyncio.TimeoutError: + pytest.fail("Static timeout 1 did not fire within 0.5 seconds") + + try: + await asyncio.wait_for(static_timeout_2_fired.wait(), timeout=0.5) + except asyncio.TimeoutError: + pytest.fail("Static timeout 2 did not fire within 0.5 seconds") + + try: + await asyncio.wait_for(static_interval_fired.wait(), timeout=1.0) + except asyncio.TimeoutError: + pytest.fail("Static interval did not fire within 1 second") + + try: + await asyncio.wait_for(static_interval_cancelled.wait(), timeout=2.0) + except asyncio.TimeoutError: + pytest.fail("Static interval was not cancelled within 2 seconds") + + # Verify static interval ran at least 3 times + assert static_interval_count >= 2, ( + f"Expected static interval to run at least 3 times, got {static_interval_count + 1}" + ) + + # Verify static timeout was cancelled + assert static_timeout_cancelled.is_set(), ( + "Static timeout should have been cancelled" + ) + + # Wait for dynamic string tests + try: + await asyncio.wait_for(dynamic_timeout_fired.wait(), timeout=1.0) + except asyncio.TimeoutError: + pytest.fail("Dynamic timeout did not fire within 1 second") + + try: + await asyncio.wait_for(dynamic_interval_fired.wait(), timeout=1.5) + except asyncio.TimeoutError: + pytest.fail("Dynamic interval did not fire within 1.5 seconds") + + # Wait for cancel test + try: + await asyncio.wait_for(cancel_test_done.wait(), timeout=1.0) + except asyncio.TimeoutError: + pytest.fail("Cancel test did not complete within 1 second") + + # Wait for final results + try: + await asyncio.wait_for(final_results_logged.wait(), timeout=4.0) + except asyncio.TimeoutError: + pytest.fail("Final results were not logged within 4 seconds") + + # Verify results + assert timeout_count >= 3, f"Expected at least 3 timeouts, got {timeout_count}" + assert interval_count >= 3, ( + f"Expected at least 3 interval fires, got {interval_count}" + ) + + # Empty string timeout DOES fire (scheduler accepts empty names) + assert empty_string_timeout_fired.is_set(), "Empty string timeout should fire"