v2.2.0

[Thelmos]

• Add on_state() handler to states
• New run_machine() method to invoke machine execution (includes a
  check_timed_transitions() call)
• New timed_switchoff.ino example sketch to ilustrate new on_state()
  and run_machine() funcionality
• Corrections:
  • make_transition() correctly initialices timed transitions start
    milliseconds (make_transition() is now a fsm method)
  • Initial state on_enter() handler is now correctly executed on fsm
    first run
  • Removed Serial.println(now); trace in Fsm.cpp
  • Correct initialization of m_num_timed_transitions

[Thelmos]

Solves Issues: #10, #5 and may be #3 (trigger can be called in on_state() instead of in on_enter())
Includes Pull requests: #8, #7, #5

[jonblack]

@Thelmos Great job! Did you mean to add a copy of Fsm.cpp to the timed_switchoff example?

[jonblack]

@Thelmos Also, in the light_switch example you don't call run_machine under the assumption that since on_state isn't being used, it's not required; however, m_initialized defaults to false and will never change, so transitions will never be triggered.

I suggest we require at least one call to run_machine, which can either go in setup or loop depending on the user's requirements. What do you think? Do you have a better idea?

[Thelmos]

Mmmm, you are right, run_machine() is now the main library method so it is a nice idea to require at least one call, in fact it also runs on_enter() for first state.

Regarding the Fsm.cpp in the timed_switchoff example can be removed, was used for testing.

[Thelmos]

I may have had a good idea, ...

What if we replace all trigger(event) calls with run_machine(event) calls, then in run_machine(event) we can call on_state() for curren state , check timed transitions and finally call trigger(event) (this last one only if no timed transition is launched and an event is provided to run_machine()).

This would have several benefits:

• We have only one API execution method to handle all FSM operation.
• A FSM with arduino interrupt only events and no timed transitions can call run_machine(event) in event handlers with no need for a call in main loop.
• Timed transitions would be more acurate due to the fact that run_machine() would be called more frecuently.

As a drawback run_machine(event) call would be less eficient than current trigger(event) call, but I think it's worth yet.

What do you think about this idea?

[corna]

Why have you squashed all the PR's commits? By doing this you have removed the git history and the original authors of #8, #7 and #5.

[Thelmos]

This library is usefull for my projects, but I need some changes to improve some aspects ... So I made a fork and coded some changes, some of that changes were in onther PR but not applied in the way I needed.

I made a PR in case library owner wants to merge my changes in main repository, it's up to @jonblack decide what PRs he accepts, as it's up to me what PR I made.

[jonblack]

What if we replace all trigger(event) calls with run_machine(event) calls, then in run_machine(event) we can call on_state() for curren state , check timed transitions and finally call trigger(event) (this last one only if no timed transition is launched and an event is provided to run_machine()).

@Thelmos I was also thinking about this but I'm beginning to favour splitting it up more so that the methods have a clear responsibilities:

• init_machine should only call on_enter and on_state (if present). We make it clear in documentation that this goes in setup. It's always required.
• check_machine should call on_state (if present) for the current state and check_timed_transitions. We make it clear in documentation that this goes in loop. It's always required. Assert that init_machine has been called.
• trigger manually invokes a state change. We make it clear in the documentation that trigger can never be called from on_enter, on_transition, and on_exit; it can be called in on_state. Assert that init_machine has been called.

The problem I have with a single run_machine interface is that it's not as clear what it's doing if you just look at the call in the client.

@redge76 As far as I can tell, this solution would work for you as well since trigger can be called in on_state, which is where you'd handle your messages. on_state itself is called from check_machine, removing the trigger recursion. You mentioned that you want to avoid putting things in loop, but didn't give an explanation. I'd love to hear it.

This library is usefull for my projects, but I need some changes to improve some aspects ... So I made a fork and coded some changes, some of that changes were in onther PR but not applied in the way I needed.

@Thelmos How were the changes in other PR's not applied in the way you needed? If there's a difference it's worth discussing so everyone is happy.

I agree it's a shame the other pull request commit history isn't included.

[Thelmos]

I think you are right, simplifying interface makes it more obscure and less flexible.

Regarding changes in other PR, I don't like for example how the correction for timed transitions was done in corna@b5b4a46 Looks more natural to me sove this in make_transition() than creating a new function update_timed_transitions_starts() and call it in three diferent places.

I was in a hurry with my project, it is now almost finished using my fork, thats why I implement all changes so fast, but feel free to reject my PR, your idea looks better than my run_machine() only solution ;)

[redge76]

I would like to avoid because you have to wait for some loop execution before arriving to the destination state. but I think I can live with this.
For example, if I need to have a A->B-> transition with B directly redirecting to C, If you can call trigger in the enter() callback of B, your machine is never really in the B state.
With the on_state() solution, the machine is in B state for a very little moment...(1 loop execution)
I don't know if it's a big deal.
If the B state is where you display, something, then you can't setup the display in the enter() function because you never know if you won't need to go to another state in the on_state() callback.
Otherwise the screen will flicker

[Thelmos]

You are right @redge76, I have found some other situations where a "decoupled" trigger() call would be nice, if your project relays on arduino interrups for event handling, calling trigger() can be a bad idea, as it can be very time consumming depending on your on_transition() and on_enter() functions.

Perhaps trigger() must only "mark" transitions to be fired and return as fast as posible, and then check_machine() resolve the transition and state change. This way trigger() function would be also reentrant.

[jonblack]

I've made the changes I mentioned in the branch https://github.com/jonblack/arduino-fsm/tree/Thelmos-master. I wanted to merge it into this pull-request but couldn't find out how (if it's even possible). If anyone knows how, please fill me in.

The update contains:

• A process method, which is the same as check_machine but with a different name;
• An init method, which is the same as init_machine but with a different name;
• check_timed_transitions is now private;
• All examples have been updated.

@Thelmos I was going to implement "marking transitions" with trigger but wondered if it'd cause problems. Would we need to maintain the order in which transitions are triggered? If so, we'd need a queue to manage it rather than marking transitions. Is this overkill? Would the queue end up so large it'd affect performance?

I do like the idea of trigger marking transitions and process performing them. process is already doing this for timed transitions anyway, so it fits nicely.

[Thelmos]

I will take a look at your branch asap.

Regarding trigger() function... I think we can keep it simple and make it store only last triggered event that could fire a transition from current state, so next process() call will perform that transition.

We also need to decide what process() function will check in first place, timed transitions or triggered events, ... I would go for timed transitions.

[jonblack]

@Thelmos I agree it's better to keep it simple. Let's store the last transition, then. I also agree that timed transitions should be processed first.

[bjoernhaeuser]

I wanted to merge it into this pull-request but couldn't find out how (if it's even possible). If anyone knows how, please fill me in.

That is not easily possible. You would need to create a patch and move that into into your own repository. :-(

[jonblack]

Yeah, since that comment I've come across this a lot. It's kind of a shame you can't collaborate on pull requests. For now I've been pulling locally, making changes that need adding, and then merging into master and pushing.

[neryortez]

Is version 2.2.0 working ?

[jonblack]

Give it a try and see. I haven't looked at this in a while. I'm hoping to go through all these issues soon as I'll have some free time on my hands. If you find something wrong in the meantime, it'd be a great help.

[redge76]

Hi,
Just a message to address a big "thank you" to jonblack and thelmos. The v2.2 is just perfect.
About my stupid idea of calling trigger in a on_enter in #3 . I just use a transition to the same state with my code in a on_transition function. With this I can do some sort of event-driven programming.
The only thing I would need is a get_state_name or get_state_ID to be able to check what the current state is. (current workaround is to have a global variable updated in the on_enter() of each state :-/)

You really need to merge this pull request to the master branch. (or just delete everything in master and push all the files of thelmos ) Currently the version distributed in platformio and arduino is 2.1 :-(

[neryortez]

I'm playing with it to see if it works alright...

You really need to merge this pull request to the master branch. (or just delete everything in master and push all the files of thelmos )

What about the changes on the Thelmos-master branch?
I think it'd be better to merge that branch to the master, since @jonblack has already worked on that.

[redge76]

What about the changes on the Thelmos-master branch?

I mean to "copy" all files from thelmos to master.
Yes it's better to merge but @jonblack said previously : " I wanted to merge it into this pull-request but couldn't find out how "

[jonblack]

I'm going through both my arduino libraries now and updating them. After next week, I have a bit of time to work on it. I'm sorry this is taking so long. I'll get there :)