mirror of
https://github.com/esphome/esphome.git
synced 2026-09-11 15:27:33 +00:00
[api] Drop the unused base decode(), group the vtable asserts, and test the decode emitters
A generated decodable message that lost its decode() would now fail to compile instead of silently keeping its defaults. The asserts cover exactly the classes that derive from ProtoDecodableMessage, under one ifdef per run instead of one per line, and the generator tests pin the inline decode() wrapper, its absence on fixed vector messages, and the this-> free static body.
This commit is contained in:
@@ -4191,9 +4191,6 @@ uint32_t BluetoothSetConnectionParamsResponse::calc_size_msg(const void *self) {
|
||||
#ifndef HAS_PROTO_MESSAGE_DUMP
|
||||
static_assert(!std::is_polymorphic_v<HelloRequest>, "decodable messages carry no vtable");
|
||||
static_assert(!std::is_polymorphic_v<DisconnectRequest>, "decodable messages carry no vtable");
|
||||
static_assert(!std::is_polymorphic_v<DisconnectResponse>, "decodable messages carry no vtable");
|
||||
static_assert(!std::is_polymorphic_v<PingRequest>, "decodable messages carry no vtable");
|
||||
static_assert(!std::is_polymorphic_v<PingResponse>, "decodable messages carry no vtable");
|
||||
#ifdef USE_COVER
|
||||
static_assert(!std::is_polymorphic_v<CoverCommandRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
@@ -4221,8 +4218,6 @@ static_assert(!std::is_polymorphic_v<ParsedTimezone>, "decodable messages carry
|
||||
static_assert(!std::is_polymorphic_v<GetTimeResponse>, "decodable messages carry no vtable");
|
||||
#ifdef USE_API_USER_DEFINED_ACTIONS
|
||||
static_assert(!std::is_polymorphic_v<ExecuteServiceArgument>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_API_USER_DEFINED_ACTIONS
|
||||
static_assert(!std::is_polymorphic_v<ExecuteServiceRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_CAMERA
|
||||
@@ -4257,23 +4252,11 @@ static_assert(!std::is_polymorphic_v<SubscribeBluetoothLEAdvertisementsRequest>,
|
||||
#endif
|
||||
#ifdef USE_BLUETOOTH_PROXY_CONNECTIONS
|
||||
static_assert(!std::is_polymorphic_v<BluetoothDeviceRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_BLUETOOTH_PROXY_CONNECTIONS
|
||||
static_assert(!std::is_polymorphic_v<BluetoothGATTGetServicesRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_BLUETOOTH_PROXY_CONNECTIONS
|
||||
static_assert(!std::is_polymorphic_v<BluetoothGATTReadRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_BLUETOOTH_PROXY_CONNECTIONS
|
||||
static_assert(!std::is_polymorphic_v<BluetoothGATTWriteRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_BLUETOOTH_PROXY_CONNECTIONS
|
||||
static_assert(!std::is_polymorphic_v<BluetoothGATTReadDescriptorRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_BLUETOOTH_PROXY_CONNECTIONS
|
||||
static_assert(!std::is_polymorphic_v<BluetoothGATTWriteDescriptorRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_BLUETOOTH_PROXY_CONNECTIONS
|
||||
static_assert(!std::is_polymorphic_v<BluetoothGATTNotifyRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_BLUETOOTH_PROXY
|
||||
@@ -4281,32 +4264,14 @@ static_assert(!std::is_polymorphic_v<BluetoothScannerSetModeRequest>, "decodable
|
||||
#endif
|
||||
#ifdef USE_VOICE_ASSISTANT
|
||||
static_assert(!std::is_polymorphic_v<SubscribeVoiceAssistantRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_VOICE_ASSISTANT
|
||||
static_assert(!std::is_polymorphic_v<VoiceAssistantResponse>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_VOICE_ASSISTANT
|
||||
static_assert(!std::is_polymorphic_v<VoiceAssistantEventData>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_VOICE_ASSISTANT
|
||||
static_assert(!std::is_polymorphic_v<VoiceAssistantEventResponse>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_VOICE_ASSISTANT
|
||||
static_assert(!std::is_polymorphic_v<VoiceAssistantAudio>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_VOICE_ASSISTANT
|
||||
static_assert(!std::is_polymorphic_v<VoiceAssistantTimerEventResponse>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_VOICE_ASSISTANT
|
||||
static_assert(!std::is_polymorphic_v<VoiceAssistantAnnounceRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_VOICE_ASSISTANT
|
||||
static_assert(!std::is_polymorphic_v<VoiceAssistantExternalWakeWord>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_VOICE_ASSISTANT
|
||||
static_assert(!std::is_polymorphic_v<VoiceAssistantConfigurationRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_VOICE_ASSISTANT
|
||||
static_assert(!std::is_polymorphic_v<VoiceAssistantSetConfiguration>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_ALARM_CONTROL_PANEL
|
||||
@@ -4332,8 +4297,6 @@ static_assert(!std::is_polymorphic_v<UpdateCommandRequest>, "decodable messages
|
||||
#endif
|
||||
#ifdef USE_ZWAVE_PROXY
|
||||
static_assert(!std::is_polymorphic_v<ZWaveProxyFrame>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_ZWAVE_PROXY
|
||||
static_assert(!std::is_polymorphic_v<ZWaveProxyRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#if defined(USE_IR_RF) || defined(USE_RADIO_FREQUENCY)
|
||||
@@ -4341,17 +4304,9 @@ static_assert(!std::is_polymorphic_v<InfraredRFTransmitRawTimingsRequest>, "deco
|
||||
#endif
|
||||
#ifdef USE_SERIAL_PROXY
|
||||
static_assert(!std::is_polymorphic_v<SerialProxyConfigureRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_SERIAL_PROXY
|
||||
static_assert(!std::is_polymorphic_v<SerialProxyWriteRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_SERIAL_PROXY
|
||||
static_assert(!std::is_polymorphic_v<SerialProxySetModemPinsRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_SERIAL_PROXY
|
||||
static_assert(!std::is_polymorphic_v<SerialProxyGetModemPinsRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_SERIAL_PROXY
|
||||
static_assert(!std::is_polymorphic_v<SerialProxyRequest>, "decodable messages carry no vtable");
|
||||
#endif
|
||||
#ifdef USE_BLUETOOTH_PROXY_CONNECTIONS
|
||||
|
||||
@@ -728,8 +728,6 @@ class ProtoDecodableMessage : public ProtoMessage {
|
||||
/// Walk \p buffer and hand every field to \p field. The generated decode() passes the message's
|
||||
/// own decode_field, so decodable messages carry no vtable.
|
||||
static void decode_fields(void *msg, const uint8_t *buffer, size_t length, DecodeFieldFn field);
|
||||
/// A decodable message without fields has nothing to read
|
||||
void decode(const uint8_t *buffer, size_t length) {}
|
||||
|
||||
/**
|
||||
* Count occurrences of a repeated field in a protobuf buffer.
|
||||
|
||||
@@ -2536,8 +2536,8 @@ def build_message_type(
|
||||
|
||||
# Get source direction to determine if we need decode/encode methods
|
||||
source = message_source_map[desc.name]
|
||||
needs_decode = source in (SOURCE_BOTH, SOURCE_CLIENT)
|
||||
needs_encode = source in (SOURCE_BOTH, SOURCE_SERVER)
|
||||
needs_decode = message_needs_decode(source)
|
||||
needs_encode = message_needs_encode(source)
|
||||
|
||||
# Add MESSAGE_TYPE method if this is a service message
|
||||
if message_id is not None:
|
||||
@@ -2846,6 +2846,23 @@ def get_field_opt(
|
||||
return field.options.Extensions[opt]
|
||||
|
||||
|
||||
def message_needs_decode(source: int) -> bool:
|
||||
return source in (SOURCE_BOTH, SOURCE_CLIENT)
|
||||
|
||||
|
||||
def message_needs_encode(source: int) -> bool:
|
||||
return source in (SOURCE_BOTH, SOURCE_SERVER)
|
||||
|
||||
|
||||
def is_decodable_class(desc: descriptor.DescriptorProto, source: int) -> bool:
|
||||
"""Whether the generated class derives from ProtoDecodableMessage: decoded, and either on a
|
||||
decodable base class or with at least one live field."""
|
||||
return message_needs_decode(source) and (
|
||||
get_base_class(desc) is not None
|
||||
or any(not field.options.deprecated for field in desc.field)
|
||||
)
|
||||
|
||||
|
||||
def get_base_class(desc: descriptor.DescriptorProto) -> str | None:
|
||||
"""Get the base_class option from a message descriptor."""
|
||||
if not desc.options.HasExtension(pb.base_class):
|
||||
@@ -3402,7 +3419,7 @@ static void dump_bytes_field(DumpBuffer &out, const char *field_name, const uint
|
||||
continue
|
||||
|
||||
s, c, dc = build_message_type(m, base_class_fields, message_source_map)
|
||||
if message_source_map[m.name] in (SOURCE_BOTH, SOURCE_CLIENT):
|
||||
if is_decodable_class(m, message_source_map[m.name]):
|
||||
decodable_messages.append((m.name, message_ifdef_map.get(m.name)))
|
||||
msg_ifdef = message_ifdef_map.get(m.name)
|
||||
|
||||
@@ -3433,9 +3450,17 @@ static void dump_bytes_field(DumpBuffer &out, const char *field_name, const uint
|
||||
# decode() passes decode_field explicitly, so without the dump virtuals no decodable message
|
||||
# may carry a vtable; a build at any level below VERY_VERBOSE proves it
|
||||
cpp += "#ifndef HAS_PROTO_MESSAGE_DUMP\n"
|
||||
assert_ifdef = None
|
||||
for name, msg_ifdef in decodable_messages:
|
||||
line = f'static_assert(!std::is_polymorphic_v<{name}>, "decodable messages carry no vtable");'
|
||||
cpp += "\n".join(wrap_with_ifdef(line, msg_ifdef)) + "\n"
|
||||
if msg_ifdef != assert_ifdef:
|
||||
if assert_ifdef is not None:
|
||||
cpp += "#endif\n"
|
||||
if msg_ifdef is not None:
|
||||
cpp += _make_ifdef_line(msg_ifdef) + "\n"
|
||||
assert_ifdef = msg_ifdef
|
||||
cpp += f'static_assert(!std::is_polymorphic_v<{name}>, "decodable messages carry no vtable");\n'
|
||||
if assert_ifdef is not None:
|
||||
cpp += "#endif\n"
|
||||
cpp += "#endif\n"
|
||||
|
||||
content += """\
|
||||
|
||||
@@ -285,8 +285,9 @@ def test_a_fixed64_field_fails_at_generation_time() -> None:
|
||||
build_message_type(desc, {}, {"Wide": SOURCE_CLIENT})
|
||||
|
||||
|
||||
def test_message_gets_a_single_decode_field_override() -> None:
|
||||
"""All wire types of a decoded message land in one decode_field() switch."""
|
||||
def test_message_decodes_through_one_static_decode_field() -> None:
|
||||
"""All wire types of a decoded message land in one static decode_field() switch that the
|
||||
inline decode() hands to the shared loop."""
|
||||
desc = descriptor_pb2.DescriptorProto(name="Mixed")
|
||||
desc.field.add(name="name", number=1, type=STRING)
|
||||
desc.field.add(name="count", number=2, type=UINT32)
|
||||
@@ -308,3 +309,34 @@ def test_message_gets_a_single_decode_field_override() -> None:
|
||||
(3, "WIRE_TYPE_FIXED32"),
|
||||
):
|
||||
assert f"case proto_tag({number}, {wire_type}):" in cpp, cpp
|
||||
# The static body works on the cast message, never on this
|
||||
assert "auto &msg = *static_cast<Mixed *>(self);" in cpp
|
||||
assert "this->" not in cpp
|
||||
assert (
|
||||
header.count(
|
||||
"ProtoDecodableMessage::decode_fields(this, buffer, length, &decode_field);"
|
||||
)
|
||||
== 1
|
||||
)
|
||||
|
||||
|
||||
def test_fixed_vector_message_keeps_its_own_decode() -> None:
|
||||
"""A message that sizes a FixedVector first decodes through its own decode(), not the inline one."""
|
||||
desc = descriptor_pb2.DescriptorProto(name="Sized")
|
||||
field = desc.field.add(name="values", number=1, type=UINT32)
|
||||
field.label = descriptor_pb2.FieldDescriptorProto.LABEL_REPEATED
|
||||
field.options.Extensions[pb.fixed_vector] = True
|
||||
header, cpp, _ = build_message_type(desc, {}, {"Sized": SOURCE_CLIENT})
|
||||
assert (
|
||||
"ProtoDecodableMessage::decode_fields(this, buffer, length, &decode_field);"
|
||||
not in header
|
||||
)
|
||||
assert header.count("void decode(const uint8_t *buffer, size_t length);") == 1
|
||||
assert "void Sized::decode(const uint8_t *buffer, size_t length) {" in cpp
|
||||
assert "ProtoDecodableMessage::count_repeated_field(buffer, length, 1)" in cpp
|
||||
assert (
|
||||
cpp.count(
|
||||
"ProtoDecodableMessage::decode_fields(this, buffer, length, &decode_field);"
|
||||
)
|
||||
== 1
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user