From 068594be5e9b3426b0660cc2e6d756c4c6d3eff4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 4 Jul 2025 07:29:37 -0500 Subject: [PATCH 1/7] Make defer FIFO --- esphome/core/scheduler.cpp | 88 ++++++++++++++++++++++++++++++-------- esphome/core/scheduler.h | 14 ++++++ 2 files changed, 83 insertions(+), 19 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 5c01b4f3f4..e0d2b70102 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -73,8 +73,6 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type if (delay == SCHEDULER_DONT_RUN) return; - const auto now = this->millis_(); - // Create and populate the scheduler item auto item = make_unique(); item->component = component; @@ -83,6 +81,16 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type item->callback = std::move(func); item->remove = false; + // Special handling for defer() (delay = 0, type = TIMEOUT) + if (delay == 0 && type == SchedulerItem::TIMEOUT) { + // Put in defer queue for guaranteed FIFO execution + LockGuard guard{this->lock_}; + this->defer_queue_.push_back(std::move(item)); + return; + } + + const auto now = this->millis_(); + // Type-specific setup if (type == SchedulerItem::INTERVAL) { item->interval = delay; @@ -209,6 +217,28 @@ optional HOT Scheduler::next_schedule_in() { return item->next_execution_ - now; } void HOT Scheduler::call() { + // Process defer queue first to guarantee FIFO execution order for deferred items. + // Previously, defer() used the heap which gave undefined order for equal timestamps, + // causing race conditions on multi-core systems (ESP32, RP2040, BK7200). + // With the defer queue: + // - Deferred items (delay=0) go directly to defer_queue_ in set_timer_common_ + // - Items execute in exact order they were deferred (FIFO guarantee) + // - No deferred items exist in to_add_, so processing order doesn't affect correctness + while (!this->defer_queue_.empty()) { + std::unique_ptr item; + { + LockGuard guard{this->lock_}; + if (this->defer_queue_.empty()) // Double-check with lock held + break; + item = std::move(this->defer_queue_.front()); + this->defer_queue_.pop_front(); + } + // Skip if item was marked for removal or component failed + if (!this->should_skip_item_(item.get())) { + this->execute_item_(item.get()); + } + } + const auto now = this->millis_(); this->process_to_add(); @@ -294,13 +324,7 @@ void HOT Scheduler::call() { // Warning: During callback(), a lot of stuff can happen, including: // - timeouts/intervals get added, potentially invalidating vector pointers // - timeouts/intervals get cancelled - { - uint32_t now_ms = millis(); - WarnIfComponentBlockingGuard guard{item->component, now_ms}; - item->callback(); - // Call finish to ensure blocking time is properly calculated and reported - guard.finish(); - } + this->execute_item_(item.get()); } { @@ -364,6 +388,26 @@ void HOT Scheduler::push_(std::unique_ptr item) { LockGuard guard{this->lock_}; this->to_add_.push_back(std::move(item)); } +// Helper function to check if item matches criteria for cancellation +bool HOT Scheduler::matches_item_(const std::unique_ptr &item, Component *component, + const char *name_cstr, SchedulerItem::Type type) { + if (item->component != component || item->type != type || item->remove) { + return false; + } + const char *item_name = item->get_name(); + return item_name != nullptr && strcmp(name_cstr, item_name) == 0; +} + +// Helper to execute a scheduler item +void HOT Scheduler::execute_item_(SchedulerItem *item) { + App.set_current_component(item->component); + + uint32_t now_ms = millis(); + WarnIfComponentBlockingGuard guard{item->component, now_ms}; + item->callback(); + guard.finish(); +} + // Common implementation for cancel operations bool HOT Scheduler::cancel_item_common_(Component *component, bool is_static_string, const void *name_ptr, SchedulerItem::Type type) { @@ -379,19 +423,25 @@ bool HOT Scheduler::cancel_item_common_(Component *component, bool is_static_str LockGuard guard{this->lock_}; bool ret = false; - for (auto &it : this->items_) { - 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; + // Check all containers for matching items + for (auto &item : this->defer_queue_) { + if (this->matches_item_(item, component, name_cstr, type)) { + item->remove = true; ret = true; } } - for (auto &it : this->to_add_) { - 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; + + for (auto &item : this->items_) { + if (this->matches_item_(item, component, name_cstr, type)) { + item->remove = true; + ret = true; + this->to_remove_++; // Only track removals for heap items + } + } + + for (auto &item : this->to_add_) { + if (this->matches_item_(item, component, name_cstr, type)) { + item->remove = true; ret = true; } } diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index a64968932e..e617eb99c2 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -2,6 +2,7 @@ #include #include +#include #include "esphome/core/component.h" #include "esphome/core/helpers.h" @@ -145,6 +146,18 @@ class Scheduler { bool cancel_item_(Component *component, const std::string &name, SchedulerItem::Type type); bool cancel_item_(Component *component, const char *name, SchedulerItem::Type type); + // Helper functions for cancel operations + bool matches_item_(const std::unique_ptr &item, Component *component, const char *name_cstr, + SchedulerItem::Type type); + + // Helper to execute a scheduler item + void execute_item_(SchedulerItem *item); + + // Helper to check if item should be skipped + bool should_skip_item_(const SchedulerItem *item) const { + return item->remove || (item->component != nullptr && item->component->is_failed()); + } + bool empty_() { this->cleanup_(); return this->items_.empty(); @@ -153,6 +166,7 @@ class Scheduler { Mutex lock_; std::vector> items_; std::vector> to_add_; + std::deque> defer_queue_; // FIFO queue for defer() calls uint32_t last_millis_{0}; uint16_t millis_major_{0}; uint32_t to_remove_{0}; From ba4c268956d7e94c282faf4081b450f3fd2c6a9d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 4 Jul 2025 07:35:24 -0500 Subject: [PATCH 2/7] Make defer FIFO --- esphome/components/web_server/web_server.cpp | 42 ++------------------ esphome/components/web_server/web_server.h | 11 ----- 2 files changed, 3 insertions(+), 50 deletions(-) diff --git a/esphome/components/web_server/web_server.cpp b/esphome/components/web_server/web_server.cpp index f576507c0f..693a0e0127 100644 --- a/esphome/components/web_server/web_server.cpp +++ b/esphome/components/web_server/web_server.cpp @@ -244,11 +244,7 @@ void DeferredUpdateEventSourceList::on_client_disconnect_(DeferredUpdateEventSou } #endif -WebServer::WebServer(web_server_base::WebServerBase *base) : base_(base) { -#ifdef USE_ESP32 - to_schedule_lock_ = xSemaphoreCreateMutex(); -#endif -} +WebServer::WebServer(web_server_base::WebServerBase *base) : base_(base) {} #ifdef USE_WEBSERVER_CSS_INCLUDE void WebServer::set_css_include(const char *css_include) { this->css_include_ = css_include; } @@ -297,30 +293,7 @@ void WebServer::setup() { // getting a lot of events this->set_interval(10000, [this]() { this->events_.try_send_nodefer("", "ping", millis(), 30000); }); } -void WebServer::loop() { -#ifdef USE_ESP32 - // Check atomic flag first to avoid taking semaphore when queue is empty - if (this->to_schedule_has_items_.load(std::memory_order_relaxed) && xSemaphoreTake(this->to_schedule_lock_, 0L)) { - std::function fn; - if (!to_schedule_.empty()) { - // scheduler execute things out of order which may lead to incorrect state - // this->defer(std::move(to_schedule_.front())); - // let's execute it directly from the loop - fn = std::move(to_schedule_.front()); - to_schedule_.pop_front(); - if (to_schedule_.empty()) { - this->to_schedule_has_items_.store(false, std::memory_order_relaxed); - } - } - xSemaphoreGive(this->to_schedule_lock_); - if (fn) { - fn(); - } - } -#endif - - this->events_.loop(); -} +void WebServer::loop() { this->events_.loop(); } void WebServer::dump_config() { ESP_LOGCONFIG(TAG, "Web Server:\n" @@ -2061,16 +2034,7 @@ void WebServer::add_sorting_group(uint64_t group_id, const std::string &group_na } #endif -void WebServer::schedule_(std::function &&f) { -#ifdef USE_ESP32 - xSemaphoreTake(this->to_schedule_lock_, portMAX_DELAY); - to_schedule_.push_back(std::move(f)); - this->to_schedule_has_items_.store(true, std::memory_order_relaxed); - xSemaphoreGive(this->to_schedule_lock_); -#else - this->defer(std::move(f)); -#endif -} +void WebServer::schedule_(std::function &&f) { this->defer(std::move(f)); } } // namespace web_server } // namespace esphome diff --git a/esphome/components/web_server/web_server.h b/esphome/components/web_server/web_server.h index c654d83bbd..fdb14dab19 100644 --- a/esphome/components/web_server/web_server.h +++ b/esphome/components/web_server/web_server.h @@ -14,12 +14,6 @@ #include #include #include -#ifdef USE_ESP32 -#include -#include -#include -#include -#endif #if USE_WEBSERVER_VERSION >= 2 extern const uint8_t ESPHOME_WEBSERVER_INDEX_HTML[] PROGMEM; @@ -522,11 +516,6 @@ class WebServer : public Controller, public Component, public AsyncWebHandler { const char *js_include_{nullptr}; #endif bool expose_log_{true}; -#ifdef USE_ESP32 - std::deque> to_schedule_; - SemaphoreHandle_t to_schedule_lock_; - std::atomic to_schedule_has_items_{false}; -#endif }; } // namespace web_server From e21334b7faf0fc4cee41a22ab31733392ade0254 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 4 Jul 2025 07:42:37 -0500 Subject: [PATCH 3/7] Make defer FIFO --- esphome/components/web_server/web_server.cpp | 28 +++++++++----------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/esphome/components/web_server/web_server.cpp b/esphome/components/web_server/web_server.cpp index 693a0e0127..52273f248d 100644 --- a/esphome/components/web_server/web_server.cpp +++ b/esphome/components/web_server/web_server.cpp @@ -488,13 +488,13 @@ void WebServer::handle_switch_request(AsyncWebServerRequest *request, const UrlM std::string data = this->switch_json(obj, obj->state, detail); request->send(200, "application/json", data.c_str()); } else if (match.method_equals("toggle")) { - this->schedule_([obj]() { obj->toggle(); }); + this->defer([obj]() { obj->toggle(); }); request->send(200); } else if (match.method_equals("turn_on")) { - this->schedule_([obj]() { obj->turn_on(); }); + this->defer([obj]() { obj->turn_on(); }); request->send(200); } else if (match.method_equals("turn_off")) { - this->schedule_([obj]() { obj->turn_off(); }); + this->defer([obj]() { obj->turn_off(); }); request->send(200); } else { request->send(404); @@ -530,7 +530,7 @@ void WebServer::handle_button_request(AsyncWebServerRequest *request, const UrlM std::string data = this->button_json(obj, detail); request->send(200, "application/json", data.c_str()); } else if (match.method_equals("press")) { - this->schedule_([obj]() { obj->press(); }); + this->defer([obj]() { obj->press(); }); request->send(200); return; } else { @@ -610,7 +610,7 @@ void WebServer::handle_fan_request(AsyncWebServerRequest *request, const UrlMatc std::string data = this->fan_json(obj, detail); request->send(200, "application/json", data.c_str()); } else if (match.method_equals("toggle")) { - this->schedule_([obj]() { obj->toggle().perform(); }); + this->defer([obj]() { obj->toggle().perform(); }); request->send(200); } else if (match.method_equals("turn_on") || match.method_equals("turn_off")) { auto call = match.method_equals("turn_on") ? obj->turn_on() : obj->turn_off(); @@ -642,7 +642,7 @@ void WebServer::handle_fan_request(AsyncWebServerRequest *request, const UrlMatc return; } } - this->schedule_([call]() mutable { call.perform(); }); + this->defer([call]() mutable { call.perform(); }); request->send(200); } else { request->send(404); @@ -691,7 +691,7 @@ void WebServer::handle_light_request(AsyncWebServerRequest *request, const UrlMa std::string data = this->light_json(obj, detail); request->send(200, "application/json", data.c_str()); } else if (match.method_equals("toggle")) { - this->schedule_([obj]() { obj->toggle().perform(); }); + this->defer([obj]() { obj->toggle().perform(); }); request->send(200); } else if (match.method_equals("turn_on")) { auto call = obj->turn_on(); @@ -748,7 +748,7 @@ void WebServer::handle_light_request(AsyncWebServerRequest *request, const UrlMa call.set_effect(effect); } - this->schedule_([call]() mutable { call.perform(); }); + this->defer([call]() mutable { call.perform(); }); request->send(200); } else if (match.method_equals("turn_off")) { auto call = obj->turn_off(); @@ -758,7 +758,7 @@ void WebServer::handle_light_request(AsyncWebServerRequest *request, const UrlMa call.set_transition_length(*transition * 1000); } } - this->schedule_([call]() mutable { call.perform(); }); + this->defer([call]() mutable { call.perform(); }); request->send(200); } else { request->send(404); @@ -1414,13 +1414,13 @@ void WebServer::handle_lock_request(AsyncWebServerRequest *request, const UrlMat std::string data = this->lock_json(obj, obj->state, detail); request->send(200, "application/json", data.c_str()); } else if (match.method_equals("lock")) { - this->schedule_([obj]() { obj->lock(); }); + this->defer([obj]() { obj->lock(); }); request->send(200); } else if (match.method_equals("unlock")) { - this->schedule_([obj]() { obj->unlock(); }); + this->defer([obj]() { obj->unlock(); }); request->send(200); } else if (match.method_equals("open")) { - this->schedule_([obj]() { obj->open(); }); + this->defer([obj]() { obj->open(); }); request->send(200); } else { request->send(404); @@ -1657,7 +1657,7 @@ void WebServer::handle_update_request(AsyncWebServerRequest *request, const UrlM return; } - this->schedule_([obj]() mutable { obj->perform(); }); + this->defer([obj]() mutable { obj->perform(); }); request->send(200); return; } @@ -2034,8 +2034,6 @@ void WebServer::add_sorting_group(uint64_t group_id, const std::string &group_na } #endif -void WebServer::schedule_(std::function &&f) { this->defer(std::move(f)); } - } // namespace web_server } // namespace esphome #endif From db86f87fc3c5520158ff5b5f98e184b629b03b17 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 4 Jul 2025 07:42:59 -0500 Subject: [PATCH 4/7] Make defer FIFO --- esphome/components/web_server/web_server.cpp | 18 +++++++++--------- esphome/components/web_server/web_server.h | 1 - 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/esphome/components/web_server/web_server.cpp b/esphome/components/web_server/web_server.cpp index 52273f248d..c47ea5b092 100644 --- a/esphome/components/web_server/web_server.cpp +++ b/esphome/components/web_server/web_server.cpp @@ -843,7 +843,7 @@ void WebServer::handle_cover_request(AsyncWebServerRequest *request, const UrlMa } } - this->schedule_([call]() mutable { call.perform(); }); + this->defer([call]() mutable { call.perform(); }); request->send(200); return; } @@ -901,7 +901,7 @@ void WebServer::handle_number_request(AsyncWebServerRequest *request, const UrlM call.set_value(*value); } - this->schedule_([call]() mutable { call.perform(); }); + this->defer([call]() mutable { call.perform(); }); request->send(200); return; } @@ -976,7 +976,7 @@ void WebServer::handle_date_request(AsyncWebServerRequest *request, const UrlMat call.set_date(value); } - this->schedule_([call]() mutable { call.perform(); }); + this->defer([call]() mutable { call.perform(); }); request->send(200); return; } @@ -1035,7 +1035,7 @@ void WebServer::handle_time_request(AsyncWebServerRequest *request, const UrlMat call.set_time(value); } - this->schedule_([call]() mutable { call.perform(); }); + this->defer([call]() mutable { call.perform(); }); request->send(200); return; } @@ -1093,7 +1093,7 @@ void WebServer::handle_datetime_request(AsyncWebServerRequest *request, const Ur call.set_datetime(value); } - this->schedule_([call]() mutable { call.perform(); }); + this->defer([call]() mutable { call.perform(); }); request->send(200); return; } @@ -1210,7 +1210,7 @@ void WebServer::handle_select_request(AsyncWebServerRequest *request, const UrlM call.set_option(option.c_str()); // NOLINT } - this->schedule_([call]() mutable { call.perform(); }); + this->defer([call]() mutable { call.perform(); }); request->send(200); return; } @@ -1297,7 +1297,7 @@ void WebServer::handle_climate_request(AsyncWebServerRequest *request, const Url call.set_target_temperature(*target_temperature); } - this->schedule_([call]() mutable { call.perform(); }); + this->defer([call]() mutable { call.perform(); }); request->send(200); return; } @@ -1491,7 +1491,7 @@ void WebServer::handle_valve_request(AsyncWebServerRequest *request, const UrlMa } } - this->schedule_([call]() mutable { call.perform(); }); + this->defer([call]() mutable { call.perform(); }); request->send(200); return; } @@ -1556,7 +1556,7 @@ void WebServer::handle_alarm_control_panel_request(AsyncWebServerRequest *reques return; } - this->schedule_([call]() mutable { call.perform(); }); + this->defer([call]() mutable { call.perform(); }); request->send(200); return; } diff --git a/esphome/components/web_server/web_server.h b/esphome/components/web_server/web_server.h index fdb14dab19..6bb683a22f 100644 --- a/esphome/components/web_server/web_server.h +++ b/esphome/components/web_server/web_server.h @@ -496,7 +496,6 @@ class WebServer : public Controller, public Component, public AsyncWebHandler { protected: void add_sorting_info_(JsonObject &root, EntityBase *entity); - void schedule_(std::function &&f); web_server_base::WebServerBase *base_; #ifdef USE_ARDUINO DeferredUpdateEventSourceList events_; From 5dd76966c383a4512ed5a60744cacbe5d6bdb5b1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 4 Jul 2025 07:55:01 -0500 Subject: [PATCH 5/7] cover --- .../fixtures/defer_fifo_simple.yaml | 36 +++++++++++++++++++ tests/integration/test_defer_fifo_simple.py | 29 +++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 tests/integration/fixtures/defer_fifo_simple.yaml create mode 100644 tests/integration/test_defer_fifo_simple.py diff --git a/tests/integration/fixtures/defer_fifo_simple.yaml b/tests/integration/fixtures/defer_fifo_simple.yaml new file mode 100644 index 0000000000..5cb675e77e --- /dev/null +++ b/tests/integration/fixtures/defer_fifo_simple.yaml @@ -0,0 +1,36 @@ +esphome: + name: defer-fifo-simple + on_boot: + - lambda: |- + // Simple test: defer 10 items and verify they execute in order + static int execution_order = 0; + static bool test_passed = true; + + for (int i = 0; i < 10; i++) { + int expected = i; + App.scheduler.set_timeout((Component*)nullptr, nullptr, 0, [expected]() { + ESP_LOGD("defer_test", "Deferred item %d executed, order %d", expected, execution_order); + if (execution_order != expected) { + ESP_LOGE("defer_test", "FIFO violation: expected %d but got execution order %d", expected, execution_order); + test_passed = false; + } + execution_order++; + + if (execution_order == 10) { + if (test_passed) { + ESP_LOGI("defer_test", "✓ FIFO order test PASSED - all 10 items executed in correct order"); + } else { + ESP_LOGE("defer_test", "✗ FIFO order test FAILED - items executed out of order"); + } + } + }); + } + + ESP_LOGD("defer_test", "Deferred 10 items, waiting for execution..."); + +host: + +logger: + level: DEBUG + +api: diff --git a/tests/integration/test_defer_fifo_simple.py b/tests/integration/test_defer_fifo_simple.py new file mode 100644 index 0000000000..7b3cf21737 --- /dev/null +++ b/tests/integration/test_defer_fifo_simple.py @@ -0,0 +1,29 @@ +"""Simple test that defer() maintains FIFO order.""" + +import asyncio + +import pytest + +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_defer_fifo_simple( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test that defer() maintains FIFO order with a simple test.""" + + async with run_compiled(yaml_config), api_client_connected() as client: + # Just verify we can connect and the device is running + device_info = await client.device_info() + assert device_info is not None + assert device_info.name == "defer-fifo-simple" + + # Give the test component time to run + await asyncio.sleep(5) + + # The component will log results, we mainly want to ensure + # it doesn't crash and completes successfully + print("Defer FIFO simple test completed") From a4d5f39fb6e5139896466e8e9b4c1d8bc3105e44 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 4 Jul 2025 07:59:12 -0500 Subject: [PATCH 6/7] cover --- .../fixtures/defer_fifo_simple.yaml | 68 +++++++++++++++---- 1 file changed, 55 insertions(+), 13 deletions(-) diff --git a/tests/integration/fixtures/defer_fifo_simple.yaml b/tests/integration/fixtures/defer_fifo_simple.yaml index 5cb675e77e..75aee41ebf 100644 --- a/tests/integration/fixtures/defer_fifo_simple.yaml +++ b/tests/integration/fixtures/defer_fifo_simple.yaml @@ -2,31 +2,73 @@ esphome: name: defer-fifo-simple on_boot: - lambda: |- - // Simple test: defer 10 items and verify they execute in order - static int execution_order = 0; - static bool test_passed = true; + // Test 1: Test set_timeout with 0 delay (direct scheduler call) + static int set_timeout_order = 0; + static bool set_timeout_passed = true; + ESP_LOGD("defer_test", "Test 1: Testing set_timeout(0) for FIFO order..."); for (int i = 0; i < 10; i++) { int expected = i; App.scheduler.set_timeout((Component*)nullptr, nullptr, 0, [expected]() { - ESP_LOGD("defer_test", "Deferred item %d executed, order %d", expected, execution_order); - if (execution_order != expected) { - ESP_LOGE("defer_test", "FIFO violation: expected %d but got execution order %d", expected, execution_order); - test_passed = false; + ESP_LOGD("defer_test", "set_timeout(0) item %d executed, order %d", expected, set_timeout_order); + if (set_timeout_order != expected) { + ESP_LOGE("defer_test", "FIFO violation in set_timeout: expected %d but got execution order %d", expected, set_timeout_order); + set_timeout_passed = false; } - execution_order++; + set_timeout_order++; - if (execution_order == 10) { - if (test_passed) { - ESP_LOGI("defer_test", "✓ FIFO order test PASSED - all 10 items executed in correct order"); + if (set_timeout_order == 10) { + if (set_timeout_passed) { + ESP_LOGI("defer_test", "✓ Test 1 PASSED - set_timeout(0) maintains FIFO order"); } else { - ESP_LOGE("defer_test", "✗ FIFO order test FAILED - items executed out of order"); + ESP_LOGE("defer_test", "✗ Test 1 FAILED - set_timeout(0) executed out of order"); } + + // Start Test 2 after Test 1 completes + App.scheduler.set_timeout((Component*)nullptr, nullptr, 100, []() { + // Test 2: Test defer() method (component method) + static int defer_order = 0; + static bool defer_passed = true; + + ESP_LOGD("defer_test", "Test 2: Testing defer() for FIFO order..."); + + // Create a test component class that exposes defer() + class TestComponent : public Component { + public: + void test_defer() { + for (int i = 0; i < 10; i++) { + int expected = i; + this->defer([expected]() { + ESP_LOGD("defer_test", "defer() item %d executed, order %d", expected, defer_order); + if (defer_order != expected) { + ESP_LOGE("defer_test", "FIFO violation in defer: expected %d but got execution order %d", expected, defer_order); + defer_passed = false; + } + defer_order++; + + if (defer_order == 10) { + if (defer_passed) { + ESP_LOGI("defer_test", "✓ Test 2 PASSED - defer() maintains FIFO order"); + ESP_LOGI("defer_test", "✓ ALL TESTS PASSED - Both set_timeout(0) and defer() maintain FIFO order"); + } else { + ESP_LOGE("defer_test", "✗ Test 2 FAILED - defer() executed out of order"); + } + } + }); + } + } + }; + + TestComponent test_component; + test_component.test_defer(); + + ESP_LOGD("defer_test", "Deferred 10 items using defer(), waiting for execution..."); + }); } }); } - ESP_LOGD("defer_test", "Deferred 10 items, waiting for execution..."); + ESP_LOGD("defer_test", "Deferred 10 items using set_timeout(0), waiting for execution..."); host: From 465019e5100d57da5cdeb540b773a453bf662af9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 4 Jul 2025 08:04:16 -0500 Subject: [PATCH 7/7] cover --- .../fixtures/defer_fifo_simple.yaml | 17 +++++- tests/integration/test_defer_fifo_simple.py | 61 +++++++++++++++++-- 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/tests/integration/fixtures/defer_fifo_simple.yaml b/tests/integration/fixtures/defer_fifo_simple.yaml index 75aee41ebf..29c1a2bf38 100644 --- a/tests/integration/fixtures/defer_fifo_simple.yaml +++ b/tests/integration/fixtures/defer_fifo_simple.yaml @@ -47,12 +47,19 @@ esphome: defer_order++; if (defer_order == 10) { + bool all_passed = set_timeout_passed && defer_passed; if (defer_passed) { ESP_LOGI("defer_test", "✓ Test 2 PASSED - defer() maintains FIFO order"); - ESP_LOGI("defer_test", "✓ ALL TESTS PASSED - Both set_timeout(0) and defer() maintain FIFO order"); + if (all_passed) { + ESP_LOGI("defer_test", "✓ ALL TESTS PASSED - Both set_timeout(0) and defer() maintain FIFO order"); + } } else { ESP_LOGE("defer_test", "✗ Test 2 FAILED - defer() executed out of order"); } + + // Publish test results + id(test_complete)->publish_state(true); + id(test_passed)->publish_state(all_passed); } }); } @@ -76,3 +83,11 @@ logger: level: DEBUG api: + +binary_sensor: + - platform: template + name: "Test Complete" + id: test_complete + - platform: template + name: "Test Passed" + id: test_passed diff --git a/tests/integration/test_defer_fifo_simple.py b/tests/integration/test_defer_fifo_simple.py index 7b3cf21737..95a14e64b7 100644 --- a/tests/integration/test_defer_fifo_simple.py +++ b/tests/integration/test_defer_fifo_simple.py @@ -2,6 +2,7 @@ import asyncio +from aioesphomeapi import BinarySensorInfo, BinarySensorState, EntityState import pytest from .types import APIClientConnectedFactory, RunCompiledFunction @@ -16,14 +17,62 @@ async def test_defer_fifo_simple( """Test that defer() maintains FIFO order with a simple test.""" async with run_compiled(yaml_config), api_client_connected() as client: - # Just verify we can connect and the device is running + # Verify we can connect device_info = await client.device_info() assert device_info is not None assert device_info.name == "defer-fifo-simple" - # Give the test component time to run - await asyncio.sleep(5) + # List entities to get the keys + entity_info, _ = await asyncio.wait_for( + client.list_entities_services(), timeout=5.0 + ) - # The component will log results, we mainly want to ensure - # it doesn't crash and completes successfully - print("Defer FIFO simple test completed") + # Find our test entities + test_complete_entity: BinarySensorInfo | None = None + test_passed_entity: BinarySensorInfo | None = None + + for entity in entity_info: + if isinstance(entity, BinarySensorInfo): + if entity.object_id == "test_complete": + test_complete_entity = entity + elif entity.object_id == "test_passed": + test_passed_entity = entity + + assert test_complete_entity is not None, "test_complete sensor not found" + assert test_passed_entity is not None, "test_passed sensor not found" + + # Get the event loop + loop = asyncio.get_running_loop() + + # Subscribe to state changes + states: dict[int, EntityState] = {} + test_complete_future: asyncio.Future[BinarySensorState] = loop.create_future() + test_passed_future: asyncio.Future[BinarySensorState] = loop.create_future() + + def on_state(state: EntityState) -> None: + states[state.key] = state + # Check if this is our test_complete binary sensor + if isinstance(state, BinarySensorState): + if state.key == test_complete_entity.key: + if state.state and not test_complete_future.done(): + test_complete_future.set_result(state) + elif state.key == test_passed_entity.key: + if not test_passed_future.done(): + test_passed_future.set_result(state) + + client.subscribe_states(on_state) + + # Wait for test completion with timeout + try: + await asyncio.wait_for(test_complete_future, timeout=10.0) + test_passed_state = await asyncio.wait_for(test_passed_future, timeout=1.0) + except asyncio.TimeoutError: + pytest.fail( + f"Test did not complete within 10 seconds. " + f"Received states: {list(states.values())}" + ) + + # Verify the test passed + assert test_passed_state.state is True, ( + "FIFO test failed - items executed out of order" + )