diff --git a/custom_components/adaptive_lighting/color_and_brightness.py b/custom_components/adaptive_lighting/color_and_brightness.py index c91278fc..5e1d8a75 100644 --- a/custom_components/adaptive_lighting/color_and_brightness.py +++ b/custom_components/adaptive_lighting/color_and_brightness.py @@ -528,5 +528,13 @@ def lerp(x: float, x1: float, x2: float, y1: float, y2: float) -> float: def clamp(value: float, minimum: float, maximum: float) -> float: - """Clamp value between minimum and maximum.""" - return max(minimum, min(value, maximum)) + """Clamp value between minimum and maximum. + + `minimum` is not assumed to be <= `maximum`: a user may intentionally + configure `min_brightness > max_brightness` (or the equivalent for color + temperature) for an inverted timescale (#1421). Sort the bounds first so + that case clamps against the real lower/upper bound instead of + collapsing to `minimum` for every input. + """ + low, high = (minimum, maximum) if minimum <= maximum else (maximum, minimum) + return max(low, min(value, high)) diff --git a/tests/test_color_and_brightness.py b/tests/test_color_and_brightness.py index 32425055..86cec8a8 100644 --- a/tests/test_color_and_brightness.py +++ b/tests/test_color_and_brightness.py @@ -7,6 +7,8 @@ from astral.location import Location from homeassistant.components.adaptive_lighting.color_and_brightness import ( SunEvent, SunEvents, + SunLightSettings, + clamp, ) # Create a mock astral location object (its `.observer` is passed to `SunEvents`) @@ -207,3 +209,91 @@ def test_closest_event(tzinfo_and_location): event_name, ts = sun_events.closest_event(sunrise) assert event_name == SunEvent.SUNRISE assert ts == location.sunrise(sunrise.date()).timestamp() + + +def _make_brightness_settings( + tzinfo, + location, + *, + min_brightness, + max_brightness, + brightness_mode, +): + """Build a SunLightSettings with only the fields brightness_pct() needs.""" + return SunLightSettings( + name="test", + astral_observer=location.observer, + adapt_until_sleep=False, + max_brightness=max_brightness, + max_color_temp=6500, + min_brightness=min_brightness, + min_color_temp=2000, + sleep_brightness=1, + sleep_rgb_or_color_temp="color_temp", + sleep_color_temp=2000, + sleep_rgb_color=(255, 56, 0), + sunrise_time=None, + min_sunrise_time=None, + max_sunrise_time=None, + sunset_time=None, + min_sunset_time=None, + max_sunset_time=None, + brightness_mode_time_dark=dt.timedelta(minutes=30), + brightness_mode_time_light=dt.timedelta(minutes=30), + brightness_mode=brightness_mode, + timezone=tzinfo, + ) + + +def test_clamp_handles_inverted_bounds(): + """A user can intentionally set min_brightness > max_brightness for an + inverted timescale (#1421, e.g. a porch light that should be brighter at + night than during the day). clamp() must still bound the value between + whichever of the two is actually smaller/larger, not silently collapse + to `minimum` for every input the way `max(minimum, min(value, maximum))` + does when minimum > maximum. + """ + assert clamp(50, 100, 15) == 50 + assert clamp(0, 100, 15) == 15 + assert clamp(200, 100, 15) == 100 + + +def test_clamp_normal_bounds_unaffected(): + """The ordinary min <= max case must keep behaving exactly as before.""" + assert clamp(50, 0, 100) == 50 + assert clamp(-10, 0, 100) == 0 + assert clamp(150, 0, 100) == 100 + + +@pytest.mark.parametrize("brightness_mode", ["linear", "tanh"]) +def test_brightness_pct_varies_with_inverted_brightness_bounds( + tzinfo_and_location, + brightness_mode, +): + """#1421: with min_brightness > max_brightness, linear/tanh modes got + stuck returning min_brightness for every sample, because the final + `clamp(brightness, self.min_brightness, self.max_brightness)` call + collapsed to `minimum` regardless of the computed value. Sampling a few + points around sunrise must show the brightness actually move instead of + being pinned to one value. + """ + tzinfo, location = tzinfo_and_location + settings = _make_brightness_settings( + tzinfo, + location, + min_brightness=100, + max_brightness=15, + brightness_mode=brightness_mode, + ) + + sunrise = location.sunrise(dt.datetime(2022, 6, 1).date()) + samples = [ + settings.brightness_pct( + sunrise + dt.timedelta(minutes=offset), + is_sleep=False, + ) + for offset in (-20, -10, 0, 10, 20) + ] + + assert len({round(value) for value in samples}) > 1, samples + assert all(15 <= value <= 100 for value in samples), samples diff --git a/tests/test_switch.py b/tests/test_switch.py index a48800f2..37e9df38 100644 --- a/tests/test_switch.py +++ b/tests/test_switch.py @@ -1727,7 +1727,7 @@ async def test_change_switch_settings_service(hass): # Test changing to illegal max brightness with pytest.raises( voluptuous.error.MultipleInvalid, - match="value must be at most 100 for dictionary", + match="value must be at most 100", ): await change_switch_settings(**{CONF_MAX_BRIGHTNESS: 5000})