Allow progmem on any string field and split the ProgmemStringRef tests

Long or unforced progmem strings encode through encode_progmem_bytes, which
API action strings need. The ProgmemStringRef tests move to their own file.
This commit is contained in:
J. Nick Koston
2026-10-08 20:04:18 -10:00
parent 140488723d
commit 88f7fc7454
5 changed files with 96 additions and 69 deletions
+11 -8
View File
@@ -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<const uint8_t *>(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()",
@@ -0,0 +1,16 @@
#include <gtest/gtest.h>
#include "esphome/core/entity_base.h"
#include <type_traits>
#include <utility>
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<decltype(std::declval<const EntityBase &>().get_name()), const StringRef &>);
#pragma GCC diagnostic pop
} // namespace esphome::core::testing
@@ -0,0 +1,49 @@
#include <gtest/gtest.h>
#include "esphome/core/string_ref.h"
#include <type_traits>
#include <utility>
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<decltype(std::declval<const ProgmemStringRef &>().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
-46
View File
@@ -1,12 +1,9 @@
#include <gtest/gtest.h>
#include "esphome/core/entity_base.h"
#include "esphome/core/string_ref.h"
#include <iterator>
#include <string>
#include <type_traits>
#include <utility>
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<decltype(std::declval<const ProgmemStringRef &>().ram_ref()), const StringRef &>);
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
static_assert(std::is_same_v<decltype(std::declval<const EntityBase &>().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
@@ -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)