diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 7e2b2741a8..aa981d0b05 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -68,7 +68,7 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type // Still need to cancel existing timer if name is not empty if (name_cstr != nullptr && name_cstr[0] != '\0') { LockGuard guard{this->lock_}; - this->cancel_item_locked_(component, name_cstr, type, false); + this->cancel_item_locked_(component, name_cstr, type); } return; } @@ -87,7 +87,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 LockGuard guard{this->lock_}; - this->cancel_item_locked_(component, name_cstr, type, true); + this->cancel_item_locked_(component, name_cstr, type); this->defer_queue_.push_back(std::move(item)); return; } @@ -127,7 +127,7 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type // If name is provided, do atomic cancel-and-add if (name_cstr != nullptr && name_cstr[0] != '\0') { // Cancel existing items - this->cancel_item_locked_(component, name_cstr, type, false); + this->cancel_item_locked_(component, name_cstr, type); } // Add new item directly to to_add_ // since we have the lock held @@ -448,12 +448,11 @@ bool HOT Scheduler::cancel_item_(Component *component, bool is_static_string, co // 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, false); + 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 check_defer_only) { +bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_cstr, SchedulerItem::Type type) { size_t total_cancelled = 0; // Check all containers for matching items @@ -466,9 +465,6 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c total_cancelled++; } } - if (check_defer_only) { - return total_cancelled > 0; - } } #endif diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index abf52f5c13..39cee5a876 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -143,7 +143,7 @@ class Scheduler { 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 check_defer_only); + bool cancel_item_locked_(Component *component, const char *name, SchedulerItem::Type type); // 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) { diff --git a/tests/integration/fixtures/scheduler_defer_cancels_regular.yaml b/tests/integration/fixtures/scheduler_defer_cancels_regular.yaml new file mode 100644 index 0000000000..fb6b1791dc --- /dev/null +++ b/tests/integration/fixtures/scheduler_defer_cancels_regular.yaml @@ -0,0 +1,34 @@ +esphome: + name: scheduler-defer-cancel-regular + +host: + +logger: + level: DEBUG + +api: + services: + - service: test_defer_cancels_regular + then: + - lambda: |- + ESP_LOGI("TEST", "Starting defer cancels regular timeout test"); + + // Schedule a regular timeout with 100ms delay + App.scheduler.set_timeout(nullptr, "test_timeout", 100, []() { + ESP_LOGE("TEST", "ERROR: Regular timeout executed - should have been cancelled!"); + }); + + ESP_LOGI("TEST", "Scheduled regular timeout with 100ms delay"); + + // Immediately schedule a deferred timeout (0 delay) with the same name + // This should cancel the regular timeout + App.scheduler.set_timeout(nullptr, "test_timeout", 0, []() { + ESP_LOGI("TEST", "SUCCESS: Deferred timeout executed"); + }); + + ESP_LOGI("TEST", "Scheduled deferred timeout - should cancel regular timeout"); + + // Schedule test completion after 200ms (after regular timeout would have fired) + App.scheduler.set_timeout(nullptr, "test_complete", 200, []() { + ESP_LOGI("TEST", "Test complete"); + }); diff --git a/tests/integration/test_scheduler_defer_cancel_regular.py b/tests/integration/test_scheduler_defer_cancel_regular.py new file mode 100644 index 0000000000..57b7134feb --- /dev/null +++ b/tests/integration/test_scheduler_defer_cancel_regular.py @@ -0,0 +1,90 @@ +"""Test that a deferred timeout cancels a regular timeout with the same name.""" + +import asyncio + +from aioesphomeapi import UserService +import pytest + +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_scheduler_defer_cancels_regular( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test that set_timeout(name, 0) cancels a previously scheduled set_timeout(name, delay).""" + + # Create a future to signal test completion + loop = asyncio.get_running_loop() + test_complete_future: asyncio.Future[None] = loop.create_future() + + # Track log messages + log_messages: list[str] = [] + error_detected = False + + def on_log_line(line: str) -> None: + nonlocal error_detected + if "TEST" in line: + log_messages.append(line) + + if "ERROR: Regular timeout executed" in line: + error_detected = True + + if "Test complete" in line and not test_complete_future.done(): + test_complete_future.set_result(None) + + 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-defer-cancel-regular" + + # List services + _, services = await asyncio.wait_for( + client.list_entities_services(), timeout=5.0 + ) + + # Find our test service + test_service: UserService | None = None + for service in services: + if service.name == "test_defer_cancels_regular": + test_service = service + break + + assert test_service is not None, "test_defer_cancels_regular service not found" + + # Execute the test + client.execute_service(test_service, {}) + + # Wait for test completion + try: + await asyncio.wait_for(test_complete_future, timeout=5.0) + except asyncio.TimeoutError: + pytest.fail(f"Test timed out. Log messages: {log_messages}") + + # Verify results + assert not error_detected, ( + f"Regular timeout should have been cancelled but it executed! Logs: {log_messages}" + ) + + # Verify the deferred timeout executed + assert any( + "SUCCESS: Deferred timeout executed" in msg for msg in log_messages + ), f"Deferred timeout should have executed. Logs: {log_messages}" + + # Verify the expected sequence of events + assert any( + "Starting defer cancels regular timeout test" in msg for msg in log_messages + ) + assert any( + "Scheduled regular timeout with 100ms delay" in msg for msg in log_messages + ) + assert any( + "Scheduled deferred timeout - should cancel regular timeout" in msg + for msg in log_messages + )