From 5e68282519caf8601bf2192923b1975d85580044 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 22 Mar 2026 10:14:52 -1000 Subject: [PATCH] [light] Fix constant_brightness broken by gamma LUT refactor (#15048) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- esphome/components/light/light_color_values.h | 10 + esphome/components/light/light_state.cpp | 47 ++++- .../fixtures/light_constant_brightness.yaml | 57 ++++++ .../test_light_constant_brightness.py | 188 ++++++++++++++++++ 4 files changed, 296 insertions(+), 6 deletions(-) create mode 100644 tests/integration/fixtures/light_constant_brightness.yaml create mode 100644 tests/integration/test_light_constant_brightness.py diff --git a/esphome/components/light/light_color_values.h b/esphome/components/light/light_color_values.h index 3a9ca8c8c2..a2c2dbca46 100644 --- a/esphome/components/light/light_color_values.h +++ b/esphome/components/light/light_color_values.h @@ -154,6 +154,16 @@ class LightColorValues { } /// Convert these light color values to an CWWW representation with the given parameters. + /// + /// Note on gamma and constant_brightness: This method operates on the raw/internal channel + /// values stored in this object. For cold_white_ and warm_white_ specifically, these + /// may already be gamma-uncorrected when derived from a color_temperature value. + /// For constant_brightness=false, additional gamma for the output can be applied after + /// this method since gamma commutes with simple multiplication. For constant_brightness=true, + /// the caller (LightState::current_values_as_cwww) must apply gamma to the individual + /// channel values BEFORE the balancing formula, because the nonlinear max/sum ratio does + /// not commute with gamma. See LightState::current_values_as_cwww() for the correct + /// implementation. void as_cwww(float *cold_white, float *warm_white, bool constant_brightness = false) const { if (this->color_mode_ & ColorCapability::COLD_WARM_WHITE) { const float cw_level = this->cold_white_; diff --git a/esphome/components/light/light_state.cpp b/esphome/components/light/light_state.cpp index 161092532a..1b736d84f6 100644 --- a/esphome/components/light/light_state.cpp +++ b/esphome/components/light/light_state.cpp @@ -223,12 +223,11 @@ void LightState::current_values_as_rgbw(float *red, float *green, float *blue, f } void LightState::current_values_as_rgbww(float *red, float *green, float *blue, float *cold_white, float *warm_white, bool constant_brightness) { - this->current_values.as_rgbww(red, green, blue, cold_white, warm_white, constant_brightness); + this->current_values.as_rgb(red, green, blue); *red = this->gamma_correct_lut(*red); *green = this->gamma_correct_lut(*green); *blue = this->gamma_correct_lut(*blue); - *cold_white = this->gamma_correct_lut(*cold_white); - *warm_white = this->gamma_correct_lut(*warm_white); + this->current_values_as_cwww(cold_white, warm_white, constant_brightness); } void LightState::current_values_as_rgbct(float *red, float *green, float *blue, float *color_temperature, float *white_brightness) { @@ -241,9 +240,45 @@ void LightState::current_values_as_rgbct(float *red, float *green, float *blue, *white_brightness = this->gamma_correct_lut(*white_brightness); } void LightState::current_values_as_cwww(float *cold_white, float *warm_white, bool constant_brightness) { - this->current_values.as_cwww(cold_white, warm_white, constant_brightness); - *cold_white = this->gamma_correct_lut(*cold_white); - *warm_white = this->gamma_correct_lut(*warm_white); + if (!constant_brightness) { + // Without constant_brightness, gamma commutes with simple multiplication: + // gamma(white_level * cw) = gamma(white_level) * gamma(cw) + // (since gamma(a*b) = (a*b)^g = a^g * b^g = gamma(a) * gamma(b)) + // so applying gamma after is mathematically equivalent and simpler. + this->current_values.as_cwww(cold_white, warm_white, false); + *cold_white = this->gamma_correct_lut(*cold_white); + *warm_white = this->gamma_correct_lut(*warm_white); + return; + } + + // For constant_brightness mode, gamma MUST be applied to the individual + // channel values BEFORE the balancing formula (max/sum ratio), not after. + // + // Why: The cold_white_ and warm_white_ values stored in LightColorValues + // are gamma-uncorrected (see transform_parameters_() which applies + // gamma_uncorrect to the linear CW/WW fractions derived from color + // temperature). Applying gamma_correct here recovers the original linear + // fractions, which the constant_brightness formula then uses to distribute + // power evenly. The max/sum formula ensures cold+warm PWM output sums to + // a constant, keeping total power (and perceived brightness) the same + // across all color temperatures. + // + // Applying gamma AFTER the formula would be incorrect because gamma is + // nonlinear: gamma(a/b) != gamma(a)/gamma(b), so the carefully balanced + // ratio would be distorted, causing a severe brightness dip at mid-range + // color temperatures. + const auto &v = this->current_values; + if (!(v.get_color_mode() & ColorCapability::COLD_WARM_WHITE)) { + *cold_white = *warm_white = 0; + return; + } + + const float cw_level = this->gamma_correct_lut(v.get_cold_white()); + const float ww_level = this->gamma_correct_lut(v.get_warm_white()); + const float white_level = this->gamma_correct_lut(v.get_state() * v.get_brightness()); + const float sum = cw_level > 0 || ww_level > 0 ? cw_level + ww_level : 1; // Don't divide by zero. + *cold_white = white_level * std::max(cw_level, ww_level) * cw_level / sum; + *warm_white = white_level * std::max(cw_level, ww_level) * ww_level / sum; } void LightState::current_values_as_ct(float *color_temperature, float *white_brightness) { auto traits = this->get_traits(); diff --git a/tests/integration/fixtures/light_constant_brightness.yaml b/tests/integration/fixtures/light_constant_brightness.yaml new file mode 100644 index 0000000000..4357a16d58 --- /dev/null +++ b/tests/integration/fixtures/light_constant_brightness.yaml @@ -0,0 +1,57 @@ +esphome: + name: light-cb-test +host: +api: # Port will be automatically injected +logger: + level: DEBUG + +output: + - platform: template + id: cb_cold_white_output + type: float + write_action: + - logger.log: + format: "CB_CW_OUTPUT:%.6f" + args: [state] + - platform: template + id: cb_warm_white_output + type: float + write_action: + - logger.log: + format: "CB_WW_OUTPUT:%.6f" + args: [state] + - platform: template + id: ncb_cold_white_output + type: float + write_action: + - logger.log: + format: "NCB_CW_OUTPUT:%.6f" + args: [state] + - platform: template + id: ncb_warm_white_output + type: float + write_action: + - logger.log: + format: "NCB_WW_OUTPUT:%.6f" + args: [state] + +light: + - platform: cwww + name: "Test CB Light" + id: test_cb_light + cold_white: cb_cold_white_output + warm_white: cb_warm_white_output + cold_white_color_temperature: 6536 K + warm_white_color_temperature: 2000 K + constant_brightness: true + gamma_correct: 2.8 + + - platform: cwww + name: "Test NCB Light" + id: test_ncb_light + cold_white: ncb_cold_white_output + warm_white: ncb_warm_white_output + cold_white_color_temperature: 6536 K + warm_white_color_temperature: 2000 K + constant_brightness: false + gamma_correct: 2.8 diff --git a/tests/integration/test_light_constant_brightness.py b/tests/integration/test_light_constant_brightness.py new file mode 100644 index 0000000000..622dc0e065 --- /dev/null +++ b/tests/integration/test_light_constant_brightness.py @@ -0,0 +1,188 @@ +"""Integration test for constant_brightness with gamma correction. + +Tests both constant_brightness: true and false cwww lights with gamma +correction in a single compilation to verify: +- constant_brightness: true maintains constant total CW+WW power output +- constant_brightness: false correctly varies total power across color temps + +This is a regression test for https://github.com/esphome/esphome/issues/15040 +where the gamma LUT refactor (#14123) broke constant_brightness by applying +gamma after the balancing formula instead of before it. +""" + +from __future__ import annotations + +import asyncio +import re +from typing import Any + +from aioesphomeapi import EntityState, LightInfo, LightState +import pytest + +from .state_utils import InitialStateHelper +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_light_constant_brightness( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test constant_brightness true and false behavior with gamma correction.""" + # Track output values for both lights from log lines + cb_cw_pattern = re.compile(r"(? None: + for pattern, key in [ + (cb_cw_pattern, "cb_cw"), + (cb_ww_pattern, "cb_ww"), + (ncb_cw_pattern, "ncb_cw"), + (ncb_ww_pattern, "ncb_ww"), + ]: + match = pattern.search(line) + if match: + latest[key] = float(match.group(1)) + + loop = asyncio.get_running_loop() + + async with ( + run_compiled(yaml_config, line_callback=on_log_line), + api_client_connected() as client, + ): + entities, _ = await client.list_entities_services() + lights = [e for e in entities if isinstance(e, LightInfo)] + cb_light = next(e for e in lights if e.object_id.endswith("cb_light")) + ncb_light = next(e for e in lights if e.object_id.endswith("ncb_light")) + + # Use InitialStateHelper to wait for initial state broadcast + initial_state_helper = InitialStateHelper(entities) + + # Track state changes per light key + state_futures: dict[int, asyncio.Future[EntityState]] = {} + + 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) + + client.subscribe_states(initial_state_helper.on_state_wrapper(on_state)) + + try: + await initial_state_helper.wait_for_initial_states() + except TimeoutError: + pytest.fail("Timeout waiting for initial states") + + async def send_and_wait( + light_key: int, timeout: float = 5.0, **kwargs: Any + ) -> LightState: + """Send a light command and wait for the state response.""" + state_futures[light_key] = loop.create_future() + client.light_command(key=light_key, **kwargs) + try: + return await asyncio.wait_for(state_futures[light_key], timeout=timeout) + except TimeoutError: + pytest.fail(f"Timeout waiting for light state after command: {kwargs}") + + # --- Test constant_brightness: true --- + + # Turn on CB light at full brightness + await send_and_wait( + cb_light.key, + state=True, + brightness=1.0, + color_temperature=153.0, + transition_length=0, + ) + + test_mireds = [ + 153.0, # Pure cold white + 200.0, # Mostly cold + 280.0, # Mixed + 326.5, # Midpoint + 400.0, # Mostly warm + 500.0, # Pure warm white + ] + + cb_totals: list[tuple[float, float, float]] = [] + for mireds in test_mireds: + await send_and_wait( + cb_light.key, color_temperature=mireds, transition_length=0 + ) + cb_totals.append((mireds, latest["cb_cw"], latest["cb_ww"])) + + # All totals should be approximately equal (constant brightness) + reference_total = next((cw + ww for _, cw, ww in cb_totals if cw + ww > 0), 0) + assert reference_total > 0, ( + f"Reference total power is zero, CB light outputs not working. " + f"Values: {cb_totals}" + ) + + for mireds, cw, ww in cb_totals: + total = cw + ww + assert total == pytest.approx(reference_total, rel=0.05), ( + f"constant_brightness: Total power at {mireds} mireds " + f"({total:.4f}) differs from reference ({reference_total:.4f}) " + f"by more than 5%. CW={cw:.4f}, WW={ww:.4f}. " + f"All values: {cb_totals}" + ) + + # --- Test constant_brightness: false --- + + # Turn on NCB light at full brightness + await send_and_wait( + ncb_light.key, + state=True, + brightness=1.0, + color_temperature=153.0, + transition_length=0, + ) + + ncb_totals: list[tuple[float, float, float]] = [] + for mireds in test_mireds: + await send_and_wait( + ncb_light.key, color_temperature=mireds, transition_length=0 + ) + ncb_totals.append((mireds, latest["ncb_cw"], latest["ncb_ww"])) + + extreme_cw = ncb_totals[0] # 153 mireds - pure cold + extreme_ww = ncb_totals[-1] # 500 mireds - pure warm + midpoint = ncb_totals[3] # 326.5 mireds - midpoint + + # At pure cold white, WW should be ~0 + assert extreme_cw[2] == pytest.approx(0.0, abs=0.01), ( + f"Pure cold white should have WW~0, got WW={extreme_cw[2]:.4f}" + ) + # At pure warm white, CW should be ~0 + assert extreme_ww[1] == pytest.approx(0.0, abs=0.01), ( + f"Pure warm white should have CW~0, got CW={extreme_ww[1]:.4f}" + ) + + # At midpoint, both channels should be non-zero + assert midpoint[1] > 0.05, f"Midpoint CW should be >0.05, got {midpoint[1]:.4f}" + assert midpoint[2] > 0.05, f"Midpoint WW should be >0.05, got {midpoint[2]:.4f}" + + # Total power at midpoint should be higher than at the extremes + midpoint_total = midpoint[1] + midpoint[2] + extreme_cw_total = extreme_cw[1] + extreme_cw[2] + extreme_ww_total = extreme_ww[1] + extreme_ww[2] + + assert midpoint_total > extreme_cw_total, ( + f"Midpoint total ({midpoint_total:.4f}) should be > pure CW total " + f"({extreme_cw_total:.4f}). All values: {ncb_totals}" + ) + assert midpoint_total > extreme_ww_total, ( + f"Midpoint total ({midpoint_total:.4f}) should be > pure WW total " + f"({extreme_ww_total:.4f}). All values: {ncb_totals}" + )