diff --git a/esphome/components/api/api_frame_helper_plaintext.cpp b/esphome/components/api/api_frame_helper_plaintext.cpp index ed3cc8934e..5069dbf68b 100644 --- a/esphome/components/api/api_frame_helper_plaintext.cpp +++ b/esphome/components/api/api_frame_helper_plaintext.cpp @@ -295,9 +295,8 @@ APIError APIPlaintextFrameHelper::write_protobuf_messages(ProtoWriteBuffer buffe buf_start[header_offset] = 0x00; // indicator // Encode varints directly into buffer - ProtoVarInt(msg.payload_size).encode_to_buffer_unchecked(buf_start + header_offset + 1, size_varint_len); - ProtoVarInt(msg.message_type) - .encode_to_buffer_unchecked(buf_start + header_offset + 1 + size_varint_len, type_varint_len); + encode_varint_to_buffer(msg.payload_size, buf_start + header_offset + 1); + encode_varint_to_buffer(msg.message_type, buf_start + header_offset + 1 + size_varint_len); // Add iovec for this message (header + payload) size_t msg_len = static_cast(total_header_len + msg.payload_size); diff --git a/esphome/components/api/proto.h b/esphome/components/api/proto.h index 41ea0043f9..8ac79633cf 100644 --- a/esphome/components/api/proto.h +++ b/esphome/components/api/proto.h @@ -57,6 +57,16 @@ inline uint16_t count_packed_varints(const uint8_t *data, size_t len) { return count; } +/// Encode a varint directly into a pre-allocated buffer. +/// Caller must ensure buffer has space (use ProtoSize::varint() to calculate). +inline void encode_varint_to_buffer(uint32_t val, uint8_t *buffer) { + while (val > 0x7F) { + *buffer++ = static_cast(val | 0x80); + val >>= 7; + } + *buffer = static_cast(val); +} + /* * StringRef Ownership Model for API Protocol Messages * =================================================== @@ -93,17 +103,17 @@ class ProtoVarInt { ProtoVarInt() : value_(0) {} explicit ProtoVarInt(uint64_t value) : value_(value) {} + /// Parse a varint from buffer. consumed must be a valid pointer (not null). static optional parse(const uint8_t *buffer, uint32_t len, uint32_t *consumed) { - if (len == 0) { - if (consumed != nullptr) - *consumed = 0; +#ifdef ESPHOME_DEBUG_API + assert(consumed != nullptr); +#endif + if (len == 0) return {}; - } // Most common case: single-byte varint (values 0-127) if ((buffer[0] & 0x80) == 0) { - if (consumed != nullptr) - *consumed = 1; + *consumed = 1; return ProtoVarInt(buffer[0]); } @@ -122,14 +132,11 @@ class ProtoVarInt { result |= uint64_t(val & 0x7F) << uint64_t(bitpos); bitpos += 7; if ((val & 0x80) == 0) { - if (consumed != nullptr) - *consumed = i + 1; + *consumed = i + 1; return ProtoVarInt(result); } } - if (consumed != nullptr) - *consumed = 0; return {}; // Incomplete or invalid varint } @@ -153,50 +160,6 @@ class ProtoVarInt { // with ZigZag encoding return decode_zigzag64(this->value_); } - /** - * Encode the varint value to a pre-allocated buffer without bounds checking. - * - * @param buffer The pre-allocated buffer to write the encoded varint to - * @param len The size of the buffer in bytes - * - * @note The caller is responsible for ensuring the buffer is large enough - * to hold the encoded value. Use ProtoSize::varint() to calculate - * the exact size needed before calling this method. - * @note No bounds checking is performed for performance reasons. - */ - void encode_to_buffer_unchecked(uint8_t *buffer, size_t len) { - uint64_t val = this->value_; - if (val <= 0x7F) { - buffer[0] = val; - return; - } - size_t i = 0; - while (val && i < len) { - uint8_t temp = val & 0x7F; - val >>= 7; - if (val) { - buffer[i++] = temp | 0x80; - } else { - buffer[i++] = temp; - } - } - } - void encode(std::vector &out) { - uint64_t val = this->value_; - if (val <= 0x7F) { - out.push_back(val); - return; - } - while (val) { - uint8_t temp = val & 0x7F; - val >>= 7; - if (val) { - out.push_back(temp | 0x80); - } else { - out.push_back(temp); - } - } - } protected: uint64_t value_; @@ -256,8 +219,20 @@ class ProtoWriteBuffer { public: ProtoWriteBuffer(std::vector *buffer) : buffer_(buffer) {} void write(uint8_t value) { this->buffer_->push_back(value); } - void encode_varint_raw(ProtoVarInt value) { value.encode(*this->buffer_); } - void encode_varint_raw(uint32_t value) { this->encode_varint_raw(ProtoVarInt(value)); } + void encode_varint_raw(uint32_t value) { + while (value > 0x7F) { + this->buffer_->push_back(static_cast(value | 0x80)); + value >>= 7; + } + this->buffer_->push_back(static_cast(value)); + } + void encode_varint_raw_64(uint64_t value) { + while (value > 0x7F) { + this->buffer_->push_back(static_cast(value | 0x80)); + value >>= 7; + } + this->buffer_->push_back(static_cast(value)); + } /** * Encode a field key (tag/wire type combination). * @@ -307,13 +282,13 @@ class ProtoWriteBuffer { if (value == 0 && !force) return; this->encode_field_raw(field_id, 0); // type 0: Varint - uint64 - this->encode_varint_raw(ProtoVarInt(value)); + this->encode_varint_raw_64(value); } void encode_bool(uint32_t field_id, bool value, bool force = false) { if (!value && !force) return; this->encode_field_raw(field_id, 0); // type 0: Varint - bool - this->write(0x01); + this->buffer_->push_back(value ? 0x01 : 0x00); } void encode_fixed32(uint32_t field_id, uint32_t value, bool force = false) { if (value == 0 && !force) @@ -938,13 +913,15 @@ inline void ProtoWriteBuffer::encode_message(uint32_t field_id, const ProtoMessa this->buffer_->resize(this->buffer_->size() + varint_length_bytes); // Write the length varint directly - ProtoVarInt(msg_length_bytes).encode_to_buffer_unchecked(this->buffer_->data() + begin, varint_length_bytes); + encode_varint_to_buffer(msg_length_bytes, this->buffer_->data() + begin); // Now encode the message content - it will append to the buffer value.encode(*this); +#ifdef ESPHOME_DEBUG_API // Verify that the encoded size matches what we calculated assert(this->buffer_->size() == begin + varint_length_bytes + msg_length_bytes); +#endif } // Implementation of decode_to_message - must be after ProtoDecodableMessage is defined diff --git a/esphome/core/defines.h b/esphome/core/defines.h index bfe9d620df..7e6df31ea2 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -14,6 +14,7 @@ #define ESPHOME_PROJECT_VERSION_30 "v2" #define ESPHOME_VARIANT "ESP32" #define ESPHOME_DEBUG_SCHEDULER +#define ESPHOME_DEBUG_API // Default threading model for static analysis (ESP32 is multi-threaded with atomics) #define ESPHOME_THREAD_MULTI_ATOMICS diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 50e8d4122b..36df1bc83e 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -197,6 +197,7 @@ async def yaml_config(request: pytest.FixtureRequest, unused_tcp_port: int) -> s " platformio_options:\n" " build_flags:\n" ' - "-DDEBUG" # Enable assert() statements\n' + ' - "-DESPHOME_DEBUG_API" # Enable API protocol asserts\n' ' - "-g" # Add debug symbols', )