diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index 6c5b487212..3594dabc32 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -1133,12 +1133,11 @@ SubscribeLogsResponse::calc_size_msg(const void *self) { bool NoiseEncryptionSetKeyRequest::decode_field(uint32_t tag, const uint8_t *data, proto_varint_value_t scalar) { const ProtoFieldValue value(data, scalar); switch (PROTO_DECODE_KEY(tag)) { - case PROTO_DECODE_CASE(1, 2): { + case PROTO_DECODE_CASE(1, 2): PROTO_DECODE_GUARD(tag, 1, 2); this->key = value.data(); this->key_len = value.size(); break; - } default: return false; } @@ -1247,12 +1246,11 @@ bool HomeassistantActionResponse::decode_field(uint32_t tag, const uint8_t *data this->error_message = StringRef(reinterpret_cast(value.data()), value.size()); break; #ifdef USE_API_HOMEASSISTANT_ACTION_RESPONSES_JSON - case PROTO_DECODE_CASE(4, 2): { + case PROTO_DECODE_CASE(4, 2): PROTO_DECODE_GUARD(tag, 4, 2); this->response_data = value.data(); this->response_data_len = value.size(); break; - } #endif default: return false; @@ -1362,12 +1360,11 @@ bool GetTimeResponse::decode_field(uint32_t tag, const uint8_t *data, proto_vari PROTO_DECODE_GUARD(tag, 1, 5); this->epoch_seconds = value.as_fixed32(); break; - case PROTO_DECODE_CASE(3, 2): { + case PROTO_DECODE_CASE(3, 2): PROTO_DECODE_GUARD(tag, 3, 2); value.decode_to_message(this->parsed_timezone); this->has_parsed_timezone = true; break; - } default: return false; } @@ -1492,12 +1489,11 @@ bool ExecuteServiceRequest::decode_field(uint32_t tag, const uint8_t *data, prot PROTO_DECODE_GUARD(tag, 1, 5); this->key = value.as_fixed32(); break; - case PROTO_DECODE_CASE(2, 2): { + case PROTO_DECODE_CASE(2, 2): PROTO_DECODE_GUARD(tag, 2, 2); this->args.emplace_back(); value.decode_to_message(this->args.back()); break; - } #ifdef USE_API_USER_DEFINED_ACTION_RESPONSES case PROTO_DECODE_CASE(3, 0): PROTO_DECODE_GUARD(tag, 3, 0); @@ -2877,12 +2873,11 @@ bool BluetoothGATTWriteRequest::decode_field(uint32_t tag, const uint8_t *data, PROTO_DECODE_GUARD(tag, 3, 0); this->response = value.as_varint() != 0; break; - case PROTO_DECODE_CASE(4, 2): { + case PROTO_DECODE_CASE(4, 2): PROTO_DECODE_GUARD(tag, 4, 2); this->data = value.data(); this->data_len = value.size(); break; - } default: return false; } @@ -2915,12 +2910,11 @@ bool BluetoothGATTWriteDescriptorRequest::decode_field(uint32_t tag, const uint8 PROTO_DECODE_GUARD(tag, 2, 0); this->handle = value.as_varint(); break; - case PROTO_DECODE_CASE(3, 2): { + case PROTO_DECODE_CASE(3, 2): PROTO_DECODE_GUARD(tag, 3, 2); this->data = value.data(); this->data_len = value.size(); break; - } default: return false; } @@ -3209,12 +3203,11 @@ bool VoiceAssistantEventResponse::decode_field(uint32_t tag, const uint8_t *data PROTO_DECODE_GUARD(tag, 1, 0); this->event_type = static_cast(value.as_varint()); break; - case PROTO_DECODE_CASE(2, 2): { + case PROTO_DECODE_CASE(2, 2): PROTO_DECODE_GUARD(tag, 2, 2); this->data.emplace_back(); value.decode_to_message(this->data.back()); break; - } default: return false; } @@ -3223,22 +3216,20 @@ bool VoiceAssistantEventResponse::decode_field(uint32_t tag, const uint8_t *data bool VoiceAssistantAudio::decode_field(uint32_t tag, const uint8_t *data, proto_varint_value_t scalar) { const ProtoFieldValue value(data, scalar); switch (PROTO_DECODE_KEY(tag)) { - case PROTO_DECODE_CASE(1, 2): { + case PROTO_DECODE_CASE(1, 2): PROTO_DECODE_GUARD(tag, 1, 2); this->data = value.data(); this->data_len = value.size(); break; - } case PROTO_DECODE_CASE(2, 0): PROTO_DECODE_GUARD(tag, 2, 0); this->end = value.as_varint() != 0; break; - case PROTO_DECODE_CASE(3, 2): { + case PROTO_DECODE_CASE(3, 2): PROTO_DECODE_GUARD(tag, 3, 2); this->data2 = value.data(); this->data2_len = value.size(); break; - } default: return false; } @@ -3390,12 +3381,11 @@ bool VoiceAssistantExternalWakeWord::decode_field(uint32_t tag, const uint8_t *d bool VoiceAssistantConfigurationRequest::decode_field(uint32_t tag, const uint8_t *data, proto_varint_value_t scalar) { const ProtoFieldValue value(data, scalar); switch (PROTO_DECODE_KEY(tag)) { - case PROTO_DECODE_CASE(1, 2): { + case PROTO_DECODE_CASE(1, 2): PROTO_DECODE_GUARD(tag, 1, 2); this->external_wake_words.emplace_back(); value.decode_to_message(this->external_wake_words.back()); break; - } default: return false; } @@ -4137,12 +4127,11 @@ bool UpdateCommandRequest::decode_field(uint32_t tag, const uint8_t *data, proto bool ZWaveProxyFrame::decode_field(uint32_t tag, const uint8_t *data, proto_varint_value_t scalar) { const ProtoFieldValue value(data, scalar); switch (PROTO_DECODE_KEY(tag)) { - case PROTO_DECODE_CASE(1, 2): { + case PROTO_DECODE_CASE(1, 2): PROTO_DECODE_GUARD(tag, 1, 2); this->data = value.data(); this->data_len = value.size(); break; - } default: return false; } @@ -4171,12 +4160,11 @@ bool ZWaveProxyRequest::decode_field(uint32_t tag, const uint8_t *data, proto_va PROTO_DECODE_GUARD(tag, 1, 0); this->type = static_cast(value.as_varint()); break; - case PROTO_DECODE_CASE(2, 2): { + case PROTO_DECODE_CASE(2, 2): PROTO_DECODE_GUARD(tag, 2, 2); this->data = value.data(); this->data_len = value.size(); break; - } default: return false; } @@ -4271,13 +4259,12 @@ bool InfraredRFTransmitRawTimingsRequest::decode_field(uint32_t tag, const uint8 PROTO_DECODE_GUARD(tag, 4, 0); this->repeat_count = value.as_varint(); break; - case PROTO_DECODE_CASE(5, 2): { + case PROTO_DECODE_CASE(5, 2): PROTO_DECODE_GUARD(tag, 5, 2); this->timings_data_ = value.data(); this->timings_length_ = value.size(); this->timings_count_ = count_packed_varints(value.data(), value.size()); break; - } case PROTO_DECODE_CASE(6, 0): PROTO_DECODE_GUARD(tag, 6, 0); this->modulation = value.as_varint(); @@ -4419,12 +4406,11 @@ bool SerialProxyWriteRequest::decode_field(uint32_t tag, const uint8_t *data, pr PROTO_DECODE_GUARD(tag, 1, 0); this->instance = value.as_varint(); break; - case PROTO_DECODE_CASE(2, 2): { + case PROTO_DECODE_CASE(2, 2): PROTO_DECODE_GUARD(tag, 2, 2); this->data = value.data(); this->data_len = value.size(); break; - } default: return false; } diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index ddbd072426..ae55aa52a3 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -229,41 +229,36 @@ class TypeInfo(ABC): def class_member(self) -> str: return f"{self.cpp_type} {self.field_name}{{{self.default_value}}};" - def decode_case(self, wire_type: WireType, body: str) -> str: - """Emit one decode_field() case, keyed through the PROTO_DECODE_* macros in proto.h. - - 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)});" - if "\n" in body: - return f"{label} {{\n" + indent(f"{guard}\n{body}\nbreak;") + "\n}" - return f"{label}\n" + indent(f"{guard}\n{body}\nbreak;") + def decode_case(self, body: str) -> str: + """Emit one decode_field() case, keyed through the PROTO_DECODE_* macros in proto.h.""" + wire_type = int(self.wire_type) + return f"case PROTO_DECODE_CASE({self.number}, {wire_type}):\n" + indent( + f"PROTO_DECODE_GUARD(tag, {self.number}, {wire_type});\n{body}\nbreak;" + ) # Decode expression per wire type; a decodable type sets exactly one. decode_varint = None decode_length = None decode_32bit = None - def decode_expr(self) -> tuple[WireType, str] | None: - """Wire type and value expression for decoding this field, or None.""" - for wire_type, content in ( - (WireType.VARINT, self.decode_varint), - (WireType.LENGTH_DELIMITED, self.decode_length), - (WireType.FIXED32, self.decode_32bit), - ): - if content is not None: - return wire_type, content - return None + def _decode_expr(self) -> str | None: + return next( + ( + expr + for expr in (self.decode_varint, self.decode_length, self.decode_32bit) + if expr is not None + ), + None, + ) + + def _decode_store(self, expr: str) -> str: + return f"this->{self.field_name} = {expr};" @property def decode_content(self) -> str | None: """The decode_field() case for this field, or None when it is never decoded.""" - expr = self.decode_expr() - if expr is None: - return None - wire_type, content = expr - return self.decode_case(wire_type, f"this->{self.field_name} = {content};") + expr = self._decode_expr() + return None if expr is None else self.decode_case(self._decode_store(expr)) # Mapping from encode_func to raw encode expression template. # When a forced field has a single-byte tag, the code generator emits @@ -1008,19 +1003,13 @@ class MessageType(TypeInfo): @property def decode_content(self) -> str: # Custom decode that doesn't use templates + body = f"value.decode_to_message(this->{self.field_name});" if self._track_presence: # decode_to_message() cannot report failure, so setting the flag # afterwards only documents intent; a status-returning decode could # gate it for real without touching callers. - return self.decode_case( - WireType.LENGTH_DELIMITED, - f"value.decode_to_message(this->{self.field_name});\n" - f"this->has_{self.name} = true;", - ) - return self.decode_case( - WireType.LENGTH_DELIMITED, - f"value.decode_to_message(this->{self.field_name});", - ) + body += f"\nthis->has_{self.name} = true;" + return self.decode_case(body) def dump(self, name: str) -> str: return f"{name}.dump_to(out);" @@ -1217,7 +1206,6 @@ class PointerToBytesBufferType(PointerToBufferTypeBase): @property def decode_content(self) -> str: return self.decode_case( - WireType.LENGTH_DELIMITED, f"this->{self.field_name} = value.data();\n" f"this->{self.field_name}_len = value.size();", ) @@ -1282,7 +1270,6 @@ class PointerToStringBufferType(PointerToBufferTypeBase): @property def decode_content(self) -> str: return self.decode_case( - WireType.LENGTH_DELIMITED, f"this->{self.field_name} = StringRef(reinterpret_cast(value.data()), value.size());", ) @@ -1356,7 +1343,6 @@ class PackedBufferTypeInfo(TypeInfo): def decode_content(self) -> str: """Store pointer to buffer and calculate count of packed varints.""" return self.decode_case( - WireType.LENGTH_DELIMITED, f"this->{self.field_name}_data_ = value.data();\n" f"this->{self.field_name}_length_ = value.size();\n" f"this->{self.field_name}_count_ = count_packed_varints(value.data(), value.size());", @@ -1447,7 +1433,6 @@ class FixedArrayBytesType(TypeInfo): @property def decode_content(self) -> str: return self.decode_case( - WireType.LENGTH_DELIMITED, f"this->{self.field_name}_len = std::min(value.size(), {self.array_size});\n" f"memcpy(this->{self.field_name}, value.data(), this->{self.field_name}_len);", ) @@ -2124,6 +2109,12 @@ class RepeatedTypeInfo(TypeInfo): """ return self._ti.wire_type + def _decode_expr(self) -> str | None: + return self._ti._decode_expr() + + def _decode_store(self, expr: str) -> str: + return f"this->{self.field_name}.push_back({expr});" + @property def decode_content(self) -> str | None: # Pointer fields don't support decoding @@ -2132,17 +2123,10 @@ class RepeatedTypeInfo(TypeInfo): if isinstance(self._ti, MessageType): # Special handling for non-template message decoding return self.decode_case( - WireType.LENGTH_DELIMITED, f"this->{self.field_name}.emplace_back();\n" - f"value.decode_to_message(this->{self.field_name}.back());", + f"value.decode_to_message(this->{self.field_name}.back());" ) - expr = self._ti.decode_expr() - if expr is None: - return None - wire_type, content = expr - return self.decode_case( - wire_type, f"this->{self.field_name}.push_back({content});" - ) + return super().decode_content @property def _ti_is_bool(self) -> bool: diff --git a/tests/integration/test_api_decode_wire_types.py b/tests/integration/test_api_decode_wire_types.py index b20a96acce..af5857193c 100644 --- a/tests/integration/test_api_decode_wire_types.py +++ b/tests/integration/test_api_decode_wire_types.py @@ -3,7 +3,6 @@ not, skip unknown fields, and handle two byte tags, varints and length prefixes. from __future__ import annotations -import asyncio from collections.abc import Callable import struct @@ -18,7 +17,7 @@ from aioesphomeapi import ( import pytest from .raw_api_client import MESSAGE_TYPE_OF, RawApiClient, encode_varint -from .state_utils import InitialStateHelper, require_entity +from .state_utils import InitialStateHelper, StateWaiter, require_entity from .types import APIClientConnectedFactory, RunCompiledFunction SWITCH_COMMAND = MESSAGE_TYPE_OF[api_pb2.SwitchCommandRequest] @@ -29,10 +28,6 @@ def tag(field: int, wire_type: int) -> bytes: return encode_varint((field << 3) | wire_type) -def key_field(key: int) -> bytes: - return tag(1, WIRE_FIXED32) + struct.pack(" None: - loop = asyncio.get_running_loop() async with ( run_compiled(yaml_config), api_client_connected() as client, @@ -51,104 +45,79 @@ async def test_api_decode_wire_types( light = require_entity(entities, "wire_light") text = require_entity(entities, "wire_text") number = require_entity(entities, "wire_number") + key = tag(1, WIRE_FIXED32) + struct.pack(" None: - for pred, fut in waiters: - if not fut.done() and pred(state): - fut.set_result(state) - - async def expect(pred: Callable[[EntityState], bool]) -> EntityState: - fut: asyncio.Future[EntityState] = loop.create_future() - waiters.append((pred, fut)) - try: - return await asyncio.wait_for(fut, 5.0) - finally: - waiters.remove((pred, fut)) + if isinstance(state, SwitchState) and state.key == switch.key: + switch_states.append(state.state) + waiter.on_state(state) def switch_is(value: bool) -> Callable[[EntityState], bool]: return lambda s: ( isinstance(s, SwitchState) and s.key == switch.key and s.state is value ) + def number_is(value: float) -> Callable[[EntityState], bool]: + return lambda s: ( + isinstance(s, NumberState) and s.key == number.key and s.state == value + ) + + initial = InitialStateHelper(entities) client.subscribe_states(initial.on_state_wrapper(on_state)) await initial.wait_for_initial_states() await raw.connect() # A well formed command: fixed32 key, varint state - await raw.send_raw( - SWITCH_COMMAND, key_field(switch.key) + tag(2, WIRE_VARINT) + b"\x01" - ) - await expect(switch_is(True)) - await raw.send_raw( - SWITCH_COMMAND, key_field(switch.key) + tag(2, WIRE_VARINT) + b"\x00" - ) - await expect(switch_is(False)) + await raw.send_raw(SWITCH_COMMAND, key + on) + await waiter.expect(switch_is(True)) + await raw.send_raw(SWITCH_COMMAND, key + off) + await waiter.expect(switch_is(False)) - # The same field with the wrong wire type is dropped: a length delimited or fixed32 - # "state" must not turn the switch on, and a varint key never matches an entity + # The same field with the wrong wire type is dropped, and a varint key never matches an + # entity; each of these would turn the switch on if the payload were read as a varint + seen = len(switch_states) + await raw.send_raw(SWITCH_COMMAND, key + tag(2, WIRE_LENGTH) + b"\x01\x01") await raw.send_raw( - SWITCH_COMMAND, key_field(switch.key) + tag(2, WIRE_LENGTH) + b"\x01\x01" + SWITCH_COMMAND, key + tag(2, WIRE_FIXED32) + b"\x01\x00\x00\x00" ) await raw.send_raw( - SWITCH_COMMAND, - key_field(switch.key) + tag(2, WIRE_FIXED32) + b"\x01\x00\x00\x00", + SWITCH_COMMAND, tag(1, WIRE_VARINT) + encode_varint(switch.key) + on ) + # A later command on the same connection proves the bad frames were fully consumed; + # the number state arriving means any switch state from them would already be here + client.number_command(number.key, -77.5) + await waiter.expect(number_is(-77.5)) + assert len(switch_states) == seen + + # An unknown field ahead of the known ones is skipped; field 200 needs a two byte tag await raw.send_raw( - SWITCH_COMMAND, - tag(1, WIRE_VARINT) - + encode_varint(switch.key) - + tag(2, WIRE_VARINT) - + b"\x01", + SWITCH_COMMAND, tag(200, WIRE_VARINT) + encode_varint(300) + key + on ) - # An unknown field ahead of the known ones is skipped and the rest still decodes; - # field 200 needs a two byte tag - await raw.send_raw( - SWITCH_COMMAND, - tag(200, WIRE_VARINT) - + encode_varint(300) - + key_field(switch.key) - + tag(2, WIRE_VARINT) - + b"\x01", - ) - state = await expect(switch_is(True)) - assert state.state is True - await raw.send_raw( - SWITCH_COMMAND, key_field(switch.key) + tag(2, WIRE_VARINT) + b"\x00" - ) - await expect(switch_is(False)) + await waiter.expect(switch_is(True)) # Two byte tags (effect fields 18 and 19) and a two byte varint (300 ms transition) client.light_command( light.key, state=True, brightness=0.5, transition_length=0.3, effect="Pulse" ) - await expect( + await waiter.expect( lambda s: ( isinstance(s, LightState) and s.key == light.key and s.effect == "Pulse" ) ) client.light_command(light.key, effect="None", state=False) - await expect( + await waiter.expect( lambda s: isinstance(s, LightState) and s.key == light.key and not s.state ) # A string whose length prefix needs two varint bytes long_text = "w" * 200 client.text_command(text.key, long_text) - await expect( + await waiter.expect( lambda s: ( isinstance(s, TextState) and s.key == text.key and s.state == long_text ) ) - - # A negative fixed32 float - client.number_command(number.key, -77.5) - await expect( - lambda s: ( - isinstance(s, NumberState) and s.key == number.key and s.state == -77.5 - ) - ) diff --git a/tests/unit_tests/components/api/test_api_protobuf_generator.py b/tests/unit_tests/components/api/test_api_protobuf_generator.py index afdbc94737..aad6621b1d 100644 --- a/tests/unit_tests/components/api/test_api_protobuf_generator.py +++ b/tests/unit_tests/components/api/test_api_protobuf_generator.py @@ -188,11 +188,10 @@ def test_multi_byte_tag_fixed32_falls_back_to_the_generic_helper( def _decode_case(field_type: int, number: int) -> str: """Return the decode_field() case the generator emits for one decoded field.""" - field = descriptor_pb2.FieldDescriptorProto( - name="value", number=number, type=field_type - ) - ti = create_field_type_info(field, needs_decode=True, needs_encode=False) - return ti.decode_content + field = _field(field_type, number) + return create_field_type_info( + field, needs_decode=True, needs_encode=False + ).decode_content @pytest.mark.parametrize( @@ -236,12 +235,3 @@ def test_message_gets_a_single_decode_field_override() -> None: assert "const ProtoFieldValue value(data, scalar);" in cpp for number, wire_type in ((1, 2), (2, 0), (3, 5)): assert f"case PROTO_DECODE_CASE({number}, {wire_type}):" in cpp, cpp - - -def test_multi_statement_decode_cases_are_scoped() -> None: - """Bodies with several statements or locals get their own block so no jump crosses an initialization.""" - case = _decode_case(BYTES, 4) - lines = case.splitlines() - assert lines[0] == "case PROTO_DECODE_CASE(4, 2): {", case - assert lines[-1] == "}", case - assert "value.data();" in case and "value.size();" in case