Fix regression: BK7231N devices not returning entities via API (#9283)

esphome/components/api/api_connection.cpp

@@ -1687,7 +1687,9 @@ void APIConnection::DeferredBatch::add_item(EntityBase *entity, MessageCreator c
   // O(n) but optimized for RAM and not performance.
   for (auto &item : items) {
     if (item.entity == entity && item.message_type == message_type) {
-      // Update the existing item with the new creator
+      // Clean up old creator before replacing
+      item.creator.cleanup(message_type);
+      // Move assign the new creator
       item.creator = std::move(creator);
       return;
     }
@@ -1730,11 +1732,11 @@ void APIConnection::process_batch_() {
     return;
   }
 
-  size_t num_items = this->deferred_batch_.items.size();
+  size_t num_items = this->deferred_batch_.size();
 
   // Fast path for single message - allocate exact size needed
   if (num_items == 1) {
-    const auto &item = this->deferred_batch_.items[0];
+    const auto &item = this->deferred_batch_[0];
 
     // Let the creator calculate size and encode if it fits
     uint16_t payload_size =
@@ -1764,7 +1766,8 @@ void APIConnection::process_batch_() {
 
   // Pre-calculate exact buffer size needed based on message types
   uint32_t total_estimated_size = 0;
-  for (const auto &item : this->deferred_batch_.items) {
+  for (size_t i = 0; i < this->deferred_batch_.size(); i++) {
+    const auto &item = this->deferred_batch_[i];
     total_estimated_size += get_estimated_message_size(item.message_type);
   }
 
@@ -1785,7 +1788,8 @@ void APIConnection::process_batch_() {
   uint32_t current_offset = 0;
 
   // Process items and encode directly to buffer
-  for (const auto &item : this->deferred_batch_.items) {
+  for (size_t i = 0; i < this->deferred_batch_.size(); i++) {
+    const auto &item = this->deferred_batch_[i];
     // Try to encode message
     // The creator will calculate overhead to determine if the message fits
     uint16_t payload_size = item.creator(item.entity, this, remaining_size, false, item.message_type);
@@ -1840,17 +1844,15 @@ void APIConnection::process_batch_() {
   // Log messages after send attempt for VV debugging
   // It's safe to use the buffer for logging at this point regardless of send result
   for (size_t i = 0; i < items_processed; i++) {
-    const auto &item = this->deferred_batch_.items[i];
+    const auto &item = this->deferred_batch_[i];
     this->log_batch_item_(item);
   }
 #endif
 
   // Handle remaining items more efficiently
-  if (items_processed < this->deferred_batch_.items.size()) {
-    // Remove processed items from the beginning
-    this->deferred_batch_.items.erase(this->deferred_batch_.items.begin(),
-                                      this->deferred_batch_.items.begin() + items_processed);
-
+  if (items_processed < this->deferred_batch_.size()) {
+    // Remove processed items from the beginning with proper cleanup
+    this->deferred_batch_.remove_front(items_processed);
     // Reschedule for remaining items
     this->schedule_batch_();
   } else {
@@ -1861,23 +1863,16 @@ void APIConnection::process_batch_() {
 
 uint16_t APIConnection::MessageCreator::operator()(EntityBase *entity, APIConnection *conn, uint32_t remaining_size,
                                                    bool is_single, uint16_t message_type) const {
-  if (has_tagged_string_ptr_()) {
-    // Handle string-based messages
-    switch (message_type) {
 #ifdef USE_EVENT
-      case EventResponse::MESSAGE_TYPE: {
-        auto *e = static_cast<event::Event *>(entity);
-        return APIConnection::try_send_event_response(e, *get_string_ptr_(), conn, remaining_size, is_single);
-      }
-#endif
-      default:
-        // Should not happen, return 0 to indicate no message
-        return 0;
-    }
-  } else {
-    // Function pointer case
-    return data_.ptr(entity, conn, remaining_size, is_single);
+  // Special case: EventResponse uses string pointer
+  if (message_type == EventResponse::MESSAGE_TYPE) {
+    auto *e = static_cast<event::Event *>(entity);
+    return APIConnection::try_send_event_response(e, *data_.string_ptr, conn, remaining_size, is_single);
   }
+#endif
+
+  // All other message types use function pointers
+  return data_.function_ptr(entity, conn, remaining_size, is_single);
 }
 
 uint16_t APIConnection::try_send_list_info_done(EntityBase *entity, APIConnection *conn, uint32_t remaining_size,

esphome/components/api/api_connection.h

@@ -451,96 +451,53 @@ class APIConnection : public APIServerConnection {
   // Function pointer type for message encoding
   using MessageCreatorPtr = uint16_t (*)(EntityBase *, APIConnection *, uint32_t remaining_size, bool is_single);
 
-  // Optimized MessageCreator class using tagged pointer
   class MessageCreator {
-    // Ensure pointer alignment allows LSB tagging
-    static_assert(alignof(std::string *) > 1, "String pointer alignment must be > 1 for LSB tagging");
-
    public:
     // Constructor for function pointer
-    MessageCreator(MessageCreatorPtr ptr) {
-      // Function pointers are always aligned, so LSB is 0
-      data_.ptr = ptr;
-    }
+    MessageCreator(MessageCreatorPtr ptr) { data_.function_ptr = ptr; }
 
     // Constructor for string state capture
-    explicit MessageCreator(const std::string &str_value) {
-      // Allocate string and tag the pointer
-      auto *str = new std::string(str_value);
-      // Set LSB to 1 to indicate string pointer
-      data_.tagged = reinterpret_cast<uintptr_t>(str) | 1;
-    }
+    explicit MessageCreator(const std::string &str_value) { data_.string_ptr = new std::string(str_value); }
 
-    // Destructor
-    ~MessageCreator() {
-      if (has_tagged_string_ptr_()) {
-        delete get_string_ptr_();
-      }
-    }
+    // No destructor - cleanup must be called explicitly with message_type
 
-    // Copy constructor
-    MessageCreator(const MessageCreator &other) {
-      if (other.has_tagged_string_ptr_()) {
-        auto *str = new std::string(*other.get_string_ptr_());
-        data_.tagged = reinterpret_cast<uintptr_t>(str) | 1;
-      } else {
-        data_ = other.data_;
-      }
-    }
+    // Delete copy operations - MessageCreator should only be moved
+    MessageCreator(const MessageCreator &other) = delete;
+    MessageCreator &operator=(const MessageCreator &other) = delete;
 
     // Move constructor
-    MessageCreator(MessageCreator &&other) noexcept : data_(other.data_) { other.data_.ptr = nullptr; }
-
-    // Assignment operators (needed for batch deduplication)
-    MessageCreator &operator=(const MessageCreator &other) {
-      if (this != &other) {
-        // Clean up current string data if needed
-        if (has_tagged_string_ptr_()) {
-          delete get_string_ptr_();
-        }
-        // Copy new data
-        if (other.has_tagged_string_ptr_()) {
-          auto *str = new std::string(*other.get_string_ptr_());
-          data_.tagged = reinterpret_cast<uintptr_t>(str) | 1;
-        } else {
-          data_ = other.data_;
-        }
-      }
-      return *this;
-    }
+    MessageCreator(MessageCreator &&other) noexcept : data_(other.data_) { other.data_.function_ptr = nullptr; }
 
+    // Move assignment
     MessageCreator &operator=(MessageCreator &&other) noexcept {
       if (this != &other) {
-        // Clean up current string data if needed
-        if (has_tagged_string_ptr_()) {
-          delete get_string_ptr_();
-        }
-        // Move data
+        // IMPORTANT: Caller must ensure cleanup() was called if this contains a string!
+        // In our usage, this happens in add_item() deduplication and vector::erase()
         data_ = other.data_;
-        // Reset other to safe state
-        other.data_.ptr = nullptr;
+        other.data_.function_ptr = nullptr;
       }
       return *this;
     }
 
-    // Call operator - now accepts message_type as parameter
+    // Call operator - uses message_type to determine union type
     uint16_t operator()(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, bool is_single,
                         uint16_t message_type) const;
 
-   private:
-    // Check if this contains a string pointer
-    bool has_tagged_string_ptr_() const { return (data_.tagged & 1) != 0; }
-
-    // Get the actual string pointer (clears the tag bit)
-    std::string *get_string_ptr_() const {
-      // NOLINTNEXTLINE(performance-no-int-to-ptr)
-      return reinterpret_cast<std::string *>(data_.tagged & ~uintptr_t(1));
+    // Manual cleanup method - must be called before destruction for string types
+    void cleanup(uint16_t message_type) {
+#ifdef USE_EVENT
+      if (message_type == EventResponse::MESSAGE_TYPE && data_.string_ptr != nullptr) {
+        delete data_.string_ptr;
+        data_.string_ptr = nullptr;
+      }
+#endif
     }
 
-    union {
-      MessageCreatorPtr ptr;
-      uintptr_t tagged;
-    } data_;  // 4 bytes on 32-bit
+   private:
+    union Data {
+      MessageCreatorPtr function_ptr;
+      std::string *string_ptr;
+    } data_;  // 4 bytes on 32-bit, 8 bytes on 64-bit - same as before
   };
 
   // Generic batching mechanism for both state updates and entity info
@@ -558,20 +515,46 @@ class APIConnection : public APIServerConnection {
     std::vector<BatchItem> items;
     uint32_t batch_start_time{0};
 
+   private:
+    // Helper to cleanup items from the beginning
+    void cleanup_items_(size_t count) {
+      for (size_t i = 0; i < count; i++) {
+        items[i].creator.cleanup(items[i].message_type);
+      }
+    }
+
+   public:
     DeferredBatch() {
       // Pre-allocate capacity for typical batch sizes to avoid reallocation
       items.reserve(8);
     }
 
+    ~DeferredBatch() {
+      // Ensure cleanup of any remaining items
+      clear();
+    }
+
     // Add item to the batch
     void add_item(EntityBase *entity, MessageCreator creator, uint16_t message_type);
     // Add item to the front of the batch (for high priority messages like ping)
     void add_item_front(EntityBase *entity, MessageCreator creator, uint16_t message_type);
+
+    // Clear all items with proper cleanup
     void clear() {
+      cleanup_items_(items.size());
       items.clear();
       batch_start_time = 0;
     }
+
+    // Remove processed items from the front with proper cleanup
+    void remove_front(size_t count) {
+      cleanup_items_(count);
+      items.erase(items.begin(), items.begin() + count);
+    }
+
     bool empty() const { return items.empty(); }
+    size_t size() const { return items.size(); }
+    const BatchItem &operator[](size_t index) const { return items[index]; }
   };
 
   // DeferredBatch here (16 bytes, 4-byte aligned)