From 9a2fc8aa51ef2cffa6248489d7d9ccd81e2f1e47 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 7 Nov 2025 23:40:36 -0600 Subject: [PATCH 1/3] part --- esphome/components/sensor/sensor.cpp | 24 +++++--- esphome/components/sensor/sensor.h | 5 +- .../components/text_sensor/text_sensor.cpp | 22 ++++--- esphome/components/text_sensor/text_sensor.h | 6 +- esphome/core/helpers.h | 61 +++++++++++++++++++ 5 files changed, 95 insertions(+), 23 deletions(-) diff --git a/esphome/components/sensor/sensor.cpp b/esphome/components/sensor/sensor.cpp index 92da4345b7..651f8283e0 100644 --- a/esphome/components/sensor/sensor.cpp +++ b/esphome/components/sensor/sensor.cpp @@ -72,9 +72,9 @@ StateClass Sensor::get_state_class() { void Sensor::publish_state(float state) { this->raw_state = state; - if (this->raw_callback_) { - this->raw_callback_->call(state); - } + + // Call raw callbacks (before filters) + this->callbacks_.call_first(this->raw_count_, state); ESP_LOGV(TAG, "'%s': Received new state %f", this->name_.c_str(), state); @@ -85,12 +85,12 @@ void Sensor::publish_state(float state) { } } -void Sensor::add_on_state_callback(std::function &&callback) { this->callback_.add(std::move(callback)); } +void Sensor::add_on_state_callback(std::function &&callback) { + this->callbacks_.add_second(std::move(callback)); +} + void Sensor::add_on_raw_state_callback(std::function &&callback) { - if (!this->raw_callback_) { - this->raw_callback_ = make_unique>(); - } - this->raw_callback_->add(std::move(callback)); + this->callbacks_.add_first(std::move(callback), &this->raw_count_); } void Sensor::add_filter(Filter *filter) { @@ -130,7 +130,13 @@ void Sensor::internal_send_state_to_frontend(float state) { this->state = state; ESP_LOGD(TAG, "'%s': Sending state %.5f %s with %d decimals of accuracy", this->get_name().c_str(), state, this->get_unit_of_measurement_ref().c_str(), this->get_accuracy_decimals()); - this->callback_.call(state); + + // Call filtered callbacks (after filters) + this->callbacks_.call_second(this->raw_count_, state); + +#if defined(USE_SENSOR) && defined(USE_CONTROLLER_REGISTRY) + ControllerRegistry::notify_sensor_update(this); +#endif } } // namespace sensor diff --git a/esphome/components/sensor/sensor.h b/esphome/components/sensor/sensor.h index a4210e5e6c..42e540a349 100644 --- a/esphome/components/sensor/sensor.h +++ b/esphome/components/sensor/sensor.h @@ -124,8 +124,7 @@ class Sensor : public EntityBase, public EntityBase_DeviceClass, public EntityBa void internal_send_state_to_frontend(float state); protected: - std::unique_ptr> raw_callback_; ///< Storage for raw state callbacks (lazy allocated). - CallbackManager callback_; ///< Storage for filtered state callbacks. + PartitionedCallbackManager callbacks_; Filter *filter_list_{nullptr}; ///< Store all active filters. @@ -140,6 +139,8 @@ class Sensor : public EntityBase, public EntityBase_DeviceClass, public EntityBa uint8_t force_update : 1; uint8_t reserved : 5; // Reserved for future use } sensor_flags_{}; + + uint8_t raw_count_{0}; ///< Number of raw callbacks (partition point in callbacks_ vector) }; } // namespace sensor diff --git a/esphome/components/text_sensor/text_sensor.cpp b/esphome/components/text_sensor/text_sensor.cpp index 0294d65861..57e9fd4259 100644 --- a/esphome/components/text_sensor/text_sensor.cpp +++ b/esphome/components/text_sensor/text_sensor.cpp @@ -24,9 +24,9 @@ void log_text_sensor(const char *tag, const char *prefix, const char *type, Text void TextSensor::publish_state(const std::string &state) { this->raw_state = state; - if (this->raw_callback_) { - this->raw_callback_->call(state); - } + + // Call raw callbacks (before filters) + this->callbacks_.call_first(this->raw_count_, state); ESP_LOGV(TAG, "'%s': Received new state %s", this->name_.c_str(), state.c_str()); @@ -68,13 +68,11 @@ void TextSensor::clear_filters() { } void TextSensor::add_on_state_callback(std::function callback) { - this->callback_.add(std::move(callback)); + this->callbacks_.add_second(std::move(callback)); } + void TextSensor::add_on_raw_state_callback(std::function callback) { - if (!this->raw_callback_) { - this->raw_callback_ = make_unique>(); - } - this->raw_callback_->add(std::move(callback)); + this->callbacks_.add_first(std::move(callback), &this->raw_count_); } std::string TextSensor::get_state() const { return this->state; } @@ -83,7 +81,13 @@ void TextSensor::internal_send_state_to_frontend(const std::string &state) { this->state = state; this->set_has_state(true); ESP_LOGD(TAG, "'%s': Sending state '%s'", this->name_.c_str(), state.c_str()); - this->callback_.call(state); + + // Call filtered callbacks (after filters) + this->callbacks_.call_second(this->raw_count_, state); + +#if defined(USE_TEXT_SENSOR) && defined(USE_CONTROLLER_REGISTRY) + ControllerRegistry::notify_text_sensor_update(this); +#endif } } // namespace text_sensor diff --git a/esphome/components/text_sensor/text_sensor.h b/esphome/components/text_sensor/text_sensor.h index db2e857ae3..1f4f3170e0 100644 --- a/esphome/components/text_sensor/text_sensor.h +++ b/esphome/components/text_sensor/text_sensor.h @@ -58,11 +58,11 @@ class TextSensor : public EntityBase, public EntityBase_DeviceClass { void internal_send_state_to_frontend(const std::string &state); protected: - std::unique_ptr> - raw_callback_; ///< Storage for raw state callbacks (lazy allocated). - CallbackManager callback_; ///< Storage for filtered state callbacks. + PartitionedCallbackManager callbacks_; Filter *filter_list_{nullptr}; ///< Store all active filters. + + uint8_t raw_count_{0}; ///< Number of raw callbacks (partition point in callbacks_ vector) }; } // namespace text_sensor diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 48af7f674a..732a8a66af 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -869,6 +869,67 @@ template class CallbackManager { std::vector> callbacks_; }; +template class PartitionedCallbackManager; + +/** Helper class for callbacks partitioned into two sections. + * + * Uses a single vector partitioned into two sections: [first_0, ..., first_m-1, second_0, ..., second_n-1] + * The partition point is tracked externally by the caller (typically stored in the entity class for optimal alignment). + * + * Memory efficient: Only stores a 4-byte pointer. The partition count lives in the entity class where it can be + * packed with other small fields to avoid padding waste. + * + * @tparam Ts The arguments for the callbacks, wrapped in void(). + */ +template class PartitionedCallbackManager { + public: + /// Add a callback to the first partition. + void add_first(std::function &&callback, uint8_t *first_count) { + if (!this->callbacks_) { + this->callbacks_ = make_unique>>(); + } + + // Add to first partition: append then swap into position + this->callbacks_->push_back(std::move(callback)); + if (*first_count < this->callbacks_->size() - 1) { + std::swap((*this->callbacks_)[*first_count], (*this->callbacks_)[this->callbacks_->size() - 1]); + } + (*first_count)++; + } + + /// Add a callback to the second partition. + void add_second(std::function &&callback) { + if (!this->callbacks_) { + this->callbacks_ = make_unique>>(); + } + + // Add to second partition: just append (already at end after first partition) + this->callbacks_->push_back(std::move(callback)); + } + + /// Call all callbacks in the first partition. + void call_first(uint8_t first_count, Ts... args) { + if (this->callbacks_) { + for (size_t i = 0; i < first_count; i++) { + (*this->callbacks_)[i](args...); + } + } + } + + /// Call all callbacks in the second partition. + void call_second(uint8_t first_count, Ts... args) { + if (this->callbacks_) { + for (size_t i = first_count; i < this->callbacks_->size(); i++) { + (*this->callbacks_)[i](args...); + } + } + } + + protected: + /// Partitioned callback storage: [first_0, ..., first_m-1, second_0, ..., second_n-1] + std::unique_ptr>> callbacks_; +}; + /// Helper class to deduplicate items in a series of values. template class Deduplicator { public: From 6feaa8dd1393deb2be259f65d04d2b98a0d85aed Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 9 Nov 2025 23:10:06 -0600 Subject: [PATCH 2/3] preserve order --- esphome/core/helpers.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 732a8a66af..4588c759e8 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -889,10 +889,11 @@ template class PartitionedCallbackManager { this->callbacks_ = make_unique>>(); } - // Add to first partition: append then swap into position + // Add to first partition: append then rotate into position this->callbacks_->push_back(std::move(callback)); if (*first_count < this->callbacks_->size() - 1) { - std::swap((*this->callbacks_)[*first_count], (*this->callbacks_)[this->callbacks_->size() - 1]); + // Use std::rotate to maintain registration order in second partition + std::rotate(this->callbacks_->begin() + *first_count, this->callbacks_->end() - 1, this->callbacks_->end()); } (*first_count)++; } From 0f136a888cb8c94a4360be48569f1959ae0833e4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 9 Nov 2025 23:19:02 -0600 Subject: [PATCH 3/3] Merge branch 'dev' into parition_callbacks and address Copilot review - Resolved conflicts in sensor.cpp and text_sensor.cpp to keep the PartitionedCallbackManager approach from this branch - Fixed platform-dependent pointer size documentation (4 bytes on 32-bit, 8 bytes on 64-bit) - Fixed potential integer underflow in add_first comparison - Added documentation explaining asymmetric API design rationale --- esphome/core/helpers.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 732a8a66af..d61edd20e2 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -876,8 +876,12 @@ template class PartitionedCallbackManager; * Uses a single vector partitioned into two sections: [first_0, ..., first_m-1, second_0, ..., second_n-1] * The partition point is tracked externally by the caller (typically stored in the entity class for optimal alignment). * - * Memory efficient: Only stores a 4-byte pointer. The partition count lives in the entity class where it can be - * packed with other small fields to avoid padding waste. + * Memory efficient: Only stores a single pointer (4 bytes on 32-bit platforms, 8 bytes on 64-bit platforms). + * The partition count lives in the entity class where it can be packed with other small fields to avoid padding waste. + * + * Design rationale: The asymmetric API (add_first takes first_count*, while call_first/call_second take it by value) + * is intentional - add_first must increment the count, while call methods only read it. This avoids storing first_count + * internally, saving memory per instance. * * @tparam Ts The arguments for the callbacks, wrapped in void(). */ @@ -891,7 +895,8 @@ template class PartitionedCallbackManager { // Add to first partition: append then swap into position this->callbacks_->push_back(std::move(callback)); - if (*first_count < this->callbacks_->size() - 1) { + // Avoid potential underflow: rewrite comparison to not subtract from size() + if (*first_count + 1 < this->callbacks_->size()) { std::swap((*this->callbacks_)[*first_count], (*this->callbacks_)[this->callbacks_->size() - 1]); } (*first_count)++;