From a4ec1791e28f9ae949fe4eb6415c16e9e88e2ba0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 7 Sep 2026 17:10:39 +0200 Subject: [PATCH] Make nested scopes exact, point the examples at loop(), keep the test component alive --- esphome/core/application.h | 30 +++++++++++-------- tests/components/core/test_blocking_scope.cpp | 23 ++++++++++---- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/esphome/core/application.h b/esphome/core/application.h index d3e4e28fb2..15e3e36117 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -635,8 +635,8 @@ class LoopBlockingGuard { /// Leaves a stretch of the current loop pass out of the blocking warning. /// /// Only for work that cannot be made shorter and cannot be split across -/// passes: bringing up a radio, the initial connect of a network stack, a -/// key generation whose cost is the algorithm itself. The warning then keeps +/// passes: enabling a radio, the initial connect of a network stack, a key +/// generation whose cost is the algorithm itself. The warning then keeps /// reporting everything else in the pass, and the component's threshold does /// not ratchet up over the one step nothing can be done about. /// @@ -646,22 +646,27 @@ class LoopBlockingGuard { /// the warning exists to find, and wrapping them in this scope hides the /// bug instead of fixing it. If in doubt, leave the warning in. /// -/// Main loop task only. The watchdog is not fed inside the scope, so the -/// work must still finish within the watchdog timeout. +/// Only work timed by a LoopBlockingGuard is affected, that is a component's +/// loop() or a scheduler callback; setup() is not timed, so the scope does +/// nothing there. Main loop task only. The watchdog is not fed inside the +/// scope, so the work must still finish within the watchdog timeout. Scopes +/// may nest; the outermost one decides how much of the pass is left out. /// -/// { -/// UnavoidableBlockingScope scope; -/// bring_up_radio(); +/// void MyComponent::loop() { +/// if (this->needs_key_) { +/// UnavoidableBlockingScope scope; +/// this->generate_key_(); +/// } /// } class UnavoidableBlockingScope { public: - UnavoidableBlockingScope() : started_(millis()) {} + UnavoidableBlockingScope() : started_(millis()), pass_start_(App.get_loop_component_start_time()) {} ~UnavoidableBlockingScope() { - // Move the pass start forward by the time spent here, but never past now: - // nested scopes count the inner stretch twice, and a start in the future - // would underflow the guard's subtraction + // Move the pass start seen at entry forward by the time spent here, so an + // outer scope overrides an inner one instead of adding to it; never past + // now, which would underflow the guard's subtraction const uint32_t now = millis(); - const uint32_t moved = App.get_loop_component_start_time() + (now - this->started_); + const uint32_t moved = this->pass_start_ + (now - this->started_); App.set_loop_component_start_time_(static_cast(now - moved) < 0 ? now : moved); } UnavoidableBlockingScope(const UnavoidableBlockingScope &) = delete; @@ -669,6 +674,7 @@ class UnavoidableBlockingScope { private: uint32_t started_; + uint32_t pass_start_; }; // Phase A: drain wake notifications and run the scheduler. Invoked on every diff --git a/tests/components/core/test_blocking_scope.cpp b/tests/components/core/test_blocking_scope.cpp index 92822d0efd..69b2d37ade 100644 --- a/tests/components/core/test_blocking_scope.cpp +++ b/tests/components/core/test_blocking_scope.cpp @@ -32,10 +32,12 @@ TEST(UnavoidableBlockingScope, ZeroLengthScopeLeavesTheStartAlone) { EXPECT_LE(App.get_loop_component_start_time() - pass_start, millis() - before); } -// Nesting counts the inner stretch twice; the start must still never pass now -TEST(UnavoidableBlockingScope, NestedScopesNeverMoveTheStartPastNow) { +// Nested scopes leave out the outer span exactly once, and never move the +// start past now +TEST(UnavoidableBlockingScope, NestedScopesExcuseTheOuterSpanOnce) { const uint32_t pass_start = millis(); LoopBlockingGuard guard(nullptr, nullptr, pass_start); + const uint32_t before = millis(); { UnavoidableBlockingScope outer; { @@ -44,16 +46,25 @@ TEST(UnavoidableBlockingScope, NestedScopesNeverMoveTheStartPastNow) { } delay(5); } - const uint32_t now = millis(); - EXPECT_GE(static_cast(now - App.get_loop_component_start_time()), 0); - EXPECT_GE(App.get_loop_component_start_time() - pass_start, 35u); + const uint32_t excused = millis() - before; + const uint32_t moved = App.get_loop_component_start_time() - pass_start; + EXPECT_GE(moved, 35u); + EXPECT_LE(moved, excused); + EXPECT_GE(static_cast(millis() - App.get_loop_component_start_time()), 0); } +namespace { +// Static: the guard publishes the component to App and nothing clears it class DummyComponent : public Component {}; +DummyComponent &blocking_test_component() { + static DummyComponent component; + return component; +} +} // namespace // The excused stretch must neither warn nor ratchet the component's threshold TEST(UnavoidableBlockingScope, ExcusedStretchDoesNotRatchetTheThreshold) { - DummyComponent component; + DummyComponent &component = blocking_test_component(); uint32_t threshold_before = 0; component.should_warn_of_blocking(0, threshold_before); {