From 7aeed8455e47b8f0cf804492081b02488ffa190e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 7 Oct 2026 17:24:27 -1000 Subject: [PATCH] [api] Add a progmem option for bytes fields kept in flash (#20318) --- esphome/components/api/api.proto | 4 +- esphome/components/api/api_connection.h | 3 -- esphome/components/api/api_options.proto | 5 ++ esphome/components/api/api_pb2.cpp | 2 +- esphome/components/api/api_pb2_dump.cpp | 17 ++++-- esphome/components/api/api_wizard.cpp | 21 +------- esphome/components/api/api_wizard.h | 4 -- esphome/components/api/proto.h | 16 ++++++ script/api_protobuf/api_protobuf.py | 46 ++++++++++++++-- tests/components/api/test_wizard.cpp | 6 +-- .../api/test_api_protobuf_generator.py | 53 +++++++++++++++++++ 11 files changed, 138 insertions(+), 39 deletions(-) diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index 2648b5d0d8..0d2ac36937 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -468,11 +468,9 @@ message DeviceWizardResponse { option (id) = 157; option (source) = SOURCE_SERVER; option (ifdef) = "USE_API_WIZARD"; - // Not logged: the data is in flash, which ESP8266 cannot read for a dump - option (log) = false; // The data is in flash, and the device sends it from there - bytes data = 1 [(pointer_to_buffer) = true]; + bytes data = 1 [(pointer_to_buffer) = true, (progmem) = true]; } // Sets the entity id of a wizard input (see WizardInputField.key). Clients send diff --git a/esphome/components/api/api_connection.h b/esphome/components/api/api_connection.h index 4d927b2885..b7f0bda0ed 100644 --- a/esphome/components/api/api_connection.h +++ b/esphome/components/api/api_connection.h @@ -423,9 +423,6 @@ class APIConnection final : public APIServerConnectionBase { bool send_ping_response_(); bool send_device_info_response_(); bool send_device_capabilities_response_(); -#ifdef USE_API_WIZARD - bool send_device_wizard_response_(); -#endif #ifdef USE_API_NOISE bool send_noise_encryption_set_key_response_(const NoiseEncryptionSetKeyRequest &msg); #endif diff --git a/esphome/components/api/api_options.proto b/esphome/components/api/api_options.proto index 66295b3d53..8e8fa99833 100644 --- a/esphome/components/api/api_options.proto +++ b/esphome/components/api/api_options.proto @@ -122,4 +122,9 @@ extend google.protobuf.FieldOptions { // to true when the field arrives, so an all-default submessage can be told apart // from an absent one (e.g. a UTC ParsedTimezone, which is all zeros). optional bool track_presence = 50020 [default=false]; + + // progmem: The pointer_to_buffer bytes field points into flash (PROGMEM). + // The generated encode and dump copy it with progmem_memcpy, as ESP8266 + // cannot read flash with ordinary loads. + optional bool progmem = 50021 [default=false]; } diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index 67a5d9792e..646a8f4211 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -352,7 +352,7 @@ uint32_t DeviceCapabilitiesResponse::calc_size_msg(const void *self) { #ifdef USE_API_WIZARD uint8_t *DeviceWizardResponse::encode_msg(const void *self, uint8_t *__restrict__ pos PROTO_ENCODE_DEBUG_PARAM) { const auto &msg = *static_cast(self); - pos = ProtoEncode::encode_bytes(pos PROTO_ENCODE_DEBUG_ARG, 1, msg.data, msg.data_len); + pos = ProtoEncode::encode_progmem_bytes(pos PROTO_ENCODE_DEBUG_ARG, 1, msg.data, msg.data_len); return pos; } uint32_t DeviceWizardResponse::calc_size_msg(const void *self) { diff --git a/esphome/components/api/api_pb2_dump.cpp b/esphome/components/api/api_pb2_dump.cpp index 966ba8f2bf..196a4249cc 100644 --- a/esphome/components/api/api_pb2_dump.cpp +++ b/esphome/components/api/api_pb2_dump.cpp @@ -114,15 +114,26 @@ template static void dump_field(DumpBuffer &out, const char *field_n out.append("\n"); } +// Bytes shown by a bytes field dump: 160 bytes is 480 chars with separators, to fit a typical log buffer +static constexpr size_t DUMP_BYTES_MAX = 160; + // Helper for bytes fields - uses stack buffer to avoid heap allocation -// Buffer sized for 160 bytes of data (480 chars with separators) to fit typical log buffer // field_name is a PROGMEM pointer (flash on ESP8266, regular pointer on other platforms) static void dump_bytes_field(DumpBuffer &out, const char *field_name, const uint8_t *data, size_t len, int indent = 2) { - char hex_buf[format_hex_pretty_size(160)]; + char hex_buf[format_hex_pretty_size(DUMP_BYTES_MAX)]; append_field_prefix(out, field_name, indent); format_hex_pretty_to(hex_buf, data, len); out.append(hex_buf).append("\n"); } + +// Helper for bytes fields in flash: copies the shown bytes out with progmem_memcpy first +static void dump_progmem_bytes_field(DumpBuffer &out, const char *field_name, const uint8_t *data, size_t len, + int indent = 2) { + uint8_t data_buf[DUMP_BYTES_MAX]; + len = std::min(len, sizeof(data_buf)); + progmem_memcpy(data_buf, data, len); + dump_bytes_field(out, field_name, data_buf, len, indent); +} #pragma GCC diagnostic pop template<> const char *proto_enum_to_string(enums::DisconnectReason value) { @@ -1119,7 +1130,7 @@ const char *DeviceCapabilitiesResponse::dump_to(DumpBuffer &out) const { #ifdef USE_API_WIZARD const char *DeviceWizardResponse::dump_to(DumpBuffer &out) const { MessageDumpHelper helper(out, ESPHOME_PSTR("DeviceWizardResponse")); - dump_bytes_field(out, ESPHOME_PSTR("data"), this->data, this->data_len); + dump_progmem_bytes_field(out, ESPHOME_PSTR("data"), this->data, this->data_len); return out.c_str(); } #endif diff --git a/esphome/components/api/api_wizard.cpp b/esphome/components/api/api_wizard.cpp index ec270ab411..986017a138 100644 --- a/esphome/components/api/api_wizard.cpp +++ b/esphome/components/api/api_wizard.cpp @@ -13,17 +13,6 @@ namespace esphome::api { static const char *const TAG = "api.wizard"; -uint8_t *wizard_encode_response(const void *self, uint8_t *__restrict__ pos PROTO_ENCODE_DEBUG_PARAM) { - const auto &msg = *static_cast(self); - if (msg.data_len == 0) - return pos; - pos = ProtoEncode::encode_field_raw(pos PROTO_ENCODE_DEBUG_ARG, 1, 2); // type 2: Length-delimited - pos = ProtoEncode::encode_varint_raw(pos PROTO_ENCODE_DEBUG_ARG, msg.data_len); - PROTO_ENCODE_CHECK_BOUNDS(pos, msg.data_len); - progmem_memcpy(pos, msg.data, msg.data_len); - return pos + msg.data_len; -} - #ifdef USE_API_WIZARD_INPUTS static bool wizard_entity_id_valid(const char *entity_id, size_t length) { return length > 0 && length < WIZARD_ENTITY_ID_BUFFER_SIZE && memchr(entity_id, '.', length) != nullptr; @@ -49,17 +38,11 @@ const char *wizard_set_input(const WizardInputSetRequest &msg) { } #endif // USE_API_WIZARD_INPUTS -bool APIConnection::send_device_wizard_response_() { +void APIConnection::on_device_wizard_request() { DeviceWizardResponse resp; resp.data = API_WIZARD_DATA; resp.data_len = API_WIZARD_DATA_SIZE; - // Not send_message: the data is in flash, so wizard_encode_response copies it out - return this->send_message_(DeviceWizardResponse::calc_size_msg(&resp), DeviceWizardResponse::MESSAGE_TYPE, - &wizard_encode_response, &resp); -} - -void APIConnection::on_device_wizard_request() { - if (!this->send_device_wizard_response_()) { + if (!this->send_message(resp)) { this->on_fatal_error(); } } diff --git a/esphome/components/api/api_wizard.h b/esphome/components/api/api_wizard.h index 948b9445f2..7d5a566b55 100644 --- a/esphome/components/api/api_wizard.h +++ b/esphome/components/api/api_wizard.h @@ -22,10 +22,6 @@ static constexpr size_t WIZARD_ENTITY_ID_BUFFER_SIZE = 256; /// (components/api/wizard.py) and kept in flash. API_WIZARD_DATA_SIZE bytes long. extern const uint8_t API_WIZARD_DATA[] PROGMEM; -/// Encodes a DeviceWizardResponse like the generated encoder would. The data is in flash, which ESP8266 can only read -/// with progmem_memcpy, so the generated encoder (a plain memcpy) cannot be used. Plain memcpy elsewhere. -uint8_t *wizard_encode_response(const void *self, uint8_t *pos PROTO_ENCODE_DEBUG_PARAM); - #ifdef USE_API_WIZARD_INPUTS /// Where the entity id of an input is kept, found by the key the client uses for it. struct WizardInputEntry { diff --git a/esphome/components/api/proto.h b/esphome/components/api/proto.h index 5f2a90d547..0fdda5067e 100644 --- a/esphome/components/api/proto.h +++ b/esphome/components/api/proto.h @@ -3,6 +3,7 @@ #include "api_pb2_defines.h" #include "api_buffer.h" #include "esphome/core/component.h" +#include "esphome/core/hal.h" #include "esphome/core/helpers.h" #include "esphome/core/log.h" #include "esphome/core/progmem.h" @@ -465,6 +466,21 @@ class ProtoEncode { uint32_t field_id, const uint8_t *data, size_t len) { return encode_string_force(pos PROTO_ENCODE_DEBUG_ARG, field_id, reinterpret_cast(data), len); } + /// encode_bytes for data in flash (PROGMEM), which ESP8266 can only read with progmem_memcpy. + [[nodiscard]] static inline uint8_t *encode_progmem_bytes(uint8_t *__restrict__ pos PROTO_ENCODE_DEBUG_PARAM, + uint32_t field_id, const uint8_t *data, size_t len) { + if (len == 0) + return pos; + return encode_progmem_bytes_force(pos PROTO_ENCODE_DEBUG_ARG, field_id, data, len); + } + [[nodiscard]] static inline uint8_t *encode_progmem_bytes_force(uint8_t *__restrict__ pos PROTO_ENCODE_DEBUG_PARAM, + uint32_t field_id, const uint8_t *data, size_t len) { + pos = encode_field_raw(pos PROTO_ENCODE_DEBUG_ARG, field_id, 2); // type 2: Length-delimited + pos = encode_varint_raw(pos PROTO_ENCODE_DEBUG_ARG, len); + PROTO_ENCODE_CHECK_BOUNDS(pos, len); + progmem_memcpy(pos, data, len); + return pos + len; + } [[nodiscard]] static inline uint8_t *encode_uint32_force(uint8_t *__restrict__ pos PROTO_ENCODE_DEBUG_PARAM, uint32_t field_id, uint32_t value) { pos = encode_field_raw(pos PROTO_ENCODE_DEBUG_ARG, field_id, 0); diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index f644859d01..701765bb41 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -536,6 +536,16 @@ def create_field_type_info( f"track_presence on field '{field.name}' has no effect; it requires " "a non-repeated message field in a message that is decoded" ) + if get_field_opt(field, pb.progmem, False) and ( + field.label == FieldDescriptorProto.LABEL_REPEATED + or field.type != 12 + or not get_field_opt(field, pb.pointer_to_buffer, False) + or get_field_opt(field, pb.fixed_array_size) is not None + ): + raise ValueError( + f"progmem on field '{field.name}' requires a non-repeated bytes field " + "with pointer_to_buffer" + ) 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): @@ -1158,6 +1168,11 @@ 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 @@ -1168,6 +1183,14 @@ class PointerToBytesBufferType(PointerToBufferTypeBase): @property def encode_content(self) -> str: + if self.progmem: + return _encode_call( + "encode_progmem_bytes", + str(self.number), + f"this->{self.field_name}", + f"this->{self.field_name}_len", + force=self.force, + ) if result := self._encode_bytes_with_precomputed_tag( f"this->{self.field_name}", f"this->{self.field_name}_len" ): @@ -1194,8 +1217,9 @@ class PointerToBytesBufferType(PointerToBufferTypeBase): @property def dump_content(self) -> str: + dump_fn = "dump_progmem_bytes_field" if self.progmem else "dump_bytes_field" return ( - f'dump_bytes_field(out, ESPHOME_PSTR("{self.name}"), ' + f'{dump_fn}(out, ESPHOME_PSTR("{self.name}"), ' f"this->{self.field_name}, this->{self.field_name}_len);" ) @@ -2609,6 +2633,11 @@ def build_message_type( ): fixed_vector_fields.append((field.name, field.number)) + if needs_decode and get_field_opt(field, pb.progmem, False): + raise ValueError( + f"progmem on field '{field.name}' of {desc.name} requires a message " + "that is only encoded; received data is never in flash" + ) ti = create_field_type_info(field, needs_decode, needs_encode) # Skip field declarations for fields that are in the base class @@ -3319,15 +3348,26 @@ template static void dump_field(DumpBuffer &out, const char *field_n out.append("\\n"); } +// Bytes shown by a bytes field dump: 160 bytes is 480 chars with separators, to fit a typical log buffer +static constexpr size_t DUMP_BYTES_MAX = 160; + // Helper for bytes fields - uses stack buffer to avoid heap allocation -// Buffer sized for 160 bytes of data (480 chars with separators) to fit typical log buffer // field_name is a PROGMEM pointer (flash on ESP8266, regular pointer on other platforms) static void dump_bytes_field(DumpBuffer &out, const char *field_name, const uint8_t *data, size_t len, int indent = 2) { - char hex_buf[format_hex_pretty_size(160)]; + char hex_buf[format_hex_pretty_size(DUMP_BYTES_MAX)]; append_field_prefix(out, field_name, indent); format_hex_pretty_to(hex_buf, data, len); out.append(hex_buf).append("\\n"); } + +// Helper for bytes fields in flash: copies the shown bytes out with progmem_memcpy first +static void dump_progmem_bytes_field(DumpBuffer &out, const char *field_name, const uint8_t *data, size_t len, + int indent = 2) { + uint8_t data_buf[DUMP_BYTES_MAX]; + len = std::min(len, sizeof(data_buf)); + progmem_memcpy(data_buf, data, len); + dump_bytes_field(out, field_name, data_buf, len, indent); +} #pragma GCC diagnostic pop """ diff --git a/tests/components/api/test_wizard.cpp b/tests/components/api/test_wizard.cpp index 2e6b38edb9..033c8b627e 100644 --- a/tests/components/api/test_wizard.cpp +++ b/tests/components/api/test_wizard.cpp @@ -68,7 +68,7 @@ TEST(DeviceWizard, ResponseSendsTheDataUnchanged) { Bytes expected{0x0a, 0xc8, 0x01}; expected.insert(expected.end(), API_WIZARD_DATA, API_WIZARD_DATA + API_WIZARD_DATA_SIZE); - EXPECT_EQ(encode(resp, &DeviceWizardResponse::calc_size_msg, &wizard_encode_response), expected); + EXPECT_EQ(encode(resp, &DeviceWizardResponse::calc_size_msg, &DeviceWizardResponse::encode_msg), expected); } TEST(DeviceWizard, ShortDataHasAOneByteLength) { @@ -77,13 +77,13 @@ TEST(DeviceWizard, ShortDataHasAOneByteLength) { resp.data_len = 3; Bytes expected{0x0a, 0x03, API_WIZARD_DATA[0], API_WIZARD_DATA[1], API_WIZARD_DATA[2]}; - EXPECT_EQ(encode(resp, &DeviceWizardResponse::calc_size_msg, &wizard_encode_response), expected); + EXPECT_EQ(encode(resp, &DeviceWizardResponse::calc_size_msg, &DeviceWizardResponse::encode_msg), expected); } TEST(DeviceWizard, NoDataEncodesNothing) { DeviceWizardResponse resp; EXPECT_EQ(resp.calculate_size(), 0u); - EXPECT_TRUE(encode(resp, &DeviceWizardResponse::calc_size_msg, &wizard_encode_response).empty()); + EXPECT_TRUE(encode(resp, &DeviceWizardResponse::calc_size_msg, &DeviceWizardResponse::encode_msg).empty()); } TEST(DeviceWizard, CapabilitiesAnnounceTheWizard) { 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 2e4eb67a24..5e29b06ef0 100644 --- a/tests/unit_tests/components/api/test_api_protobuf_generator.py +++ b/tests/unit_tests/components/api/test_api_protobuf_generator.py @@ -360,3 +360,56 @@ def test_repeated_strings_generate_in_every_direction(source: int) -> None: assert "nullptr" not in header if source != SOURCE_SERVER: assert "case proto_tag(1, WIRE_TYPE_LENGTH_DELIMITED):" in cpp + + +def _progmem_field( + *, pointer_to_buffer: bool = True, force: bool = False, repeated: bool = False +) -> descriptor_pb2.FieldDescriptorProto: + field = _field(12, force=force, repeated=repeated) + field.options.Extensions[pb.progmem] = True + if pointer_to_buffer: + field.options.Extensions[pb.pointer_to_buffer] = True + return field + + +@pytest.mark.parametrize( + ("force", "encode_fn"), + [(False, "encode_progmem_bytes("), (True, "encode_progmem_bytes_force(")], +) +def test_progmem_bytes_field_copies_from_flash(force: bool, encode_fn: str) -> None: + """A (progmem) field encodes and dumps through the progmem_memcpy helpers.""" + ti = create_field_type_info(_progmem_field(force=force), needs_decode=False) + assert encode_fn in ti.encode_content + assert "encode_bytes" not in ti.encode_content + assert "dump_progmem_bytes_field(" in ti.dump_content + + +@pytest.mark.parametrize( + "field", + [ + pytest.param(_progmem_field(pointer_to_buffer=False), id="no_pointer"), + pytest.param(_progmem_field(repeated=True), id="repeated"), + ], +) +def test_progmem_rejected_where_it_cannot_apply( + field: descriptor_pb2.FieldDescriptorProto, +) -> None: + """(progmem) on a field that would not copy from flash fails instead of silently using memcpy.""" + with pytest.raises(ValueError, match="progmem on field 'value'"): + create_field_type_info(field, needs_decode=False) + + +def test_progmem_rejected_in_a_decoded_message() -> None: + """Received data is never in flash, so a decoded message cannot use (progmem).""" + desc = descriptor_pb2.DescriptorProto(name="Received") + desc.field.add().CopyFrom(_progmem_field()) + with pytest.raises(ValueError, match="progmem on field 'value' of Received"): + build_message_type(desc, {}, {"Received": SOURCE_CLIENT}) + + +def test_progmem_rejected_on_non_bytes_field() -> None: + """(progmem) only applies to bytes fields.""" + field = _field(9) + field.options.Extensions[pb.progmem] = True + with pytest.raises(ValueError, match="progmem on field 'value'"): + create_field_type_info(field, needs_decode=False)