mirror of
https://github.com/esphome/esphome.git
synced 2026-05-30 15:28:34 +08:00
[light] Avoid addressable transition stall at low gamma-corrected values (#15726)
This commit is contained in:
committed by
Jesse Hills
parent
ef44491c69
commit
0a568a3e1e
@@ -58,6 +58,12 @@ void AddressableLightTransformer::start() {
|
|||||||
// our transition will handle brightness, disable brightness in correction.
|
// our transition will handle brightness, disable brightness in correction.
|
||||||
this->light_.correction_.set_local_brightness(255);
|
this->light_.correction_.set_local_brightness(255);
|
||||||
this->target_color_ *= to_uint8_scale(end_values.get_brightness() * end_values.get_state());
|
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) {
|
inline constexpr uint8_t subtract_scaled_difference(uint8_t a, uint8_t b, int32_t scale) {
|
||||||
@@ -97,12 +103,57 @@ optional<LightColorValues> AddressableLightTransformer::apply() {
|
|||||||
// non-linear when applying small deltas.
|
// non-linear when applying small deltas.
|
||||||
|
|
||||||
if (smoothed_progress > this->last_transition_progress_ && this->last_transition_progress_ < 1.f) {
|
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));
|
// Lazy uniformity scan: deferred from start() so the LED output's setup() has run and the
|
||||||
for (auto led : this->light_) {
|
// frame buffer is valid. When every LED already has the same color (the common case: plain
|
||||||
led.set_rgbw(subtract_scaled_difference(this->target_color_.red, led.get_red(), scale),
|
// turn_on/turn_off on a uniform strip), interpolate math-only against a single start color.
|
||||||
subtract_scaled_difference(this->target_color_.green, led.get_green(), scale),
|
// Avoiding the per-step read-back through the 8-bit stored byte prevents gamma round-trip
|
||||||
subtract_scaled_difference(this->target_color_.blue, led.get_blue(), scale),
|
// quantization from stalling the fade at low values (e.g. gamma 2.8 pre-gamma values <27
|
||||||
subtract_scaled_difference(this->target_color_.white, led.get_white(), scale));
|
// 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->last_transition_progress_ = smoothed_progress;
|
||||||
this->light_.schedule_show();
|
this->light_.schedule_show();
|
||||||
|
|||||||
@@ -115,6 +115,9 @@ class AddressableLightTransformer : public LightTransformer {
|
|||||||
AddressableLight &light_;
|
AddressableLight &light_;
|
||||||
float last_transition_progress_{0.0f};
|
float last_transition_progress_{0.0f};
|
||||||
Color target_color_{};
|
Color target_color_{};
|
||||||
|
Color uniform_start_color_{};
|
||||||
|
bool uniform_start_scanned_{false};
|
||||||
|
bool uniform_start_is_uniform_{false};
|
||||||
};
|
};
|
||||||
|
|
||||||
} // namespace esphome::light
|
} // namespace esphome::light
|
||||||
|
|||||||
@@ -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);
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
CODEOWNERS = ["@esphome/tests"]
|
||||||
@@ -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)
|
||||||
+52
@@ -0,0 +1,52 @@
|
|||||||
|
#pragma once
|
||||||
|
|
||||||
|
#include <cstddef>
|
||||||
|
#include <cstdint>
|
||||||
|
#include <memory>
|
||||||
|
|
||||||
|
#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<uint8_t[]> buf_;
|
||||||
|
std::unique_ptr<uint8_t[]> effect_data_;
|
||||||
|
};
|
||||||
|
|
||||||
|
} // namespace esphome::mock_addressable_light
|
||||||
@@ -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"
|
||||||
|
)
|
||||||
Reference in New Issue
Block a user