Skip to content

fix: prevent transient Zigbee off-state from cancelling adaptation#1460

Open
florianhorner wants to merge 3 commits intobasnijholt:mainfrom
florianhorner:florianhorner/fix-transient-off-cancels-adaptation
Open

fix: prevent transient Zigbee off-state from cancelling adaptation#1460
florianhorner wants to merge 3 commits intobasnijholt:mainfrom
florianhorner:florianhorner/fix-transient-off-cancels-adaptation

Conversation

@florianhorner
Copy link
Copy Markdown
Contributor

Summary

Fixes #1449 — lights turning off immediately (~680ms) after being turned on, particularly with Zigbee devices like FUT035Z and separate_turn_on_commands=True.

Root cause

When a Zigbee device briefly reports an off state during a turn-on transition (common with MiBoxer/FUT controllers), the state_changed_event_listener's on→off branch at line 2539 unconditionally called reset(), which cancelled the in-flight adaptation task. The off→on branch had an is_proactively_adapting guard (line 2557) but the on→off branch did not.

Additionally, just_turned_off() had a context-equality check (line 2760, added in #696) that fired before _off_to_on_state_event_is_from_turn_on, causing false positives when both events shared a context ID inherited from the same service call chain.

Changes

  1. Add is_proactively_adapting guard to the on→off branch — Transient off states during proactive adaptation now skip reset() (preserving the adaptation task) while still recording on_to_off_event to prevent stale state.

  2. Reorder and tighten just_turned_off() checks_off_to_on_state_event_is_from_turn_on now runs before the context-equality check, so real light.turn_on calls are never misclassified as phantom events. The context-equality check now also requires a matching turn_off_event, preventing false positives from inherited Zigbee context IDs while preserving the original Bail adapting if on event equals off event context #696 protection.

  3. Fix truthiness bug in _split_service_call_data — Changed if service_data.get(attribute) to if service_data.get(attribute) is not None so brightness=0 is no longer silently dropped from split service data.

Analysis

This fix was validated by 8 independent code reviews across 4 different AI models (Claude Opus, OpenAI Codex/o3, Perplexity Deep Research, adversarial agents). The Perplexity thread with the full analysis is available here for reference: https://www.perplexity.ai/search/i-m-analyzing-a-confirmed-bug-UNxwI3FaRouo_nwUM6mymw

How to verify

Users experiencing #1449 should look for this new debug log line when the transient off is handled correctly:

Detected an 'on' → 'off' event for '<light>' with context.id='<id>' during proactive adaptation, skipping reset

The absence of Cancelled ongoing brightness adaptation calls after a turn-on confirms the fix is working.

Test plan

  • Verify lights with separate_turn_on_commands=True no longer turn off intermittently after turn-on
  • Verify lights with transitions still properly cancel adaptation on genuine light.turn_off calls
  • Verify sleep_brightness=0 works correctly with separate_turn_on_commands=True
  • Verify rapid on/off/on toggling still behaves correctly

🤖 Generated with Claude Code

…asnijholt#1449)

Three fixes for lights turning off immediately after being turned on:

1. Add is_proactively_adapting guard to the on→off branch in
   state_changed_event_listener. Previously, a transient off state reported
   by Zigbee devices (e.g., FUT035Z) during a turn-on transition would
   unconditionally call reset(), cancelling the in-flight adaptation task.
   The off→on branch already had this guard; the on→off branch did not.

2. Reorder checks in just_turned_off() so that
   _off_to_on_state_event_is_from_turn_on runs BEFORE the context-equality
   check. The context-equality check (added in basnijholt#696 for lights polling as
   "on" during turn_off transitions) was incorrectly firing for the reverse
   case where a device briefly reports "off" during a turn_on. Also tighten
   the context-equality check to require a matching turn_off_event,
   preventing false positives from inherited Zigbee context IDs.

3. Fix truthiness check in _split_service_call_data that silently dropped
   brightness=0 values. Changed to explicit None check so that zero values
   are preserved in split service call data.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@basnijholt
Copy link
Copy Markdown
Owner

Sorry it took me so long to get to this. I've been a bit overwhelmed by the number of AI-assisted PRs opened here recently, and I've also been spending nearly every spare hour on my biggest project so far, MindRoom.

I'm very supportive of using AI for coding, but many of these PRs still need careful human review because even plausible-looking changes can introduce subtle breakage. That backlog made me postpone reviewing them for a while.

I've now done a batch review with Codex / GPT-5.5 (xhigh). This comment is AI-assisted, but I've reviewed it before posting.

This looks like the right direction to me. I do not see a blocking issue in the implementation, and the _split_service_call_data change is also important because it preserves zero-valued brightness data instead of dropping it as falsey.

The one thing I would really like before merging is regression coverage. This PR touches subtle context/cancellation behavior, so tests for the transient off-state case and the zero-brightness split data case would make this much safer to carry forward.

@basnijholt
Copy link
Copy Markdown
Owner

Follow-up from a deeper Codex / GPT-5.5 (xhigh) pass.

I re-read the actual context/cancellation change and still do not see a code-level blocker. The ordering change in just_turned_off() makes sense: a real light.turn_on should win over the old same-context polling-artifact shortcut, and preserving 0 in _split_service_call_data() is also important for sleep_brightness: 0.

The missing piece is regression coverage. Could you add tests for:

  1. A proactive adaptation where the device briefly reports off, verifying that the adaptation task is not cancelled/reset.
  2. The existing polling-artifact case where a real turn_off still causes just_turned_off() to return True.
  3. _split_service_call_data() preserving brightness: 0.

This is subtle enough that I would be much more comfortable merging it with those cases pinned down.

jaynis added a commit to jaynis/adaptive-lighting that referenced this pull request Apr 24, 2026
…holt#1460)

Signed-off-by: jaynis <kranz.jannis@googlemail.com>
florianhorner added a commit to florianhorner/adaptive-lighting that referenced this pull request Apr 26, 2026
…or expectations)

Fork main carried tests written against a planned "PR simplification" that
never landed: tests asserted that STEP_OPTIONS, ROOM_PRESETS,
CONF_ROOM_PRESET, CONF_ENABLE_DIAGNOSTIC_SENSORS, LightStatus,
LightStatusInfo, STATUS_PRIORITY, SIGNAL_STATUS_UPDATED, expand_light_groups,
and sensor.py had been removed — but the code still ships them.

Delete the predicate tests for code that was not removed. The test suite
is now a faithful description of what the code actually does.

* tests/test_const_and_helpers.py — deleted (entire file was these
  predicate-removal asserts).

* tests/test_config_flow.py — drop seven tests that asserted the
  single-step options flow (test_options, test_incorrect_options,
  test_options_single_step_completes, test_options_no_room_preset_field,
  test_options_no_sleep_step, test_options_sleep_brightness_zero_rejected,
  test_options_sleep_brightness_one_accepted).

* tests/test_adaptation_utils.py — fix the
  test_split_service_call_data_falsy_values parametrize set to assert
  PR basnijholt#1460's actual behavior (brightness=0 preserved by `is not None`,
  not dropped silently).

* strings.json + translations/en.json — drop the URL from
  options.step.init.description. Hassfest forbids URLs in translation
  strings. Other locale catalogs still carry the URL and will need a
  Weblate sync.

* tests/__init__.py — add minimal package-init with docstring. Ruff
  INP001 + D104 were blocking every commit because tests/ was an
  implicit namespace package with no __init__.py.

* tests/test_init.py — black formatting drift fixed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Light Immediately Turning Off After Turning On

2 participants