From 8a15c18066d9d65515c81e8de3d5521a90d908f5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 13 Oct 2025 17:05:13 -1000 Subject: [PATCH] [bluetooth_proxy] Use FixedVector for GATT characteristics and descriptors (#11214) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- esphome/components/api/api.proto | 4 +- esphome/components/api/api_options.proto | 6 + esphome/components/api/api_pb2.h | 4 +- esphome/components/api/proto.h | 26 +++- .../bluetooth_proxy/bluetooth_connection.cpp | 13 +- esphome/core/helpers.h | 121 ++++++++++++++++-- script/api_protobuf/api_protobuf.py | 4 + 7 files changed, 146 insertions(+), 32 deletions(-) diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index 87f477799d..9b714d00f1 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -1519,7 +1519,7 @@ message BluetoothGATTCharacteristic { repeated uint64 uuid = 1 [(fixed_array_size) = 2, (fixed_array_skip_zero) = true]; uint32 handle = 2; uint32 properties = 3; - repeated BluetoothGATTDescriptor descriptors = 4; + repeated BluetoothGATTDescriptor descriptors = 4 [(fixed_vector) = true]; // New field for efficient UUID (v1.12+) // Only one of uuid or short_uuid will be set. @@ -1531,7 +1531,7 @@ message BluetoothGATTCharacteristic { message BluetoothGATTService { repeated uint64 uuid = 1 [(fixed_array_size) = 2, (fixed_array_skip_zero) = true]; uint32 handle = 2; - repeated BluetoothGATTCharacteristic characteristics = 3; + repeated BluetoothGATTCharacteristic characteristics = 3 [(fixed_vector) = true]; // New field for efficient UUID (v1.12+) // Only one of uuid or short_uuid will be set. diff --git a/esphome/components/api/api_options.proto b/esphome/components/api/api_options.proto index 633f39b552..ead8ac0bbc 100644 --- a/esphome/components/api/api_options.proto +++ b/esphome/components/api/api_options.proto @@ -64,4 +64,10 @@ extend google.protobuf.FieldOptions { // This is typically done through methods returning const T& or special accessor // methods like get_options() or supported_modes_for_api_(). optional string container_pointer = 50001; + + // fixed_vector: Use FixedVector instead of std::vector for repeated fields + // When set, the repeated field will use FixedVector which requires calling + // init(size) before adding elements. This eliminates std::vector template overhead + // and is ideal when the exact size is known before populating the array. + optional bool fixed_vector = 50013 [default=false]; } diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index d9e68ece9b..1798458393 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -1923,7 +1923,7 @@ class BluetoothGATTCharacteristic final : public ProtoMessage { std::array uuid{}; uint32_t handle{0}; uint32_t properties{0}; - std::vector descriptors{}; + FixedVector descriptors{}; uint32_t short_uuid{0}; void encode(ProtoWriteBuffer buffer) const override; void calculate_size(ProtoSize &size) const override; @@ -1937,7 +1937,7 @@ class BluetoothGATTService final : public ProtoMessage { public: std::array uuid{}; uint32_t handle{0}; - std::vector characteristics{}; + FixedVector characteristics{}; uint32_t short_uuid{0}; void encode(ProtoWriteBuffer buffer) const override; void calculate_size(ProtoSize &size) const override; diff --git a/esphome/components/api/proto.h b/esphome/components/api/proto.h index 9d780692ec..a6a09bf7c5 100644 --- a/esphome/components/api/proto.h +++ b/esphome/components/api/proto.h @@ -749,13 +749,29 @@ class ProtoSize { template inline void add_repeated_message(uint32_t field_id_size, const std::vector &messages) { // Skip if the vector is empty - if (messages.empty()) { - return; + if (!messages.empty()) { + // Use the force version for all messages in the repeated field + for (const auto &message : messages) { + add_message_object_force(field_id_size, message); + } } + } - // Use the force version for all messages in the repeated field - for (const auto &message : messages) { - add_message_object_force(field_id_size, message); + /** + * @brief Calculates and adds the sizes of all messages in a repeated field to the total message size (FixedVector + * version) + * + * @tparam MessageType The type of the nested messages in the FixedVector + * @param messages FixedVector of message objects + */ + template + inline void add_repeated_message(uint32_t field_id_size, const FixedVector &messages) { + // Skip if the fixed vector is empty + if (!messages.empty()) { + // Use the force version for all messages in the repeated field + for (const auto &message : messages) { + add_message_object_force(field_id_size, message); + } } } }; diff --git a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp index cde82fbfb0..fcc344dda9 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp @@ -230,8 +230,8 @@ void BluetoothConnection::send_service_for_discovery_() { service_resp.handle = service_result.start_handle; if (total_char_count > 0) { - // Reserve space and process characteristics - service_resp.characteristics.reserve(total_char_count); + // Initialize FixedVector with exact count and process characteristics + service_resp.characteristics.init(total_char_count); uint16_t char_offset = 0; esp_gattc_char_elem_t char_result; while (true) { // characteristics @@ -253,9 +253,7 @@ void BluetoothConnection::send_service_for_discovery_() { service_resp.characteristics.emplace_back(); auto &characteristic_resp = service_resp.characteristics.back(); - fill_gatt_uuid(characteristic_resp.uuid, characteristic_resp.short_uuid, char_result.uuid, use_efficient_uuids); - characteristic_resp.handle = char_result.char_handle; characteristic_resp.properties = char_result.properties; char_offset++; @@ -271,12 +269,11 @@ void BluetoothConnection::send_service_for_discovery_() { return; } if (total_desc_count == 0) { - // No descriptors, continue to next characteristic continue; } - // Reserve space and process descriptors - characteristic_resp.descriptors.reserve(total_desc_count); + // Initialize FixedVector with exact count and process descriptors + characteristic_resp.descriptors.init(total_desc_count); uint16_t desc_offset = 0; esp_gattc_descr_elem_t desc_result; while (true) { // descriptors @@ -297,9 +294,7 @@ void BluetoothConnection::send_service_for_discovery_() { characteristic_resp.descriptors.emplace_back(); auto &descriptor_resp = characteristic_resp.descriptors.back(); - fill_gatt_uuid(descriptor_resp.uuid, descriptor_resp.short_uuid, desc_result.uuid, use_efficient_uuids); - descriptor_resp.handle = desc_result.handle; desc_offset++; } diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index ed1d5ac108..e352c9c415 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -168,26 +168,83 @@ template class FixedVector { size_t size_{0}; size_t capacity_{0}; - public: - FixedVector() = default; - - ~FixedVector() { - if (data_ != nullptr) { - delete[] data_; + // Helper to destroy all elements without freeing memory + void destroy_elements_() { + // Only call destructors for non-trivially destructible types + if constexpr (!std::is_trivially_destructible::value) { + for (size_t i = 0; i < size_; i++) { + data_[i].~T(); + } } } - // Disable copy to avoid accidental copies + // Helper to destroy elements and free memory + void cleanup_() { + if (data_ != nullptr) { + destroy_elements_(); + // Free raw memory + ::operator delete(data_); + } + } + + // Helper to reset pointers after cleanup + void reset_() { + data_ = nullptr; + capacity_ = 0; + size_ = 0; + } + + public: + FixedVector() = default; + + ~FixedVector() { cleanup_(); } + + // Disable copy operations (avoid accidental expensive copies) FixedVector(const FixedVector &) = delete; FixedVector &operator=(const FixedVector &) = delete; - // Allocate capacity - can only be called once on empty vector - void init(size_t n) { - if (data_ == nullptr && n > 0) { - data_ = new T[n]; - capacity_ = n; - size_ = 0; + // Enable move semantics (allows use in move-only containers like std::vector) + FixedVector(FixedVector &&other) noexcept : data_(other.data_), size_(other.size_), capacity_(other.capacity_) { + other.reset_(); + } + + FixedVector &operator=(FixedVector &&other) noexcept { + if (this != &other) { + // Delete our current data + cleanup_(); + // Take ownership of other's data + data_ = other.data_; + size_ = other.size_; + capacity_ = other.capacity_; + // Leave other in valid empty state + other.reset_(); } + return *this; + } + + // Allocate capacity - can be called multiple times to reinit + void init(size_t n) { + cleanup_(); + reset_(); + if (n > 0) { + // Allocate raw memory without calling constructors + // sizeof(T) is correct here for any type T (value types, pointers, etc.) + // NOLINTNEXTLINE(bugprone-sizeof-expression) + data_ = static_cast(::operator new(n * sizeof(T))); + capacity_ = n; + } + } + + // Clear the vector (destroy all elements, reset size to 0, keep capacity) + void clear() { + destroy_elements_(); + size_ = 0; + } + + // Shrink capacity to fit current size (frees all memory) + void shrink_to_fit() { + cleanup_(); + reset_(); } /// Add element without bounds checking @@ -195,16 +252,52 @@ template class FixedVector { /// Silently ignores pushes beyond capacity (no exception or assertion) void push_back(const T &value) { if (size_ < capacity_) { - data_[size_++] = value; + // Use placement new to construct the object in pre-allocated memory + new (&data_[size_]) T(value); + size_++; } } + /// Add element by move without bounds checking + /// Caller must ensure sufficient capacity was allocated via init() + /// Silently ignores pushes beyond capacity (no exception or assertion) + void push_back(T &&value) { + if (size_ < capacity_) { + // Use placement new to move-construct the object in pre-allocated memory + new (&data_[size_]) T(std::move(value)); + size_++; + } + } + + /// Emplace element without bounds checking - constructs in-place + /// Caller must ensure sufficient capacity was allocated via init() + /// Returns reference to the newly constructed element + /// NOTE: Caller MUST ensure size_ < capacity_ before calling + T &emplace_back() { + // Use placement new to default-construct the object in pre-allocated memory + new (&data_[size_]) T(); + size_++; + return data_[size_ - 1]; + } + + /// Access last element (no bounds checking - matches std::vector behavior) + /// Caller must ensure vector is not empty (size() > 0) + T &back() { return data_[size_ - 1]; } + const T &back() const { return data_[size_ - 1]; } + size_t size() const { return size_; } + bool empty() const { return size_ == 0; } /// Access element without bounds checking (matches std::vector behavior) /// Caller must ensure index is valid (i < size()) T &operator[](size_t i) { return data_[i]; } const T &operator[](size_t i) const { return data_[i]; } + + // Iterator support for range-based for loops + T *begin() { return data_; } + T *end() { return data_ + size_; } + const T *begin() const { return data_; } + const T *end() const { return data_ + size_; } }; ///@} diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 487c187372..9a55f1d136 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -1415,6 +1415,8 @@ class RepeatedTypeInfo(TypeInfo): # Check if this is a pointer field by looking for container_pointer option self._container_type = get_field_opt(field, pb.container_pointer, "") self._use_pointer = bool(self._container_type) + # Check if this should use FixedVector instead of std::vector + self._use_fixed_vector = get_field_opt(field, pb.fixed_vector, False) # For repeated fields, we need to get the base type info # but we can't call create_field_type_info as it would cause recursion @@ -1438,6 +1440,8 @@ class RepeatedTypeInfo(TypeInfo): if "<" in self._container_type and ">" in self._container_type: return f"const {self._container_type}*" return f"const {self._container_type}<{self._ti.cpp_type}>*" + if self._use_fixed_vector: + return f"FixedVector<{self._ti.cpp_type}>" return f"std::vector<{self._ti.cpp_type}>" @property