digint / tinyfsm

A simple C++ finite state machine library
https://digint.ch/tinyfsm
MIT License
945 stars 174 forks source link

Calling dispatch() and/or transit() from action function #38

Open MarioIvancic opened 1 year ago

MarioIvancic commented 1 year ago

In the comment for transit function there is a note: do not send events in action_function definisions. As I understand, that limitation is bacause of order of operations inside transit() functions:

current_state_ptr->exit();
action_function();
current_state_ptr = &_state_instance<S>::value;
current_state_ptr->entry();

Any state change caused by action_function() will be overriden by the next line.

What if we rearrange operations so that action is last:

current_state_ptr->exit();
current_state_ptr = &_state_instance<S>::value;
current_state_ptr->entry();
action_function();

Now we can call all API functions without limitation. Any state change caused by action_function() will count as any other. What do you think?

franzrammerstorfer commented 1 year ago

I think the order is correct, since the action represents the transition, hence the call is 'between' the state change. But I have an issue as well. In case the action function fails, I want to transit to 'FAILED' state via 'send_event'. This will be overridden... How to deal with that?

hlovdal commented 1 year ago

You can send events in action functions, this is already given as an example in doc/40-Usage.md:

$ git blame -- doc/40-Usage.md | sed -n 151,166p
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 151)     void Idle::react(Call const & e) {
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 152)       dest_floor = e.floor;
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 153)
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 154)       if(dest_floor == current_floor)
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 155)         return;
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 156)
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 157)       /* lambda function used for transition action */
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 158)       auto action = [] {
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 159)         if(dest_floor > current_floor)
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 160)           send_event(MotorUp());
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 161)         else if(dest_floor < current_floor)
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 162)           send_event(MotorDown());
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 163)       };
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 164)
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 165)       transit<Moving>(action);
c0160d77 doc/30-Usage.txt (Axel Burri 2012-11-26 19:32:25 +0100 166)     };

and as you can see it has been documented as supported since 2012-11-26. So the issue is just that the code comment has not updated correspondingly. Looking at the history, the comment was last modified in 2022 ...

$ git blame -- include/tinyfsm.hpp | sed -n 141,149p
^f97cc16 (Axel Burri 2012-11-23 18:30:12 +0100 141)     template<typename S, typename ActionFunction>
^f97cc16 (Axel Burri 2012-11-23 18:30:12 +0100 142)     void transit(ActionFunction action_function) {
b3967257 (Axel Burri 2018-09-18 19:26:44 +0200 143)       static_assert(is_same_fsm<F, S>::value, "transit to different state machine");
1c4d9034 (Axel Burri 2018-04-20 16:18:12 +0200 144)       current_state_ptr->exit();
9bd0af08 (Axel Burri 2022-06-11 13:43:57 +0200 145)       // NOTE: do not send events in action_function definisions.
^f97cc16 (Axel Burri 2012-11-23 18:30:12 +0100 146)       action_function();
dde1d497 (Axel Burri 2018-05-09 19:40:41 +0200 147)       current_state_ptr = &_state_instance<S>::value;
1c4d9034 (Axel Burri 2018-04-20 16:18:12 +0200 148)       current_state_ptr->entry();
^f97cc16 (Axel Burri 2012-11-23 18:30:12 +0100 149)     }

... however it actually originates from 2012-11-23, 3 days earlier:

$ git blame 9bd0af08^ -- include/tinyfsm.hpp | sed -n 141,150p
^f97cc16 (Axel Burri 2012-11-23 18:30:12 +0100 141)     template<typename S, typename ActionFunction>
^f97cc16 (Axel Burri 2012-11-23 18:30:12 +0100 142)     void transit(ActionFunction action_function) {
b3967257 (Axel Burri 2018-09-18 19:26:44 +0200 143)       static_assert(is_same_fsm<F, S>::value, "transit to different state machine");
1c4d9034 (Axel Burri 2018-04-20 16:18:12 +0200 144)       current_state_ptr->exit();
^f97cc16 (Axel Burri 2012-11-23 18:30:12 +0100 145)       // NOTE: we get into deep trouble if the action_function sends a new event.
^f97cc16 (Axel Burri 2012-11-23 18:30:12 +0100 146)       // TODO: implement a mechanism to check for reentrancy
^f97cc16 (Axel Burri 2012-11-23 18:30:12 +0100 147)       action_function();
dde1d497 (Axel Burri 2018-05-09 19:40:41 +0200 148)       current_state_ptr = &_state_instance<S>::value;
1c4d9034 (Axel Burri 2018-04-20 16:18:12 +0200 149)       current_state_ptr->entry();
^f97cc16 (Axel Burri 2012-11-23 18:30:12 +0100 150)     }

However commit f97cc162 is the very first commit in this repository ("initial import") so its origin is most likely older than that.

franzrammerstorfer commented 1 year ago

You may send events to other state machines (like the MotorUp() / MotorDown()) in your comment, but it is not working if you send events to the underlying state machine.