From 22fc3aab392c8cad0c2d8a74e6074b83e7c17139 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 4 Mar 2026 13:19:46 -0500 Subject: [PATCH 01/13] [ld2420] Fix buffer overflows in simple mode, energy mode, and calibration (#14458) Co-authored-by: Claude Opus 4.6 --- esphome/components/ld2420/ld2420.cpp | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/esphome/components/ld2420/ld2420.cpp b/esphome/components/ld2420/ld2420.cpp index f14400d15a4..1e671363c9d 100644 --- a/esphome/components/ld2420/ld2420.cpp +++ b/esphome/components/ld2420/ld2420.cpp @@ -460,6 +460,10 @@ void LD2420Component::handle_energy_mode_(uint8_t *buffer, int len) { uint8_t index = 6; // Start at presence byte position uint16_t range; const uint8_t elements = sizeof(this->gate_energy_) / sizeof(this->gate_energy_[0]); + if (len < static_cast(index + 1 + sizeof(range) + elements * sizeof(this->gate_energy_[0]))) { + ESP_LOGW(TAG, "Energy frame too short: %d bytes", len); + return; + } this->set_presence_(buffer[index]); index++; memcpy(&range, &buffer[index], sizeof(range)); @@ -471,8 +475,11 @@ void LD2420Component::handle_energy_mode_(uint8_t *buffer, int len) { } if (this->current_operating_mode == OP_CALIBRATE_MODE) { - this->update_radar_data(gate_energy_, sample_number_counter); - this->sample_number_counter > CALIBRATE_SAMPLES ? this->sample_number_counter = 0 : this->sample_number_counter++; + this->update_radar_data(gate_energy_, this->sample_number_counter); + this->sample_number_counter++; + if (this->sample_number_counter >= CALIBRATE_SAMPLES) { + this->sample_number_counter = 0; + } } // Resonable refresh rate for home assistant database size health @@ -503,22 +510,20 @@ void LD2420Component::handle_simple_mode_(const uint8_t *inbuf, int len) { char *endptr{nullptr}; char outbuf[bufsize]{0}; while (true) { - if (inbuf[pos - 2] == 'O' && inbuf[pos - 1] == 'F' && inbuf[pos] == 'F') { + if (pos >= 2 && inbuf[pos - 2] == 'O' && inbuf[pos - 1] == 'F' && inbuf[pos] == 'F') { this->set_presence_(false); - } else if (inbuf[pos - 1] == 'O' && inbuf[pos] == 'N') { + } else if (pos >= 1 && inbuf[pos - 1] == 'O' && inbuf[pos] == 'N') { this->set_presence_(true); } if (inbuf[pos] >= '0' && inbuf[pos] <= '9') { if (index < bufsize - 1) { outbuf[index++] = inbuf[pos]; - pos++; } + } + if (pos < len - 1) { + pos++; } else { - if (pos < len - 1) { - pos++; - } else { - break; - } + break; } } outbuf[index] = '\0'; From 4928e678d1f54ee261789ab283280f317b425357 Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Wed, 4 Mar 2026 13:37:22 -0600 Subject: [PATCH 02/13] [mixer][resampler][speaker] Use core static task manager (#14454) --- .../mixer/speaker/mixer_speaker.cpp | 91 +++------------ .../components/mixer/speaker/mixer_speaker.h | 18 +-- .../resampler/speaker/resampler_speaker.cpp | 60 +--------- .../resampler/speaker/resampler_speaker.h | 19 +-- .../speaker/media_player/audio_pipeline.cpp | 110 ++++-------------- .../speaker/media_player/audio_pipeline.h | 13 +-- 6 files changed, 49 insertions(+), 262 deletions(-) diff --git a/esphome/components/mixer/speaker/mixer_speaker.cpp b/esphome/components/mixer/speaker/mixer_speaker.cpp index 100acbebc33..8e1278206f4 100644 --- a/esphome/components/mixer/speaker/mixer_speaker.cpp +++ b/esphome/components/mixer/speaker/mixer_speaker.cpp @@ -438,24 +438,14 @@ void MixerSpeaker::loop() { // Handle pending start request if (event_group_bits & MIXER_TASK_COMMAND_START) { // Only start the task if it's fully stopped and cleaned up - if (!this->status_has_error() && (this->task_handle_ == nullptr) && (this->task_stack_buffer_ == nullptr)) { - esp_err_t err = this->start_task_(); - switch (err) { - case ESP_OK: - xEventGroupClearBits(this->event_group_, MIXER_TASK_COMMAND_START); - break; - case ESP_ERR_NO_MEM: - ESP_LOGE(TAG, "Failed to start; retrying in 1 second"); - this->status_momentary_error("memory-failure", 1000); - return; - case ESP_ERR_INVALID_STATE: - ESP_LOGE(TAG, "Failed to start; retrying in 1 second"); - this->status_momentary_error("task-failure", 1000); - return; - default: - ESP_LOGE(TAG, "Failed to start; retrying in 1 second"); - this->status_momentary_error("failure", 1000); - return; + if (!this->status_has_error() && !this->task_.is_created()) { + if (this->task_.create(audio_mixer_task, "mixer", TASK_STACK_SIZE, (void *) this, MIXER_TASK_PRIORITY, + this->task_stack_in_psram_)) { + xEventGroupClearBits(this->event_group_, MIXER_TASK_COMMAND_START); + } else { + ESP_LOGE(TAG, "Failed to start; retrying in 1 second"); + this->status_momentary_error("failure", 1000); + return; } } } @@ -478,13 +468,12 @@ void MixerSpeaker::loop() { xEventGroupClearBits(this->event_group_, MIXER_TASK_STATE_STOPPING); } if (event_group_bits & MIXER_TASK_STATE_STOPPED) { - if (this->delete_task_() == ESP_OK) { - ESP_LOGD(TAG, "Stopped"); - xEventGroupClearBits(this->event_group_, MIXER_TASK_ALL_BITS); - } + this->task_.deallocate(); + ESP_LOGD(TAG, "Stopped"); + xEventGroupClearBits(this->event_group_, MIXER_TASK_ALL_BITS); } - if (this->task_handle_ != nullptr) { + if (this->task_.is_created()) { // If the mixer task is running, check if all source speakers are stopped bool all_stopped = true; @@ -497,7 +486,7 @@ void MixerSpeaker::loop() { // Send stop command signal to the mixer task since no source speakers are active xEventGroupSetBits(this->event_group_, MIXER_TASK_COMMAND_STOP); } - } else if (this->task_stack_buffer_ == nullptr) { + } else { // Task is fully stopped and cleaned up, check if we can disable loop event_group_bits = xEventGroupGetBits(this->event_group_); if (event_group_bits == 0) { @@ -538,60 +527,6 @@ esp_err_t MixerSpeaker::start(audio::AudioStreamInfo &stream_info) { return ESP_OK; } -esp_err_t MixerSpeaker::start_task_() { - if (this->task_stack_buffer_ == nullptr) { - if (this->task_stack_in_psram_) { - RAMAllocator stack_allocator(RAMAllocator::ALLOC_EXTERNAL); - this->task_stack_buffer_ = stack_allocator.allocate(TASK_STACK_SIZE); - } else { - RAMAllocator stack_allocator(RAMAllocator::ALLOC_INTERNAL); - this->task_stack_buffer_ = stack_allocator.allocate(TASK_STACK_SIZE); - } - } - - if (this->task_stack_buffer_ == nullptr) { - return ESP_ERR_NO_MEM; - } - - if (this->task_handle_ == nullptr) { - this->task_handle_ = xTaskCreateStatic(audio_mixer_task, "mixer", TASK_STACK_SIZE, (void *) this, - MIXER_TASK_PRIORITY, this->task_stack_buffer_, &this->task_stack_); - } - - if (this->task_handle_ == nullptr) { - return ESP_ERR_INVALID_STATE; - } - - return ESP_OK; -} - -esp_err_t MixerSpeaker::delete_task_() { - if (this->task_handle_ != nullptr) { - // Delete the task - vTaskDelete(this->task_handle_); - this->task_handle_ = nullptr; - } - - if ((this->task_handle_ == nullptr) && (this->task_stack_buffer_ != nullptr)) { - // Deallocate the task stack buffer - if (this->task_stack_in_psram_) { - RAMAllocator stack_allocator(RAMAllocator::ALLOC_EXTERNAL); - stack_allocator.deallocate(this->task_stack_buffer_, TASK_STACK_SIZE); - } else { - RAMAllocator stack_allocator(RAMAllocator::ALLOC_INTERNAL); - stack_allocator.deallocate(this->task_stack_buffer_, TASK_STACK_SIZE); - } - - this->task_stack_buffer_ = nullptr; - } - - if ((this->task_handle_ != nullptr) || (this->task_stack_buffer_ != nullptr)) { - return ESP_ERR_INVALID_STATE; - } - - return ESP_OK; -} - void MixerSpeaker::copy_frames(const int16_t *input_buffer, audio::AudioStreamInfo input_stream_info, int16_t *output_buffer, audio::AudioStreamInfo output_stream_info, uint32_t frames_to_transfer) { diff --git a/esphome/components/mixer/speaker/mixer_speaker.h b/esphome/components/mixer/speaker/mixer_speaker.h index e920f9895a0..0e0b33c39bc 100644 --- a/esphome/components/mixer/speaker/mixer_speaker.h +++ b/esphome/components/mixer/speaker/mixer_speaker.h @@ -8,8 +8,8 @@ #include "esphome/core/component.h" #include "esphome/core/helpers.h" +#include "esphome/core/static_task.h" -#include #include #include @@ -143,8 +143,6 @@ class MixerSpeaker : public Component { /// @param stream_info The calling source speaker's audio stream information /// @return ESP_ERR_NOT_SUPPORTED if the incoming stream is incompatible due to unsupported bits per sample /// ESP_ERR_INVALID_ARG if the incoming stream is incompatible to be mixed with the other input audio stream - /// ESP_ERR_NO_MEM if there isn't enough memory for the task's stack - /// ESP_ERR_INVALID_STATE if the task fails to start /// ESP_OK if the incoming stream is compatible and the mixer task starts esp_err_t start(audio::AudioStreamInfo &stream_info); @@ -188,16 +186,6 @@ class MixerSpeaker : public Component { static void audio_mixer_task(void *params); - /// @brief Starts the mixer task after allocating memory for the task stack. - /// @return ESP_ERR_NO_MEM if there isn't enough memory for the task's stack - /// ESP_ERR_INVALID_STATE if the task didn't start - /// ESP_OK if successful - esp_err_t start_task_(); - - /// @brief If the task is stopped, it sets the task handle to the nullptr and deallocates its stack - /// @return ESP_OK if the task was stopped, ESP_ERR_INVALID_STATE otherwise. - esp_err_t delete_task_(); - EventGroupHandle_t event_group_{nullptr}; FixedVector source_speakers_; @@ -207,9 +195,7 @@ class MixerSpeaker : public Component { bool queue_mode_; bool task_stack_in_psram_{false}; - TaskHandle_t task_handle_{nullptr}; - StaticTask_t task_stack_; - StackType_t *task_stack_buffer_{nullptr}; + StaticTask task_; optional audio_stream_info_; diff --git a/esphome/components/resampler/speaker/resampler_speaker.cpp b/esphome/components/resampler/speaker/resampler_speaker.cpp index 74420f906a1..1303bc459e5 100644 --- a/esphome/components/resampler/speaker/resampler_speaker.cpp +++ b/esphome/components/resampler/speaker/resampler_speaker.cpp @@ -147,7 +147,7 @@ void ResamplerSpeaker::loop() { xEventGroupClearBits(this->event_group_, ResamplingEventGroupBits::STATE_STOPPING); } if (event_group_bits & ResamplingEventGroupBits::STATE_STOPPED) { - this->delete_task_(); + this->task_.deallocate(); ESP_LOGD(TAG, "Stopped"); xEventGroupClearBits(this->event_group_, ResamplingEventGroupBits::ALL_BITS); } @@ -190,7 +190,7 @@ void ResamplerSpeaker::loop() { this->output_speaker_->stop(); } - if (this->output_speaker_->is_stopped() && (this->task_handle_ == nullptr)) { + if (this->output_speaker_->is_stopped() && !this->task_.is_created()) { // Only transition to stopped state once the output speaker and resampler task are fully stopped this->waiting_for_output_ = false; this->state_ = speaker::STATE_STOPPED; @@ -209,9 +209,6 @@ void ResamplerSpeaker::loop() { void ResamplerSpeaker::set_start_error_(esp_err_t err) { switch (err) { - case ESP_ERR_INVALID_STATE: - this->status_set_error(LOG_STR("Task failed to start")); - break; case ESP_ERR_NO_MEM: this->status_set_error(LOG_STR("Not enough memory")); break; @@ -267,36 +264,12 @@ esp_err_t ResamplerSpeaker::start_() { if (this->requires_resampling_()) { // Start the resampler task to handle converting sample rates - return this->start_task_(); - } - - return ESP_OK; -} - -esp_err_t ResamplerSpeaker::start_task_() { - if (this->task_stack_buffer_ == nullptr) { - if (this->task_stack_in_psram_) { - RAMAllocator stack_allocator(RAMAllocator::ALLOC_EXTERNAL); - this->task_stack_buffer_ = stack_allocator.allocate(TASK_STACK_SIZE); - } else { - RAMAllocator stack_allocator(RAMAllocator::ALLOC_INTERNAL); - this->task_stack_buffer_ = stack_allocator.allocate(TASK_STACK_SIZE); + if (!this->task_.create(resample_task, "resampler", TASK_STACK_SIZE, (void *) this, RESAMPLER_TASK_PRIORITY, + this->task_stack_in_psram_)) { + return ESP_ERR_NO_MEM; } } - if (this->task_stack_buffer_ == nullptr) { - return ESP_ERR_NO_MEM; - } - - if (this->task_handle_ == nullptr) { - this->task_handle_ = xTaskCreateStatic(resample_task, "resampler", TASK_STACK_SIZE, (void *) this, - RESAMPLER_TASK_PRIORITY, this->task_stack_buffer_, &this->task_stack_); - } - - if (this->task_handle_ == nullptr) { - return ESP_ERR_INVALID_STATE; - } - return ESP_OK; } @@ -305,33 +278,12 @@ void ResamplerSpeaker::stop() { this->send_command_(ResamplingEventGroupBits::CO void ResamplerSpeaker::enter_stopping_state_() { this->state_ = speaker::STATE_STOPPING; this->state_start_ms_ = App.get_loop_component_start_time(); - if (this->task_handle_ != nullptr) { + if (this->task_.is_created()) { xEventGroupSetBits(this->event_group_, ResamplingEventGroupBits::TASK_COMMAND_STOP); } this->output_speaker_->stop(); } -void ResamplerSpeaker::delete_task_() { - if (this->task_handle_ != nullptr) { - // Delete the suspended task - vTaskDelete(this->task_handle_); - this->task_handle_ = nullptr; - } - - if (this->task_stack_buffer_ != nullptr) { - // Deallocate the task stack buffer - if (this->task_stack_in_psram_) { - RAMAllocator stack_allocator(RAMAllocator::ALLOC_EXTERNAL); - stack_allocator.deallocate(this->task_stack_buffer_, TASK_STACK_SIZE); - } else { - RAMAllocator stack_allocator(RAMAllocator::ALLOC_INTERNAL); - stack_allocator.deallocate(this->task_stack_buffer_, TASK_STACK_SIZE); - } - - this->task_stack_buffer_ = nullptr; - } -} - void ResamplerSpeaker::finish() { this->send_command_(ResamplingEventGroupBits::COMMAND_FINISH); } bool ResamplerSpeaker::has_buffered_data() const { diff --git a/esphome/components/resampler/speaker/resampler_speaker.h b/esphome/components/resampler/speaker/resampler_speaker.h index c1ebd7e7b5b..cdbc1c22db4 100644 --- a/esphome/components/resampler/speaker/resampler_speaker.h +++ b/esphome/components/resampler/speaker/resampler_speaker.h @@ -7,8 +7,8 @@ #include "esphome/components/speaker/speaker.h" #include "esphome/core/component.h" +#include "esphome/core/static_task.h" -#include #include namespace esphome { @@ -57,15 +57,9 @@ class ResamplerSpeaker : public Component, public speaker::Speaker { protected: /// @brief Starts the output speaker after setting the resampled stream info. If resampling is required, it starts the /// task. - /// @return ESP_OK if resampling is required - /// return value of start_task_() if resampling is required - esp_err_t start_(); - - /// @brief Starts the resampler task after allocating the task stack /// @return ESP_OK if successful, - /// ESP_ERR_NO_MEM if the task stack couldn't be allocated - /// ESP_ERR_INVALID_STATE if the task wasn't created - esp_err_t start_task_(); + /// ESP_ERR_NO_MEM if the resampler task couldn't be created + esp_err_t start_(); /// @brief Transitions to STATE_STOPPING, records the stopping timestamp, sends the task stop command if the task is /// running, and stops the output speaker. @@ -74,9 +68,6 @@ class ResamplerSpeaker : public Component, public speaker::Speaker { /// @brief Sets the appropriate status error based on the start failure reason. void set_start_error_(esp_err_t err); - /// @brief Deletes the resampler task if suspended, deallocates the task stack, and resets the related pointers. - void delete_task_(); - /// @brief Sends a command via event group bits, enables the loop, and optionally wakes the main loop. void send_command_(uint32_t command_bit, bool wake_loop = false); @@ -92,9 +83,7 @@ class ResamplerSpeaker : public Component, public speaker::Speaker { bool task_stack_in_psram_{false}; bool waiting_for_output_{false}; - TaskHandle_t task_handle_{nullptr}; - StaticTask_t task_stack_; - StackType_t *task_stack_buffer_{nullptr}; + StaticTask task_; audio::AudioStreamInfo target_stream_info_; diff --git a/esphome/components/speaker/media_player/audio_pipeline.cpp b/esphome/components/speaker/media_player/audio_pipeline.cpp index 177743feb1d..8cea3abcfc9 100644 --- a/esphome/components/speaker/media_player/audio_pipeline.cpp +++ b/esphome/components/speaker/media_player/audio_pipeline.cpp @@ -87,20 +87,20 @@ void AudioPipeline::set_pause_state(bool pause_state) { } void AudioPipeline::suspend_tasks() { - if (this->read_task_handle_ != nullptr) { - vTaskSuspend(this->read_task_handle_); + if (this->read_task_.is_created()) { + vTaskSuspend(this->read_task_.get_handle()); } - if (this->decode_task_handle_ != nullptr) { - vTaskSuspend(this->decode_task_handle_); + if (this->decode_task_.is_created()) { + vTaskSuspend(this->decode_task_.get_handle()); } } void AudioPipeline::resume_tasks() { - if (this->read_task_handle_ != nullptr) { - vTaskResume(this->read_task_handle_); + if (this->read_task_.is_created()) { + vTaskResume(this->read_task_.get_handle()); } - if (this->decode_task_handle_ != nullptr) { - vTaskResume(this->decode_task_handle_); + if (this->decode_task_.is_created()) { + vTaskResume(this->decode_task_.get_handle()); } } @@ -159,7 +159,7 @@ AudioPipelineState AudioPipeline::process_state() { // Init command pending if (!(event_bits & EventGroupBits::PIPELINE_COMMAND_STOP)) { // Only start if there is no pending stop command - if ((this->read_task_handle_ == nullptr) || (this->decode_task_handle_ == nullptr)) { + if (!this->read_task_.is_created() || !this->decode_task_.is_created()) { // At least one task isn't running this->start_tasks_(); } @@ -202,8 +202,9 @@ AudioPipelineState AudioPipeline::process_state() { if (!this->is_playing_) { // The tasks have been stopped for two ``process_state`` calls in a row, so delete the tasks - if ((this->read_task_handle_ != nullptr) || (this->decode_task_handle_ != nullptr)) { - this->delete_tasks_(); + if (this->read_task_.is_created() || this->decode_task_.is_created()) { + this->read_task_.deallocate(); + this->decode_task_.deallocate(); if (this->hard_stop_) { // Stop command was sent, so immediately end the playback this->speaker_->stop(); @@ -234,7 +235,7 @@ AudioPipelineState AudioPipeline::process_state() { } } - if ((this->read_task_handle_ == nullptr) && (this->decode_task_handle_ == nullptr)) { + if (!this->read_task_.is_created() && !this->decode_task_.is_created()) { // No tasks are running, so the pipeline is stopped. xEventGroupClearBits(this->event_group_, EventGroupBits::PIPELINE_COMMAND_STOP); return AudioPipelineState::STOPPED; @@ -262,94 +263,25 @@ esp_err_t AudioPipeline::allocate_communications_() { } esp_err_t AudioPipeline::start_tasks_() { - if (this->read_task_handle_ == nullptr) { - if (this->read_task_stack_buffer_ == nullptr) { - // Reader task uses the AudioReader class which uses esp_http_client. This crashes on IDF 5.4 if the task stack is - // in PSRAM. As a workaround, always allocate the read task in internal memory. - RAMAllocator stack_allocator(RAMAllocator::ALLOC_INTERNAL); - this->read_task_stack_buffer_ = stack_allocator.allocate(READ_TASK_STACK_SIZE); - } - - if (this->read_task_stack_buffer_ == nullptr) { + if (!this->read_task_.is_created()) { + // Reader task uses the AudioReader class which uses esp_http_client. This crashes on IDF 5.4 if the task stack is + // in PSRAM. As a workaround, always allocate the read task in internal memory. + if (!this->read_task_.create(read_task, (this->base_name_ + "_read").c_str(), READ_TASK_STACK_SIZE, (void *) this, + this->priority_, false)) { return ESP_ERR_NO_MEM; } - - if (this->read_task_handle_ == nullptr) { - this->read_task_handle_ = - xTaskCreateStatic(read_task, (this->base_name_ + "_read").c_str(), READ_TASK_STACK_SIZE, (void *) this, - this->priority_, this->read_task_stack_buffer_, &this->read_task_stack_); - } - - if (this->read_task_handle_ == nullptr) { - return ESP_ERR_INVALID_STATE; - } } - if (this->decode_task_handle_ == nullptr) { - if (this->decode_task_stack_buffer_ == nullptr) { - if (this->task_stack_in_psram_) { - RAMAllocator stack_allocator(RAMAllocator::ALLOC_EXTERNAL); - this->decode_task_stack_buffer_ = stack_allocator.allocate(DECODE_TASK_STACK_SIZE); - } else { - RAMAllocator stack_allocator(RAMAllocator::ALLOC_INTERNAL); - this->decode_task_stack_buffer_ = stack_allocator.allocate(DECODE_TASK_STACK_SIZE); - } - } - - if (this->decode_task_stack_buffer_ == nullptr) { + if (!this->decode_task_.is_created()) { + if (!this->decode_task_.create(decode_task, (this->base_name_ + "_decode").c_str(), DECODE_TASK_STACK_SIZE, + (void *) this, this->priority_, this->task_stack_in_psram_)) { return ESP_ERR_NO_MEM; } - - if (this->decode_task_handle_ == nullptr) { - this->decode_task_handle_ = - xTaskCreateStatic(decode_task, (this->base_name_ + "_decode").c_str(), DECODE_TASK_STACK_SIZE, (void *) this, - this->priority_, this->decode_task_stack_buffer_, &this->decode_task_stack_); - } - - if (this->decode_task_handle_ == nullptr) { - return ESP_ERR_INVALID_STATE; - } } return ESP_OK; } -void AudioPipeline::delete_tasks_() { - if (this->read_task_handle_ != nullptr) { - vTaskDelete(this->read_task_handle_); - - if (this->read_task_stack_buffer_ != nullptr) { - if (this->task_stack_in_psram_) { - RAMAllocator stack_allocator(RAMAllocator::ALLOC_EXTERNAL); - stack_allocator.deallocate(this->read_task_stack_buffer_, READ_TASK_STACK_SIZE); - } else { - RAMAllocator stack_allocator(RAMAllocator::ALLOC_INTERNAL); - stack_allocator.deallocate(this->read_task_stack_buffer_, READ_TASK_STACK_SIZE); - } - - this->read_task_stack_buffer_ = nullptr; - this->read_task_handle_ = nullptr; - } - } - - if (this->decode_task_handle_ != nullptr) { - vTaskDelete(this->decode_task_handle_); - - if (this->decode_task_stack_buffer_ != nullptr) { - if (this->task_stack_in_psram_) { - RAMAllocator stack_allocator(RAMAllocator::ALLOC_EXTERNAL); - stack_allocator.deallocate(this->decode_task_stack_buffer_, DECODE_TASK_STACK_SIZE); - } else { - RAMAllocator stack_allocator(RAMAllocator::ALLOC_INTERNAL); - stack_allocator.deallocate(this->decode_task_stack_buffer_, DECODE_TASK_STACK_SIZE); - } - - this->decode_task_stack_buffer_ = nullptr; - this->decode_task_handle_ = nullptr; - } - } -} - void AudioPipeline::read_task(void *params) { AudioPipeline *this_pipeline = (AudioPipeline *) params; diff --git a/esphome/components/speaker/media_player/audio_pipeline.h b/esphome/components/speaker/media_player/audio_pipeline.h index 6fffde6c206..2c785728359 100644 --- a/esphome/components/speaker/media_player/audio_pipeline.h +++ b/esphome/components/speaker/media_player/audio_pipeline.h @@ -8,10 +8,10 @@ #include "esphome/components/speaker/speaker.h" #include "esphome/core/ring_buffer.h" +#include "esphome/core/static_task.h" #include "esp_err.h" -#include #include #include @@ -104,9 +104,6 @@ class AudioPipeline { /// @return ESP_OK if successful or an appropriate error if not esp_err_t start_tasks_(); - /// @brief Resets the task related pointers and deallocates their stacks. - void delete_tasks_(); - std::string base_name_; UBaseType_t priority_; @@ -143,15 +140,11 @@ class AudioPipeline { // Handles reading the media file from flash or a url static void read_task(void *params); - TaskHandle_t read_task_handle_{nullptr}; - StaticTask_t read_task_stack_; - StackType_t *read_task_stack_buffer_{nullptr}; + StaticTask read_task_; // Decodes the media file into PCM audio static void decode_task(void *params); - TaskHandle_t decode_task_handle_{nullptr}; - StaticTask_t decode_task_stack_; - StackType_t *decode_task_stack_buffer_{nullptr}; + StaticTask decode_task_; }; } // namespace speaker From 0c883b80c4376906eb5dc67f4b77d1249ee67b1d Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 4 Mar 2026 16:05:49 -0500 Subject: [PATCH 03/13] [inkplate][ezo_pmp][ezo][packet_transport] Fix use-after-free bugs (#14467) Co-authored-by: Claude Opus 4.6 --- esphome/components/ezo/ezo.cpp | 3 +- esphome/components/ezo_pmp/ezo_pmp.cpp | 36 ++++++++++--------- esphome/components/ezo_pmp/ezo_pmp.h | 2 +- esphome/components/inkplate/inkplate.cpp | 20 ++++++++--- .../packet_transport/packet_transport.cpp | 2 +- .../packet_transport/packet_transport.h | 2 +- 6 files changed, 39 insertions(+), 26 deletions(-) diff --git a/esphome/components/ezo/ezo.cpp b/esphome/components/ezo/ezo.cpp index e4036021df7..2dc65b7d14c 100644 --- a/esphome/components/ezo/ezo.cpp +++ b/esphome/components/ezo/ezo.cpp @@ -66,8 +66,9 @@ void EZOSensor::loop() { if (to_run->command_type == EzoCommandType::EZO_SLEEP || to_run->command_type == EzoCommandType::EZO_I2C) { // Commands with no return data + bool update_address = to_run->command_type == EzoCommandType::EZO_I2C; this->commands_.pop_front(); - if (to_run->command_type == EzoCommandType::EZO_I2C) + if (update_address) this->address_ = this->new_address_; return; } diff --git a/esphome/components/ezo_pmp/ezo_pmp.cpp b/esphome/components/ezo_pmp/ezo_pmp.cpp index bf6e3926b87..4ce4da57ffc 100644 --- a/esphome/components/ezo_pmp/ezo_pmp.cpp +++ b/esphome/components/ezo_pmp/ezo_pmp.cpp @@ -165,22 +165,23 @@ void EzoPMP::read_command_result_() { continue; } - switch (current_parameter) { - case 1: - first_parameter_buffer[position_in_parameter_buffer] = current_char; - first_parameter_buffer[position_in_parameter_buffer + 1] = '\0'; - break; - case 2: - second_parameter_buffer[position_in_parameter_buffer] = current_char; - second_parameter_buffer[position_in_parameter_buffer + 1] = '\0'; - break; - case 3: - third_parameter_buffer[position_in_parameter_buffer] = current_char; - third_parameter_buffer[position_in_parameter_buffer + 1] = '\0'; - break; + if (position_in_parameter_buffer < sizeof(first_parameter_buffer) - 1) { + switch (current_parameter) { + case 1: + first_parameter_buffer[position_in_parameter_buffer] = current_char; + first_parameter_buffer[position_in_parameter_buffer + 1] = '\0'; + break; + case 2: + second_parameter_buffer[position_in_parameter_buffer] = current_char; + second_parameter_buffer[position_in_parameter_buffer + 1] = '\0'; + break; + case 3: + third_parameter_buffer[position_in_parameter_buffer] = current_char; + third_parameter_buffer[position_in_parameter_buffer + 1] = '\0'; + break; + } + position_in_parameter_buffer++; } - - position_in_parameter_buffer++; } auto parsed_first_parameter = parse_number(first_parameter_buffer); @@ -404,7 +405,8 @@ void EzoPMP::send_next_command_() { break; case EZO_PMP_COMMAND_EXEC_ARBITRARY_COMMAND_ADDRESS: // Run an arbitrary command - command_buffer_length = snprintf((char *) command_buffer, sizeof(command_buffer), "%s", this->arbitrary_command_); + command_buffer_length = + snprintf((char *) command_buffer, sizeof(command_buffer), "%s", this->arbitrary_command_.c_str()); ESP_LOGI(TAG, "Sending arbitrary command: %s", (char *) command_buffer); break; @@ -541,7 +543,7 @@ void EzoPMP::change_i2c_address(int address) { } void EzoPMP::exec_arbitrary_command(const std::basic_string &command) { - this->arbitrary_command_ = command.c_str(); + this->arbitrary_command_ = command; this->queue_command_(EZO_PMP_COMMAND_EXEC_ARBITRARY_COMMAND_ADDRESS, 0, 0, true); } diff --git a/esphome/components/ezo_pmp/ezo_pmp.h b/esphome/components/ezo_pmp/ezo_pmp.h index d4917e7f4b4..bbfd8991707 100644 --- a/esphome/components/ezo_pmp/ezo_pmp.h +++ b/esphome/components/ezo_pmp/ezo_pmp.h @@ -85,7 +85,7 @@ class EzoPMP : public PollingComponent, public i2c::I2CDevice { bool is_paused_flag_ = false; bool is_dosing_flag_ = false; - const char *arbitrary_command_{nullptr}; + std::string arbitrary_command_{}; void send_next_command_(); void read_command_result_(); diff --git a/esphome/components/inkplate/inkplate.cpp b/esphome/components/inkplate/inkplate.cpp index c921c643fa3..df9c2b29c78 100644 --- a/esphome/components/inkplate/inkplate.cpp +++ b/esphome/components/inkplate/inkplate.cpp @@ -63,16 +63,26 @@ void Inkplate::initialize_() { if (buffer_size == 0) return; - if (this->partial_buffer_ != nullptr) + if (this->partial_buffer_ != nullptr) { allocator.deallocate(this->partial_buffer_, buffer_size); - if (this->partial_buffer_2_ != nullptr) + this->partial_buffer_ = nullptr; + } + if (this->partial_buffer_2_ != nullptr) { allocator.deallocate(this->partial_buffer_2_, buffer_size * 2); - if (this->buffer_ != nullptr) + this->partial_buffer_2_ = nullptr; + } + if (this->buffer_ != nullptr) { allocator.deallocate(this->buffer_, buffer_size); - if (this->glut_ != nullptr) + this->buffer_ = nullptr; + } + if (this->glut_ != nullptr) { allocator32.deallocate(this->glut_, 256 * 9); - if (this->glut2_ != nullptr) + this->glut_ = nullptr; + } + if (this->glut2_ != nullptr) { allocator32.deallocate(this->glut2_, 256 * 9); + this->glut2_ = nullptr; + } this->buffer_ = allocator.allocate(buffer_size); if (this->buffer_ == nullptr) { diff --git a/esphome/components/packet_transport/packet_transport.cpp b/esphome/components/packet_transport/packet_transport.cpp index 7b7a8523986..d2c59200017 100644 --- a/esphome/components/packet_transport/packet_transport.cpp +++ b/esphome/components/packet_transport/packet_transport.cpp @@ -249,7 +249,7 @@ void PacketTransport::init_data_() { } else { add(this->data_, DATA_KEY); } - for (auto pkey : this->ping_keys_) { + for (const auto &pkey : this->ping_keys_) { add(this->data_, PING_KEY); add(this->data_, pkey.second); } diff --git a/esphome/components/packet_transport/packet_transport.h b/esphome/components/packet_transport/packet_transport.h index 57f40874b53..a2367442317 100644 --- a/esphome/components/packet_transport/packet_transport.h +++ b/esphome/components/packet_transport/packet_transport.h @@ -150,7 +150,7 @@ class PacketTransport : public PollingComponent { std::vector ping_header_{}; std::vector header_{}; std::vector data_{}; - std::map ping_keys_{}; + std::map ping_keys_{}; const char *platform_name_{""}; void add_key_(const char *name, uint32_t key); void send_ping_pong_request_(); From e11a91411b800497b812a9baf374fca36af5e085 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 4 Mar 2026 16:36:52 -0500 Subject: [PATCH 04/13] [esp32_improv][rf_bridge][esp32_ble_server][display][lvgl][pipsolar] Fix unsigned integer underflows (#14466) Co-authored-by: Claude Opus 4.6 Co-authored-by: Clyde Stubbs <2366188+clydebarrow@users.noreply.github.com> --- esphome/components/display/display.cpp | 3 +++ esphome/components/esp32_ble_server/ble_characteristic.cpp | 6 +++++- esphome/components/esp32_improv/esp32_improv_component.cpp | 2 ++ esphome/components/lvgl/lvgl_esphome.cpp | 2 +- esphome/components/pipsolar/pipsolar.cpp | 4 +++- esphome/components/rf_bridge/rf_bridge.cpp | 2 +- 6 files changed, 15 insertions(+), 4 deletions(-) diff --git a/esphome/components/display/display.cpp b/esphome/components/display/display.cpp index 2bd7d036006..f8569b6e7c9 100644 --- a/esphome/components/display/display.cpp +++ b/esphome/components/display/display.cpp @@ -661,6 +661,9 @@ void Display::printf(int x, int y, BaseFont *font, const char *format, ...) { void Display::set_writer(display_writer_t &&writer) { this->writer_ = writer; } void Display::set_pages(std::vector pages) { + if (pages.empty()) + return; + for (auto *page : pages) page->set_parent(this); diff --git a/esphome/components/esp32_ble_server/ble_characteristic.cpp b/esphome/components/esp32_ble_server/ble_characteristic.cpp index a1b1ff94bb3..d4ccefd9b29 100644 --- a/esphome/components/esp32_ble_server/ble_characteristic.cpp +++ b/esphome/components/esp32_ble_server/ble_characteristic.cpp @@ -209,7 +209,11 @@ void BLECharacteristic::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt esp_gatt_rsp_t response; if (param->read.is_long) { - if (this->value_.size() - this->value_read_offset_ < max_offset) { + if (this->value_read_offset_ >= this->value_.size()) { + response.attr_value.len = 0; + response.attr_value.offset = this->value_read_offset_; + this->value_read_offset_ = 0; + } else if (this->value_.size() - this->value_read_offset_ < max_offset) { // Last message in the chain response.attr_value.len = this->value_.size() - this->value_read_offset_; response.attr_value.offset = this->value_read_offset_; diff --git a/esphome/components/esp32_improv/esp32_improv_component.cpp b/esphome/components/esp32_improv/esp32_improv_component.cpp index 83bc842a3d3..e4ae49f2356 100644 --- a/esphome/components/esp32_improv/esp32_improv_component.cpp +++ b/esphome/components/esp32_improv/esp32_improv_component.cpp @@ -314,6 +314,8 @@ void ESP32ImprovComponent::dump_config() { } void ESP32ImprovComponent::process_incoming_data_() { + if (this->incoming_data_.size() < 3) + return; uint8_t length = this->incoming_data_[1]; #if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERBOSE diff --git a/esphome/components/lvgl/lvgl_esphome.cpp b/esphome/components/lvgl/lvgl_esphome.cpp index bb373abb88b..3e447e9169d 100644 --- a/esphome/components/lvgl/lvgl_esphome.cpp +++ b/esphome/components/lvgl/lvgl_esphome.cpp @@ -421,7 +421,7 @@ void LvglComponent::write_random_() { col = col / this->draw_rounding * this->draw_rounding; auto row = random_uint32() % this->disp_drv_.ver_res; row = row / this->draw_rounding * this->draw_rounding; - auto size = (random_uint32() % 32) / this->draw_rounding * this->draw_rounding - 1; + auto size = ((random_uint32() % 32) / this->draw_rounding + 2) * this->draw_rounding - 1; lv_area_t area; area.x1 = col; area.y1 = row; diff --git a/esphome/components/pipsolar/pipsolar.cpp b/esphome/components/pipsolar/pipsolar.cpp index f95bf4aedb9..9c5caec7758 100644 --- a/esphome/components/pipsolar/pipsolar.cpp +++ b/esphome/components/pipsolar/pipsolar.cpp @@ -162,13 +162,15 @@ void Pipsolar::loop() { } uint8_t Pipsolar::check_incoming_length_(uint8_t length) { - if (this->read_pos_ - 3 == length) { + if (this->read_pos_ >= 3 && this->read_pos_ - 3 == length) { return 1; } return 0; } uint8_t Pipsolar::check_incoming_crc_() { + if (this->read_pos_ < 3) + return 0; uint16_t crc16; crc16 = this->pipsolar_crc_(read_buffer_, read_pos_ - 3); if (((uint8_t) ((crc16) >> 8)) == read_buffer_[read_pos_ - 3] && diff --git a/esphome/components/rf_bridge/rf_bridge.cpp b/esphome/components/rf_bridge/rf_bridge.cpp index d8c148145ce..700e2ba1623 100644 --- a/esphome/components/rf_bridge/rf_bridge.cpp +++ b/esphome/components/rf_bridge/rf_bridge.cpp @@ -74,7 +74,7 @@ bool RFBridgeComponent::parse_bridge_byte_(uint8_t byte) { data.length = raw[2]; data.protocol = raw[3]; char next_byte[3]; // 2 hex chars + null - for (uint8_t i = 0; i < data.length - 1; i++) { + for (uint8_t i = 0; i + 1 < data.length; i++) { buf_append_printf(next_byte, sizeof(next_byte), 0, "%02X", raw[4 + i]); data.code += next_byte; } From 61ea6c3b2f759c325c1428b371e4e9f8a00425fb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 4 Mar 2026 12:46:26 -1000 Subject: [PATCH 05/13] [ci] Add missing issues: write permission to codeowner approval workflow (#14477) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/workflows/codeowner-approved-label.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/codeowner-approved-label.yml b/.github/workflows/codeowner-approved-label.yml index 217ae06419b..200f18f5448 100644 --- a/.github/workflows/codeowner-approved-label.yml +++ b/.github/workflows/codeowner-approved-label.yml @@ -12,7 +12,8 @@ on: types: [submitted, dismissed] permissions: - pull-requests: write + issues: write + pull-requests: read contents: read jobs: From 55103c0652bdece270ac09e9f3224c2693d7e2d2 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 4 Mar 2026 18:18:14 -0500 Subject: [PATCH 06/13] [ds2484] Fix read64() using uint8_t accumulator instead of uint64_t (#14479) Co-authored-by: Claude Opus 4.6 --- esphome/components/ds2484/ds2484.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esphome/components/ds2484/ds2484.cpp b/esphome/components/ds2484/ds2484.cpp index 7c890ff4339..0b36f868749 100644 --- a/esphome/components/ds2484/ds2484.cpp +++ b/esphome/components/ds2484/ds2484.cpp @@ -110,9 +110,9 @@ uint8_t DS2484OneWireBus::read8() { } uint64_t DS2484OneWireBus::read64() { - uint8_t response = 0; + uint64_t response = 0; for (uint8_t i = 0; i < 8; i++) { - response |= (this->read8() << (i * 8)); + response |= (static_cast(this->read8()) << (i * 8)); } return response; } From b6d7e8e14de939b4d6714a42138edb914fd346c4 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 4 Mar 2026 18:18:28 -0500 Subject: [PATCH 07/13] [sgp30] Fix serial number truncation from 48-bit to 24-bit (#14478) Co-authored-by: Claude Opus 4.6 --- esphome/components/sgp30/sgp30.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/esphome/components/sgp30/sgp30.cpp b/esphome/components/sgp30/sgp30.cpp index 18814405d48..35e5b3dd42b 100644 --- a/esphome/components/sgp30/sgp30.cpp +++ b/esphome/components/sgp30/sgp30.cpp @@ -41,7 +41,9 @@ void SGP30Component::setup() { this->mark_failed(); return; } - this->serial_number_ = encode_uint24(raw_serial_number[0], raw_serial_number[1], raw_serial_number[2]); + this->serial_number_ = (static_cast(raw_serial_number[0]) << 32) | + (static_cast(raw_serial_number[1]) << 16) | + static_cast(raw_serial_number[2]); ESP_LOGD(TAG, "Serial number: %" PRIu64, this->serial_number_); // Featureset identification for future use From c8e7f78a2567e51d4fa178799ad3351ce046272d Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Wed, 4 Mar 2026 18:32:50 -0500 Subject: [PATCH 08/13] [zwave_proxy] Fix uint8_t overflow for buffer index and frame end (#14480) Co-authored-by: Claude Opus 4.6 --- esphome/components/zwave_proxy/zwave_proxy.cpp | 4 ++++ esphome/components/zwave_proxy/zwave_proxy.h | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/esphome/components/zwave_proxy/zwave_proxy.cpp b/esphome/components/zwave_proxy/zwave_proxy.cpp index 8506b19e7f4..b0836ac0728 100644 --- a/esphome/components/zwave_proxy/zwave_proxy.cpp +++ b/esphome/components/zwave_proxy/zwave_proxy.cpp @@ -281,6 +281,10 @@ bool ZWaveProxy::parse_byte_(uint8_t byte) { break; } case ZWAVE_PARSING_STATE_READ_BL_MENU: + if (this->buffer_index_ >= this->buffer_.size()) { + this->parsing_state_ = ZWAVE_PARSING_STATE_WAIT_START; + break; + } this->buffer_[this->buffer_index_++] = byte; if (!byte) { this->parsing_state_ = ZWAVE_PARSING_STATE_WAIT_START; diff --git a/esphome/components/zwave_proxy/zwave_proxy.h b/esphome/components/zwave_proxy/zwave_proxy.h index eb26316f492..12cb9a90a1e 100644 --- a/esphome/components/zwave_proxy/zwave_proxy.h +++ b/esphome/components/zwave_proxy/zwave_proxy.h @@ -81,10 +81,10 @@ class ZWaveProxy : public uart::UARTDevice, public Component { api::APIConnection *api_connection_{nullptr}; // Current subscribed client uint32_t setup_time_{0}; // Time when setup() was called - // 8-bit values (grouped together to minimize padding) - uint8_t buffer_index_{0}; // Index for populating the data buffer - uint8_t end_frame_after_{0}; // Payload reception ends after this index - uint8_t last_response_{0}; // Last response type sent + // Small values (grouped by size to minimize padding) + uint16_t buffer_index_{0}; // Index for populating the data buffer + uint16_t end_frame_after_{0}; // Payload reception ends after this index + uint8_t last_response_{0}; // Last response type sent ZWaveParsingState parsing_state_{ZWAVE_PARSING_STATE_WAIT_START}; bool in_bootloader_{false}; // True if the device is detected to be in bootloader mode bool home_id_ready_{false}; // True when home ID has been received from Z-Wave module From c0143ac6d662967a8a3fbb008242ee4bc5f92a1f Mon Sep 17 00:00:00 2001 From: Brandon Harvey <8107750+bharvey88@users.noreply.github.com> Date: Wed, 4 Mar 2026 19:46:40 -0600 Subject: [PATCH 09/13] [ai] Add docs note about keeping component index pages in sync (#14465) Co-authored-by: Brandon Harvey Co-authored-by: J. Nick Koston --- .ai/instructions.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.ai/instructions.md b/.ai/instructions.md index 3c24177827f..240a47a52fd 100644 --- a/.ai/instructions.md +++ b/.ai/instructions.md @@ -286,6 +286,7 @@ This document provides essential context for AI models interacting with this pro * **Documentation Contributions:** * Documentation is hosted in the separate `esphome/esphome-docs` repository. * The contribution workflow is the same as for the codebase. + * When editing a component's documentation page, also update the corresponding component index page to ensure both pages remain in sync. * **Best Practices:** * **Component Development:** Keep dependencies minimal, provide clear error messages, and write comprehensive docstrings and tests. From 5df4fd0a271251d0dc29e0b76148b50550cfb582 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 4 Mar 2026 15:51:51 -1000 Subject: [PATCH 10/13] [tests] Fix flaky uart_mock integration tests (#14476) --- .../external_components/uart_mock/__init__.py | 5 + .../uart_mock/uart_mock.cpp | 37 +- .../external_components/uart_mock/uart_mock.h | 4 + .../fixtures/uart_mock_ld2410.yaml | 8 + .../uart_mock_ld2410_engineering.yaml | 8 + .../fixtures/uart_mock_ld2412.yaml | 8 + .../uart_mock_ld2412_engineering.yaml | 8 + .../fixtures/uart_mock_modbus.yaml | 8 + .../fixtures/uart_mock_modbus_timing.yaml | 8 + tests/integration/state_utils.py | 96 ++++- tests/integration/test_uart_mock_ld2410.py | 353 ++++++----------- tests/integration/test_uart_mock_ld2412.py | 362 ++++++------------ tests/integration/test_uart_mock_modbus.py | 14 +- 13 files changed, 420 insertions(+), 499 deletions(-) diff --git a/tests/integration/fixtures/external_components/uart_mock/__init__.py b/tests/integration/fixtures/external_components/uart_mock/__init__.py index 8deab4c21ec..abb3abcc419 100644 --- a/tests/integration/fixtures/external_components/uart_mock/__init__.py +++ b/tests/integration/fixtures/external_components/uart_mock/__init__.py @@ -43,6 +43,7 @@ CONF_INJECT_RX = "inject_rx" CONF_EXPECT_TX = "expect_tx" CONF_PERIODIC_RX = "periodic_rx" CONF_ON_TX = "on_tx" +CONF_AUTO_START = "auto_start" UART_PARITY_OPTIONS = { "NONE": uart.UARTParityOptions.UART_CONFIG_PARITY_NONE, @@ -95,6 +96,7 @@ CONFIG_SCHEMA = cv.Schema( cv.Optional(CONF_INJECTIONS, default=[]): cv.ensure_list(INJECTION_SCHEMA), cv.Optional(CONF_RESPONSES, default=[]): cv.ensure_list(RESPONSE_SCHEMA), cv.Optional(CONF_PERIODIC_RX, default=[]): cv.ensure_list(PERIODIC_RX_SCHEMA), + cv.Optional(CONF_AUTO_START, default=True): cv.boolean, cv.Optional(CONF_ON_TX): automation.validate_automation( { cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(MockUartTXTrigger), @@ -138,6 +140,9 @@ async def to_code(config): cg.add(var.set_data_bits(config[CONF_DATA_BITS])) cg.add(var.set_parity(config[CONF_PARITY])) + if not config[CONF_AUTO_START]: + cg.add(var.set_auto_start(False)) + for injection in config[CONF_INJECTIONS]: rx_data = injection[CONF_INJECT_RX] delay_ms = injection[CONF_DELAY] diff --git a/tests/integration/fixtures/external_components/uart_mock/uart_mock.cpp b/tests/integration/fixtures/external_components/uart_mock/uart_mock.cpp index a4a1c41234c..83a13793be7 100644 --- a/tests/integration/fixtures/external_components/uart_mock/uart_mock.cpp +++ b/tests/integration/fixtures/external_components/uart_mock/uart_mock.cpp @@ -16,17 +16,21 @@ void MockUartComponent::setup() { } void MockUartComponent::loop() { - uint32_t now = App.get_loop_component_start_time(); - - // Initialize scenario start time on first loop() call, after all components have - // finished setup(). This prevents injection delays from being consumed during setup. if (!this->loop_started_) { this->loop_started_ = true; - this->scenario_start_ms_ = now; - this->cumulative_delay_ms_ = 0; - ESP_LOGD(TAG, "Scenario started at %u ms", now); + if (this->auto_start_) { + this->start_scenario(); + } else { + ESP_LOGD(TAG, "Scenario waiting for manual start"); + } } + if (!this->scenario_active_) { + return; + } + + uint32_t now = App.get_loop_component_start_time(); + // Process at most ONE timed injection per loop iteration. // This ensures each injection is in a separate loop cycle, giving the consuming // component (e.g., LD2410) a chance to process each batch independently. @@ -50,6 +54,19 @@ void MockUartComponent::loop() { } } +void MockUartComponent::start_scenario() { + uint32_t now = App.get_loop_component_start_time(); + this->scenario_active_ = true; + this->scenario_start_ms_ = now; + this->cumulative_delay_ms_ = 0; + this->injection_index_ = 0; + this->tx_buffer_.clear(); + for (auto &periodic : this->periodic_rx_) { + periodic.last_inject_ms = now; + } + ESP_LOGD(TAG, "Scenario started at %u ms", now); +} + void MockUartComponent::dump_config() { ESP_LOGCONFIG(TAG, "Mock UART Component:\n" @@ -78,10 +95,12 @@ void MockUartComponent::write_array(const uint8_t *data, size_t len) { } #endif - this->try_match_response_(); + if (this->scenario_active_) { + this->try_match_response_(); + } // This directly calls a tx_hook (lambda) as an alternative to the simpler match_response mechanism. - if (this->tx_hook_) { + if (this->tx_hook_ && this->scenario_active_) { std::vector buf(data, data + len); this->tx_hook_(buf); } diff --git a/tests/integration/fixtures/external_components/uart_mock/uart_mock.h b/tests/integration/fixtures/external_components/uart_mock/uart_mock.h index 5bbc3c1bf60..b721512f96c 100644 --- a/tests/integration/fixtures/external_components/uart_mock/uart_mock.h +++ b/tests/integration/fixtures/external_components/uart_mock/uart_mock.h @@ -37,6 +37,8 @@ class MockUartComponent : public uart::UARTComponent, public Component { void add_response(const std::vector &expect_tx, const std::vector &inject_rx); void add_periodic_rx(const std::vector &data, uint32_t interval_ms); + void start_scenario(); + void set_auto_start(bool auto_start) { this->auto_start_ = auto_start; } void set_tx_hook(std::function &)> &&cb) { this->tx_hook_ = std::move(cb); } void inject_to_rx_buffer(const std::vector &data); void inject_to_rx_buffer(const uint8_t *data, size_t len); @@ -55,6 +57,8 @@ class MockUartComponent : public uart::UARTComponent, public Component { uint32_t scenario_start_ms_{0}; uint32_t cumulative_delay_ms_{0}; bool loop_started_{false}; + bool auto_start_{true}; + bool scenario_active_{false}; // TX-triggered responses struct Response { diff --git a/tests/integration/fixtures/uart_mock_ld2410.yaml b/tests/integration/fixtures/uart_mock_ld2410.yaml index 9a814682630..59838b0599c 100644 --- a/tests/integration/fixtures/uart_mock_ld2410.yaml +++ b/tests/integration/fixtures/uart_mock_ld2410.yaml @@ -20,6 +20,7 @@ uart: uart_mock: id: mock_uart baud_rate: 256000 + auto_start: false injections: # Phase 1 (t=100ms): Valid LD2410 normal mode data frame - happy path # The buffer is clean at this point, so this frame should parse correctly. @@ -143,3 +144,10 @@ binary_sensor: name: "Has Moving Target" has_still_target: name: "Has Still Target" + +button: + - platform: template + name: "Start Scenario" + id: start_scenario_btn + on_press: + - lambda: 'id(mock_uart).start_scenario();' diff --git a/tests/integration/fixtures/uart_mock_ld2410_engineering.yaml b/tests/integration/fixtures/uart_mock_ld2410_engineering.yaml index 3b730fc1f8b..4625ae85115 100644 --- a/tests/integration/fixtures/uart_mock_ld2410_engineering.yaml +++ b/tests/integration/fixtures/uart_mock_ld2410_engineering.yaml @@ -19,6 +19,7 @@ uart: uart_mock: id: mock_uart baud_rate: 256000 + auto_start: false injections: # Phase 1 (t=100ms): Valid LD2410 engineering mode data frame # Captured from a real Screek Human Presence Sensor 1U with LD2410 firmware 2.4.x @@ -154,3 +155,10 @@ binary_sensor: name: "Has Still Target" out_pin_presence_status: name: "Out Pin Presence" + +button: + - platform: template + name: "Start Scenario" + id: start_scenario_btn + on_press: + - lambda: 'id(mock_uart).start_scenario();' diff --git a/tests/integration/fixtures/uart_mock_ld2412.yaml b/tests/integration/fixtures/uart_mock_ld2412.yaml index a502f36a253..9cf9d6bb873 100644 --- a/tests/integration/fixtures/uart_mock_ld2412.yaml +++ b/tests/integration/fixtures/uart_mock_ld2412.yaml @@ -20,6 +20,7 @@ uart: uart_mock: id: mock_uart baud_rate: 256000 + auto_start: false injections: # Phase 1 (t=100ms): Valid LD2412 normal mode data frame - happy path # The buffer is clean at this point, so this frame should parse correctly. @@ -169,3 +170,10 @@ binary_sensor: name: "Has Still Target" filters: - settle: 50ms + +button: + - platform: template + name: "Start Scenario" + id: start_scenario_btn + on_press: + - lambda: 'id(mock_uart).start_scenario();' diff --git a/tests/integration/fixtures/uart_mock_ld2412_engineering.yaml b/tests/integration/fixtures/uart_mock_ld2412_engineering.yaml index 3c669fc9a9d..103dbed132f 100644 --- a/tests/integration/fixtures/uart_mock_ld2412_engineering.yaml +++ b/tests/integration/fixtures/uart_mock_ld2412_engineering.yaml @@ -19,6 +19,7 @@ uart: uart_mock: id: mock_uart baud_rate: 256000 + auto_start: false injections: # Phase 1 (t=100ms): Valid LD2412 engineering mode data frame # @@ -211,3 +212,10 @@ binary_sensor: name: "Has Still Target" filters: - settle: 50ms + +button: + - platform: template + name: "Start Scenario" + id: start_scenario_btn + on_press: + - lambda: 'id(mock_uart).start_scenario();' diff --git a/tests/integration/fixtures/uart_mock_modbus.yaml b/tests/integration/fixtures/uart_mock_modbus.yaml index 89b9b91861f..0a3492a0d2f 100644 --- a/tests/integration/fixtures/uart_mock_modbus.yaml +++ b/tests/integration/fixtures/uart_mock_modbus.yaml @@ -22,6 +22,7 @@ uart_mock: baud_rate: 9600 rx_full_threshold: 120 rx_timeout: 2 + auto_start: false debug: responses: - expect_tx: [0x01, 0x03, 0x00, 0x03, 0x00, 0x01, 0x74, 0x0A] # Read holding register 1 on device 1 @@ -38,3 +39,10 @@ sensor: name: "basic_register" address: 0x03 register_type: holding + +button: + - platform: template + name: "Start Scenario" + id: start_scenario_btn + on_press: + - lambda: 'id(virtual_uart_dev).start_scenario();' diff --git a/tests/integration/fixtures/uart_mock_modbus_timing.yaml b/tests/integration/fixtures/uart_mock_modbus_timing.yaml index cd485ca3944..c4e29e5fe8d 100644 --- a/tests/integration/fixtures/uart_mock_modbus_timing.yaml +++ b/tests/integration/fixtures/uart_mock_modbus_timing.yaml @@ -22,6 +22,7 @@ uart_mock: baud_rate: 9600 rx_full_threshold: 120 rx_timeout: 2 + auto_start: false debug: on_tx: - then: @@ -52,3 +53,10 @@ sensor: phase_a: voltage: name: sdm_voltage + +button: + - platform: template + name: "Start Scenario" + id: start_scenario_btn + on_press: + - lambda: 'id(virtual_uart_dev).start_scenario();' diff --git a/tests/integration/state_utils.py b/tests/integration/state_utils.py index b649056f2ba..e8c2cc5e663 100644 --- a/tests/integration/state_utils.py +++ b/tests/integration/state_utils.py @@ -3,10 +3,17 @@ from __future__ import annotations import asyncio +from collections.abc import Callable import logging from typing import TypeVar -from aioesphomeapi import ButtonInfo, EntityInfo, EntityState +from aioesphomeapi import ( + BinarySensorState, + ButtonInfo, + EntityInfo, + EntityState, + SensorState, +) _LOGGER = logging.getLogger(__name__) @@ -234,3 +241,90 @@ class InitialStateHelper: asyncio.TimeoutError: If initial states aren't received within timeout """ await asyncio.wait_for(self._initial_states_received, timeout=timeout) + + +class SensorStateCollector: + """Collects sensor and binary sensor state updates and provides wait helpers. + + Usage: + collector = SensorStateCollector( + sensor_names=["moving_distance", "still_distance"], + binary_sensor_names=["has_target"], + ) + # Use collector.on_state as the callback (or wrap it) + client.subscribe_states(helper.on_state_wrapper(collector.on_state)) + + # Wait for all sensors to have at least one value + await collector.wait_for_all(timeout=3.0) + + # Access collected states + assert collector.sensor_states["moving_distance"][0] == approx(100.0) + """ + + def __init__( + self, + sensor_names: list[str], + binary_sensor_names: list[str] | None = None, + entities: list[EntityInfo] | None = None, + ) -> None: + self.sensor_states: dict[str, list[float]] = {name: [] for name in sensor_names} + self.binary_states: dict[str, list[bool]] = { + name: [] for name in (binary_sensor_names or []) + } + self._key_to_sensor: dict[int, str] = {} + self._waiters: list[tuple[Callable[[], bool], asyncio.Future[bool]]] = [] + + if entities is not None: + self.build_key_mapping(entities) + + def build_key_mapping(self, entities: list[EntityInfo]) -> None: + """Build key-to-name mapping from entities. Sorted by descending length.""" + all_names = list(self.sensor_states.keys()) + list(self.binary_states.keys()) + all_names.sort(key=len, reverse=True) + self._key_to_sensor = build_key_to_entity_mapping(entities, all_names) + + def on_state(self, state: EntityState) -> None: + """Process a state update.""" + if isinstance(state, SensorState) and not state.missing_state: + sensor_name = self._key_to_sensor.get(state.key) + if sensor_name and sensor_name in self.sensor_states: + self.sensor_states[sensor_name].append(state.state) + self._check_waiters() + elif isinstance(state, BinarySensorState): + sensor_name = self._key_to_sensor.get(state.key) + if sensor_name and sensor_name in self.binary_states: + self.binary_states[sensor_name].append(state.state) + self._check_waiters() + + def _check_waiters(self) -> None: + """Check all pending waiters and resolve any whose condition is met.""" + for condition, future in self._waiters: + if not future.done() and condition(): + future.set_result(True) + + def _all_have_values(self) -> bool: + """Check if all sensor and binary sensor lists have at least one value.""" + return all(len(v) >= 1 for v in self.sensor_states.values()) and all( + len(v) >= 1 for v in self.binary_states.values() + ) + + async def wait_for_all(self, timeout: float = 3.0) -> None: + """Wait until all sensors and binary sensors have at least one value.""" + if self._all_have_values(): + return + future: asyncio.Future[bool] = asyncio.get_running_loop().create_future() + self._waiters.append((self._all_have_values, future)) + await asyncio.wait_for(future, timeout=timeout) + + def add_waiter(self, condition: Callable[[], bool]) -> asyncio.Future[bool]: + """Add a custom waiter that resolves when condition returns True. + + Returns: + A future that resolves when the condition is met. + """ + future: asyncio.Future[bool] = asyncio.get_running_loop().create_future() + if condition(): + future.set_result(True) + else: + self._waiters.append((condition, future)) + return future diff --git a/tests/integration/test_uart_mock_ld2410.py b/tests/integration/test_uart_mock_ld2410.py index e01d6ff8e82..ce0e1bb7ec2 100644 --- a/tests/integration/test_uart_mock_ld2410.py +++ b/tests/integration/test_uart_mock_ld2410.py @@ -21,16 +21,10 @@ from __future__ import annotations import asyncio from pathlib import Path -from aioesphomeapi import ( - BinarySensorInfo, - BinarySensorState, - EntityState, - SensorInfo, - SensorState, -) +from aioesphomeapi import ButtonInfo import pytest -from .state_utils import InitialStateHelper, build_key_to_entity_mapping, find_entity +from .state_utils import InitialStateHelper, SensorStateCollector, find_entity from .types import APIClientConnectedFactory, RunCompiledFunction @@ -64,100 +58,65 @@ async def test_uart_mock_ld2410( if "uart_mock" in line and "TX " in line: tx_log_lines.append(line) - # Track sensor state updates (after initial state is swallowed) - sensor_states: dict[str, list[float]] = { - "moving_distance": [], - "still_distance": [], - "moving_energy": [], - "still_energy": [], - "detection_distance": [], - } - binary_states: dict[str, list[bool]] = { - "has_target": [], - "has_moving_target": [], - "has_still_target": [], - } + collector = SensorStateCollector( + sensor_names=[ + "moving_distance", + "still_distance", + "moving_energy", + "still_energy", + "detection_distance", + ], + binary_sensor_names=[ + "has_target", + "has_moving_target", + "has_still_target", + ], + ) # Signal when we see recovery frame values - recovery_received = loop.create_future() - - def on_state(state: EntityState) -> None: - if isinstance(state, SensorState) and not state.missing_state: - sensor_name = key_to_sensor.get(state.key) - if sensor_name and sensor_name in sensor_states: - sensor_states[sensor_name].append(state.state) - # Check if this is the recovery frame (moving_distance = 50) - if ( - sensor_name == "moving_distance" - and state.state == pytest.approx(50.0) - and not recovery_received.done() - ): - recovery_received.set_result(True) - elif isinstance(state, BinarySensorState): - sensor_name = key_to_sensor.get(state.key) - if sensor_name and sensor_name in binary_states: - binary_states[sensor_name].append(state.state) + recovery_received = collector.add_waiter( + lambda: pytest.approx(50.0) in collector.sensor_states["moving_distance"] + ) async with ( run_compiled(yaml_config, line_callback=line_callback), api_client_connected() as client, ): entities, _ = await client.list_entities_services() - - # Build key mappings for all sensor types - all_names = list(sensor_states.keys()) + list(binary_states.keys()) - key_to_sensor = build_key_to_entity_mapping(entities, all_names) + collector.build_key_mapping(entities) # Set up initial state helper initial_state_helper = InitialStateHelper(entities) - client.subscribe_states(initial_state_helper.on_state_wrapper(on_state)) + client.subscribe_states( + initial_state_helper.on_state_wrapper(collector.on_state) + ) try: await initial_state_helper.wait_for_initial_states() except TimeoutError: pytest.fail("Timeout waiting for initial states") - # Phase 1 values are in the initial states (swallowed by InitialStateHelper). - # Verify them via initial_states dict. - moving_dist_entity = find_entity(entities, "moving_distance", SensorInfo) - assert moving_dist_entity is not None - initial_moving = initial_state_helper.initial_states.get(moving_dist_entity.key) - assert initial_moving is not None and isinstance(initial_moving, SensorState) - assert initial_moving.state == pytest.approx(100.0), ( - f"Initial moving distance should be 100, got {initial_moving.state}" - ) + # Start the UART mock scenario now that we're subscribed + start_btn = find_entity(entities, "start_scenario", ButtonInfo) + assert start_btn is not None, "Start Scenario button not found" + client.button_command(start_btn.key) - still_dist_entity = find_entity(entities, "still_distance", SensorInfo) - assert still_dist_entity is not None - initial_still = initial_state_helper.initial_states.get(still_dist_entity.key) - assert initial_still is not None and isinstance(initial_still, SensorState) - assert initial_still.state == pytest.approx(120.0), ( - f"Initial still distance should be 120, got {initial_still.state}" - ) + # Wait for Phase 1 - all sensors and binary sensors have at least one value + try: + await collector.wait_for_all(timeout=3.0) + except TimeoutError: + pytest.fail( + f"Timeout waiting for Phase 1 frame. Received:\n" + f" sensor_states: {collector.sensor_states}\n" + f" binary_states: {collector.binary_states}" + ) - moving_energy_entity = find_entity(entities, "moving_energy", SensorInfo) - assert moving_energy_entity is not None - initial_me = initial_state_helper.initial_states.get(moving_energy_entity.key) - assert initial_me is not None and isinstance(initial_me, SensorState) - assert initial_me.state == pytest.approx(50.0), ( - f"Initial moving energy should be 50, got {initial_me.state}" - ) - - still_energy_entity = find_entity(entities, "still_energy", SensorInfo) - assert still_energy_entity is not None - initial_se = initial_state_helper.initial_states.get(still_energy_entity.key) - assert initial_se is not None and isinstance(initial_se, SensorState) - assert initial_se.state == pytest.approx(25.0), ( - f"Initial still energy should be 25, got {initial_se.state}" - ) - - detect_dist_entity = find_entity(entities, "detection_distance", SensorInfo) - assert detect_dist_entity is not None - initial_dd = initial_state_helper.initial_states.get(detect_dist_entity.key) - assert initial_dd is not None and isinstance(initial_dd, SensorState) - assert initial_dd.state == pytest.approx(300.0), ( - f"Initial detection distance should be 300, got {initial_dd.state}" - ) + # Phase 1 values: moving=100, still=120, energy=50/25, detect=300 + assert collector.sensor_states["moving_distance"][0] == pytest.approx(100.0) + assert collector.sensor_states["still_distance"][0] == pytest.approx(120.0) + assert collector.sensor_states["moving_energy"][0] == pytest.approx(50.0) + assert collector.sensor_states["still_energy"][0] == pytest.approx(25.0) + assert collector.sensor_states["detection_distance"][0] == pytest.approx(300.0) # Wait for the recovery frame (Phase 5) to be parsed # This proves the component survived garbage + truncated + overflow @@ -165,12 +124,8 @@ async def test_uart_mock_ld2410( await asyncio.wait_for(recovery_received, timeout=15.0) except TimeoutError: pytest.fail( - f"Timeout waiting for recovery frame. Received sensor states:\n" - f" moving_distance: {sensor_states['moving_distance']}\n" - f" still_distance: {sensor_states['still_distance']}\n" - f" moving_energy: {sensor_states['moving_energy']}\n" - f" still_energy: {sensor_states['still_energy']}\n" - f" detection_distance: {sensor_states['detection_distance']}" + f"Timeout waiting for recovery frame. Received:\n" + f" sensor_states: {collector.sensor_states}" ) # Verify overflow warning was logged @@ -183,67 +138,36 @@ async def test_uart_mock_ld2410( # A5 (MAC), AB (distance res), AE (light), 61 (params), FE (config off) assert len(tx_log_lines) > 0, "Expected TX log lines from uart_mock" tx_data = " ".join(tx_log_lines) - # Verify command frame header appears (FD:FC:FB:FA) assert "FD:FC:FB:FA" in tx_data, ( "Expected LD2410 command frame header FD:FC:FB:FA in TX log" ) - # Verify command frame footer appears (04:03:02:01) assert "04:03:02:01" in tx_data, ( "Expected LD2410 command frame footer 04:03:02:01 in TX log" ) - # Recovery frame values (Phase 5, after overflow) - assert len(sensor_states["moving_distance"]) >= 1, ( - f"Expected recovery moving_distance, got: {sensor_states['moving_distance']}" - ) - # Find the recovery value (moving_distance = 50) - recovery_values = [ - v for v in sensor_states["moving_distance"] if v == pytest.approx(50.0) - ] - assert len(recovery_values) >= 1, ( - f"Expected moving_distance=50 in recovery, got: {sensor_states['moving_distance']}" - ) - # Recovery frame: moving=50, still=75, energy=100/80, detect=127 recovery_idx = next( i - for i, v in enumerate(sensor_states["moving_distance"]) + for i, v in enumerate(collector.sensor_states["moving_distance"]) if v == pytest.approx(50.0) ) - assert sensor_states["still_distance"][recovery_idx] == pytest.approx(75.0), ( - f"Recovery still distance should be 75, got {sensor_states['still_distance'][recovery_idx]}" + assert collector.sensor_states["still_distance"][recovery_idx] == pytest.approx( + 75.0 ) - assert sensor_states["moving_energy"][recovery_idx] == pytest.approx(100.0), ( - f"Recovery moving energy should be 100, got {sensor_states['moving_energy'][recovery_idx]}" + assert collector.sensor_states["moving_energy"][recovery_idx] == pytest.approx( + 100.0 ) - assert sensor_states["still_energy"][recovery_idx] == pytest.approx(80.0), ( - f"Recovery still energy should be 80, got {sensor_states['still_energy'][recovery_idx]}" - ) - assert sensor_states["detection_distance"][recovery_idx] == pytest.approx( - 127.0 - ), ( - f"Recovery detection distance should be 127, got {sensor_states['detection_distance'][recovery_idx]}" + assert collector.sensor_states["still_energy"][recovery_idx] == pytest.approx( + 80.0 ) + assert collector.sensor_states["detection_distance"][ + recovery_idx + ] == pytest.approx(127.0) - # Verify binary sensors detected targets - # Binary sensors could be in initial states or forwarded states - has_target_entity = find_entity(entities, "has_target", BinarySensorInfo) - assert has_target_entity is not None - initial_ht = initial_state_helper.initial_states.get(has_target_entity.key) - assert initial_ht is not None and isinstance(initial_ht, BinarySensorState) - assert initial_ht.state is True, "Has target should be True" - - has_moving_entity = find_entity(entities, "has_moving_target", BinarySensorInfo) - assert has_moving_entity is not None - initial_hm = initial_state_helper.initial_states.get(has_moving_entity.key) - assert initial_hm is not None and isinstance(initial_hm, BinarySensorState) - assert initial_hm.state is True, "Has moving target should be True" - - has_still_entity = find_entity(entities, "has_still_target", BinarySensorInfo) - assert has_still_entity is not None - initial_hs = initial_state_helper.initial_states.get(has_still_entity.key) - assert initial_hs is not None and isinstance(initial_hs, BinarySensorState) - assert initial_hs.state is True, "Has still target should be True" + # Verify binary sensors detected targets (from Phase 1 frame) + assert collector.binary_states["has_target"][0] is True + assert collector.binary_states["has_moving_target"][0] is True + assert collector.binary_states["has_still_target"][0] is True @pytest.mark.asyncio @@ -260,133 +184,82 @@ async def test_uart_mock_ld2410_engineering( "EXTERNAL_COMPONENT_PATH", external_components_path ) - loop = asyncio.get_running_loop() + collector = SensorStateCollector( + sensor_names=[ + "moving_distance", + "still_distance", + "moving_energy", + "still_energy", + "detection_distance", + "light", + "gate_0_move_energy", + "gate_1_move_energy", + "gate_2_move_energy", + "gate_0_still_energy", + "gate_1_still_energy", + "gate_2_still_energy", + ], + binary_sensor_names=[ + "has_target", + "has_moving_target", + "has_still_target", + "out_pin_presence", + ], + ) - # Track sensor state updates (after initial state is swallowed) - sensor_states: dict[str, list[float]] = { - "moving_distance": [], - "still_distance": [], - "moving_energy": [], - "still_energy": [], - "detection_distance": [], - "light": [], - "gate_0_move_energy": [], - "gate_1_move_energy": [], - "gate_2_move_energy": [], - "gate_0_still_energy": [], - "gate_1_still_energy": [], - "gate_2_still_energy": [], - } - binary_states: dict[str, list[bool]] = { - "has_target": [], - "has_moving_target": [], - "has_still_target": [], - "out_pin_presence": [], - } - - # Signal when we see Phase 3 frame (still_distance = 291) - phase3_received = loop.create_future() - - def on_state(state: EntityState) -> None: - if isinstance(state, SensorState) and not state.missing_state: - sensor_name = key_to_sensor.get(state.key) - if sensor_name and sensor_name in sensor_states: - sensor_states[sensor_name].append(state.state) - if ( - sensor_name == "still_distance" - and state.state == pytest.approx(291.0) - and not phase3_received.done() - ): - phase3_received.set_result(True) - elif isinstance(state, BinarySensorState): - sensor_name = key_to_sensor.get(state.key) - if sensor_name and sensor_name in binary_states: - binary_states[sensor_name].append(state.state) + # Signal when we see Phase 3 frame values + phase3_received = collector.add_waiter( + lambda: pytest.approx(291.0) in collector.sensor_states["still_distance"] + ) async with ( run_compiled(yaml_config), api_client_connected() as client, ): entities, _ = await client.list_entities_services() - - all_names = list(sensor_states.keys()) + list(binary_states.keys()) - key_to_sensor = build_key_to_entity_mapping(entities, all_names) + collector.build_key_mapping(entities) initial_state_helper = InitialStateHelper(entities) - client.subscribe_states(initial_state_helper.on_state_wrapper(on_state)) + client.subscribe_states( + initial_state_helper.on_state_wrapper(collector.on_state) + ) try: await initial_state_helper.wait_for_initial_states() except TimeoutError: pytest.fail("Timeout waiting for initial states") - # Phase 1 initial values (engineering mode frame): + # Start the UART mock scenario now that we're subscribed + start_btn = find_entity(entities, "start_scenario", ButtonInfo) + assert start_btn is not None, "Start Scenario button not found" + client.button_command(start_btn.key) + + # Wait for Phase 1 - all sensors and binary sensors have at least one value + try: + await collector.wait_for_all(timeout=3.0) + except TimeoutError: + pytest.fail( + f"Timeout waiting for Phase 1 frame. Received:\n" + f" sensor_states: {collector.sensor_states}\n" + f" binary_states: {collector.binary_states}" + ) + + # Phase 1 values (engineering mode frame): # moving=30, energy=100, still=30, energy=100, detect=0 - moving_dist_entity = find_entity(entities, "moving_distance", SensorInfo) - assert moving_dist_entity is not None - initial_moving = initial_state_helper.initial_states.get(moving_dist_entity.key) - assert initial_moving is not None and isinstance(initial_moving, SensorState) - assert initial_moving.state == pytest.approx(30.0), ( - f"Initial moving distance should be 30, got {initial_moving.state}" - ) - - still_dist_entity = find_entity(entities, "still_distance", SensorInfo) - assert still_dist_entity is not None - initial_still = initial_state_helper.initial_states.get(still_dist_entity.key) - assert initial_still is not None and isinstance(initial_still, SensorState) - assert initial_still.state == pytest.approx(30.0), ( - f"Initial still distance should be 30, got {initial_still.state}" - ) - - # Verify engineering mode sensors from initial state - # Gate 0 moving energy = 0x64 = 100 - gate0_move_entity = find_entity(entities, "gate_0_move_energy", SensorInfo) - assert gate0_move_entity is not None - initial_g0m = initial_state_helper.initial_states.get(gate0_move_entity.key) - assert initial_g0m is not None and isinstance(initial_g0m, SensorState) - assert initial_g0m.state == pytest.approx(100.0), ( - f"Gate 0 move energy should be 100, got {initial_g0m.state}" - ) - - # Gate 1 moving energy = 0x41 = 65 - gate1_move_entity = find_entity(entities, "gate_1_move_energy", SensorInfo) - assert gate1_move_entity is not None - initial_g1m = initial_state_helper.initial_states.get(gate1_move_entity.key) - assert initial_g1m is not None and isinstance(initial_g1m, SensorState) - assert initial_g1m.state == pytest.approx(65.0), ( - f"Gate 1 move energy should be 65, got {initial_g1m.state}" - ) - - # Light sensor = 0x57 = 87 - light_entity = find_entity(entities, "light", SensorInfo) - assert light_entity is not None - initial_light = initial_state_helper.initial_states.get(light_entity.key) - assert initial_light is not None and isinstance(initial_light, SensorState) - assert initial_light.state == pytest.approx(87.0), ( - f"Light sensor should be 87, got {initial_light.state}" - ) - - # Out pin presence = 0x01 = True - out_pin_entity = find_entity(entities, "out_pin_presence", BinarySensorInfo) - assert out_pin_entity is not None - initial_out = initial_state_helper.initial_states.get(out_pin_entity.key) - assert initial_out is not None and isinstance(initial_out, BinarySensorState) - assert initial_out.state is True, "Out pin presence should be True" + assert collector.sensor_states["moving_distance"][0] == pytest.approx(30.0) + assert collector.sensor_states["still_distance"][0] == pytest.approx(30.0) + assert collector.sensor_states["gate_0_move_energy"][0] == pytest.approx(100.0) + assert collector.sensor_states["gate_1_move_energy"][0] == pytest.approx(65.0) + assert collector.sensor_states["light"][0] == pytest.approx(87.0) + assert collector.binary_states["out_pin_presence"][0] is True # Wait for Phase 3 frame (still_distance = 291cm, multi-byte) try: await asyncio.wait_for(phase3_received, timeout=15.0) except TimeoutError: pytest.fail( - f"Timeout waiting for Phase 3 frame. Received sensor states:\n" - f" still_distance: {sensor_states['still_distance']}\n" - f" moving_distance: {sensor_states['moving_distance']}" + f"Timeout waiting for Phase 3 frame. Received:\n" + f" still_distance: {collector.sensor_states['still_distance']}" ) - # Phase 3: still distance = 0x0123 = 291cm (multi-byte distance test) - phase3_still = [ - v for v in sensor_states["still_distance"] if v == pytest.approx(291.0) - ] - assert len(phase3_still) >= 1, ( - f"Expected still_distance=291, got: {sensor_states['still_distance']}" - ) + assert pytest.approx(291.0) in collector.sensor_states["still_distance"] diff --git a/tests/integration/test_uart_mock_ld2412.py b/tests/integration/test_uart_mock_ld2412.py index cf7324ceed3..a964ba00738 100644 --- a/tests/integration/test_uart_mock_ld2412.py +++ b/tests/integration/test_uart_mock_ld2412.py @@ -21,16 +21,10 @@ from __future__ import annotations import asyncio from pathlib import Path -from aioesphomeapi import ( - BinarySensorInfo, - BinarySensorState, - EntityState, - SensorInfo, - SensorState, -) +from aioesphomeapi import ButtonInfo import pytest -from .state_utils import InitialStateHelper, build_key_to_entity_mapping, find_entity +from .state_utils import InitialStateHelper, SensorStateCollector, find_entity from .types import APIClientConnectedFactory, RunCompiledFunction @@ -64,104 +58,65 @@ async def test_uart_mock_ld2412( if "uart_mock" in line and "TX " in line: tx_log_lines.append(line) - # Track sensor state updates (after initial state is swallowed) - sensor_states: dict[str, list[float]] = { - "moving_distance": [], - "still_distance": [], - "moving_energy": [], - "still_energy": [], - "detection_distance": [], - } - binary_states: dict[str, list[bool]] = { - "has_target": [], - "has_moving_target": [], - "has_still_target": [], - } + collector = SensorStateCollector( + sensor_names=[ + "moving_distance", + "still_distance", + "moving_energy", + "still_energy", + "detection_distance", + ], + binary_sensor_names=[ + "has_target", + "has_moving_target", + "has_still_target", + ], + ) # Signal when we see recovery frame values - recovery_received = loop.create_future() - - def on_state(state: EntityState) -> None: - if isinstance(state, SensorState) and not state.missing_state: - sensor_name = key_to_sensor.get(state.key) - if sensor_name and sensor_name in sensor_states: - sensor_states[sensor_name].append(state.state) - # Check if this is the recovery frame (moving_distance = 50) - if ( - sensor_name == "moving_distance" - and state.state == pytest.approx(50.0) - and not recovery_received.done() - ): - recovery_received.set_result(True) - elif isinstance(state, BinarySensorState): - sensor_name = key_to_sensor.get(state.key) - if sensor_name and sensor_name in binary_states: - binary_states[sensor_name].append(state.state) + recovery_received = collector.add_waiter( + lambda: pytest.approx(50.0) in collector.sensor_states["moving_distance"] + ) async with ( run_compiled(yaml_config, line_callback=line_callback), api_client_connected() as client, ): entities, _ = await client.list_entities_services() - - # Build key mappings for all sensor types - all_names = list(sensor_states.keys()) + list(binary_states.keys()) - # Sort by descending length to avoid substring collisions - # (e.g., "still_energy" matching "gate_0_still_energy") - all_names.sort(key=len, reverse=True) - key_to_sensor = build_key_to_entity_mapping(entities, all_names) + collector.build_key_mapping(entities) # Set up initial state helper initial_state_helper = InitialStateHelper(entities) - client.subscribe_states(initial_state_helper.on_state_wrapper(on_state)) + client.subscribe_states( + initial_state_helper.on_state_wrapper(collector.on_state) + ) try: await initial_state_helper.wait_for_initial_states() except TimeoutError: pytest.fail("Timeout waiting for initial states") - # Phase 1 values are in the initial states (swallowed by InitialStateHelper). - # Verify them via initial_states dict. - moving_dist_entity = find_entity(entities, "moving_distance", SensorInfo) - assert moving_dist_entity is not None - initial_moving = initial_state_helper.initial_states.get(moving_dist_entity.key) - assert initial_moving is not None and isinstance(initial_moving, SensorState) - assert initial_moving.state == pytest.approx(100.0), ( - f"Initial moving distance should be 100, got {initial_moving.state}" - ) + # Start the UART mock scenario now that we're subscribed + start_btn = find_entity(entities, "start_scenario", ButtonInfo) + assert start_btn is not None, "Start Scenario button not found" + client.button_command(start_btn.key) - still_dist_entity = find_entity(entities, "still_distance", SensorInfo) - assert still_dist_entity is not None - initial_still = initial_state_helper.initial_states.get(still_dist_entity.key) - assert initial_still is not None and isinstance(initial_still, SensorState) - assert initial_still.state == pytest.approx(120.0), ( - f"Initial still distance should be 120, got {initial_still.state}" - ) + # Wait for Phase 1 - all sensors and binary sensors have at least one value + try: + await collector.wait_for_all(timeout=3.0) + except TimeoutError: + pytest.fail( + f"Timeout waiting for Phase 1 frame. Received:\n" + f" sensor_states: {collector.sensor_states}\n" + f" binary_states: {collector.binary_states}" + ) - moving_energy_entity = find_entity(entities, "moving_energy", SensorInfo) - assert moving_energy_entity is not None - initial_me = initial_state_helper.initial_states.get(moving_energy_entity.key) - assert initial_me is not None and isinstance(initial_me, SensorState) - assert initial_me.state == pytest.approx(50.0), ( - f"Initial moving energy should be 50, got {initial_me.state}" - ) - - still_energy_entity = find_entity(entities, "still_energy", SensorInfo) - assert still_energy_entity is not None - initial_se = initial_state_helper.initial_states.get(still_energy_entity.key) - assert initial_se is not None and isinstance(initial_se, SensorState) - assert initial_se.state == pytest.approx(25.0), ( - f"Initial still energy should be 25, got {initial_se.state}" - ) - - # LD2412 detection_distance = moving_distance when MOVE_BITMASK is set - detect_dist_entity = find_entity(entities, "detection_distance", SensorInfo) - assert detect_dist_entity is not None - initial_dd = initial_state_helper.initial_states.get(detect_dist_entity.key) - assert initial_dd is not None and isinstance(initial_dd, SensorState) - assert initial_dd.state == pytest.approx(100.0), ( - f"Initial detection distance should be 100, got {initial_dd.state}" - ) + # Phase 1 values: moving=100, still=120, energy=50/25, detect=100 + assert collector.sensor_states["moving_distance"][0] == pytest.approx(100.0) + assert collector.sensor_states["still_distance"][0] == pytest.approx(120.0) + assert collector.sensor_states["moving_energy"][0] == pytest.approx(50.0) + assert collector.sensor_states["still_energy"][0] == pytest.approx(25.0) + assert collector.sensor_states["detection_distance"][0] == pytest.approx(100.0) # Wait for the recovery frame (Phase 5) to be parsed # This proves the component survived garbage + truncated + overflow @@ -169,12 +124,8 @@ async def test_uart_mock_ld2412( await asyncio.wait_for(recovery_received, timeout=3.0) except TimeoutError: pytest.fail( - f"Timeout waiting for recovery frame. Received sensor states:\n" - f" moving_distance: {sensor_states['moving_distance']}\n" - f" still_distance: {sensor_states['still_distance']}\n" - f" moving_energy: {sensor_states['moving_energy']}\n" - f" still_energy: {sensor_states['still_energy']}\n" - f" detection_distance: {sensor_states['detection_distance']}" + f"Timeout waiting for recovery frame. Received:\n" + f" sensor_states: {collector.sensor_states}" ) # Verify overflow warning was logged @@ -185,67 +136,36 @@ async def test_uart_mock_ld2412( # Verify LD2412 sent setup commands (TX logging) assert len(tx_log_lines) > 0, "Expected TX log lines from uart_mock" tx_data = " ".join(tx_log_lines) - # Verify command frame header appears (FD:FC:FB:FA) assert "FD:FC:FB:FA" in tx_data, ( "Expected LD2412 command frame header FD:FC:FB:FA in TX log" ) - # Verify command frame footer appears (04:03:02:01) assert "04:03:02:01" in tx_data, ( "Expected LD2412 command frame footer 04:03:02:01 in TX log" ) - # Recovery frame values (Phase 5, after overflow) - assert len(sensor_states["moving_distance"]) >= 1, ( - f"Expected recovery moving_distance, got: {sensor_states['moving_distance']}" - ) - # Find the recovery value (moving_distance = 50) - recovery_values = [ - v for v in sensor_states["moving_distance"] if v == pytest.approx(50.0) - ] - assert len(recovery_values) >= 1, ( - f"Expected moving_distance=50 in recovery, got: {sensor_states['moving_distance']}" - ) - # Recovery frame: moving=50, still=75, energy=100/80, detect=50 recovery_idx = next( i - for i, v in enumerate(sensor_states["moving_distance"]) + for i, v in enumerate(collector.sensor_states["moving_distance"]) if v == pytest.approx(50.0) ) - assert sensor_states["still_distance"][recovery_idx] == pytest.approx(75.0), ( - f"Recovery still distance should be 75, got {sensor_states['still_distance'][recovery_idx]}" + assert collector.sensor_states["still_distance"][recovery_idx] == pytest.approx( + 75.0 ) - assert sensor_states["moving_energy"][recovery_idx] == pytest.approx(100.0), ( - f"Recovery moving energy should be 100, got {sensor_states['moving_energy'][recovery_idx]}" + assert collector.sensor_states["moving_energy"][recovery_idx] == pytest.approx( + 100.0 ) - assert sensor_states["still_energy"][recovery_idx] == pytest.approx(80.0), ( - f"Recovery still energy should be 80, got {sensor_states['still_energy'][recovery_idx]}" - ) - # LD2412 detection_distance = moving_distance when MOVE_BITMASK set - assert sensor_states["detection_distance"][recovery_idx] == pytest.approx( - 50.0 - ), ( - f"Recovery detection distance should be 50, got {sensor_states['detection_distance'][recovery_idx]}" + assert collector.sensor_states["still_energy"][recovery_idx] == pytest.approx( + 80.0 ) + assert collector.sensor_states["detection_distance"][ + recovery_idx + ] == pytest.approx(50.0) - # Verify binary sensors detected targets - has_target_entity = find_entity(entities, "has_target", BinarySensorInfo) - assert has_target_entity is not None - initial_ht = initial_state_helper.initial_states.get(has_target_entity.key) - assert initial_ht is not None and isinstance(initial_ht, BinarySensorState) - assert initial_ht.state is True, "Has target should be True" - - has_moving_entity = find_entity(entities, "has_moving_target", BinarySensorInfo) - assert has_moving_entity is not None - initial_hm = initial_state_helper.initial_states.get(has_moving_entity.key) - assert initial_hm is not None and isinstance(initial_hm, BinarySensorState) - assert initial_hm.state is True, "Has moving target should be True" - - has_still_entity = find_entity(entities, "has_still_target", BinarySensorInfo) - assert has_still_entity is not None - initial_hs = initial_state_helper.initial_states.get(has_still_entity.key) - assert initial_hs is not None and isinstance(initial_hs, BinarySensorState) - assert initial_hs.state is True, "Has still target should be True" + # Verify binary sensors detected targets (from Phase 1 frame) + assert collector.binary_states["has_target"][0] is True + assert collector.binary_states["has_moving_target"][0] is True + assert collector.binary_states["has_still_target"][0] is True @pytest.mark.asyncio @@ -262,120 +182,75 @@ async def test_uart_mock_ld2412_engineering( "EXTERNAL_COMPONENT_PATH", external_components_path ) - loop = asyncio.get_running_loop() - - # Track sensor state updates (after initial state is swallowed) - sensor_states: dict[str, list[float]] = { - "moving_distance": [], - "still_distance": [], - "moving_energy": [], - "still_energy": [], - "detection_distance": [], - "light": [], - "gate_0_move_energy": [], - "gate_1_move_energy": [], - "gate_2_move_energy": [], - "gate_0_still_energy": [], - "gate_1_still_energy": [], - "gate_2_still_energy": [], - } - binary_states: dict[str, list[bool]] = { - "has_target": [], - "has_moving_target": [], - "has_still_target": [], - } + collector = SensorStateCollector( + sensor_names=[ + "moving_distance", + "still_distance", + "moving_energy", + "still_energy", + "detection_distance", + "light", + "gate_0_move_energy", + "gate_1_move_energy", + "gate_2_move_energy", + "gate_0_still_energy", + "gate_1_still_energy", + "gate_2_still_energy", + ], + binary_sensor_names=[ + "has_target", + "has_moving_target", + "has_still_target", + ], + ) # Signal when we see Phase 3 frame values - phase3_still_received = loop.create_future() - phase3_detect_received = loop.create_future() - - def on_state(state: EntityState) -> None: - if isinstance(state, SensorState) and not state.missing_state: - sensor_name = key_to_sensor.get(state.key) - if sensor_name and sensor_name in sensor_states: - sensor_states[sensor_name].append(state.state) - if ( - sensor_name == "still_distance" - and state.state == pytest.approx(291.0) - and not phase3_still_received.done() - ): - phase3_still_received.set_result(True) - if ( - sensor_name == "detection_distance" - and state.state == pytest.approx(291.0) - and not phase3_detect_received.done() - ): - phase3_detect_received.set_result(True) - elif isinstance(state, BinarySensorState): - sensor_name = key_to_sensor.get(state.key) - if sensor_name and sensor_name in binary_states: - binary_states[sensor_name].append(state.state) + phase3_still_received = collector.add_waiter( + lambda: pytest.approx(291.0) in collector.sensor_states["still_distance"] + ) + phase3_detect_received = collector.add_waiter( + lambda: pytest.approx(291.0) in collector.sensor_states["detection_distance"] + ) async with ( run_compiled(yaml_config), api_client_connected() as client, ): entities, _ = await client.list_entities_services() - - all_names = list(sensor_states.keys()) + list(binary_states.keys()) - # Sort by descending length to avoid substring collisions - # (e.g., "still_energy" matching "gate_0_still_energy") - all_names.sort(key=len, reverse=True) - key_to_sensor = build_key_to_entity_mapping(entities, all_names) + collector.build_key_mapping(entities) initial_state_helper = InitialStateHelper(entities) - client.subscribe_states(initial_state_helper.on_state_wrapper(on_state)) + client.subscribe_states( + initial_state_helper.on_state_wrapper(collector.on_state) + ) try: await initial_state_helper.wait_for_initial_states() except TimeoutError: pytest.fail("Timeout waiting for initial states") - # Phase 1 initial values (engineering mode frame): + # Start the UART mock scenario now that we're subscribed + start_btn = find_entity(entities, "start_scenario", ButtonInfo) + assert start_btn is not None, "Start Scenario button not found" + client.button_command(start_btn.key) + + # Wait for Phase 1 - all sensors and binary sensors have at least one value + try: + await collector.wait_for_all(timeout=3.0) + except TimeoutError: + pytest.fail( + f"Timeout waiting for Phase 1 frame. Received:\n" + f" sensor_states: {collector.sensor_states}\n" + f" binary_states: {collector.binary_states}" + ) + + # Phase 1 values (engineering mode frame): # moving=30, energy=100, still=30, energy=100, detect=30 - moving_dist_entity = find_entity(entities, "moving_distance", SensorInfo) - assert moving_dist_entity is not None - initial_moving = initial_state_helper.initial_states.get(moving_dist_entity.key) - assert initial_moving is not None and isinstance(initial_moving, SensorState) - assert initial_moving.state == pytest.approx(30.0), ( - f"Initial moving distance should be 30, got {initial_moving.state}" - ) - - still_dist_entity = find_entity(entities, "still_distance", SensorInfo) - assert still_dist_entity is not None - initial_still = initial_state_helper.initial_states.get(still_dist_entity.key) - assert initial_still is not None and isinstance(initial_still, SensorState) - assert initial_still.state == pytest.approx(30.0), ( - f"Initial still distance should be 30, got {initial_still.state}" - ) - - # Verify engineering mode sensors from initial state - # Gate 0 moving energy = 0x64 = 100 - gate0_move_entity = find_entity(entities, "gate_0_move_energy", SensorInfo) - assert gate0_move_entity is not None - initial_g0m = initial_state_helper.initial_states.get(gate0_move_entity.key) - assert initial_g0m is not None and isinstance(initial_g0m, SensorState) - assert initial_g0m.state == pytest.approx(100.0), ( - f"Gate 0 move energy should be 100, got {initial_g0m.state}" - ) - - # Gate 1 moving energy = 0x41 = 65 - gate1_move_entity = find_entity(entities, "gate_1_move_energy", SensorInfo) - assert gate1_move_entity is not None - initial_g1m = initial_state_helper.initial_states.get(gate1_move_entity.key) - assert initial_g1m is not None and isinstance(initial_g1m, SensorState) - assert initial_g1m.state == pytest.approx(65.0), ( - f"Gate 1 move energy should be 65, got {initial_g1m.state}" - ) - - # Light sensor = 0x57 = 87 - light_entity = find_entity(entities, "light", SensorInfo) - assert light_entity is not None - initial_light = initial_state_helper.initial_states.get(light_entity.key) - assert initial_light is not None and isinstance(initial_light, SensorState) - assert initial_light.state == pytest.approx(87.0), ( - f"Light sensor should be 87, got {initial_light.state}" - ) + assert collector.sensor_states["moving_distance"][0] == pytest.approx(30.0) + assert collector.sensor_states["still_distance"][0] == pytest.approx(30.0) + assert collector.sensor_states["gate_0_move_energy"][0] == pytest.approx(100.0) + assert collector.sensor_states["gate_1_move_energy"][0] == pytest.approx(65.0) + assert collector.sensor_states["light"][0] == pytest.approx(87.0) # Wait for Phase 3 frame: still_distance = 291cm (multi-byte) try: @@ -383,25 +258,18 @@ async def test_uart_mock_ld2412_engineering( except TimeoutError: pytest.fail( f"Timeout waiting for Phase 3 still_distance. Received:\n" - f" still_distance: {sensor_states['still_distance']}\n" - f" moving_distance: {sensor_states['moving_distance']}" + f" still_distance: {collector.sensor_states['still_distance']}" ) - assert pytest.approx(291.0) in sensor_states["still_distance"], ( - f"Expected still_distance=291, got: {sensor_states['still_distance']}" - ) + assert pytest.approx(291.0) in collector.sensor_states["still_distance"] # Wait for Phase 3: detection_distance = 291 (still-only target) - # target_state=0x02 so LD2412 uses still_distance for detection_distance. - # The throttle_with_priority filter may delay this value. try: await asyncio.wait_for(phase3_detect_received, timeout=3.0) except TimeoutError: pytest.fail( - f"Timeout waiting for detection_distance=291 (still-only target). " - f"Received: {sensor_states['detection_distance']}" + f"Timeout waiting for detection_distance=291. " + f"Received: {collector.sensor_states['detection_distance']}" ) - assert pytest.approx(291.0) in sensor_states["detection_distance"], ( - f"Expected detection_distance=291, got: {sensor_states['detection_distance']}" - ) + assert pytest.approx(291.0) in collector.sensor_states["detection_distance"] diff --git a/tests/integration/test_uart_mock_modbus.py b/tests/integration/test_uart_mock_modbus.py index 309cb56dc99..bf3c0697502 100644 --- a/tests/integration/test_uart_mock_modbus.py +++ b/tests/integration/test_uart_mock_modbus.py @@ -12,10 +12,10 @@ from __future__ import annotations import asyncio from pathlib import Path -from aioesphomeapi import EntityState, SensorState +from aioesphomeapi import ButtonInfo, EntityState, SensorState import pytest -from .state_utils import InitialStateHelper, build_key_to_entity_mapping +from .state_utils import InitialStateHelper, build_key_to_entity_mapping, find_entity from .types import APIClientConnectedFactory, RunCompiledFunction @@ -74,6 +74,11 @@ async def test_uart_mock_modbus( except TimeoutError: pytest.fail("Timeout waiting for initial states") + # Start the UART mock scenario now that we're subscribed + start_btn = find_entity(entities, "start_scenario", ButtonInfo) + assert start_btn is not None, "Start Scenario button not found" + client.button_command(start_btn.key) + # Wait for basic register to be updated with successful parse try: await asyncio.wait_for(basic_register_changed, timeout=15.0) @@ -143,6 +148,11 @@ async def test_uart_mock_modbus_timing( except TimeoutError: pytest.fail("Timeout waiting for initial states") + # Start the UART mock scenario now that we're subscribed + start_btn = find_entity(entities, "start_scenario", ButtonInfo) + assert start_btn is not None, "Start Scenario button not found" + client.button_command(start_btn.key) + # Wait for voltage to be updated with successful parse try: await asyncio.wait_for(voltage_changed, timeout=15.0) From 0ff5270632f59a8fd4d7ed25cd71c2ce92ae5b07 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 4 Mar 2026 16:57:19 -1000 Subject: [PATCH 11/13] [ci] Fix codeowner approval label workflow for fork PRs (#14490) Co-authored-by: Claude Opus 4.6 --- .github/scripts/codeowners.js | 88 +++++++++- .../codeowner-approved-label-update.yml | 95 +++++++++++ .../workflows/codeowner-approved-label.yml | 153 +++++------------- 3 files changed, 217 insertions(+), 119 deletions(-) create mode 100644 .github/workflows/codeowner-approved-label-update.yml diff --git a/.github/scripts/codeowners.js b/.github/scripts/codeowners.js index 9a10391699e..5d69c11b1a2 100644 --- a/.github/scripts/codeowners.js +++ b/.github/scripts/codeowners.js @@ -2,7 +2,7 @@ // // Used by: // - codeowner-review-request.yml -// - codeowner-approved-label.yml +// - codeowner-approved-label.yml + codeowner-approved-label-update.yml // - auto-label-pr/detectors.js (detectCodeOwner) /** @@ -133,11 +133,95 @@ function loadCodeowners(repoRoot = '.') { return parseCodeowners(content); } +/** Possible label actions returned by determineLabelAction. */ +const LabelAction = Object.freeze({ + ADD: 'add', + REMOVE: 'remove', + NONE: 'none', +}); + +/** + * Determine what label action is needed for a PR based on codeowner approvals. + * + * Checks changed files against CODEOWNERS patterns, reviews, and current labels + * to decide if the label should be added, removed, or left unchanged. + * + * @param {object} github - octokit instance from actions/github-script + * @param {string} owner - repo owner + * @param {string} repo - repo name + * @param {number} pr_number - pull request number + * @param {Array} codeownersPatterns - from loadCodeowners / fetchCodeowners + * @param {string} labelName - label to manage + * @returns {Promise} + */ +async function determineLabelAction(github, owner, repo, pr_number, codeownersPatterns, labelName) { + // Get the list of changed files in this PR + const prFiles = await github.paginate( + github.rest.pulls.listFiles, + { owner, repo, pull_number: pr_number } + ); + + const changedFiles = prFiles.map(file => file.filename); + console.log(`Found ${changedFiles.length} changed files`); + + if (changedFiles.length === 0) { + console.log('No changed files found'); + return LabelAction.NONE; + } + + // Get effective owners using last-match-wins semantics + const effective = getEffectiveOwners(changedFiles, codeownersPatterns); + const componentCodeowners = effective.users; + + console.log(`Component-specific codeowners: ${Array.from(componentCodeowners).join(', ') || '(none)'}`); + + // Get current labels + const { data: currentLabels } = await github.rest.issues.listLabelsOnIssue({ + owner, repo, issue_number: pr_number + }); + const hasLabel = currentLabels.some(label => label.name === labelName); + + if (componentCodeowners.size === 0) { + console.log('No component-specific codeowners found'); + return hasLabel ? LabelAction.REMOVE : LabelAction.NONE; + } + + // Get all reviews and find latest per user + const reviews = await github.paginate( + github.rest.pulls.listReviews, + { owner, repo, pull_number: pr_number } + ); + + const latestReviewByUser = new Map(); + for (const review of reviews) { + if (!review.user || review.user.type === 'Bot' || review.state === 'COMMENTED') continue; + latestReviewByUser.set(review.user.login, review); + } + + // Check if any component-specific codeowner has an active approval + let hasCodeownerApproval = false; + for (const [login, review] of latestReviewByUser) { + if (review.state === 'APPROVED' && componentCodeowners.has(login)) { + console.log(`Codeowner '${login}' has approved`); + hasCodeownerApproval = true; + break; + } + } + + if (hasCodeownerApproval && !hasLabel) return LabelAction.ADD; + if (!hasCodeownerApproval && hasLabel) return LabelAction.REMOVE; + + console.log(`Label already ${hasLabel ? 'present' : 'absent'}, no change needed`); + return LabelAction.NONE; +} + module.exports = { globToRegex, parseCodeowners, fetchCodeowners, loadCodeowners, classifyOwners, - getEffectiveOwners + getEffectiveOwners, + LabelAction, + determineLabelAction }; diff --git a/.github/workflows/codeowner-approved-label-update.yml b/.github/workflows/codeowner-approved-label-update.yml new file mode 100644 index 00000000000..9168cce1d6b --- /dev/null +++ b/.github/workflows/codeowner-approved-label-update.yml @@ -0,0 +1,95 @@ +# Fallback for fork PRs: phase 1 (codeowner-approved-label.yml) handles +# non-fork PRs directly but can't write labels on fork PRs (read-only token). +# This workflow re-determines the action and applies it if needed. + +name: Codeowner Approved Label Update + +on: + workflow_run: + workflows: ["Codeowner Approved Label"] + types: [completed] + +permissions: + issues: write + pull-requests: read + contents: read + +jobs: + update-label: + name: Run + if: > + github.event.workflow_run.conclusion == 'success' && + github.event.workflow_run.event == 'pull_request_review' + runs-on: ubuntu-latest + steps: + - name: Get PR details + id: pr + env: + GH_TOKEN: ${{ github.token }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + REPO: ${{ github.repository }} + run: | + pr_data=$(gh pr list --repo "$REPO" --state open --search "$HEAD_SHA" \ + --json number,baseRefName --jq '.[0] // empty') + + if [ -z "$pr_data" ]; then + echo "No open PR found for SHA $HEAD_SHA, skipping" + echo "skip=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + + pr_number=$(echo "$pr_data" | jq -r '.number') + base_ref=$(echo "$pr_data" | jq -r '.baseRefName') + + echo "pr_number=$pr_number" >> "$GITHUB_OUTPUT" + echo "base_ref=$base_ref" >> "$GITHUB_OUTPUT" + echo "Found PR #$pr_number targeting $base_ref" + + - name: Checkout base repository + if: steps.pr.outputs.skip != 'true' + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + repository: ${{ github.repository }} + ref: ${{ steps.pr.outputs.base_ref }} + sparse-checkout: | + .github/scripts/codeowners.js + CODEOWNERS + + - name: Update label + if: steps.pr.outputs.skip != 'true' + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + env: + PR_NUMBER: ${{ steps.pr.outputs.pr_number }} + with: + script: | + const { loadCodeowners, determineLabelAction, LabelAction } = require('./.github/scripts/codeowners.js'); + + const owner = context.repo.owner; + const repo = context.repo.repo; + const pr_number = parseInt(process.env.PR_NUMBER, 10); + const LABEL_NAME = 'code-owner-approved'; + + console.log(`Processing PR #${pr_number} for codeowner approval label`); + + const codeownersPatterns = loadCodeowners(); + const action = await determineLabelAction( + github, owner, repo, pr_number, codeownersPatterns, LABEL_NAME + ); + + if (action === LabelAction.ADD) { + await github.rest.issues.addLabels({ + owner, repo, issue_number: pr_number, labels: [LABEL_NAME] + }); + console.log(`Added '${LABEL_NAME}' label`); + } else if (action === LabelAction.REMOVE) { + try { + await github.rest.issues.removeLabel({ + owner, repo, issue_number: pr_number, name: LABEL_NAME + }); + console.log(`Removed '${LABEL_NAME}' label`); + } catch (error) { + if (error.status !== 404) throw error; + } + } else { + console.log('No label change needed'); + } diff --git a/.github/workflows/codeowner-approved-label.yml b/.github/workflows/codeowner-approved-label.yml index 200f18f5448..12199bd0b04 100644 --- a/.github/workflows/codeowner-approved-label.yml +++ b/.github/workflows/codeowner-approved-label.yml @@ -1,9 +1,9 @@ -# This workflow adds/removes a 'code-owner-approved' label when a -# component-specific codeowner approves (or dismisses) a PR. -# This helps maintainers prioritize PRs that have codeowner sign-off. +# Adds/removes a 'code-owner-approved' label when a component-specific +# codeowner approves (or dismisses) a PR. # -# Only component-specific codeowners count — the catch-all @esphome/core -# team is excluded so the label reflects domain-expert approval. +# Handles non-fork PRs directly. For fork PRs the GITHUB_TOKEN is read-only, +# so label writes are deferred to codeowner-approved-label-update.yml which +# triggers via workflow_run with write permissions. name: Codeowner Approved Label @@ -26,134 +26,53 @@ jobs: uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: ref: ${{ github.event.pull_request.base.sha }} + sparse-checkout: | + .github/scripts/codeowners.js + CODEOWNERS - name: Check codeowner approval and update label uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + env: + PR_NUMBER: ${{ github.event.pull_request.number }} with: script: | - const { loadCodeowners, getEffectiveOwners } = require('./.github/scripts/codeowners.js'); + const { loadCodeowners, determineLabelAction, LabelAction } = require('./.github/scripts/codeowners.js'); const owner = context.repo.owner; const repo = context.repo.repo; - const pr_number = context.payload.pull_request.number; + const pr_number = parseInt(process.env.PR_NUMBER, 10); const LABEL_NAME = 'code-owner-approved'; console.log(`Processing PR #${pr_number} for codeowner approval label`); + const codeownersPatterns = loadCodeowners(); + const action = await determineLabelAction( + github, owner, repo, pr_number, codeownersPatterns, LABEL_NAME + ); + + if (action === LabelAction.NONE) { + console.log('No label change needed'); + return; + } + try { - // Get the list of changed files in this PR (with pagination) - const prFiles = await github.paginate( - github.rest.pulls.listFiles, - { - owner, - repo, - pull_number: pr_number - } - ); - - const changedFiles = prFiles.map(file => file.filename); - console.log(`Found ${changedFiles.length} changed files`); - - if (changedFiles.length === 0) { - console.log('No changed files found, skipping'); - return; - } - - // Parse CODEOWNERS from the checked-out base branch - const codeownersPatterns = loadCodeowners(); - - // Get effective owners using last-match-wins semantics - const effective = getEffectiveOwners(changedFiles, codeownersPatterns); - - // Only keep individual component-specific codeowners (exclude teams) - const componentCodeowners = effective.users; - - console.log(`Component-specific codeowners for changed files: ${Array.from(componentCodeowners).join(', ') || '(none)'}`); - - if (componentCodeowners.size === 0) { - console.log('No component-specific codeowners found for changed files'); - // Remove label if present since there are no component codeowners - try { - await github.rest.issues.removeLabel({ - owner, - repo, - issue_number: pr_number, - name: LABEL_NAME - }); - console.log(`Removed '${LABEL_NAME}' label (no component codeowners)`); - } catch (error) { - if (error.status !== 404) { - console.log(`Failed to remove label: ${error.message}`); - } - } - return; - } - - // Get all reviews on the PR - const reviews = await github.paginate( - github.rest.pulls.listReviews, - { - owner, - repo, - pull_number: pr_number - } - ); - - // Get the latest review per user (reviews are returned chronologically) - const latestReviewByUser = new Map(); - for (const review of reviews) { - // Skip bot reviews and comment-only reviews - if (!review.user || review.user.type === 'Bot' || review.state === 'COMMENTED') continue; - latestReviewByUser.set(review.user.login, review); - } - - // Check if any component-specific codeowner has an active approval - let hasCodeownerApproval = false; - for (const [login, review] of latestReviewByUser) { - if (review.state === 'APPROVED' && componentCodeowners.has(login)) { - console.log(`Codeowner '${login}' has approved`); - hasCodeownerApproval = true; - break; - } - } - - // Get current labels to check if label is already present - const { data: currentLabels } = await github.rest.issues.listLabelsOnIssue({ - owner, - repo, - issue_number: pr_number - }); - const hasLabel = currentLabels.some(label => label.name === LABEL_NAME); - - if (hasCodeownerApproval && !hasLabel) { - // Add the label + if (action === LabelAction.ADD) { await github.rest.issues.addLabels({ - owner, - repo, - issue_number: pr_number, - labels: [LABEL_NAME] + owner, repo, issue_number: pr_number, labels: [LABEL_NAME] }); console.log(`Added '${LABEL_NAME}' label`); - } else if (!hasCodeownerApproval && hasLabel) { - // Remove the label - try { - await github.rest.issues.removeLabel({ - owner, - repo, - issue_number: pr_number, - name: LABEL_NAME - }); - console.log(`Removed '${LABEL_NAME}' label`); - } catch (error) { - if (error.status !== 404) { - console.log(`Failed to remove label: ${error.message}`); - } - } - } else { - console.log(`Label already ${hasLabel ? 'present' : 'absent'}, no change needed`); + } else if (action === LabelAction.REMOVE) { + await github.rest.issues.removeLabel({ + owner, repo, issue_number: pr_number, name: LABEL_NAME + }); + console.log(`Removed '${LABEL_NAME}' label`); } - } catch (error) { - console.error(error); - core.setFailed(`Failed to process codeowner approval label: ${error.message}`); + if (error.status === 403) { + console.log('Fork PR: deferring label write to phase 2 workflow'); + } else if (error.status === 404) { + console.log('Label already removed'); + } else { + throw error; + } } From f5c37bf486d1a8905b7518478fec23a2e6e3b9ae Mon Sep 17 00:00:00 2001 From: Clyde Stubbs <2366188+clydebarrow@users.noreply.github.com> Date: Thu, 5 Mar 2026 14:24:01 +1100 Subject: [PATCH 12/13] [packet_transport] Minimise heap allocations (#14482) --- .../packet_transport/packet_transport.cpp | 79 ++-- .../packet_transport/packet_transport.h | 19 +- script/cpp_unit_test.py | 44 +- tests/components/packet_transport/common.h | 98 ++++ .../components/packet_transport/cpp_test.yaml | 11 + .../packet_transport_test.cpp | 445 ++++++++++++++++++ 6 files changed, 659 insertions(+), 37 deletions(-) create mode 100644 tests/components/packet_transport/common.h create mode 100644 tests/components/packet_transport/cpp_test.yaml create mode 100644 tests/components/packet_transport/packet_transport_test.cpp diff --git a/esphome/components/packet_transport/packet_transport.cpp b/esphome/components/packet_transport/packet_transport.cpp index d2c59200017..f241cc61142 100644 --- a/esphome/components/packet_transport/packet_transport.cpp +++ b/esphome/components/packet_transport/packet_transport.cpp @@ -3,6 +3,8 @@ #include "esphome/core/helpers.h" #include "packet_transport.h" +#include + #include "esphome/components/xxtea/xxtea.h" namespace esphome { @@ -77,7 +79,7 @@ enum DecodeResult { DECODE_EMPTY, }; -static const size_t MAX_PING_KEYS = 4; +static constexpr size_t MAX_PING_KEYS = 4; static inline void add(std::vector &vec, uint32_t data) { vec.push_back(data & 0xFF); @@ -168,7 +170,7 @@ class PacketDecoder { return true; } - bool decrypt(const uint32_t *key) { + bool decrypt(const uint32_t *key) const { if (this->get_remaining_size() % 4 != 0) { return false; } @@ -249,9 +251,9 @@ void PacketTransport::init_data_() { } else { add(this->data_, DATA_KEY); } - for (const auto &pkey : this->ping_keys_) { + for (auto &value : this->ping_keys_ | std::views::values) { add(this->data_, PING_KEY); - add(this->data_, pkey.second); + add(this->data_, value); } } @@ -331,7 +333,7 @@ void PacketTransport::update() { auto now = millis() / 1000; if (this->last_key_time_ + this->ping_pong_recyle_time_ < now) { this->resend_ping_key_ = this->ping_pong_enable_; - ESP_LOGV(TAG, "Ping request, age %u", now - this->last_key_time_); + ESP_LOGV(TAG, "Ping request, age %" PRIu32, now - this->last_key_time_); this->last_key_time_ = now; } for (const auto &provider : this->providers_) { @@ -339,24 +341,32 @@ void PacketTransport::update() { if (key_response_age > (this->ping_pong_recyle_time_ * 2u)) { #ifdef USE_STATUS_SENSOR if (provider.second.status_sensor != nullptr && provider.second.status_sensor->state) { - ESP_LOGI(TAG, "Ping status for %s timeout at %u with age %u", provider.first.c_str(), now, key_response_age); + ESP_LOGI(TAG, "Ping status for %s timeout at %" PRIu32 " with age %" PRIu32, provider.first.c_str(), now, + key_response_age); provider.second.status_sensor->publish_state(false); } #endif #ifdef USE_SENSOR - for (auto &sensor : this->remote_sensors_[provider.first]) { - sensor.second->publish_state(NAN); + auto it = this->remote_sensors_.find(provider.first); + if (it != this->remote_sensors_.end()) { + for (auto &val : it->second | std::views::values) { + val->publish_state(NAN); + } } #endif #ifdef USE_BINARY_SENSOR - for (auto &sensor : this->remote_binary_sensors_[provider.first]) { - sensor.second->invalidate_state(); + auto bs_it = this->remote_binary_sensors_.find(provider.first); + if (bs_it != this->remote_binary_sensors_.end()) { + for (auto &val : bs_it->second | std::views::values) { + val->invalidate_state(); + } } #endif } else { #ifdef USE_STATUS_SENSOR if (provider.second.status_sensor != nullptr && !provider.second.status_sensor->state) { - ESP_LOGI(TAG, "Ping status for %s restored at %u with age %u", provider.first.c_str(), now, key_response_age); + ESP_LOGI(TAG, "Ping status for %s restored at %" PRIu32 " with age %" PRIu32, provider.first.c_str(), now, + key_response_age); provider.second.status_sensor->publish_state(true); } #endif @@ -367,11 +377,16 @@ void PacketTransport::update() { void PacketTransport::add_key_(const char *name, uint32_t key) { if (!this->is_encrypted_()) return; - if (this->ping_keys_.count(name) == 0 && this->ping_keys_.size() == MAX_PING_KEYS) { - ESP_LOGW(TAG, "Ping key from %s discarded", name); - return; + auto it = this->ping_keys_.find(name); + if (it == this->ping_keys_.end()) { + if (this->ping_keys_.size() == MAX_PING_KEYS) { + ESP_LOGW(TAG, "Ping key from %s discarded", name); + return; + } + this->ping_keys_.emplace(name, key); // allocates string key once only + } else { + it->second = key; // key string already exists in map, no allocation } - this->ping_keys_[name] = key; this->updated_ = true; ESP_LOGV(TAG, "Ping key from %s now %X", name, (unsigned) key); } @@ -431,17 +446,19 @@ void PacketTransport::process_(std::span data) { return; } - if (this->providers_.count(namebuf) == 0) { + auto it = this->providers_.find(namebuf); + if (it == this->providers_.end()) { ESP_LOGVV(TAG, "Unknown hostname %s", namebuf); return; } + auto &provider = it->second; ESP_LOGV(TAG, "Found hostname %s", namebuf); #ifdef USE_SENSOR - auto &sensors = this->remote_sensors_[namebuf]; + auto &sensors = this->remote_sensors_.try_emplace(namebuf).first->second; #endif #ifdef USE_BINARY_SENSOR - auto &binary_sensors = this->remote_binary_sensors_[namebuf]; + auto &binary_sensors = this->remote_binary_sensors_.try_emplace(namebuf).first->second; #endif if (!decoder.bump_to(4)) { @@ -453,7 +470,6 @@ void PacketTransport::process_(std::span data) { return; } - auto &provider = this->providers_[namebuf]; // if encryption not used with this host, ping check is pointless since it would be easily spoofed. if (provider.encryption_key.empty()) ping_key_seen = true; @@ -495,16 +511,19 @@ void PacketTransport::process_(std::span data) { if (decoder.decode(BINARY_SENSOR_KEY, namebuf, sizeof(namebuf), byte) == DECODE_OK) { ESP_LOGV(TAG, "Got binary sensor %s %d", namebuf, byte); #ifdef USE_BINARY_SENSOR - if (binary_sensors.count(namebuf) != 0) - binary_sensors[namebuf]->publish_state(byte != 0); + auto bs = binary_sensors.find(namebuf); + if (bs != binary_sensors.end()) { + bs->second->publish_state(byte != 0); + } #endif continue; } if (decoder.decode(SENSOR_KEY, namebuf, sizeof(namebuf), rdata.u32) == DECODE_OK) { ESP_LOGV(TAG, "Got sensor %s %f", namebuf, rdata.f32); #ifdef USE_SENSOR - if (sensors.count(namebuf) != 0) - sensors[namebuf]->publish_state(rdata.f32); + auto sensor_it = sensors.find(namebuf); + if (sensor_it != sensors.end()) + sensor_it->second->publish_state(rdata.f32); #endif continue; } @@ -537,12 +556,18 @@ void PacketTransport::dump_config() { ESP_LOGCONFIG(TAG, " Remote host: %s", host.first.c_str()); ESP_LOGCONFIG(TAG, " Encrypted: %s", YESNO(!host.second.encryption_key.empty())); #ifdef USE_SENSOR - for (const auto &sensor : this->remote_sensors_[host.first.c_str()]) - ESP_LOGCONFIG(TAG, " Sensor: %s", sensor.first.c_str()); + auto rs = this->remote_sensors_.find(host.first.c_str()); + if (rs != this->remote_sensors_.end()) { + for (const auto &key : rs->second | std::views::keys) + ESP_LOGCONFIG(TAG, " Sensor: %s", key.c_str()); + } #endif #ifdef USE_BINARY_SENSOR - for (const auto &sensor : this->remote_binary_sensors_[host.first.c_str()]) - ESP_LOGCONFIG(TAG, " Binary Sensor: %s", sensor.first.c_str()); + auto rbs = this->remote_binary_sensors_.find(host.first.c_str()); + if (rbs != this->remote_binary_sensors_.end()) { + for (const auto &key : rbs->second | std::views::keys) + ESP_LOGCONFIG(TAG, " Binary Sensor: %s", key.c_str()); + } #endif } } diff --git a/esphome/components/packet_transport/packet_transport.h b/esphome/components/packet_transport/packet_transport.h index a2367442317..b3798738e2c 100644 --- a/esphome/components/packet_transport/packet_transport.h +++ b/esphome/components/packet_transport/packet_transport.h @@ -24,6 +24,9 @@ namespace esphome { namespace packet_transport { +// std::less provides allocation-free comparison with const char * +template using string_map_t = std::map>; + struct Provider { std::vector encryption_key; const char *name; @@ -79,15 +82,15 @@ class PacketTransport : public PollingComponent { #endif void add_provider(const char *hostname) { - if (this->providers_.count(hostname) == 0) { + if (!this->providers_.contains(hostname)) { Provider provider{}; provider.name = hostname; this->providers_[hostname] = provider; #ifdef USE_SENSOR - this->remote_sensors_[hostname] = std::map(); + this->remote_sensors_[hostname] = string_map_t(); #endif #ifdef USE_BINARY_SENSOR - this->remote_binary_sensors_[hostname] = std::map(); + this->remote_binary_sensors_[hostname] = string_map_t(); #endif } } @@ -139,23 +142,23 @@ class PacketTransport : public PollingComponent { #ifdef USE_SENSOR std::vector sensors_{}; - std::map> remote_sensors_{}; + string_map_t> remote_sensors_{}; #endif #ifdef USE_BINARY_SENSOR std::vector binary_sensors_{}; - std::map> remote_binary_sensors_{}; + string_map_t> remote_binary_sensors_{}; #endif - std::map providers_{}; + string_map_t providers_{}; std::vector ping_header_{}; std::vector header_{}; std::vector data_{}; - std::map ping_keys_{}; + string_map_t ping_keys_{}; const char *platform_name_{""}; void add_key_(const char *name, uint32_t key); void send_ping_pong_request_(); - inline bool is_encrypted_() { return !this->encryption_key_.empty(); } + bool is_encrypted_() const { return !this->encryption_key_.empty(); } }; } // namespace packet_transport diff --git a/script/cpp_unit_test.py b/script/cpp_unit_test.py index 02b133060aa..b87261ab332 100755 --- a/script/cpp_unit_test.py +++ b/script/cpp_unit_test.py @@ -12,6 +12,7 @@ from esphome.__main__ import command_compile, parse_args from esphome.config import validate_config from esphome.core import CORE from esphome.platformio_api import get_idedata +from esphome.yaml_util import load_yaml # This must coincide with the version in /platformio.ini PLATFORMIO_GOOGLE_TEST_LIB = "google/googletest@^1.15.2" @@ -44,6 +45,38 @@ def filter_components_without_tests(components: list[str]) -> list[str]: return filtered_components +# Name of optional per-component YAML config merged into the test build +# before validation so that platform defines (USE_SENSOR, etc.) are generated. +CPP_TEST_CONFIG_FILE = "cpp_test.yaml" + + +def load_component_test_configs(components: list[str]) -> dict: + """Load cpp_test.yaml files from test component directories. + + These configs are merged into the base test config *before* validation + so that entity registration runs during code generation, which causes + the corresponding USE_* defines to be emitted. + """ + merged: dict = {} + for component in components: + config_file = COMPONENTS_TESTS_DIR / component / CPP_TEST_CONFIG_FILE + if not config_file.exists(): + continue + component_config = load_yaml(config_file) + if not component_config: + continue + for key, value in component_config.items(): + if ( + key in merged + and isinstance(merged[key], list) + and isinstance(value, list) + ): + merged[key].extend(value) + else: + merged[key] = value + return merged + + def create_test_config(config_name: str, includes: list[str]) -> dict: """Create ESPHome test configuration for C++ unit tests. @@ -115,6 +148,11 @@ def run_tests(selected_components: list[str]) -> int: config = create_test_config(config_name, includes) + # Merge component-specific test configs (e.g. sensor instances) before + # validation so that entity registration and USE_* defines work. + extra_config = load_component_test_configs(components) + config.update(extra_config) + CORE.config_path = COMPONENTS_TESTS_DIR / "dummy.yaml" CORE.dashboard = None @@ -122,8 +160,10 @@ def run_tests(selected_components: list[str]) -> int: config = validate_config(config, {}) # Add all components and dependencies to the base configuration after validation, so their files - # are added to the build. - config.update({key: {} for key in components_with_dependencies}) + # are added to the build. Use setdefault to avoid overwriting entries that were + # already validated (e.g. sensor instances from cpp_test.yaml). + for key in components_with_dependencies: + config.setdefault(key, {}) print(f"Testing components: {', '.join(components)}") CORE.config = config diff --git a/tests/components/packet_transport/common.h b/tests/components/packet_transport/common.h new file mode 100644 index 00000000000..f8caa7bb683 --- /dev/null +++ b/tests/components/packet_transport/common.h @@ -0,0 +1,98 @@ +#pragma once +#include +#include +#include +#include +#include +#include "esphome/components/packet_transport/packet_transport.h" + +namespace esphome::packet_transport::testing { + +// Protocol constants mirrored from packet_transport.cpp for test packet construction. +static constexpr uint16_t MAGIC_NUMBER = 0x4553; +static constexpr uint16_t MAGIC_PING = 0x5048; + +// Concrete testable implementation of PacketTransport. +// Captures sent packets and exposes protected members for verification. +// +// Sensor round-trip tests require USE_SENSOR / USE_BINARY_SENSOR to be defined, +// which happens when 'sensor' and 'binary_sensor' components are in the build. +// Run with --all or include those components to enable the full test suite. +class TestablePacketTransport : public PacketTransport { + public: + using PacketTransport::add_key_; + using PacketTransport::data_; + using PacketTransport::encryption_key_; + using PacketTransport::flush_; + using PacketTransport::header_; + using PacketTransport::increment_code_; + using PacketTransport::init_data_; + using PacketTransport::is_encrypted_; + using PacketTransport::is_provider_; + using PacketTransport::name_; + using PacketTransport::ping_key_; + using PacketTransport::ping_keys_; + using PacketTransport::ping_pong_enable_; + using PacketTransport::ping_pong_recyle_time_; + using PacketTransport::process_; + using PacketTransport::providers_; + using PacketTransport::rolling_code_; + using PacketTransport::rolling_code_enable_; + using PacketTransport::send_data_; + using PacketTransport::updated_; +#ifdef USE_SENSOR + using PacketTransport::add_data_; + using PacketTransport::remote_sensors_; + using PacketTransport::sensors_; +#endif +#ifdef USE_BINARY_SENSOR + using PacketTransport::add_binary_data_; + using PacketTransport::binary_sensors_; + using PacketTransport::remote_binary_sensors_; +#endif + + // NOTE: std::vector is used here for test convenience. For production code, + // consider using StaticVector or FixedVector from esphome/core/helpers.h instead. + mutable std::vector> sent_packets; + size_t max_packet_size{512}; + bool send_enabled{true}; + + void send_packet(const std::vector &buf) const override { this->sent_packets.push_back(buf); } + size_t get_max_packet_size() override { return this->max_packet_size; } + bool should_send() override { return this->send_enabled; } + + /// Build the packet header for testing without requiring App or global_preferences. + void init_for_test(const char *name) { + this->name_ = name; + this->header_.clear(); + // MAGIC_NUMBER as uint16_t little-endian + this->header_.push_back(MAGIC_NUMBER & 0xFF); + this->header_.push_back((MAGIC_NUMBER >> 8) & 0xFF); + // Length-prefixed hostname + auto len = strlen(name); + this->header_.push_back(static_cast(len)); + for (size_t i = 0; i < len; i++) + this->header_.push_back(name[i]); + // Pad to 4-byte boundary + while (this->header_.size() & 0x3) + this->header_.push_back(0); + } +}; + +/// Build a MAGIC_PING packet for testing add_key_ / ping-pong flows. +inline std::vector build_ping_packet(const char *hostname, uint32_t key) { + std::vector packet; + packet.push_back(MAGIC_PING & 0xFF); + packet.push_back((MAGIC_PING >> 8) & 0xFF); + auto len = strlen(hostname); + packet.push_back(static_cast(len)); + for (size_t i = 0; i < len; i++) + packet.push_back(hostname[i]); + packet.push_back(key & 0xFF); + packet.push_back((key >> 8) & 0xFF); + packet.push_back((key >> 16) & 0xFF); + packet.push_back((key >> 24) & 0xFF); + return packet; +} + +} // namespace esphome::packet_transport::testing diff --git a/tests/components/packet_transport/cpp_test.yaml b/tests/components/packet_transport/cpp_test.yaml new file mode 100644 index 00000000000..fa39df3c0ae --- /dev/null +++ b/tests/components/packet_transport/cpp_test.yaml @@ -0,0 +1,11 @@ +# Extra component configuration required by C++ unit tests. +# Loaded by cpp_unit_test.py and merged into the test build config +# before validation, so that platform defines (USE_SENSOR, etc.) are generated. + +sensor: + - platform: template + id: test_cpp_sensor + +binary_sensor: + - platform: template + id: test_cpp_binary_sensor diff --git a/tests/components/packet_transport/packet_transport_test.cpp b/tests/components/packet_transport/packet_transport_test.cpp new file mode 100644 index 00000000000..d8f11ca6072 --- /dev/null +++ b/tests/components/packet_transport/packet_transport_test.cpp @@ -0,0 +1,445 @@ +#include "common.h" + +namespace esphome::packet_transport::testing { + +// --- Configuration setter tests --- + +TEST(PacketTransportTest, SetIsProvider) { + TestablePacketTransport transport; + transport.set_is_provider(true); + EXPECT_TRUE(transport.is_provider_); +} + +TEST(PacketTransportTest, SetEncryptionKey) { + TestablePacketTransport transport; + std::vector key(32, 0xAB); + transport.set_encryption_key(key); + EXPECT_EQ(transport.encryption_key_, key); + EXPECT_TRUE(transport.is_encrypted_()); +} + +TEST(PacketTransportTest, NoEncryptionByDefault) { + TestablePacketTransport transport; + EXPECT_FALSE(transport.is_encrypted_()); +} + +TEST(PacketTransportTest, SetRollingCodeEnable) { + TestablePacketTransport transport; + transport.set_rolling_code_enable(true); + EXPECT_TRUE(transport.rolling_code_enable_); +} + +TEST(PacketTransportTest, SetPingPongEnable) { + TestablePacketTransport transport; + transport.set_ping_pong_enable(true); + EXPECT_TRUE(transport.ping_pong_enable_); +} + +TEST(PacketTransportTest, SetPingPongRecycleTime) { + TestablePacketTransport transport; + transport.set_ping_pong_recycle_time(600); + EXPECT_EQ(transport.ping_pong_recyle_time_, 600u); +} + +// --- Provider management --- + +TEST(PacketTransportTest, AddProvider) { + TestablePacketTransport transport; + transport.add_provider("host1"); + EXPECT_TRUE(transport.providers_.contains("host1")); + EXPECT_EQ(transport.providers_.size(), 1u); +} + +TEST(PacketTransportTest, AddProviderDuplicate) { + TestablePacketTransport transport; + transport.add_provider("host1"); + transport.add_provider("host1"); + EXPECT_EQ(transport.providers_.size(), 1u); +} + +TEST(PacketTransportTest, SetProviderEncryption) { + TestablePacketTransport transport; + transport.add_provider("host1"); + std::vector key(32, 0xCD); + transport.set_provider_encryption("host1", key); + EXPECT_EQ(transport.providers_["host1"].encryption_key, key); +} + +// --- Sensor management (requires USE_SENSOR / USE_BINARY_SENSOR) --- + +#ifdef USE_SENSOR +TEST(PacketTransportTest, AddSensor) { + TestablePacketTransport transport; + sensor::Sensor s; + transport.add_sensor("temp", &s); + ASSERT_EQ(transport.sensors_.size(), 1u); + EXPECT_STREQ(transport.sensors_[0].id, "temp"); + EXPECT_EQ(transport.sensors_[0].sensor, &s); + EXPECT_TRUE(transport.sensors_[0].updated); +} + +TEST(PacketTransportTest, AddRemoteSensor) { + TestablePacketTransport transport; + sensor::Sensor s; + transport.add_remote_sensor("host1", "remote_temp", &s); + EXPECT_TRUE(transport.providers_.contains("host1")); + EXPECT_EQ(transport.remote_sensors_["host1"]["remote_temp"], &s); +} +#endif + +#ifdef USE_BINARY_SENSOR +TEST(PacketTransportTest, AddBinarySensor) { + TestablePacketTransport transport; + binary_sensor::BinarySensor bs; + transport.add_binary_sensor("motion", &bs); + ASSERT_EQ(transport.binary_sensors_.size(), 1u); + EXPECT_STREQ(transport.binary_sensors_[0].id, "motion"); + EXPECT_EQ(transport.binary_sensors_[0].sensor, &bs); +} + +TEST(PacketTransportTest, AddRemoteBinarySensor) { + TestablePacketTransport transport; + binary_sensor::BinarySensor bs; + transport.add_remote_binary_sensor("host1", "remote_motion", &bs); + EXPECT_TRUE(transport.providers_.contains("host1")); + EXPECT_EQ(transport.remote_binary_sensors_["host1"]["remote_motion"], &bs); +} +#endif + +// --- Unencrypted round-trip tests (require USE_SENSOR / USE_BINARY_SENSOR) --- + +#ifdef USE_SENSOR +TEST(PacketTransportTest, UnencryptedSensorRoundTrip) { + // Encoder + TestablePacketTransport encoder; + encoder.init_for_test("sender"); + sensor::Sensor local_sensor; + local_sensor.state = 42.5f; + encoder.add_sensor("temp", &local_sensor); + + encoder.send_data_(true); + ASSERT_EQ(encoder.sent_packets.size(), 1u); + + // Decoder + TestablePacketTransport decoder; + decoder.init_for_test("receiver"); + sensor::Sensor remote_sensor; + remote_sensor.state = -999.0f; // sentinel + decoder.add_remote_sensor("sender", "temp", &remote_sensor); + + auto &packet = encoder.sent_packets[0]; + decoder.process_({packet.data(), packet.size()}); + EXPECT_FLOAT_EQ(remote_sensor.state, 42.5f); +} +#endif + +#ifdef USE_BINARY_SENSOR +TEST(PacketTransportTest, UnencryptedBinarySensorRoundTrip) { + TestablePacketTransport encoder; + encoder.init_for_test("sender"); + binary_sensor::BinarySensor local_bs; + local_bs.state = true; + encoder.add_binary_sensor("motion", &local_bs); + + encoder.send_data_(true); + ASSERT_EQ(encoder.sent_packets.size(), 1u); + + TestablePacketTransport decoder; + decoder.init_for_test("receiver"); + binary_sensor::BinarySensor remote_bs; + decoder.add_remote_binary_sensor("sender", "motion", &remote_bs); + + auto &packet = encoder.sent_packets[0]; + decoder.process_({packet.data(), packet.size()}); + EXPECT_TRUE(remote_bs.state); +} +#endif + +#if defined(USE_SENSOR) && defined(USE_BINARY_SENSOR) +TEST(PacketTransportTest, MultipleSensorsRoundTrip) { + TestablePacketTransport encoder; + encoder.init_for_test("sender"); + + sensor::Sensor s1, s2; + s1.state = 10.0f; + s2.state = 20.0f; + encoder.add_sensor("s1", &s1); + encoder.add_sensor("s2", &s2); + + binary_sensor::BinarySensor bs1; + bs1.state = true; + encoder.add_binary_sensor("bs1", &bs1); + + encoder.send_data_(true); + ASSERT_EQ(encoder.sent_packets.size(), 1u); + + TestablePacketTransport decoder; + decoder.init_for_test("receiver"); + sensor::Sensor rs1, rs2; + binary_sensor::BinarySensor rbs1; + rs1.state = -999.0f; + rs2.state = -999.0f; + decoder.add_remote_sensor("sender", "s1", &rs1); + decoder.add_remote_sensor("sender", "s2", &rs2); + decoder.add_remote_binary_sensor("sender", "bs1", &rbs1); + + auto &packet = encoder.sent_packets[0]; + decoder.process_({packet.data(), packet.size()}); + + EXPECT_FLOAT_EQ(rs1.state, 10.0f); + EXPECT_FLOAT_EQ(rs2.state, 20.0f); + EXPECT_TRUE(rbs1.state); +} +#endif + +// --- Encrypted round-trip --- + +#ifdef USE_SENSOR +TEST(PacketTransportTest, EncryptedSensorRoundTrip) { + std::vector key(32); + for (int i = 0; i < 32; i++) + key[i] = i; + + TestablePacketTransport encoder; + encoder.init_for_test("sender"); + encoder.set_encryption_key(key); + sensor::Sensor local_sensor; + local_sensor.state = 99.9f; + encoder.add_sensor("temp", &local_sensor); + + encoder.send_data_(true); + ASSERT_EQ(encoder.sent_packets.size(), 1u); + + TestablePacketTransport decoder; + decoder.init_for_test("receiver"); + sensor::Sensor remote_sensor; + remote_sensor.state = -999.0f; + decoder.add_remote_sensor("sender", "temp", &remote_sensor); + decoder.set_provider_encryption("sender", key); + + auto &packet = encoder.sent_packets[0]; + decoder.process_({packet.data(), packet.size()}); + EXPECT_FLOAT_EQ(remote_sensor.state, 99.9f); +} + +// --- Selective send --- + +TEST(PacketTransportTest, SendDataOnlyUpdated) { + TestablePacketTransport encoder; + encoder.init_for_test("sender"); + + sensor::Sensor s1, s2; + s1.state = 1.0f; + s2.state = 2.0f; + encoder.add_sensor("s1", &s1); + encoder.add_sensor("s2", &s2); + + // Mark s1 as not updated, only s2 as updated + encoder.sensors_[0].updated = false; + encoder.sensors_[1].updated = true; + + encoder.send_data_(false); + ASSERT_EQ(encoder.sent_packets.size(), 1u); + + TestablePacketTransport decoder; + decoder.init_for_test("receiver"); + sensor::Sensor rs1, rs2; + rs1.state = -999.0f; + rs2.state = -999.0f; + decoder.add_remote_sensor("sender", "s1", &rs1); + decoder.add_remote_sensor("sender", "s2", &rs2); + + auto &packet = encoder.sent_packets[0]; + decoder.process_({packet.data(), packet.size()}); + + EXPECT_FLOAT_EQ(rs1.state, -999.0f); // not updated, not sent + EXPECT_FLOAT_EQ(rs2.state, 2.0f); // updated, sent +} +#endif + +// --- Ping key tests --- + +TEST(PacketTransportTest, PingKeyStoredWhenEncrypted) { + TestablePacketTransport transport; + transport.init_for_test("receiver"); + transport.set_encryption_key(std::vector(32, 0xAA)); + + auto ping = build_ping_packet("requester", 0xDEADBEEF); + transport.process_({ping.data(), ping.size()}); + + ASSERT_EQ(transport.ping_keys_.size(), 1u); + EXPECT_EQ(transport.ping_keys_["requester"], 0xDEADBEEFu); +} + +TEST(PacketTransportTest, PingKeyIgnoredWhenNotEncrypted) { + TestablePacketTransport transport; + transport.init_for_test("receiver"); + // No encryption key — add_key_ should be a no-op + + auto ping = build_ping_packet("requester", 0xDEADBEEF); + transport.process_({ping.data(), ping.size()}); + + EXPECT_TRUE(transport.ping_keys_.empty()); +} + +TEST(PacketTransportTest, PingKeyUpdatedOnRepeat) { + TestablePacketTransport transport; + transport.init_for_test("receiver"); + transport.set_encryption_key(std::vector(32, 0xAA)); + + auto ping1 = build_ping_packet("host1", 0x1111); + transport.process_({ping1.data(), ping1.size()}); + EXPECT_EQ(transport.ping_keys_["host1"], 0x1111u); + + // Same host, new key value — should update in place + auto ping2 = build_ping_packet("host1", 0x2222); + transport.process_({ping2.data(), ping2.size()}); + EXPECT_EQ(transport.ping_keys_.size(), 1u); + EXPECT_EQ(transport.ping_keys_["host1"], 0x2222u); +} + +TEST(PacketTransportTest, PingKeyMaxLimit) { + TestablePacketTransport transport; + transport.init_for_test("receiver"); + transport.set_encryption_key(std::vector(32, 0xAA)); + + // Fill to MAX_PING_KEYS (4) + for (int i = 0; i < 4; i++) { + char name[16]; + snprintf(name, sizeof(name), "host%d", i); + auto ping = build_ping_packet(name, 0x1000 + i); + transport.process_({ping.data(), ping.size()}); + } + EXPECT_EQ(transport.ping_keys_.size(), 4u); + + // 5th key should be discarded + auto ping = build_ping_packet("host4", 0x9999); + transport.process_({ping.data(), ping.size()}); + EXPECT_EQ(transport.ping_keys_.size(), 4u); + EXPECT_FALSE(transport.ping_keys_.contains("host4")); +} + +#ifdef USE_SENSOR +TEST(PacketTransportTest, PingKeyIncludedInTransmittedPacket) { + std::vector key(32, 0xBB); + + // Responder: encrypted, owns a sensor + TestablePacketTransport responder; + responder.init_for_test("responder"); + responder.set_encryption_key(key); + sensor::Sensor local_sensor; + local_sensor.state = 77.7f; + responder.add_sensor("temp", &local_sensor); + + // Requester sends a MAGIC_PING that the responder processes + auto ping = build_ping_packet("requester", 0xDEADBEEF); + responder.process_({ping.data(), ping.size()}); + ASSERT_EQ(responder.ping_keys_.size(), 1u); + + // Responder sends sensor data — ping key should be embedded + responder.send_data_(true); + ASSERT_EQ(responder.sent_packets.size(), 1u); + + // Requester: encrypted provider, ping-pong enabled, expects key 0xDEADBEEF + TestablePacketTransport requester; + requester.init_for_test("requester"); + requester.set_ping_pong_enable(true); + requester.ping_key_ = 0xDEADBEEF; + sensor::Sensor remote_sensor; + remote_sensor.state = -999.0f; + requester.add_remote_sensor("responder", "temp", &remote_sensor); + requester.set_provider_encryption("responder", key); + + // The requester decrypts the packet and finds its ping key echoed back, + // which gates the sensor data — if the key is missing, data is blocked. + auto &packet = responder.sent_packets[0]; + requester.process_({packet.data(), packet.size()}); + EXPECT_FLOAT_EQ(remote_sensor.state, 77.7f); +} + +TEST(PacketTransportTest, MissingPingKeyBlocksSensorData) { + std::vector key(32, 0xBB); + + // Responder sends data WITHOUT receiving any MAGIC_PING first — no ping keys + TestablePacketTransport responder; + responder.init_for_test("responder"); + responder.set_encryption_key(key); + sensor::Sensor local_sensor; + local_sensor.state = 77.7f; + responder.add_sensor("temp", &local_sensor); + responder.send_data_(true); + ASSERT_EQ(responder.sent_packets.size(), 1u); + + // Requester with ping-pong enabled expects a key that isn't in the packet + TestablePacketTransport requester; + requester.init_for_test("requester"); + requester.set_ping_pong_enable(true); + requester.ping_key_ = 0xDEADBEEF; + sensor::Sensor remote_sensor; + remote_sensor.state = -999.0f; + requester.add_remote_sensor("responder", "temp", &remote_sensor); + requester.set_provider_encryption("responder", key); + + auto &packet = responder.sent_packets[0]; + requester.process_({packet.data(), packet.size()}); + EXPECT_FLOAT_EQ(remote_sensor.state, -999.0f); // blocked — ping key not found +} +#endif + +// --- Process error handling --- + +TEST(PacketTransportTest, ProcessShortBuffer) { + TestablePacketTransport transport; + transport.init_for_test("receiver"); + uint8_t buf[] = {0x53}; + // Too short for a magic number - should return safely + transport.process_({buf, 1}); +} + +TEST(PacketTransportTest, ProcessBadMagic) { + TestablePacketTransport transport; + transport.init_for_test("receiver"); + uint8_t buf[] = {0xFF, 0xFF, 0x00, 0x00}; + // Wrong magic - should return safely + transport.process_({buf, sizeof(buf)}); +} + +TEST(PacketTransportTest, ProcessOwnHostname) { + TestablePacketTransport transport; + transport.init_for_test("myself"); + // Build a packet from "myself" using a separate encoder + TestablePacketTransport fake_sender; + fake_sender.init_for_test("myself"); + fake_sender.send_data_(true); + ASSERT_EQ(fake_sender.sent_packets.size(), 1u); + + auto &packet = fake_sender.sent_packets[0]; + // Should be silently ignored because hostname matches our own + transport.process_({packet.data(), packet.size()}); +} + +TEST(PacketTransportTest, ProcessUnknownHostname) { + TestablePacketTransport transport; + transport.init_for_test("receiver"); + // No providers registered - "unknown" will not be found + TestablePacketTransport sender; + sender.init_for_test("unknown"); + sender.send_data_(true); + ASSERT_EQ(sender.sent_packets.size(), 1u); + + auto &packet = sender.sent_packets[0]; + // Should return safely without crash + transport.process_({packet.data(), packet.size()}); +} + +// --- Send disabled --- + +TEST(PacketTransportTest, NoSendWhenDisabled) { + TestablePacketTransport transport; + transport.init_for_test("sender"); + transport.send_enabled = false; + transport.send_data_(true); + EXPECT_TRUE(transport.sent_packets.empty()); +} + +} // namespace esphome::packet_transport::testing From 0e2a10c5f02cd4ba37ed53dde370bdd0a54c43ac Mon Sep 17 00:00:00 2001 From: rwrozelle Date: Wed, 4 Mar 2026 19:34:13 -0800 Subject: [PATCH 13/13] [openthread] Cache is_connected() for cheap inline access (#14484) Co-authored-by: J. Nick Koston --- esphome/components/openthread/openthread.cpp | 25 ++++++------------- esphome/components/openthread/openthread.h | 5 +++- .../components/openthread/openthread_esp.cpp | 3 +++ 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/esphome/components/openthread/openthread.cpp b/esphome/components/openthread/openthread.cpp index 92897a7e96e..9452f5a41eb 100644 --- a/esphome/components/openthread/openthread.cpp +++ b/esphome/components/openthread/openthread.cpp @@ -1,9 +1,7 @@ #include "esphome/core/defines.h" #ifdef USE_OPENTHREAD #include "openthread.h" -#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 5, 0) #include "esp_openthread.h" -#endif #include @@ -48,22 +46,15 @@ void OpenThreadComponent::dump_config() { } } -bool OpenThreadComponent::is_connected() { - auto lock = InstanceLock::try_acquire(100); - if (!lock) { - ESP_LOGW(TAG, "Failed to acquire OpenThread lock in is_connected"); - return false; +void OpenThreadComponent::on_state_changed_(otChangedFlags flags, void *context) { + if (flags & OT_CHANGED_THREAD_ROLE) { + auto *self = static_cast(context); + // This runs on the OpenThread task thread with the OT lock held, + // so we can safely call otThreadGetDeviceRole directly. + otInstance *instance = esp_openthread_get_instance(); + otDeviceRole role = otThreadGetDeviceRole(instance); + self->connected_ = role >= OT_DEVICE_ROLE_CHILD; } - - otInstance *instance = lock->get_instance(); - if (instance == nullptr) { - return false; - } - - otDeviceRole role = otThreadGetDeviceRole(instance); - - // TODO: If we're a leader, check that there is at least 1 known peer - return role >= OT_DEVICE_ROLE_CHILD; } // Gets the off-mesh routable address diff --git a/esphome/components/openthread/openthread.h b/esphome/components/openthread/openthread.h index 728847afa54..d853c58f958 100644 --- a/esphome/components/openthread/openthread.h +++ b/esphome/components/openthread/openthread.h @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -26,7 +27,7 @@ class OpenThreadComponent : public Component { bool teardown() override; float get_setup_priority() const override { return setup_priority::WIFI; } - bool is_connected(); + bool is_connected() const { return this->connected_; } network::IPAddresses get_ip_addresses(); std::optional get_omr_address(); void ot_main(); @@ -42,6 +43,7 @@ class OpenThreadComponent : public Component { protected: std::optional get_omr_address_(InstanceLock &lock); + static void on_state_changed_(otChangedFlags flags, void *context); std::function factory_reset_external_callback_; #if CONFIG_OPENTHREAD_MTD uint32_t poll_period_{0}; @@ -49,6 +51,7 @@ class OpenThreadComponent : public Component { std::optional output_power_{}; bool teardown_started_{false}; bool teardown_complete_{false}; + bool connected_{false}; private: // Stores a pointer to a string literal (static storage duration). diff --git a/esphome/components/openthread/openthread_esp.cpp b/esphome/components/openthread/openthread_esp.cpp index 2af78b729f3..2296e32b7f7 100644 --- a/esphome/components/openthread/openthread_esp.cpp +++ b/esphome/components/openthread/openthread_esp.cpp @@ -175,6 +175,9 @@ void OpenThreadComponent::ot_main() { // Pass the existing dataset, or NULL which will use the preprocessor definitions ESP_ERROR_CHECK(esp_openthread_auto_start(dataset.mLength > 0 ? &dataset : nullptr)); + // Register state change callback to update connected_ reactively instead of polling + otSetStateChangedCallback(instance, OpenThreadComponent::on_state_changed_, this); + esp_openthread_launch_mainloop(); // Clean up