Skip to content

Commit 6cebe14

Browse files
authored
fix: merge last_service_data across split calls to fix detect_non_ha_changes with separate_turn_on_commands (#1426)
* test: regression test — AL must not override manual brightness with separate_turn_on_commands End-to-end scenario: user adjusts brightness via a directly-bound Zigbee switch (e.g. IKEA RODRET). No HA service call is made; ZHA reports the new brightness via async_update_entity. On the next adaptation interval AL must detect the change and stop overriding the user's brightness. The test verifies the user-visible symptom: after two adaptation cycles following a simulated direct-Zigbee brightness change, the light's brightness must still be the manually set value — not AL's own target. NOTE: this test FAILS on the current code. It is committed here to document the bug before the fix is applied in the next commit. * fix: merge last_service_data across split calls to fix detect_non_ha_changes with separate_turn_on_commands When separate_turn_on_commands=True, each adaptation cycle makes two light.turn_on calls (brightness, then color_temp). Previously each call overwrote last_service_data[light], so after the cycle only the color_temp key remained. _attributes_have_changed() then saw old_brightness=None and silently skipped the brightness comparison, so a manually-set brightness was never detected and AL kept overriding it. Fix: merge instead of overwrite so all split-call attributes accumulate: self.manager.last_service_data[light] = { **self.manager.last_service_data.get(light, {}), **service_data, } * test: add intermediate assertions to regression test Two assertions were promised in the PR description but missing: 1. After the force-adapt, assert that last_service_data contains BOTH brightness AND color — directly proving the merge fix works. 2. After the first non-forced update, assert that BRIGHTNESS is in manual_control — proving detection fired, not just that the final state is right. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * refactor: remove spurious comments, trim test docstring and assertions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: strip verbose comments from test, trim assert messages Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * test: add message to bare assert
1 parent f9ccc94 commit 6cebe14

File tree

2 files changed

+78
-3
lines changed

2 files changed

+78
-3
lines changed

custom_components/adaptive_lighting/switch.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1356,7 +1356,10 @@ async def _execute_adaptation_calls(self, data: AdaptationData) -> None:
13561356
data.context.id,
13571357
)
13581358
light = service_data[ATTR_ENTITY_ID]
1359-
self.manager.last_service_data[light] = service_data
1359+
self.manager.last_service_data[light] = {
1360+
**self.manager.last_service_data.get(light, {}),
1361+
**service_data,
1362+
}
13601363
await self.hass.services.async_call(
13611364
LIGHT_DOMAIN,
13621365
SERVICE_TURN_ON,

tests/test_switch.py

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from copy import deepcopy
1010
from random import randint
1111
from typing import Any
12-
from unittest.mock import Mock, patch
12+
from unittest.mock import AsyncMock, Mock, patch
1313

1414
import homeassistant.util.dt as dt_util
1515
import pytest
@@ -1696,7 +1696,7 @@ async def change_switch_settings(**kwargs):
16961696

16971697
async def test_cancellable_service_calls_task(hass):
16981698
"""Test the creation and execution of the task that wraps adaptation service calls."""
1699-
(light, *_) = await setup_lights(hass)
1699+
light, *_ = await setup_lights(hass)
17001700
_, switch = await setup_switch(hass, {CONF_SEPARATE_TURN_ON_COMMANDS: True})
17011701
context = switch.create_context("test")
17021702

@@ -2914,3 +2914,75 @@ async def test_adapt_only_on_bare_turn_on_respects_pause_changed_mode(hass, inte
29142914
f"With take_over_control_mode=PAUSE_CHANGED and only brightness marked "
29152915
f"as manually controlled, color_temp should still be adapted."
29162916
)
2917+
2918+
2919+
async def test_detect_non_ha_changes_with_separate_turn_on_commands(hass):
2920+
"""Regression test for detect_non_ha_changes with separate_turn_on_commands.
2921+
2922+
With separate_turn_on_commands=True, each adaptation cycle makes two sequential
2923+
light.turn_on calls (brightness, then color). If the second call overwrites
2924+
last_service_data instead of merging, brightness is dropped — and
2925+
_attributes_have_changed silently skips the brightness comparison, so a direct
2926+
Zigbee brightness change is never detected as manual control.
2927+
"""
2928+
switch, (light, *_) = await setup_lights_and_switch(
2929+
hass,
2930+
{
2931+
CONF_SEPARATE_TURN_ON_COMMANDS: True,
2932+
CONF_DETECT_NON_HA_CHANGES: True,
2933+
CONF_TAKE_OVER_CONTROL: True,
2934+
},
2935+
)
2936+
2937+
context = switch.create_context("test")
2938+
2939+
async def update(force: bool = False):
2940+
await switch._update_attrs_and_maybe_adapt_lights(
2941+
context=context,
2942+
force=force,
2943+
transition=0,
2944+
)
2945+
await hass.async_block_till_done()
2946+
2947+
await update(force=True)
2948+
2949+
last_sd = switch.manager.last_service_data.get(ENTITY_LIGHT_1)
2950+
assert last_sd is not None, "last_service_data not set after force adapt"
2951+
assert (
2952+
ATTR_BRIGHTNESS in last_sd
2953+
), f"brightness missing from last_service_data after split calls: {last_sd}"
2954+
assert (
2955+
ATTR_COLOR_TEMP_KELVIN in last_sd or ATTR_RGB_COLOR in last_sd
2956+
), f"color missing from last_service_data after split calls: {last_sd}"
2957+
2958+
al_brightness = light._brightness
2959+
switch.manager.manual_control[ENTITY_LIGHT_1] = LightControlAttributes.NONE
2960+
2961+
manual_brightness = (
2962+
al_brightness - 120 if al_brightness >= 120 else al_brightness + 120
2963+
)
2964+
light._brightness = manual_brightness
2965+
2966+
async def _flush_attr_state(hass, entity_id):
2967+
"""Mimic a ZHA attribute report: write current hardware state to HA."""
2968+
light.async_write_ha_state()
2969+
2970+
with patch(
2971+
"homeassistant.components.adaptive_lighting.switch.async_update_entity",
2972+
new=AsyncMock(side_effect=_flush_attr_state),
2973+
):
2974+
await update(force=False)
2975+
2976+
assert LightControlAttributes.BRIGHTNESS in switch.manager.manual_control.get(
2977+
ENTITY_LIGHT_1,
2978+
LightControlAttributes.NONE,
2979+
), (
2980+
f"manual_control={switch.manager.manual_control.get(ENTITY_LIGHT_1)}, "
2981+
f"last_service_data={switch.manager.last_service_data.get(ENTITY_LIGHT_1)}"
2982+
)
2983+
2984+
await update(force=False)
2985+
2986+
assert (
2987+
light._brightness == manual_brightness
2988+
), f"AL overrode manual brightness {manual_brightness} with {al_brightness}"

0 commit comments

Comments
 (0)