From fb3ce6c783d36f1b7c034616bc7813e725ecf946 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 26 Sep 2025 09:43:36 -0500 Subject: [PATCH 1/4] bot comments --- esphome/components/esp32_ble_server/ble_characteristic.cpp | 6 ++---- .../components/esp32_ble_server/ble_server_automations.cpp | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/esphome/components/esp32_ble_server/ble_characteristic.cpp b/esphome/components/esp32_ble_server/ble_characteristic.cpp index 4d0ada0ac2..fabcc75321 100644 --- a/esphome/components/esp32_ble_server/ble_characteristic.cpp +++ b/esphome/components/esp32_ble_server/ble_characteristic.cpp @@ -313,10 +313,8 @@ void BLECharacteristic::remove_client_from_notify_list_(uint16_t conn_id) { // for the common case by swapping with the last element and popping for (size_t i = 0; i < this->clients_to_notify_.size(); i++) { if (this->clients_to_notify_[i].conn_id == conn_id) { - // Swap with last element and pop - if (i != this->clients_to_notify_.size() - 1) { - this->clients_to_notify_[i] = this->clients_to_notify_.back(); - } + // Swap with last element and pop (safe even when i is the last element) + this->clients_to_notify_[i] = this->clients_to_notify_.back(); this->clients_to_notify_.pop_back(); return; } diff --git a/esphome/components/esp32_ble_server/ble_server_automations.cpp b/esphome/components/esp32_ble_server/ble_server_automations.cpp index ea6a074daa..b140e08b46 100644 --- a/esphome/components/esp32_ble_server/ble_server_automations.cpp +++ b/esphome/components/esp32_ble_server/ble_server_automations.cpp @@ -83,10 +83,8 @@ void BLECharacteristicSetValueActionManager::remove_listener_(BLECharacteristic // Since we typically have very few listeners, optimize by swapping with back and popping for (size_t i = 0; i < this->listeners_.size(); i++) { if (this->listeners_[i].characteristic == characteristic) { - // Swap with last element and pop - if (i != this->listeners_.size() - 1) { - this->listeners_[i] = this->listeners_.back(); - } + // Swap with last element and pop (safe even when i is the last element) + this->listeners_[i] = this->listeners_.back(); this->listeners_.pop_back(); return; } From 70685f2939532aa5c7b834d7335f66da0638965a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 26 Sep 2025 09:46:38 -0500 Subject: [PATCH 2/4] bot comments --- esphome/components/esp32_ble_server/ble_server_automations.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/esphome/components/esp32_ble_server/ble_server_automations.h b/esphome/components/esp32_ble_server/ble_server_automations.h index 54bc0f2632..910335826c 100644 --- a/esphome/components/esp32_ble_server/ble_server_automations.h +++ b/esphome/components/esp32_ble_server/ble_server_automations.h @@ -20,6 +20,9 @@ namespace esp32_ble_server_automations { using namespace esp32_ble; using namespace event_emitter; +// Invalid listener ID constant - 0 is used as sentinel value in EventEmitter +static constexpr EventEmitterListenerID INVALID_LISTENER_ID = 0; + class BLETriggers { public: static Trigger, uint16_t> *create_characteristic_on_write_trigger( @@ -50,7 +53,7 @@ class BLECharacteristicSetValueActionManager return entry.listener_id; } } - return 0; + return INVALID_LISTENER_ID; } void emit_pre_notify(BLECharacteristic *characteristic) { this->emit_(BLECharacteristicSetValueActionEvt::PRE_NOTIFY, characteristic); From eeff69d50bc65d1faa51d5f5b4d3503e13c08f87 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 26 Sep 2025 10:14:34 -0500 Subject: [PATCH 3/4] [event_emitter] Replace unordered_map with vector - saves 2.6KB flash, 2.3x faster --- .../components/event_emitter/event_emitter.h | 97 +++++++++++++++---- 1 file changed, 76 insertions(+), 21 deletions(-) diff --git a/esphome/components/event_emitter/event_emitter.h b/esphome/components/event_emitter/event_emitter.h index 3876a2cc14..a810187922 100644 --- a/esphome/components/event_emitter/event_emitter.h +++ b/esphome/components/event_emitter/event_emitter.h @@ -1,5 +1,4 @@ #pragma once -#include #include #include #include @@ -10,6 +9,7 @@ namespace esphome { namespace event_emitter { using EventEmitterListenerID = uint32_t; +static constexpr EventEmitterListenerID INVALID_LISTENER_ID = 0; void raise_event_emitter_full_error(); // EventEmitter class that can emit events with a specific name (it is highly recommended to use an enum class for this) @@ -17,45 +17,100 @@ void raise_event_emitter_full_error(); template class EventEmitter { public: EventEmitterListenerID on(EvtType event, std::function listener) { - EventEmitterListenerID listener_id = get_next_id_(event); - listeners_[event][listener_id] = listener; + EventEmitterListenerID listener_id = get_next_id_(); + + // Find or create event entry + EventEntry *entry = find_or_create_event_(event); + entry->listeners.push_back({listener_id, listener}); + return listener_id; } void off(EvtType event, EventEmitterListenerID id) { - if (listeners_.count(event) == 0) + EventEntry *entry = find_event_(event); + if (entry == nullptr) return; - listeners_[event].erase(id); + + // Remove listener with given id + for (auto it = entry->listeners.begin(); it != entry->listeners.end(); ++it) { + if (it->id == id) { + // Swap with last and pop for efficient removal + *it = entry->listeners.back(); + entry->listeners.pop_back(); + + // Remove event entry if no more listeners + if (entry->listeners.empty()) { + remove_event_(event); + } + return; + } + } } protected: void emit_(EvtType event, Args... args) { - if (listeners_.count(event) == 0) + EventEntry *entry = find_event_(event); + if (entry == nullptr) return; - for (const auto &listener : listeners_[event]) { - listener.second(args...); + + // Call all listeners for this event + for (const auto &listener : entry->listeners) { + listener.callback(args...); } } - EventEmitterListenerID get_next_id_(EvtType event) { - // Check if the map is full - if (listeners_[event].size() == std::numeric_limits::max()) { - // Raise an error if the map is full - raise_event_emitter_full_error(); - off(event, 0); - return 0; + private: + struct Listener { + EventEmitterListenerID id; + std::function callback; + }; + + struct EventEntry { + EvtType event; + std::vector listeners; + }; + + EventEntry *find_event_(EvtType event) { + for (auto &entry : events_) { + if (entry.event == event) { + return &entry; + } } - // Get the next ID for the given event. - EventEmitterListenerID next_id = (current_id_ + 1) % std::numeric_limits::max(); - while (listeners_[event].count(next_id) > 0) { - next_id = (next_id + 1) % std::numeric_limits::max(); + return nullptr; + } + + EventEntry *find_or_create_event_(EvtType event) { + EventEntry *entry = find_event_(event); + if (entry != nullptr) + return entry; + + // Create new event entry + events_.push_back({event, {}}); + return &events_.back(); + } + + void remove_event_(EvtType event) { + for (auto it = events_.begin(); it != events_.end(); ++it) { + if (it->event == event) { + // Swap with last and pop + *it = events_.back(); + events_.pop_back(); + return; + } + } + } + + EventEmitterListenerID get_next_id_() { + // Simple incrementing ID, wrapping around at max + EventEmitterListenerID next_id = (current_id_ + 1); + if (next_id == 0) { // Skip 0 as it's often used as "invalid" + next_id = 1; } current_id_ = next_id; return current_id_; } - private: - std::unordered_map>> listeners_; + std::vector events_; EventEmitterListenerID current_id_ = 0; }; From 4110d926dd8320d44a68bcc52d31c7ffbb1f34d5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 26 Sep 2025 10:17:48 -0500 Subject: [PATCH 4/4] preen --- esphome/components/event_emitter/event_emitter.cpp | 14 -------------- esphome/components/event_emitter/event_emitter.h | 1 - 2 files changed, 15 deletions(-) delete mode 100644 esphome/components/event_emitter/event_emitter.cpp diff --git a/esphome/components/event_emitter/event_emitter.cpp b/esphome/components/event_emitter/event_emitter.cpp deleted file mode 100644 index 8487e19c2f..0000000000 --- a/esphome/components/event_emitter/event_emitter.cpp +++ /dev/null @@ -1,14 +0,0 @@ -#include "event_emitter.h" - -namespace esphome { -namespace event_emitter { - -static const char *const TAG = "event_emitter"; - -void raise_event_emitter_full_error() { - ESP_LOGE(TAG, "EventEmitter has reached the maximum number of listeners for event"); - ESP_LOGW(TAG, "Removing listener to make space for new listener"); -} - -} // namespace event_emitter -} // namespace esphome diff --git a/esphome/components/event_emitter/event_emitter.h b/esphome/components/event_emitter/event_emitter.h index a810187922..4ea8930a33 100644 --- a/esphome/components/event_emitter/event_emitter.h +++ b/esphome/components/event_emitter/event_emitter.h @@ -10,7 +10,6 @@ namespace event_emitter { using EventEmitterListenerID = uint32_t; static constexpr EventEmitterListenerID INVALID_LISTENER_ID = 0; -void raise_event_emitter_full_error(); // EventEmitter class that can emit events with a specific name (it is highly recommended to use an enum class for this) // and a list of arguments. Supports multiple listeners for each event.