pgularski / pysm

Versatile and flexible Python State Machine library
http://pysm.readthedocs.io/
MIT License
73 stars 11 forks source link

Not returning to initial state after exit #9

Open wannesvanloock opened 4 years ago

wannesvanloock commented 4 years ago

Consider the following hierarchical state machine

from pysm import State, StateMachine, Event

def info_handler(f, info):
    def wrapper(self, event):
        print(f' >>> {info} state: {self.fully_qualified_name()}')
        return f(self, event)
    return wrapper

class GenericState(StateMachine):
    def __init__(self, name):
        super().__init__(name)

        self.handlers = {
            'enter': info_handler(self.on_enter, 'Enter'),
            'exit': info_handler(self.on_exit, 'Exit')
        }

    def fully_qualified_name(self):
        names = []
        state = self
        while state:
            names.insert(0, state.name)
            state = state.parent
        return ' > '.join(names)

    def on_enter(self, state, event):
        pass

    def on_exit(self, state, event):
        pass

    def send_event(self, name, **kwargs):
        self.root_machine.dispatch(Event(name, **kwargs))

class Move(GenericState):
    def __init__(self, name="move"):
        super().__init__(name)

    def on_enter(self, state, event):
        self.send_event('e_move_done')

class Jump(GenericState):
    def __init__(self, name="jump"):
        super().__init__(name)

    def on_enter(self, state, event):
        self.send_event('e_jump_done')

class MoveAndJump(GenericState):
    def __init__(self, name="move_and_jump"):
        super().__init__(name)

        move = Move()
        jump = Jump()

        self.add_state(move, initial=True)
        self.add_state(jump)
        self.add_transition(move, jump, events=['e_move_done'])

class WaitForResult(GenericState):
    def __init__(self, name="wait_for_result"):
        super().__init__(name)

    def on_enter(self, state, event):
        self.send_event('e_wait_done')

class Turn(GenericState):
    def __init__(self, name="turn"):
        super().__init__(name)
        self.count = 0
        move_and_jump = MoveAndJump()
        wait_for_result = WaitForResult()

        self.add_state(move_and_jump, initial=True)
        self.add_state(wait_for_result)

        self.add_transition(move_and_jump, move_and_jump, events=['e_jump_done'], condition=lambda s, e: self.count < 3, action=self.add)
        self.add_transition(move_and_jump, wait_for_result, events=['e_jump_done'], condition=lambda s, e: self.count == 3, action=self.add)

    def add(self, s, e):
        self.count += 1

    def on_exit(self, s, e):
        self.count = 0

class Idle(GenericState):
    def __init__(self, name="idle"):
        super().__init__(name)

    def on_enter(self, state, event):
        self.root_machine.initialize()

class Main(GenericState):
    def __init__(self, name="main"):
        super().__init__(name)

        idle = Idle()

        turn = Turn()

        self.add_state(idle, initial=True)
        self.add_state(turn)

        self.add_transition(idle, turn, events=['e_turn'])
        self.add_transition(turn, idle, events=['e_wait_done'])

        super().initialize()
        self.send_event('init')

if __name__=='__main__':
    s = Main()
    s.send_event('e_turn')  # Moves correctly through the states
    # At this point turn remains in wait_for_result and does not return to its initial state
    s.send_event('e_turn')  # Does not produce the correct transitions any more

As indicated above, after a first event e_turn, the turn state ends up in wait_for_result instead of its initial state. I can't find a workaround other than calling s.initialize() again to reset the entire state machine. This isn't a solution however as it should be called outside of the state machine.

Is there anything wrong with how I defined the state machine? If not, do you have any pointers on where things might be going wrong in pysm.

pgularski commented 4 years ago

Hi @wannesvanloock ,

Thanks for using pysm.

So, the culprit is here:

class WaitForResult(GenericState):
    def __init__(self, name="wait_for_result"):
        super().__init__(name)

    def on_enter(self, state, event):
        self.send_event('e_wait_done')   # << Here

You're triggering a transition (self.send_event('e_wait_done')) from within another transition action (on_enter) and therefore the state machine never finishes its transition sequence (Which again leads to a broken machine state).

To work around this limitation, basically run the self.send_event('e_wait_done') outside an action. In your case the full working code would be as follows:

from pysm import State, StateMachine, Event

def info_handler(f, info):
    def wrapper(self, event):
        print(f' >>> {info} state: {self.fully_qualified_name()}')
        return f(self, event)
    return wrapper

class GenericState(StateMachine):
    def __init__(self, name):
        super().__init__(name)

        self.handlers = {
            'enter': info_handler(self.on_enter, 'Enter'),
            'exit': info_handler(self.on_exit, 'Exit')
        }

    def fully_qualified_name(self):
        names = []
        state = self
        while state:
            names.insert(0, state.name)
            state = state.parent
        return ' > '.join(names)

    def on_enter(self, state, event):
        pass

    def on_exit(self, state, event):
        pass

    def send_event(self, name, **kwargs):
        self.root_machine.dispatch(Event(name, **kwargs))

class Move(GenericState):
    def __init__(self, name="move"):
        super().__init__(name)

    def on_enter(self, state, event):
        self.send_event('e_move_done')

class Jump(GenericState):
    def __init__(self, name="jump"):
        super().__init__(name)

    def on_enter(self, state, event):
        self.send_event('e_jump_done')

class MoveAndJump(GenericState):
    def __init__(self, name="move_and_jump"):
        super().__init__(name)

        move = Move()
        jump = Jump()

        self.add_state(move, initial=True)
        self.add_state(jump)
        self.add_transition(move, jump, events=['e_move_done'])

class WaitForResult(GenericState):
    def __init__(self, name="wait_for_result"):
        super().__init__(name)

    #  def on_enter(self, state, event):
    #      self.send_event('e_wait_done')

class Turn(GenericState):
    def __init__(self, name="turn"):
        super().__init__(name)
        self.count = 0
        move_and_jump = MoveAndJump()
        wait_for_result = WaitForResult()

        self.add_state(move_and_jump, initial=True)
        self.add_state(wait_for_result)

        self.add_transition(move_and_jump, move_and_jump, events=['e_jump_done'], condition=lambda s, e: self.count < 3, action=self.add)
        self.add_transition(move_and_jump, wait_for_result, events=['e_jump_done'], condition=lambda s, e: self.count == 3, action=self.add)

    def add(self, s, e):
        self.count += 1

    def on_exit(self, s, e):
        self.count = 0

class Idle(GenericState):
    def __init__(self, name="idle"):
        super().__init__(name)

    def on_enter(self, state, event):
        self.root_machine.initialize()

class Main(GenericState):
    def __init__(self, name="main"):
        super().__init__(name)

        idle = Idle()

        turn = Turn()

        self.add_state(idle, initial=True)
        self.add_state(turn)

        self.add_transition(idle, turn, events=['e_turn'])
        self.add_transition(turn, idle, events=['e_wait_done'])

        super().initialize()
        self.send_event('init')

if __name__=='__main__':
    s = Main()
    s.send_event('e_turn')  # Moves correctly through the states
    s.send_event('e_wait_done'). # The event is fired here... *
    # At this point turn remains in wait_for_result and does not return to its initial state
    print('---')
    s.send_event('e_turn')  # Does not produce the correct transitions any more
    s.send_event('e_wait_done') # * ... and here

It's a limitation I'm well aware of but I can't seem to find an easy solution for. I'm open for suggestions. I could register all events in a queue and fire them sequentially after a full transition is finished but I can see many other problems piling up already with such an approach.

And at the end of the day, I believe the machine should react to events coming from outside, and it should not trigger events itself. And I'd argue that again calling a transition from within the machine itself could be a design flaw. So, maybe I just should prevent users (by raising an exception) from running such code.

wannesvanloock commented 4 years ago

Thanks for the fast answer! Based on your answer, I tried triggering the event in on enter from a different thread. This way the state machine seems to work as expected.

class WaitForResult(GenericState):
    def __init__(self, name="wait_for_result"):
        super().__init__(name)

    def on_enter(self, state, event):
        threading.Thread(target=self.send_event, args=['e_wait_done']).start()

In my case, I have several hardware/software interfaces which are a member of the root machine. Based on the state of the machine, these interfaces interact and can dispatch events. In this design transitions are always triggered from within the machine. So I think it depends on your viewpoint whether calling a transition from within the machine is a design flaw.

pgularski commented 4 years ago

Neat approach - using threads, that is! Thanks for sharing!

pgularski commented 4 years ago

Come to think of it, looks like using threads without any synchronization creates a race condition here. And it seems to work for most of the times but it's not reliable.

wannesvanloock commented 4 years ago

Was also thinking about it some more. Race conditions should obviously be avoided. The reason for the wait for result state is due to a long running calculation. The better approach would be to kick off the computation in its own thread in the before action as shown below (which also behaves correctly). If in the meantime another event (e.g. failure) would happen the state machine should be able to handle this appropriately.

class WaitForResult(GenericState):
    def __init__(self, name="wait_for_result"):
        super().__init__(name)

class Turn(GenericState):
    def __init__(self, name="turn"):
        super().__init__(name)
        self.count = 0
        move_and_jump = MoveAndJump()
        wait_for_result = WaitForResult()

        self.add_state(move_and_jump, initial=True)
        self.add_state(wait_for_result)

        self.add_transition(move_and_jump, move_and_jump, events=['e_jump_done'], condition=lambda s, e: self.count < 3, action=self.add)
        self.add_transition(move_and_jump, wait_for_result, events=['e_jump_done'], condition=lambda s, e: self.count == 3, action=self.add, before=self.compute_result)

    def add(self, s, e):
        self.count += 1

    def compute_result(self, s, e):
        def long_computation():
            time.sleep(1)
            self.send_event('e_wait_done')
        threading.Thread(target=long_computation).start()