backlog: fix timer reset during drain; preserve CmndDelay value#24878
Open
joluxer wants to merge 1 commit into
Open
backlog: fix timer reset during drain; preserve CmndDelay value#24878joluxer wants to merge 1 commit into
joluxer wants to merge 1 commit into
Conversation
BacklogLoop previously left backlog_timer at the value written by CommandHandler at the start of ExecuteCommand -- millis() at command start time plus P_BACKLOG_DELAY. After a long execution (rule-trigger chain) that point was already past: TimeReached() returned true immediately on the next iteration, draining the queue without pause regardless of SetOption34. A nested CmndBacklog reset the timer to millis() during execution, compounding the problem: the queue drained as fast as commands were enqueued. BacklogLoop now unconditionally sets backlog_timer after each timed drain step. The inter-command interval becomes deterministic: exactly P_BACKLOG_DELAY between drain steps, regardless of what CommandHandler or nested Backlog commands wrote during ExecuteCommand. Exception: CmndDelay called from within a drain sets backlog_delay_guard (detected via backlog_mutex=true). BacklogLoop preserves the timer for that one step and clears the guard. External commands can still shorten an active delay -- that matches original Tasmota behaviour.
Collaborator
|
Can you double-check this: Small bug: stale
|
Contributor
Author
|
Sure, I'll look at it. Currently not at home, but back during the weekend. It's of cause an useful case, that I might have overlooked. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Problem
BacklogLoopleftbacklog_timerat whatever valueCommandHandlerhadwritten at the start of
ExecuteCommand:millis()at command start timeplus
P_BACKLOG_DELAY(SetOption34). After a long execution -- a rule-triggerchain, for example -- that point was already in the past.
TimeReached()returned true immediately on the next iteration, and the queue drained without
pause regardless of SetOption34.
A nested
CmndBackloginvoked during execution reset the timer tomillis()from within
ExecuteCommand. That madeTimeReached()true immediately forevery subsequent entry: the queue drained as fast as commands were enqueued,
turning SetOption34 into a dead letter under rule-driven Backlog activity.
This was observable as unbounded queue growth and free-heap decline under
sustained rule activity. Issuing
Backlogwith no arguments (which clears thequeue) immediately restored heap.
Fix
BacklogLoopnow unconditionally setsbacklog_timerafter each timed drainstep. The inter-command interval is deterministic: exactly
P_BACKLOG_DELAYbetween drain steps, regardless of what
CommandHandleror nestedBacklogcommands wrote during
ExecuteCommand.One exception:
CmndDelaycalled from within a drain setsbacklog_delay_guard(detected via
backlog_mutex == true).BacklogLoopthen preserves the timervalue for that one step and clears the guard. This keeps
Delayfunctionalinside Backlog sequences. External commands can still shorten an active delay --
that matches original Tasmota behaviour and is intentional.
Invariant established:
backlog_timeris always set byBacklogLoopafter each drain step.CmndDelayis the one permitted exception, signaled viabacklog_delay_guard.Changes
TasmotaGlobal_t: newbool backlog_delay_guardfield (1 byte)BacklogLoop()intasmota.ino: 4-line addition -- the existingnodelay branch is unchanged; two new branches handle the guard and
the normal timed reset
CmndDelay()insupport_command.ino: 1-line addition sets the guardMemory impact (measured: ESP8266 tasmota-4M, ESP32 tasmota32)
backlog_delay_guard(1 byte) is packed into existing padding inTasmotaGlobal_t; net static RAM delta is 0 on both targets.Tested
Build-tested on ESP8266 (all 37 targets) and ESP32 Xtensa and RISC-V
(all standard targets). Target-tested on ESP8266 (ESP-12F) with two
nearly-full rule sets generating frequent Backlog sequences: queue now
drains at the expected SetOption34 interval under sustained load.
Checklist: