Keep the RAM short string encode on memcpy and drop unused ProgmemStringRef helpers

This commit is contained in:
J. Nick Koston
2026-10-12 16:47:08 -10:00
parent dd495be861
commit 9b873e2179
5 changed files with 7 additions and 72 deletions
-12
View File
@@ -409,10 +409,6 @@ class ProtoEncode {
/// Tag must be a single-byte varint (< 128). Always encodes (no zero check).
[[nodiscard]] static inline uint8_t *encode_short_string_force(uint8_t *__restrict__ pos PROTO_ENCODE_DEBUG_PARAM,
uint8_t tag, const StringRef &ref) {
#ifdef USE_ESP8266
// memcpy_P reads RAM too, so RAM strings share the progmem copy
return encode_short_progmem_string_force(pos PROTO_ENCODE_DEBUG_ARG, tag, ProgmemStringRef(ref));
#else
#ifdef ESPHOME_DEBUG_API
assert(ref.size() < 128 && "encode_short_string_force: string exceeds max_data_length < 128");
#endif
@@ -421,10 +417,8 @@ class ProtoEncode {
pos[1] = static_cast<uint8_t>(ref.size());
std::memcpy(pos + 2, ref.c_str(), ref.size());
return pos + 2 + ref.size();
#endif
}
/// encode_short_string_force for a string that may be in flash (PROGMEM).
#ifdef USE_ESP8266
[[nodiscard]] static inline uint8_t *encode_short_progmem_string_force(
uint8_t *__restrict__ pos PROTO_ENCODE_DEBUG_PARAM, uint8_t tag, const ProgmemStringRef &ref) {
#ifdef ESPHOME_DEBUG_API
@@ -436,12 +430,6 @@ class ProtoEncode {
progmem_memcpy(pos + 2, ref.progmem_ptr(), ref.size());
return pos + 2 + ref.size();
}
#else
[[nodiscard]] static inline uint8_t *encode_short_progmem_string_force(
uint8_t *__restrict__ pos PROTO_ENCODE_DEBUG_PARAM, uint8_t tag, const ProgmemStringRef &ref) {
return encode_short_string_force(pos PROTO_ENCODE_DEBUG_ARG, tag, ref.ram_ref());
}
#endif
/// Write a precomputed tag byte + 32-bit value. Outlined on embedded: one copy beats inline stores per field.
[[nodiscard]] static PROTO_OUTLINE_FOR_SIZE uint8_t *write_tag_and_fixed32(
uint8_t *__restrict__ pos PROTO_ENCODE_DEBUG_PARAM, uint8_t tag, uint32_t value) {
+1 -20
View File
@@ -226,7 +226,7 @@ inline std::string operator+(const std::string &lhs, const StringRef &rhs) {
}
/// A string that may live in flash (on ESP8266): pointer and length, with no byte-reading operations.
/// progmem_ptr() is safe as a "%s" log argument; read the contents with write_to() or equals().
/// progmem_ptr() is safe as a "%s" log argument; compare the contents with equals().
class ProgmemStringRef {
public:
constexpr ProgmemStringRef() = default;
@@ -237,20 +237,6 @@ class ProgmemStringRef {
constexpr size_t size() const { return this->ref_.size(); }
constexpr bool empty() const { return this->ref_.empty(); }
/// Copy into buf (null terminated, truncated to fit), returns the length copied.
size_t write_to(char *buf, size_t buf_size) const {
if (buf_size == 0)
return 0;
size_t len = std::min(this->ref_.size(), buf_size - 1);
#ifdef USE_ESP8266
memcpy_P(buf, this->ref_.c_str(), len);
#else
std::memcpy(buf, this->ref_.c_str(), len);
#endif
buf[len] = '\0';
return len;
}
bool equals(const StringRef &other) const {
#ifdef USE_ESP8266
// A StringRef may be {nullptr, 0}, and memcmp_P with a null pointer is undefined even for 0 bytes
@@ -261,11 +247,6 @@ class ProgmemStringRef {
#endif
}
#ifndef USE_ESP8266
/// The string is in RAM here, so it can be viewed directly.
constexpr const StringRef &ram_ref() const { return this->ref_; }
#endif
// Remove before 2027.5.0
[[deprecated(
"May point to flash; use progmem_ptr() or the owner's accessors. Removed in 2027.5.0")]] constexpr const char *
+5 -10
View File
@@ -1158,6 +1158,11 @@ class PointerToBufferTypeBase(TypeInfo):
"""Get the wire type for this field."""
return WireType.LENGTH_DELIMITED # Uses wire type 2
@property
def progmem(self) -> bool:
"""Whether the data is in flash, so encode and dump copy it with progmem_memcpy."""
return get_field_opt(self._field, pb.progmem, False)
def get_estimated_size(self) -> int:
# field ID + length varint + typical data (assume small for pointer fields)
return self.calculate_field_id_size() + 2 + 16
@@ -1171,11 +1176,6 @@ class PointerToBytesBufferType(PointerToBufferTypeBase):
reference_type = "const uint8_t*"
const_reference_type = "const uint8_t*"
@property
def progmem(self) -> bool:
"""Whether the data is in flash, so encode and dump copy it with progmem_memcpy."""
return get_field_opt(self._field, pb.progmem, False)
@property
def public_content(self) -> list[str]:
# Use uint16_t for length - max packet size is well below 65535
@@ -1253,11 +1253,6 @@ class PointerToStringBufferType(PointerToBufferTypeBase):
one zero fill. Any encode path that copies unconditionally must check this."""
return not self._needs_decode and not self.force
@property
def progmem(self) -> bool:
"""Whether the string is in flash, so encode and dump copy it with progmem_memcpy."""
return get_field_opt(self._field, pb.progmem, False)
@property
def public_content(self) -> list[str]:
if self.progmem:
@@ -15,7 +15,6 @@ from esphome.config_validation import Invalid
from esphome.core import CORE
from esphome.cpp_generator import safe_exp
from esphome.helpers import fnv1_hash
from tests.component_tests.helpers import get_define_value
CONFIG = "tests/component_tests/api/test_action_metadata.yaml"
CONFIG_ESP8266 = "tests/component_tests/api/test_action_metadata_esp8266.yaml"
@@ -67,11 +66,10 @@ def test_esp8266_strings_are_shared_progmem_arrays(
def test_esp8266_encodes_action_strings_from_flash(
generate_main: Callable[[str | Path], str],
) -> None:
"""ESP8266 uses the same flash string table, with no scratch buffer to size."""
"""ESP8266 uses the same flash string table."""
main_cpp = generate_main(CONFIG_ESP8266)
assert "api_action0_strings[] PROGMEM" in main_cpp
assert get_define_value("API_USER_ACTION_STRINGS_SCRATCH_SIZE") is None
def test_shorthand_variables_emit_no_metadata(
@@ -2,35 +2,8 @@
#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")));