From 7aa9f0d796397171cb130b033ea9e63b455791b8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 3 May 2026 16:06:54 -0500 Subject: [PATCH 1/5] [light] Fix LightControlAction 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/light/automation.h | 5 ++++- esphome/components/light/automation.py | 5 +++-- tests/components/light/common.yaml | 15 +++++++++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/esphome/components/light/automation.h b/esphome/components/light/automation.h index a5c73997b0b..620ea3aca95 100644 --- a/esphome/components/light/automation.h +++ b/esphome/components/light/automation.h @@ -36,9 +36,12 @@ template class ToggleAction : public A // 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. `brightness: !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 LightControlAction : public Action { public: - using ApplyFn = void (*)(LightState *, LightCall &, const Ts &...); + using ApplyFn = void (*)(LightState *, LightCall &, Ts...); LightControlAction(LightState *parent, ApplyFn apply) : parent_(parent), apply_(apply) {} void play(const Ts &...x) override { diff --git a/esphome/components/light/automation.py b/esphome/components/light/automation.py index ca4018a9756..370489770df 100644 --- a/esphome/components/light/automation.py +++ b/esphome/components/light/automation.py @@ -230,11 +230,12 @@ async def light_control_to_code(config, action_id, template_arg, args): f"call.set_effect(static_cast({_resolve_effect_index(config)}));" ) - # Match LightControlAction::ApplyFn signature: const Ts &... for trigger args. + # Match LightControlAction::ApplyFn signature: trigger args forwarded + # by-value as Ts... apply_args = [ (LightState.operator("ptr"), "parent"), (LightCall.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/tests/components/light/common.yaml b/tests/components/light/common.yaml index e58f7baee4b..044a8144fad 100644 --- a/tests/components/light/common.yaml +++ b/tests/components/light/common.yaml @@ -127,6 +127,21 @@ esphome: blue: 0% transition_length: 1s +# Exercise light actions inside a trigger with non-empty Ts (number on_value +# passes float). +number: + - platform: template + id: test_number_brightness + optimistic: true + min_value: 0 + max_value: 100 + step: 1 + on_value: + then: + - light.turn_on: + id: test_monochromatic_light + brightness: !lambda "return x / 100.0;" + light: - platform: binary id: test_binary_light From dd0cff5f454534ba2572aea0ac481ea34aaae480 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 3 May 2026 16:51:27 -0500 Subject: [PATCH 2/5] [light] Reword apply-fn comments to describe codegen failure mode --- esphome/components/light/automation.h | 7 +++++-- esphome/components/light/automation.py | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/esphome/components/light/automation.h b/esphome/components/light/automation.h index 620ea3aca95..89328221335 100644 --- a/esphome/components/light/automation.h +++ b/esphome/components/light/automation.h @@ -37,8 +37,11 @@ template class ToggleAction : public A // Trigger args are forwarded to the apply function so user lambdas // (e.g. `brightness: !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 `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 &`). template class LightControlAction : public Action { public: using ApplyFn = void (*)(LightState *, LightCall &, Ts...); diff --git a/esphome/components/light/automation.py b/esphome/components/light/automation.py index 370489770df..fd75e976ec0 100644 --- a/esphome/components/light/automation.py +++ b/esphome/components/light/automation.py @@ -230,8 +230,11 @@ async def light_control_to_code(config, action_id, template_arg, args): f"call.set_effect(static_cast({_resolve_effect_index(config)}));" ) - # Match LightControlAction::ApplyFn signature: trigger args forwarded - # by-value as Ts... + # Forward trigger args as Ts... so each (type, name) pair passes through + # unchanged. Wrapping in `const &` here would emit invalid C++ source + # for trigger types that already carry a reference (e.g. `std::string &`) + # and fails on plain Python types (`float`/`bool`) which have no + # `.operator()` method. apply_args = [ (LightState.operator("ptr"), "parent"), (LightCall.operator("ref"), "call"), From 5483b27bbf76f2f5f17d92543e7abccb92a1da31 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 3 May 2026 16:54:21 -0500 Subject: [PATCH 3/5] [light] Use const remove_reference_t & to avoid copies of non-ref Ts --- esphome/components/light/automation.h | 11 +++++------ esphome/components/light/automation.py | 13 +++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/esphome/components/light/automation.h b/esphome/components/light/automation.h index 89328221335..c3451332649 100644 --- a/esphome/components/light/automation.h +++ b/esphome/components/light/automation.h @@ -37,14 +37,13 @@ template class ToggleAction : public A // Trigger args are forwarded to the apply function so user lambdas // (e.g. `brightness: !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 &`). +// 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 LightControlAction : public Action { public: - using ApplyFn = void (*)(LightState *, LightCall &, Ts...); + using ApplyFn = void (*)(LightState *, LightCall &, const std::remove_reference_t &...); LightControlAction(LightState *parent, ApplyFn apply) : parent_(parent), apply_(apply) {} void play(const Ts &...x) override { diff --git a/esphome/components/light/automation.py b/esphome/components/light/automation.py index fd75e976ec0..3a086c68c38 100644 --- a/esphome/components/light/automation.py +++ b/esphome/components/light/automation.py @@ -230,15 +230,16 @@ async def light_control_to_code(config, action_id, template_arg, args): f"call.set_effect(static_cast({_resolve_effect_index(config)}));" ) - # Forward trigger args as Ts... so each (type, name) pair passes through - # unchanged. Wrapping in `const &` here would emit invalid C++ source - # for trigger types that already carry a reference (e.g. `std::string &`) - # and fails on plain Python types (`float`/`bool`) which have no - # `.operator()` method. + # Match LightControlAction::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 = [ (LightState.operator("ptr"), "parent"), (LightCall.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)], From 22252ef5b52166dc8faa2ce26a07d2f6ee8a3d35 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 3 May 2026 17:26:39 -0500 Subject: [PATCH 4/5] [light] Forward trigger args as Ts... so inner-lambda calls type-check --- esphome/components/light/automation.h | 13 ++++++++----- esphome/components/light/automation.py | 11 ++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/esphome/components/light/automation.h b/esphome/components/light/automation.h index c3451332649..3bd0d5afbd7 100644 --- a/esphome/components/light/automation.h +++ b/esphome/components/light/automation.h @@ -37,13 +37,16 @@ template class ToggleAction : public A // Trigger args are forwarded to the apply function so user lambdas // (e.g. `brightness: !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 LightControlAction : public Action { public: - using ApplyFn = void (*)(LightState *, LightCall &, const std::remove_reference_t &...); + using ApplyFn = void (*)(LightState *, LightCall &, Ts...); LightControlAction(LightState *parent, ApplyFn apply) : parent_(parent), apply_(apply) {} void play(const Ts &...x) override { diff --git a/esphome/components/light/automation.py b/esphome/components/light/automation.py index 3a086c68c38..c796b97db06 100644 --- a/esphome/components/light/automation.py +++ b/esphome/components/light/automation.py @@ -230,16 +230,13 @@ async def light_control_to_code(config, action_id, template_arg, args): f"call.set_effect(static_cast({_resolve_effect_index(config)}));" ) - # Match LightControlAction::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 LightControlAction::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 = [ (LightState.operator("ptr"), "parent"), (LightCall.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)], From 10ea8c34921b27687da6e5f806c2f6b60ca65e17 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 3 May 2026 17:29:25 -0500 Subject: [PATCH 5/5] [light] Normalize trigger args to const remove_cvref_t & --- esphome/components/light/automation.h | 15 +++++++-------- esphome/components/light/automation.py | 18 ++++++++++++------ 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/esphome/components/light/automation.h b/esphome/components/light/automation.h index 3bd0d5afbd7..993d4a2ea66 100644 --- a/esphome/components/light/automation.h +++ b/esphome/components/light/automation.h @@ -37,16 +37,15 @@ template class ToggleAction : public A // Trigger args are forwarded to the apply function so user lambdas // (e.g. `brightness: !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 LightControlAction : public Action { public: - using ApplyFn = void (*)(LightState *, LightCall &, Ts...); + using ApplyFn = void (*)(LightState *, LightCall &, const std::remove_cvref_t &...); LightControlAction(LightState *parent, ApplyFn apply) : parent_(parent), apply_(apply) {} void play(const Ts &...x) override { diff --git a/esphome/components/light/automation.py b/esphome/components/light/automation.py index c796b97db06..cef774af38d 100644 --- a/esphome/components/light/automation.py +++ b/esphome/components/light/automation.py @@ -200,6 +200,15 @@ async def light_control_to_code(config, action_id, template_arg, args): (CONF_WARM_WHITE, "set_warm_white", 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 LightControlAction::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] = [] @@ -208,7 +217,7 @@ async def light_control_to_code(config, action_id, template_arg, args): continue value = config[conf_key] 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)});") @@ -216,7 +225,7 @@ async def light_control_to_code(config, action_id, template_arg, args): if CONF_EFFECT in config: if isinstance(config[CONF_EFFECT], Lambda): inner_lambda = await cg.process_lambda( - config[CONF_EFFECT], args, return_type=cg.std_string + config[CONF_EFFECT], normalized_args, return_type=cg.std_string ) body_lines.append( f"{{ auto __effect_s = ({inner_lambda})({fwd_args});\n" @@ -230,13 +239,10 @@ async def light_control_to_code(config, action_id, template_arg, args): f"call.set_effect(static_cast({_resolve_effect_index(config)}));" ) - # Match LightControlAction::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 = [ (LightState.operator("ptr"), "parent"), (LightCall.operator("ref"), "call"), - *args, + *normalized_args, ] apply_lambda = LambdaExpression( ["\n".join(body_lines)],