Fix internal events not always processed before external ones (regres…

…sion from 1.3.0)

CHANGELOG.rst

@@ -9,6 +9,7 @@ Unreleased
  - (Added) A ``setdefault`` function that can be used in the preamble and actions of a statechart to assign default values to variables.
  - (Changed) Meta-Event *step started* has a ``time`` attribute.
  - (Changed) The current event queue can be consulted as a list using ``interpreter._event_queue``. 
+ - (Fixed) Internal events are processed before external ones (regression introduced in 1.3.0).
  - (Fixed) Hook-errors reported by ``sismic-bdd`` CLI are a little bit more verbose (`#81 <https://github.com/AlexandreDecan/sismic/issues/81>`__).
  - (Fixed) Optional transition for ``testing.transition_is_processed``, as previously promised by its documentation but not implemented.
  - (Removed) Internal module ``sismic.interpreter.queue``.

docs/execution.rst

@@ -12,7 +12,8 @@ In particular, eventless transitions are processed *before* transitions containi
 *before* external events, and the simulation follows a inner-first/source-state and run-to-completion semantics.
 
 The main difference between SCXML and Sismic's default interpreter resides in how multiple transitions
-can be triggered simultaneously. This may occur for transitions in orthogonal/parallel states, or when transitions declaring the same event have guards that are not mutually exclusive.
+can be triggered simultaneously. This may occur for transitions in orthogonal/parallel states, or when transitions 
+declaring the same event have guards that are not mutually exclusive.
 
 Simulating the simultaneous triggering of multiple transitions is problematic,
 since it implies to make a non-deterministic choice on the order in which the transitions must be processed,
@@ -170,25 +171,26 @@ For convenience, :py:meth:`~sismic.interpreter.Interpreter.queue` returns the in
 
 Notice that :py:meth:`~sismic.interpreter.Interpreter.execute_once` consumes at most one event at a time.
 In the above example, the *clack* event is not yet processed.
-This can be checked by looking at the current event queue of the interpreter.
+This can be checked by looking at the external event queue of the interpreter.
 
 .. testcode:: interpreter
 
-    for time, event in interpreter._event_queue:
+    for time, event in interpreter._external_queue:
         print(event.name)
 
 .. testoutput:: interpreter
 
     clack
     clock
 
-.. 
-    Queued events can be removed from the queue by calling the :py:meth:`~sismic.interpreter.cancel` method of 
-    the interpreter. This method accepts both the name of an event or an event instance, and remove the 
-    first corresponding event from the queue.
-    .. testcode:: interpreter
-    interpreter.cancel('clock')
-    assert interpreter._event_queue == [(0, Event('clack'))]
+.. note::
+
+    An interpreter has two event queues, one for external events (the ones that are added using 
+    :py:meth:`~sismic.interpreter.Interpreter.queue`, and one for internal events (the ones that 
+    are sent from within the statechart). External events are stored in ``_external_queue`` while 
+    internal events are stored in ``_internal_queue``. Internal events are always processed before
+    external ones. To access the next event that will be processed by the interpreter, use the 
+    :py:meth:`~sismic.interpreter.Interpreter._select_event` method. 
 
 To process all events **at once**, one can repeatedly call :py:meth:`~sismic.interpreter.Interpreter.execute_once` until
 it returns a ``None`` value, meaning that nothing happened during the last call. For instance:

sismic/interpreter/default.py

@@ -69,8 +69,9 @@ def __init__(self, statechart: Statechart, *,
         # Set of active states
         self._configuration = set()  # type: Set[str]
 
-        # Event queue
-        self._event_queue = []  # type: List[Tuple[float, Event]]
+        # Event queues
+        self._internal_queue = []  # type: List[Tuple[float, InternalEvent]]
+        self._external_queue = []  # type: List[Tuple[Float, Event]]
 
         # Bound callables
         self._bound = []  # type: List[Callable[[Event], Any]]
@@ -187,7 +188,7 @@ def bind_property_statechart(self, statechart_or_interpreter: Union[Statechart,
 
     def queue(self, event_or_name: Union[str, Event], *events_or_names: Union[str, Event]) -> 'Interpreter':
         """
-        Queue one or more events to the interpreter queue.
+        Queue one or more events to the external event queue.
 
         If a DelayedEvent is provided, its delay must be a positive number.
         The provided event will be processed by the first call to `execute_once`
@@ -210,20 +211,6 @@ def queue(self, event_or_name: Union[str, Event], *events_or_names: Union[str, E
 
         return self
 
-    # def cancel(self, event_or_name: Union[str, Event]) -> bool:
-    #     """
-    #     Remove first occurrence of given event (or name) from the interpreter queue.
-
-    #     :param event_or_name: an *Event* instance of the name of an event to cancel.
-    #     :return: True if the event was found and removed, False otherwise.
-    #     """
-    #     event = Event(event_or_name) if isinstance(event_or_name, str) else event_or_name
-    #     for i, (time, queued_event) in enumerate(self._event_queue):
-    #         if queued_event == event: 
-    #             self._event_queue.pop(i)
-    #             return True
-    #     return False
-
     def execute(self, max_steps: int = -1) -> List[MacroStep]:
         """
         Repeatedly calls *execute_once* and return a list containing
@@ -269,7 +256,7 @@ def execute_once(self) -> Optional[MacroStep]:
             return None
 
         # Notify properties
-        self._notify_properties('step started', time=self._time)
+        self._notify_properties('step started', time=self.time)
 
         # Consume event if it triggered a transition
         if computed_steps[0].event is not None:
@@ -285,7 +272,7 @@ def execute_once(self) -> Optional[MacroStep]:
             executed_steps.append(self._apply_step(step))
             executed_steps.extend(self._stabilize())
 
-        macro_step = MacroStep(time=self._time, steps=executed_steps)
+        macro_step = MacroStep(time=self.time, steps=executed_steps)
 
         # Check state invariants
         configuration = self.configuration  # Use self.configuration to benefit from the sorting by depth
@@ -301,16 +288,18 @@ def execute_once(self) -> Optional[MacroStep]:
 
     def _queue_event(self, event: Event):
         """
-        Convenient helper to queue events to the current event queue.
+        Convenient helper to queue events wrt. to internal/external and their time.
 
         :param event: Event to queue.
         """
-        time = self._time + (event.delay if isinstance(event, DelayedEvent) else 0)
+        queue = self._internal_queue if isinstance(event, InternalEvent) else self._external_queue
+
+        time = self.time + (event.delay if isinstance(event, DelayedEvent) else 0)
         position = bisect.bisect_right(  # type: ignore
-            _KeyifyList(self._event_queue, lambda t: (t[0], not isinstance(t[1], InternalEvent))),
+            _KeyifyList(queue, lambda t: (t[0], not isinstance(t[1], InternalEvent))),
             (time, not isinstance(event, InternalEvent))
         )
-        self._event_queue.insert(position, (time, event))
+        queue.insert(position, (time, event))
 
     def _raise_event(self, event: Union[InternalEvent, MetaEvent]) -> None:
         """
@@ -375,20 +364,21 @@ def _check_properties(self, macro_step: Optional[MacroStep]):
             if property_statechart.final:
                 raise PropertyStatechartError(property_statechart, self.configuration, macro_step, self.context)
 
-    def _select_event(self, *, consume: bool) -> Optional[Event]:
+    def _select_event(self, *, consume: bool=False) -> Optional[Event]:
         """
-        Return the next available event if any.
-        This method prioritizes internal events over external ones.
-
-        :param consume: Indicates whether event should be consumed.
+        Return the next event to process. 
+        Internal events have priority over external ones. 
+        
+        :param consume: Indicates whether event should be consumed, default to False.
         :return: An instance of Event or None if no event is available
         """
-        if len(self._event_queue) > 0:
-            time, event = self._event_queue[0]
-            if time <= self.time:
-                if consume:
-                    self._event_queue.pop(0)
-                return event
+        for queue in (self._internal_queue, self._external_queue):
+            if len(queue) > 0:
+                time, event = queue[0]
+                if time <= self.time:
+                    if consume:
+                        queue.pop(0)
+                    return event
         return None
 
     def _select_transitions(self, event: Optional[Event], states: Iterable[str], *,
@@ -527,7 +517,7 @@ def _compute_steps(self) -> Optional[List[MicroStep]]:
             return [MicroStep(entered_states=[cast(str, self._statechart.root)])]
 
         # Select transitions
-        event = self._select_event(consume=False)
+        event = self._select_event()
         transitions = self._select_transitions(event, states=self._configuration)
 
         # No transition can be triggered?

tests/test_interpreter.py

@@ -590,10 +590,10 @@ def test_bind(self, interpreter):
         assert i2.queue in i1._bound
 
         i1._raise_event(InternalEvent('test'))
-        assert i1._select_event(consume=False) == Event('test')
-        assert isinstance(i1._select_event(consume=False), InternalEvent)
-        assert i2._select_event(consume=False) == Event('test')
-        assert not isinstance(i2._select_event(consume=False), InternalEvent)
+        assert i1._select_event() == Event('test')
+        assert isinstance(i1._select_event(), InternalEvent)
+        assert i2._select_event() == Event('test')
+        assert not isinstance(i2._select_event(), InternalEvent)
 
     def test_bind_callable(self, interpreter):
         i1, i2 = interpreter
@@ -603,10 +603,10 @@ def test_bind_callable(self, interpreter):
 
         i1._raise_event(InternalEvent('test'))
 
-        assert i1._select_event(consume=False) == Event('test')
-        assert isinstance(i1._select_event(consume=False), InternalEvent)
-        assert i2._select_event(consume=False) == Event('test')
-        assert not isinstance(i2._select_event(consume=False), InternalEvent)
+        assert i1._select_event() == Event('test')
+        assert isinstance(i1._select_event(), InternalEvent)
+        assert i2._select_event() == Event('test')
+        assert not isinstance(i2._select_event(), InternalEvent)
 
     def test_unbind(self, interpreter):
         i1, i2 = interpreter
@@ -630,8 +630,8 @@ def test_metaevent(self, interpreter):
 
         i1._raise_event(MetaEvent('test'))
 
-        assert i1._select_event(consume=False) is None
-        assert i2._select_event(consume=False) is None
+        assert i1._select_event() is None
+        assert i2._select_event() is None
 
     def test_event(self, interpreter):
         i1, i2 = interpreter
@@ -641,8 +641,8 @@ def test_event(self, interpreter):
         with pytest.raises(ValueError):
             i1._raise_event(Event('test'))
 
-        assert i1._select_event(consume=False) is None
-        assert i2._select_event(consume=False) is None
+        assert i1._select_event() is None
+        assert i2._select_event() is None
 
 
 def test_interpreter_is_serialisable(microwave):
@@ -713,42 +713,24 @@ def test_consume_order(self, interpreter):
     def test_internal_first(self, interpreter):
         interpreter.queue(DelayedEvent('test1', delay=0))
         interpreter._raise_event(InternalEvent('test2'))
-        interpreter.queue(DelayedEvent('test3', delay=1))
+        interpreter.queue(DelayedEvent('test3', delay=2))
+
+        event = interpreter._select_event()
+        assert isinstance(event, InternalEvent) and event == Event('test2')
 
+        interpreter._time = 2
         event = interpreter._select_event(consume=True)
         assert isinstance(event, InternalEvent) and event == Event('test2')
+
+        interpreter._raise_event(InternalEvent('test4'))
+        # Queue is (0, test1) ; (2, test3) ; (2, test4) but test4 is internal
+
+        event = interpreter._select_event(consume=True)
+        assert isinstance(event, InternalEvent) and event == Event('test4')
         event = interpreter._select_event(consume=True)
         assert isinstance(event, DelayedEvent) and event == Event('test1')
-        assert interpreter._select_event(consume=True) is None
-
-        interpreter._time = 1
         event = interpreter._select_event(consume=True)
         assert isinstance(event, DelayedEvent) and event == Event('test3')
-
-    # def test_cancel_event(self, interpreter):
-    #     interpreter.queue('test1')
-    #     interpreter.queue(Event('test2', x=1))
-    #     interpreter.queue('test3')
-
-    #     # Normal cancellation
-    #     assert interpreter.cancel('test1')
-    #     assert interpreter._select_event(consume=False) == Event('test2', x=1)
-
-    #     # Cancellation of unknown event
-    #     assert not interpreter.cancel('test4')
 
-    #     # Cancellation satisfies event parameters
-    #     assert not interpreter.cancel('test2')
-    #     assert interpreter._select_event(consume=False) == Event('test2', x=1)
-    #     assert not interpreter.cancel(Event('test2', x=2))
-    #     assert interpreter._select_event(consume=False) == Event('test2', x=1)
-    #     assert interpreter.cancel(Event('test2', x=1))
-    #     assert interpreter._select_event(consume=False) == Event('test3')
-
-    #     # Cancellation only cancels first occurrence
-    #     interpreter.queue('test3')
-    #     interpreter.queue('test3')
-    #     interpreter.cancel('test3')
-    #     assert interpreter._select_event(consume=True) == Event('test3')
-    #     assert interpreter._select_event(consume=True) == Event('test3')
-    #     assert interpreter._select_event(consume=True) is None
+
+