pytransitions / transitions

A lightweight, object-oriented finite state machine implementation in Python with many extensions
MIT License
5.5k stars 525 forks source link

get_triggers / get_transitions with HierarchicalMachine #449

Closed alexandretanem closed 3 years ago

alexandretanem commented 4 years ago

Taking the example from the Readme "Reuse of previously created HSMs", i tried to print the result of get_triggers on the parent Machine, for each state.

Is it the expected behaviour ? Wouldn't it be interesting to also have children's possible triggers ?

from transitions.extensions import HierarchicalMachine as Machine

count_states = ['1', '2', 'done']
count_trans = [
    ['increase', '1', '2'],
    ['decrease', '2', '1'],
    ['done', '2', 'done'],
    ['reset', '*', '1']
]

counter = Machine(states=count_states, transitions=count_trans, initial='1')

counter.increase()
states = ['waiting', 'collecting', {'name': 'counting', 'children': counter}]

transitions = [
    ['collect', 'waiting', 'collecting'],
    ['wait', ['collecting','counting_done'], 'waiting'],
    ['count', 'collecting', 'counting']
]

def print_state_trig(machine):
    print("State : "+machine.state+" > "+str(machine.get_triggers(machine.state)))

collector = Machine(states=states, transitions=transitions, initial='waiting', auto_transitions=False)

print_state_trig(collector)
collector.collect()
print_state_trig(collector)
collector.count()
print_state_trig(collector)
collector.increase()
print_state_trig(collector)
collector.done()
print_state_trig(collector)
collector.wait()

# Output : 
# State : waiting > ['collect']
# State : collecting > ['wait', 'count']
# State : counting_1 > []       ## Expected : State : counting_1 > ['increase', 'reset']
# State : counting_2 > []       ## Expected : State : counting_2 > ['decrease', 'done', 'reset']
# State : counting_done > ['wait']
aleneum commented 3 years ago

Hi @alexandretanem

thank you for that bug report.

Is it the expected behaviour ?

absolutely not!

Wouldn't it be interesting to also have children's possible triggers ?

absolutely!

I hope 9b61da9 solved that issue. I haven't checked trigger name resolution with deeply nested transitions but your example (with counter = (... auto_transitions=True)) returns the expected result:

from transitions.extensions import HierarchicalMachine as Machine

count_states = ['1', '2', 'done']
count_trans = [
    ['increase', '1', '2'],
    ['decrease', '2', '1'],
    ['done', '2', 'done'],
    ['reset', '*', '1']
]

counter = Machine(states=count_states, transitions=count_trans, initial='1', auto_transitions=False)

counter.increase()
states = ['waiting', 'collecting', {'name': 'counting', 'children': counter}]

transitions = [
    ['collect', 'waiting', 'collecting'],
    ['wait', ['collecting','counting_done'], 'waiting'],
    ['count', 'collecting', 'counting']
]

def print_state_trig(machine):
    print("State : "+machine.state+" > "+str(machine.get_triggers(machine.state)))

collector = Machine(states=states, transitions=transitions, initial='waiting', auto_transitions=False)

print_state_trig(collector)
collector.collect()
print_state_trig(collector)
collector.count()
print_state_trig(collector)
collector.increase()
print_state_trig(collector)
collector.done()
print_state_trig(collector)
collector.wait()

# Output : 
# State : waiting > ['collect']
# State : collecting > ['wait', 'count']
# State : counting_1 > ['increase', 'reset']
# State : counting_2 > ['decrease', 'done', 'reset']
# State : counting_done > ['reset', 'wait']
alexandretanem commented 3 years ago

Hi @aleneum, Nice ! Thank you for the quick fix, works as expected :+1:

alexandretanem commented 3 years ago

I reopen because I think that there is the same problem with get_transitions method : If i replace the function print_state_trig as following :

def print_state_trig(machine):
    print("State : "+machine.state+" > "+str(machine.get_transitions(source=machine.state)))

The example gives :

State : waiting > [<NestedTransition('waiting', 'collecting')@139840188429360>] State : collecting > [<NestedTransition('collecting', 'waiting')@139840188429840>, <NestedTransition('collecting', 'counting')@139840188429648>] State : counting_1 > [] State : counting_2 > [] State : counting_done > [<NestedTransition('counting_done', 'waiting')@139840188429888>]

aleneum commented 3 years ago

@alexandretanem: you are right, both methods (get_triggers/get_transitions) haven't been adapted after the changes in 0.8.0 to HierarchicalStateMachine. It also shows, that they aren't tested well enough right now. So thanks again for your bug report.

The return value now is:

State : waiting > [<NestedTransition('waiting', 'collecting')@140358781977360>]
State : collecting > [<NestedTransition('collecting', 'waiting')@140358781977456>, <NestedTransition('collecting', 'counting')@140358781978224>]
State : counting_1 > [<NestedTransition('1', '2')@140358781875056>, <NestedTransition('1', '1')@140358781919584>]
State : counting_2 > [<NestedTransition('2', '1')@140358781874960>, <NestedTransition('2', 'done')@140358781877456>, <NestedTransition('2', '1')@140358781921120>]
State : counting_done > [<NestedTransition('counting_done', 'waiting')@140358781977648>, <NestedTransition('done', '1')@140358781919920>]

which looks about right since there is only one transition for each event. I will create a test case from your (or better our example -_-) to catch errors for simple configurations at least. If you feel like it you can of course test more complex configurations and report back if you find something not working out.

Cheers.