diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index c32efc27962..5c29733f342 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -537,22 +537,17 @@ def create_field_type_info( "a non-repeated message field in a message that is decoded" ) if get_field_opt(field, pb.progmem, False): - is_short_string = ( - field.type == FieldDescriptorProto.TYPE_STRING - and get_field_opt(field, pb.force, False) - and (get_field_opt(field, pb.max_data_length) or 128) < 128 - ) is_pointer_bytes = ( field.type == FieldDescriptorProto.TYPE_BYTES and get_field_opt(field, pb.pointer_to_buffer, False) and get_field_opt(field, pb.fixed_array_size) is None ) if field.label == FieldDescriptorProto.LABEL_REPEATED or not ( - is_short_string or is_pointer_bytes + field.type == FieldDescriptorProto.TYPE_STRING or is_pointer_bytes ): raise ValueError( - f"progmem on field '{field.name}' requires a forced string field with " - "max_data_length below 128, or a bytes field with pointer_to_buffer" + f"progmem on field '{field.name}' requires a non-repeated string field, " + "or a bytes field with pointer_to_buffer" ) if field.label == FieldDescriptorProto.LABEL_REPEATED: # Check if this is a packed_buffer field (zero-copy packed repeated) @@ -1288,6 +1283,14 @@ class PointerToStringBufferType(PointerToBufferTypeBase): else "encode_short_string_force" ) return _encode_call(encode_fn, str(tag), f"this->{self.field_name}") + if self.progmem: + return _encode_call( + "encode_progmem_bytes", + str(self.number), + f"reinterpret_cast(this->{self.field_name}.progmem_ptr())", + f"this->{self.field_name}.size()", + force=self.force, + ) if result := self._encode_bytes_with_precomputed_tag( f"this->{self.field_name}.c_str()", f"this->{self.field_name}.size()", diff --git a/tests/components/core/test_entity_base_name.cpp b/tests/components/core/test_entity_base_name.cpp new file mode 100644 index 00000000000..18af5278a14 --- /dev/null +++ b/tests/components/core/test_entity_base_name.cpp @@ -0,0 +1,16 @@ +#include + +#include "esphome/core/entity_base.h" + +#include +#include + +namespace esphome::core::testing { + +// External components return get_name() from `const StringRef &` getters, so it must stay a reference +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +static_assert(std::is_same_v().get_name()), const StringRef &>); +#pragma GCC diagnostic pop + +} // namespace esphome::core::testing diff --git a/tests/components/core/test_progmem_string_ref.cpp b/tests/components/core/test_progmem_string_ref.cpp new file mode 100644 index 00000000000..fe6dd0e24a2 --- /dev/null +++ b/tests/components/core/test_progmem_string_ref.cpp @@ -0,0 +1,49 @@ +#include + +#include "esphome/core/string_ref.h" + +#include +#include + +namespace esphome::core::testing { + +// ram_ref() must return a reference so callers can hand it out as `const StringRef &` +static_assert(std::is_same_v().ram_ref()), const StringRef &>); + +TEST(ProgmemStringRef, WriteToCopiesAndTerminates) { + ProgmemStringRef ref("Kitchen", 7); + char buf[16]; + EXPECT_EQ(ref.write_to(buf, sizeof(buf)), 7u); + EXPECT_STREQ(buf, "Kitchen"); +} + +TEST(ProgmemStringRef, WriteToTruncatesToFit) { + ProgmemStringRef ref("Kitchen", 7); + char buf[4]; + EXPECT_EQ(ref.write_to(buf, sizeof(buf)), 3u); + EXPECT_STREQ(buf, "Kit"); +} + +TEST(ProgmemStringRef, WriteToZeroSizeWritesNothing) { + ProgmemStringRef ref("Kitchen", 7); + char buf[1] = {'x'}; + EXPECT_EQ(ref.write_to(buf, 0), 0u); + EXPECT_EQ(buf[0], 'x'); +} + +TEST(ProgmemStringRef, EqualsComparesLengthAndContent) { + ProgmemStringRef ref("Kitchen", 7); + EXPECT_TRUE(ref.equals(StringRef("Kitchen"))); + EXPECT_FALSE(ref.equals(StringRef("Kitche"))); + EXPECT_FALSE(ref.equals(StringRef("Kitchens"))); + EXPECT_FALSE(ref.equals(StringRef("Bedroom"))); +} + +TEST(ProgmemStringRef, DefaultIsEmpty) { + ProgmemStringRef ref; + EXPECT_TRUE(ref.empty()); + EXPECT_EQ(ref.size(), 0u); + EXPECT_TRUE(ref.equals(StringRef(""))); +} + +} // namespace esphome::core::testing diff --git a/tests/components/core/test_string_ref.cpp b/tests/components/core/test_string_ref.cpp index aaecbb662d8..f66800dd178 100644 --- a/tests/components/core/test_string_ref.cpp +++ b/tests/components/core/test_string_ref.cpp @@ -1,12 +1,9 @@ #include -#include "esphome/core/entity_base.h" #include "esphome/core/string_ref.h" #include #include -#include -#include namespace esphome::core::testing { @@ -164,47 +161,4 @@ TEST(StringRefNullEmpty, NumericConversionsReturnZero) { EXPECT_EQ(stof(null_empty), 0.0f); } -TEST(ProgmemStringRef, WriteToCopiesAndTerminates) { - ProgmemStringRef ref("Kitchen", 7); - char buf[16]; - EXPECT_EQ(ref.write_to(buf, sizeof(buf)), 7u); - EXPECT_STREQ(buf, "Kitchen"); -} - -TEST(ProgmemStringRef, WriteToTruncatesToFit) { - ProgmemStringRef ref("Kitchen", 7); - char buf[4]; - EXPECT_EQ(ref.write_to(buf, sizeof(buf)), 3u); - EXPECT_STREQ(buf, "Kit"); -} - -TEST(ProgmemStringRef, WriteToZeroSizeWritesNothing) { - ProgmemStringRef ref("Kitchen", 7); - char buf[1] = {'x'}; - EXPECT_EQ(ref.write_to(buf, 0), 0u); - EXPECT_EQ(buf[0], 'x'); -} - -TEST(ProgmemStringRef, EqualsComparesLengthAndContent) { - ProgmemStringRef ref("Kitchen", 7); - EXPECT_TRUE(ref.equals(StringRef("Kitchen"))); - EXPECT_FALSE(ref.equals(StringRef("Kitche"))); - EXPECT_FALSE(ref.equals(StringRef("Kitchens"))); - EXPECT_FALSE(ref.equals(StringRef("Bedroom"))); -} - -// get_name() must keep returning a reference: external code returns it from `const StringRef &` getters -static_assert(std::is_same_v().ram_ref()), const StringRef &>); -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wdeprecated-declarations" -static_assert(std::is_same_v().get_name()), const StringRef &>); -#pragma GCC diagnostic pop - -TEST(ProgmemStringRef, DefaultIsEmpty) { - ProgmemStringRef ref; - EXPECT_TRUE(ref.empty()); - EXPECT_EQ(ref.size(), 0u); - EXPECT_TRUE(ref.equals(StringRef(""))); -} - } // namespace esphome::core::testing 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 93a21bd985f..4927b620205 100644 --- a/tests/unit_tests/components/api/test_api_protobuf_generator.py +++ b/tests/unit_tests/components/api/test_api_protobuf_generator.py @@ -415,27 +415,32 @@ def test_progmem_rejected_on_non_bytes_field() -> None: create_field_type_info(field, needs_decode=False) -def test_progmem_string_field_copies_from_flash() -> None: - """A short forced (progmem) string field encodes and dumps through the progmem helpers.""" - field = _field(9, force=True) - field.options.Extensions[pb.progmem] = True - field.options.Extensions[pb.max_data_length] = 120 - ti = create_field_type_info(field, needs_decode=False) - assert "ProgmemStringRef value{};" in ti.public_content - assert "encode_short_progmem_string_force(" in ti.encode_content - assert "dump_progmem_string_field(" in ti.dump_content - - @pytest.mark.parametrize( - ("force", "max_len"), [(False, 120), (True, None), (True, 200)] + ("force", "max_len", "encode_fn"), + [ + (True, 120, "encode_short_progmem_string_force("), + (False, None, "encode_progmem_bytes("), + (True, None, "encode_progmem_bytes_force("), + ], ) -def test_progmem_string_requires_short_forced_field( - force: bool, max_len: int | None +def test_progmem_string_field_copies_from_flash( + force: bool, max_len: int | None, encode_fn: str ) -> None: - """A (progmem) string field must take the short encode path, the only one that reads flash.""" + """A (progmem) string field is a ProgmemStringRef that encodes and dumps through the progmem helpers.""" field = _field(9, force=force) field.options.Extensions[pb.progmem] = True if max_len is not None: field.options.Extensions[pb.max_data_length] = max_len + ti = create_field_type_info(field, needs_decode=False) + assert "ProgmemStringRef value{};" in ti.public_content + assert encode_fn in ti.encode_content + assert "encode_string" not in ti.encode_content + assert "dump_progmem_string_field(" in ti.dump_content + + +def test_progmem_rejected_on_repeated_string() -> None: + """(progmem) on a repeated string field fails instead of silently using memcpy.""" + field = _field(9, repeated=True) + field.options.Extensions[pb.progmem] = True with pytest.raises(ValueError, match="progmem on field 'value'"): create_field_type_info(field, needs_decode=False)