diff --git a/esphome/components/api/__init__.py b/esphome/components/api/__init__.py index 9611228ba1d..44d8b443676 100644 --- a/esphome/components/api/__init__.py +++ b/esphome/components/api/__init__.py @@ -251,9 +251,6 @@ def _validate_supports_response(value: Any) -> str: return cv.enum(SUPPORTS_RESPONSE_OPTIONS, lower=True)(value) -# ESP8266 copies every string of an action into a stack buffer sized by codegen; keep it small -ESP8266_ACTION_STRINGS_MAX_TOTAL = 384 - VARIABLE_SCHEMA = cv.Schema( { cv.Required(CONF_TYPE): cv.one_of(*SERVICE_ARG_NATIVE_TYPES, lower=True), @@ -484,29 +481,6 @@ def _action_strings(conf: ConfigType, has_metadata: bool) -> list[str | None]: return strings -def _action_strings_size(strings: list[str | None]) -> int: - """Bytes needed to copy every string out of flash, each with its terminator.""" - return sum( - len(string.encode("utf-8")) + 1 for string in strings if string is not None - ) - - -def _validate_esp8266_action_strings(config: ConfigType) -> ConfigType: - if not CORE.is_esp8266: - return config - actions = config.get(CONF_ACTIONS, []) - has_metadata = _has_action_metadata(actions) - for conf in actions: - size = _action_strings_size(_action_strings(conf, has_metadata)) - if size > ESP8266_ACTION_STRINGS_MAX_TOTAL: - raise cv.Invalid( - f"Action '{conf[CONF_ACTION]}' has {size} bytes of name, variable name, " - f"description and example text; ESP8266 allows at most " - f"{ESP8266_ACTION_STRINGS_MAX_TOTAL} bytes per action" - ) - return config - - def _validate_outgoing_host_ipv6(config: ConfigType) -> ConfigType: """An IPv6 host can never be parsed, so never dialed, without IPv6.""" if ( @@ -531,7 +505,6 @@ def _validate_wizard(config: ConfigType) -> ConfigType: FINAL_VALIDATE_SCHEMA = cv.All( - _validate_esp8266_action_strings, _validate_outgoing_host_ipv6, _validate_wizard, ) @@ -587,7 +560,6 @@ async def to_code(config: ConfigType) -> None: if config[CONF_HOMEASSISTANT_STATES]: cg.add_define("USE_API_HOMEASSISTANT_STATES") - scratch_size = 0 if actions: # Metadata is compiled in for every action once any action declares it, because the # string table layout is fixed by the define rather than per action @@ -639,8 +611,6 @@ async def to_code(config: ConfigType) -> None: func_args.append((native, name)) strings = _action_strings(conf, has_metadata) table = _add_action_strings(index, strings) - if CORE.is_esp8266: - scratch_size = max(scratch_size, _action_strings_size(strings)) # Template args: supports_response mode, then user service arg types templ = cg.TemplateArguments(supports_response, *service_template_args) # Key is hashed here because the name is not readable at runtime on ESP8266 @@ -667,9 +637,6 @@ async def to_code(config: ConfigType) -> None: cg.add(auto.add_action(unregister_action)) # Register all services at once - single allocation, no reallocations cg.add(var.initialize_user_services(triggers)) - if CORE.is_esp8266 and has_user_actions: - # Stack buffer that list-entities copies PROGMEM strings into, sized for the largest action - cg.add_define("API_USER_ACTION_STRINGS_SCRATCH_SIZE", max(scratch_size, 1)) if (wizard_config := config.get(wizard.CONF_WIZARD)) is not None: await wizard.to_code(wizard_config) diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index 2a64948e155..b1b38f08cb7 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -1026,8 +1026,8 @@ message SubscribeHomeassistantServicesRequest { } message HomeassistantServiceMap { - string key = 1; - string value = 2; + string key = 1 [(progmem) = true]; + string value = 2 [(progmem) = true]; } message HomeassistantActionRequest { @@ -1036,7 +1036,7 @@ message HomeassistantActionRequest { option (no_delay) = true; option (ifdef) = "USE_API_HOMEASSISTANT_SERVICES"; - string service = 1; + string service = 1 [(progmem) = true]; repeated HomeassistantServiceMap data = 2 [(fixed_vector) = true]; repeated HomeassistantServiceMap data_template = 3 [(fixed_vector) = true]; repeated HomeassistantServiceMap variables = 4 [(fixed_vector) = true]; @@ -1157,21 +1157,21 @@ enum SupportsResponseType { } message ListEntitiesServicesArgument { option (ifdef) = "USE_API_USER_DEFINED_ACTIONS"; - string name = 1; + string name = 1 [(progmem) = true]; ServiceArgType type = 2; - string description = 3 [(field_ifdef) = "USE_API_USER_DEFINED_ACTION_METADATA"]; - string example = 4 [(field_ifdef) = "USE_API_USER_DEFINED_ACTION_METADATA"]; + string description = 3 [(field_ifdef) = "USE_API_USER_DEFINED_ACTION_METADATA", (progmem) = true]; + string example = 4 [(field_ifdef) = "USE_API_USER_DEFINED_ACTION_METADATA", (progmem) = true]; } message ListEntitiesServicesResponse { option (id) = 41; option (source) = SOURCE_SERVER; option (ifdef) = "USE_API_USER_DEFINED_ACTIONS"; - string name = 1; + string name = 1 [(progmem) = true]; fixed32 key = 2 [(force) = true]; repeated ListEntitiesServicesArgument args = 3 [(fixed_vector) = true]; SupportsResponseType supports_response = 4; - string description = 5 [(field_ifdef) = "USE_API_USER_DEFINED_ACTION_METADATA"]; + string description = 5 [(field_ifdef) = "USE_API_USER_DEFINED_ACTION_METADATA", (progmem) = true]; } message ExecuteServiceArgument { option (ifdef) = "USE_API_USER_DEFINED_ACTIONS"; diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index 519ce36f595..7c5d3aadc2f 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -1140,8 +1140,8 @@ uint32_t NoiseEncryptionSetKeyResponse::calc_size_msg(const void *self) { #ifdef USE_API_HOMEASSISTANT_SERVICES uint8_t *HomeassistantServiceMap::encode_msg(const void *self, uint8_t *__restrict__ pos PROTO_ENCODE_DEBUG_PARAM) { const auto &msg = *static_cast(self); - pos = ProtoEncode::encode_string(pos PROTO_ENCODE_DEBUG_ARG, 1, msg.key); - pos = ProtoEncode::encode_string(pos PROTO_ENCODE_DEBUG_ARG, 2, msg.value); + pos = ProtoEncode::encode_progmem_string(pos PROTO_ENCODE_DEBUG_ARG, 1, msg.key); + pos = ProtoEncode::encode_progmem_string(pos PROTO_ENCODE_DEBUG_ARG, 2, msg.value); return pos; } uint32_t HomeassistantServiceMap::calc_size_msg(const void *self) { @@ -1153,7 +1153,7 @@ uint32_t HomeassistantServiceMap::calc_size_msg(const void *self) { } uint8_t *HomeassistantActionRequest::encode_msg(const void *self, uint8_t *__restrict__ pos PROTO_ENCODE_DEBUG_PARAM) { const auto &msg = *static_cast(self); - pos = ProtoEncode::encode_string(pos PROTO_ENCODE_DEBUG_ARG, 1, msg.service); + pos = ProtoEncode::encode_progmem_string(pos PROTO_ENCODE_DEBUG_ARG, 1, msg.service); for (auto &it : msg.data) { pos = ProtoEncode::encode_sub_message(pos PROTO_ENCODE_DEBUG_ARG, 2, it); } @@ -1324,13 +1324,13 @@ void GetTimeResponse::decode_field(void *self, uint32_t tag, const uint8_t *data uint8_t *ListEntitiesServicesArgument::encode_msg(const void *self, uint8_t *__restrict__ pos PROTO_ENCODE_DEBUG_PARAM) { const auto &msg = *static_cast(self); - pos = ProtoEncode::encode_string(pos PROTO_ENCODE_DEBUG_ARG, 1, msg.name); + pos = ProtoEncode::encode_progmem_string(pos PROTO_ENCODE_DEBUG_ARG, 1, msg.name); pos = ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 2, static_cast(msg.type)); #ifdef USE_API_USER_DEFINED_ACTION_METADATA - pos = ProtoEncode::encode_string(pos PROTO_ENCODE_DEBUG_ARG, 3, msg.description); + pos = ProtoEncode::encode_progmem_string(pos PROTO_ENCODE_DEBUG_ARG, 3, msg.description); #endif #ifdef USE_API_USER_DEFINED_ACTION_METADATA - pos = ProtoEncode::encode_string(pos PROTO_ENCODE_DEBUG_ARG, 4, msg.example); + pos = ProtoEncode::encode_progmem_string(pos PROTO_ENCODE_DEBUG_ARG, 4, msg.example); #endif return pos; } @@ -1350,14 +1350,14 @@ uint32_t ListEntitiesServicesArgument::calc_size_msg(const void *self) { uint8_t *ListEntitiesServicesResponse::encode_msg(const void *self, uint8_t *__restrict__ pos PROTO_ENCODE_DEBUG_PARAM) { const auto &msg = *static_cast(self); - pos = ProtoEncode::encode_string(pos PROTO_ENCODE_DEBUG_ARG, 1, msg.name); + pos = ProtoEncode::encode_progmem_string(pos PROTO_ENCODE_DEBUG_ARG, 1, msg.name); pos = ProtoEncode::write_tag_and_fixed32(pos PROTO_ENCODE_DEBUG_ARG, 21, msg.key); for (auto &it : msg.args) { pos = ProtoEncode::encode_sub_message(pos PROTO_ENCODE_DEBUG_ARG, 3, it); } pos = ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 4, static_cast(msg.supports_response)); #ifdef USE_API_USER_DEFINED_ACTION_METADATA - pos = ProtoEncode::encode_string(pos PROTO_ENCODE_DEBUG_ARG, 5, msg.description); + pos = ProtoEncode::encode_progmem_string(pos PROTO_ENCODE_DEBUG_ARG, 5, msg.description); #endif return pos; } diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index dcb7c01f110..bc21c46d5c3 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -1388,8 +1388,8 @@ class NoiseEncryptionSetKeyResponse final : public ProtoMessage { #ifdef USE_API_HOMEASSISTANT_SERVICES class HomeassistantServiceMap final : public ProtoMessage { public: - StringRef key{nullptr, 0}; // null until set, encode only - StringRef value{nullptr, 0}; // null until set, encode only + ProgmemStringRef key{}; + ProgmemStringRef value{}; static uint8_t *encode_msg(const void *self, uint8_t *pos PROTO_ENCODE_DEBUG_PARAM); uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const { return encode_msg(this, buffer.get_pos() PROTO_ENCODE_DEBUG_ARG); @@ -1409,7 +1409,7 @@ class HomeassistantActionRequest final : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP const LogString *message_name() const override { return LOG_STR("homeassistant_action_request"); } #endif - StringRef service{nullptr, 0}; // null until set, encode only + ProgmemStringRef service{}; FixedVector data{}; FixedVector data_template{}; FixedVector variables{}; @@ -1576,13 +1576,13 @@ class GetTimeResponse final : public ProtoDecodableMessage { #ifdef USE_API_USER_DEFINED_ACTIONS class ListEntitiesServicesArgument final : public ProtoMessage { public: - StringRef name{nullptr, 0}; // null until set, encode only + ProgmemStringRef name{}; enums::ServiceArgType type{}; #ifdef USE_API_USER_DEFINED_ACTION_METADATA - StringRef description{nullptr, 0}; // null until set, encode only + ProgmemStringRef description{}; #endif #ifdef USE_API_USER_DEFINED_ACTION_METADATA - StringRef example{nullptr, 0}; // null until set, encode only + ProgmemStringRef example{}; #endif static uint8_t *encode_msg(const void *self, uint8_t *pos PROTO_ENCODE_DEBUG_PARAM); uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const { @@ -1603,12 +1603,12 @@ class ListEntitiesServicesResponse final : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP const LogString *message_name() const override { return LOG_STR("list_entities_services_response"); } #endif - StringRef name{nullptr, 0}; // null until set, encode only + ProgmemStringRef name{}; uint32_t key{0}; FixedVector args{}; enums::SupportsResponseType supports_response{}; #ifdef USE_API_USER_DEFINED_ACTION_METADATA - StringRef description{nullptr, 0}; // null until set, encode only + ProgmemStringRef description{}; #endif static uint8_t *encode_msg(const void *self, uint8_t *pos PROTO_ENCODE_DEBUG_PARAM); uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const { diff --git a/esphome/components/api/api_pb2_dump.cpp b/esphome/components/api/api_pb2_dump.cpp index e361c9ca2ef..b0c6d9dd016 100644 --- a/esphome/components/api/api_pb2_dump.cpp +++ b/esphome/components/api/api_pb2_dump.cpp @@ -1519,13 +1519,13 @@ const char *NoiseEncryptionSetKeyResponse::dump_to(DumpBuffer &out) const { #ifdef USE_API_HOMEASSISTANT_SERVICES const char *HomeassistantServiceMap::dump_to(DumpBuffer &out) const { MessageDumpHelper helper(out, ESPHOME_PSTR("HomeassistantServiceMap")); - dump_field(out, ESPHOME_PSTR("key"), this->key); - dump_field(out, ESPHOME_PSTR("value"), this->value); + dump_progmem_string_field(out, ESPHOME_PSTR("key"), this->key); + dump_progmem_string_field(out, ESPHOME_PSTR("value"), this->value); return out.c_str(); } const char *HomeassistantActionRequest::dump_to(DumpBuffer &out) const { MessageDumpHelper helper(out, ESPHOME_PSTR("HomeassistantActionRequest")); - dump_field(out, ESPHOME_PSTR("service"), this->service); + dump_progmem_string_field(out, ESPHOME_PSTR("service"), this->service); for (const auto &it : this->data) { out.append(4, ' ').append_p(ESPHOME_PSTR("data")).append(": "); it.dump_to(out); @@ -1620,19 +1620,19 @@ const char *GetTimeResponse::dump_to(DumpBuffer &out) const { #ifdef USE_API_USER_DEFINED_ACTIONS const char *ListEntitiesServicesArgument::dump_to(DumpBuffer &out) const { MessageDumpHelper helper(out, ESPHOME_PSTR("ListEntitiesServicesArgument")); - dump_field(out, ESPHOME_PSTR("name"), this->name); + dump_progmem_string_field(out, ESPHOME_PSTR("name"), this->name); dump_field(out, ESPHOME_PSTR("type"), static_cast(this->type)); #ifdef USE_API_USER_DEFINED_ACTION_METADATA - dump_field(out, ESPHOME_PSTR("description"), this->description); + dump_progmem_string_field(out, ESPHOME_PSTR("description"), this->description); #endif #ifdef USE_API_USER_DEFINED_ACTION_METADATA - dump_field(out, ESPHOME_PSTR("example"), this->example); + dump_progmem_string_field(out, ESPHOME_PSTR("example"), this->example); #endif return out.c_str(); } const char *ListEntitiesServicesResponse::dump_to(DumpBuffer &out) const { MessageDumpHelper helper(out, ESPHOME_PSTR("ListEntitiesServicesResponse")); - dump_field(out, ESPHOME_PSTR("name"), this->name); + dump_progmem_string_field(out, ESPHOME_PSTR("name"), this->name); dump_field(out, ESPHOME_PSTR("key"), this->key); for (const auto &it : this->args) { out.append(4, ' ').append_p(ESPHOME_PSTR("args")).append(": "); @@ -1641,7 +1641,7 @@ const char *ListEntitiesServicesResponse::dump_to(DumpBuffer &out) const { } dump_field(out, ESPHOME_PSTR("supports_response"), static_cast(this->supports_response)); #ifdef USE_API_USER_DEFINED_ACTION_METADATA - dump_field(out, ESPHOME_PSTR("description"), this->description); + dump_progmem_string_field(out, ESPHOME_PSTR("description"), this->description); #endif return out.c_str(); } diff --git a/esphome/components/api/api_server.cpp b/esphome/components/api/api_server.cpp index f63039a9714..a8d14256dcb 100644 --- a/esphome/components/api/api_server.cpp +++ b/esphome/components/api/api_server.cpp @@ -547,7 +547,7 @@ void APIServer::send_homeassistant_action(const HomeassistantActionRequest &call // arrive before the subscription and are lost - warn instead of failing silently. ESP_LOGW(TAG, "Home Assistant %s '%.*s' dropped; %s", call.is_event ? LOG_STR_LITERAL("event") : LOG_STR_LITERAL("action"), - static_cast(call.service.size()), call.service.empty() ? "" : call.service.c_str(), + static_cast(call.service.size()), call.service.empty() ? "" : call.service.progmem_ptr(), this->is_connected() ? LOG_STR_LITERAL("client has not subscribed to actions (yet)") : LOG_STR_LITERAL("no client connected")); } diff --git a/esphome/components/api/custom_api_device.h b/esphome/components/api/custom_api_device.h index c2b0f14bcc5..208c956eb17 100644 --- a/esphome/components/api/custom_api_device.h +++ b/esphome/components/api/custom_api_device.h @@ -247,7 +247,7 @@ class CustomAPIDevice { */ void call_homeassistant_service(const std::string &service_name) { HomeassistantActionRequest resp; - resp.service = StringRef(service_name); + resp.service = ProgmemStringRef(StringRef(service_name)); global_api_server->send_homeassistant_action(resp); } @@ -267,12 +267,12 @@ class CustomAPIDevice { */ void call_homeassistant_service(const std::string &service_name, const std::map &data) { HomeassistantActionRequest resp; - resp.service = StringRef(service_name); + resp.service = ProgmemStringRef(StringRef(service_name)); resp.data.init(data.size()); for (auto &it : data) { auto &kv = resp.data.emplace_back(); - kv.key = StringRef(it.first); - kv.value = StringRef(it.second); // data map lives until send completes + kv.key = ProgmemStringRef(StringRef(it.first)); + kv.value = ProgmemStringRef(StringRef(it.second)); // data map lives until send completes } global_api_server->send_homeassistant_action(resp); } @@ -289,7 +289,7 @@ class CustomAPIDevice { */ void fire_homeassistant_event(const std::string &event_name) { HomeassistantActionRequest resp; - resp.service = StringRef(event_name); + resp.service = ProgmemStringRef(StringRef(event_name)); resp.is_event = true; global_api_server->send_homeassistant_action(resp); } @@ -309,13 +309,13 @@ class CustomAPIDevice { */ void fire_homeassistant_event(const std::string &service_name, const std::map &data) { HomeassistantActionRequest resp; - resp.service = StringRef(service_name); + resp.service = ProgmemStringRef(StringRef(service_name)); resp.is_event = true; resp.data.init(data.size()); for (auto &it : data) { auto &kv = resp.data.emplace_back(); - kv.key = StringRef(it.first); - kv.value = StringRef(it.second); // data map lives until send completes + kv.key = ProgmemStringRef(StringRef(it.first)); + kv.value = ProgmemStringRef(StringRef(it.second)); // data map lives until send completes } global_api_server->send_homeassistant_action(resp); } diff --git a/esphome/components/api/homeassistant_service.h b/esphome/components/api/homeassistant_service.h index e7798a5d267..8964e8ca352 100644 --- a/esphome/components/api/homeassistant_service.h +++ b/esphome/components/api/homeassistant_service.h @@ -116,42 +116,18 @@ template class HomeAssistantServiceCallAction final : public Act const Field *fields = this->fields_; const size_t total = 1 + this->data_count_ + this->data_template_count_ + this->variables_count_; - // Lambda results, and on ESP8266 the RAM copies of the flash strings, must live until the send + // Lambda results must live until the send; the constant strings are encoded from flash where they are size_t lambda_count = 0; -#ifdef USE_ESP8266 - size_t flash_len = 0; -#endif - for (size_t i = 0; i < total; i++) { + for (size_t i = 0; i < total; i++) lambda_count += fields[i].fn != nullptr; -#ifdef USE_ESP8266 - if (fields[i].fn == nullptr) - flash_len += ESPHOME_strlen_P(fields[i].value); - if (fields[i].key != nullptr) - flash_len += ESPHOME_strlen_P(fields[i].key); -#endif - } FixedVector results; results.init(lambda_count); -#ifdef USE_ESP8266 - SmallBufferWithHeapFallback<128, char> flash_copy(flash_len); - char *cursor = flash_copy.get(); -#endif - auto string_ref = [&](const char *str) { -#ifdef USE_ESP8266 - size_t len = ESPHOME_strlen_P(str); - memcpy_P(cursor, str, len); - StringRef ref(cursor, len); - cursor += len; - return ref; -#else - return StringRef(str); -#endif - }; + auto string_ref = [](const char *str) { return ProgmemStringRef(str, ESPHOME_strlen_P(str)); }; auto value_ref = [&](const Field &field) { if (field.fn == nullptr) return string_ref(field.value); results.push_back(field.fn(x...)); - return StringRef(results.back()); + return ProgmemStringRef(StringRef(results.back())); }; auto fill = [&](FixedVector &dest, uint8_t count) { dest.init(count); diff --git a/esphome/components/api/list_entities.cpp b/esphome/components/api/list_entities.cpp index 507b098fb48..57ff616ca7c 100644 --- a/esphome/components/api/list_entities.cpp +++ b/esphome/components/api/list_entities.cpp @@ -99,8 +99,7 @@ ListEntitiesIterator::ListEntitiesIterator(APIConnection *client) : client_(clie static constexpr uint8_t SERVICE_YIELD_INTERVAL = 3; bool ListEntitiesIterator::on_service(UserServiceDescriptor *service) { - UserActionScratch scratch; - auto resp = service->encode_list_service_response(scratch); + auto resp = service->encode_list_service_response(); if (!this->client_->send_message(resp)) return false; // at_ is this service's index diff --git a/esphome/components/api/user_services.cpp b/esphome/components/api/user_services.cpp index fad3cde29b9..a9836170640 100644 --- a/esphome/components/api/user_services.cpp +++ b/esphome/components/api/user_services.cpp @@ -1,47 +1,36 @@ #include "user_services.h" #include "esphome/core/hal.h" #include "esphome/core/log.h" +#include "esphome/core/progmem.h" #include "esphome/core/string_ref.h" namespace esphome::api { -StringRef UserServiceStatic::str_(size_t idx, std::span &scratch) const { +ProgmemStringRef UserServiceStatic::str_(size_t idx) const { const char *s = progmem_read_ptr(&this->strings_[idx]); if (s == nullptr) return {}; -#ifdef USE_ESP8266 - // Codegen sizes the scratch buffer for the largest service; the bound only guards other callers - if (scratch.empty()) - return {}; - size_t len = strnlen_P(s, scratch.size() - 1); - progmem_memcpy(scratch.data(), s, len); - scratch[len] = '\0'; - StringRef ref(scratch.data(), len); - scratch = scratch.subspan(len + 1); - return ref; -#else - return StringRef(s); -#endif + return ProgmemStringRef(s, ESPHOME_strlen_P(s)); } ListEntitiesServicesResponse UserServiceStatic::encode_list_service_response_( - std::span arg_types, std::span scratch) const { + std::span arg_types) const { ListEntitiesServicesResponse msg; - msg.name = this->str_(0, scratch); + msg.name = this->str_(0); msg.key = this->key_; msg.supports_response = this->supports_response_; #ifdef USE_API_USER_DEFINED_ACTION_METADATA - msg.description = this->str_(1, scratch); + msg.description = this->str_(1); #endif msg.args.init(arg_types.size()); for (size_t i = 0; i < arg_types.size(); i++) { size_t base = USER_ACTION_HEADER_STRINGS + i * USER_ACTION_ARG_STRINGS; auto &arg = msg.args.emplace_back(); arg.type = arg_types[i]; - arg.name = this->str_(base, scratch); + arg.name = this->str_(base); #ifdef USE_API_USER_DEFINED_ACTION_METADATA - arg.description = this->str_(base + 1, scratch); - arg.example = this->str_(base + 2, scratch); + arg.description = this->str_(base + 1); + arg.example = this->str_(base + 2); #endif } return msg; diff --git a/esphome/components/api/user_services.h b/esphome/components/api/user_services.h index 3b17bdb7bcd..057e04dd7a7 100644 --- a/esphome/components/api/user_services.h +++ b/esphome/components/api/user_services.h @@ -20,9 +20,8 @@ class APIServer; class UserServiceDescriptor { public: - /// Build the list-entities message. On ESP8266 the strings live in PROGMEM and are copied into - /// `scratch`, so the returned message is only valid while `scratch` is; other platforms ignore it. - virtual ListEntitiesServicesResponse encode_list_service_response(std::span scratch) = 0; + /// Build the list-entities message; its strings may point into flash (PROGMEM). + virtual ListEntitiesServicesResponse encode_list_service_response() = 0; virtual bool execute_service(const ExecuteServiceRequest &req) = 0; #ifdef USE_API_USER_DEFINED_ACTION_RESPONSES @@ -37,13 +36,6 @@ template T get_execute_arg_value(const ExecuteServiceArgument &arg); template enums::ServiceArgType to_service_arg_type(); -// Scratch buffer list-entities hands to encode_list_service_response(); only ESP8266 copies into it -#ifdef USE_ESP8266 -using UserActionScratch = std::array; -#else -using UserActionScratch = std::array; -#endif - // Non-template base for YAML-defined services so the list-entities encoder is compiled once. // All strings live in one PROGMEM pointer table emitted by codegen (see _action_strings in // __init__.py), so each service costs a single pointer of RAM. Layout: the action name, then @@ -63,12 +55,9 @@ class UserServiceStatic : public UserServiceDescriptor { : strings_(strings), key_(key), supports_response_(supports_response) {} protected: - ListEntitiesServicesResponse encode_list_service_response_(std::span arg_types, - std::span scratch) const; - /// Reference table entry `idx`; nullptr gives an empty StringRef. - /// On ESP8266 the bytes are copied out of PROGMEM into `scratch` with a terminator, and the span - /// is advanced past the copy. - StringRef str_(size_t idx, std::span &scratch) const; + ListEntitiesServicesResponse encode_list_service_response_(std::span arg_types) const; + /// Table entry `idx` as it sits in flash; nullptr gives an empty string. + ProgmemStringRef str_(size_t idx) const; const char *const *strings_; // PROGMEM pointer table, read with progmem_read_ptr() uint32_t key_; @@ -79,9 +68,9 @@ template class UserServiceBase : public UserServiceStatic { public: using UserServiceStatic::UserServiceStatic; - ListEntitiesServicesResponse encode_list_service_response(std::span scratch) override { + ListEntitiesServicesResponse encode_list_service_response() override { std::array arg_types = {to_service_arg_type()...}; - return this->encode_list_service_response_(arg_types, scratch); + return this->encode_list_service_response_(arg_types); } bool execute_service(const ExecuteServiceRequest &req) override { @@ -125,9 +114,9 @@ template class UserServiceDynamic : public UserServiceDescriptor this->key_ = fnv1_hash(this->name_.c_str()); } - ListEntitiesServicesResponse encode_list_service_response(std::span /*scratch*/) override { + ListEntitiesServicesResponse encode_list_service_response() override { ListEntitiesServicesResponse msg; - msg.name = StringRef(this->name_); + msg.name = ProgmemStringRef(StringRef(this->name_)); msg.key = this->key_; msg.supports_response = enums::SUPPORTS_RESPONSE_NONE; // Dynamic services don't support responses yet std::array arg_types = {to_service_arg_type()...}; @@ -135,7 +124,7 @@ template class UserServiceDynamic : public UserServiceDescriptor for (size_t i = 0; i < sizeof...(Ts); i++) { auto &arg = msg.args.emplace_back(); arg.type = arg_types[i]; - arg.name = StringRef(this->arg_names_[i]); + arg.name = ProgmemStringRef(StringRef(this->arg_names_[i])); } return msg; } diff --git a/esphome/components/homeassistant/button/homeassistant_button.cpp b/esphome/components/homeassistant/button/homeassistant_button.cpp index 85ed0c42e99..f49cef8ee30 100644 --- a/esphome/components/homeassistant/button/homeassistant_button.cpp +++ b/esphome/components/homeassistant/button/homeassistant_button.cpp @@ -5,6 +5,7 @@ #include "esphome/components/api/api_pb2.h" #include "esphome/components/api/api_server.h" #include "esphome/core/log.h" +#include "esphome/core/progmem.h" #include "esphome/core/string_ref.h" namespace esphome::homeassistant { @@ -31,22 +32,21 @@ void HomeassistantButton::press_action() { } #endif - static constexpr auto SERVICE_BUTTON = StringRef::from_lit("button.press"); - static constexpr auto SERVICE_INPUT_BUTTON = StringRef::from_lit("input_button.press"); - static constexpr auto ENTITY_ID_KEY = StringRef::from_lit("entity_id"); - static constexpr char INPUT_PREFIX[] = "input_"; + // In flash on the ESP8266 and encoded from there + static const char SERVICE_BUTTON[] PROGMEM = "button.press"; + static const char SERVICE_INPUT_BUTTON[] PROGMEM = "input_button.press"; + static const char ENTITY_ID_KEY[] PROGMEM = "entity_id"; api::HomeassistantActionRequest resp; - if (strncmp(this->entity_id_, INPUT_PREFIX, sizeof(INPUT_PREFIX) - 1) == 0) { - resp.service = SERVICE_INPUT_BUTTON; - } else { - resp.service = SERVICE_BUTTON; - } + // An input helper entity id starts with the "input_" its action name starts with + const bool input_helper = ESPHOME_strncmp_P(this->entity_id_, SERVICE_INPUT_BUTTON, sizeof("input_") - 1) == 0; + resp.service = + input_helper ? ProgmemStringRef::from_lit(SERVICE_INPUT_BUTTON) : ProgmemStringRef::from_lit(SERVICE_BUTTON); resp.data.init(1); auto &entity_id_kv = resp.data.emplace_back(); - entity_id_kv.key = ENTITY_ID_KEY; - entity_id_kv.value = StringRef(this->entity_id_); + entity_id_kv.key = ProgmemStringRef::from_lit(ENTITY_ID_KEY); + entity_id_kv.value = ProgmemStringRef(StringRef(this->entity_id_)); api::global_api_server->send_homeassistant_action(resp); } diff --git a/esphome/components/homeassistant/number/homeassistant_number.cpp b/esphome/components/homeassistant/number/homeassistant_number.cpp index fabe2d87364..e1e10896d1e 100644 --- a/esphome/components/homeassistant/number/homeassistant_number.cpp +++ b/esphome/components/homeassistant/number/homeassistant_number.cpp @@ -5,6 +5,7 @@ #include "esphome/components/api/api_pb2.h" #include "esphome/components/api/api_server.h" #include "esphome/core/log.h" +#include "esphome/core/progmem.h" #include "esphome/core/string_ref.h" namespace esphome::homeassistant { @@ -89,31 +90,29 @@ void HomeassistantNumber::control(float value) { this->publish_state(value); - static constexpr auto NUMBER_SERVICE_NAME = StringRef::from_lit("number.set_value"); - static constexpr auto INPUT_NUMBER_SERVICE_NAME = StringRef::from_lit("input_number.set_value"); - static constexpr char INPUT_PREFIX[] = "input_"; - static constexpr auto ENTITY_ID_KEY = StringRef::from_lit("entity_id"); - static constexpr auto VALUE_KEY = StringRef::from_lit("value"); + // In flash on the ESP8266 and encoded from there + static const char NUMBER_SERVICE_NAME[] PROGMEM = "number.set_value"; + static const char INPUT_NUMBER_SERVICE_NAME[] PROGMEM = "input_number.set_value"; + static const char ENTITY_ID_KEY[] PROGMEM = "entity_id"; + static const char VALUE_KEY[] PROGMEM = "value"; api::HomeassistantActionRequest resp; - // input_number entities have their own set_value action - if (strncmp(this->entity_id_, INPUT_PREFIX, sizeof(INPUT_PREFIX) - 1) == 0) { - resp.service = INPUT_NUMBER_SERVICE_NAME; - } else { - resp.service = NUMBER_SERVICE_NAME; - } + // An input helper entity id starts with the "input_" its action name starts with + const bool input_helper = ESPHOME_strncmp_P(this->entity_id_, INPUT_NUMBER_SERVICE_NAME, sizeof("input_") - 1) == 0; + resp.service = input_helper ? ProgmemStringRef::from_lit(INPUT_NUMBER_SERVICE_NAME) + : ProgmemStringRef::from_lit(NUMBER_SERVICE_NAME); resp.data.init(2); auto &entity_id = resp.data.emplace_back(); - entity_id.key = ENTITY_ID_KEY; - entity_id.value = StringRef(this->entity_id_); + entity_id.key = ProgmemStringRef::from_lit(ENTITY_ID_KEY); + entity_id.value = ProgmemStringRef(StringRef(this->entity_id_)); auto &entity_value = resp.data.emplace_back(); - entity_value.key = VALUE_KEY; + entity_value.key = ProgmemStringRef::from_lit(VALUE_KEY); // Stack buffer - no heap allocation; %g produces shortest representation char value_buf[16]; buf_append_printf(value_buf, sizeof(value_buf), 0, "%g", value); - entity_value.value = StringRef(value_buf); + entity_value.value = ProgmemStringRef(StringRef(value_buf)); api::global_api_server->send_homeassistant_action(resp); } diff --git a/esphome/components/homeassistant/select/homeassistant_select.cpp b/esphome/components/homeassistant/select/homeassistant_select.cpp index f8e658ac8f6..10db7dca7a7 100644 --- a/esphome/components/homeassistant/select/homeassistant_select.cpp +++ b/esphome/components/homeassistant/select/homeassistant_select.cpp @@ -6,6 +6,7 @@ #include "esphome/components/api/api_server.h" #include "esphome/core/helpers.h" #include "esphome/core/log.h" +#include "esphome/core/progmem.h" #include "esphome/core/string_ref.h" namespace esphome::homeassistant { @@ -237,27 +238,26 @@ void HomeassistantSelect::control(size_t index) { } #endif - static constexpr auto SERVICE_SELECT = StringRef::from_lit("select.select_option"); - static constexpr auto SERVICE_INPUT_SELECT = StringRef::from_lit("input_select.select_option"); - static constexpr auto ENTITY_ID_KEY = StringRef::from_lit("entity_id"); - static constexpr auto OPTION_KEY = StringRef::from_lit("option"); - static constexpr char INPUT_PREFIX[] = "input_"; + // In flash on the ESP8266 and encoded from there + static const char SERVICE_SELECT[] PROGMEM = "select.select_option"; + static const char SERVICE_INPUT_SELECT[] PROGMEM = "input_select.select_option"; + static const char ENTITY_ID_KEY[] PROGMEM = "entity_id"; + static const char OPTION_KEY[] PROGMEM = "option"; api::HomeassistantActionRequest resp; - if (strncmp(this->entity_id_, INPUT_PREFIX, sizeof(INPUT_PREFIX) - 1) == 0) { - resp.service = SERVICE_INPUT_SELECT; - } else { - resp.service = SERVICE_SELECT; - } + // An input helper entity id starts with the "input_" its action name starts with + const bool input_helper = ESPHOME_strncmp_P(this->entity_id_, SERVICE_INPUT_SELECT, sizeof("input_") - 1) == 0; + resp.service = + input_helper ? ProgmemStringRef::from_lit(SERVICE_INPUT_SELECT) : ProgmemStringRef::from_lit(SERVICE_SELECT); resp.data.init(2); auto &entity_id = resp.data.emplace_back(); - entity_id.key = ENTITY_ID_KEY; - entity_id.value = StringRef(this->entity_id_); + entity_id.key = ProgmemStringRef::from_lit(ENTITY_ID_KEY); + entity_id.value = ProgmemStringRef(StringRef(this->entity_id_)); auto &option = resp.data.emplace_back(); - option.key = OPTION_KEY; - option.value = StringRef(this->option_at(index)); + option.key = ProgmemStringRef::from_lit(OPTION_KEY); + option.value = ProgmemStringRef(StringRef(this->option_at(index))); api::global_api_server->send_homeassistant_action(resp); } diff --git a/esphome/components/homeassistant/switch/homeassistant_switch.cpp b/esphome/components/homeassistant/switch/homeassistant_switch.cpp index 584573d8818..f09b8b9dc55 100644 --- a/esphome/components/homeassistant/switch/homeassistant_switch.cpp +++ b/esphome/components/homeassistant/switch/homeassistant_switch.cpp @@ -1,6 +1,7 @@ #include "homeassistant_switch.h" #include "esphome/components/api/api_server.h" #include "esphome/core/log.h" +#include "esphome/core/progmem.h" #include "esphome/core/string_ref.h" namespace esphome::homeassistant { @@ -47,21 +48,18 @@ void HomeassistantSwitch::write_state(bool state) { } #endif - static constexpr auto SERVICE_ON = StringRef::from_lit("homeassistant.turn_on"); - static constexpr auto SERVICE_OFF = StringRef::from_lit("homeassistant.turn_off"); - static constexpr auto ENTITY_ID_KEY = StringRef::from_lit("entity_id"); + // In flash on the ESP8266 and encoded from there + static const char SERVICE_ON[] PROGMEM = "homeassistant.turn_on"; + static const char SERVICE_OFF[] PROGMEM = "homeassistant.turn_off"; + static const char ENTITY_ID_KEY[] PROGMEM = "entity_id"; api::HomeassistantActionRequest resp; - if (state) { - resp.service = SERVICE_ON; - } else { - resp.service = SERVICE_OFF; - } + resp.service = state ? ProgmemStringRef::from_lit(SERVICE_ON) : ProgmemStringRef::from_lit(SERVICE_OFF); resp.data.init(1); auto &entity_id_kv = resp.data.emplace_back(); - entity_id_kv.key = ENTITY_ID_KEY; - entity_id_kv.value = StringRef(this->entity_id_); + entity_id_kv.key = ProgmemStringRef::from_lit(ENTITY_ID_KEY); + entity_id_kv.value = ProgmemStringRef(StringRef(this->entity_id_)); api::global_api_server->send_homeassistant_action(resp); } diff --git a/esphome/components/homeassistant/text/homeassistant_text.cpp b/esphome/components/homeassistant/text/homeassistant_text.cpp index 6c006224dc6..66377bf008a 100644 --- a/esphome/components/homeassistant/text/homeassistant_text.cpp +++ b/esphome/components/homeassistant/text/homeassistant_text.cpp @@ -6,6 +6,7 @@ #include "esphome/components/api/api_server.h" #include "esphome/core/helpers.h" #include "esphome/core/log.h" +#include "esphome/core/progmem.h" #include "esphome/core/string_ref.h" namespace esphome::homeassistant { @@ -86,27 +87,26 @@ void HomeassistantText::control(const std::string &value) { } #endif - static constexpr auto SERVICE_TEXT = StringRef::from_lit("text.set_value"); - static constexpr auto SERVICE_INPUT_TEXT = StringRef::from_lit("input_text.set_value"); - static constexpr auto ENTITY_ID_KEY = StringRef::from_lit("entity_id"); - static constexpr auto VALUE_KEY = StringRef::from_lit("value"); - static constexpr char INPUT_PREFIX[] = "input_"; + // In flash on the ESP8266 and encoded from there + static const char SERVICE_TEXT[] PROGMEM = "text.set_value"; + static const char SERVICE_INPUT_TEXT[] PROGMEM = "input_text.set_value"; + static const char ENTITY_ID_KEY[] PROGMEM = "entity_id"; + static const char VALUE_KEY[] PROGMEM = "value"; api::HomeassistantActionRequest resp; - if (strncmp(this->entity_id_, INPUT_PREFIX, sizeof(INPUT_PREFIX) - 1) == 0) { - resp.service = SERVICE_INPUT_TEXT; - } else { - resp.service = SERVICE_TEXT; - } + // An input helper entity id starts with the "input_" its action name starts with + const bool input_helper = ESPHOME_strncmp_P(this->entity_id_, SERVICE_INPUT_TEXT, sizeof("input_") - 1) == 0; + resp.service = + input_helper ? ProgmemStringRef::from_lit(SERVICE_INPUT_TEXT) : ProgmemStringRef::from_lit(SERVICE_TEXT); resp.data.init(2); auto &entity_id_kv = resp.data.emplace_back(); - entity_id_kv.key = ENTITY_ID_KEY; - entity_id_kv.value = StringRef(this->entity_id_); + entity_id_kv.key = ProgmemStringRef::from_lit(ENTITY_ID_KEY); + entity_id_kv.value = ProgmemStringRef(StringRef(this->entity_id_)); auto &value_kv = resp.data.emplace_back(); - value_kv.key = VALUE_KEY; - value_kv.value = StringRef(value.data(), value.size()); + value_kv.key = ProgmemStringRef::from_lit(VALUE_KEY); + value_kv.value = ProgmemStringRef(value.data(), value.size()); api::global_api_server->send_homeassistant_action(resp); } diff --git a/esphome/core/defines.h b/esphome/core/defines.h index fc6dd82ed98..cc48c778cf6 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -314,7 +314,6 @@ #define USE_API_USER_DEFINED_ACTION_RESPONSES #define USE_API_USER_DEFINED_ACTION_RESPONSES_JSON #define API_MAX_SEND_QUEUE 8 -#define API_USER_ACTION_STRINGS_SCRATCH_SIZE 64 #define API_WIZARD_DATA_SIZE 1 #define API_WIZARD_INPUT_COUNT 1 #define MAX_API_CONNECTIONS 6 diff --git a/esphome/core/string_ref.h b/esphome/core/string_ref.h index d8c68d7e7fa..c3a4ff339c2 100644 --- a/esphome/core/string_ref.h +++ b/esphome/core/string_ref.h @@ -232,6 +232,8 @@ class ProgmemStringRef { constexpr ProgmemStringRef() = default; constexpr ProgmemStringRef(const char *s, size_t len) : ref_(s, len) {} constexpr explicit ProgmemStringRef(const StringRef &s) : ref_(s) {} + /// A PROGMEM char array, with its length known at compile time + template static constexpr ProgmemStringRef from_lit(const char (&s)[N]) { return {s, N - 1}; } constexpr const char *progmem_ptr() const { return this->ref_.c_str(); } constexpr size_t size() const { return this->ref_.size(); } diff --git a/tests/component_tests/api/test_action_metadata.py b/tests/component_tests/api/test_action_metadata.py index 69c9605a675..2b7662b6655 100644 --- a/tests/component_tests/api/test_action_metadata.py +++ b/tests/component_tests/api/test_action_metadata.py @@ -8,18 +8,13 @@ import pytest from esphome.components.api import ( _action_strings, - _action_strings_size, _has_action_metadata, - _validate_esp8266_action_strings, validate_variable, ) from esphome.config_validation import Invalid -from esphome.const import PlatformFramework 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 -from tests.component_tests.types import SetCoreConfigCallable CONFIG = "tests/component_tests/api/test_action_metadata.yaml" CONFIG_ESP8266 = "tests/component_tests/api/test_action_metadata_esp8266.yaml" @@ -45,7 +40,6 @@ def test_metadata_is_emitted_as_progmem_table( ) assert f"(api_action0_strings, {safe_exp(fnv1_hash('play_buzzer'))});" in main_cpp assert "USE_API_USER_DEFINED_ACTION_METADATA" in {d.name for d in CORE.defines} - assert get_define_value("API_USER_ACTION_STRINGS_SCRATCH_SIZE") is None def test_esp8266_strings_are_shared_progmem_arrays( @@ -69,15 +63,13 @@ def test_esp8266_strings_are_shared_progmem_arrays( ) -def test_esp8266_sizes_scratch_buffer_for_largest_action( +def test_esp8266_encodes_action_strings_from_flash( generate_main: Callable[[str | Path], str], ) -> None: - """ESP8266 gets a scratch buffer define equal to the byte total of the largest action.""" - generate_main(CONFIG_ESP8266) + """ESP8266 uses the same flash string table.""" + main_cpp = generate_main(CONFIG_ESP8266) - # play_buzzer: name, description, two variable names, one description, one example, - # each with a terminator - assert get_define_value("API_USER_ACTION_STRINGS_SCRATCH_SIZE") == "117" + assert "api_action0_strings[] PROGMEM" in main_cpp def test_shorthand_variables_emit_no_metadata( @@ -113,36 +105,8 @@ def test_variable_rejects_invalid(value: object) -> None: validate_variable(value) -def _oversized_action_config() -> dict: - return { - "actions": [ - { - "action": "big", - "description": "x" * 300, - "variables": {"a": {"type": "string", "example": "y" * 300}}, - } - ] - } - - -def test_esp8266_rejects_actions_over_string_budget( - set_core_config: SetCoreConfigCallable, -) -> None: - set_core_config(PlatformFramework.ESP8266_ARDUINO) - with pytest.raises(Invalid, match="ESP8266 allows at most 384 bytes"): - _validate_esp8266_action_strings(_oversized_action_config()) - - -def test_other_platforms_have_no_string_budget( - set_core_config: SetCoreConfigCallable, -) -> None: - set_core_config(PlatformFramework.ESP32_IDF) - config = _oversized_action_config() - assert _validate_esp8266_action_strings(config) is config - - -def test_empty_metadata_is_unset_and_not_counted() -> None: - """An empty description or example emits nullptr and takes no scratch space.""" +def test_empty_metadata_is_unset() -> None: + """An empty description or example emits nullptr.""" conf = { "action": "a", "description": "", @@ -150,8 +114,6 @@ def test_empty_metadata_is_unset_and_not_counted() -> None: } strings = _action_strings(conf, has_metadata=True) assert strings == ["a", None, "b", None, "ex"] - # Every emitted string counts its terminator: "a" + "b" + "ex" - assert _action_strings_size(strings) == 2 + 2 + 3 def test_empty_metadata_does_not_enable_the_define() -> None: diff --git a/tests/components/core/test_progmem_string_ref.cpp b/tests/components/core/test_progmem_string_ref.cpp index fe6dd0e24a2..1f40d53cabd 100644 --- a/tests/components/core/test_progmem_string_ref.cpp +++ b/tests/components/core/test_progmem_string_ref.cpp @@ -1,5 +1,6 @@ #include +#include "esphome/core/progmem.h" #include "esphome/core/string_ref.h" #include @@ -39,6 +40,13 @@ TEST(ProgmemStringRef, EqualsComparesLengthAndContent) { EXPECT_FALSE(ref.equals(StringRef("Bedroom"))); } +TEST(ProgmemStringRef, FromLitTakesTheLengthAtCompileTime) { + static const char NAME[] PROGMEM = "entity_id"; + constexpr ProgmemStringRef ref = ProgmemStringRef::from_lit(NAME); + static_assert(ref.size() == 9); + EXPECT_TRUE(ref.equals(StringRef("entity_id"))); +} + TEST(ProgmemStringRef, DefaultIsEmpty) { ProgmemStringRef ref; EXPECT_TRUE(ref.empty());