From e8d3c06b60f5a44d6ae31a902cc2e4f5b055a98f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 23 Sep 2026 10:36:42 +0100 Subject: [PATCH] [core][cover] Add register_apply_action for actions that only forward values (#19469) --- AGENTS.md | 14 ++ esphome/automation.py | 193 +++++++++++++++- esphome/components/cover/__init__.py | 113 +--------- esphome/components/cover/automation.h | 44 ---- esphome/components/template/cover/__init__.py | 39 +--- esphome/config_validation.py | 1 + esphome/core/automation.h | 9 +- esphome/core/base_automation.h | 15 ++ esphome/core/helpers.cpp | 9 + esphome/core/progmem.h | 8 + esphome/cpp_generator.py | 4 +- tests/components/template/common-base.yaml | 4 +- .../integration/test_cover_control_action.py | 4 +- tests/unit_tests/test_automation.py | 213 +++++++++++++++++- tests/unit_tests/test_cpp_generator.py | 9 + 15 files changed, 485 insertions(+), 194 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 448bf49114..cf78daa5a3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -443,6 +443,20 @@ file does, and it is the authority when they disagree. The most useful starting Use `synchronous=True` for actions that run to completion inside `play()` without deferring. Use `synchronous=False` if the action may suspend/defer execution (e.g. `delay`, `wait_until`, `script.wait`) or store trigger arguments for later use. + **Actions that only forward templatable values to their parent need no C++ class.** Register them + with `register_apply_action`; do not write a `TEMPLATABLE_VALUE` class or a builder for this shape. + ```python + automation.register_apply_action( + "my_component.set_gains", + schema, + automation.ApplyField(CONF_KP, "set_kp", cg.float_), + automation.ApplyField(CONF_KI, "set_ki", cg.float_), + ) + ``` + The `ApplyField`, `ApplyCall` and `register_apply_action` docstrings in `esphome/automation.py` cover + the rest; `cover.control` and `cover.template.publish` are in-tree examples. `TEMPLATABLE_VALUE` with + `cg.templatable` stays for actions whose `play()` has real logic beyond forwarding values. + * **Conditions:** ```cpp template class MyCondition : public Condition { diff --git a/esphome/automation.py b/esphome/automation.py index 3ffda50c81..373f924e4d 100644 --- a/esphome/automation.py +++ b/esphome/automation.py @@ -1,5 +1,8 @@ +from collections.abc import Callable from dataclasses import dataclass, field import logging +import string +from typing import Any import esphome.codegen as cg import esphome.config_validation as cv @@ -18,15 +21,17 @@ from esphome.const import ( CONF_TYPE_ID, CONF_UPDATE_INTERVAL, ) -from esphome.core import ID, Lambda +from esphome.core import CORE, ID, EsphomeError, Lambda from esphome.cpp_generator import ( + FlashStringLiteral, LambdaExpression, MockObj, MockObjClass, TemplateArgsType, + call_lambda, ) from esphome.schema_extractors import SCHEMA_EXTRACT, schema_extractor -from esphome.types import ConfigType +from esphome.types import ConfigType, SafeExpType from esphome.util import Registry @@ -57,6 +62,7 @@ def maybe_conf(conf, *validators): with cv.remove_prepend_path([conf]): return validator({conf: value}) + validate.inner_schema = validator return validate @@ -207,6 +213,189 @@ validate_action_list = cv.validate_registry("action", ACTION_REGISTRY) validate_condition = cv.validate_registry_entry("condition", CONDITION_REGISTRY) validate_condition_list = cv.validate_registry("condition", CONDITION_REGISTRY) +ApplyAction = cg.esphome_ns.class_("ApplyAction", Action) + + +def flash_string(config: ConfigType, value: str) -> str: + """Default renderer for ``std::string`` constants; copies the literal out of flash on ESP8266.""" + if CORE.is_esp8266: + return f"progmem_string({FlashStringLiteral(value)})" + return str(cg.safe_exp(value)) + + +@dataclass(frozen=True) +class ApplyCall: + """One statement from config keys, e.g. ``"set_range({}, {})"`` with ``((CONF_LOW, cg.float_), ...)``. + + Each arg is ``(conf_key, type_)`` or ``(conf_key, type_, const_fn)``. A ``conf_key`` may be a + path into nested sections. A plain ``str`` ``type_`` is raw C++ type text and may use + ``{parent}``. ``const_fn(config, value)`` renders a constant's argument text; a lambda bypasses + it. The statement is skipped when none of its keys is set, always emitted when it has no + keys, and a partial set is a config error. + """ + + target: str + args: tuple[tuple[Any, ...], ...] = () + + def __post_init__(self) -> None: + fields = [ + f for _, f, _, _ in string.Formatter().parse(self.target) if f is not None + ] + if any(fields): + raise ValueError( + f"apply target {self.target!r}: only bare {{}} placeholders" + ) + if len(fields) != len(self.args): + raise ValueError( + f"apply target {self.target!r} has {len(fields)} " + f"placeholder(s) for {len(self.args)} config key(s)" + ) + if any(len(arg) not in (2, 3) for arg in self.args): + raise ValueError( + f"apply target {self.target!r}: each arg is (conf_key, type_[, const_fn])" + ) + + +@dataclass(frozen=True) +class ApplyField: + """One config key forwarded as ``target(value)``, or as statement ``target`` when it has ``{}``. + + Double a literal brace in a template. ``conf_key`` may be a path into nested sections. + ``type_`` may be a C++ type string using ``{parent}`` when the type is only known per + instance. ``const_fn(config, value)`` renders a constant's argument text when ``cg.safe_exp`` + is not the right spelling (unit conversion belongs in the validator); a lambda bypasses it, + so the target must also take a plain ``type_``. An absent key emits nothing. + """ + + conf_key: str | tuple[str, ...] + target: str + type_: SafeExpType + const_fn: Callable[[ConfigType, Any], str] | None = None + + def call(self) -> ApplyCall: + target = self.target if "{}" in self.target else f"{self.target}({{}})" + return ApplyCall(target, ((self.conf_key, self.type_, self.const_fn),)) + + +def _config_lookup(config: ConfigType, key: str | tuple[str, ...]) -> Any: + if isinstance(key, str): + return config.get(key) + for part in key: + if (config := config.get(part)) is None: + return None + return config + + +def _dict_schema(schema: Any) -> Any: + """The dict-backed cv.Schema inside cv.All and maybe_* wrappers, or None; cv.Any is not inspected.""" + if isinstance(schema, dict): + return cv.Schema(schema) + if isinstance(getattr(schema, "schema", None), dict): + return schema + if isinstance(schema, cv.All): + inner = schema.validators + else: + inner = ( + getattr(schema, "inner_schema", None), + ) # maybe_conf / maybe_simple_value + for candidate in inner: + if candidate is not None and (found := _dict_schema(candidate)) is not None: + return found + return None + + +def _check_key_in_schema( + name: str, schema: Any, conf_key: str | tuple[str, ...] +) -> None: + """Reject a key path the schema does not have; a typo would otherwise be a silent no-op. + + Only dict-backed schemas, also inside cv.All and maybe_* wrappers, can be checked. + """ + for part in (conf_key,) if isinstance(conf_key, str) else conf_key: + if (schema := _dict_schema(schema)) is None: + return + markers = { + getattr(marker, "schema", marker): marker for marker in schema.schema + } + if part not in markers: + raise ValueError(f"{name}: config key {part!r} is not in the schema") + schema = schema.schema[markers[part]] + + +def register_apply_action( + name: str, + schema: cv.Schema, + *fields: ApplyField | ApplyCall, + call: str | None = None, +) -> None: + """Register an action that only forwards config values to its parent, with no C++ class. + + Generates one stateless function for ``ApplyAction``: parent and constants are baked + in, lambdas are called inline with the trigger args. With ``call`` every statement targets + the call object ``auto apply_call = parent->call()``, and ``apply_call.perform()`` is appended. + """ + statements_spec = [ + ( + c.target, + [(arg[0], arg[1], arg[2] if len(arg) == 3 else None) for arg in c.args], + ) + for c in (f if isinstance(f, ApplyCall) else f.call() for f in fields) + ] + for _, members in statements_spec: + for conf_key, _, _ in members: + _check_key_in_schema(name, schema, conf_key) + + async def builder( + config: ConfigType, + action_id: ID, + template_arg: cg.TemplateArguments, + args: TemplateArgsType, + ) -> MockObj: + # Global-scope qualified so a trigger arg named like the id cannot shadow it. + parent = f"::{await cg.get_variable(config[CONF_ID])}" + # Must match ApplyAction::ApplyFn exactly for the function pointer conversion. + lambda_args = [ + (cg.RawExpression(f"const std::remove_cvref_t<{cg.safe_exp(t)}> &"), arg) + for t, arg in args + ] + receiver = "apply_call." if call else f"{parent}->" + statements: list[str] = [] + for target, members in statements_spec: + values = [_config_lookup(config, key) for key, _, _ in members] + if members and all(value is None for value in values): + continue + if any(value is None for value in values): + keys = [key for key, _, _ in members] + raise EsphomeError(f"{name}: {target!r} needs all of {keys}") + exprs: list[str] = [] + for (_, type_, const_fn), value in zip(members, values, strict=True): + if isinstance(value, Lambda): + if isinstance(type_, str): + type_ = cg.RawExpression(type_.format(parent=parent)) + inner = await cg.process_lambda( + value, lambda_args, return_type=type_ + ) + exprs.append(str(call_lambda(inner))) + elif const_fn is not None: + exprs.append(const_fn(config, value)) + elif type_ is cg.std_string: + exprs.append(flash_string(config, value)) + else: + exprs.append(str(cg.safe_exp(value))) + statements.append(f"{receiver}{target.format(*exprs)};") + if call: + statements = [ + f"auto apply_call = {parent}->{call}();", + *statements, + "apply_call.perform();", + ] + apply_lambda = LambdaExpression( + ["\n".join(statements)], lambda_args, capture="", return_type=cg.void + ) + return cg.new_Pvariable(action_id, template_arg, apply_lambda) + + register_action(name, ApplyAction, schema, synchronous=True)(builder) + def validate_potentially_and_condition(value): if isinstance(value, list): diff --git a/esphome/components/cover/__init__.py b/esphome/components/cover/__init__.py index 011b2c2f04..05a9afad61 100644 --- a/esphome/components/cover/__init__.py +++ b/esphome/components/cover/__init__.py @@ -1,5 +1,3 @@ -from collections.abc import Callable -from dataclasses import dataclass import logging from esphome import automation @@ -38,14 +36,14 @@ from esphome.const import ( DEVICE_CLASS_SHUTTER, DEVICE_CLASS_WINDOW, ) -from esphome.core import CORE, ID, CoroPriority, Lambda, coroutine_with_priority +from esphome.core import CORE, ID, CoroPriority, coroutine_with_priority from esphome.core.entity_helpers import ( entity_duplicate_validator, queue_entity_register, setup_device_class, setup_entity, ) -from esphome.cpp_generator import LambdaExpression, MockObj, MockObjClass +from esphome.cpp_generator import MockObj, MockObjClass from esphome.types import ConfigType, SafeExpType, TemplateArgsType IS_PLATFORM_COMPONENT = True @@ -70,7 +68,6 @@ _LOGGER = logging.getLogger(__name__) cover_ns = cg.esphome_ns.namespace("cover") Cover = cover_ns.class_("Cover", cg.EntityBase) -CoverCall = cover_ns.class_("CoverCall") COVER_OPEN = cover_ns.COVER_OPEN COVER_CLOSED = cover_ns.COVER_CLOSED @@ -94,8 +91,6 @@ OpenAction = cover_ns.class_("OpenAction", automation.Action) CloseAction = cover_ns.class_("CloseAction", automation.Action) StopAction = cover_ns.class_("StopAction", automation.Action) ToggleAction = cover_ns.class_("ToggleAction", automation.Action) -ControlAction = cover_ns.class_("ControlAction", automation.Action) -CoverPublishAction = cover_ns.class_("CoverPublishAction", automation.Action) CoverIsOpenCondition = cover_ns.class_("CoverIsOpenCondition", Condition) CoverIsClosedCondition = cover_ns.class_("CoverIsClosedCondition", Condition) CoverOpenedTrigger = cover_ns.class_( @@ -319,107 +314,19 @@ COVER_CONTROL_ACTION_SCHEMA = cv.Schema( ) -@dataclass(frozen=True) -class ApplyField: - """One field in a folded-lambda action. - - `conf_key` is the YAML key looked up in `config`. When present, the - helper emits `statement_fn(target, value_expr)` into the lambda body. - `target` is whatever the statement function needs to identify the - field (typically a setter name like `"set_position"` or a struct - member like `"position"`). `type_` is the C++ return type for - `cg.process_lambda` when the value is a user lambda. - """ - - conf_key: str - target: str - type_: object - - -async def build_apply_lambda_action( - config: ConfigType, - action_id: ID, - template_arg: cg.TemplateArguments, - args: TemplateArgsType, - fields: tuple[ApplyField, ...], - prefix_args: list[tuple[object, str]], - statement_fn: Callable[[str, str], str], -) -> MockObj: - """Fold configured fields into a single stateless apply lambda action. - - Used by both `cover.control` and `cover.template.publish` (and shared - with the template/cover platform). Constants are emitted as flash - immediates; user lambdas are invoked inline so trigger args still flow. - Trigger arg types are normalized to `const std::remove_cvref_t &` - to match the ApplyFn signature for any T (value, ref, or const-ref). - """ - paren = await cg.get_variable(config[CONF_ID]) - # Normalize trigger args to `const std::remove_cvref_t &` so the - # apply lambda and any inner field lambdas (generated below via - # `process_lambda`) share one parameter spelling that's well-formed for - # any T. - normalized_args = [ - (cg.RawExpression(f"const std::remove_cvref_t<{cg.safe_exp(t)}> &"), n) - for t, n in args - ] - - fwd_args = ", ".join(name for _, name in args) - body_lines: list[str] = [] - for field in fields: - if (value := config.get(field.conf_key)) is None: - continue - if isinstance(value, Lambda): - inner = await cg.process_lambda( - value, normalized_args, return_type=field.type_ - ) - value_expr = f"({inner})({fwd_args})" - else: - value_expr = str(cg.safe_exp(value)) - body_lines.append(statement_fn(field.target, value_expr)) - - apply_args = [ - *prefix_args, - *normalized_args, - ] - apply_lambda = LambdaExpression( - ["\n".join(body_lines)], - apply_args, - capture="", - return_type=cg.void, - ) - return cg.new_Pvariable(action_id, template_arg, paren, apply_lambda) - - # CONF_STATE and CONF_POSITION are cv.Exclusive in the schema, so at most # one is present and both dispatch to set_position. -_COVER_CONTROL_FIELDS: tuple[ApplyField, ...] = ( - ApplyField(CONF_STOP, "set_stop", cg.bool_), - ApplyField(CONF_STATE, "set_position", cg.float_), - ApplyField(CONF_POSITION, "set_position", cg.float_), - ApplyField(CONF_TILT, "set_tilt", cg.float_), +automation.register_apply_action( + "cover.control", + COVER_CONTROL_ACTION_SCHEMA, + automation.ApplyField(CONF_STOP, "set_stop", cg.bool_), + automation.ApplyField(CONF_STATE, "set_position", cg.float_), + automation.ApplyField(CONF_POSITION, "set_position", cg.float_), + automation.ApplyField(CONF_TILT, "set_tilt", cg.float_), + call="make_call", ) -@automation.register_action( - "cover.control", ControlAction, COVER_CONTROL_ACTION_SCHEMA, synchronous=True -) -async def cover_control_to_code( - config: ConfigType, - action_id: ID, - template_arg: cg.TemplateArguments, - args: TemplateArgsType, -) -> MockObj: - return await build_apply_lambda_action( - config=config, - action_id=action_id, - template_arg=template_arg, - args=args, - fields=_COVER_CONTROL_FIELDS, - prefix_args=[(CoverCall.operator("ref"), "call")], - statement_fn=lambda setter, expr: f"call.{setter}({expr});", - ) - - COVER_CONDITION_SCHEMA = cv.maybe_simple_value( {cv.Required(CONF_ID): cv.use_id(Cover)}, key=CONF_ID ) diff --git a/esphome/components/cover/automation.h b/esphome/components/cover/automation.h index 0a5a447ab9..20f56e1753 100644 --- a/esphome/components/cover/automation.h +++ b/esphome/components/cover/automation.h @@ -46,50 +46,6 @@ template class ToggleAction final : public Action { Cover *cover_; }; -// All configured fields are baked into a single stateless lambda whose -// constants live in flash. Each action stores only one function pointer -// plus one parent pointer, regardless of how many fields the user set. -// Trigger args are forwarded to the apply function so user lambdas -// (e.g. `position: !lambda "return x;"`) keep working. -// -// Trigger args are normalized to `const std::remove_cvref_t &...` so -// the codegen can emit a matching parameter list for both the apply lambda -// and any inner field lambdas without producing invalid C++ source text -// (e.g. `const T & &` if Ts already carries a reference, or `const const -// T &` if Ts already carries a const). This keeps trigger args no-copy -// regardless of whether the trigger supplies `T`, `T &`, or `const T &`. - -template class ControlAction final : public Action { - public: - using ApplyFn = void (*)(CoverCall &, const std::remove_cvref_t &...); - ControlAction(Cover *cover, ApplyFn apply) : cover_(cover), apply_(apply) {} - - void play(const Ts &...x) override { - auto call = this->cover_->make_call(); - this->apply_(call, x...); - call.perform(); - } - - protected: - Cover *cover_; - ApplyFn apply_; -}; - -template class CoverPublishAction final : public Action { - public: - using ApplyFn = void (*)(Cover *, const std::remove_cvref_t &...); - CoverPublishAction(Cover *cover, ApplyFn apply) : cover_(cover), apply_(apply) {} - - void play(const Ts &...x) override { - this->apply_(this->cover_, x...); - this->cover_->publish_state(); - } - - protected: - Cover *cover_; - ApplyFn apply_; -}; - template class CoverPositionCondition final : public Condition { public: CoverPositionCondition(Cover *cover) : cover_(cover) {} diff --git a/esphome/components/template/cover/__init__.py b/esphome/components/template/cover/__init__.py index 0e6f96e9f5..39df5affcf 100644 --- a/esphome/components/template/cover/__init__.py +++ b/esphome/components/template/cover/__init__.py @@ -20,9 +20,6 @@ from esphome.const import ( CONF_TILT_ACTION, CONF_TILT_LAMBDA, ) -from esphome.core import ID -from esphome.cpp_generator import MockObj -from esphome.types import ConfigType, TemplateArgsType from .. import template_ns @@ -120,17 +117,8 @@ async def to_code(config): # CONF_STATE and CONF_POSITION are cv.Exclusive in the schema, so at most # one is present and both map to the position field. -_COVER_PUBLISH_FIELDS: tuple[cover.ApplyField, ...] = ( - cover.ApplyField(CONF_STATE, "position", cg.float_), - cover.ApplyField(CONF_POSITION, "position", cg.float_), - cover.ApplyField(CONF_TILT, "tilt", cg.float_), - cover.ApplyField(CONF_CURRENT_OPERATION, "current_operation", cover.CoverOperation), -) - - -@automation.register_action( +automation.register_apply_action( "cover.template.publish", - cover.CoverPublishAction, cv.Schema( { cv.Required(CONF_ID): cv.use_id(cover.Cover), @@ -142,22 +130,11 @@ _COVER_PUBLISH_FIELDS: tuple[cover.ApplyField, ...] = ( cv.Optional(CONF_TILT): cv.templatable(cv.zero_to_one_float), } ), - synchronous=True, + automation.ApplyField(CONF_STATE, "position = {}", cg.float_), + automation.ApplyField(CONF_POSITION, "position = {}", cg.float_), + automation.ApplyField(CONF_TILT, "tilt = {}", cg.float_), + automation.ApplyField( + CONF_CURRENT_OPERATION, "current_operation = {}", cover.CoverOperation + ), + automation.ApplyCall("publish_state()"), ) -async def cover_template_publish_to_code( - config: ConfigType, - action_id: ID, - template_arg: cg.TemplateArguments, - args: TemplateArgsType, -) -> MockObj: - # Mutates Cover fields directly (no CoverCall) since publish is a state - # push, not a control request. - return await cover.build_apply_lambda_action( - config=config, - action_id=action_id, - template_arg=template_arg, - args=args, - fields=_COVER_PUBLISH_FIELDS, - prefix_args=[(cover.Cover.operator("ptr"), "cover")], - statement_fn=lambda field, expr: f"cover->{field} = {expr};", - ) diff --git a/esphome/config_validation.py b/esphome/config_validation.py index 1623117a36..579da9a315 100644 --- a/esphome/config_validation.py +++ b/esphome/config_validation.py @@ -2354,6 +2354,7 @@ def maybe_simple_value(*validators, **kwargs): return validator(value) return validator({key: value}) + validate.inner_schema = validator return validate diff --git a/esphome/core/automation.h b/esphome/core/automation.h index 5f010521dc..b6058925b4 100644 --- a/esphome/core/automation.h +++ b/esphome/core/automation.h @@ -317,13 +317,8 @@ template class TemplatableValue { case STATIC_STRING: return std::string(this->static_str_); #ifdef USE_ESP8266 - case FLASH_STRING: { - // PROGMEM pointer — must use _P functions to access on ESP8266 - size_t len = strlen_P(this->static_str_); - std::string result(len, '\0'); - memcpy_P(result.data(), this->static_str_, len); - return result; - } + case FLASH_STRING: + return progmem_string(reinterpret_cast(this->static_str_)); #endif case NONE: default: diff --git a/esphome/core/base_automation.h b/esphome/core/base_automation.h index 999b38bd5c..b0c4b64995 100644 --- a/esphome/core/base_automation.h +++ b/esphome/core/base_automation.h @@ -11,6 +11,7 @@ #include #include +#include #include namespace esphome { @@ -252,6 +253,20 @@ template class StatelessLambdaAction : public Action { void (*f_)(Ts...); }; +/// Runs one codegen-generated function that has the parent and every field baked in, so the +/// action holds one pointer. Args pass by const reference so a std::string arg is never copied; +/// StatelessLambdaAction keeps by-value parameters because user `lambda:` code owns them. +template class ApplyAction final : public Action { + public: + using ApplyFn = void (*)(const std::remove_cvref_t &...); + explicit ApplyAction(ApplyFn apply) : apply_(apply) {} + + void play(const Ts &...x) override { this->apply_(x...); } + + protected: + ApplyFn apply_; +}; + /// Simple continuation action that calls play_next_ on a parent action. /// Used internally by IfAction, WhileAction, RepeatAction, etc. to chain actions. /// Memory: 4-8 bytes (parent pointer) vs 40 bytes (LambdaAction with std::function). diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index 433d2547b0..313daacbd0 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -20,6 +20,15 @@ namespace esphome { +#ifdef USE_ESP8266 +std::string progmem_string(ProgmemStr str) { + auto *src = reinterpret_cast(str); + std::string result(strlen_P(src), '\0'); + memcpy_P(result.data(), src, result.size()); + return result; +} +#endif + static const char *const TAG = "helpers"; __attribute__((noinline, cold)) void *callback_manager_grow(void *data, uint16_t size, uint16_t &capacity, diff --git a/esphome/core/progmem.h b/esphome/core/progmem.h index d349418d02..992b3c0e92 100644 --- a/esphome/core/progmem.h +++ b/esphome/core/progmem.h @@ -4,6 +4,7 @@ #include #include #include +#include #include "esphome/core/hal.h" // For PROGMEM definition @@ -54,6 +55,13 @@ using ProgmemStr = const char *; namespace esphome { +/// Copies a string stored with ESPHOME_F into a std::string. +#ifdef USE_ESP8266 +std::string progmem_string(ProgmemStr str); +#else +inline std::string progmem_string(ProgmemStr str) { return std::string(str); } +#endif + /// Helper for C++20 string literal template arguments template struct FixedString { char data[N]{}; diff --git a/esphome/cpp_generator.py b/esphome/cpp_generator.py index 173002438a..f394337604 100644 --- a/esphome/cpp_generator.py +++ b/esphome/cpp_generator.py @@ -1211,8 +1211,8 @@ def call_lambda(lamb: LambdaExpression) -> Expression: # Developer error if this is called with a lambda that doesn't have a return type assert lamb.return_type is not None, "Lambda must have a return type to be called" expr = lamb.content.strip() - if re.match(r"^return\b", expr) and expr.endswith(";"): - # Convert a lambda returning a simple expression to just that expression + # A lone `return ;` reduces to the expression; anything longer is called as is. + if re.match(r"^return\b", expr) and expr.endswith(";") and expr.count(";") == 1: expr = RawExpression(expr[6:-1].strip()) # Don't cast if the return type is a class if isinstance(lamb.return_type, MockObjClass): diff --git a/tests/components/template/common-base.yaml b/tests/components/template/common-base.yaml index 02aedaf167..53e1f7af6f 100644 --- a/tests/components/template/common-base.yaml +++ b/tests/components/template/common-base.yaml @@ -319,7 +319,7 @@ cover: logger.log: Cover is closed # Exercise cover.control / cover.template.publish action variants so they # get build coverage in CI (and so memory-impact analysis on PRs that - # touch ControlAction / CoverPublishAction sees real instances). + # touch these actions sees real instances). - platform: template name: "Template Cover Actions" id: template_cover_actions @@ -493,7 +493,7 @@ valve: stop_action: - logger.log: stop_action # Exercise valve.control with various field combinations so the - # ControlAction codegen paths get build coverage. + # valve.control codegen paths get build coverage. - valve.control: id: template_valve stop: true diff --git a/tests/integration/test_cover_control_action.py b/tests/integration/test_cover_control_action.py index 9c7395371b..ec9a94bf4f 100644 --- a/tests/integration/test_cover_control_action.py +++ b/tests/integration/test_cover_control_action.py @@ -1,4 +1,4 @@ -"""Integration test for cover ControlAction and CoverPublishAction. +"""Integration test for the cover.control and cover.template.publish actions. Tests that cover.control and cover.template.publish automation actions work correctly with the single stateless apply lambda/function pointer @@ -22,7 +22,7 @@ async def test_cover_control_action( run_compiled: RunCompiledFunction, api_client_connected: APIClientConnectedFactory, ) -> None: - """Test cover ControlAction/CoverPublishAction with constants and lambdas.""" + """Test cover.control and cover.template.publish with constants and lambdas.""" loop = asyncio.get_running_loop() async with run_compiled(yaml_config), api_client_connected() as client: cover_state_future: asyncio.Future[CoverState] | None = None diff --git a/tests/unit_tests/test_automation.py b/tests/unit_tests/test_automation.py index 07ea753360..c7c5ae998e 100644 --- a/tests/unit_tests/test_automation.py +++ b/tests/unit_tests/test_automation.py @@ -8,12 +8,17 @@ from unittest.mock import AsyncMock, MagicMock, call, patch import pytest from esphome.automation import ( + ApplyAction, + ApplyCall, + ApplyField, CallbackAutomation, TriggerForwarder, TriggerOnFalseForwarder, TriggerOnTrueForwarder, build_callback_automations, has_non_synchronous_actions, + maybe_simple_id, + register_apply_action, register_bare_action, register_bare_condition, register_parented_action, @@ -22,8 +27,9 @@ from esphome.automation import ( register_simple_condition, ) import esphome.codegen as cg +import esphome.config_validation as cv from esphome.const import CONF_ID -from esphome.core import ID +from esphome.core import CORE, ID, KEY_CORE, KEY_TARGET_PLATFORM, EsphomeError, Lambda from esphome.cpp_generator import MockObj, RawExpression from esphome.util import Registry, RegistryEntry @@ -594,3 +600,208 @@ def test_shared_builders_keep_synchronous_flag( assert actions["my.simple"].synchronous is synchronous assert actions["my.bare"].synchronous is synchronous assert actions["my.parented"].synchronous is synchronous + + +async def _run_apply_action( + registries: tuple[Registry, Registry], + fields: tuple[ApplyField | ApplyCall, ...], + config: dict[str, object], + args: list[tuple[object, str]] | None = None, + call: str | None = None, + platform: str = "esp32", +) -> RegistryEntry: + """Register an apply action and run its builder with the given config.""" + CORE.data[KEY_CORE] = {KEY_TARGET_PLATFORM: platform} + actions, _ = registries + register_apply_action("my.apply", None, *fields, call=call) + entry = actions["my.apply"] + args = args or [] + template_arg = cg.TemplateArguments(*(t for t, _ in args)) + await entry.fun({CONF_ID: PARENT_ID, **config}, ID("obj_1"), template_arg, args) + return entry + + +def _apply_lambda(mock_cg: MockCodegen) -> str: + return str(mock_cg.new_pvariable.call_args.args[2]) + + +@pytest.mark.asyncio +async def test_register_apply_action_entry( + registries: tuple[Registry, Registry], mock_cg: MockCodegen +) -> None: + entry = await _run_apply_action(registries, (), {}, args=[(cg.int32, "x")]) + assert entry.type_id is ApplyAction + assert entry.synchronous is True + mock_cg.get_variable.assert_awaited_once_with(PARENT_ID) + action_id, template_arg, _ = mock_cg.new_pvariable.call_args.args + assert action_id == ID("obj_1") + assert str(template_arg) == "" + + +@pytest.mark.asyncio +async def test_apply_constants( + registries: tuple[Registry, Registry], mock_cg: MockCodegen +) -> None: + """Constants are immediates, strings stay in flash, absent keys emit nothing, order is kept.""" + fields = ( + ApplyField("kp", "set_kp", cg.float_), + ApplyField("ki", "set_ki", cg.float_), + ApplyField("on", "set_on", cg.bool_), + ApplyField("song", "play", cg.std_string), + ApplyField("position", "position = {}", cg.float_), + ApplyCall("publish_state()"), + ) + config = {"kp": 0.0, "on": False, "song": "a:b", "position": 0.5} + await _run_apply_action(registries, fields, config) + text = _apply_lambda(mock_cg) + lines = [ + f"::{PARENT_OBJ}->set_kp(0.0f);", + f"::{PARENT_OBJ}->set_on(false);", + f'::{PARENT_OBJ}->play("a:b");', + f"::{PARENT_OBJ}->position = 0.5f;", + f"::{PARENT_OBJ}->publish_state();", + ] + positions = [text.index(line) for line in lines] + assert positions == sorted(positions) + assert "set_ki" not in text + + +@pytest.mark.asyncio +async def test_apply_lambdas( + registries: tuple[Registry, Registry], mock_cg: MockCodegen +) -> None: + """A single return reduces to a cast, anything longer is called inline with the trigger args.""" + fields = ( + ApplyField("kp", "set_kp", cg.float_), + ApplyField("ki", "set_ki", cg.float_), + ) + config = { + "kp": Lambda("return x * 2;"), + "ki": Lambda("if (x) return 1.0f;\nreturn 2.0f;"), + } + await _run_apply_action(registries, fields, config, args=[(cg.int32, "x")]) + text = _apply_lambda(mock_cg) + assert text.startswith("[](const std::remove_cvref_t & x) -> void {") + # The parent is global-scope qualified, so an arg named like the id cannot shadow it. + assert f"::{PARENT_OBJ}->set_kp(" in text + assert f"::{PARENT_OBJ}->set_kp(static_cast(x * 2));" in text + # Outer apply lambda and inner field lambda spell the trigger arg identically. + assert text.count("const std::remove_cvref_t & x") == 2 + assert ( + f"::{PARENT_OBJ}->set_ki([](const std::remove_cvref_t & x) -> float {{" + in text + ) + assert "}(x));" in text + + +@pytest.mark.asyncio +async def test_apply_call_keys( + registries: tuple[Registry, Registry], mock_cg: MockCodegen +) -> None: + """A multi-key call needs all keys, is skipped with none, and errors on a partial set.""" + fields = ( + ApplyCall("set_range({}, {})", (("low", cg.float_), ("high", cg.float_))), + ) + await _run_apply_action(registries, fields, {"low": 1.0, "high": 2.0}) + assert f"::{PARENT_OBJ}->set_range(1.0f, 2.0f);" in _apply_lambda(mock_cg) + + mock_cg.new_pvariable.reset_mock() + await _run_apply_action(registries, fields, {}) + assert "set_range" not in _apply_lambda(mock_cg) + + with pytest.raises(EsphomeError, match="needs all of"): + await _run_apply_action(registries, fields, {"low": 1.0}) + + +@pytest.mark.asyncio +async def test_apply_action_call_shape( + registries: tuple[Registry, Registry], mock_cg: MockCodegen +) -> None: + fields = (ApplyField("brightness", "set_brightness", cg.float_),) + await _run_apply_action(registries, fields, {"brightness": 0.5}, call="make_call") + text = _apply_lambda(mock_cg) + lines = [ + f"auto apply_call = ::{PARENT_OBJ}->make_call();", + "apply_call.set_brightness(0.5f);", + "apply_call.perform();", + ] + positions = [text.index(line) for line in lines] + assert positions == sorted(positions) + + +@pytest.mark.asyncio +async def test_apply_field_nested_key_const_fn_and_type_string( + registries: tuple[Registry, Registry], mock_cg: MockCodegen +) -> None: + fields = ( + ApplyField(("vertical", "direction"), "set_direction", cg.int_), + ApplyField( + "name", + "set_name", + cg.std_string, + const_fn=lambda config, value: f"{cg.safe_exp(value)}, {len(value)}", + ), + ApplyField("value", "value() = {}", "decltype({parent}->value())"), + ) + config = { + "vertical": {"direction": 3}, + "name": "abc", + "value": Lambda("return 42;"), + } + await _run_apply_action(registries, fields, config) + text = _apply_lambda(mock_cg) + assert f"::{PARENT_OBJ}->set_direction(3);" in text + assert f'::{PARENT_OBJ}->set_name("abc", 3);' in text + assert ( + f"::{PARENT_OBJ}->value() = static_castvalue())>(42);" + in text + ) + + mock_cg.new_pvariable.reset_mock() + await _run_apply_action(registries, fields[:1], {}) + assert "set_direction" not in _apply_lambda(mock_cg) + + +def test_apply_registration_checks(registries: tuple[Registry, Registry]) -> None: + with pytest.raises(ValueError, match="2 placeholder"): + ApplyCall("set_range({}, {})", (("low", cg.float_),)) + with pytest.raises(ValueError, match="only bare"): + ApplyCall("if ({}) {parent}->reset()", (("reset", cg.bool_),)) + ApplyCall("set_flags({{{}}})", (("flags", cg.int_),)) + with pytest.raises(ValueError, match="each arg is"): + ApplyCall("set_kp({})", (("kp", cg.float_, None, "extra"),)) + schema = cv.Schema({cv.Required(CONF_ID): cv.string, cv.Optional("kp"): cv.float_}) + register_apply_action("my.ok", schema, ApplyField("kp", "set_kp", cg.float_)) + with pytest.raises(ValueError, match="'kd' is not in the schema"): + register_apply_action("my.bad", schema, ApplyField("kd", "set_kd", cg.float_)) + either = cv.Any(schema, cv.Schema({cv.Optional("kd"): cv.float_})) + register_apply_action("my.any", either, ApplyField("kd", "set_kd", cg.float_)) + for wrapped in ( + maybe_simple_id(schema), + maybe_simple_id(schema.schema), + cv.All(schema), + cv.maybe_simple_value(schema, key="kp"), + ): + with pytest.raises(ValueError, match="'kd' is not in the schema"): + register_apply_action( + "my.bad", wrapped, ApplyField("kd", "set_kd", cg.float_) + ) + nested = cv.Schema({cv.Optional("v"): cv.Schema({cv.Optional("dir"): cv.int_})}) + register_apply_action( + "my.nested", nested, ApplyField(("v", "dir"), "set_dir", cg.int_) + ) + with pytest.raises(ValueError, match="'dri' is not in the schema"): + register_apply_action( + "my.bad2", nested, ApplyField(("v", "dri"), "set_dir", cg.int_) + ) + + +@pytest.mark.asyncio +async def test_apply_string_constant_stays_in_flash_on_esp8266( + registries: tuple[Registry, Registry], mock_cg: MockCodegen +) -> None: + fields = (ApplyField("song", "play", cg.std_string),) + await _run_apply_action(registries, fields, {"song": "a:b"}, platform="esp8266") + assert f'::{PARENT_OBJ}->play(progmem_string(ESPHOME_F("a:b")));' in _apply_lambda( + mock_cg + ) diff --git a/tests/unit_tests/test_cpp_generator.py b/tests/unit_tests/test_cpp_generator.py index 052513ce97..a618ad50e0 100644 --- a/tests/unit_tests/test_cpp_generator.py +++ b/tests/unit_tests/test_cpp_generator.py @@ -251,6 +251,15 @@ class TestCallLambda: assert isinstance(result, cg.StaticCastExpression) assert str(result) == "static_cast(foo + 1)" + def test_call_lambda__return_with_trailing_statements_is_called(self) -> None: + """Only a lone return statement reduces; a longer body is called as is.""" + lamb = cg.LambdaExpression(("return 1;\nfoo();",), (), "", ct.int_) + + result = cg.call_lambda(lamb) + + assert isinstance(result, cg.CallExpression) + assert str(result).endswith("}()") + def test_call_lambda__return_expression_with_class_return_type_no_cast(self): """A class return type is not cast, since static_cast doesn't apply to arbitrary class types."""