Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

State timeouts? #198

Closed
Eric24 opened this issue Mar 11, 2017 · 15 comments
Closed

State timeouts? #198

Eric24 opened this issue Mar 11, 2017 · 15 comments

Comments

@Eric24
Copy link

Eric24 commented Mar 11, 2017

It's a very interesting project with a lot of rich functionality. One of the the things I was looking for in an FSM was the ability to automatically transition to a specified state after a period of time in the current state. For example, the FMS is in "tank-filling" state, waiting for an external sensor to tell me the tank is full and move on to "tank-full" state, but if that external event doesn't happen within 30 seconds, I instead want to move to "tank-fill-error" state. Of course I could implement something like this with an external timer that fires after 30 seconds and triggers a transition from "tank-filling" to "tank-fill-error", but it seems like the use-case for this sort of thing is common enough to be built in--essentially, for any state transition, the ability to pass an optional "timeout state" and timeout value. I'd love to hear your thoughts on this.

@aleneum
Copy link
Member

aleneum commented Mar 13, 2017

Hi @Eric24,

I also use timeouts in my projects but in my oppinion this should be handled by the model or the specific state. Let's see if others may have another view on this.

Until then, a straight forward way to do it in the model could look like this:

from threading import Thread
import time
from transitions import Machine


class Timeout(Thread):

    def __init__(self, func, timeout):
        super(Timeout, self).__init__()
        self.func = func
        self.timeout = timeout
        self.canceled = False
        self.start()

    def run(self):
        time.sleep(self.timeout/1000.0)
        if not self.canceled:
            self.func()


class Model:

    def __init__(self):
        self.timer = None

    def on_enter_B(self):
        self.timer = Timeout(self.timeout, 1000)

    def on_exit_B(self):
        self.timer.canceled = True


model = Model()

transitions = [['timeout', 'B', 'C']]
machine = Machine(model, states=['A', 'B', 'C'], transitions=transitions, initial='A')
model.to_B()
print(model.state)  # >>> B
time.sleep(1.1)
print(model.state)  # >>> C

or as part of a subclassed State which allows easier individual timeout handling:

class TimeoutState(State):

    def __init__(self, timeout):
        self.timeout = timeout

b = TimeoutState('B', enter='set_timeout')
m = Machine(model, states=['A', b, 'C']) ...

@Eric24
Copy link
Author

Eric24 commented Mar 13, 2017

Yes, that's essentially my current approach. I'm curious as to why you feel that this shouldn't be part of the state machine "internals"? To me, it seems like "core functionality", and as such, why trouble the developer to handle their own timeouts explicitly? For my purposes, having 30 or more states, most with timeouts, it seems like a lot of unnecessary boilerplate.

@aleneum
Copy link
Member

aleneum commented Mar 28, 2017

For my purposes, having 30 or more states ... it seems like a lot of unnecessary boilerplate.

If you subclass Machine, the extra lines of code scale quite well for many states:

class TimeoutState(State):

    def __init__(self, timeout=None, *args, **kwargs):
        self.timeout = timeout 
        super(TimeoutState, self).__init__(*args, **kwargs)

    def enter(self, event_data):
        # initialise timeout here

class TimeoutMachine(Machine):

    def _create_state(self, *args, **kwargs):
        return TimeoutState(*args, **kwargs)

states = [{'name': 'A', 'timeout': 1000},
          {'name': 'B', 'timeout': 2000'},...]

machine = TimeoutMachine(states=states)

I'm curious as to why you feel that this shouldn't be part of the state machine "internals"?

I do not consider timeouts as a core functionality. However, I do think this might be a valuable addition to transitions as part of a model/state extension. We collect ideas in #146 for that module. I will definitely add your suggestion there and in case it receives support/feedback by the community, we will discuss how it can be added to transitions.

@valste
Copy link

valste commented Apr 6, 2017

Very good idea! -> upvote

@valste
Copy link

valste commented Apr 7, 2017

Me again guys :)

Can't get run the above code.
Please take a look at http://stackoverflow.com/questions/43275092

What the ... ;)

Thanks!

@valste
Copy link

valste commented May 15, 2017

My next try to implement default timouts using def _create_state(self, *args, **kwargs) fails permanently on exiting the states.

Please see the issue on stack http://stackoverflow.com/questions/43937365/setting-and-canceling-of-predefined-timeouts-for-hierarchical-state-machine-fail

Thnx!

@bedge
Copy link

bedge commented Jun 2, 2017

I got here looking for the same thing. Built-in support for timeouts would make for less overall LOC in the client apps.
Great idea

@bedge
Copy link

bedge commented Jun 6, 2017

Just to add my $.02, I ended up just using threading.Timers:

    def timer_poll_start(self, event):
        self.timer_poll = Timer(poll_interval, self.timer_poll)
        self.timer_poll.start()

    def timer_poll_cancel(self, event):
        self.timer_poll.cancel()

@aleneum
Copy link
Member

aleneum commented Jul 5, 2017

We plan to ship timeouts with transitions 0.6.0. See #229 or the whole dev-state-extension branch for further info.

@bedge
Copy link

bedge commented Jul 5, 2017

What restrictions does this place on code that uses timeouts? Presumably it needs threads, which precludes the ability to use joblib in the same process (with python 2.x only anyway)

I've already solved this problem by separating my concurrent code in a different process, but I'm curious if the implementation has the same restrictions as I ran into.

@aleneum
Copy link
Member

aleneum commented Jul 5, 2017

Presumably it needs threads

Correct. It actually uses Timer similar to your code snippet above.

but I'm curious if the implementation has the same restrictions as I ran into

Yeah, it does. I am not aware of any convenient solution which allows timeouts without threads 🤔.

I could think of two threadless solutions but both have major issues:

a) Using async forces the event loop on the user
b) Using polls/ticks allows synchronised execution but is not really convenient to use. This way the user has more or less to implement his/her own event loop and poll the machine/model regularly or the event won't trigger in time.

@bedge: What's your lesson learned for timeouts/threading? Should transitions Timeout may support both approaches? Threads and polling? Or do you know about a better third way?

@bedge
Copy link

bedge commented Jul 5, 2017

I believe that the threading restrictions are mainly python <3 only and that >3 has fixed many of the threading problems and inconsistencies.
That said, I've been stuck in a 2.7 world so my experience with 3.x is limited to lusting while window shopping. However, I will be attempting a 3.x migration in the near future so I'll keep this in mind during that process.
In short, I found the addition of timers to the model to be of sufficient benefit to outweigh the threading impact drawback.
The only thing I would suggest is that if a user isn't using transitions timers, the timer support should not interfere with using joblib of other threading APIs.

@aleneum
Copy link
Member

aleneum commented Jul 6, 2017

The only thing I would suggest is that if a user isn't using transitions timers, the timer support should not interfere with using joblib of other threading APIs.

well, that should not be the case except joblib already complains when the threading module is imported. Timers are only created when Timeout is explicitly used.

@bedge
Copy link

bedge commented Jul 10, 2017

While it's unfortunate, it's not really transitions' fault that the python 2.x threading has issues.
It's probably good practice to keep the state machine in a separate executable anyway. Anything that needs massive concurrency should be popened as a new process. (IMO)

@aleneum
Copy link
Member

aleneum commented Aug 20, 2017

It's probably good practice to keep the state machine in a separate executable anyway

agree, from what I heard in my department so far, one should use a single threaded state machine where ever possible. Closing this issue now since 0.6.0 has been merged with master and also pushed to pypi.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants