[climate] Address review feedback (const ref args, utf-8 byte length, comments)

This commit is contained in:
J. Nick Koston
2026-04-29 06:54:23 -05:00
parent c9db304887
commit a7a45a556b
4 changed files with 28 additions and 24 deletions
+11 -7
View File
@@ -490,9 +490,9 @@ async def climate_control_to_code(config, action_id, template_arg, args):
# All configured fields are folded into a single stateless lambda whose
# constants live in flash; the action stores only a function pointer.
# `call_setter` is the ClimateCall method invoked in the lambda body —
# for custom_fan_mode/custom_preset this dispatches to the std::string
# overload of set_fan_mode/set_preset respectively.
# For custom_fan_mode/custom_preset the static-string path emits the
# (const char *, size_t) overload of set_fan_mode/set_preset to avoid
# constructing a std::string and calling runtime strlen.
FIELDS = (
(CONF_MODE, "set_mode", ClimateMode),
(CONF_TARGET_TEMPERATURE, "set_target_temperature", cg.float_),
@@ -516,16 +516,20 @@ async def climate_control_to_code(config, action_id, template_arg, args):
inner = await cg.process_lambda(value, args, return_type=type_)
body_lines.append(f"call.{setter}(({inner})({fwd_args}));")
elif type_ is cg.std_string:
# Static custom strings: emit a flash literal and pass the codegen-known
# length to skip the runtime strlen inside set_fan_mode/set_preset.
# Static custom strings: emit a flash literal and pass the
# UTF-8 byte length to skip the runtime strlen inside
# set_fan_mode/set_preset.
literal = cg.safe_exp(value)
body_lines.append(f"call.{setter}({literal}, {len(value)});")
body_lines.append(
f"call.{setter}({literal}, {len(value.encode('utf-8'))});"
)
else:
body_lines.append(f"call.{setter}({cg.safe_exp(value)});")
# Match ControlAction::ApplyFn signature: const Ts &... for trigger args.
apply_args = [
(ClimateCall.operator("ref"), "call"),
*args,
*((t.operator("const").operator("ref"), n) for t, n in args),
]
apply_lambda = LambdaExpression(
["\n".join(body_lines)],
+5 -5
View File
@@ -6,13 +6,13 @@
namespace esphome::climate {
// All configured fields are baked into a single stateless lambda whose
// constants live in flash. The action only stores a function pointer
// (4 bytes) plus the parent (4 bytes), regardless of how many fields the
// user set. Trigger args are forwarded to the apply function so user
// lambdas (e.g. `target_temperature: !lambda "return x;"`) keep working.
// constants live in flash. The action only stores 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. `target_temperature: !lambda "return x;"`) keep working.
template<typename... Ts> class ControlAction : public Action<Ts...> {
public:
using ApplyFn = void (*)(ClimateCall &, Ts...);
using ApplyFn = void (*)(ClimateCall &, const Ts &...);
ControlAction(Climate *climate, ApplyFn apply) : climate_(climate), apply_(apply) {}
void play(const Ts &...x) override {
@@ -44,7 +44,7 @@ climate:
max_temperature: 30 °C
button:
# Test 1: mode only (mask 1)
# mode only
- platform: template
id: btn_mode
name: "Set Mode Heat"
@@ -53,7 +53,7 @@ button:
id: test_climate
mode: HEAT
# Test 2: mode + low + high (mask 0b1101 = 13)
# mode + target_temperature_low + target_temperature_high
- platform: template
id: btn_mode_temps
name: "Set Mode Temps"
@@ -64,7 +64,7 @@ button:
target_temperature_low: 19.0 °C
target_temperature_high: 23.0 °C
# Test 3: just target_temp_low (mask 0b0100 = 4)
# target_temperature_low only
- platform: template
id: btn_low_only
name: "Set Low Only"
@@ -73,7 +73,7 @@ button:
id: test_climate
target_temperature_low: 17.5 °C
# Test 4: lambda for target_temperature_high (exercises lambda path)
# Lambda path: target_temperature_high computed at runtime
- platform: template
id: btn_lambda_high
name: "Lambda High"
@@ -82,7 +82,7 @@ button:
id: test_climate
target_temperature_high: !lambda "return id(test_target_temp);"
# Test 5: turn off via mode
# mode only — turn off via mode
- platform: template
id: btn_off
name: "Set Off"
@@ -1,8 +1,8 @@
"""Integration test for climate ControlAction.
Tests that climate.control automation actions work correctly with the
per-instance bitmask field storage. Exercises multiple field combinations
to cover different bitmask variants and the lambda path.
single stateless apply lambda/function pointer implementation. Exercises
multiple field combinations and the lambda path.
"""
from __future__ import annotations
@@ -61,24 +61,24 @@ async def test_climate_control_action(
client.button_command(btn.key)
return await wait_for_climate_state()
# Test 1: mode only (mask 1) — set HEAT
# mode only — set HEAT
state = await press_and_wait("Set Mode Heat")
assert state.mode == ClimateMode.HEAT
# Test 2: mode + low + high (mask 13) — HEAT_COOL with both temps
# mode + target_temperature_low + target_temperature_high
state = await press_and_wait("Set Mode Temps")
assert state.mode == ClimateMode.HEAT_COOL
assert state.target_temperature_low == pytest.approx(19.0, abs=0.5)
assert state.target_temperature_high == pytest.approx(23.0, abs=0.5)
# Test 3: low only (mask 4)
# target_temperature_low only
state = await press_and_wait("Set Low Only")
assert state.target_temperature_low == pytest.approx(17.5, abs=0.5)
# Test 4: lambda high — global is 21.5
# lambda path: target_temperature_high computed at runtime
state = await press_and_wait("Lambda High")
assert state.target_temperature_high == pytest.approx(21.5, abs=0.5)
# Test 5: turn off via mode (mask 1)
# mode only — turn off via mode
state = await press_and_wait("Set Off")
assert state.mode == ClimateMode.OFF