From d3342d6a1aa5dbd35ba72ff52fd5425f4a89d133 Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Tue, 15 Jul 2025 19:20:18 +1200 Subject: [PATCH] [component] Fix ``is_ready`` flag when loop disabled (#9501) Co-authored-by: J. Nick Koston --- esphome/core/component.cpp | 1 + .../loop_test_component/__init__.py | 28 ++++++++++- .../loop_test_component.cpp | 24 +++++++++ .../loop_test_component/loop_test_component.h | 25 ++++++++++ .../fixtures/loop_disable_enable.yaml | 32 ++++++++++++ tests/integration/test_loop_disable_enable.py | 50 +++++++++++++++++++ 6 files changed, 159 insertions(+), 1 deletion(-) diff --git a/esphome/core/component.cpp b/esphome/core/component.cpp index b360e1d20b..c47f16b5f7 100644 --- a/esphome/core/component.cpp +++ b/esphome/core/component.cpp @@ -264,6 +264,7 @@ void Component::set_retry(uint32_t initial_wait_time, uint8_t max_attempts, std: bool Component::is_failed() const { return (this->component_state_ & COMPONENT_STATE_MASK) == COMPONENT_STATE_FAILED; } bool Component::is_ready() const { return (this->component_state_ & COMPONENT_STATE_MASK) == COMPONENT_STATE_LOOP || + (this->component_state_ & COMPONENT_STATE_MASK) == COMPONENT_STATE_LOOP_DONE || (this->component_state_ & COMPONENT_STATE_MASK) == COMPONENT_STATE_SETUP; } bool Component::can_proceed() { return true; } diff --git a/tests/integration/fixtures/external_components/loop_test_component/__init__.py b/tests/integration/fixtures/external_components/loop_test_component/__init__.py index b66d4598f4..3f3a40db09 100644 --- a/tests/integration/fixtures/external_components/loop_test_component/__init__.py +++ b/tests/integration/fixtures/external_components/loop_test_component/__init__.py @@ -1,7 +1,7 @@ from esphome import automation import esphome.codegen as cg import esphome.config_validation as cv -from esphome.const import CONF_COMPONENTS, CONF_ID, CONF_NAME +from esphome.const import CONF_COMPONENTS, CONF_ID, CONF_NAME, CONF_UPDATE_INTERVAL CODEOWNERS = ["@esphome/tests"] @@ -10,10 +10,15 @@ LoopTestComponent = loop_test_component_ns.class_("LoopTestComponent", cg.Compon LoopTestISRComponent = loop_test_component_ns.class_( "LoopTestISRComponent", cg.Component ) +LoopTestUpdateComponent = loop_test_component_ns.class_( + "LoopTestUpdateComponent", cg.PollingComponent +) CONF_DISABLE_AFTER = "disable_after" CONF_TEST_REDUNDANT_OPERATIONS = "test_redundant_operations" CONF_ISR_COMPONENTS = "isr_components" +CONF_UPDATE_COMPONENTS = "update_components" +CONF_DISABLE_LOOP_AFTER = "disable_loop_after" COMPONENT_CONFIG_SCHEMA = cv.Schema( { @@ -31,11 +36,23 @@ ISR_COMPONENT_CONFIG_SCHEMA = cv.Schema( } ) +UPDATE_COMPONENT_CONFIG_SCHEMA = cv.Schema( + { + cv.GenerateID(): cv.declare_id(LoopTestUpdateComponent), + cv.Required(CONF_NAME): cv.string, + cv.Optional(CONF_DISABLE_LOOP_AFTER, default=0): cv.int_, + cv.Optional(CONF_UPDATE_INTERVAL, default="1s"): cv.update_interval, + } +) + CONFIG_SCHEMA = cv.Schema( { cv.GenerateID(): cv.declare_id(LoopTestComponent), cv.Required(CONF_COMPONENTS): cv.ensure_list(COMPONENT_CONFIG_SCHEMA), cv.Optional(CONF_ISR_COMPONENTS): cv.ensure_list(ISR_COMPONENT_CONFIG_SCHEMA), + cv.Optional(CONF_UPDATE_COMPONENTS): cv.ensure_list( + UPDATE_COMPONENT_CONFIG_SCHEMA + ), } ).extend(cv.COMPONENT_SCHEMA) @@ -94,3 +111,12 @@ async def to_code(config): var = cg.new_Pvariable(isr_config[CONF_ID]) await cg.register_component(var, isr_config) cg.add(var.set_name(isr_config[CONF_NAME])) + + # Create update test components + for update_config in config.get(CONF_UPDATE_COMPONENTS, []): + var = cg.new_Pvariable(update_config[CONF_ID]) + await cg.register_component(var, update_config) + + cg.add(var.set_name(update_config[CONF_NAME])) + cg.add(var.set_disable_loop_after(update_config[CONF_DISABLE_LOOP_AFTER])) + cg.add(var.set_update_interval(update_config[CONF_UPDATE_INTERVAL])) diff --git a/tests/integration/fixtures/external_components/loop_test_component/loop_test_component.cpp b/tests/integration/fixtures/external_components/loop_test_component/loop_test_component.cpp index 470740c534..28a05d3d45 100644 --- a/tests/integration/fixtures/external_components/loop_test_component/loop_test_component.cpp +++ b/tests/integration/fixtures/external_components/loop_test_component/loop_test_component.cpp @@ -39,5 +39,29 @@ void LoopTestComponent::service_disable() { this->disable_loop(); } +// LoopTestUpdateComponent implementation +void LoopTestUpdateComponent::setup() { + ESP_LOGI(TAG, "[%s] LoopTestUpdateComponent setup called", this->name_.c_str()); +} + +void LoopTestUpdateComponent::loop() { + this->loop_count_++; + ESP_LOGI(TAG, "[%s] LoopTestUpdateComponent loop count: %d", this->name_.c_str(), this->loop_count_); + + // Disable loop after specified count to test component.update when loop is disabled + if (this->disable_loop_after_ > 0 && this->loop_count_ == this->disable_loop_after_) { + ESP_LOGI(TAG, "[%s] Disabling loop after %d iterations", this->name_.c_str(), this->disable_loop_after_); + this->disable_loop(); + } +} + +void LoopTestUpdateComponent::update() { + this->update_count_++; + // Check if loop is disabled by testing component state + bool loop_disabled = this->component_state_ == COMPONENT_STATE_LOOP_DONE; + ESP_LOGI(TAG, "[%s] LoopTestUpdateComponent update() called, count: %d, loop_disabled: %s", this->name_.c_str(), + this->update_count_, loop_disabled ? "YES" : "NO"); +} + } // namespace loop_test_component } // namespace esphome diff --git a/tests/integration/fixtures/external_components/loop_test_component/loop_test_component.h b/tests/integration/fixtures/external_components/loop_test_component/loop_test_component.h index 5c43dd4b43..cdc04d491b 100644 --- a/tests/integration/fixtures/external_components/loop_test_component/loop_test_component.h +++ b/tests/integration/fixtures/external_components/loop_test_component/loop_test_component.h @@ -4,6 +4,7 @@ #include "esphome/core/log.h" #include "esphome/core/application.h" #include "esphome/core/automation.h" +#include "esphome/core/helpers.h" namespace esphome { namespace loop_test_component { @@ -54,5 +55,29 @@ template class DisableAction : public Action { LoopTestComponent *parent_; }; +// Component with update() method to test component.update action +class LoopTestUpdateComponent : public PollingComponent { + public: + LoopTestUpdateComponent() : PollingComponent(1000) {} // Default 1s update interval + + void set_name(const std::string &name) { this->name_ = name; } + void set_disable_loop_after(int count) { this->disable_loop_after_ = count; } + + void setup() override; + void loop() override; + void update() override; + + int get_update_count() const { return this->update_count_; } + int get_loop_count() const { return this->loop_count_; } + + float get_setup_priority() const override { return setup_priority::DATA; } + + protected: + std::string name_; + int loop_count_{0}; + int update_count_{0}; + int disable_loop_after_{0}; +}; + } // namespace loop_test_component } // namespace esphome diff --git a/tests/integration/fixtures/loop_disable_enable.yaml b/tests/integration/fixtures/loop_disable_enable.yaml index f19d7f60ca..f87fe9130b 100644 --- a/tests/integration/fixtures/loop_disable_enable.yaml +++ b/tests/integration/fixtures/loop_disable_enable.yaml @@ -40,6 +40,13 @@ loop_test_component: - id: isr_test name: "isr_test" + # Update test component to test component.update when loop is disabled + update_components: + - id: update_test_component + name: "update_test" + disable_loop_after: 3 # Disable loop after 3 iterations + update_interval: 0.1s # Fast update interval for testing + # Interval to re-enable the self_disable_10 component after some time interval: - interval: 0.5s @@ -51,3 +58,28 @@ interval: - logger.log: "Re-enabling self_disable_10 via service" - loop_test_component.enable: id: self_disable_10 + + # Test component.update on a component with disabled loop + - interval: 0.1s + then: + - lambda: |- + static bool manual_update_done = false; + if (!manual_update_done && + id(update_test_component).get_loop_count() == 3 && + id(update_test_component).get_update_count() >= 3) { + ESP_LOGI("main", "Manually calling component.update on update_test_component with disabled loop"); + manual_update_done = true; + } + - if: + condition: + lambda: |- + static bool manual_update_triggered = false; + if (!manual_update_triggered && + id(update_test_component).get_loop_count() == 3 && + id(update_test_component).get_update_count() >= 3) { + manual_update_triggered = true; + return true; + } + return false; + then: + - component.update: update_test_component diff --git a/tests/integration/test_loop_disable_enable.py b/tests/integration/test_loop_disable_enable.py index d5f868aa93..e93fc32178 100644 --- a/tests/integration/test_loop_disable_enable.py +++ b/tests/integration/test_loop_disable_enable.py @@ -45,11 +45,18 @@ async def test_loop_disable_enable( isr_component_disabled = asyncio.Event() isr_component_re_enabled = asyncio.Event() isr_component_pure_re_enabled = asyncio.Event() + # Events for update component testing + update_component_loop_disabled = asyncio.Event() + update_component_manual_update_called = asyncio.Event() # Track loop counts for components self_disable_10_counts: list[int] = [] normal_component_counts: list[int] = [] isr_component_counts: list[int] = [] + # Track update component behavior + update_component_loop_count = 0 + update_component_update_count = 0 + update_component_manual_update_count = 0 def on_log_line(line: str) -> None: """Process each log line from the process output.""" @@ -59,6 +66,7 @@ async def test_loop_disable_enable( if ( "loop_test_component" not in clean_line and "loop_test_isr_component" not in clean_line + and "Manually calling component.update" not in clean_line ): return @@ -112,6 +120,23 @@ async def test_loop_disable_enable( elif "Running after pure ISR re-enable!" in clean_line: isr_component_pure_re_enabled.set() + # Update component events + elif "[update_test]" in clean_line: + if "LoopTestUpdateComponent loop count:" in clean_line: + nonlocal update_component_loop_count + update_component_loop_count = int( + clean_line.split("LoopTestUpdateComponent loop count: ")[1] + ) + elif "LoopTestUpdateComponent update() called" in clean_line: + nonlocal update_component_update_count + update_component_update_count += 1 + if "Manually calling component.update" in " ".join(log_messages[-5:]): + nonlocal update_component_manual_update_count + update_component_manual_update_count += 1 + update_component_manual_update_called.set() + elif "Disabling loop after" in clean_line: + update_component_loop_disabled.set() + # Write, compile and run the ESPHome device with log callback async with ( run_compiled(yaml_config, line_callback=on_log_line), @@ -205,3 +230,28 @@ async def test_loop_disable_enable( assert final_count > 10, ( f"Component didn't run after pure ISR enable: got {final_count} counts total" ) + + # Test component.update functionality when loop is disabled + # Wait for update component to disable its loop + try: + await asyncio.wait_for(update_component_loop_disabled.wait(), timeout=3.0) + except asyncio.TimeoutError: + pytest.fail("Update component did not disable its loop within 3 seconds") + + # Verify it ran exactly 3 loops before disabling + assert update_component_loop_count == 3, ( + f"Expected 3 loop iterations before disable, got {update_component_loop_count}" + ) + + # Wait for manual component.update to be called + try: + await asyncio.wait_for( + update_component_manual_update_called.wait(), timeout=5.0 + ) + except asyncio.TimeoutError: + pytest.fail("Manual component.update was not called within 5 seconds") + + # The key test: verify that manual component.update worked after loop was disabled + assert update_component_manual_update_count >= 1, ( + "component.update did not fire after loop was disabled" + )