From fedc058f7224c28befa7324507f21e5bbe359b3d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 7 Sep 2026 10:49:14 +0200 Subject: [PATCH] [api] Trim the decode dispatch comments --- esphome/components/api/proto.cpp | 5 ++--- esphome/components/api/proto.h | 24 ++++++++---------------- script/api_protobuf/api_protobuf.py | 12 +++--------- 3 files changed, 13 insertions(+), 28 deletions(-) diff --git a/esphome/components/api/proto.cpp b/esphome/components/api/proto.cpp index bc82d53dbf..511a925186 100644 --- a/esphome/components/api/proto.cpp +++ b/esphome/components/api/proto.cpp @@ -214,8 +214,7 @@ void ProtoDecodableMessage::decode(const uint8_t *buffer, size_t length) { const uint8_t *ptr = buffer; const uint8_t *end = buffer + length; - // Single-byte varints dominate real messages (tags, small lengths, bools, enums), so that case - // advances the cursor inline; a merged path would materialize the consumed count and add it. + // Single-byte varints dominate, so that case advances the cursor inline. auto read_varint = [&](proto_varint_value_t &value) ESPHOME_ALWAYS_INLINE { if (ptr == end) return false; @@ -240,7 +239,7 @@ void ProtoDecodableMessage::decode(const uint8_t *buffer, size_t length) { uint32_t tag = static_cast(tag_value); uint32_t field_type = tag & WIRE_TYPE_MASK; - // Payload start for scalar wire types; length-delimited fields advance it past the length. + // Length-delimited fields move this past the length prefix const uint8_t *data = ptr; proto_varint_value_t scalar; diff --git a/esphome/components/api/proto.h b/esphome/components/api/proto.h index cce1bd254f..b5c224f211 100644 --- a/esphome/components/api/proto.h +++ b/esphome/components/api/proto.h @@ -170,13 +170,9 @@ class ProtoVarInt { class ProtoMessage; class ProtoSize; -// Generated decode_field() bodies switch on PROTO_DECODE_KEY and label each case with -// PROTO_DECODE_CASE. Embedded targets compile switches to compare chains (ESP-IDF passes -// -fno-jump-tables), so keying on the full wire tag costs one compare per field and needs no -// separate wire type check. The host compiler turns the dense field number switch into a jump -// table, so there the key is the field number and PROTO_DECODE_GUARD compares the tag against the -// one the case declares, which rejects the wrong wire type in a single compare. Both forms drop a -// field that arrives with a wire type it does not declare, as the per wire type virtuals did. +// Switch key for generated decode_field() bodies. Embedded builds use compare chains +// (-fno-jump-tables), so the full tag is one compare per field with no guard. The host gets a +// jump table from the dense field number switch plus a tag compare that rejects the wrong wire type. #ifdef USE_HOST #define PROTO_DECODE_KEY(tag) ((tag) >> 3) #define PROTO_DECODE_CASE(field_id, wire_type) (field_id) @@ -189,10 +185,8 @@ class ProtoSize; #define PROTO_DECODE_GUARD(tag, field_id, wire_type) (void) 0 #endif -/// Payload of one decoded field as decode_field() receives it: the payload pointer and one scalar -/// that holds the varint or fixed32 value, or the byte length of a length-delimited field. The -/// wire type in the tag says which reading applies; the accessors do not check. Built by the -/// generated decode_field() from its two register arguments, so it never exists in memory. +/// One decoded field: the payload pointer and a scalar holding the varint or fixed32 value, or the +/// length of a length-delimited field. The wire type in the tag says which applies; accessors do not check. class ProtoFieldValue { public: ProtoFieldValue(const uint8_t *data, proto_varint_value_t scalar) : data_(data), scalar_(scalar) {} @@ -746,11 +740,9 @@ class ProtoDecodableMessage : public ProtoMessage { protected: ~ProtoDecodableMessage() = default; - /// Store one decoded field. \p tag is the wire tag (field number and wire type), \p data points at - /// the field payload and \p scalar is the varint or fixed32 value, or the payload length for a - /// length-delimited field. Three register arguments keep the shared loop free of spills. Return - /// false for an unknown field or one that arrived with a wire type it does not declare. - /// One virtual instead of one per wire type keeps each message's vtable at a single slot. + /// Store one decoded field; \p scalar is the varint or fixed32 value, or the length of the + /// length-delimited payload at \p data. Return false for an unknown field or wrong wire type. + /// Three register arguments keep the decode loop free of spills. // NOTE: wire type 1 (64-bit fixed) is not supported virtual bool decode_field(uint32_t tag, const uint8_t *data, proto_varint_value_t scalar) { return false; } }; diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 4397abf833..ddbd072426 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -229,13 +229,10 @@ class TypeInfo(ABC): def class_member(self) -> str: return f"{self.cpp_type} {self.field_name}{{{self.default_value}}};" - # Cases are keyed through the PROTO_DECODE_* macros in proto.h, which is where the - # host and embedded switch shapes are explained. def decode_case(self, wire_type: WireType, body: str) -> str: - """Emit one decode_field() case for a field and the wire type it expects. + """Emit one decode_field() case, keyed through the PROTO_DECODE_* macros in proto.h. - Multi-statement bodies get their own block so a local in one case cannot be - jumped over by a later case label. + Multi-statement bodies get a block so a case label never jumps over a local. """ label = f"case PROTO_DECODE_CASE({self.number}, {int(wire_type)}):" guard = f"PROTO_DECODE_GUARD(tag, {self.number}, {int(wire_type)});" @@ -243,8 +240,7 @@ class TypeInfo(ABC): return f"{label} {{\n" + indent(f"{guard}\n{body}\nbreak;") + "\n}" return f"{label}\n" + indent(f"{guard}\n{body}\nbreak;") - # Value expression that decodes this type from the ProtoFieldValue, per wire type. - # A type sets exactly one of them; None everywhere means the field is never decoded. + # Decode expression per wire type; a decodable type sets exactly one. decode_varint = None decode_length = None decode_32bit = None @@ -2703,8 +2699,6 @@ def build_message_type( cpp = "" if decode: - # One virtual per message: the shared decode loop parses the payload for the wire - # type and hands it over with the tag, so a single switch covers every field. o = f"bool {desc.name}::decode_field(uint32_t tag, const uint8_t *data, proto_varint_value_t scalar) {{\n" o += " const ProtoFieldValue value(data, scalar);\n" o += " switch (PROTO_DECODE_KEY(tag)) {\n"