Use preorder recursion to remove need for Dispatch queue

project-chip · Nov 26, 2021 · a841f37 · a841f37
1 parent c23c8e0
commit a841f37
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 45 deletions.
diff --git a/src/lib/support/StateMachine.h b/src/lib/support/StateMachine.h
@@ -18,7 +18,6 @@
 
 #pragma once
 
-#include <deque>
 #include <lib/core/Optional.h>
 #include <lib/support/Variant.h>
 
@@ -128,8 +127,6 @@ class Context
 
     /**
      * Dispatch an event to the current state.
-     * @note This call can result in the current state being deleted.  Do not
-     * access current state memory after calling this method.
      * @param evt a variant holding an Event for the State Machine.
      */
     virtual void Dispatch(const TEvent & evt) = 0;
@@ -167,8 +164,6 @@ class Context
  *     const char *GetName() { return ""; }
  *   }
  * @endcode
- * @note The State::Enter() method is allowed to Dispatch events.  However, dispatch calls
- * from State::Exit() are ignored.
  *
  * The TTransitions table type is implemented with an overloaded callable operator method
  * to match the combinations of State / Event variants that may produce a new-state return.
@@ -195,8 +190,6 @@ class Context
  *     }
  *   }
  * @endcode
- * @note The Transition table is allowed to Dispatch events.  However,
- * dispatched events will be ignored if a new state transition is returned.
  *
  * @tparam TState a variant holding the States.
  * @tparam TEvent a variant holding the Events.
@@ -210,41 +203,19 @@ class StateMachine : public Context<TEvent>
     ~StateMachine() override = default;
     void Dispatch(const TEvent & evt) override
     {
-        auto inProcess = !events.empty();
-        events.push_back(evt);
-        if (!inProcess)
+        auto newState = mTransitions(mCurrentState, evt);
+        if (newState.HasValue())
         {
-            HandleEvents();
+            auto oldState = mCurrentState;
+            oldState.Exit();
+            mCurrentState = newState.Value();
+            mCurrentState.LogTransition(oldState.GetName());
+            mCurrentState.Enter();
         }
     }
 
     TState mCurrentState;
-
-private:
-    void HandleEvents()
-    {
-        while (!events.empty())
-        {
-            auto count    = events.size();
-            auto optState = mTransitions(mCurrentState, events.front());
-            if (optState.HasValue())
-            {
-                auto newState = optState.Value();
-                newState.LogTransition(mCurrentState.GetName());
-                mCurrentState.Exit();
-                mCurrentState = newState;
-                while (events.size() > count)
-                {
-                    events.pop_back(); // events discarded per design
-                }
-                mCurrentState.Enter();
-            }
-            events.pop_front();
-        }
-    }
-
     TTransitions & mTransitions;
-    std::deque<TEvent> events{};
 };
 
 } // namespace StateMachine

diff --git a/src/lib/support/tests/TestStateMachine.cpp b/src/lib/support/tests/TestStateMachine.cpp
@@ -117,8 +117,8 @@ struct Transitions
         }
         else if (state.Is<State2>() && event.Is<Event4>())
         {
-            // illegal - Returned Transition will cause events
-            // dispatched from the transitions table to be ignored.
+            // Potentially unintended behavior - dispatches an event,
+            // but terminates in State1 anyway.
             mCtx.Dispatch(Event::Create<Event2>());
             return mFactory.CreateState1();
         }
@@ -176,7 +176,7 @@ void TestTransitions(nlTestSuite * inSuite, void * inContext)
     NL_TEST_ASSERT(inSuite, fsm.mStateMachine.mCurrentState.Is<State1>());
 }
 
-void TestTransitionsDispatch(nlTestSuite * inSuite, void * inContext)
+void TestDispatchFromTransitionsTable(nlTestSuite * inSuite, void * inContext)
 {
     // in State1
     SimpleStateMachine fsm;
@@ -186,17 +186,16 @@ void TestTransitionsDispatch(nlTestSuite * inSuite, void * inContext)
     NL_TEST_ASSERT(inSuite, fsm.mStateMachine.mCurrentState.Is<State2>());
 }
 
-void TestIllegalDispatch(nlTestSuite * inSuite, void * inContext)
+void TestDispatchAndTransitioinFromTransitionTable(nlTestSuite * inSuite, void * inContext)
 {
     // in State1
     SimpleStateMachine fsm;
     // transition to State2
     fsm.mStateMachine.Dispatch(Event::Create<Event2>());
     NL_TEST_ASSERT(inSuite, fsm.mStateMachine.mCurrentState.Is<State2>());
     // Dispatch Event4, which dispatches Event2 and transitions to State1.
-    // The transition is processed first.  If a transition has occurred,
-    // subsequent events are ignored.  In this case, this means we remain
-    // in State1.  Dispatching Event2 will not have an effect.
+    // Transition to State1 occurs after dispatch of Event2, and so this
+    // is where we ultimately arrive.
     fsm.mStateMachine.Dispatch(Event::Create<Event4>());
     NL_TEST_ASSERT(inSuite, fsm.mStateMachine.mCurrentState.Is<State1>());
 }
@@ -247,8 +246,8 @@ static const nlTest sTests[] = {
     NL_TEST_DEF("TestInit", TestInit),
     NL_TEST_DEF("TestIgnoredEvents", TestIgnoredEvents),
     NL_TEST_DEF("TestTransitions", TestTransitions),
-    NL_TEST_DEF("TestTransitionsDispatch", TestTransitionsDispatch),
-    NL_TEST_DEF("TestIllegalDispatch", TestIllegalDispatch),
+    NL_TEST_DEF("TestDispatchFromTransitionsTable", TestDispatchFromTransitionsTable),
+    NL_TEST_DEF("TestDispatchAndTransitioinFromTransitionTable", TestDispatchAndTransitioinFromTransitionTable),
     NL_TEST_DEF("TestMethodExec", TestMethodExec),
     NL_TEST_SENTINEL(),
 };