From fb6920a5b1421e7aa6f18df0e6e2d52f31ed01ec Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 3 May 2026 16:12:08 -0500 Subject: [PATCH 1/4] [valve] Fix ControlAction trigger args with reference types `const Ts &...` is ill-formed when Ts is already a reference (e.g. a trigger that passes `std::string &`). Forward Ts by-value so the generated lambda matches ApplyFn for any valid trigger arg type. --- esphome/components/valve/__init__.py | 5 +++-- esphome/components/valve/automation.h | 5 ++++- tests/components/template/common-base.yaml | 13 +++++++++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/esphome/components/valve/__init__.py b/esphome/components/valve/__init__.py index 7377aea1ed2..bc7e2ba48d9 100644 --- a/esphome/components/valve/__init__.py +++ b/esphome/components/valve/__init__.py @@ -251,10 +251,11 @@ async def valve_control_to_code(config, action_id, template_arg, args): else: body_lines.append(f"call.{setter}({cg.safe_exp(value)});") - # Match ControlAction::ApplyFn signature: const Ts &... for trigger args. + # Match ControlAction::ApplyFn signature: trigger args forwarded + # by-value as Ts... apply_args = [ (ValveCall.operator("ref"), "call"), - *((t.operator("const").operator("ref"), n) for t, n in args), + *args, ] apply_lambda = LambdaExpression( ["\n".join(body_lines)], diff --git a/esphome/components/valve/automation.h b/esphome/components/valve/automation.h index ae9ac0db762..81fcd122003 100644 --- a/esphome/components/valve/automation.h +++ b/esphome/components/valve/automation.h @@ -52,9 +52,12 @@ template class ToggleAction : public Action { // 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. +// +// Ts... must be forwarded by-value: `const T &` is ill-formed when T is +// already a reference type (some triggers pass `std::string &`). template class ControlAction : public Action { public: - using ApplyFn = void (*)(ValveCall &, const Ts &...); + using ApplyFn = void (*)(ValveCall &, Ts...); ControlAction(Valve *valve, ApplyFn apply) : valve_(valve), apply_(apply) {} void play(const Ts &...x) override { diff --git a/tests/components/template/common-base.yaml b/tests/components/template/common-base.yaml index 819eaa8bbfe..984ef129ad4 100644 --- a/tests/components/template/common-base.yaml +++ b/tests/components/template/common-base.yaml @@ -356,6 +356,19 @@ number: min_value: 0 max_value: 100 step: 1 + # Exercise valve.control inside a trigger with non-empty Ts (number on_value + # passes float). + - platform: template + id: template_valve_position_number + optimistic: true + min_value: 0 + max_value: 100 + step: 1 + on_value: + then: + - valve.control: + id: template_valve + position: !lambda "return x / 100.0f;" select: - platform: template From 431385ebc1ea01cb194e7ae4ac066212a46ac6ad Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 3 May 2026 16:56:14 -0500 Subject: [PATCH 2/4] [valve] Use const remove_reference_t & to avoid copies of non-ref Ts --- esphome/components/valve/__init__.py | 10 +++++++--- esphome/components/valve/automation.h | 8 +++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/esphome/components/valve/__init__.py b/esphome/components/valve/__init__.py index bc7e2ba48d9..e072f70d1b1 100644 --- a/esphome/components/valve/__init__.py +++ b/esphome/components/valve/__init__.py @@ -251,11 +251,15 @@ async def valve_control_to_code(config, action_id, template_arg, args): else: body_lines.append(f"call.{setter}({cg.safe_exp(value)});") - # Match ControlAction::ApplyFn signature: trigger args forwarded - # by-value as Ts... + # Match ControlAction::ApplyFn signature: `const std::remove_reference_t &` + # for each trigger arg so non-reference Ts stay no-copy (`const T &`) and + # reference Ts collapse correctly without producing `const T & &`. apply_args = [ (ValveCall.operator("ref"), "call"), - *args, + *( + (cg.RawExpression(f"const std::remove_reference_t<{cg.safe_exp(t)}> &"), n) + for t, n in args + ), ] apply_lambda = LambdaExpression( ["\n".join(body_lines)], diff --git a/esphome/components/valve/automation.h b/esphome/components/valve/automation.h index 81fcd122003..cc71999c359 100644 --- a/esphome/components/valve/automation.h +++ b/esphome/components/valve/automation.h @@ -53,11 +53,13 @@ template class ToggleAction : public Action { // Trigger args are forwarded to the apply function so user lambdas // (e.g. `position: !lambda "return x;"`) keep working. // -// Ts... must be forwarded by-value: `const T &` is ill-formed when T is -// already a reference type (some triggers pass `std::string &`). +// Trigger args are forwarded as `const std::remove_reference_t &...` +// (instead of `const Ts &...`) so codegen can emit the same form in the +// apply lambda's parameter list without producing `const T & &` for +// triggers whose Ts already carries a reference (e.g. `std::string &`). template class ControlAction : public Action { public: - using ApplyFn = void (*)(ValveCall &, Ts...); + using ApplyFn = void (*)(ValveCall &, const std::remove_reference_t &...); ControlAction(Valve *valve, ApplyFn apply) : valve_(valve), apply_(apply) {} void play(const Ts &...x) override { From b62d88a5d120264e77c83ca47bc16da89272f4cb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 3 May 2026 17:25:30 -0500 Subject: [PATCH 3/4] [valve] Forward trigger args as Ts... so inner-lambda calls type-check --- esphome/components/valve/__init__.py | 11 ++++------- esphome/components/valve/automation.h | 13 ++++++++----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/esphome/components/valve/__init__.py b/esphome/components/valve/__init__.py index e072f70d1b1..f39eb4dc8d7 100644 --- a/esphome/components/valve/__init__.py +++ b/esphome/components/valve/__init__.py @@ -251,15 +251,12 @@ async def valve_control_to_code(config, action_id, template_arg, args): else: body_lines.append(f"call.{setter}({cg.safe_exp(value)});") - # Match ControlAction::ApplyFn signature: `const std::remove_reference_t &` - # for each trigger arg so non-reference Ts stay no-copy (`const T &`) and - # reference Ts collapse correctly without producing `const T & &`. + # Match ControlAction::ApplyFn signature: forward trigger args as Ts... + # so the apply lambda's parameter types match both ApplyFn and the + # inner field lambdas (which are generated from `args` directly). apply_args = [ (ValveCall.operator("ref"), "call"), - *( - (cg.RawExpression(f"const std::remove_reference_t<{cg.safe_exp(t)}> &"), n) - for t, n in args - ), + *args, ] apply_lambda = LambdaExpression( ["\n".join(body_lines)], diff --git a/esphome/components/valve/automation.h b/esphome/components/valve/automation.h index cc71999c359..88c4ed760df 100644 --- a/esphome/components/valve/automation.h +++ b/esphome/components/valve/automation.h @@ -53,13 +53,16 @@ template class ToggleAction : public Action { // Trigger args are forwarded to the apply function so user lambdas // (e.g. `position: !lambda "return x;"`) keep working. // -// Trigger args are forwarded as `const std::remove_reference_t &...` -// (instead of `const Ts &...`) so codegen can emit the same form in the -// apply lambda's parameter list without producing `const T & &` for -// triggers whose Ts already carries a reference (e.g. `std::string &`). +// Trigger args are forwarded as `Ts...`. The previous `const Ts &...` +// form caused codegen to emit `const T &` for each arg in the apply +// lambda's parameter list, which is invalid C++ source text when T is +// already a reference (e.g. `const std::string & &` for triggers that +// pass `std::string &`). Forwarding `Ts...` lets the codegen reuse the +// trigger's `args` types unchanged for both the apply lambda and any +// inner field lambdas, so they always type-match. template class ControlAction : public Action { public: - using ApplyFn = void (*)(ValveCall &, const std::remove_reference_t &...); + using ApplyFn = void (*)(ValveCall &, Ts...); ControlAction(Valve *valve, ApplyFn apply) : valve_(valve), apply_(apply) {} void play(const Ts &...x) override { From 7c667bb9f485386a0dcbb52ada19a723266e4b60 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 3 May 2026 17:31:09 -0500 Subject: [PATCH 4/4] [valve] Normalize trigger args to const remove_cvref_t & --- esphome/components/valve/__init__.py | 16 +++++++++++----- esphome/components/valve/automation.h | 15 +++++++-------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/esphome/components/valve/__init__.py b/esphome/components/valve/__init__.py index f39eb4dc8d7..d82a9fdec28 100644 --- a/esphome/components/valve/__init__.py +++ b/esphome/components/valve/__init__.py @@ -240,23 +240,29 @@ async def valve_control_to_code(config, action_id, template_arg, args): (CONF_POSITION, "set_position", cg.float_), ) + # 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 (value, ref, or const-ref). Matches ControlAction::ApplyFn. + 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 conf_key, setter, type_ in FIELDS: if (value := config.get(conf_key)) is None: continue if isinstance(value, Lambda): - inner = await cg.process_lambda(value, args, return_type=type_) + inner = await cg.process_lambda(value, normalized_args, return_type=type_) body_lines.append(f"call.{setter}(({inner})({fwd_args}));") else: body_lines.append(f"call.{setter}({cg.safe_exp(value)});") - # Match ControlAction::ApplyFn signature: forward trigger args as Ts... - # so the apply lambda's parameter types match both ApplyFn and the - # inner field lambdas (which are generated from `args` directly). apply_args = [ (ValveCall.operator("ref"), "call"), - *args, + *normalized_args, ] apply_lambda = LambdaExpression( ["\n".join(body_lines)], diff --git a/esphome/components/valve/automation.h b/esphome/components/valve/automation.h index 88c4ed760df..27c0e329f08 100644 --- a/esphome/components/valve/automation.h +++ b/esphome/components/valve/automation.h @@ -53,16 +53,15 @@ template class ToggleAction : public Action { // Trigger args are forwarded to the apply function so user lambdas // (e.g. `position: !lambda "return x;"`) keep working. // -// Trigger args are forwarded as `Ts...`. The previous `const Ts &...` -// form caused codegen to emit `const T &` for each arg in the apply -// lambda's parameter list, which is invalid C++ source text when T is -// already a reference (e.g. `const std::string & &` for triggers that -// pass `std::string &`). Forwarding `Ts...` lets the codegen reuse the -// trigger's `args` types unchanged for both the apply lambda and any -// inner field lambdas, so they always type-match. +// 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 : public Action { public: - using ApplyFn = void (*)(ValveCall &, Ts...); + using ApplyFn = void (*)(ValveCall &, const std::remove_cvref_t &...); ControlAction(Valve *valve, ApplyFn apply) : valve_(valve), apply_(apply) {} void play(const Ts &...x) override {