From 229dee4940f424a3113113f41871985ff16c0bb9 Mon Sep 17 00:00:00 2001 From: zekageri Date: Fri, 10 Apr 2026 16:17:26 +0200 Subject: [PATCH] adjustments --- README.md | 6 +- .../adapters/logger_observer.h | 7 +- src/esp_state_machine/state_machine.h | 54 +++- .../test_esp_state_machine.cpp | 291 ++++++++++++++++++ 4 files changed, 342 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index e1d8cde..6983dff 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,11 @@ For a successful dispatch: 4. Entry callbacks for the target state. 5. Machine-level transition observers. -`begin(initialState)` invokes entry callbacks for the initial state with `bootstrap=true`. `end()` invokes exit callbacks for the current state with `shutdown=true`. +`begin(initialState)` invokes entry callbacks for the initial state with `bootstrap=true`. `end()` invokes exit callbacks for the current state with `shutdown=true`. If `end()` is requested from inside a callback, shutdown is deferred until the active callback chain completes. + +## Runtime Cost +- Dispatch scans registered transitions linearly, and callback/observer invocation scans the registered callback lists linearly. +- Guards, actions, state callbacks, transition observers, and rejection observers are stored as `std::function`; prefer registering them during setup and keep captures small on memory-constrained targets. ## Optional Adapters ### EventBus Bridge diff --git a/src/esp_state_machine/adapters/logger_observer.h b/src/esp_state_machine/adapters/logger_observer.h index a027c43..5ad8d6c 100644 --- a/src/esp_state_machine/adapters/logger_observer.h +++ b/src/esp_state_machine/adapters/logger_observer.h @@ -47,7 +47,12 @@ template class ESPStateMachineLoggerObserver { ); } - return transitionCallbackId_ != 0 && (!options_.logRejected || rejectedCallbackId_ != 0); + if (transitionCallbackId_ == 0 || (options_.logRejected && rejectedCallbackId_ == 0)) { + detach(); + return false; + } + + return true; } void detach() { diff --git a/src/esp_state_machine/state_machine.h b/src/esp_state_machine/state_machine.h index f0b5e77..19dd915 100644 --- a/src/esp_state_machine/state_machine.h +++ b/src/esp_state_machine/state_machine.h @@ -167,6 +167,7 @@ template class ESPStateMachine { previousState_ = initialState; lastStatus_ = StateMachineDispatchStatus::NotStarted; sequence_ = 0; + stopPending_ = false; started_ = true; dispatching_ = true; @@ -175,27 +176,23 @@ template class ESPStateMachine { context.otherState = initialState; context.bootstrap = true; invokeStateCallbacks(enterCallbacks_, initialState, context); - dispatching_ = false; + completeCallbackRun(); return true; } void end() { - if (!started_ || dispatching_) { - started_ = false; + if (!started_) { + stopPending_ = false; return; } - dispatching_ = true; - StateCallbackContext context{}; - context.state = currentState_; - context.otherState = currentState_; - context.sequence = sequence_; - context.shutdown = true; - invokeStateCallbacks(exitCallbacks_, currentState_, context); - dispatching_ = false; + if (dispatching_) { + stopPending_ = true; + return; + } - started_ = false; + runShutdownCallbacks(); } bool isStarted() const { @@ -305,7 +302,7 @@ template class ESPStateMachine { result.status = StateMachineDispatchStatus::Transitioned; result.transitioned = true; setLastDispatch(event, result.status); - dispatching_ = false; + completeCallbackRun(); return result; } @@ -321,7 +318,7 @@ template class ESPStateMachine { rejected.status = result.status; invokeRejectedObservers(rejected); - dispatching_ = false; + completeCallbackRun(); return result; } @@ -431,6 +428,34 @@ template class ESPStateMachine { lastStatus_ = status; } + void completeCallbackRun() { + dispatching_ = false; + if (stopPending_) { + runShutdownCallbacks(); + } + } + + void runShutdownCallbacks() { + if (!started_) { + stopPending_ = false; + return; + } + + stopPending_ = false; + dispatching_ = true; + + StateCallbackContext context{}; + context.state = currentState_; + context.otherState = currentState_; + context.sequence = sequence_; + context.shutdown = true; + invokeStateCallbacks(exitCallbacks_, currentState_, context); + + dispatching_ = false; + stopPending_ = false; + started_ = false; + } + std::vector transitions_{}; std::vector enterCallbacks_{}; std::vector exitCallbacks_{}; @@ -445,4 +470,5 @@ template class ESPStateMachine { StateMachineCallbackId nextCallbackId_ = 1; bool started_ = false; bool dispatching_ = false; + bool stopPending_ = false; }; diff --git a/test/test_esp_state_machine/test_esp_state_machine.cpp b/test/test_esp_state_machine/test_esp_state_machine.cpp index 8d883df..d2e5dcb 100644 --- a/test/test_esp_state_machine/test_esp_state_machine.cpp +++ b/test/test_esp_state_machine/test_esp_state_machine.cpp @@ -48,6 +48,17 @@ void expectEqual(const T &actual, const T &expected, const std::string &message) } } +void expectOrder( + const std::vector &actual, + const std::vector &expected, + const std::string &message +) { + expectEqual(actual.size(), expected.size(), message + " size"); + for (size_t i = 0; i < expected.size(); ++i) { + expectEqual(actual[i], expected[i], message + " entry " + std::to_string(i)); + } +} + void testBeginEndLifecycleCallbacks() { ESPStateMachine machine; bool bootstrapped = false; @@ -165,6 +176,223 @@ void testCallbackOrderAndReentrantBusy() { expectEqual(order[3], std::string("observer"), "observer should run fourth"); } +void testEndDuringActionDefersShutdown() { + ESPStateMachine machine; + std::vector order; + + TransitionOptions options; + options.action = [&machine, &order](const TransitionContext &) { + order.push_back("action"); + machine.end(); + }; + + machine.addTransition(TestState::Idle, TestEvent::Start, TestState::Connecting, options); + machine.onExit(TestState::Idle, [&order](const StateCallbackContext &) { + order.push_back("exit"); + }); + machine.onEnter( + TestState::Connecting, + [&order](const StateCallbackContext &) { order.push_back("enter"); } + ); + machine.onTransition([&order](const TransitionContext &) { + order.push_back("observer"); + }); + machine.onExit(TestState::Connecting, [&order](const StateCallbackContext &ctx) { + if (ctx.shutdown) { + order.push_back("shutdown"); + } + }); + + machine.begin(TestState::Idle); + order.clear(); + + auto result = machine.dispatch(TestEvent::Start); + expectTrue(result.ok(), "end from action should still complete transition"); + expectFalse(machine.isStarted(), "end from action should stop after dispatch completes"); + expectEqual( + machine.currentState(), + TestState::Connecting, + "deferred shutdown should keep target state" + ); + expectOrder(order, {"exit", "action", "enter", "observer", "shutdown"}, "end from action order"); +} + +void testEndDuringEnterDefersShutdown() { + ESPStateMachine machine; + std::vector order; + + machine.addTransition(TestState::Idle, TestEvent::Start, TestState::Connecting); + machine.onExit(TestState::Idle, [&order](const StateCallbackContext &) { + order.push_back("exit"); + }); + machine.onEnter( + TestState::Connecting, + [&machine, &order](const StateCallbackContext &) { + order.push_back("enter"); + machine.end(); + } + ); + machine.onTransition([&order](const TransitionContext &) { + order.push_back("observer"); + }); + machine.onExit(TestState::Connecting, [&order](const StateCallbackContext &ctx) { + if (ctx.shutdown) { + order.push_back("shutdown"); + } + }); + + machine.begin(TestState::Idle); + order.clear(); + + auto result = machine.dispatch(TestEvent::Start); + expectTrue(result.ok(), "end from enter should still complete transition"); + expectFalse(machine.isStarted(), "end from enter should stop after dispatch completes"); + expectOrder(order, {"exit", "enter", "observer", "shutdown"}, "end from enter order"); +} + +void testEndDuringExitDefersShutdown() { + ESPStateMachine machine; + std::vector order; + + machine.addTransition(TestState::Idle, TestEvent::Start, TestState::Connecting); + machine.onExit(TestState::Idle, [&machine, &order](const StateCallbackContext &) { + order.push_back("exit"); + machine.end(); + }); + machine.onEnter( + TestState::Connecting, + [&order](const StateCallbackContext &) { order.push_back("enter"); } + ); + machine.onTransition([&order](const TransitionContext &) { + order.push_back("observer"); + }); + machine.onExit(TestState::Connecting, [&order](const StateCallbackContext &ctx) { + if (ctx.shutdown) { + order.push_back("shutdown"); + } + }); + + machine.begin(TestState::Idle); + order.clear(); + + auto result = machine.dispatch(TestEvent::Start); + expectTrue(result.ok(), "end from exit should still complete transition"); + expectFalse(machine.isStarted(), "end from exit should stop after dispatch completes"); + expectOrder(order, {"exit", "enter", "observer", "shutdown"}, "end from exit order"); +} + +void testEndDuringTransitionObserverDefersShutdown() { + ESPStateMachine machine; + std::vector order; + + machine.addTransition(TestState::Idle, TestEvent::Start, TestState::Connecting); + machine.onExit(TestState::Idle, [&order](const StateCallbackContext &) { + order.push_back("exit"); + }); + machine.onEnter( + TestState::Connecting, + [&order](const StateCallbackContext &) { order.push_back("enter"); } + ); + machine.onTransition([&machine, &order](const TransitionContext &) { + order.push_back("observer"); + machine.end(); + }); + machine.onExit(TestState::Connecting, [&order](const StateCallbackContext &ctx) { + if (ctx.shutdown) { + order.push_back("shutdown"); + } + }); + + machine.begin(TestState::Idle); + order.clear(); + + auto result = machine.dispatch(TestEvent::Start); + expectTrue(result.ok(), "end from observer should still complete transition"); + expectFalse(machine.isStarted(), "end from observer should stop after dispatch completes"); + expectOrder(order, {"exit", "enter", "observer", "shutdown"}, "end from observer order"); +} + +void testEndDuringRejectedObserverDefersShutdown() { + ESPStateMachine machine; + std::vector order; + + machine.onRejected([&machine, &order](const RejectedEventContext &) { + order.push_back("rejected"); + machine.end(); + }); + machine.onExit(TestState::Idle, [&order](const StateCallbackContext &ctx) { + if (ctx.shutdown) { + order.push_back("shutdown"); + } + }); + + machine.begin(TestState::Idle); + auto result = machine.dispatch(TestEvent::Reset); + + expectEqual( + result.status, + StateMachineDispatchStatus::NoTransition, + "rejected observer dispatch should reject" + ); + expectFalse(machine.isStarted(), "end from rejected observer should stop after observer completes"); + expectOrder(order, {"rejected", "shutdown"}, "end from rejected observer order"); +} + +void testBeginTwiceAndDispatchAfterEnd() { + ESPStateMachine machine; + + machine.addTransition(TestState::Idle, TestEvent::Start, TestState::Connecting); + expectTrue(machine.begin(TestState::Idle), "first begin should succeed"); + expectFalse(machine.begin(TestState::Online), "second begin should fail while started"); + + machine.end(); + auto result = machine.dispatch(TestEvent::Start); + expectEqual( + result.status, + StateMachineDispatchStatus::NotStarted, + "dispatch after end should be NotStarted" + ); + expectFalse(result.transitioned, "dispatch after end should not transition"); +} + +void testInactiveCallbacksStayInert() { + ESPStateMachine machine; + uint32_t enterCount = 0; + uint32_t exitCount = 0; + uint32_t transitionCount = 0; + uint32_t rejectedCount = 0; + + const StateMachineCallbackId enterId = machine.onEnter( + TestState::Idle, + [&enterCount](const StateCallbackContext &) { enterCount++; } + ); + const StateMachineCallbackId exitId = machine.onExit( + TestState::Idle, + [&exitCount](const StateCallbackContext &) { exitCount++; } + ); + const StateMachineCallbackId transitionId = machine.onTransition( + [&transitionCount](const TransitionContext &) { transitionCount++; } + ); + const StateMachineCallbackId rejectedId = machine.onRejected( + [&rejectedCount](const RejectedEventContext &) { rejectedCount++; } + ); + + expectTrue(machine.offCallback(enterId), "enter callback should deactivate"); + expectTrue(machine.offCallback(exitId), "exit callback should deactivate"); + expectTrue(machine.offCallback(transitionId), "transition observer should deactivate"); + expectTrue(machine.offCallback(rejectedId), "rejected observer should deactivate"); + + machine.addTransition(TestState::Idle, TestEvent::Start, TestState::Connecting); + machine.begin(TestState::Idle); + machine.dispatch(TestEvent::Start); + machine.dispatch(TestEvent::Reset); + + expectEqual(enterCount, static_cast(0), "inactive enter callback should stay inert"); + expectEqual(exitCount, static_cast(0), "inactive exit callback should stay inert"); + expectEqual(transitionCount, static_cast(0), "inactive transition observer should stay inert"); + expectEqual(rejectedCount, static_cast(0), "inactive rejected observer should stay inert"); +} + void testEventBusBridgeDispatchesBoundEvent() { ESPEventBus bus; ESPStateMachine machine; @@ -267,6 +495,60 @@ void testLoggerObserverLogsTransitionsAndRejections() { expectEqual(logger.warnCount, static_cast(1), "logger should record one rejection"); } +void testLoggerAttachFailureCanRetry() { + ESPLogger logger; + ESPStateMachine machine; + ESPStateMachineLoggerObserver observer; + + logger.init(); + machine.addTransition(TestState::Idle, TestEvent::Start, TestState::Connecting); + machine.begin(TestState::Idle); + expectFalse( + observer.attach(machine, logger, {"TEST_FSM", stateName, eventName, true}), + "logger observer attach should fail while machine is started" + ); + + machine.end(); + expectTrue( + observer.attach(machine, logger, {"TEST_FSM", stateName, eventName, true}), + "logger observer should retry cleanly after failed attach" + ); + + machine.begin(TestState::Idle); + machine.dispatch(TestEvent::Start); + machine.dispatch(TestEvent::Reset); + + expectEqual(logger.infoCount, static_cast(1), "retried logger should record transition"); + expectEqual(logger.warnCount, static_cast(1), "retried logger should record rejection"); +} + +void testLoggerDetachAndReattach() { + ESPLogger logger; + ESPStateMachine machine; + ESPStateMachineLoggerObserver observer; + + logger.init(); + machine.addTransition(TestState::Idle, TestEvent::Start, TestState::Connecting); + + expectTrue( + observer.attach(machine, logger, {"TEST_FSM", stateName, eventName, true}), + "logger observer should attach before detach" + ); + observer.detach(); + expectTrue( + observer.attach(machine, logger, {"TEST_FSM", stateName, eventName, true}), + "logger observer should reattach after detach" + ); + + machine.begin(TestState::Idle); + machine.dispatch(TestEvent::Start); + observer.detach(); + machine.dispatch(TestEvent::Reset); + + expectEqual(logger.infoCount, static_cast(1), "reattached logger should log transition once"); + expectEqual(logger.warnCount, static_cast(0), "detached logger should not log rejection"); +} + } // namespace int main() { @@ -276,10 +558,19 @@ int main() { testNoTransitionAndGuardRejected(); testGuardedBranchingOrder(); testCallbackOrderAndReentrantBusy(); + testEndDuringActionDefersShutdown(); + testEndDuringEnterDefersShutdown(); + testEndDuringExitDefersShutdown(); + testEndDuringTransitionObserverDefersShutdown(); + testEndDuringRejectedObserverDefersShutdown(); + testBeginTwiceAndDispatchAfterEnd(); + testInactiveCallbacksStayInert(); testEventBusBridgeDispatchesBoundEvent(); testTimerBridgeTimeoutAndStaleCancel(); testTimerBridgeFiresTimeout(); testLoggerObserverLogsTransitionsAndRejections(); + testLoggerAttachFailureCanRetry(); + testLoggerDetachAndReattach(); } catch (const std::exception &error) { std::cerr << "FAIL: " << error.what() << '\n'; return 1;