From 91e1a4ff7630b4fb6f50e970785cc8196ad0fa5a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 21 Jul 2025 12:49:48 -1000 Subject: [PATCH 01/13] fixed arrays --- esphome/components/api/api.proto | 8 +- esphome/components/api/api_pb2.cpp | 30 ++--- esphome/components/api/api_pb2.h | 14 +- .../bluetooth_proxy/bluetooth_connection.cpp | 28 ++-- script/api_protobuf/api_protobuf.py | 123 ++++++++++++++++++ 5 files changed, 163 insertions(+), 40 deletions(-) diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index e7c2fcaf8a..93e84702e2 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -1463,19 +1463,19 @@ message BluetoothGATTGetServicesRequest { } message BluetoothGATTDescriptor { - repeated uint64 uuid = 1; + repeated uint64 uuid = 1 [(fixed_array_size) = 2]; uint32 handle = 2; } message BluetoothGATTCharacteristic { - repeated uint64 uuid = 1; + repeated uint64 uuid = 1 [(fixed_array_size) = 2]; uint32 handle = 2; uint32 properties = 3; repeated BluetoothGATTDescriptor descriptors = 4; } message BluetoothGATTService { - repeated uint64 uuid = 1; + repeated uint64 uuid = 1 [(fixed_array_size) = 2]; uint32 handle = 2; repeated BluetoothGATTCharacteristic characteristics = 3; } @@ -1486,7 +1486,7 @@ message BluetoothGATTGetServicesResponse { option (ifdef) = "USE_BLUETOOTH_PROXY"; uint64 address = 1; - repeated BluetoothGATTService services = 2; + repeated BluetoothGATTService services = 2 [(fixed_array_size) = 1]; } message BluetoothGATTGetServicesDoneResponse { diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index 28d135ed6d..9d8b109986 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -1891,21 +1891,19 @@ bool BluetoothGATTGetServicesRequest::decode_varint(uint32_t field_id, ProtoVarI return true; } void BluetoothGATTDescriptor::encode(ProtoWriteBuffer buffer) const { - for (auto &it : this->uuid) { + for (const auto &it : this->uuid) { buffer.encode_uint64(1, it, true); } buffer.encode_uint32(2, this->handle); } void BluetoothGATTDescriptor::calculate_size(uint32_t &total_size) const { - if (!this->uuid.empty()) { - for (const auto &it : this->uuid) { - ProtoSize::add_uint64_field_repeated(total_size, 1, it); - } + for (const auto &it : this->uuid) { + ProtoSize::add_uint64_field_repeated(total_size, 1, it); } ProtoSize::add_uint32_field(total_size, 1, this->handle); } void BluetoothGATTCharacteristic::encode(ProtoWriteBuffer buffer) const { - for (auto &it : this->uuid) { + for (const auto &it : this->uuid) { buffer.encode_uint64(1, it, true); } buffer.encode_uint32(2, this->handle); @@ -1915,17 +1913,15 @@ void BluetoothGATTCharacteristic::encode(ProtoWriteBuffer buffer) const { } } void BluetoothGATTCharacteristic::calculate_size(uint32_t &total_size) const { - if (!this->uuid.empty()) { - for (const auto &it : this->uuid) { - ProtoSize::add_uint64_field_repeated(total_size, 1, it); - } + for (const auto &it : this->uuid) { + ProtoSize::add_uint64_field_repeated(total_size, 1, it); } ProtoSize::add_uint32_field(total_size, 1, this->handle); ProtoSize::add_uint32_field(total_size, 1, this->properties); ProtoSize::add_repeated_message(total_size, 1, this->descriptors); } void BluetoothGATTService::encode(ProtoWriteBuffer buffer) const { - for (auto &it : this->uuid) { + for (const auto &it : this->uuid) { buffer.encode_uint64(1, it, true); } buffer.encode_uint32(2, this->handle); @@ -1934,23 +1930,23 @@ void BluetoothGATTService::encode(ProtoWriteBuffer buffer) const { } } void BluetoothGATTService::calculate_size(uint32_t &total_size) const { - if (!this->uuid.empty()) { - for (const auto &it : this->uuid) { - ProtoSize::add_uint64_field_repeated(total_size, 1, it); - } + for (const auto &it : this->uuid) { + ProtoSize::add_uint64_field_repeated(total_size, 1, it); } ProtoSize::add_uint32_field(total_size, 1, this->handle); ProtoSize::add_repeated_message(total_size, 1, this->characteristics); } void BluetoothGATTGetServicesResponse::encode(ProtoWriteBuffer buffer) const { buffer.encode_uint64(1, this->address); - for (auto &it : this->services) { + for (const auto &it : this->services) { buffer.encode_message(2, it, true); } } void BluetoothGATTGetServicesResponse::calculate_size(uint32_t &total_size) const { ProtoSize::add_uint64_field(total_size, 1, this->address); - ProtoSize::add_repeated_message(total_size, 1, this->services); + for (const auto &it : this->services) { + ProtoSize::add_message_object_repeated(total_size, 1, it); + } } void BluetoothGATTGetServicesDoneResponse::encode(ProtoWriteBuffer buffer) const { buffer.encode_uint64(1, this->address); diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index 7255aa7903..7a9726f6e4 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -1796,7 +1796,8 @@ class BluetoothGATTGetServicesRequest : public ProtoDecodableMessage { }; class BluetoothGATTDescriptor : public ProtoMessage { public: - std::vector uuid{}; + std::array uuid{}; + size_t uuid_index_{0}; uint32_t handle{0}; void encode(ProtoWriteBuffer buffer) const override; void calculate_size(uint32_t &total_size) const override; @@ -1808,7 +1809,8 @@ class BluetoothGATTDescriptor : public ProtoMessage { }; class BluetoothGATTCharacteristic : public ProtoMessage { public: - std::vector uuid{}; + std::array uuid{}; + size_t uuid_index_{0}; uint32_t handle{0}; uint32_t properties{0}; std::vector descriptors{}; @@ -1822,7 +1824,8 @@ class BluetoothGATTCharacteristic : public ProtoMessage { }; class BluetoothGATTService : public ProtoMessage { public: - std::vector uuid{}; + std::array uuid{}; + size_t uuid_index_{0}; uint32_t handle{0}; std::vector characteristics{}; void encode(ProtoWriteBuffer buffer) const override; @@ -1836,12 +1839,13 @@ class BluetoothGATTService : public ProtoMessage { class BluetoothGATTGetServicesResponse : public ProtoMessage { public: static constexpr uint8_t MESSAGE_TYPE = 71; - static constexpr uint8_t ESTIMATED_SIZE = 38; + static constexpr uint8_t ESTIMATED_SIZE = 21; #ifdef HAS_PROTO_MESSAGE_DUMP const char *message_name() const override { return "bluetooth_gatt_get_services_response"; } #endif uint64_t address{0}; - std::vector services{}; + std::array services{}; + size_t services_index_{0}; void encode(ProtoWriteBuffer buffer) const override; void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP diff --git a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp index 7c883b74a2..aae08286c9 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp @@ -13,16 +13,17 @@ namespace bluetooth_proxy { static const char *const TAG = "bluetooth_proxy.connection"; -static std::vector get_128bit_uuid_vec(esp_bt_uuid_t uuid_source) { +static std::array get_128bit_uuid_array(esp_bt_uuid_t uuid_source) { esp_bt_uuid_t uuid = espbt::ESPBTUUID::from_uuid(uuid_source).as_128bit().get_uuid(); - return std::vector{((uint64_t) uuid.uuid.uuid128[15] << 56) | ((uint64_t) uuid.uuid.uuid128[14] << 48) | - ((uint64_t) uuid.uuid.uuid128[13] << 40) | ((uint64_t) uuid.uuid.uuid128[12] << 32) | - ((uint64_t) uuid.uuid.uuid128[11] << 24) | ((uint64_t) uuid.uuid.uuid128[10] << 16) | - ((uint64_t) uuid.uuid.uuid128[9] << 8) | ((uint64_t) uuid.uuid.uuid128[8]), - ((uint64_t) uuid.uuid.uuid128[7] << 56) | ((uint64_t) uuid.uuid.uuid128[6] << 48) | - ((uint64_t) uuid.uuid.uuid128[5] << 40) | ((uint64_t) uuid.uuid.uuid128[4] << 32) | - ((uint64_t) uuid.uuid.uuid128[3] << 24) | ((uint64_t) uuid.uuid.uuid128[2] << 16) | - ((uint64_t) uuid.uuid.uuid128[1] << 8) | ((uint64_t) uuid.uuid.uuid128[0])}; + return std::array{ + ((uint64_t) uuid.uuid.uuid128[15] << 56) | ((uint64_t) uuid.uuid.uuid128[14] << 48) | + ((uint64_t) uuid.uuid.uuid128[13] << 40) | ((uint64_t) uuid.uuid.uuid128[12] << 32) | + ((uint64_t) uuid.uuid.uuid128[11] << 24) | ((uint64_t) uuid.uuid.uuid128[10] << 16) | + ((uint64_t) uuid.uuid.uuid128[9] << 8) | ((uint64_t) uuid.uuid.uuid128[8]), + ((uint64_t) uuid.uuid.uuid128[7] << 56) | ((uint64_t) uuid.uuid.uuid128[6] << 48) | + ((uint64_t) uuid.uuid.uuid128[5] << 40) | ((uint64_t) uuid.uuid.uuid128[4] << 32) | + ((uint64_t) uuid.uuid.uuid128[3] << 24) | ((uint64_t) uuid.uuid.uuid128[2] << 16) | + ((uint64_t) uuid.uuid.uuid128[1] << 8) | ((uint64_t) uuid.uuid.uuid128[0])}; } void BluetoothConnection::dump_config() { @@ -95,9 +96,8 @@ void BluetoothConnection::send_service_for_discovery_() { api::BluetoothGATTGetServicesResponse resp; resp.address = this->address_; - resp.services.emplace_back(); - auto &service_resp = resp.services.back(); - service_resp.uuid = get_128bit_uuid_vec(service_result.uuid); + auto &service_resp = resp.services[0]; + service_resp.uuid = get_128bit_uuid_array(service_result.uuid); service_resp.handle = service_result.start_handle; // Get the number of characteristics directly with one call @@ -136,7 +136,7 @@ void BluetoothConnection::send_service_for_discovery_() { service_resp.characteristics.emplace_back(); auto &characteristic_resp = service_resp.characteristics.back(); - characteristic_resp.uuid = get_128bit_uuid_vec(char_result.uuid); + characteristic_resp.uuid = get_128bit_uuid_array(char_result.uuid); characteristic_resp.handle = char_result.char_handle; characteristic_resp.properties = char_result.properties; char_offset++; @@ -176,7 +176,7 @@ void BluetoothConnection::send_service_for_discovery_() { characteristic_resp.descriptors.emplace_back(); auto &descriptor_resp = characteristic_resp.descriptors.back(); - descriptor_resp.uuid = get_128bit_uuid_vec(desc_result.uuid); + descriptor_resp.uuid = get_128bit_uuid_array(desc_result.uuid); descriptor_resp.handle = desc_result.handle; desc_offset++; } diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 2678b7009a..3ca390658b 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -327,6 +327,9 @@ def create_field_type_info( ) -> TypeInfo: """Create the appropriate TypeInfo instance for a field, handling repeated fields and custom options.""" if field.label == 3: # repeated + # Check if this repeated field has fixed_array_size option + if (fixed_size := get_field_opt(field, pb.fixed_array_size)) is not None: + return FixedArrayRepeatedType(field, fixed_size) return RepeatedTypeInfo(field) # Check for fixed_array_size option on bytes fields @@ -883,6 +886,126 @@ class SInt64Type(TypeInfo): return self.calculate_field_id_size() + 3 # field ID + 3 bytes typical varint +class FixedArrayRepeatedType(TypeInfo): + """Special type for fixed-size repeated fields using std::array.""" + + def __init__(self, field: descriptor.FieldDescriptorProto, size: int) -> None: + super().__init__(field) + self.array_size = size + # Create the element type info + validate_field_type(field.type, field.name) + self._ti: TypeInfo = TYPE_INFO[field.type](field) + + @property + def cpp_type(self) -> str: + return f"std::array<{self._ti.cpp_type}, {self.array_size}>" + + @property + def reference_type(self) -> str: + return f"{self.cpp_type} &" + + @property + def const_reference_type(self) -> str: + return f"const {self.cpp_type} &" + + @property + def wire_type(self) -> WireType: + """Get the wire type for this fixed array field.""" + return self._ti.wire_type + + @property + def public_content(self) -> list[str]: + # Add the array member and a counter for decoding + return [ + f"{self.cpp_type} {self.field_name}{{}};", + f"size_t {self.field_name}_index_{{0}};", + ] + + @property + def decode_varint_content(self) -> str: + content = self._ti.decode_varint + if content is None: + return None + return f"case {self.number}: if (this->{self.field_name}_index_ < {self.array_size}) {{ this->{self.field_name}[this->{self.field_name}_index_++] = {content}; }} break;" + + @property + def decode_length_content(self) -> str: + content = self._ti.decode_length + if content is None and isinstance(self._ti, MessageType): + # Special handling for non-template message decoding + return f"case {self.number}: if (this->{self.field_name}_index_ < {self.array_size}) {{ value.decode_to_message(this->{self.field_name}[this->{self.field_name}_index_++]); }} break;" + if content is None: + return None + return f"case {self.number}: if (this->{self.field_name}_index_ < {self.array_size}) {{ this->{self.field_name}[this->{self.field_name}_index_++] = {content}; }} break;" + + @property + def decode_32bit_content(self) -> str: + content = self._ti.decode_32bit + if content is None: + return None + return f"case {self.number}: if (this->{self.field_name}_index_ < {self.array_size}) {{ this->{self.field_name}[this->{self.field_name}_index_++] = {content}; }} break;" + + @property + def decode_64bit_content(self) -> str: + content = self._ti.decode_64bit + if content is None: + return None + return f"case {self.number}: if (this->{self.field_name}_index_ < {self.array_size}) {{ this->{self.field_name}[this->{self.field_name}_index_++] = {content}; }} break;" + + @property + def _ti_is_bool(self) -> bool: + # std::array doesn't have the same specialization issues as std::vector for bool + return False + + @property + def encode_content(self) -> str: + o = f"for (const auto &it : this->{self.field_name}) {{\n" + if isinstance(self._ti, EnumType): + o += f" buffer.{self._ti.encode_func}({self.number}, static_cast(it), true);\n" + else: + o += f" buffer.{self._ti.encode_func}({self.number}, it, true);\n" + o += "}" + return o + + @property + def dump_content(self) -> str: + o = f"for (const auto &it : this->{self.field_name}) {{\n" + o += f' out.append(" {self.name}: ");\n' + o += indent(self._ti.dump("it")) + "\n" + o += ' out.append("\\n");\n' + o += "}\n" + return o + + def dump(self, _: str): + pass + + def get_size_calculation(self, name: str, force: bool = False) -> str: + # For fixed arrays, we always encode all elements + # Check if this is a fixed-size type by seeing if it has a fixed byte count + num_bytes = self._ti.get_fixed_size_bytes() + if num_bytes is not None: + # Fixed types have constant size per element, so we can multiply + field_id_size = self._ti.calculate_field_id_size() + # Pre-calculate the total bytes per element + bytes_per_element = field_id_size + num_bytes + o = f"total_size += {self.array_size} * {bytes_per_element};" + else: + # Other types need the actual value + o = f"for (const auto &it : {name}) {{\n" + o += f" {self._ti.get_size_calculation('it', True)}\n" + o += "}" + return o + + def get_estimated_size(self) -> int: + # For fixed arrays, estimate underlying type size * array size + underlying_size = ( + self._ti.get_estimated_size() + if hasattr(self._ti, "get_estimated_size") + else 8 + ) + return underlying_size * self.array_size + + class RepeatedTypeInfo(TypeInfo): def __init__(self, field: descriptor.FieldDescriptorProto) -> None: super().__init__(field) From 37cbcd5110a8e7522091895ad333721d969006ca Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 21 Jul 2025 12:55:05 -1000 Subject: [PATCH 02/13] preen --- esphome/components/api/api_pb2.h | 4 --- script/api_protobuf/api_protobuf.py | 40 +++++++++++------------------ 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index 7a9726f6e4..1d052f6114 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -1797,7 +1797,6 @@ class BluetoothGATTGetServicesRequest : public ProtoDecodableMessage { class BluetoothGATTDescriptor : public ProtoMessage { public: std::array uuid{}; - size_t uuid_index_{0}; uint32_t handle{0}; void encode(ProtoWriteBuffer buffer) const override; void calculate_size(uint32_t &total_size) const override; @@ -1810,7 +1809,6 @@ class BluetoothGATTDescriptor : public ProtoMessage { class BluetoothGATTCharacteristic : public ProtoMessage { public: std::array uuid{}; - size_t uuid_index_{0}; uint32_t handle{0}; uint32_t properties{0}; std::vector descriptors{}; @@ -1825,7 +1823,6 @@ class BluetoothGATTCharacteristic : public ProtoMessage { class BluetoothGATTService : public ProtoMessage { public: std::array uuid{}; - size_t uuid_index_{0}; uint32_t handle{0}; std::vector characteristics{}; void encode(ProtoWriteBuffer buffer) const override; @@ -1845,7 +1842,6 @@ class BluetoothGATTGetServicesResponse : public ProtoMessage { #endif uint64_t address{0}; std::array services{}; - size_t services_index_{0}; void encode(ProtoWriteBuffer buffer) const override; void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 3ca390658b..1031b294e2 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -887,7 +887,11 @@ class SInt64Type(TypeInfo): class FixedArrayRepeatedType(TypeInfo): - """Special type for fixed-size repeated fields using std::array.""" + """Special type for fixed-size repeated fields using std::array. + + Fixed arrays are only supported for encoding (SOURCE_SERVER) since we cannot + control how many items we receive when decoding. + """ def __init__(self, field: descriptor.FieldDescriptorProto, size: int) -> None: super().__init__(field) @@ -915,42 +919,28 @@ class FixedArrayRepeatedType(TypeInfo): @property def public_content(self) -> list[str]: - # Add the array member and a counter for decoding - return [ - f"{self.cpp_type} {self.field_name}{{}};", - f"size_t {self.field_name}_index_{{0}};", - ] + # Just the array member, no index needed since we don't decode + return [f"{self.cpp_type} {self.field_name}{{}};"] @property def decode_varint_content(self) -> str: - content = self._ti.decode_varint - if content is None: - return None - return f"case {self.number}: if (this->{self.field_name}_index_ < {self.array_size}) {{ this->{self.field_name}[this->{self.field_name}_index_++] = {content}; }} break;" + # Fixed arrays don't support decoding + return None @property def decode_length_content(self) -> str: - content = self._ti.decode_length - if content is None and isinstance(self._ti, MessageType): - # Special handling for non-template message decoding - return f"case {self.number}: if (this->{self.field_name}_index_ < {self.array_size}) {{ value.decode_to_message(this->{self.field_name}[this->{self.field_name}_index_++]); }} break;" - if content is None: - return None - return f"case {self.number}: if (this->{self.field_name}_index_ < {self.array_size}) {{ this->{self.field_name}[this->{self.field_name}_index_++] = {content}; }} break;" + # Fixed arrays don't support decoding + return None @property def decode_32bit_content(self) -> str: - content = self._ti.decode_32bit - if content is None: - return None - return f"case {self.number}: if (this->{self.field_name}_index_ < {self.array_size}) {{ this->{self.field_name}[this->{self.field_name}_index_++] = {content}; }} break;" + # Fixed arrays don't support decoding + return None @property def decode_64bit_content(self) -> str: - content = self._ti.decode_64bit - if content is None: - return None - return f"case {self.number}: if (this->{self.field_name}_index_ < {self.array_size}) {{ this->{self.field_name}[this->{self.field_name}_index_++] = {content}; }} break;" + # Fixed arrays don't support decoding + return None @property def _ti_is_bool(self) -> bool: From 8ee06cdc8cc2fbc1bba9fdf183cae980c78ecc65 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 21 Jul 2025 12:56:57 -1000 Subject: [PATCH 03/13] cleanup --- script/api_protobuf/api_protobuf.py | 35 +++++++++++++---------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 1031b294e2..a1c34e6369 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -922,25 +922,8 @@ class FixedArrayRepeatedType(TypeInfo): # Just the array member, no index needed since we don't decode return [f"{self.cpp_type} {self.field_name}{{}};"] - @property - def decode_varint_content(self) -> str: - # Fixed arrays don't support decoding - return None - - @property - def decode_length_content(self) -> str: - # Fixed arrays don't support decoding - return None - - @property - def decode_32bit_content(self) -> str: - # Fixed arrays don't support decoding - return None - - @property - def decode_64bit_content(self) -> str: - # Fixed arrays don't support decoding - return None + # No decode methods needed - fixed arrays don't support decoding + # The base class TypeInfo already returns None for all decode properties @property def _ti_is_bool(self) -> bool: @@ -1389,6 +1372,20 @@ def build_message_type( needs_decode = source in (SOURCE_BOTH, SOURCE_CLIENT) needs_encode = source in (SOURCE_BOTH, SOURCE_SERVER) + # Validate that fixed_array_size is only used in encode-only messages + if needs_decode: + for field in desc.field: + if ( + field.label == 3 + and get_field_opt(field, pb.fixed_array_size) is not None + ): + raise ValueError( + f"Message '{desc.name}' uses fixed_array_size on field '{field.name}' " + f"but has source={['SOURCE_BOTH', 'SOURCE_SERVER', 'SOURCE_CLIENT'][source]}. " + f"Fixed arrays are only supported for SOURCE_SERVER (encode-only) messages " + f"since we cannot trust or control the number of items received from clients." + ) + # Add MESSAGE_TYPE method if this is a service message if message_id is not None: # Validate that message_id fits in uint8_t From 5f14579af8f11a30d9b789c4ef7451fb87d5c89a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 21 Jul 2025 13:00:30 -1000 Subject: [PATCH 04/13] cleanup --- script/api_protobuf/api_protobuf.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index a1c34e6369..d26a7bb621 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -596,6 +596,8 @@ class MessageType(TypeInfo): return self._get_simple_size_calculation(name, force, "add_message_object") def get_estimated_size(self) -> int: + # For message types, we can't easily estimate the submessage size without + # access to the actual message definition. This is just a rough estimate. return ( self.calculate_field_id_size() + 16 ) # field ID + 16 bytes estimated submessage From 6d6bf8250168100c5789eb18cef1c3d884895fbc Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 21 Jul 2025 13:02:46 -1000 Subject: [PATCH 05/13] cleanup --- script/api_protobuf/api_protobuf.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index d26a7bb621..d39e7422fb 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -927,11 +927,6 @@ class FixedArrayRepeatedType(TypeInfo): # No decode methods needed - fixed arrays don't support decoding # The base class TypeInfo already returns None for all decode properties - @property - def _ti_is_bool(self) -> bool: - # std::array doesn't have the same specialization issues as std::vector for bool - return False - @property def encode_content(self) -> str: o = f"for (const auto &it : this->{self.field_name}) {{\n" @@ -951,9 +946,6 @@ class FixedArrayRepeatedType(TypeInfo): o += "}\n" return o - def dump(self, _: str): - pass - def get_size_calculation(self, name: str, force: bool = False) -> str: # For fixed arrays, we always encode all elements # Check if this is a fixed-size type by seeing if it has a fixed byte count From 9a391df0f008b1d73ef80c562f11827419221e44 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 21 Jul 2025 13:03:21 -1000 Subject: [PATCH 06/13] cleanup --- script/api_protobuf/api_protobuf.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index d39e7422fb..cb87ddda3b 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -946,6 +946,11 @@ class FixedArrayRepeatedType(TypeInfo): o += "}\n" return o + def dump(self, name: str) -> str: + # This is used when dumping the array itself (not its elements) + # Since dump_content handles the iteration, this is not used directly + return "" + def get_size_calculation(self, name: str, force: bool = False) -> str: # For fixed arrays, we always encode all elements # Check if this is a fixed-size type by seeing if it has a fixed byte count From f034069b5efdd6e3c294d630183998f4524505c4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 21 Jul 2025 13:04:23 -1000 Subject: [PATCH 07/13] cleanup --- script/api_protobuf/api_protobuf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index cb87ddda3b..ab96ecd62b 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -957,7 +957,7 @@ class FixedArrayRepeatedType(TypeInfo): num_bytes = self._ti.get_fixed_size_bytes() if num_bytes is not None: # Fixed types have constant size per element, so we can multiply - field_id_size = self._ti.calculate_field_id_size() + field_id_size = self.calculate_field_id_size() # Pre-calculate the total bytes per element bytes_per_element = field_id_size + num_bytes o = f"total_size += {self.array_size} * {bytes_per_element};" From b3abebfb37a6fc338f48e0a02702928aeb8890a0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 21 Jul 2025 13:08:51 -1000 Subject: [PATCH 08/13] cleanup --- script/api_protobuf/api_protobuf.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index ab96ecd62b..dedd58d5a1 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -970,11 +970,7 @@ class FixedArrayRepeatedType(TypeInfo): def get_estimated_size(self) -> int: # For fixed arrays, estimate underlying type size * array size - underlying_size = ( - self._ti.get_estimated_size() - if hasattr(self._ti, "get_estimated_size") - else 8 - ) + underlying_size = self._ti.get_estimated_size() return underlying_size * self.array_size From bc6b1ffc1428464d06cfed910cce0139b7f88e93 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 21 Jul 2025 13:12:30 -1000 Subject: [PATCH 09/13] cleanup --- script/api_protobuf/api_protobuf.py | 33 +++++++++++++++++------------ 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index dedd58d5a1..453f4ffe10 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -1367,20 +1367,6 @@ def build_message_type( needs_decode = source in (SOURCE_BOTH, SOURCE_CLIENT) needs_encode = source in (SOURCE_BOTH, SOURCE_SERVER) - # Validate that fixed_array_size is only used in encode-only messages - if needs_decode: - for field in desc.field: - if ( - field.label == 3 - and get_field_opt(field, pb.fixed_array_size) is not None - ): - raise ValueError( - f"Message '{desc.name}' uses fixed_array_size on field '{field.name}' " - f"but has source={['SOURCE_BOTH', 'SOURCE_SERVER', 'SOURCE_CLIENT'][source]}. " - f"Fixed arrays are only supported for SOURCE_SERVER (encode-only) messages " - f"since we cannot trust or control the number of items received from clients." - ) - # Add MESSAGE_TYPE method if this is a service message if message_id is not None: # Validate that message_id fits in uint8_t @@ -1416,6 +1402,19 @@ def build_message_type( if field.options.deprecated: continue + # Validate that fixed_array_size is only used in encode-only messages + if ( + needs_decode + and field.label == 3 + and get_field_opt(field, pb.fixed_array_size) is not None + ): + raise ValueError( + f"Message '{desc.name}' uses fixed_array_size on field '{field.name}' " + f"but has source={SOURCE_NAMES[source]}. " + f"Fixed arrays are only supported for SOURCE_SERVER (encode-only) messages " + f"since we cannot trust or control the number of items received from clients." + ) + ti = create_field_type_info(field, needs_decode, needs_encode) # Skip field declarations for fields that are in the base class @@ -1605,6 +1604,12 @@ SOURCE_BOTH = 0 SOURCE_SERVER = 1 SOURCE_CLIENT = 2 +SOURCE_NAMES = { + SOURCE_BOTH: "SOURCE_BOTH", + SOURCE_SERVER: "SOURCE_SERVER", + SOURCE_CLIENT: "SOURCE_CLIENT", +} + RECEIVE_CASES: dict[int, tuple[str, str | None]] = {} ifdefs: dict[str, str] = {} From 55272dd0fdc905d981a6126fe211421cfe462d34 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 21 Jul 2025 13:13:45 -1000 Subject: [PATCH 10/13] cleanup --- esphome/components/api/api_pb2.cpp | 8 ++------ script/api_protobuf/api_protobuf.py | 13 +++++++++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index 9d8b109986..f2c4d1797f 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -1938,15 +1938,11 @@ void BluetoothGATTService::calculate_size(uint32_t &total_size) const { } void BluetoothGATTGetServicesResponse::encode(ProtoWriteBuffer buffer) const { buffer.encode_uint64(1, this->address); - for (const auto &it : this->services) { - buffer.encode_message(2, it, true); - } + buffer.encode_message(2, this->services[0], true); } void BluetoothGATTGetServicesResponse::calculate_size(uint32_t &total_size) const { ProtoSize::add_uint64_field(total_size, 1, this->address); - for (const auto &it : this->services) { - ProtoSize::add_message_object_repeated(total_size, 1, it); - } + ProtoSize::add_message_object_repeated(total_size, 1, this->services[0]); } void BluetoothGATTGetServicesDoneResponse::encode(ProtoWriteBuffer buffer) const { buffer.encode_uint64(1, this->address); diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 453f4ffe10..b9d7f1fd6e 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -929,6 +929,14 @@ class FixedArrayRepeatedType(TypeInfo): @property def encode_content(self) -> str: + # Special case for single-element arrays - no loop needed + if self.array_size == 1: + if isinstance(self._ti, EnumType): + return f"buffer.{self._ti.encode_func}({self.number}, static_cast(this->{self.field_name}[0]), true);" + else: + return f"buffer.{self._ti.encode_func}({self.number}, this->{self.field_name}[0], true);" + + # Multiple elements need a loop o = f"for (const auto &it : this->{self.field_name}) {{\n" if isinstance(self._ti, EnumType): o += f" buffer.{self._ti.encode_func}({self.number}, static_cast(it), true);\n" @@ -953,6 +961,11 @@ class FixedArrayRepeatedType(TypeInfo): def get_size_calculation(self, name: str, force: bool = False) -> str: # For fixed arrays, we always encode all elements + + # Special case for single-element arrays - no loop needed + if self.array_size == 1: + return self._ti.get_size_calculation(f"{name}[0]", True) + # Check if this is a fixed-size type by seeing if it has a fixed byte count num_bytes = self._ti.get_fixed_size_bytes() if num_bytes is not None: From 7b9acd39e14eb670ec8db2619e7c97543a06a2d2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 21 Jul 2025 13:17:18 -1000 Subject: [PATCH 11/13] cleanup --- esphome/components/api/api_pb2.cpp | 30 ++++++++++++----------------- script/api_protobuf/api_protobuf.py | 23 +++++++++++++++++++++- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index f2c4d1797f..09a1522fb4 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -1891,21 +1891,18 @@ bool BluetoothGATTGetServicesRequest::decode_varint(uint32_t field_id, ProtoVarI return true; } void BluetoothGATTDescriptor::encode(ProtoWriteBuffer buffer) const { - for (const auto &it : this->uuid) { - buffer.encode_uint64(1, it, true); - } + buffer.encode_uint64(1, this->uuid[0], true); + buffer.encode_uint64(1, this->uuid[1], true); buffer.encode_uint32(2, this->handle); } void BluetoothGATTDescriptor::calculate_size(uint32_t &total_size) const { - for (const auto &it : this->uuid) { - ProtoSize::add_uint64_field_repeated(total_size, 1, it); - } + ProtoSize::add_uint64_field_repeated(total_size, 1, this->uuid[0]); + ProtoSize::add_uint64_field_repeated(total_size, 1, this->uuid[1]); ProtoSize::add_uint32_field(total_size, 1, this->handle); } void BluetoothGATTCharacteristic::encode(ProtoWriteBuffer buffer) const { - for (const auto &it : this->uuid) { - buffer.encode_uint64(1, it, true); - } + buffer.encode_uint64(1, this->uuid[0], true); + buffer.encode_uint64(1, this->uuid[1], true); buffer.encode_uint32(2, this->handle); buffer.encode_uint32(3, this->properties); for (auto &it : this->descriptors) { @@ -1913,26 +1910,23 @@ void BluetoothGATTCharacteristic::encode(ProtoWriteBuffer buffer) const { } } void BluetoothGATTCharacteristic::calculate_size(uint32_t &total_size) const { - for (const auto &it : this->uuid) { - ProtoSize::add_uint64_field_repeated(total_size, 1, it); - } + ProtoSize::add_uint64_field_repeated(total_size, 1, this->uuid[0]); + ProtoSize::add_uint64_field_repeated(total_size, 1, this->uuid[1]); ProtoSize::add_uint32_field(total_size, 1, this->handle); ProtoSize::add_uint32_field(total_size, 1, this->properties); ProtoSize::add_repeated_message(total_size, 1, this->descriptors); } void BluetoothGATTService::encode(ProtoWriteBuffer buffer) const { - for (const auto &it : this->uuid) { - buffer.encode_uint64(1, it, true); - } + buffer.encode_uint64(1, this->uuid[0], true); + buffer.encode_uint64(1, this->uuid[1], true); buffer.encode_uint32(2, this->handle); for (auto &it : this->characteristics) { buffer.encode_message(3, it, true); } } void BluetoothGATTService::calculate_size(uint32_t &total_size) const { - for (const auto &it : this->uuid) { - ProtoSize::add_uint64_field_repeated(total_size, 1, it); - } + ProtoSize::add_uint64_field_repeated(total_size, 1, this->uuid[0]); + ProtoSize::add_uint64_field_repeated(total_size, 1, this->uuid[1]); ProtoSize::add_uint32_field(total_size, 1, this->handle); ProtoSize::add_repeated_message(total_size, 1, this->characteristics); } diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index b9d7f1fd6e..ccc87fb5a6 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -936,7 +936,20 @@ class FixedArrayRepeatedType(TypeInfo): else: return f"buffer.{self._ti.encode_func}({self.number}, this->{self.field_name}[0], true);" - # Multiple elements need a loop + # Special case for 2-element arrays - unroll the loop + if self.array_size == 2: + if isinstance(self._ti, EnumType): + return ( + f"buffer.{self._ti.encode_func}({self.number}, static_cast(this->{self.field_name}[0]), true);\n" + f" buffer.{self._ti.encode_func}({self.number}, static_cast(this->{self.field_name}[1]), true);" + ) + else: + return ( + f"buffer.{self._ti.encode_func}({self.number}, this->{self.field_name}[0], true);\n" + f" buffer.{self._ti.encode_func}({self.number}, this->{self.field_name}[1], true);" + ) + + # 3 or more elements need a loop o = f"for (const auto &it : this->{self.field_name}) {{\n" if isinstance(self._ti, EnumType): o += f" buffer.{self._ti.encode_func}({self.number}, static_cast(it), true);\n" @@ -966,6 +979,14 @@ class FixedArrayRepeatedType(TypeInfo): if self.array_size == 1: return self._ti.get_size_calculation(f"{name}[0]", True) + # Special case for 2-element arrays - unroll the calculation + if self.array_size == 2: + return ( + self._ti.get_size_calculation(f"{name}[0]", True) + + "\n " + + self._ti.get_size_calculation(f"{name}[1]", True) + ) + # Check if this is a fixed-size type by seeing if it has a fixed byte count num_bytes = self._ti.get_fixed_size_bytes() if num_bytes is not None: From 767ec53cfa3853a0317831eff5de5515718d44cd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 21 Jul 2025 13:18:10 -1000 Subject: [PATCH 12/13] cleanup --- script/api_protobuf/api_protobuf.py | 40 ++++++++++++----------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index ccc87fb5a6..1b16b1e842 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -929,32 +929,26 @@ class FixedArrayRepeatedType(TypeInfo): @property def encode_content(self) -> str: - # Special case for single-element arrays - no loop needed + # Helper to generate encode statement for a single element + def encode_element(element: str) -> str: + if isinstance(self._ti, EnumType): + return f"buffer.{self._ti.encode_func}({self.number}, static_cast({element}), true);" + else: + return f"buffer.{self._ti.encode_func}({self.number}, {element}, true);" + + # Unroll small arrays for efficiency if self.array_size == 1: - if isinstance(self._ti, EnumType): - return f"buffer.{self._ti.encode_func}({self.number}, static_cast(this->{self.field_name}[0]), true);" - else: - return f"buffer.{self._ti.encode_func}({self.number}, this->{self.field_name}[0], true);" + return encode_element(f"this->{self.field_name}[0]") + elif self.array_size == 2: + return ( + encode_element(f"this->{self.field_name}[0]") + + "\n " + + encode_element(f"this->{self.field_name}[1]") + ) - # Special case for 2-element arrays - unroll the loop - if self.array_size == 2: - if isinstance(self._ti, EnumType): - return ( - f"buffer.{self._ti.encode_func}({self.number}, static_cast(this->{self.field_name}[0]), true);\n" - f" buffer.{self._ti.encode_func}({self.number}, static_cast(this->{self.field_name}[1]), true);" - ) - else: - return ( - f"buffer.{self._ti.encode_func}({self.number}, this->{self.field_name}[0], true);\n" - f" buffer.{self._ti.encode_func}({self.number}, this->{self.field_name}[1], true);" - ) - - # 3 or more elements need a loop + # Use loops for larger arrays o = f"for (const auto &it : this->{self.field_name}) {{\n" - if isinstance(self._ti, EnumType): - o += f" buffer.{self._ti.encode_func}({self.number}, static_cast(it), true);\n" - else: - o += f" buffer.{self._ti.encode_func}({self.number}, it, true);\n" + o += f" {encode_element('it')}\n" o += "}" return o From 4c62f43dcd43bf34fbf8061a5529ff13ace18ab3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 21 Jul 2025 13:19:19 -1000 Subject: [PATCH 13/13] cleanup --- script/api_protobuf/api_protobuf.py | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 1b16b1e842..2d49ac5ece 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -981,19 +981,10 @@ class FixedArrayRepeatedType(TypeInfo): + self._ti.get_size_calculation(f"{name}[1]", True) ) - # Check if this is a fixed-size type by seeing if it has a fixed byte count - num_bytes = self._ti.get_fixed_size_bytes() - if num_bytes is not None: - # Fixed types have constant size per element, so we can multiply - field_id_size = self.calculate_field_id_size() - # Pre-calculate the total bytes per element - bytes_per_element = field_id_size + num_bytes - o = f"total_size += {self.array_size} * {bytes_per_element};" - else: - # Other types need the actual value - o = f"for (const auto &it : {name}) {{\n" - o += f" {self._ti.get_size_calculation('it', True)}\n" - o += "}" + # Use loops for larger arrays + o = f"for (const auto &it : {name}) {{\n" + o += f" {self._ti.get_size_calculation('it', True)}\n" + o += "}" return o def get_estimated_size(self) -> int: