From 2e534ce41ec8bd6da77746f0517302fb58201a8f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 18 Jun 2025 11:49:25 +0200 Subject: [PATCH] Reduce CPU overhead by allowing components to disable their loop() (#9089) --- esphome/components/anova/anova.cpp | 6 +- esphome/components/bedjet/bedjet_hub.cpp | 6 +- .../bedjet/climate/bedjet_climate.cpp | 6 +- .../ble_client/sensor/ble_rssi_sensor.cpp | 6 +- .../ble_client/sensor/ble_sensor.cpp | 6 +- .../text_sensor/ble_text_sensor.cpp | 6 +- .../captive_portal/captive_portal.cpp | 9 +- .../captive_portal/captive_portal.h | 5 +- .../esp32_ble_client/ble_client_base.cpp | 17 +- .../esp32_ble_client/ble_client_base.h | 2 + .../esp32_improv/esp32_improv_component.cpp | 3 + .../components/online_image/online_image.cpp | 4 + esphome/components/preferences/syncer.h | 2 + esphome/components/rtttl/rtttl.cpp | 9 +- esphome/components/safe_mode/safe_mode.cpp | 2 + esphome/components/sntp/sntp_component.cpp | 6 + esphome/components/tlc5971/tlc5971.cpp | 5 +- esphome/core/application.cpp | 69 +++++++- esphome/core/application.h | 28 ++++ esphome/core/component.cpp | 40 +++-- esphome/core/component.h | 21 +++ tests/integration/conftest.py | 26 ++- .../loop_test_component/__init__.py | 78 +++++++++ .../loop_test_component.cpp | 43 +++++ .../loop_test_component/loop_test_component.h | 58 +++++++ .../fixtures/loop_disable_enable.yaml | 48 ++++++ tests/integration/test_loop_disable_enable.py | 150 ++++++++++++++++++ tests/integration/types.py | 14 +- 28 files changed, 646 insertions(+), 29 deletions(-) create mode 100644 tests/integration/fixtures/external_components/loop_test_component/__init__.py create mode 100644 tests/integration/fixtures/external_components/loop_test_component/loop_test_component.cpp create mode 100644 tests/integration/fixtures/external_components/loop_test_component/loop_test_component.h create mode 100644 tests/integration/fixtures/loop_disable_enable.yaml create mode 100644 tests/integration/test_loop_disable_enable.py diff --git a/esphome/components/anova/anova.cpp b/esphome/components/anova/anova.cpp index ebf6c1d037..d0e8f6827f 100644 --- a/esphome/components/anova/anova.cpp +++ b/esphome/components/anova/anova.cpp @@ -17,7 +17,11 @@ void Anova::setup() { this->current_request_ = 0; } -void Anova::loop() {} +void Anova::loop() { + // Parent BLEClientNode has a loop() method, but this component uses + // polling via update() and BLE callbacks so loop isn't needed + this->disable_loop(); +} void Anova::control(const ClimateCall &call) { if (call.get_mode().has_value()) { diff --git a/esphome/components/bedjet/bedjet_hub.cpp b/esphome/components/bedjet/bedjet_hub.cpp index 7ebed2e78d..007ca1ca7d 100644 --- a/esphome/components/bedjet/bedjet_hub.cpp +++ b/esphome/components/bedjet/bedjet_hub.cpp @@ -480,7 +480,11 @@ void BedJetHub::set_clock(uint8_t hour, uint8_t minute) { /* Internal */ -void BedJetHub::loop() {} +void BedJetHub::loop() { + // Parent BLEClientNode has a loop() method, but this component uses + // polling via update() and BLE callbacks so loop isn't needed + this->disable_loop(); +} void BedJetHub::update() { this->dispatch_status_(); } void BedJetHub::dump_config() { diff --git a/esphome/components/bedjet/climate/bedjet_climate.cpp b/esphome/components/bedjet/climate/bedjet_climate.cpp index 854129f816..f22d312b5a 100644 --- a/esphome/components/bedjet/climate/bedjet_climate.cpp +++ b/esphome/components/bedjet/climate/bedjet_climate.cpp @@ -83,7 +83,11 @@ void BedJetClimate::reset_state_() { this->publish_state(); } -void BedJetClimate::loop() {} +void BedJetClimate::loop() { + // This component is controlled via the parent BedJetHub + // Empty loop not needed, disable to save CPU cycles + this->disable_loop(); +} void BedJetClimate::control(const ClimateCall &call) { ESP_LOGD(TAG, "Received BedJetClimate::control"); diff --git a/esphome/components/ble_client/sensor/ble_rssi_sensor.cpp b/esphome/components/ble_client/sensor/ble_rssi_sensor.cpp index 81d244ce6d..663c52ac10 100644 --- a/esphome/components/ble_client/sensor/ble_rssi_sensor.cpp +++ b/esphome/components/ble_client/sensor/ble_rssi_sensor.cpp @@ -11,7 +11,11 @@ namespace ble_client { static const char *const TAG = "ble_rssi_sensor"; -void BLEClientRSSISensor::loop() {} +void BLEClientRSSISensor::loop() { + // Parent BLEClientNode has a loop() method, but this component uses + // polling via update() and BLE GAP callbacks so loop isn't needed + this->disable_loop(); +} void BLEClientRSSISensor::dump_config() { LOG_SENSOR("", "BLE Client RSSI Sensor", this); diff --git a/esphome/components/ble_client/sensor/ble_sensor.cpp b/esphome/components/ble_client/sensor/ble_sensor.cpp index f91b07fee2..d0ccfe1f2e 100644 --- a/esphome/components/ble_client/sensor/ble_sensor.cpp +++ b/esphome/components/ble_client/sensor/ble_sensor.cpp @@ -11,7 +11,11 @@ namespace ble_client { static const char *const TAG = "ble_sensor"; -void BLESensor::loop() {} +void BLESensor::loop() { + // Parent BLEClientNode has a loop() method, but this component uses + // polling via update() and BLE callbacks so loop isn't needed + this->disable_loop(); +} void BLESensor::dump_config() { LOG_SENSOR("", "BLE Sensor", this); diff --git a/esphome/components/ble_client/text_sensor/ble_text_sensor.cpp b/esphome/components/ble_client/text_sensor/ble_text_sensor.cpp index 5083e235c6..e7da297fa0 100644 --- a/esphome/components/ble_client/text_sensor/ble_text_sensor.cpp +++ b/esphome/components/ble_client/text_sensor/ble_text_sensor.cpp @@ -14,7 +14,11 @@ static const char *const TAG = "ble_text_sensor"; static const std::string EMPTY = ""; -void BLETextSensor::loop() {} +void BLETextSensor::loop() { + // Parent BLEClientNode has a loop() method, but this component uses + // polling via update() and BLE callbacks so loop isn't needed + this->disable_loop(); +} void BLETextSensor::dump_config() { LOG_TEXT_SENSOR("", "BLE Text Sensor", this); diff --git a/esphome/components/captive_portal/captive_portal.cpp b/esphome/components/captive_portal/captive_portal.cpp index 31e6c51f0f..2c1ce17fb3 100644 --- a/esphome/components/captive_portal/captive_portal.cpp +++ b/esphome/components/captive_portal/captive_portal.cpp @@ -37,7 +37,12 @@ void CaptivePortal::handle_wifisave(AsyncWebServerRequest *request) { request->redirect("/?save"); } -void CaptivePortal::setup() {} +void CaptivePortal::setup() { +#ifndef USE_ARDUINO + // No DNS server needed for non-Arduino frameworks + this->disable_loop(); +#endif +} void CaptivePortal::start() { this->base_->init(); if (!this->initialized_) { @@ -50,6 +55,8 @@ void CaptivePortal::start() { this->dns_server_->setErrorReplyCode(DNSReplyCode::NoError); network::IPAddress ip = wifi::global_wifi_component->wifi_soft_ap_ip(); this->dns_server_->start(53, "*", ip); + // Re-enable loop() when DNS server is started + this->enable_loop(); #endif this->base_->get_server()->onNotFound([this](AsyncWebServerRequest *req) { diff --git a/esphome/components/captive_portal/captive_portal.h b/esphome/components/captive_portal/captive_portal.h index 24d1295e6a..94db7fef50 100644 --- a/esphome/components/captive_portal/captive_portal.h +++ b/esphome/components/captive_portal/captive_portal.h @@ -21,8 +21,11 @@ class CaptivePortal : public AsyncWebHandler, public Component { void dump_config() override; #ifdef USE_ARDUINO void loop() override { - if (this->dns_server_ != nullptr) + if (this->dns_server_ != nullptr) { this->dns_server_->processNextRequest(); + } else { + this->disable_loop(); + } } #endif float get_setup_priority() const override; diff --git a/esphome/components/esp32_ble_client/ble_client_base.cpp b/esphome/components/esp32_ble_client/ble_client_base.cpp index 4e61fb287c..8ae1eb1bac 100644 --- a/esphome/components/esp32_ble_client/ble_client_base.cpp +++ b/esphome/components/esp32_ble_client/ble_client_base.cpp @@ -22,6 +22,16 @@ void BLEClientBase::setup() { this->connection_index_ = connection_index++; } +void BLEClientBase::set_state(espbt::ClientState st) { + ESP_LOGV(TAG, "[%d] [%s] Set state %d", this->connection_index_, this->address_str_.c_str(), (int) st); + ESPBTClient::set_state(st); + + if (st == espbt::ClientState::READY_TO_CONNECT) { + // Enable loop when we need to connect + this->enable_loop(); + } +} + void BLEClientBase::loop() { if (!esp32_ble::global_ble->is_active()) { this->set_state(espbt::ClientState::INIT); @@ -37,9 +47,14 @@ void BLEClientBase::loop() { } // READY_TO_CONNECT means we have discovered the device // and the scanner has been stopped by the tracker. - if (this->state_ == espbt::ClientState::READY_TO_CONNECT) { + else if (this->state_ == espbt::ClientState::READY_TO_CONNECT) { this->connect(); } + // If its idle, we can disable the loop as set_state + // will enable it again when we need to connect. + else if (this->state_ == espbt::ClientState::IDLE) { + this->disable_loop(); + } } float BLEClientBase::get_setup_priority() const { return setup_priority::AFTER_BLUETOOTH; } diff --git a/esphome/components/esp32_ble_client/ble_client_base.h b/esphome/components/esp32_ble_client/ble_client_base.h index 89ac04e38c..814a9664d9 100644 --- a/esphome/components/esp32_ble_client/ble_client_base.h +++ b/esphome/components/esp32_ble_client/ble_client_base.h @@ -93,6 +93,8 @@ class BLEClientBase : public espbt::ESPBTClient, public Component { bool check_addr(esp_bd_addr_t &addr) { return memcmp(addr, this->remote_bda_, sizeof(esp_bd_addr_t)) == 0; } + void set_state(espbt::ClientState st) override; + protected: int gattc_if_; esp_bd_addr_t remote_bda_; diff --git a/esphome/components/esp32_improv/esp32_improv_component.cpp b/esphome/components/esp32_improv/esp32_improv_component.cpp index 9d84d38968..d41094fda1 100644 --- a/esphome/components/esp32_improv/esp32_improv_component.cpp +++ b/esphome/components/esp32_improv/esp32_improv_component.cpp @@ -168,6 +168,8 @@ void ESP32ImprovComponent::loop() { case improv::STATE_PROVISIONED: { this->incoming_data_.clear(); this->set_status_indicator_state_(false); + // Provisioning complete, no further loop execution needed + this->disable_loop(); break; } } @@ -254,6 +256,7 @@ void ESP32ImprovComponent::start() { ESP_LOGD(TAG, "Setting Improv to start"); this->should_start_ = true; + this->enable_loop(); } void ESP32ImprovComponent::stop() { diff --git a/esphome/components/online_image/online_image.cpp b/esphome/components/online_image/online_image.cpp index 8030bd0095..3f1d58fb45 100644 --- a/esphome/components/online_image/online_image.cpp +++ b/esphome/components/online_image/online_image.cpp @@ -178,18 +178,21 @@ void OnlineImage::update() { if (this->format_ == ImageFormat::BMP) { ESP_LOGD(TAG, "Allocating BMP decoder"); this->decoder_ = make_unique(this); + this->enable_loop(); } #endif // USE_ONLINE_IMAGE_BMP_SUPPORT #ifdef USE_ONLINE_IMAGE_JPEG_SUPPORT if (this->format_ == ImageFormat::JPEG) { ESP_LOGD(TAG, "Allocating JPEG decoder"); this->decoder_ = esphome::make_unique(this); + this->enable_loop(); } #endif // USE_ONLINE_IMAGE_JPEG_SUPPORT #ifdef USE_ONLINE_IMAGE_PNG_SUPPORT if (this->format_ == ImageFormat::PNG) { ESP_LOGD(TAG, "Allocating PNG decoder"); this->decoder_ = make_unique(this); + this->enable_loop(); } #endif // USE_ONLINE_IMAGE_PNG_SUPPORT @@ -212,6 +215,7 @@ void OnlineImage::update() { void OnlineImage::loop() { if (!this->decoder_) { // Not decoding at the moment => nothing to do. + this->disable_loop(); return; } if (!this->downloader_ || this->decoder_->is_finished()) { diff --git a/esphome/components/preferences/syncer.h b/esphome/components/preferences/syncer.h index 8976a1fe15..b6b422d4ba 100644 --- a/esphome/components/preferences/syncer.h +++ b/esphome/components/preferences/syncer.h @@ -12,6 +12,8 @@ class IntervalSyncer : public Component { void setup() override { if (this->write_interval_ != 0) { set_interval(this->write_interval_, []() { global_preferences->sync(); }); + // When using interval-based syncing, we don't need the loop + this->disable_loop(); } } void loop() override { diff --git a/esphome/components/rtttl/rtttl.cpp b/esphome/components/rtttl/rtttl.cpp index e24816fd83..2c4a0f917f 100644 --- a/esphome/components/rtttl/rtttl.cpp +++ b/esphome/components/rtttl/rtttl.cpp @@ -142,8 +142,10 @@ void Rtttl::stop() { } void Rtttl::loop() { - if (this->note_duration_ == 0 || this->state_ == State::STATE_STOPPED) + if (this->note_duration_ == 0 || this->state_ == State::STATE_STOPPED) { + this->disable_loop(); return; + } #ifdef USE_SPEAKER if (this->speaker_ != nullptr) { @@ -391,6 +393,11 @@ void Rtttl::set_state_(State state) { this->state_ = state; ESP_LOGV(TAG, "State changed from %s to %s", LOG_STR_ARG(state_to_string(old_state)), LOG_STR_ARG(state_to_string(state))); + + // Clear loop_done when transitioning from STOPPED to any other state + if (old_state == State::STATE_STOPPED && state != State::STATE_STOPPED) { + this->enable_loop(); + } } } // namespace rtttl diff --git a/esphome/components/safe_mode/safe_mode.cpp b/esphome/components/safe_mode/safe_mode.cpp index 89c9242357..5a62604269 100644 --- a/esphome/components/safe_mode/safe_mode.cpp +++ b/esphome/components/safe_mode/safe_mode.cpp @@ -42,6 +42,8 @@ void SafeModeComponent::loop() { ESP_LOGI(TAG, "Boot seems successful; resetting boot loop counter"); this->clean_rtc(); this->boot_successful_ = true; + // Disable loop since we no longer need to check + this->disable_loop(); } } diff --git a/esphome/components/sntp/sntp_component.cpp b/esphome/components/sntp/sntp_component.cpp index f9a9981c52..c7642d0637 100644 --- a/esphome/components/sntp/sntp_component.cpp +++ b/esphome/components/sntp/sntp_component.cpp @@ -67,6 +67,12 @@ void SNTPComponent::loop() { time.minute, time.second); this->time_sync_callback_.call(); this->has_time_ = true; + +#ifdef USE_ESP_IDF + // On ESP-IDF, time sync is permanent and update() doesn't force resync + // Time is now synchronized, no need to check anymore + this->disable_loop(); +#endif } } // namespace sntp diff --git a/esphome/components/tlc5971/tlc5971.cpp b/esphome/components/tlc5971/tlc5971.cpp index ebcc3af361..05ff0a0080 100644 --- a/esphome/components/tlc5971/tlc5971.cpp +++ b/esphome/components/tlc5971/tlc5971.cpp @@ -24,8 +24,10 @@ void TLC5971::dump_config() { } void TLC5971::loop() { - if (!this->update_) + if (!this->update_) { + this->disable_loop(); return; + } uint32_t command; @@ -93,6 +95,7 @@ void TLC5971::set_channel_value(uint16_t channel, uint16_t value) { return; if (this->pwm_amounts_[channel] != value) { this->update_ = true; + this->enable_loop(); } this->pwm_amounts_[channel] = value; } diff --git a/esphome/core/application.cpp b/esphome/core/application.cpp index 4ed96f7300..58df49f0f2 100644 --- a/esphome/core/application.cpp +++ b/esphome/core/application.cpp @@ -97,7 +97,13 @@ void Application::loop() { // Feed WDT with time this->feed_wdt(last_op_end_time); - for (Component *component : this->looping_components_) { + // Mark that we're in the loop for safe reentrant modifications + this->in_loop_ = true; + + for (this->current_loop_index_ = 0; this->current_loop_index_ < this->looping_components_active_end_; + this->current_loop_index_++) { + Component *component = this->looping_components_[this->current_loop_index_]; + // Update the cached time before each component runs this->loop_component_start_time_ = last_op_end_time; @@ -112,6 +118,8 @@ void Application::loop() { this->app_state_ |= new_app_state; this->feed_wdt(last_op_end_time); } + + this->in_loop_ = false; this->app_state_ = new_app_state; // Use the last component's end time instead of calling millis() again @@ -235,9 +243,66 @@ void Application::teardown_components(uint32_t timeout_ms) { } void Application::calculate_looping_components_() { + // First add all active components for (auto *obj : this->components_) { - if (obj->has_overridden_loop()) + if (obj->has_overridden_loop() && + (obj->get_component_state() & COMPONENT_STATE_MASK) != COMPONENT_STATE_LOOP_DONE) { this->looping_components_.push_back(obj); + } + } + + this->looping_components_active_end_ = this->looping_components_.size(); + + // Then add all inactive (LOOP_DONE) components + // This handles components that called disable_loop() during setup, before this method runs + for (auto *obj : this->components_) { + if (obj->has_overridden_loop() && + (obj->get_component_state() & COMPONENT_STATE_MASK) == COMPONENT_STATE_LOOP_DONE) { + this->looping_components_.push_back(obj); + } + } +} + +void Application::disable_component_loop_(Component *component) { + // This method must be reentrant - components can disable themselves during their own loop() call + // Linear search to find component in active section + // Most configs have 10-30 looping components (30 is on the high end) + // O(n) is acceptable here as we optimize for memory, not complexity + for (uint16_t i = 0; i < this->looping_components_active_end_; i++) { + if (this->looping_components_[i] == component) { + // Move last active component to this position + this->looping_components_active_end_--; + if (i != this->looping_components_active_end_) { + std::swap(this->looping_components_[i], this->looping_components_[this->looping_components_active_end_]); + + // If we're currently iterating and just swapped the current position + if (this->in_loop_ && i == this->current_loop_index_) { + // Decrement so we'll process the swapped component next + this->current_loop_index_--; + } + } + return; + } + } +} + +void Application::enable_component_loop_(Component *component) { + // This method must be reentrant - components can re-enable themselves during their own loop() call + // Single pass through all components to find and move if needed + // With typical 10-30 components, O(n) is faster than maintaining a map + const uint16_t size = this->looping_components_.size(); + for (uint16_t i = 0; i < size; i++) { + if (this->looping_components_[i] == component) { + if (i < this->looping_components_active_end_) { + return; // Already active + } + // Found in inactive section - move to active + if (i != this->looping_components_active_end_) { + std::swap(this->looping_components_[i], this->looping_components_[this->looping_components_active_end_]); + } + this->looping_components_active_end_++; + return; + } } } diff --git a/esphome/core/application.h b/esphome/core/application.h index f04ea05d8e..ea298638d2 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -572,13 +572,41 @@ class Application { void calculate_looping_components_(); + // These methods are called by Component::disable_loop() and Component::enable_loop() + // Components should not call these directly - use this->disable_loop() or this->enable_loop() + // to ensure component state is properly updated along with the loop partition + void disable_component_loop_(Component *component); + void enable_component_loop_(Component *component); + void feed_wdt_arch_(); /// Perform a delay while also monitoring socket file descriptors for readiness void yield_with_select_(uint32_t delay_ms); std::vector components_{}; + + // Partitioned vector design for looping components + // ================================================= + // Components are partitioned into [active | inactive] sections: + // + // looping_components_: [A, B, C, D | E, F] + // ^ + // looping_components_active_end_ (4) + // + // - Components A,B,C,D are active and will be called in loop() + // - Components E,F are inactive (disabled/failed) and won't be called + // - No flag checking needed during iteration - just loop 0 to active_end_ + // - When a component is disabled, it's swapped with the last active component + // and active_end_ is decremented + // - When a component is enabled, it's swapped with the first inactive component + // and active_end_ is incremented + // - This eliminates branch mispredictions from flag checking in the hot loop std::vector looping_components_{}; + uint16_t looping_components_active_end_{0}; + + // For safe reentrant modifications during iteration + uint16_t current_loop_index_{0}; + bool in_loop_{false}; #ifdef USE_BINARY_SENSOR std::vector binary_sensors_{}; diff --git a/esphome/core/component.cpp b/esphome/core/component.cpp index 0a4606074a..3117f49ac1 100644 --- a/esphome/core/component.cpp +++ b/esphome/core/component.cpp @@ -30,17 +30,18 @@ const float LATE = -100.0f; } // namespace setup_priority -// Component state uses bits 0-1 (4 states) -const uint8_t COMPONENT_STATE_MASK = 0x03; +// Component state uses bits 0-2 (8 states, 5 used) +const uint8_t COMPONENT_STATE_MASK = 0x07; const uint8_t COMPONENT_STATE_CONSTRUCTION = 0x00; const uint8_t COMPONENT_STATE_SETUP = 0x01; const uint8_t COMPONENT_STATE_LOOP = 0x02; const uint8_t COMPONENT_STATE_FAILED = 0x03; -// Status LED uses bits 2-3 -const uint8_t STATUS_LED_MASK = 0x0C; +const uint8_t COMPONENT_STATE_LOOP_DONE = 0x04; +// Status LED uses bits 3-4 +const uint8_t STATUS_LED_MASK = 0x18; const uint8_t STATUS_LED_OK = 0x00; -const uint8_t STATUS_LED_WARNING = 0x04; // Bit 2 -const uint8_t STATUS_LED_ERROR = 0x08; // Bit 3 +const uint8_t STATUS_LED_WARNING = 0x08; // Bit 3 +const uint8_t STATUS_LED_ERROR = 0x10; // Bit 4 const uint16_t WARN_IF_BLOCKING_OVER_MS = 50U; ///< Initial blocking time allowed without warning const uint16_t WARN_IF_BLOCKING_INCREMENT_MS = 10U; ///< How long the blocking time must be larger to warn again @@ -113,6 +114,9 @@ void Component::call() { case COMPONENT_STATE_FAILED: // NOLINT(bugprone-branch-clone) // State failed: Do nothing break; + case COMPONENT_STATE_LOOP_DONE: // NOLINT(bugprone-branch-clone) + // State loop done: Do nothing, component has finished its work + break; default: break; } @@ -136,14 +140,30 @@ bool Component::should_warn_of_blocking(uint32_t blocking_time) { return false; } void Component::mark_failed() { - ESP_LOGE(TAG, "Component %s was marked as failed.", this->get_component_source()); + ESP_LOGE(TAG, "Component %s was marked as failed", this->get_component_source()); this->component_state_ &= ~COMPONENT_STATE_MASK; this->component_state_ |= COMPONENT_STATE_FAILED; this->status_set_error(); + // Also remove from loop since failed components shouldn't loop + App.disable_component_loop_(this); +} +void Component::disable_loop() { + ESP_LOGD(TAG, "%s loop disabled", this->get_component_source()); + this->component_state_ &= ~COMPONENT_STATE_MASK; + this->component_state_ |= COMPONENT_STATE_LOOP_DONE; + App.disable_component_loop_(this); +} +void Component::enable_loop() { + if ((this->component_state_ & COMPONENT_STATE_MASK) == COMPONENT_STATE_LOOP_DONE) { + ESP_LOGD(TAG, "%s loop enabled", this->get_component_source()); + this->component_state_ &= ~COMPONENT_STATE_MASK; + this->component_state_ |= COMPONENT_STATE_LOOP; + App.enable_component_loop_(this); + } } void Component::reset_to_construction_state() { if ((this->component_state_ & COMPONENT_STATE_MASK) == COMPONENT_STATE_FAILED) { - ESP_LOGI(TAG, "Component %s is being reset to construction state.", this->get_component_source()); + ESP_LOGI(TAG, "Component %s is being reset to construction state", this->get_component_source()); this->component_state_ &= ~COMPONENT_STATE_MASK; this->component_state_ |= COMPONENT_STATE_CONSTRUCTION; // Clear error status when resetting @@ -276,8 +296,8 @@ uint32_t WarnIfComponentBlockingGuard::finish() { } if (should_warn) { const char *src = component_ == nullptr ? "" : component_->get_component_source(); - ESP_LOGW(TAG, "Component %s took a long time for an operation (%" PRIu32 " ms).", src, blocking_time); - ESP_LOGW(TAG, "Components should block for at most 30 ms."); + ESP_LOGW(TAG, "Component %s took a long time for an operation (%" PRIu32 " ms)", src, blocking_time); + ESP_LOGW(TAG, "Components should block for at most 30 ms"); } return curr_time; diff --git a/esphome/core/component.h b/esphome/core/component.h index f77d40ae35..a37d64086a 100644 --- a/esphome/core/component.h +++ b/esphome/core/component.h @@ -58,6 +58,7 @@ extern const uint8_t COMPONENT_STATE_CONSTRUCTION; extern const uint8_t COMPONENT_STATE_SETUP; extern const uint8_t COMPONENT_STATE_LOOP; extern const uint8_t COMPONENT_STATE_FAILED; +extern const uint8_t COMPONENT_STATE_LOOP_DONE; extern const uint8_t STATUS_LED_MASK; extern const uint8_t STATUS_LED_OK; extern const uint8_t STATUS_LED_WARNING; @@ -150,6 +151,26 @@ class Component { this->mark_failed(); } + /** Disable this component's loop. The loop() method will no longer be called. + * + * This is useful for components that only need to run for a certain period of time + * or when inactive, saving CPU cycles. + * + * @note Components should call this->disable_loop() on themselves, not on other components. + * This ensures the component's state is properly updated along with the loop partition. + */ + void disable_loop(); + + /** Enable this component's loop. The loop() method will be called normally. + * + * This is useful for components that transition between active and inactive states + * and need to re-enable their loop() method when becoming active again. + * + * @note Components should call this->enable_loop() on themselves, not on other components. + * This ensures the component's state is properly updated along with the loop partition. + */ + void enable_loop(); + bool is_failed() const; bool is_ready() const; diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 90377300a6..525e3541b3 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -3,7 +3,7 @@ from __future__ import annotations import asyncio -from collections.abc import AsyncGenerator, Generator +from collections.abc import AsyncGenerator, Callable, Generator from contextlib import AbstractAsyncContextManager, asynccontextmanager import logging import os @@ -46,6 +46,7 @@ if platform.system() == "Windows": "Integration tests are not supported on Windows", allow_module_level=True ) + import pty # not available on Windows @@ -362,7 +363,10 @@ async def api_client_connected( async def _read_stream_lines( - stream: asyncio.StreamReader, lines: list[str], output_stream: TextIO + stream: asyncio.StreamReader, + lines: list[str], + output_stream: TextIO, + line_callback: Callable[[str], None] | None = None, ) -> None: """Read lines from a stream, append to list, and echo to output stream.""" log_parser = LogParser() @@ -380,6 +384,9 @@ async def _read_stream_lines( file=output_stream, flush=True, ) + # Call the callback if provided + if line_callback: + line_callback(decoded_line.rstrip()) @asynccontextmanager @@ -388,6 +395,7 @@ async def run_binary_and_wait_for_port( host: str, port: int, timeout: float = PORT_WAIT_TIMEOUT, + line_callback: Callable[[str], None] | None = None, ) -> AsyncGenerator[None]: """Run a binary, wait for it to open a port, and clean up on exit.""" # Create a pseudo-terminal to make the binary think it's running interactively @@ -435,7 +443,9 @@ async def run_binary_and_wait_for_port( # Read from output stream output_tasks = [ asyncio.create_task( - _read_stream_lines(output_reader, stdout_lines, sys.stdout) + _read_stream_lines( + output_reader, stdout_lines, sys.stdout, line_callback + ) ) ] @@ -515,6 +525,7 @@ async def run_compiled_context( compile_esphome: CompileFunction, port: int, port_socket: socket.socket | None = None, + line_callback: Callable[[str], None] | None = None, ) -> AsyncGenerator[None]: """Context manager to write, compile and run an ESPHome configuration.""" # Write the YAML config @@ -528,7 +539,9 @@ async def run_compiled_context( port_socket.close() # Run the binary and wait for the API server to start - async with run_binary_and_wait_for_port(binary_path, LOCALHOST, port): + async with run_binary_and_wait_for_port( + binary_path, LOCALHOST, port, line_callback=line_callback + ): yield @@ -542,7 +555,9 @@ async def run_compiled( port, port_socket = reserved_tcp_port def _run_compiled( - yaml_content: str, filename: str | None = None + yaml_content: str, + filename: str | None = None, + line_callback: Callable[[str], None] | None = None, ) -> AbstractAsyncContextManager[asyncio.subprocess.Process]: return run_compiled_context( yaml_content, @@ -551,6 +566,7 @@ async def run_compiled( compile_esphome, port, port_socket, + line_callback=line_callback, ) yield _run_compiled diff --git a/tests/integration/fixtures/external_components/loop_test_component/__init__.py b/tests/integration/fixtures/external_components/loop_test_component/__init__.py new file mode 100644 index 0000000000..c5eda67d1e --- /dev/null +++ b/tests/integration/fixtures/external_components/loop_test_component/__init__.py @@ -0,0 +1,78 @@ +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 + +CODEOWNERS = ["@esphome/tests"] + +loop_test_component_ns = cg.esphome_ns.namespace("loop_test_component") +LoopTestComponent = loop_test_component_ns.class_("LoopTestComponent", cg.Component) + +CONF_DISABLE_AFTER = "disable_after" +CONF_TEST_REDUNDANT_OPERATIONS = "test_redundant_operations" + +COMPONENT_CONFIG_SCHEMA = cv.Schema( + { + cv.GenerateID(): cv.declare_id(LoopTestComponent), + cv.Required(CONF_NAME): cv.string, + cv.Optional(CONF_DISABLE_AFTER, default=0): cv.int_, + cv.Optional(CONF_TEST_REDUNDANT_OPERATIONS, default=False): cv.boolean, + } +) + +CONFIG_SCHEMA = cv.Schema( + { + cv.GenerateID(): cv.declare_id(LoopTestComponent), + cv.Required(CONF_COMPONENTS): cv.ensure_list(COMPONENT_CONFIG_SCHEMA), + } +).extend(cv.COMPONENT_SCHEMA) + +# Define actions +EnableAction = loop_test_component_ns.class_("EnableAction", automation.Action) +DisableAction = loop_test_component_ns.class_("DisableAction", automation.Action) + + +@automation.register_action( + "loop_test_component.enable", + EnableAction, + cv.Schema( + { + cv.Required(CONF_ID): cv.use_id(LoopTestComponent), + } + ), +) +async def enable_to_code(config, action_id, template_arg, args): + parent = await cg.get_variable(config[CONF_ID]) + var = cg.new_Pvariable(action_id, template_arg, parent) + return var + + +@automation.register_action( + "loop_test_component.disable", + DisableAction, + cv.Schema( + { + cv.Required(CONF_ID): cv.use_id(LoopTestComponent), + } + ), +) +async def disable_to_code(config, action_id, template_arg, args): + parent = await cg.get_variable(config[CONF_ID]) + var = cg.new_Pvariable(action_id, template_arg, parent) + return var + + +async def to_code(config): + # The parent config doesn't actually create a component + # We just create each sub-component + for comp_config in config[CONF_COMPONENTS]: + var = cg.new_Pvariable(comp_config[CONF_ID]) + await cg.register_component(var, comp_config) + + cg.add(var.set_name(comp_config[CONF_NAME])) + cg.add(var.set_disable_after(comp_config[CONF_DISABLE_AFTER])) + cg.add( + var.set_test_redundant_operations( + comp_config[CONF_TEST_REDUNDANT_OPERATIONS] + ) + ) 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 new file mode 100644 index 0000000000..470740c534 --- /dev/null +++ b/tests/integration/fixtures/external_components/loop_test_component/loop_test_component.cpp @@ -0,0 +1,43 @@ +#include "loop_test_component.h" + +namespace esphome { +namespace loop_test_component { + +void LoopTestComponent::setup() { ESP_LOGI(TAG, "[%s] Setup called", this->name_.c_str()); } + +void LoopTestComponent::loop() { + this->loop_count_++; + ESP_LOGI(TAG, "[%s] Loop count: %d", this->name_.c_str(), this->loop_count_); + + // Test self-disable after specified count + if (this->disable_after_ > 0 && this->loop_count_ == this->disable_after_) { + ESP_LOGI(TAG, "[%s] Disabling self after %d loops", this->name_.c_str(), this->disable_after_); + this->disable_loop(); + } + + // Test redundant operations + if (this->test_redundant_operations_ && this->loop_count_ == 5) { + if (this->name_ == "redundant_enable") { + ESP_LOGI(TAG, "[%s] Testing enable when already enabled", this->name_.c_str()); + this->enable_loop(); + } else if (this->name_ == "redundant_disable") { + ESP_LOGI(TAG, "[%s] Testing disable when will be disabled", this->name_.c_str()); + // We'll disable at count 10, but try to disable again at 5 + this->disable_loop(); + ESP_LOGI(TAG, "[%s] First disable complete", this->name_.c_str()); + } + } +} + +void LoopTestComponent::service_enable() { + ESP_LOGI(TAG, "[%s] Service enable called", this->name_.c_str()); + this->enable_loop(); +} + +void LoopTestComponent::service_disable() { + ESP_LOGI(TAG, "[%s] Service disable called", this->name_.c_str()); + this->disable_loop(); +} + +} // 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 new file mode 100644 index 0000000000..5c43dd4b43 --- /dev/null +++ b/tests/integration/fixtures/external_components/loop_test_component/loop_test_component.h @@ -0,0 +1,58 @@ +#pragma once + +#include "esphome/core/component.h" +#include "esphome/core/log.h" +#include "esphome/core/application.h" +#include "esphome/core/automation.h" + +namespace esphome { +namespace loop_test_component { + +static const char *const TAG = "loop_test_component"; + +class LoopTestComponent : public Component { + public: + void set_name(const std::string &name) { this->name_ = name; } + void set_disable_after(int count) { this->disable_after_ = count; } + void set_test_redundant_operations(bool test) { this->test_redundant_operations_ = test; } + + void setup() override; + void loop() override; + + // Service methods for external control + void service_enable(); + void service_disable(); + + 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 disable_after_{0}; + bool test_redundant_operations_{false}; +}; + +template class EnableAction : public Action { + public: + EnableAction(LoopTestComponent *parent) : parent_(parent) {} + + void play(Ts... x) override { this->parent_->service_enable(); } + + protected: + LoopTestComponent *parent_; +}; + +template class DisableAction : public Action { + public: + DisableAction(LoopTestComponent *parent) : parent_(parent) {} + + void play(Ts... x) override { this->parent_->service_disable(); } + + protected: + LoopTestComponent *parent_; +}; + +} // namespace loop_test_component +} // namespace esphome diff --git a/tests/integration/fixtures/loop_disable_enable.yaml b/tests/integration/fixtures/loop_disable_enable.yaml new file mode 100644 index 0000000000..17010f7c34 --- /dev/null +++ b/tests/integration/fixtures/loop_disable_enable.yaml @@ -0,0 +1,48 @@ +esphome: + name: loop-test + +host: +api: +logger: + level: DEBUG + +external_components: + - source: + type: local + path: EXTERNAL_COMPONENT_PATH + +loop_test_component: + components: + # Component that disables itself after 10 loops + - id: self_disable_10 + name: "self_disable_10" + disable_after: 10 + + # Component that never disables itself (for re-enable test) + - id: normal_component + name: "normal_component" + disable_after: 0 + + # Component that tests enable when already enabled + - id: redundant_enable + name: "redundant_enable" + test_redundant_operations: true + disable_after: 0 + + # Component that tests disable when already disabled + - id: redundant_disable + name: "redundant_disable" + test_redundant_operations: true + disable_after: 10 + +# Interval to re-enable the self_disable_10 component after some time +interval: + - interval: 0.5s + then: + - if: + condition: + lambda: 'return id(self_disable_10).get_loop_count() == 10;' + then: + - logger.log: "Re-enabling self_disable_10 via service" + - loop_test_component.enable: + id: self_disable_10 diff --git a/tests/integration/test_loop_disable_enable.py b/tests/integration/test_loop_disable_enable.py new file mode 100644 index 0000000000..84301c25d8 --- /dev/null +++ b/tests/integration/test_loop_disable_enable.py @@ -0,0 +1,150 @@ +"""Integration test for loop disable/enable functionality.""" + +from __future__ import annotations + +import asyncio +from pathlib import Path +import re + +import pytest + +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_loop_disable_enable( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test that components can disable and enable their loop() method.""" + # Get the absolute path to the external components directory + external_components_path = str( + Path(__file__).parent / "fixtures" / "external_components" + ) + + # Replace the placeholder in the YAML config with the actual path + yaml_config = yaml_config.replace( + "EXTERNAL_COMPONENT_PATH", external_components_path + ) + + # Track log messages and events + log_messages: list[str] = [] + + # Event fired when self_disable_10 component disables itself after 10 loops + self_disable_10_disabled = asyncio.Event() + # Event fired when normal_component reaches 10 loops + normal_component_10_loops = asyncio.Event() + # Event fired when redundant_enable component tests enabling when already enabled + redundant_enable_tested = asyncio.Event() + # Event fired when redundant_disable component tests disabling when already disabled + redundant_disable_tested = asyncio.Event() + # Event fired when self_disable_10 component is re-enabled and runs again (count > 10) + self_disable_10_re_enabled = asyncio.Event() + + # Track loop counts for components + self_disable_10_counts: list[int] = [] + normal_component_counts: list[int] = [] + + def on_log_line(line: str) -> None: + """Process each log line from the process output.""" + # Strip ANSI color codes + clean_line = re.sub(r"\x1b\[[0-9;]*m", "", line) + + if "loop_test_component" not in clean_line: + return + + log_messages.append(clean_line) + + # Track specific events using the cleaned line + if "[self_disable_10]" in clean_line: + if "Loop count:" in clean_line: + # Extract loop count + try: + count = int(clean_line.split("Loop count: ")[1]) + self_disable_10_counts.append(count) + # Check if component was re-enabled (count > 10) + if count > 10: + self_disable_10_re_enabled.set() + except (IndexError, ValueError): + pass + elif "Disabling self after 10 loops" in clean_line: + self_disable_10_disabled.set() + + elif "[normal_component]" in clean_line and "Loop count:" in clean_line: + try: + count = int(clean_line.split("Loop count: ")[1]) + normal_component_counts.append(count) + if count >= 10: + normal_component_10_loops.set() + except (IndexError, ValueError): + pass + + elif ( + "[redundant_enable]" in clean_line + and "Testing enable when already enabled" in clean_line + ): + redundant_enable_tested.set() + + elif ( + "[redundant_disable]" in clean_line + and "Testing disable when will be disabled" in clean_line + ): + redundant_disable_tested.set() + + # Write, compile and run the ESPHome device with log callback + async with ( + run_compiled(yaml_config, line_callback=on_log_line), + api_client_connected() as client, + ): + # Verify we can connect and get device info + device_info = await client.device_info() + assert device_info is not None + assert device_info.name == "loop-test" + + # Wait for self_disable_10 to disable itself + try: + await asyncio.wait_for(self_disable_10_disabled.wait(), timeout=10.0) + except asyncio.TimeoutError: + pytest.fail("self_disable_10 did not disable itself within 10 seconds") + + # Verify it ran at least 10 times before disabling + assert len([c for c in self_disable_10_counts if c <= 10]) == 10, ( + f"Expected exactly 10 loops before disable, got {[c for c in self_disable_10_counts if c <= 10]}" + ) + assert self_disable_10_counts[:10] == list(range(1, 11)), ( + f"Expected first 10 counts to be 1-10, got {self_disable_10_counts[:10]}" + ) + + # Wait for normal_component to run at least 10 times + try: + await asyncio.wait_for(normal_component_10_loops.wait(), timeout=10.0) + except asyncio.TimeoutError: + pytest.fail( + f"normal_component did not reach 10 loops within timeout, got {len(normal_component_counts)}" + ) + + # Wait for redundant operation tests + try: + await asyncio.wait_for(redundant_enable_tested.wait(), timeout=10.0) + except asyncio.TimeoutError: + pytest.fail("redundant_enable did not test enabling when already enabled") + + try: + await asyncio.wait_for(redundant_disable_tested.wait(), timeout=10.0) + except asyncio.TimeoutError: + pytest.fail( + "redundant_disable did not test disabling when will be disabled" + ) + + # Wait to see if self_disable_10 gets re-enabled + try: + await asyncio.wait_for(self_disable_10_re_enabled.wait(), timeout=5.0) + except asyncio.TimeoutError: + pytest.fail("self_disable_10 was not re-enabled within 5 seconds") + + # Component was re-enabled - verify it ran more times + later_self_disable_counts = [c for c in self_disable_10_counts if c > 10] + assert later_self_disable_counts, ( + "self_disable_10 was re-enabled but did not run additional times" + ) diff --git a/tests/integration/types.py b/tests/integration/types.py index 6fc3e9435e..5e4bfaa29d 100644 --- a/tests/integration/types.py +++ b/tests/integration/types.py @@ -13,7 +13,19 @@ from aioesphomeapi import APIClient ConfigWriter = Callable[[str, str | None], Awaitable[Path]] CompileFunction = Callable[[Path], Awaitable[Path]] RunFunction = Callable[[Path], Awaitable[asyncio.subprocess.Process]] -RunCompiledFunction = Callable[[str, str | None], AbstractAsyncContextManager[None]] + + +class RunCompiledFunction(Protocol): + """Protocol for run_compiled function with optional line callback.""" + + def __call__( # noqa: E704 + self, + yaml_content: str, + filename: str | None = None, + line_callback: Callable[[str], None] | None = None, + ) -> AbstractAsyncContextManager[None]: ... + + WaitFunction = Callable[[APIClient, float], Awaitable[bool]]