From 0a568a3e1eaf6a10c7d954a9873888a800b00537 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 14 Apr 2026 09:45:42 -1000 Subject: [PATCH] [light] Avoid addressable transition stall at low gamma-corrected values (#15726) --- .../components/light/addressable_light.cpp | 63 +++++++++- esphome/components/light/addressable_light.h | 3 + .../addressable_light_transition.yaml | 29 +++++ .../mock_addressable_light/__init__.py | 1 + .../mock_addressable_light/light.py | 23 ++++ .../mock_addressable_light.h | 52 ++++++++ .../test_addressable_light_transition.py | 119 ++++++++++++++++++ 7 files changed, 284 insertions(+), 6 deletions(-) create mode 100644 tests/integration/fixtures/addressable_light_transition.yaml create mode 100644 tests/integration/fixtures/external_components/mock_addressable_light/__init__.py create mode 100644 tests/integration/fixtures/external_components/mock_addressable_light/light.py create mode 100644 tests/integration/fixtures/external_components/mock_addressable_light/mock_addressable_light.h create mode 100644 tests/integration/test_addressable_light_transition.py diff --git a/esphome/components/light/addressable_light.cpp b/esphome/components/light/addressable_light.cpp index 2f6ffc9a38..d2f5913f4b 100644 --- a/esphome/components/light/addressable_light.cpp +++ b/esphome/components/light/addressable_light.cpp @@ -58,6 +58,12 @@ void AddressableLightTransformer::start() { // our transition will handle brightness, disable brightness in correction. this->light_.correction_.set_local_brightness(255); this->target_color_ *= to_uint8_scale(end_values.get_brightness() * end_values.get_state()); + + // Uniformity scan is deferred to the first apply() call. start() can run before the underlying + // LED output's setup() has allocated its frame buffer (e.g. on_boot at priority > HARDWARE + // triggering a transition), and reading through ESPColorView would deref a null buffer. + this->uniform_start_scanned_ = false; + this->uniform_start_is_uniform_ = false; } inline constexpr uint8_t subtract_scaled_difference(uint8_t a, uint8_t b, int32_t scale) { @@ -97,12 +103,57 @@ optional AddressableLightTransformer::apply() { // non-linear when applying small deltas. if (smoothed_progress > this->last_transition_progress_ && this->last_transition_progress_ < 1.f) { - int32_t scale = int32_t(256.f * std::max((1.f - smoothed_progress) / (1.f - this->last_transition_progress_), 0.f)); - for (auto led : this->light_) { - led.set_rgbw(subtract_scaled_difference(this->target_color_.red, led.get_red(), scale), - subtract_scaled_difference(this->target_color_.green, led.get_green(), scale), - subtract_scaled_difference(this->target_color_.blue, led.get_blue(), scale), - subtract_scaled_difference(this->target_color_.white, led.get_white(), scale)); + // Lazy uniformity scan: deferred from start() so the LED output's setup() has run and the + // frame buffer is valid. When every LED already has the same color (the common case: plain + // turn_on/turn_off on a uniform strip), interpolate math-only against a single start color. + // Avoiding the per-step read-back through the 8-bit stored byte prevents gamma round-trip + // quantization from stalling the fade at low values (e.g. gamma 2.8 pre-gamma values <27 + // round to stored 0, freezing progress). + if (!this->uniform_start_scanned_) { + this->uniform_start_scanned_ = true; + if (this->light_.size() > 0) { + Color first = this->light_[0].get(); + bool uniform = true; + for (int32_t i = 1; i < this->light_.size(); i++) { + if (this->light_[i].get() != first) { + uniform = false; + break; + } + } + if (uniform) { + this->uniform_start_color_ = first; + this->uniform_start_is_uniform_ = true; + } + } + } + if (this->uniform_start_is_uniform_) { + // All LEDs started at the same color: compute the interpolated value once and write it to + // every LED. No read-back, so each LED's stored byte advances through every gamma threshold + // as smoothed_progress crosses it, instead of stalling at 0 for low pre-gamma values. + // + // Trade-off: any mid-transition writes to individual LEDs (e.g. from a user lambda) will be + // overwritten on the next apply() here. The fallback path below would have respected them + // via its read-back. Concurrent per-LED mutation during a transition isn't a pattern we + // support, so this is acceptable. + // lerp(start, target, progress) via existing helper: target - (target-start)*(1-progress). + const Color &start = this->uniform_start_color_; + int32_t remaining = int32_t(256.f * (1.f - smoothed_progress)); + uint8_t r = subtract_scaled_difference(this->target_color_.red, start.red, remaining); + uint8_t g = subtract_scaled_difference(this->target_color_.green, start.green, remaining); + uint8_t b = subtract_scaled_difference(this->target_color_.blue, start.blue, remaining); + uint8_t w = subtract_scaled_difference(this->target_color_.white, start.white, remaining); + for (auto led : this->light_) { + led.set_rgbw(r, g, b, w); + } + } else { + int32_t scale = + int32_t(256.f * std::max((1.f - smoothed_progress) / (1.f - this->last_transition_progress_), 0.f)); + for (auto led : this->light_) { + led.set_rgbw(subtract_scaled_difference(this->target_color_.red, led.get_red(), scale), + subtract_scaled_difference(this->target_color_.green, led.get_green(), scale), + subtract_scaled_difference(this->target_color_.blue, led.get_blue(), scale), + subtract_scaled_difference(this->target_color_.white, led.get_white(), scale)); + } } this->last_transition_progress_ = smoothed_progress; this->light_.schedule_show(); diff --git a/esphome/components/light/addressable_light.h b/esphome/components/light/addressable_light.h index 17cdb7d6f6..0202ad380a 100644 --- a/esphome/components/light/addressable_light.h +++ b/esphome/components/light/addressable_light.h @@ -115,6 +115,9 @@ class AddressableLightTransformer : public LightTransformer { AddressableLight &light_; float last_transition_progress_{0.0f}; Color target_color_{}; + Color uniform_start_color_{}; + bool uniform_start_scanned_{false}; + bool uniform_start_is_uniform_{false}; }; } // namespace esphome::light diff --git a/tests/integration/fixtures/addressable_light_transition.yaml b/tests/integration/fixtures/addressable_light_transition.yaml new file mode 100644 index 0000000000..7b847dd803 --- /dev/null +++ b/tests/integration/fixtures/addressable_light_transition.yaml @@ -0,0 +1,29 @@ +esphome: + name: addr-light-transition +host: +api: +logger: + level: DEBUG + +external_components: + - source: + type: local + path: EXTERNAL_COMPONENT_PATH + +light: + - platform: mock_addressable_light + output_id: strip_output + id: strip + name: "Test Strip" + num_leds: 4 + gamma_correct: 2.8 + default_transition_length: 0s + +sensor: + - platform: template + name: "led0_red_raw" + id: led0_red_raw + update_interval: 10ms + accuracy_decimals: 0 + lambda: |- + return (float) id(strip_output).get_raw_red(0); diff --git a/tests/integration/fixtures/external_components/mock_addressable_light/__init__.py b/tests/integration/fixtures/external_components/mock_addressable_light/__init__.py new file mode 100644 index 0000000000..e8cfff8e1f --- /dev/null +++ b/tests/integration/fixtures/external_components/mock_addressable_light/__init__.py @@ -0,0 +1 @@ +CODEOWNERS = ["@esphome/tests"] diff --git a/tests/integration/fixtures/external_components/mock_addressable_light/light.py b/tests/integration/fixtures/external_components/mock_addressable_light/light.py new file mode 100644 index 0000000000..293d2854f4 --- /dev/null +++ b/tests/integration/fixtures/external_components/mock_addressable_light/light.py @@ -0,0 +1,23 @@ +import esphome.codegen as cg +from esphome.components import light +import esphome.config_validation as cv +from esphome.const import CONF_NUM_LEDS, CONF_OUTPUT_ID +from esphome.types import ConfigType + +mock_addressable_light_ns = cg.esphome_ns.namespace("mock_addressable_light") +MockAddressableLight = mock_addressable_light_ns.class_( + "MockAddressableLight", light.AddressableLight +) + +CONFIG_SCHEMA = light.ADDRESSABLE_LIGHT_SCHEMA.extend( + { + cv.GenerateID(CONF_OUTPUT_ID): cv.declare_id(MockAddressableLight), + cv.Optional(CONF_NUM_LEDS, default=4): cv.positive_not_null_int, + } +) + + +async def to_code(config: ConfigType) -> None: + var = cg.new_Pvariable(config[CONF_OUTPUT_ID], config[CONF_NUM_LEDS]) + await light.register_light(var, config) + await cg.register_component(var, config) diff --git a/tests/integration/fixtures/external_components/mock_addressable_light/mock_addressable_light.h b/tests/integration/fixtures/external_components/mock_addressable_light/mock_addressable_light.h new file mode 100644 index 0000000000..c6b0d10601 --- /dev/null +++ b/tests/integration/fixtures/external_components/mock_addressable_light/mock_addressable_light.h @@ -0,0 +1,52 @@ +#pragma once + +#include +#include +#include + +#include "esphome/components/light/addressable_light.h" +#include "esphome/core/component.h" + +namespace esphome::mock_addressable_light { + +// In-memory addressable light for host-mode integration tests. Exposes the raw +// per-LED byte buffer (post-gamma-correction, as the hardware would see it) +// so tests can observe transition behavior without real hardware. +class MockAddressableLight : public light::AddressableLight { + public: + explicit MockAddressableLight(uint16_t num_leds) + : num_leds_(num_leds), buf_(new uint8_t[num_leds * 4]()), effect_data_(new uint8_t[num_leds]()) {} + + void setup() override {} + void write_state(light::LightState *state) override {} + int32_t size() const override { return this->num_leds_; } + void clear_effect_data() override { + for (uint16_t i = 0; i < this->num_leds_; i++) + this->effect_data_[i] = 0; + } + light::LightTraits get_traits() override { + auto traits = light::LightTraits(); + traits.set_supported_color_modes({light::ColorMode::RGB}); + return traits; + } + + // Accessors for tests: return the raw stored byte (post gamma correction), + // which is what actual LED hardware would receive. + uint8_t get_raw_red(uint16_t index) const { return this->buf_[index * 4 + 0]; } + uint8_t get_raw_green(uint16_t index) const { return this->buf_[index * 4 + 1]; } + uint8_t get_raw_blue(uint16_t index) const { return this->buf_[index * 4 + 2]; } + uint8_t get_raw_white(uint16_t index) const { return this->buf_[index * 4 + 3]; } + + protected: + light::ESPColorView get_view_internal(int32_t index) const override { + size_t pos = index * 4; + return {this->buf_.get() + pos + 0, this->buf_.get() + pos + 1, this->buf_.get() + pos + 2, + this->buf_.get() + pos + 3, this->effect_data_.get() + index, &this->correction_}; + } + + uint16_t num_leds_; + std::unique_ptr buf_; + std::unique_ptr effect_data_; +}; + +} // namespace esphome::mock_addressable_light diff --git a/tests/integration/test_addressable_light_transition.py b/tests/integration/test_addressable_light_transition.py new file mode 100644 index 0000000000..37fecde595 --- /dev/null +++ b/tests/integration/test_addressable_light_transition.py @@ -0,0 +1,119 @@ +"""Integration test for addressable light transitions with gamma correction. + +Regression test for a bug where a long turn-on transition on an addressable +light with gamma correction (e.g. gamma_correct: 2.8) produced no visible +output for ~90% of the transition duration, then jumped to the target in the +final ~10%. Root cause: the transition algorithm read each LED's current value +back through the 8-bit stored byte every step; at gamma 2.8 any pre-gamma value +below ~27 rounds to stored byte 0, so the stored byte stalled at 0 until +progress was high enough for a single step to produce a large-enough pre-gamma +value to clear the gamma threshold. + +The fix interpolates against a cached start color when all LEDs started at the +same value (the common case for plain turn_on/turn_off), avoiding the round-trip. + +This test uses a host-only mock addressable light that exposes the raw stored +byte of each LED, so we can observe the transition directly. +""" + +from __future__ import annotations + +import asyncio + +from aioesphomeapi import LightInfo, SensorInfo, SensorState +import pytest + +from .state_utils import InitialStateHelper, require_entity +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_addressable_light_transition( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """With gamma 2.8, the stored raw byte must rise visibly well before the end.""" + async with run_compiled(yaml_config), api_client_connected() as client: + entities, _ = await client.list_entities_services() + light = require_entity(entities, "test_strip", LightInfo) + sensor = require_entity(entities, "led0_red_raw", SensorInfo) + + # Track the raw-byte sensor. It polls every 10ms in the fixture, and + # ESPHome sensors publish on every change, so we collect a time series. + # Samples are stored as absolute (loop_time, value); we rebase to the + # command-issue time after the run so pre-command samples are strictly + # negative and reliably excluded. + loop = asyncio.get_running_loop() + samples: list[tuple[float, float]] = [] + + def on_state(state: object) -> None: + if not isinstance(state, SensorState) or state.key != sensor.key: + return + samples.append((loop.time(), state.state)) + + # InitialStateHelper swallows the first state ESPHome sends per entity + # on subscribe, so on_state only sees real post-subscribe updates. + initial_state_helper = InitialStateHelper(entities) + client.subscribe_states(initial_state_helper.on_state_wrapper(on_state)) + await initial_state_helper.wait_for_initial_states() + + # Start transition: off -> full white over 1 second. This is the + # scenario from the bug report, compressed in time. + transition_s = 1.0 + command_time = loop.time() + client.light_command( + key=light.key, + state=True, + rgb=(1.0, 1.0, 1.0), + brightness=1.0, + transition_length=transition_s, + ) + + # Let the full transition run, plus margin for the final sample. + await asyncio.sleep(transition_s + 0.2) + + # Rebase to command-issue time. Pre-command samples have t < 0 and are + # excluded; everything else is in seconds since the command was issued. + post_command = [ + (t - command_time, v) for (t, v) in samples if t >= command_time + ] + assert post_command, "no sensor samples received after command was issued" + + # Assertion 1: the transition is not stalled. With the bug, the raw + # byte stays at 0 until ~90% of the transition duration. With the fix, + # it becomes nonzero in the first ~30% (for gamma 2.8, pre-gamma 76 + # clears the gamma threshold at progress ~0.30). Require the first + # nonzero sample to land well before 50% of the transition duration, + # measured from the command-issue time. The 50% bound (rather than + # 70%) leaves headroom for assertion 2's mid-window check. + first_nonzero = next(((t, v) for (t, v) in post_command if v > 0), None) + assert first_nonzero is not None, ( + "raw byte never rose above 0 during the transition — the fade stalled" + ) + assert first_nonzero[0] < transition_s * 0.5, ( + f"raw byte only rose above 0 at t={first_nonzero[0]:.3f}s " + f"(>{transition_s * 0.5:.3f}s after command) — transition is stalling" + ) + + # Assertion 2: by mid-late transition, the raw byte should have reached + # a substantial fraction of its final value. Bound the window to + # [50%, 90%] of the transition so the post-transition settled value + # (which always reaches 255) can't satisfy this assertion — that would + # let "stays at 0 then jumps at 99%" regressions slip through. + mid_window = [ + v + for (t, v) in post_command + if transition_s * 0.5 <= t <= transition_s * 0.9 + ] + assert mid_window, "no samples captured in mid-transition window" + assert max(mid_window) >= 100, ( + f"raw byte peaked at only {max(mid_window)} between 50%–90% of " + "transition (expected >= 100 for white target at gamma 2.8)" + ) + + # Assertion 3: final value reaches target. Gamma 2.8 of 255 is 255. + final_samples = [v for (_, v) in post_command[-5:]] + assert max(final_samples) >= 250, ( + f"final raw byte was {max(final_samples)}, expected >= 250" + )