Callback::create() can store callables inline (no heap allocation)
when they fit in sizeof(void*) — i.e., a single [this] capture on
32-bit platforms. Many automation triggers captured [this, parent]
in their callback lambdas, doubling the capture size and forcing
heap allocation.
Store the parent/state pointer as a class member and capture only
[this] in the lambda. This trades 4 bytes of member storage for
avoiding a permanent heap allocation per callback registration.
Components changed: cover, valve, lock, fan, media_player, datetime,
display_menu_base, graphical_display_menu, esp32_improv, mqtt_fan.
Guard against launching a second FreeRTOS update task while one is
already running. Without this, calling update() twice (e.g. from both
the polling interval and the initial check interval) could spawn two
concurrent tasks both writing to the same UpdateInfo.
With the single-defer structure from the race fix PR, clearing the
handle only needs to happen in one place — at the top of the deferred
lambda, before any other work.
Address review feedback:
- Move error_str out of public UpdateInfo into file-local TaskResult
- Add container.reset() before vTaskDelete (doesn't call destructors)
- Remove redundant has_progress/progress assignments after move
- Add comment on delete after std::move
Address Copilot review feedback:
- Call container->end() when status_code != HTTP_STATUS_OK (pre-existing
bug, but easy to fix while we're here)
- Update error_str comment to say "update check failure" not "fetch failure"
- Move state computation to main loop callback (avoids 2 extra lambda captures)
- Add error_str field to UpdateInfo so error info rides with the pointer
- Allocate UpdateInfo once at top of fetch_manifest_ (single ownership path)
- Lambda captures only 2 pointers (8 bytes), fits std::function SBO
Reduces ESP8266 flash delta from +480 to ~+288 bytes.
The update task runs in a FreeRTOS thread on ESP32 and was writing
directly to update_info_ std::string fields while the main loop
reads them via API (as StringRef pointers into the string buffer),
MQTT, web server, and Prometheus. This is undefined behavior that
can cause use-after-free crashes when a string reallocation
invalidates a StringRef held by the API serialization path.
Move all update_info_ and state_ writes into the existing defer()
callback so they execute on the main loop. The task now accumulates
results in a local UpdateInfo struct and moves it into update_info_
in the deferred callback, eliminating the cross-thread data race
with zero steady-state memory overhead.
Move the fast-path checks from cleanup_(), process_to_add(), and
process_defer_queue_() into inline methods in the header. The common
case — nothing to clean up, nothing to add, nothing deferred — now
resolves with just an atomic load at the call site, avoiding a function
call into the .cpp translation unit entirely.
The slow paths remain in scheduler.cpp as cleanup_slow_path_(),
process_to_add_slow_path_(), and process_defer_queue_slow_path_().
Also moves the process_defer_queue_() body out of the header into
the .cpp file, reducing code bloat in every translation unit that
includes scheduler.h.
Add find_first parameter to cancel_item_locked_ and
mark_matching_items_removed_locked_, defaulting to false (cancel all).
When find_first=true, stops after the first match and exits across
containers. set_timer_common_ passes find_first=true where the
cancel-before-add invariant guarantees at most one match.
The public cancel path uses the default find_first=false to cancel ALL
matches, needed for DelayAction parallel mode where skip_cancel=true
can create multiple items with the same key.
Also fixes the Scheduler_SetInterval benchmark to call
process_to_add() every kKeyCount registrations with a final drain
after the inner loop, reflecting production behavior.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
The test was calling show_logs() which invokes _wait_for_serial_port("/dev/ttyUSB0").
Since that path doesn't exist in CI/dev environments, it spun in a 30-second polling
loop before returning. Add mock_wait_for_serial_port fixture to skip the wait.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
J. Nick Koston