diff --git a/esphome/automation.py b/esphome/automation.py index 2f970b915b..c8d0e4c574 100644 --- a/esphome/automation.py +++ b/esphome/automation.py @@ -345,14 +345,28 @@ async def _apply_parent(config: ConfigType, id_key: str = CONF_ID) -> str: def _apply_lambda_args(args: TemplateArgsType) -> TemplateArgsType: - # Must match ApplyAction::ApplyFn and ApplyCondition::CheckFn exactly for the function - # pointer conversion. + # The generated function's parameters; a std::string arg is never copied. return [ (cg.RawExpression(f"const std::remove_cvref_t<{cg.safe_exp(t)}> &"), arg) for t, arg in args ] +def _apply_function( + id_: ID, + return_type: SafeExpType, + template_arg: cg.TemplateArguments, + lambda_args: TemplateArgsType, + statements: list[str], +) -> MockObj: + """Emit the generated function and declare ``id_`` as the ``ApplyAction`` or + ``ApplyCondition`` templated on it, so ``play()`` calls it directly.""" + fn = cg.static_function( + f"esphome__{id_.id}__fn", return_type, lambda_args, statements + ) + return cg.new_Pvariable(id_, cg.TemplateArguments(fn, *template_arg)) + + async def _render_values( name: str, target: str, @@ -403,8 +417,8 @@ def register_apply_action( ) -> None: """Register an action that only forwards config values to its parent, with no C++ class. - Generates one stateless function for ``ApplyAction``: the parent (read from - ``id_key``) and constants are baked in, lambdas are called inline with the trigger args. + Generates one static function with the parent (read from ``id_key``) and constants baked + in, lambdas called inline with the trigger args, and an ``ApplyAction`` templated on it. A constant that is an id (``cv.use_id`` under ``cv.templatable``) is the object it names. With ``call`` every statement targets the call object ``auto apply_call = parent->call()``, and ``apply_call.perform()`` is appended. @@ -449,10 +463,9 @@ def register_apply_action( *statements, "apply_call.perform();", ] - apply_lambda = LambdaExpression( - ["\n".join(statements)], lambda_args, capture="", return_type=cg.void + return _apply_function( + action_id, cg.void, template_arg, lambda_args, statements ) - return cg.new_Pvariable(action_id, template_arg, apply_lambda) register_action(name, ApplyAction, schema, synchronous=True)(builder) @@ -466,7 +479,7 @@ def register_apply_condition( ``ApplyCall`` such as ``ApplyCall("state == {}", ((CONF_STATE, cg.bool_),))`` compares against config values, all of which must be present. Write ``== false`` to negate. String constants are plain literals, so compare a ``std::string`` or ``StringRef`` member. - Generates one stateless function for ``ApplyCondition``. + Generates one static predicate and an ``ApplyCondition`` templated on it. """ call = check if isinstance(check, ApplyCall) else ApplyCall(check) members = call.members @@ -492,13 +505,13 @@ def register_apply_condition( lambda_args, compare=True, ) - check_lambda = LambdaExpression( - [f"return {parent}->{call.target.format(*exprs)};"], + return _apply_function( + condition_id, + cg.bool_, + template_arg, lambda_args, - capture="", - return_type=cg.bool_, + [f"return {parent}->{call.target.format(*exprs)};"], ) - return cg.new_Pvariable(condition_id, template_arg, check_lambda) register_condition(name, ApplyCondition, schema)(builder) diff --git a/esphome/codegen.py b/esphome/codegen.py index 3dfec65f23..4de1d8d5c1 100644 --- a/esphome/codegen.py +++ b/esphome/codegen.py @@ -42,6 +42,7 @@ from esphome.cpp_generator import ( # noqa: F401 set_cpp_standard, statement, static_const_array, + static_function, templatable, variable, with_local_variable, diff --git a/esphome/core/base_automation.h b/esphome/core/base_automation.h index 703c7b40e3..8762cbe711 100644 --- a/esphome/core/base_automation.h +++ b/esphome/core/base_automation.h @@ -253,30 +253,19 @@ 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; +/// Runs one codegen-generated function that has the parent and every field baked in. The +/// function is a template argument, so play() calls it directly and the object is just the +/// Action base. 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 { +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_; + void play(const Ts &...x) override { Fn(x...); } }; /// Condition counterpart of ApplyAction: one codegen-generated predicate with the parent baked in. -template class ApplyCondition final : public Condition { +template class ApplyCondition final : public Condition { public: - using CheckFn = bool (*)(const std::remove_cvref_t &...); - explicit ApplyCondition(CheckFn check) : check_(check) {} - - bool check(const Ts &...x) override { return this->check_(x...); } - - protected: - CheckFn check_; + bool check(const Ts &...x) override { return Fn(x...); } }; /// Simple continuation action that calls play_next_ on a parent action. diff --git a/esphome/cpp_generator.py b/esphome/cpp_generator.py index a681583053..b0c3533e04 100644 --- a/esphome/cpp_generator.py +++ b/esphome/cpp_generator.py @@ -19,7 +19,7 @@ from esphome.core import ( TimePeriodNanoseconds, TimePeriodSeconds, ) -from esphome.helpers import cpp_string_escape, indent_all_but_first_and_last +from esphome.helpers import cpp_string_escape, indent, indent_all_but_first_and_last from esphome.types import Expression, SafeExpType, TemplateArgsType from esphome.util import OrderedDict from esphome.yaml_util import ESPHomeDataBase @@ -670,6 +670,28 @@ def new_Pvariable(id_: ID, *args: SafeExpType) -> "MockObj": return Pvariable(id_, rhs) +def static_function( + name: str, + return_type: SafeExpType, + parameters: TemplateArgsType, + body: list[str], +) -> RawExpression: + """Emit ``static (parameters) { body }`` at global scope and return an + expression naming it, for use as a template argument or a function pointer. + + Every id the body names must already be declared, which holds when the statements were + rendered through ``get_variable`` or ``process_lambda``. + """ + params = ParameterListExpression(*parameters) + add_global( + RawStatement( + f"static {safe_exp(return_type)} {name}({params}) {{\n" + f"{indent(chr(10).join(body))}\n}}" + ) + ) + return RawExpression(name) + + def add(expression: Expression | Statement, prepend: bool = False): """Add an expression to the codegen section. diff --git a/tests/components/http_request/http_request.yaml b/tests/components/http_request/http_request.yaml index 172e0c235a..eb8a6d33e7 100644 --- a/tests/components/http_request/http_request.yaml +++ b/tests/components/http_request/http_request.yaml @@ -53,8 +53,8 @@ esphome: # Regression test for esphome/esphome#16224: a light.turn_on action # nested inside on_response with capture_response: true puts # `std::string &` into the trigger's Ts..., which exposed a codegen - # bug where the apply lambda's parameter list did not match the - # ApplyFn signature. + # bug where the generated function's parameter list did not match + # the automation's arguments. - light.turn_on: id: test_regression_light brightness: 100% diff --git a/tests/unit_tests/test_automation.py b/tests/unit_tests/test_automation.py index 001756d72f..3a902a429f 100644 --- a/tests/unit_tests/test_automation.py +++ b/tests/unit_tests/test_automation.py @@ -509,6 +509,8 @@ class MockCodegen(NamedTuple): get_variable: AsyncMock new_pvariable: MagicMock register_parented: AsyncMock + add_global: MagicMock + calls: MagicMock # new_pvariable and add_global attached, to check their order @pytest.fixture @@ -520,10 +522,16 @@ def mock_cg() -> Generator[MockCodegen]: patch( "esphome.codegen.register_parented", new_callable=AsyncMock ) as register_parented, + patch("esphome.cpp_generator.add_global") as add_global, ): get_variable.return_value = PARENT_OBJ new_pvariable.return_value = NEW_OBJ - yield MockCodegen(get_variable, new_pvariable, register_parented) + calls = MagicMock() + calls.attach_mock(new_pvariable, "new_pvariable") + calls.attach_mock(add_global, "add_global") + yield MockCodegen( + get_variable, new_pvariable, register_parented, add_global, calls + ) @pytest.fixture @@ -649,8 +657,8 @@ async def _run_apply_condition( return await _run_entry(conditions["my.check"], config, args, platform, id_key) -def _apply_lambda(mock_cg: MockCodegen) -> str: - return str(mock_cg.new_pvariable.call_args.args[2]) +def _apply_definition(mock_cg: MockCodegen) -> str: + return str(mock_cg.add_global.call_args.args[0]) @pytest.mark.asyncio @@ -661,9 +669,14 @@ async def test_register_apply_action_entry( 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 + action_id, template_arg = mock_cg.new_pvariable.call_args.args assert action_id == ID("obj_1") - assert str(template_arg) == "" + assert str(template_arg) == "" + # The definition must precede the storage line that names the function. + assert [c[0] for c in mock_cg.calls.mock_calls] == ["add_global", "new_pvariable"] + assert _apply_definition(mock_cg).startswith( + "static void esphome__obj_1__fn(const std::remove_cvref_t & x) {" + ) @pytest.mark.asyncio @@ -692,7 +705,7 @@ async def test_apply_constants( ) config = {"kp": 0.0, "on": False, "song": "a:b", "position": 0.5} await _run_apply_action(registries, fields, config) - text = _apply_lambda(mock_cg) + text = _apply_definition(mock_cg) lines = [ f"::{PARENT_OBJ}->set_kp(0.0f);", f"::{PARENT_OBJ}->set_on(false);", @@ -715,7 +728,9 @@ async def test_apply_id_constant_is_the_named_object( fields = (ApplyField("target", "switch_to_output", cg.RawExpression("Speaker *")),) await _run_apply_action(registries, fields, {"target": ID("speaker_b")}) mock_cg.get_variable.assert_any_await(ID("speaker_b")) - assert f"::{PARENT_OBJ}->switch_to_output(::speaker_b);" in _apply_lambda(mock_cg) + assert f"::{PARENT_OBJ}->switch_to_output(::speaker_b);" in str( + _apply_definition(mock_cg) + ) @pytest.mark.asyncio @@ -728,7 +743,9 @@ async def test_apply_condition_id_constant_is_the_named_object( check = ApplyCall("is_output({})", (("target", cg.RawExpression("Speaker *")),)) await _run_apply_condition(registries, check, {"target": ID("speaker_b")}) mock_cg.get_variable.assert_any_await(ID("speaker_b")) - assert f"return ::{PARENT_OBJ}->is_output(::speaker_b);" in _apply_lambda(mock_cg) + assert f"return ::{PARENT_OBJ}->is_output(::speaker_b);" in str( + _apply_definition(mock_cg) + ) @pytest.mark.asyncio @@ -745,8 +762,10 @@ async def test_apply_lambdas( "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 {") + text = _apply_definition(mock_cg) + assert text.startswith( + "static void esphome__obj_1__fn(const std::remove_cvref_t & x) {" + ) # 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 @@ -768,11 +787,11 @@ async def test_apply_call_keys( 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) + assert f"::{PARENT_OBJ}->set_range(1.0f, 2.0f);" in _apply_definition(mock_cg) mock_cg.new_pvariable.reset_mock() await _run_apply_action(registries, fields, {}) - assert "set_range" not in _apply_lambda(mock_cg) + assert "set_range" not in _apply_definition(mock_cg) with pytest.raises(EsphomeError, match="needs all of"): await _run_apply_action(registries, fields, {"low": 1.0}) @@ -784,7 +803,7 @@ async def test_apply_action_call_shape( ) -> 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) + text = _apply_definition(mock_cg) lines = [ f"auto apply_call = ::{PARENT_OBJ}->make_call();", "apply_call.set_brightness(0.5f);", @@ -814,7 +833,7 @@ async def test_apply_field_nested_key_const_fn_and_type_string( "value": Lambda("return 42;"), } await _run_apply_action(registries, fields, config) - text = _apply_lambda(mock_cg) + text = _apply_definition(mock_cg) assert f"::{PARENT_OBJ}->set_direction(3);" in text assert f'::{PARENT_OBJ}->set_name("abc", 3);' in text assert ( @@ -824,7 +843,7 @@ async def test_apply_field_nested_key_const_fn_and_type_string( mock_cg.new_pvariable.reset_mock() await _run_apply_action(registries, fields[:1], {}) - assert "set_direction" not in _apply_lambda(mock_cg) + assert "set_direction" not in _apply_definition(mock_cg) def test_apply_registration_checks(registries: tuple[Registry, Registry]) -> None: @@ -883,8 +902,9 @@ async def test_apply_string_constant_stays_in_flash_on_esp8266( ) -> 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 + assert ( + f'::{PARENT_OBJ}->play(progmem_string(ESPHOME_F("a:b")));' + in _apply_definition(mock_cg) ) @@ -900,7 +920,7 @@ async def test_apply_literal_with_length_is_plain_on_every_platform( await _run_apply_action( registries, fields, {"option": "h\u00e9llo"}, platform=platform ) - text = _apply_lambda(mock_cg) + text = _apply_definition(mock_cg) assert f'::{PARENT_OBJ}->set_option("h\\303\\251llo", 6);' in text assert "progmem_string" not in text @@ -913,11 +933,13 @@ async def test_register_apply_condition_predicate( registries, "is_playing()", {}, args=[(cg.int32, "x")] ) assert entry.type_id is ApplyCondition - condition_id, template_arg, check = mock_cg.new_pvariable.call_args.args + condition_id, template_arg = mock_cg.new_pvariable.call_args.args assert condition_id == ID("obj_1") - assert str(template_arg) == "" - text = str(check) - assert text.startswith("[](const std::remove_cvref_t & x) -> bool {") + assert str(template_arg) == "" + text = _apply_definition(mock_cg) + assert text.startswith( + "static bool esphome__obj_1__fn(const std::remove_cvref_t & x) {" + ) assert f"return ::{PARENT_OBJ}->is_playing();" in text @@ -927,7 +949,7 @@ async def test_apply_condition_compares_config_value( ) -> None: check = ApplyCall("state == {}", (("state", cg.bool_),)) await _run_apply_condition(registries, check, {"state": True}) - assert f"return ::{PARENT_OBJ}->state == true;" in _apply_lambda(mock_cg) + assert f"return ::{PARENT_OBJ}->state == true;" in _apply_definition(mock_cg) with pytest.raises(EsphomeError, match="needs all of"): await _run_apply_condition(registries, check, {}) @@ -940,7 +962,7 @@ async def test_apply_condition_string_constant_is_a_plain_literal( ) -> None: check = ApplyCall("state == {}", (("state", cg.std_string),)) await _run_apply_condition(registries, check, {"state": "two"}, platform=platform) - assert f'return ::{PARENT_OBJ}->state == "two";' in _apply_lambda(mock_cg) + assert f'return ::{PARENT_OBJ}->state == "two";' in _apply_definition(mock_cg) @pytest.mark.asyncio @@ -964,6 +986,6 @@ async def test_apply_condition_string_lambda_paths( await _run_apply_condition( registries, check, {"state": Lambda(body)}, args=[(cg.std_string, "x")] ) - text = _apply_lambda(mock_cg) + text = _apply_definition(mock_cg) assert expected in text assert ("-> std::string {" in text) is called