From fb65096ea3dc4cccf53681b501dec01fd2ec9538 Mon Sep 17 00:00:00 2001 From: Brandon Harvey <8107750+bharvey88@users.noreply.github.com> Date: Sun, 30 Aug 2026 13:52:51 -0500 Subject: [PATCH] [api] Add description and example metadata to user-defined actions (#18881) Co-authored-by: J. Nick Koston --- esphome/codegen.py | 1 + esphome/components/api/__init__.py | 138 ++++++++++++++-- esphome/components/api/api.proto | 3 + esphome/components/api/api_pb2.cpp | 18 ++ esphome/components/api/api_pb2.h | 11 +- esphome/components/api/api_pb2_dump.cpp | 9 + esphome/components/api/list_entities.cpp | 3 +- esphome/components/api/user_services.cpp | 43 +++++ esphome/components/api/user_services.h | 93 ++++++----- esphome/components/const/__init__.py | 1 + esphome/core/defines.h | 2 + esphome/cpp_types.py | 1 + .../api/test_action_metadata.py | 155 ++++++++++++++++++ .../api/test_action_metadata.yaml | 14 ++ .../api/test_action_metadata_common.yaml | 18 ++ .../api/test_action_metadata_esp8266.yaml | 14 ++ .../api/test_action_metadata_shorthand.yaml | 19 +++ .../api/test_homeassistant_action.py | 4 +- tests/components/api/common-base.yaml | 6 +- tests/components/api/common.yaml | 3 +- .../fixtures/api_action_metadata.yaml | 26 +++ tests/integration/test_api_action_metadata.py | 65 ++++++++ 22 files changed, 591 insertions(+), 56 deletions(-) create mode 100644 tests/component_tests/api/test_action_metadata.py create mode 100644 tests/component_tests/api/test_action_metadata.yaml create mode 100644 tests/component_tests/api/test_action_metadata_common.yaml create mode 100644 tests/component_tests/api/test_action_metadata_esp8266.yaml create mode 100644 tests/component_tests/api/test_action_metadata_shorthand.yaml create mode 100644 tests/integration/fixtures/api_action_metadata.yaml create mode 100644 tests/integration/test_api_action_metadata.py diff --git a/esphome/codegen.py b/esphome/codegen.py index 2aa6a70abd..5debb52b4e 100644 --- a/esphome/codegen.py +++ b/esphome/codegen.py @@ -78,6 +78,7 @@ from esphome.cpp_types import ( # noqa: F401 StringRef, arduino_json_ns, bool_, + char, const_char_ptr, double, esphome_ns, diff --git a/esphome/components/api/__init__.py b/esphome/components/api/__init__.py index 2e891a9663..3568318dad 100644 --- a/esphome/components/api/__init__.py +++ b/esphome/components/api/__init__.py @@ -5,6 +5,7 @@ from typing import Any from esphome import automation from esphome.automation import Condition import esphome.codegen as cg +from esphome.components.const import CONF_DESCRIPTION from esphome.components.logger import request_log_listener # ENCRYPTION_SCHEMA and validate_encryption_key are re-exported for external @@ -41,10 +42,12 @@ from esphome.const import ( CONF_TAG, CONF_THEN, CONF_TRIGGER_ID, + CONF_TYPE, CONF_VARIABLES, ) from esphome.core import CORE, ID, CoroPriority, EsphomeError, coroutine_with_priority from esphome.cpp_generator import MockObj, TemplateArgsType +from esphome.helpers import fnv1_hash from esphome.types import ConfigFragmentType, ConfigType # Compat alias: downstream consumers (e.g. device-builder) referenced the @@ -125,6 +128,7 @@ SERVICE_ARG_FALLBACK_TYPES: dict[str, MockObj] = { } CONF_BATCH_DELAY = "batch_delay" CONF_CUSTOM_SERVICES = "custom_services" +CONF_EXAMPLE = "example" CONF_HOMEASSISTANT_SERVICES = "homeassistant_services" CONF_HOMEASSISTANT_STATES = "homeassistant_states" CONF_LISTEN_BACKLOG = "listen_backlog" @@ -228,14 +232,30 @@ 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), + cv.Optional(CONF_DESCRIPTION): cv.string_strict, + cv.Optional(CONF_EXAMPLE): cv.string_strict, + } +) + +# Accepts the plain `name: type` shorthand or the full mapping form +validate_variable = cv.maybe_simple_value(VARIABLE_SCHEMA, key=CONF_TYPE) + + ACTIONS_SCHEMA = automation.validate_automation( { cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(UserServiceTrigger), cv.Exclusive(CONF_SERVICE, group_of_exclusion=CONF_ACTION): cv.valid_name, cv.Exclusive(CONF_ACTION, group_of_exclusion=CONF_ACTION): cv.valid_name, + cv.Optional(CONF_DESCRIPTION): cv.string_strict, cv.Optional(CONF_VARIABLES, default={}): cv.Schema( { - cv.validate_id_name: cv.one_of(*SERVICE_ARG_NATIVE_TYPES, lower=True), + cv.validate_id_name: validate_variable, } ), # No default - auto-detected by _auto_detect_supports_response @@ -352,6 +372,85 @@ CONFIG_SCHEMA = cv.All( ) +def _has_action_metadata(actions: list[ConfigType]) -> bool: + # Empty strings count as unset, matching _action_strings + return any( + conf.get(CONF_DESCRIPTION) + or any( + var_.get(CONF_DESCRIPTION) or var_.get(CONF_EXAMPLE) + for var_ in conf[CONF_VARIABLES].values() + ) + for conf in actions + ) + + +def _action_strings(conf: ConfigType, has_metadata: bool) -> list[str | None]: + """Strings of one action in the table order UserServiceStatic (user_services.h) expects.""" + # An empty description or example is treated as unset + strings: list[str | None] = [conf[CONF_ACTION]] + if has_metadata: + strings.append(conf.get(CONF_DESCRIPTION) or None) + for name, var_ in conf[CONF_VARIABLES].items(): + strings.append(name) + if has_metadata: + strings += [ + var_.get(CONF_DESCRIPTION) or None, + var_.get(CONF_EXAMPLE) or 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 + + +FINAL_VALIDATE_SCHEMA = _validate_esp8266_action_strings + + +def _add_action_strings( + index: int, strings: list[str | None], interned: dict[str, MockObj] +) -> MockObj: + """Emit the PROGMEM string table for one action. + + Each string is its own PROGMEM array because on ESP8266 .rodata is RAM, and identical + strings are shared between actions through `interned`. + """ + entries: list[MockObj] = [] + for string in strings: + if string is None: + entries.append(cg.nullptr) + continue + if (var := interned.get(string)) is None: + var = interned[string] = cg.progmem_array( + ID(f"api_action_str{len(interned)}", is_declaration=True, type=cg.char), + string, + ) + entries.append(var) + return cg.progmem_array( + ID(f"api_action{index}_strings", is_declaration=True, type=cg.const_char_ptr), + entries, + ) + + @coroutine_with_priority(CoroPriority.WEB) async def to_code(config: ConfigType) -> None: var = cg.new_Pvariable(config[CONF_ID]) @@ -371,8 +470,10 @@ async def to_code(config: ConfigType) -> None: cg.add_define("MAX_API_CONNECTIONS", config[CONF_MAX_CONNECTIONS]) cg.add_define("API_MAX_SEND_QUEUE", config[CONF_MAX_SEND_QUEUE]) + actions = config.get(CONF_ACTIONS, []) + has_user_actions = bool(actions) or config[CONF_CUSTOM_SERVICES] # Set USE_API_USER_DEFINED_ACTIONS if any services are enabled - if config.get(CONF_ACTIONS) or config[CONF_CUSTOM_SERVICES]: + if has_user_actions: cg.add_define("USE_API_USER_DEFINED_ACTIONS") # Set USE_API_CUSTOM_SERVICES if external components need dynamic service registration @@ -385,10 +486,17 @@ async def to_code(config: ConfigType) -> None: if config[CONF_HOMEASSISTANT_STATES]: cg.add_define("USE_API_HOMEASSISTANT_STATES") - if actions := config.get(CONF_ACTIONS, []): + 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 + has_metadata = _has_action_metadata(actions) + if has_metadata: + cg.add_define("USE_API_USER_DEFINED_ACTION_METADATA") + interned_strings: dict[str, MockObj] = {} # Collect all triggers first, then register all at once with initializer_list triggers: list[cg.MockObj] = [] - for conf in actions: + for index, conf in enumerate(actions): func_args: list[tuple[MockObj, str]] = [] service_template_args: list[MockObj] = [] # User service argument types @@ -421,22 +529,23 @@ async def to_code(config: ConfigType) -> None: conf.get(CONF_THEN, []) ) - service_arg_names: list[str] = [] for name, var_ in conf[CONF_VARIABLES].items(): - if has_non_synchronous and var_ in SERVICE_ARG_FALLBACK_TYPES: - native = SERVICE_ARG_FALLBACK_TYPES[var_] + var_type = var_[CONF_TYPE] + if has_non_synchronous and var_type in SERVICE_ARG_FALLBACK_TYPES: + native = SERVICE_ARG_FALLBACK_TYPES[var_type] else: - native = SERVICE_ARG_NATIVE_TYPES[var_] + native = SERVICE_ARG_NATIVE_TYPES[var_type] service_template_args.append(native) func_args.append((native, name)) - service_arg_names.append(name) + strings = _action_strings(conf, has_metadata) + table = _add_action_strings(index, strings, interned_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 trigger = cg.new_Pvariable( - conf[CONF_TRIGGER_ID], - templ, - conf[CONF_ACTION], - service_arg_names, + conf[CONF_TRIGGER_ID], templ, table, fnv1_hash(conf[CONF_ACTION]) ) triggers.append(trigger) auto = await automation.build_automation(trigger, func_args, conf) @@ -458,6 +567,9 @@ async def to_code(config: ConfigType) -> None: cg.add(auto.add_actions([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 CONF_ON_CLIENT_CONNECTED in config: cg.add_define("USE_API_CLIENT_CONNECTED_TRIGGER") diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index c11700782e..3a0e0abea9 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -1034,6 +1034,8 @@ message ListEntitiesServicesArgument { option (ifdef) = "USE_API_USER_DEFINED_ACTIONS"; string name = 1; 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"]; } message ListEntitiesServicesResponse { option (id) = 41; @@ -1044,6 +1046,7 @@ message ListEntitiesServicesResponse { 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"]; } 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 f56d791b67..2de1f0a15c 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -1275,12 +1275,24 @@ uint8_t *ListEntitiesServicesArgument::encode(ProtoWriteBuffer &buffer PROTO_ENC uint8_t *__restrict__ pos = buffer.get_pos(); ProtoEncode::encode_string(pos PROTO_ENCODE_DEBUG_ARG, 1, this->name); ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 2, static_cast(this->type)); +#ifdef USE_API_USER_DEFINED_ACTION_METADATA + ProtoEncode::encode_string(pos PROTO_ENCODE_DEBUG_ARG, 3, this->description); +#endif +#ifdef USE_API_USER_DEFINED_ACTION_METADATA + ProtoEncode::encode_string(pos PROTO_ENCODE_DEBUG_ARG, 4, this->example); +#endif return pos; } uint32_t ListEntitiesServicesArgument::calculate_size() const { uint32_t size = 0; size += ProtoSize::calc_length(1, this->name.size()); size += this->type ? 2 : 0; +#ifdef USE_API_USER_DEFINED_ACTION_METADATA + size += ProtoSize::calc_length(1, this->description.size()); +#endif +#ifdef USE_API_USER_DEFINED_ACTION_METADATA + size += ProtoSize::calc_length(1, this->example.size()); +#endif return size; } uint8_t *ListEntitiesServicesResponse::encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const { @@ -1291,6 +1303,9 @@ uint8_t *ListEntitiesServicesResponse::encode(ProtoWriteBuffer &buffer PROTO_ENC ProtoEncode::encode_sub_message(pos PROTO_ENCODE_DEBUG_ARG, buffer, 3, it); } ProtoEncode::encode_uint32(pos PROTO_ENCODE_DEBUG_ARG, 4, static_cast(this->supports_response)); +#ifdef USE_API_USER_DEFINED_ACTION_METADATA + ProtoEncode::encode_string(pos PROTO_ENCODE_DEBUG_ARG, 5, this->description); +#endif return pos; } uint32_t ListEntitiesServicesResponse::calculate_size() const { @@ -1303,6 +1318,9 @@ uint32_t ListEntitiesServicesResponse::calculate_size() const { } } size += this->supports_response ? 2 : 0; +#ifdef USE_API_USER_DEFINED_ACTION_METADATA + size += ProtoSize::calc_length(1, this->description.size()); +#endif return size; } bool ExecuteServiceArgument::decode_varint(uint32_t field_id, proto_varint_value_t value) { diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index bed28d2956..5c3429a63a 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -1317,6 +1317,12 @@ class ListEntitiesServicesArgument final : public ProtoMessage { public: StringRef name{}; enums::ServiceArgType type{}; +#ifdef USE_API_USER_DEFINED_ACTION_METADATA + StringRef description{}; +#endif +#ifdef USE_API_USER_DEFINED_ACTION_METADATA + StringRef example{}; +#endif uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const; uint32_t calculate_size() const; #ifdef HAS_PROTO_MESSAGE_DUMP @@ -1328,7 +1334,7 @@ class ListEntitiesServicesArgument final : public ProtoMessage { class ListEntitiesServicesResponse final : public ProtoMessage { public: static constexpr uint16_t MESSAGE_TYPE = 41; - static constexpr uint8_t ESTIMATED_SIZE = 50; + static constexpr uint8_t ESTIMATED_SIZE = 59; #ifdef HAS_PROTO_MESSAGE_DUMP const LogString *message_name() const override { return LOG_STR("list_entities_services_response"); } #endif @@ -1336,6 +1342,9 @@ class ListEntitiesServicesResponse final : public ProtoMessage { uint32_t key{0}; FixedVector args{}; enums::SupportsResponseType supports_response{}; +#ifdef USE_API_USER_DEFINED_ACTION_METADATA + StringRef description{}; +#endif uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const; uint32_t calculate_size() const; #ifdef HAS_PROTO_MESSAGE_DUMP diff --git a/esphome/components/api/api_pb2_dump.cpp b/esphome/components/api/api_pb2_dump.cpp index 846c0ad652..dced81ee30 100644 --- a/esphome/components/api/api_pb2_dump.cpp +++ b/esphome/components/api/api_pb2_dump.cpp @@ -1500,6 +1500,12 @@ const char *ListEntitiesServicesArgument::dump_to(DumpBuffer &out) const { MessageDumpHelper helper(out, ESPHOME_PSTR("ListEntitiesServicesArgument")); dump_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); +#endif +#ifdef USE_API_USER_DEFINED_ACTION_METADATA + dump_field(out, ESPHOME_PSTR("example"), this->example); +#endif return out.c_str(); } const char *ListEntitiesServicesResponse::dump_to(DumpBuffer &out) const { @@ -1512,6 +1518,9 @@ const char *ListEntitiesServicesResponse::dump_to(DumpBuffer &out) const { out.append("\n"); } 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); +#endif return out.c_str(); } const char *ExecuteServiceArgument::dump_to(DumpBuffer &out) const { diff --git a/esphome/components/api/list_entities.cpp b/esphome/components/api/list_entities.cpp index 57ff616ca7..507b098fb4 100644 --- a/esphome/components/api/list_entities.cpp +++ b/esphome/components/api/list_entities.cpp @@ -99,7 +99,8 @@ ListEntitiesIterator::ListEntitiesIterator(APIConnection *client) : client_(clie static constexpr uint8_t SERVICE_YIELD_INTERVAL = 3; bool ListEntitiesIterator::on_service(UserServiceDescriptor *service) { - auto resp = service->encode_list_service_response(); + UserActionScratch scratch; + auto resp = service->encode_list_service_response(scratch); 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 28a43c656c..fad3cde29b 100644 --- a/esphome/components/api/user_services.cpp +++ b/esphome/components/api/user_services.cpp @@ -1,9 +1,52 @@ #include "user_services.h" +#include "esphome/core/hal.h" #include "esphome/core/log.h" #include "esphome/core/string_ref.h" namespace esphome::api { +StringRef UserServiceStatic::str_(size_t idx, std::span &scratch) 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 +} + +ListEntitiesServicesResponse UserServiceStatic::encode_list_service_response_( + std::span arg_types, std::span scratch) const { + ListEntitiesServicesResponse msg; + msg.name = this->str_(0, scratch); + msg.key = this->key_; + msg.supports_response = this->supports_response_; +#ifdef USE_API_USER_DEFINED_ACTION_METADATA + msg.description = this->str_(1, scratch); +#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); +#ifdef USE_API_USER_DEFINED_ACTION_METADATA + arg.description = this->str_(base + 1, scratch); + arg.example = this->str_(base + 2, scratch); +#endif + } + return msg; +} + template<> bool get_execute_arg_value(const ExecuteServiceArgument &arg) { return arg.bool_; } template<> int32_t get_execute_arg_value(const ExecuteServiceArgument &arg) { if (arg.legacy_int != 0) diff --git a/esphome/components/api/user_services.h b/esphome/components/api/user_services.h index ea57d0944b..3b17bdb7bc 100644 --- a/esphome/components/api/user_services.h +++ b/esphome/components/api/user_services.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -19,7 +20,9 @@ class APIServer; class UserServiceDescriptor { public: - virtual ListEntitiesServicesResponse encode_list_service_response() = 0; + /// 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; virtual bool execute_service(const ExecuteServiceRequest &req) = 0; #ifdef USE_API_USER_DEFINED_ACTION_RESPONSES @@ -34,29 +37,51 @@ template T get_execute_arg_value(const ExecuteServiceArgument &arg); template enums::ServiceArgType to_service_arg_type(); -// Base class for YAML-defined services (most common case) -// Stores only pointers to string literals in flash - no heap allocation -template class UserServiceBase : public UserServiceDescriptor { - public: - UserServiceBase(const char *name, const std::array &arg_names, - enums::SupportsResponseType supports_response = enums::SUPPORTS_RESPONSE_NONE) - : name_(name), arg_names_(arg_names), supports_response_(supports_response) { - this->key_ = fnv1_hash(name); - } +// 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 - ListEntitiesServicesResponse encode_list_service_response() override { - ListEntitiesServicesResponse msg; - msg.name = StringRef(this->name_); - msg.key = this->key_; - msg.supports_response = this->supports_response_; +// 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 +// each argument name; with USE_API_USER_DEFINED_ACTION_METADATA the action description follows +// the name and every argument is (name, description, example). Unset metadata is nullptr. +#ifdef USE_API_USER_DEFINED_ACTION_METADATA +static constexpr size_t USER_ACTION_HEADER_STRINGS = 2; +static constexpr size_t USER_ACTION_ARG_STRINGS = 3; +#else +static constexpr size_t USER_ACTION_HEADER_STRINGS = 1; +static constexpr size_t USER_ACTION_ARG_STRINGS = 1; +#endif +class UserServiceStatic : public UserServiceDescriptor { + public: + UserServiceStatic(const char *const *strings, uint32_t key, + enums::SupportsResponseType supports_response = enums::SUPPORTS_RESPONSE_NONE) + : 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; + + const char *const *strings_; // PROGMEM pointer table, read with progmem_read_ptr() + uint32_t key_; + enums::SupportsResponseType supports_response_; +}; + +template class UserServiceBase : public UserServiceStatic { + public: + using UserServiceStatic::UserServiceStatic; + + ListEntitiesServicesResponse encode_list_service_response(std::span scratch) override { std::array arg_types = {to_service_arg_type()...}; - msg.args.init(sizeof...(Ts)); - 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]); - } - return msg; + return this->encode_list_service_response_(arg_types, scratch); } bool execute_service(const ExecuteServiceRequest &req) override { @@ -89,12 +114,6 @@ template class UserServiceBase : public UserServiceDescriptor { void execute_(const ArgsContainer &args, uint32_t call_id, bool return_response, std::index_sequence /*type*/) { this->execute(call_id, return_response, (get_execute_arg_value(args[S]))...); } - - // Pointers to string literals in flash - no heap allocation - const char *name_; - std::array arg_names_; - uint32_t key_{0}; - enums::SupportsResponseType supports_response_{enums::SUPPORTS_RESPONSE_NONE}; }; // Separate class for custom_api_device services (rare case) @@ -106,7 +125,7 @@ template class UserServiceDynamic : public UserServiceDescriptor this->key_ = fnv1_hash(this->name_.c_str()); } - ListEntitiesServicesResponse encode_list_service_response() override { + ListEntitiesServicesResponse encode_list_service_response(std::span /*scratch*/) override { ListEntitiesServicesResponse msg; msg.name = StringRef(this->name_); msg.key = this->key_; @@ -167,8 +186,8 @@ template class UserServiceTrigger final : public UserServiceBase, public Trigger { public: - UserServiceTrigger(const char *name, const std::array &arg_names) - : UserServiceBase(name, arg_names, enums::SUPPORTS_RESPONSE_NONE) {} + UserServiceTrigger(const char *const *strings, uint32_t key) + : UserServiceBase(strings, key, enums::SUPPORTS_RESPONSE_NONE) {} protected: void execute(uint32_t /*call_id*/, bool /*return_response*/, Ts... x) override { this->trigger(x...); } @@ -179,8 +198,8 @@ template class UserServiceTrigger final : public UserServiceBase, public Trigger { public: - UserServiceTrigger(const char *name, const std::array &arg_names) - : UserServiceBase(name, arg_names, enums::SUPPORTS_RESPONSE_OPTIONAL) {} + UserServiceTrigger(const char *const *strings, uint32_t key) + : UserServiceBase(strings, key, enums::SUPPORTS_RESPONSE_OPTIONAL) {} protected: void execute(uint32_t call_id, bool return_response, Ts... x) override { @@ -193,8 +212,8 @@ template class UserServiceTrigger final : public UserServiceBase, public Trigger { public: - UserServiceTrigger(const char *name, const std::array &arg_names) - : UserServiceBase(name, arg_names, enums::SUPPORTS_RESPONSE_ONLY) {} + UserServiceTrigger(const char *const *strings, uint32_t key) + : UserServiceBase(strings, key, enums::SUPPORTS_RESPONSE_ONLY) {} protected: void execute(uint32_t call_id, bool /*return_response*/, Ts... x) override { this->trigger(call_id, x...); } @@ -205,8 +224,8 @@ template class UserServiceTrigger final : public UserServiceBase, public Trigger { public: - UserServiceTrigger(const char *name, const std::array &arg_names) - : UserServiceBase(name, arg_names, enums::SUPPORTS_RESPONSE_STATUS) {} + UserServiceTrigger(const char *const *strings, uint32_t key) + : UserServiceBase(strings, key, enums::SUPPORTS_RESPONSE_STATUS) {} protected: void execute(uint32_t call_id, bool /*return_response*/, Ts... x) override { this->trigger(call_id, x...); } diff --git a/esphome/components/const/__init__.py b/esphome/components/const/__init__.py index 10710c8d29..e445a4abde 100644 --- a/esphome/components/const/__init__.py +++ b/esphome/components/const/__init__.py @@ -16,6 +16,7 @@ CONF_CO2_EQUIVALENT = "co2_equivalent" CONF_COLOR_DEPTH = "color_depth" CONF_CRC_ENABLE = "crc_enable" CONF_DATA_BITS = "data_bits" +CONF_DESCRIPTION = "description" CONF_DRAW_ROUNDING = "draw_rounding" CONF_ENABLE_OTA_DOWNGRADE_PROTECTION = "enable_ota_downgrade_protection" CONF_ENABLED = "enabled" diff --git a/esphome/core/defines.h b/esphome/core/defines.h index 625d4879f5..1f5a10d47d 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -218,9 +218,11 @@ #define USE_API_PLAINTEXT #define USE_API_USER_DEFINED_ACTIONS #define USE_API_CUSTOM_SERVICES +#define USE_API_USER_DEFINED_ACTION_METADATA #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 MAX_API_CONNECTIONS 6 // The Improv library is not in the Zephyr tidy environment #define USE_IMPROV_SERIAL diff --git a/esphome/cpp_types.py b/esphome/cpp_types.py index aeaa4480a8..45d6559b3f 100644 --- a/esphome/cpp_types.py +++ b/esphome/cpp_types.py @@ -14,6 +14,7 @@ std_string_ref = std_ns.namespace("string &") std_vector = std_ns.class_("vector") std_span = std_ns.class_("span") int8 = global_ns.namespace("int8_t") +char = global_ns.namespace("char") uint8 = global_ns.namespace("uint8_t") uint16 = global_ns.namespace("uint16_t") uint32 = global_ns.namespace("uint32_t") diff --git a/tests/component_tests/api/test_action_metadata.py b/tests/component_tests/api/test_action_metadata.py new file mode 100644 index 0000000000..adbfdf306e --- /dev/null +++ b/tests/component_tests/api/test_action_metadata.py @@ -0,0 +1,155 @@ +"""Tests for user-defined action field metadata (description / example).""" + +from collections.abc import Callable +from pathlib import Path + +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" +CONFIG_SHORTHAND = "tests/component_tests/api/test_action_metadata_shorthand.yaml" + + +def test_metadata_is_emitted_as_progmem_table( + generate_main: Callable[[str | Path], str], +) -> None: + """Every action string is a PROGMEM array referenced from one PROGMEM table.""" + main_cpp = generate_main(CONFIG) + + assert ( + 'static constexpr char api_action_str0[] PROGMEM = "play_buzzer";' in main_cpp + ) + assert ( + 'static constexpr char api_action_str1[] PROGMEM = "Play an RTTTL melody on the buzzer";' + in main_cpp + ) + assert ( + 'static constexpr char api_action_str4[] PROGMEM = "two_short:d=4,o=5,b=100:16e6,16e6";' + in main_cpp + ) + assert ( + "static constexpr const char * api_action0_strings[] PROGMEM = {" + "api_action_str0, api_action_str1, api_action_str2, api_action_str3, " + "api_action_str4, api_action_str5, nullptr, nullptr};" in main_cpp + ) + # An action without metadata still carries the metadata slots (as nullptr) + assert ( + "static constexpr const char * api_action1_strings[] PROGMEM = {" + "api_action_str6, nullptr, api_action_str7, nullptr, nullptr};" in main_cpp + ) + 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_sizes_scratch_buffer_for_largest_action( + 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) + + # 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" + + +def test_shorthand_variables_emit_no_metadata( + generate_main: Callable[[str | Path], str], +) -> None: + """The name: type shorthand emits a name-only table and no define.""" + main_cpp = generate_main(CONFIG_SHORTHAND) + + assert ( + "static constexpr const char * api_action0_strings[] PROGMEM = " + "{api_action_str0, api_action_str1};" in main_cpp + ) + assert "USE_API_USER_DEFINED_ACTION_METADATA" not in {d.name for d in CORE.defines} + + +def test_variable_shorthand_normalizes_to_mapping() -> None: + """A bare type string validates to the mapping form.""" + assert validate_variable("string") == {"type": "string"} + + +@pytest.mark.parametrize( + "value", + [ + {"description": "no type given"}, + {"type": "string", "selector": "text"}, + "stringy", + {"type": "stringy"}, + ], +) +def test_variable_rejects_invalid(value: object) -> None: + """Missing or unknown type and unknown keys raise in both forms.""" + with pytest.raises(Invalid): + 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.""" + conf = { + "action": "a", + "description": "", + "variables": {"b": {"type": "int", "description": "", "example": "ex"}}, + } + 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: + actions = [ + { + "action": "a", + "description": "", + "variables": {"b": {"type": "int", "example": ""}}, + } + ] + assert not _has_action_metadata(actions) + actions[0]["variables"]["b"]["example"] = "1" + assert _has_action_metadata(actions) diff --git a/tests/component_tests/api/test_action_metadata.yaml b/tests/component_tests/api/test_action_metadata.yaml new file mode 100644 index 0000000000..c998713874 --- /dev/null +++ b/tests/component_tests/api/test_action_metadata.yaml @@ -0,0 +1,14 @@ +esphome: + name: test + +esp32: + board: esp32dev + +wifi: + ssid: MySSID + password: password1 + +logger: + +packages: + api: !include test_action_metadata_common.yaml diff --git a/tests/component_tests/api/test_action_metadata_common.yaml b/tests/component_tests/api/test_action_metadata_common.yaml new file mode 100644 index 0000000000..bd161efe63 --- /dev/null +++ b/tests/component_tests/api/test_action_metadata_common.yaml @@ -0,0 +1,18 @@ +api: + actions: + - action: play_buzzer + description: Play an RTTTL melody on the buzzer + variables: + song_str: + type: string + description: RTTTL melody string + example: "two_short:d=4,o=5,b=100:16e6,16e6" + volume: + type: int + then: + - logger.log: Action Called + - action: plain_action + variables: + value: int + then: + - logger.log: Action Called diff --git a/tests/component_tests/api/test_action_metadata_esp8266.yaml b/tests/component_tests/api/test_action_metadata_esp8266.yaml new file mode 100644 index 0000000000..94a5839b28 --- /dev/null +++ b/tests/component_tests/api/test_action_metadata_esp8266.yaml @@ -0,0 +1,14 @@ +esphome: + name: test + +esp8266: + board: d1_mini + +wifi: + ssid: MySSID + password: password1 + +logger: + +packages: + api: !include test_action_metadata_common.yaml diff --git a/tests/component_tests/api/test_action_metadata_shorthand.yaml b/tests/component_tests/api/test_action_metadata_shorthand.yaml new file mode 100644 index 0000000000..aa2e1ab424 --- /dev/null +++ b/tests/component_tests/api/test_action_metadata_shorthand.yaml @@ -0,0 +1,19 @@ +esphome: + name: test + +esp32: + board: esp32dev + +wifi: + ssid: MySSID + password: password1 + +logger: + +api: + actions: + - action: plain_action + variables: + value: int + then: + - logger.log: Action Called diff --git a/tests/component_tests/api/test_homeassistant_action.py b/tests/component_tests/api/test_homeassistant_action.py index 611353e7c5..6ee5ac3412 100644 --- a/tests/component_tests/api/test_homeassistant_action.py +++ b/tests/component_tests/api/test_homeassistant_action.py @@ -9,7 +9,7 @@ def test_synchronous_chain_keeps_zero_copy_args(generate_main): assert ( "api::UserServiceTrigger" - '("zero_copy_args", {"message"})' in main_cpp + "(api_action0_strings," in main_cpp ) @@ -22,7 +22,7 @@ def test_response_callback_args_are_owning(generate_main): assert ( "api::UserServiceTrigger" - '("response_args", {"message"})' in main_cpp + "(api_action1_strings," in main_cpp ) assert "api::HomeAssistantServiceCallAction" in main_cpp assert "api::HomeAssistantServiceCallAction" not in main_cpp diff --git a/tests/components/api/common-base.yaml b/tests/components/api/common-base.yaml index c9eb200471..5e3139da48 100644 --- a/tests/components/api/common-base.yaml +++ b/tests/components/api/common-base.yaml @@ -61,8 +61,12 @@ api: reboot_timeout: 0min actions: - action: hello_world + description: Log a greeting variables: - name: string + name: + type: string + description: Name to greet + example: World then: - logger.log: format: Hello World %s! diff --git a/tests/components/api/common.yaml b/tests/components/api/common.yaml index 6115838b6d..42eb32a92a 100644 --- a/tests/components/api/common.yaml +++ b/tests/components/api/common.yaml @@ -1,4 +1,5 @@ -<<: !include common-base.yaml +packages: + base: !include common-base.yaml api: encryption: diff --git a/tests/integration/fixtures/api_action_metadata.yaml b/tests/integration/fixtures/api_action_metadata.yaml new file mode 100644 index 0000000000..802b965110 --- /dev/null +++ b/tests/integration/fixtures/api_action_metadata.yaml @@ -0,0 +1,26 @@ +esphome: + name: api-action-metadata-test +host: +api: + batch_delay: 0ms + actions: + - action: play_buzzer + description: Play an RTTTL melody on the buzzer + variables: + song_str: + type: string + description: RTTTL melody string + example: "two_short:d=4,o=5,b=100:16e6,16e6" + volume: + type: int + then: + - logger.log: + format: "Buzzer: %s" + args: [song_str.c_str()] + - action: plain_action + variables: + value: int + then: + - logger.log: "Plain action called" + +logger: diff --git a/tests/integration/test_api_action_metadata.py b/tests/integration/test_api_action_metadata.py new file mode 100644 index 0000000000..74d40f141b --- /dev/null +++ b/tests/integration/test_api_action_metadata.py @@ -0,0 +1,65 @@ +"""Integration test for user-defined action field metadata.""" + +from __future__ import annotations + +import asyncio +import re + +import pytest + +from esphome.helpers import fnv1_hash + +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_api_action_metadata( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Action and argument metadata reach the client and the actions still run.""" + loop = asyncio.get_running_loop() + buzzer_called = loop.create_future() + plain_called = loop.create_future() + buzzer_pattern = re.compile(r"Buzzer: two_short") + plain_pattern = re.compile(r"Plain action called") + + def check_output(line: str) -> None: + if not buzzer_called.done() and buzzer_pattern.search(line): + buzzer_called.set_result(True) + elif not plain_called.done() and plain_pattern.search(line): + plain_called.set_result(True) + + async with ( + run_compiled(yaml_config, line_callback=check_output), + api_client_connected() as client, + ): + _, services = await client.list_entities_services() + + by_name = {service.name: service for service in services} + assert set(by_name) == {"play_buzzer", "plain_action"} + # Keys are hashed at codegen time and must match what the client expects + for name, service in by_name.items(): + assert service.key == fnv1_hash(name), name + + buzzer = by_name["play_buzzer"] + assert buzzer.description == "Play an RTTTL melody on the buzzer" + args = {arg.name: arg for arg in buzzer.args} + assert args["song_str"].description == "RTTTL melody string" + assert args["song_str"].example == "two_short:d=4,o=5,b=100:16e6,16e6" + # An arg without metadata sends empty strings + assert args["volume"].description == "" + assert args["volume"].example == "" + + # An action without metadata sends empty strings + plain = by_name["plain_action"] + assert plain.description == "" + assert plain.args[0].description == "" + + await client.execute_service( + buzzer, {"song_str": "two_short:d=4,o=5,b=100:16e6,16e6", "volume": 3} + ) + await client.execute_service(plain, {"value": 1}) + await asyncio.wait_for(buzzer_called, timeout=5.0) + await asyncio.wait_for(plain_called, timeout=5.0)