Skip to content

Commit 6024fdd

Browse files
author
Florian Horner
committed
Merge PR basnijholt#1379: Add regression test for separate_turn_on_commands race condition (basnijholt)
Conflicts resolved: - tests/test_switch.py: kept fork's test_automation_turn_on_from_off_not_marked_as_manual_control (from basnijholt#1378 fix) and appended upstream's test_separate_turn_on_commands_respects_light_off_state at end of file so both regression tests coexist.
2 parents 624862a + 45756aa commit 6024fdd

1 file changed

Lines changed: 145 additions & 0 deletions

File tree

tests/test_switch.py

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2993,3 +2993,148 @@ async def _flush_attr_state(hass, entity_id):
29932993
assert (
29942994
light.brightness == manual_brightness
29952995
), f"AL overrode manual brightness {manual_brightness} with {al_brightness}"
2996+
2997+
2998+
async def test_separate_turn_on_commands_respects_light_off_state(hass):
2999+
"""Test that split commands are not sent when light is turned off between commands.
3000+
3001+
Regression test for https://github.com/basnijholt/adaptive-lighting/issues/1373
3002+
3003+
When `separate_turn_on_commands: true` is enabled and a light is turned off between
3004+
the split brightness and color commands, the second command should be skipped.
3005+
3006+
The bug occurs because:
3007+
1. When proactive adaptation context exists, the off-check is bypassed in
3008+
`_execute_adaptation_calls` (switch.py:1337-1349)
3009+
2. The proactive context is never cleared when `light.turn_off` is called
3010+
3. This causes the second split command (color) to be sent to an off light
3011+
3012+
This results in lights showing "on at 0% brightness" in the UI after being
3013+
turned off, which is confusing for users.
3014+
"""
3015+
switch, _ = await setup_lights_and_switch(
3016+
hass,
3017+
{
3018+
CONF_INTERCEPT: True,
3019+
CONF_SEPARATE_TURN_ON_COMMANDS: True,
3020+
},
3021+
all_lights=True,
3022+
)
3023+
3024+
_mock_sun_light_settings(
3025+
switch,
3026+
{
3027+
ATTR_BRIGHTNESS_PCT: 67,
3028+
ATTR_COLOR_TEMP_KELVIN: 3448,
3029+
"force_rgb_color": False,
3030+
},
3031+
)
3032+
3033+
# Track all light.turn_on service calls with timestamps
3034+
turn_on_calls = []
3035+
turn_off_time = None
3036+
3037+
async def track_turn_on_calls(event: Event) -> None:
3038+
if (
3039+
event.data.get("domain") == LIGHT_DOMAIN
3040+
and event.data.get("service") == SERVICE_TURN_ON
3041+
):
3042+
turn_on_calls.append(
3043+
{"event": event, "after_turn_off": turn_off_time is not None},
3044+
)
3045+
3046+
hass.bus.async_listen(EVENT_CALL_SERVICE, track_turn_on_calls)
3047+
3048+
# Turn off the light first to start from a known state
3049+
await hass.services.async_call(
3050+
LIGHT_DOMAIN,
3051+
SERVICE_TURN_OFF,
3052+
{ATTR_ENTITY_ID: ENTITY_LIGHT_3},
3053+
blocking=True,
3054+
)
3055+
await hass.async_block_till_done()
3056+
assert hass.states.get(ENTITY_LIGHT_3).state == STATE_OFF
3057+
3058+
# Clear the tracked calls
3059+
turn_on_calls.clear()
3060+
3061+
# Turn on the light - this triggers proactive adaptation with split commands
3062+
turn_on_context = Context(id="test_turn_on_context")
3063+
await hass.services.async_call(
3064+
LIGHT_DOMAIN,
3065+
SERVICE_TURN_ON,
3066+
{ATTR_ENTITY_ID: ENTITY_LIGHT_3},
3067+
blocking=True,
3068+
context=turn_on_context,
3069+
)
3070+
await hass.async_block_till_done()
3071+
3072+
# The first split command (brightness) should have been sent
3073+
assert len(turn_on_calls) >= 1, "Expected at least one turn_on call"
3074+
3075+
# Verify proactive adaptation context is set
3076+
# BUG: This context will remain set even after turn_off
3077+
assert switch.manager.is_proactively_adapting(
3078+
"test_turn_on_context",
3079+
), "Proactive adaptation context should be set after turn_on"
3080+
3081+
# Mark that we're about to turn off
3082+
turn_off_time = True
3083+
3084+
# Now immediately turn off the light before the second split command
3085+
await hass.services.async_call(
3086+
LIGHT_DOMAIN,
3087+
SERVICE_TURN_OFF,
3088+
{ATTR_ENTITY_ID: ENTITY_LIGHT_3},
3089+
blocking=True,
3090+
)
3091+
await hass.async_block_till_done()
3092+
3093+
# BUG VERIFICATION: The proactive context should be cleared on turn_off,
3094+
# but currently it's not, causing the second split command to bypass the off-check
3095+
# After the fix, this should return False
3096+
proactive_still_set = switch.manager.is_proactively_adapting("test_turn_on_context")
3097+
3098+
# Wait for all adaptation tasks to complete
3099+
await asyncio.gather(*switch.manager.adaptation_tasks)
3100+
await hass.async_block_till_done()
3101+
3102+
# Check for calls made after turn_off
3103+
calls_after_turn_off = [c for c in turn_on_calls if c["after_turn_off"]]
3104+
3105+
# The light should be OFF
3106+
final_state = hass.states.get(ENTITY_LIGHT_3)
3107+
3108+
# The bug manifests in two ways:
3109+
# 1. Proactive context is still set after turn_off (should be cleared)
3110+
# 2. Additional turn_on calls are made after turn_off (second split command)
3111+
# 3. Light ends up in ON state after turn_off
3112+
3113+
if proactive_still_set:
3114+
# This is the root cause of the bug
3115+
pytest.fail(
3116+
"Bug confirmed: Proactive adaptation context is still set after light.turn_off. "
3117+
"This causes the off-check to be bypassed for the second split command. "
3118+
"The fix should clear proactive context when light.turn_off is detected.",
3119+
)
3120+
3121+
if final_state.state == STATE_ON:
3122+
pytest.fail(
3123+
f"Bug confirmed: Light is ON after turn_off was called. "
3124+
f"The second split command (color) was sent to the off light. "
3125+
f"Calls after turn_off: {len(calls_after_turn_off)}",
3126+
)
3127+
3128+
# Verify no turn_on calls were made after turn_off
3129+
for call_info in calls_after_turn_off:
3130+
call_entity = (
3131+
call_info["event"].data.get("service_data", {}).get(ATTR_ENTITY_ID)
3132+
)
3133+
if call_entity == ENTITY_LIGHT_3 or ENTITY_LIGHT_3 in (call_entity or []):
3134+
pytest.fail(
3135+
f"Bug confirmed: A light.turn_on call was made after light.turn_off. "
3136+
f"This is the race condition where the second split command bypasses "
3137+
f"the off-check due to proactive adaptation context not being cleared. "
3138+
f"Call data: {call_info['event'].data}",
3139+
)
3140+

0 commit comments

Comments
 (0)