From 53544997248cc9537384e50c5a2158581b07e6da Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 21 Aug 2026 15:24:12 -0500 Subject: [PATCH] Harden track_presence generator option and fix test fixtures Set the presence flag after the submessage decode, dump it in verbose protobuf dumps, and raise at generation time when track_presence is set on a field where it has no effect. Retire the absent-field expectation in the integration test so the UTC phase sees the first matching state, and align the Sydney TZ strings with the 03:00 fall-back the struct helper uses. --- esphome/components/api/api_pb2.cpp | 2 +- esphome/components/api/api_pb2_dump.cpp | 1 + script/api_protobuf/api_protobuf.py | 21 +++++++++++++++++-- tests/components/time/posix_tz.cpp | 8 +++---- .../test_api_get_time_response_timezone.py | 3 +++ 5 files changed, 28 insertions(+), 7 deletions(-) diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index 4f59b85260..33611c5ee1 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -1250,8 +1250,8 @@ bool ParsedTimezone::decode_length(uint32_t field_id, ProtoLengthDelimited value bool GetTimeResponse::decode_length(uint32_t field_id, ProtoLengthDelimited value) { switch (field_id) { case 3: - this->has_parsed_timezone = true; value.decode_to_message(this->parsed_timezone); + this->has_parsed_timezone = true; break; default: return false; diff --git a/esphome/components/api/api_pb2_dump.cpp b/esphome/components/api/api_pb2_dump.cpp index 8cb6a36f59..d54215ba2e 100644 --- a/esphome/components/api/api_pb2_dump.cpp +++ b/esphome/components/api/api_pb2_dump.cpp @@ -1468,6 +1468,7 @@ const char *ParsedTimezone::dump_to(DumpBuffer &out) const { const char *GetTimeResponse::dump_to(DumpBuffer &out) const { MessageDumpHelper helper(out, ESPHOME_PSTR("GetTimeResponse")); dump_field(out, ESPHOME_PSTR("epoch_seconds"), this->epoch_seconds); + dump_field(out, ESPHOME_PSTR("has_parsed_timezone"), this->has_parsed_timezone); out.append(2, ' ').append_p(ESPHOME_PSTR("parsed_timezone")).append(": "); this->parsed_timezone.dump_to(out); out.append("\n"); diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index c3037caa4e..dc3dd4b868 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -498,6 +498,15 @@ def create_field_type_info( needs_encode: bool = True, ) -> TypeInfo: """Create the appropriate TypeInfo instance for a field, handling repeated fields and custom options.""" + if get_field_opt(field, pb.track_presence, False) and ( + field.label == FieldDescriptorProto.LABEL_REPEATED + or field.type != 11 + or not needs_decode + ): + raise ValueError( + f"track_presence on field '{field.name}' has no effect; it requires " + "a non-repeated message field in a message that is decoded" + ) if field.label == FieldDescriptorProto.LABEL_REPEATED: # Check if this is a packed_buffer field (zero-copy packed repeated) if get_field_opt(field, pb.packed_buffer, False): @@ -541,6 +550,8 @@ def create_field_type_info( return PointerToStringBufferType(field, None) validate_field_type(field.type, field.name) + if field.type == 11: + return MessageType(field, needs_decode, needs_encode) return TYPE_INFO[field.type](field) @@ -955,10 +966,13 @@ class MessageType(TypeInfo): def decode_length_content(self) -> str: # Custom decode that doesn't use templates 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 ( f"case {self.number}:\n" - f" this->has_{self.name} = true;\n" f" value.decode_to_message(this->{self.field_name});\n" + f" this->has_{self.name} = true;\n" f" break;" ) return f"case {self.number}: value.decode_to_message(this->{self.field_name}); break;" @@ -968,7 +982,10 @@ class MessageType(TypeInfo): @property def dump_content(self) -> str: - o = f'out.append(2, \' \').append_p(ESPHOME_PSTR("{self.name}")).append(": ");\n' + o = "" + if self._track_presence: + o += f'dump_field(out, ESPHOME_PSTR("has_{self.name}"), this->has_{self.name});\n' + o += f'out.append(2, \' \').append_p(ESPHOME_PSTR("{self.name}")).append(": ");\n' o += f"this->{self.field_name}.dump_to(out);\n" o += 'out.append("\\n");' return o diff --git a/tests/components/time/posix_tz.cpp b/tests/components/time/posix_tz.cpp index 3a9ad7648b..760328a518 100644 --- a/tests/components/time/posix_tz.cpp +++ b/tests/components/time/posix_tz.cpp @@ -84,7 +84,7 @@ static ParsedTimezone make_new_zealand() { return tz; } -// Helper to build Australia/Sydney timezone (AEST-10AEDT,M10.1.0,M4.1.0) +// Helper to build Australia/Sydney timezone (AEST-10AEDT,M10.1.0,M4.1.0/3) static ParsedTimezone make_australia_sydney() { ParsedTimezone tz{}; tz.std_offset_seconds = -10 * 3600; @@ -674,9 +674,9 @@ TEST(RecalcTimestampLocal, FallBackRepeatedHour) { } TEST(RecalcTimestampLocal, SouthernHemisphereDST) { - // Set timezone to Australia/Sydney (AEST-10AEDT,M10.1.0,M4.1.0) + // Set timezone to Australia/Sydney (AEST-10AEDT,M10.1.0,M4.1.0/3) // DST starts first Sunday of October, ends first Sunday of April - const char *tz_str = "AEST-10AEDT,M10.1.0,M4.1.0"; + const char *tz_str = "AEST-10AEDT,M10.1.0,M4.1.0/3"; setenv("TZ", tz_str, 1); tzset(); auto tz = make_australia_sydney(); @@ -817,7 +817,7 @@ TEST(RecalcTimestampLocal, MinimalFieldsNoDST) { TEST(RecalcTimestampLocal, YearBoundaryDST) { // Test southern hemisphere DST across year boundary // Australia/Sydney: DST active from October to April (spans Jan 1) - const char *tz_str = "AEST-10AEDT,M10.1.0,M4.1.0"; + const char *tz_str = "AEST-10AEDT,M10.1.0,M4.1.0/3"; setenv("TZ", tz_str, 1); tzset(); auto tz = make_australia_sydney(); diff --git a/tests/integration/test_api_get_time_response_timezone.py b/tests/integration/test_api_get_time_response_timezone.py index 3aa5c374af..c90380607f 100644 --- a/tests/integration/test_api_get_time_response_timezone.py +++ b/tests/integration/test_api_get_time_response_timezone.py @@ -55,6 +55,9 @@ async def test_api_get_time_response_timezone( client._connection.send_messages((resp,)) await tracker.await_must_not_change(future, "tz_offset", timeout=1.0) assert tracker.sensor_states["tz_offset"][-1] == target + # Retire the expectation so it cannot swallow the first matching state + # meant for the next phase + future.cancel() # Present but all zero (genuine UTC): applied future = tracker.expect("tz_offset", 0)