backlog feature: fast-lane queue for nodelay commands (USE_BACKLOG_FASTLANE)#24882
Open
joluxer wants to merge 7 commits into
Open
backlog feature: fast-lane queue for nodelay commands (USE_BACKLOG_FASTLANE)#24882joluxer wants to merge 7 commits into
joluxer wants to merge 7 commits 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.
Introduce SettingsParam(index) as a named function returning a uint8_t reference to Settings->param[index]. Adds bounds protection: an index >= PARAM8_SIZE returns a reference to a stable zero byte instead of an out-of-bounds access. Replace three direct Settings->param[] accesses in the SetOption handler (CmndSetoption, GetOption, CmndSetoptionBase) to use the new accessor.
Move the backlog queue, timer, and control flags out of TasmotaGlobal_t and the global .ino scope into a dedicated translation unit (support_backlog.cpp). The state was accessible from any .ino file; the new module limits mutation to explicit paths through the Backlog:: API. All internal state is held in an anonymous namespace. The public interface is declared in support_backlog.h. A one-line support_backlog.ino adapter keeps BacklogLoop() visible to the unity build. Removes from TasmotaGlobal_t: backlog_timer, backlog_nodelay, backlog_mutex, backlog_delay_guard, backlog_no_mqtt_response. Removes global LList<char*> backlog. Adds SuppressMqttResponse() to support.ino as the designated mutation path for TasmotaGlobal.no_mqtt_response from backlog context. No intended behavior change.
SetOption166 (default 0) controls whether external commands (MQTT, Serial, Button, ...) extend the Backlog drain window after execution. SO166=0: CommandHandler sets the drain timer to millis() + BACKLOG_EXT_DELAY (100 ms, separate from SO34, overridable at build time via user_config_override.h). SO166=1: BacklogLoop is the sole timer owner; external commands do not stall the drain. The inter-command delay within a sequence (SO34) is unchanged. Each queue entry now carries a flavor byte (bit0=nodelay, bit1=no_mqtt_resp) baked in from staged flags at enqueue time. D_CMND_NODELAY is resolved in CmndBacklog() and ExecuteCommandBlock() at enqueue rather than stored as a queue token. Loop() reads the flavor byte per drain step; _nodelay and _no_mqtt_resp each split into _staged (set by callers) and _current (set from the flavor byte, read by IsNodelay()). ExecuteCommandBlock() resets staged flags to plain-Backlog defaults before its tokenisation loop: IF blocks no longer inherit ambient state from a preceding CmndBacklog() call. ScheduleDelay() is a no-op when _nodelay_current is set. Default-configuration behavior: the inter-command delay (SO34) is unchanged. The post-external-command drain window changes from SO34 to BACKLOG_EXT_DELAY (100 ms vs. 200 ms default) -- this is intentional and separately configurable.
Backlog16 N: set runtime chunk size for Backlog21-29 paged content dump (default 20; non-persistent). Backlog17 0/1: enable per-drain-step trace log at INFO level (BLG: tag). Logs queue depth before drain, flavor byte, and command string per entry. Build flag BACKLOG_TRACE_SOURCE extends the trace with the command source. Backlog20: JSON statistics snapshot covering depth, timer state, staged and current flags, per-operation counters, high-watermark values, and runtime config (SO34/SO166, chunk size). Bytes/Discarded/MaxBytes report 0 in this commit; the follow-up byte-limit commit fills them with real values. Backlog21-29: paged queue content as JSON. Page = index - 21, chunk size from Backlog16. Entries appear on individual log lines and in an assembled MQTT/Web-Console response. The entry header is generalized to accommodate the optional BACKLOG_TRACE_SOURCE source byte. A compile-time offset replaces the previous hardcoded value, keeping the conditional out of every access site. The source parameter on EnqueueCmd/InsertCmd is unconditional with a "not annotated" default. A running depth counter alongside the linked-list length serves as a cross-check; underflow is logged at error level. Per-operation counters for drain, enqueue, insert, and mutex-skip feed the Backlog20 snapshot.
Without a size bound the queue can grow until a malloc fails mid-sequence, leaving partial command sets in flight and the node unreachable. The byte limit trades completeness for availability: a sequence that would exceed the limit is rejected whole instead of partially enqueued. The node stays reachable and reports the rejection in the log. Empirically confirmed on ESP8266: 30+ min under sustained MQTT load without crash; limit adjustment under load drains the queue cleanly. Backlog18 N: set the byte limit at runtime (0 resets to the compile-time default). Clamped to a minimum confirmed stable on ESP8266 under load. Platform-dependent compile-time defaults are in tasmota.h; override via user_config_override.h. Accounting covers the command string, the linked-list node, and heap-allocator bookkeeping per allocation - the overhead a string-length check alone would miss. The formula is centralized so that a change to the overhead model touches one place. LList gains a public node-size constant so the overhead calculation stays inside the queue module rather than depending on LList internals. Per-command fallback checks guard the rules IF/ENDIF insertion path and any direct caller that bypasses the sequence-level gate. Backlog20 (added in the preceding diagnostics commit) now reports real byte counters and the active limit instead of placeholder zeros.
Nodelay sequences shared the timer-gated queue with timed sequences. An external command triggering the SO166 drain window stalled the whole queue by 100 ms even for commands that were queued without delay. A dedicated fast lane removes that coupling. When USE_BACKLOG_FASTLANE is defined and SetOption167 is active, nodelay commands are placed in a separate queue and drained within a per-call time budget, bypassing the SO34/SO166 timer gate. At least one command is drained per BacklogLoop() call regardless of budget. The budget defaults to the current main-loop sleep interval so the burst scales naturally; Backlog19 N overrides it at runtime (0=sleep, 50-250 ms). Common drain logic is factored so both lanes share the same path; the per-entry trace log (Backlog17) tags each entry by lane so fast-lane and timed-lane activity are distinguishable in the log. The sleep-blocking check is corrected: the previous test read a post-drain state for a pre-drain decision. The replacement checks whether the fast queue has pending work or the timed-lane timer has already lapsed - both genuine pre-drain signals. Backlog20 reports fast-lane depth, drain count, and effective budget. Backlog21-29 includes a lane field per entry.
Collaborator
|
Hi, I understand the implementation, but I fear it looks a little bit as an overkill. Backlog is supposed to serve very minimalistic use cases, whereas full scripting with Berry (ESP32 and above) offer much more flexibility. Do you have concrete examples where this should be used? I also fear that additional backlog options makes it hard to use and understand to most users that have basic needs. @arendst any opinion? |
Collaborator
|
Here are the findings found by gpt-5.5: can you double check please? (if we go for merging) Findings:
|
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
2 participants
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
Part of the backlog improvement series documented in discussion #24776.
Implementation choices are open to discussion. If you'd prefer a different approach, please comment -- I'll revise.
Situation
Commands queued via Backlog0/2 share the timer-gated queue with timed
sequences. The SO166 post-external-command window (100 ms) stalls the
whole queue, including entries that explicitly requested no delay. Under
sustained MQTT load this made the nodelay path unreliable.
Changes
Opt-in:
USE_BACKLOG_FASTLANEat build time, SetOption167 at runtime.Backlog0/2 commands are routed to a separate fast-lane queue at enqueue
time and drained per BacklogLoop() call with no timer gate, within a
burst budget. At least one entry is always drained. The budget defaults
to the current Sleep setting; Backlog19 N overrides it at runtime
(0=Sleep, 50-250 ms, out-of-range clamped).
The sleep-blocking check in the main loop is corrected to test
pre-drain state: whether the fast queue has pending work or the
timed-lane timer has lapsed.
Backlog19 N- burst budget (runtime, non-persistent).Backlog20extended: FastLane, FastDepth, FastDrained, FastBudget,FastBudgetMs.
Backlog21-29extended: Lane field ("F"/"T") per entry.Memory impact (measured)
Ref: pr/backlog-diagnostics tip (a97fc92c)
Cumulative PR6-PR10 vs development (5e8019d):
Tested
Build-tested on ESP8266 (tasmota-4M) and ESP32 (tasmota32).
Target test on ESP8266 (ESP-12F): fast-lane drain confirmed under
sustained MQTT load; SetOption167 toggle at runtime verified; Backlog19
budget adjustment under load confirmed. A device in continuous
production use showed significant behavioral stabilization.
Requires pr/backlog-diagnostics: the fast-lane queue extends Backlog20/21-29 diagnostics and relies on the byte-limit infrastructure for queue-depth tracking.
Checklist: