Skip to content

hour_effect_v2: fix UTC/localTime mismatch, notify ordering, and input-pin self-heal - #27

Open
KrX3D wants to merge 1 commit into
hour_effect_v2from
fix/hour-effect-v2-review-bugs
Open

hour_effect_v2: fix UTC/localTime mismatch, notify ordering, and input-pin self-heal#27
KrX3D wants to merge 1 commit into
hour_effect_v2from
fix/hour-effect-v2-review-bugs

Conversation

@KrX3D

@KrX3D KrX3D commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Summary

Follow-up review pass on the hour_effect_v2 usermod (after the earlier #23/#26 fixes), looking specifically for remaining bugs. Found and fixed four issues:

  • Stale-message check compared the wrong clock. onMqttMessage's /NotificationEffect handler rejects a retained/replayed message once its own durationMs has elapsed since it was sent, by comparing the payload's msgTimestamp (a real UTC Unix timestamp, e.g. Home Assistant's as_timestamp(now())) against WLED's global localTime. localTime is timezone/DST-adjusted (updateLocalTime() in ntp.cpp), so on any device configured for a non-zero UTC offset the computed age was off by roughly the timezone offset — enough to exceed maxAge (capped at 600s) and make every fresh notification effect look "stale" and get silently dropped. Now compares against toki.second(), the raw NTP UTC seconds, instead.
  • Notify fired before the LED state it described. applyEffectSettings() had a comment claiming it notifies after turning the LEDs on so stateUpdated() sees the correct bri, but the actual statement order still called stateUpdated() before _SetLedsOn(). Any MQTT/UDP listener (e.g. Home Assistant) receiving the notification while the LEDs were off would see the new effect colors reported against bri=0. Reordered so _SetLedsOn() runs first; also restored the internalStateChange guard around the subsequent stateUpdated() call, since _SetLedsOn() clears that flag at its own end and stateUpdated() synchronously re-enters this usermod's own onStateChange().
  • Input-pin presence mode couldn't self-heal. Every MQTT presence path calls reconcilePresenceLed() when a message reports "no change" but presence is still true and the LEDs are off (e.g. someone turned them off manually via the app/HA), so it can safely turn them back on later. checkInputPin() had no equivalent for the steady-state (unchanged) poll, so a user relying on the physical input pin had no way to recover once the LEDs were switched off externally while the pin stayed active.
  • Mojibake in the in-file docs. The JSON config example comments had "Küche" corrupted to a replacement character / ? in two places from a prior lossy save.

Test plan

  • Static review only in this session (no PlatformIO toolchain available in this sandbox to build/flash). The change is a 4-hunk, single-file diff with no new dependencies or API surface changes.
  • Recommend building and flashing to a real device before merging, especially to confirm the notification-effect stale-check fix with a non-UTC timezone configured.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J6bJtispewBKR2dXBcNWpw


Generated by Claude Code

…t-pin self-heal

- parseNotificationEffectPayload's stale-message check compared msgTimestamp
  (a real UTC epoch, e.g. HA's as_timestamp(now())) against localTime, which
  is timezone/DST-adjusted. On any non-UTC device this made every fresh
  notification effect look stale and get dropped. Use toki.second() (raw NTP
  UTC seconds) instead.
- applyEffectSettings() called stateUpdated() before _SetLedsOn() despite a
  comment claiming the order was already fixed, so MQTT/UDP listeners were
  notified of the new effect colors while bri was still 0. Reordered to turn
  the LEDs on first, restoring the internalStateChange guard around the
  stateUpdated() call so it doesn't misinterpret its own notification as an
  external brightness change.
- checkInputPin() never called reconcilePresenceLed() on an unchanged read,
  unlike every MQTT presence path, so pin-based presence had no way to
  self-heal if the LEDs were turned off externally while the pin stayed
  active. Added the same steady-state reconcile call.
- Fixed mojibake ("Küche" corrupted to replacement/`?` characters) in the
  in-file JSON config documentation examples.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J6bJtispewBKR2dXBcNWpw
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.

2 participants