fix: Inner external transition overwrites outer internal transition (#…

…140)
erikzenker · Jan 25, 2021 · c99cfdb · c99cfdb
1 parent e7b4927
commit c99cfdb
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 23 deletions.
diff --git a/src/include/hsm/details/sm.h b/src/include/hsm/details/sm.h
@@ -282,8 +282,8 @@ template <class RootState, class... OptionalParameters> class sm {
     void fill_dispatch_table(OptionalParameters&... optionalParameters)
     {
         auto optionalDependency = bh::make_basic_tuple(std::ref(optionalParameters)...);
-        fill_dispatch_table_with_external_transitions(rootState(), m_statesMap, optionalDependency);
         fill_dispatch_table_with_internal_transitions(rootState(), m_statesMap, optionalDependency);
+        fill_dispatch_table_with_external_transitions(rootState(), m_statesMap, optionalDependency);
         fill_dispatch_table_with_deferred_events(rootState(), optionalDependency);
     }
 };

diff --git a/src/include/hsm/gen/hsm.h b/src/include/hsm/gen/hsm.h
@@ -1265,11 +1265,11 @@ template <class State> constexpr auto collect_child_states(State state)
 #include <boost/hana/basic_tuple.hpp>
 #include <boost/hana/equal.hpp>
 #include <boost/hana/filter.hpp>
+#include <boost/hana/functional/capture.hpp>
 #include <boost/hana/not.hpp>
 #include <boost/hana/prepend.hpp>
 #include <boost/hana/size.hpp>
 #include <boost/hana/transform.hpp>
-#include <boost/hana/functional/capture.hpp>
 
 namespace hsm {
 
@@ -1344,16 +1344,17 @@ constexpr auto get_internal_transitions = [](auto states) {
         bh::transform(
             states,
             [](auto parentState) {
-                constexpr auto extend = bh::capture(parentState)([](auto parentState, auto transition) {
-                    // Folowing lines satisfies older gcc -Werror=unused-but-set-parameter   
-                    (void) transition;
-                    if constexpr (has_transition_table(parentState)) {
-                        return extend_internal_transition(
-                            transition, collect_child_states(parentState));
-                    } else {
-                        return bh::make_basic_tuple();
-                    }
-                });
+                constexpr auto extend
+                    = bh::capture(parentState)([](auto parentState, auto transition) {
+                          // Folowing lines satisfies older gcc -Werror=unused-but-set-parameter
+                          (void)transition;
+                          if constexpr (has_transition_table(parentState)) {
+                              return extend_internal_transition(
+                                  transition, collect_child_states(parentState));
+                          } else {
+                              return bh::make_basic_tuple();
+                          }
+                      });
 
                 return bh::transform(get_internal_transition_table(parentState), extend);
             }),
@@ -2183,8 +2184,8 @@ template <class RootState, class... OptionalParameters> class sm {
     void fill_dispatch_table(OptionalParameters&... optionalParameters)
     {
         auto optionalDependency = bh::make_basic_tuple(std::ref(optionalParameters)...);
-        fill_dispatch_table_with_external_transitions(rootState(), m_statesMap, optionalDependency);
         fill_dispatch_table_with_internal_transitions(rootState(), m_statesMap, optionalDependency);
+        fill_dispatch_table_with_external_transitions(rootState(), m_statesMap, optionalDependency);
         fill_dispatch_table_with_deferred_events(rootState(), optionalDependency);
     }
 };

diff --git a/test/integration/internal_transition.cpp b/test/integration/internal_transition.cpp
@@ -93,8 +93,7 @@ struct MainState {
         // clang-format off
         return hsm::transition_table(
             * hsm::state<S1> + hsm::event<e1> = hsm::state<S2>
-            , hsm::state<S1> + hsm::event<e2> = hsm::state<S2>
-            , hsm::state<S1> + hsm::event<e7> / hsm::log = hsm::state<S2>
+            , hsm::state<S1> + hsm::event<e7> = hsm::state<S2>
             , hsm::state<S1> + hsm::event<e3> = hsm::state<SubState>
             , hsm::state<S2> + hsm::event<e6> = hsm::state<S1>
         );
@@ -105,9 +104,9 @@ struct MainState {
     {
         // clang-format off
         return hsm::transition_table(
-            + (hsm::event<e1> / hsm::log),
+            + (hsm::event<e1> ),
             + (hsm::event<e2> / action),
-            + (hsm::event<e5> [fail]),
+            + (hsm::event<e5> [fail] / hsm::log),
             + (hsm::event<e4> [fail] / action)
         );
         // clang-format on
@@ -120,19 +119,19 @@ class InternalTransitionTests : public Test {
     hsm::sm<MainState> sm;
 };
 
-TEST_F(InternalTransitionTests, should_overwrite_external_by_internal_transition)
+TEST_F(InternalTransitionTests, should_overwrite_internal_by_inner_external_transition)
 {
     ASSERT_TRUE(sm.is(hsm::state<S1>));
     sm.process_event(e1 {});
-    ASSERT_TRUE(sm.is(hsm::state<S1>));
+    ASSERT_TRUE(sm.is(hsm::state<S2>));
 }
 
-TEST_F(InternalTransitionTests, should_overwrite_external_by_internal_transition_in_substate)
+TEST_F(InternalTransitionTests, should_overwrite_internal_by_inner_transition_in_substate)
 {
     sm.process_event(e3 {});
     ASSERT_TRUE(sm.is(hsm::state<SubState>, hsm::state<S1>));
     sm.process_event(e1 {});
-    ASSERT_TRUE(sm.is(hsm::state<SubState>, hsm::state<S1>));
+    ASSERT_TRUE(sm.is(hsm::state<SubState>, hsm::state<S2>));
 }
 
 TEST_F(InternalTransitionTests, should_call_action_on_internal_transition)
@@ -170,8 +169,7 @@ TEST_F(InternalTransitionTests, should_guard_internal_transition_with_action)
 }
 
 TEST_F(
-    InternalTransitionTests,
-    DISABLED_should_not_overwrite_substate_transition_by_parent_internal_transition)
+    InternalTransitionTests, should_not_overwrite_substate_transition_by_parent_internal_transition)
 {
     sm.process_event(e3 {});
     ASSERT_TRUE(sm.is(hsm::state<SubState>, hsm::state<S1>));