From cb47dfcf0e44e0ce060f5851f6d42000f37c3970 Mon Sep 17 00:00:00 2001 From: Clyde Stubbs <2366188+clydebarrow@users.noreply.github.com> Date: Thu, 1 Oct 2026 07:22:41 +1000 Subject: [PATCH] [light] Fix white flash on turn-on during an effect, start strobe on its first color (#19916) Co-authored-by: Claude Opus 5.5 --- esphome/components/light/base_light_effects.h | 5 + esphome/components/light/light_call.cpp | 33 +++- esphome/components/light/light_call.h | 1 + .../fixtures/light_repeat_effect.yaml | 49 ++++++ tests/integration/test_light_repeat_effect.py | 152 ++++++++++++++++++ 5 files changed, 232 insertions(+), 8 deletions(-) create mode 100644 tests/integration/fixtures/light_repeat_effect.yaml create mode 100644 tests/integration/test_light_repeat_effect.py diff --git a/esphome/components/light/base_light_effects.h b/esphome/components/light/base_light_effects.h index ba3fba6c12..e32dbde952 100644 --- a/esphome/components/light/base_light_effects.h +++ b/esphome/components/light/base_light_effects.h @@ -163,6 +163,11 @@ struct StrobeLightEffectColor { class StrobeLightEffect : public LightEffect { public: explicit StrobeLightEffect(const char *name) : LightEffect(name) {} + void start() override { + // Place the cycle at the end of the last color, so the first apply() switches straight to the first color + this->at_color_ = this->colors_.size() - 1; + this->last_switch_ = millis() - this->colors_.back().duration; + } void apply() override { const uint32_t now = millis(); if (now - this->last_switch_ < this->colors_[this->at_color_].duration) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index f540d2f31f..7e54fccebc 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -156,11 +156,13 @@ void LightCall::perform() { ESP_LOGV(TAG, " Effect: '%.*s'", (int) effect_s.size(), effect_s.c_str()); } - this->parent_->start_effect_(this->effect_); + if (this->effect_ != this->parent_->active_effect_index_) { + this->parent_->start_effect_(this->effect_); - // Also set light color values when starting an effect - // For example to turn off the light - this->parent_->set_immediately_(v, true); + // Also set light color values when starting an effect + // For example to turn off the light + this->parent_->set_immediately_(v, true); + } } else { // INSTANT CHANGE this->parent_->set_immediately_(v, publish); @@ -193,10 +195,9 @@ LightColorValues LightCall::validate_() { auto *name = this->parent_->get_name().c_str(); auto traits = this->parent_->get_traits(); -#ifdef USE_LIGHT_RESUME_EFFECT // Snapshot before the adjustments below add flags of their own + const bool sets_values = (this->flags_ & VALUE_FLAGS_MASK) != 0; const bool plain_turn_on = this->has_state() && this->state_ && (this->flags_ & ~STATE_ONLY_FLAGS_MASK) == 0; -#endif // USE_LIGHT_RESUME_EFFECT // Color mode check if (this->has_color_mode() && !traits.supports_color_mode(this->color_mode_)) { @@ -348,9 +349,25 @@ LightColorValues LightCall::validate_() { } #endif // USE_LIGHT_RESUME_EFFECT - // If effect is already active, remove effect start + // A plain turn-on of a lit light keeps a running effect as it is. + // Effects' own calls don't publish, so they are not caught here. + if (plain_turn_on && this->get_publish_() && this->parent_->remote_values.is_on() && + this->parent_->active_effect_index_ != 0) { + this->effect_ = this->parent_->active_effect_index_; + this->set_flag_(FLAG_HAS_EFFECT); + } + + // If effect is already active, remove effect start. When a lit light that stays on gets no new values or flash, + // keep the flag and drop any transition, which has nothing to fade to; perform() then leaves the running effect + // undisturbed. A call that turns the light off keeps no flag, so the turn-off block below stops the effect. + // has_brightness() catches the brightness added above to make the turn-on visible. if (this->has_effect_() && this->effect_ == this->parent_->active_effect_index_) { - this->clear_flag_(FLAG_HAS_EFFECT); + if (sets_values || this->has_brightness() || this->has_flash_() || this->effect_ == 0 || + !this->parent_->remote_values.is_on() || !v.is_on()) { + this->clear_flag_(FLAG_HAS_EFFECT); + } else { + this->clear_flag_(FLAG_HAS_TRANSITION); + } } // validate effect index diff --git a/esphome/components/light/light_call.h b/esphome/components/light/light_call.h index c9f6af7c91..a226690853 100644 --- a/esphome/components/light/light_call.h +++ b/esphome/components/light/light_call.h @@ -217,6 +217,7 @@ class LightCall { static constexpr uint16_t CLAMP_FLAGS_MASK = 0x00FFu; // bits 0-7 // Flags a plain turn-on may carry; any other flag means the caller asked for something specific static constexpr uint16_t STATE_ONLY_FLAGS_MASK = FLAG_HAS_STATE | FLAG_PUBLISH | FLAG_SAVE; + static constexpr uint16_t VALUE_FLAGS_MASK = CLAMP_FLAGS_MASK | FLAG_HAS_COLOR_TEMPERATURE | FLAG_HAS_COLOR_MODE; inline bool has_transition_() { return (this->flags_ & FLAG_HAS_TRANSITION) != 0; } inline bool has_flash_() { return (this->flags_ & FLAG_HAS_FLASH) != 0; } diff --git a/tests/integration/fixtures/light_repeat_effect.yaml b/tests/integration/fixtures/light_repeat_effect.yaml new file mode 100644 index 0000000000..85dc346f2c --- /dev/null +++ b/tests/integration/fixtures/light_repeat_effect.yaml @@ -0,0 +1,49 @@ +esphome: + name: light-repeat-effect +host: +api: # Port will be automatically injected +logger: + level: DEBUG + +output: + - platform: template + id: red_output + type: float + write_action: + - logger.log: + format: "RED_OUTPUT:%.4f" + args: [state] + - platform: template + id: green_output + type: float + write_action: + - logger.log: + format: "GREEN_OUTPUT:%.4f" + args: [state] + - platform: template + id: blue_output + type: float + write_action: + - logger.log: + format: "BLUE_OUTPUT:%.4f" + args: [state] + +light: + - platform: rgb + name: "Test RGB Light" + id: test_rgb_light + red: red_output + green: green_output + blue: blue_output + effects: + - strobe: + name: "Slow Strobe" + colors: + - red: 100% + green: 0% + blue: 0% + duration: 1s + - red: 0% + green: 0% + blue: 100% + duration: 1s diff --git a/tests/integration/test_light_repeat_effect.py b/tests/integration/test_light_repeat_effect.py new file mode 100644 index 0000000000..e8e4cf8e12 --- /dev/null +++ b/tests/integration/test_light_repeat_effect.py @@ -0,0 +1,152 @@ +"""Integration test for starting a strobe effect and for turn-ons sent while it runs. + +Starting the strobe used to leave the last published colour (white here) on the output until +the first color's duration had passed since boot, then skip to the second color. + +A turn-on naming the effect that is already running, or a plain turn-on of a lit light, used +to get the default transition (or the requested one). That faded the output towards white, which the strobe never +updates, until the next strobe step snapped it back. +""" + +from __future__ import annotations + +import asyncio +import re +from typing import Any + +from aioesphomeapi import EntityState, LightState +import pytest + +from .state_utils import InitialStateHelper +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_light_repeat_effect( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """The strobe starts on its first color, and turn-ons while it runs don't fade to white.""" + output_pattern = re.compile(r"(RED|GREEN|BLUE)_OUTPUT:([\d.]+)") + outputs: dict[str, list[float]] = {"RED": [], "GREEN": [], "BLUE": []} + green = outputs["GREEN"] + blue = outputs["BLUE"] + warnings: list[str] = [] + + def on_log_line(line: str) -> None: + if match := output_pattern.search(line): + outputs[match.group(1)].append(float(match.group(2))) + elif "effect cannot be used with" in line: + warnings.append(line) + + async with ( + run_compiled(yaml_config, line_callback=on_log_line), + api_client_connected() as client, + ): + entities, _ = await client.list_entities_services() + light = next(e for e in entities if e.object_id == "test_rgb_light") + + state_futures: dict[int, asyncio.Future[LightState]] = {} + + def on_state(state: EntityState) -> None: + if isinstance(state, LightState) and state.key in state_futures: + future = state_futures[state.key] + if not future.done(): + future.set_result(state) + + initial_state_helper = InitialStateHelper(entities) + client.subscribe_states(initial_state_helper.on_state_wrapper(on_state)) + await initial_state_helper.wait_for_initial_states() + + async def send_and_wait(timeout: float = 5.0, **kwargs: Any) -> LightState: + """Send a light command and wait for the matching state response.""" + state_futures[light.key] = asyncio.get_running_loop().create_future() + client.light_command(key=light.key, **kwargs) + return await asyncio.wait_for(state_futures[light.key], timeout=timeout) + + # Plain turn-on leaves the published colour at the white default + state = await send_and_wait(state=True, transition_length=0.0) + assert state.state is True + + green.clear() + blue.clear() + state = await send_and_wait(state=True, effect="Slow Strobe") + assert state.effect == "Slow Strobe" + await asyncio.sleep(0.3) + + # The first color is red; white shows green, and the second color is blue + assert green and blue, "No output observed after starting the strobe" + assert max(green) == pytest.approx(0.0, abs=0.01), ( + f"Starting the strobe showed white: green={green}" + ) + assert max(blue) == pytest.approx(0.0, abs=0.01), ( + f"Starting the strobe skipped its first color: blue={blue}" + ) + + for description, command in ( + ("Repeating the running effect", {"effect": "Slow Strobe"}), + ( + "Repeating the running effect with a transition", + {"effect": "Slow Strobe", "transition_length": 2.0}, + ), + ("A plain turn-on", {}), + ): + green.clear() + state = await send_and_wait(state=True, **command) + assert state.effect == "Slow Strobe", f"{description} changed the effect" + # Cover the rest of the current strobe step and the start of the next one + await asyncio.sleep(1.0) + + assert green, "No green output observed while the strobe was running" + assert max(green) == pytest.approx(0.0, abs=0.01), ( + f"{description} faded the output towards white: {green}" + ) + + assert not warnings, f"Unexpected warnings: {warnings}" + + # A flash requested with the running effect still flashes, here to white + green.clear() + state = await send_and_wait(state=True, effect="Slow Strobe", flash_length=0.5) + assert not warnings, f"Unexpected warnings: {warnings}" + await asyncio.sleep(0.2) + assert max(green) == pytest.approx(1.0, abs=0.01), ( + f"The flash was dropped: green={green}" + ) + await asyncio.sleep(0.5) + + # Brightness 0 with no state turns the light off but leaves the effect + # running; naming that effect must still turn the light on + state = await send_and_wait(brightness=0.0) + assert state.state is False + state = await send_and_wait(state=True, effect="Slow Strobe") + assert state.state is True, "Turn-on with the running effect was dropped" + assert state.brightness == pytest.approx(1.0) + assert state.effect == "Slow Strobe" + + # A lit light at brightness 0 is made visible by a turn-on naming the effect + state = await send_and_wait(state=True, brightness=0.0) + assert state.state is True + assert state.brightness == pytest.approx(0.0) + state = await send_and_wait(state=True, effect="Slow Strobe") + assert state.brightness == pytest.approx(1.0), ( + "Turn-on with the running effect did not make the light visible" + ) + assert state.effect == "Slow Strobe" + + # Turning the light off while naming the running effect must stop the effect, not just + # publish the light as off and leave the effect driving the outputs + warnings.clear() + state = await send_and_wait(state=False, effect="Slow Strobe") + assert state.state is False + # Let the default turn-off transition finish, then watch a full strobe cycle + await asyncio.sleep(1.5) + for values in outputs.values(): + values.clear() + await asyncio.sleep(2.2) + assert max((v for values in outputs.values() for v in values), default=0.0) == ( + pytest.approx(0.0, abs=0.01) + ), f"The effect kept driving the outputs after turn-off: {outputs}" + assert not warnings, f"Unexpected warnings: {warnings}" + + client.light_command(key=light.key, effect="None")