Found reason for bug in UC2 (#69)

* Found reason for bug in UC2 * Improve BT controller (no functional change) * Remove transitions generation in case automatic transitions are present * Update expected compiled output --------- Signed-off-by: Marco Lampacrescia <[email protected]>
convince-project · Nov 25, 2024 · b26a411 · b26a411
1 parent 7ea13a5
commit b26a411
Show file tree

Hide file tree

Showing 17 changed files with 97 additions and 48 deletions.
diff --git a/examples b/examples
@@ -0,0 +1 @@
+test/jani_generator/_test_data/
diff --git a/graphics b/graphics
diff --git a/src/as2fm/jani_generator/scxml_helpers/scxml_tags.py b/src/as2fm/jani_generator/scxml_helpers/scxml_tags.py
@@ -595,6 +595,8 @@ def get_guard_exp_for_prev_conditions(self, event_name: str) -> Optional[JaniExp
     def add_unhandled_transitions(self, transitions_set: Set[str]):
         """Add self-loops for transitions that weren't handled yet."""
         for event_name in transitions_set:
+            if not self._generate_empty_event_transitions:
+                continue
             if event_name in self._events_no_condition or len(event_name) == 0:
                 continue
             guard_exp = self.get_guard_exp_for_prev_conditions(event_name)
@@ -616,6 +618,8 @@ def add_unhandled_transitions(self, transitions_set: Set[str]):
             self._events_no_condition.append(event_name)
 
     def write_model(self):
+        # Whether we should auto-generate empty self-loops for unhandled events
+        has_event_transition: bool = False
         state_name = self.element.get_id()
         self.automaton.add_location(state_name)
         # Dictionary tracking the conditional trigger-based transitions
@@ -625,7 +629,11 @@ def write_model(self):
         for child in self.children:
             transition_events = child.element.get_events()
             assert len(transition_events) <= 1, "Multiple events in a transition not supported."
-            transition_event: str = "" if len(transition_events) == 0 else transition_events[0]
+            if len(transition_events) == 0:
+                transition_event: str = ""
+            else:
+                transition_event = transition_events[0]
+                has_event_transition = True
             assert transition_event not in self._events_no_condition, (
                 f"Event {transition_event} in state {self.element.get_id()} has already a base"
                 "exit condition."
@@ -645,6 +653,12 @@ def write_model(self):
                 if transition_event not in self._event_to_conditions:
                     self._event_to_conditions[transition_event] = []
                 self._event_to_conditions[transition_event].append(transition_condition)
+        # if "" in self._events_no_condition, then we can transition to new states without events
+        assert not (has_event_transition and "" in self._events_no_condition), (
+            f"Model {self.call_trace[0].get_name()} at state {self.element.get_id()} can always "
+            "transition without an event trigger: not event-based transitions expected."
+        )
+        self._generate_empty_event_transitions = "" not in self._events_no_condition
 
 
 class TransitionTag(BaseTag):

diff --git a/src/as2fm/resources/bt_control_nodes/always_failure.scxml b/src/as2fm/resources/bt_control_nodes/always_failure.scxml
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <scxml
     xmlns="http://www.w3.org/2005/07/scxml"
-    initial="idle"
+    initial="init"
     version="1.0"
     name="AlwaysFailure"
     model_src="https://raw.githubusercontent.com/BehaviorTree/BehaviorTree.CPP/refs/heads/v3.8/include/behaviortree_cpp_v3/actions/always_failure_node.h">
@@ -17,9 +17,14 @@
         </data>
     </datamodel>
 
-    <state id="idle">
-        <!-- Check if the state is valid. If not, go to error and stop -->
+    <state id="init">
+        <!-- Check if the config is valid. If not, go to error and stop -->
         <transition target="error" cond="children_count != 0" />
+        <!-- The condition here will be the negation of the one above -->
+        <transition target="idle" />
+    </state>
+
+    <state id="idle">
         <!-- React to an incoming BT Tick -->
         <bt_tick target="idle">
             <bt_return_status status="FAILURE" />

diff --git a/src/as2fm/resources/bt_control_nodes/always_success.scxml b/src/as2fm/resources/bt_control_nodes/always_success.scxml
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <scxml
     xmlns="http://www.w3.org/2005/07/scxml"
-    initial="idle"
+    initial="init"
     version="1.0"
     name="AlwaysSuccess"
     model_src="https://raw.githubusercontent.com/BehaviorTree/BehaviorTree.CPP/refs/heads/v3.8/include/behaviortree_cpp_v3/actions/always_success_node.h">
@@ -17,9 +17,14 @@
         </data>
     </datamodel>
 
-    <state id="idle">
-        <!-- Check if the state is valid. If not, go to error and stop -->
+    <state id="init">
+        <!-- Check if the config is valid. If not, go to error and stop -->
         <transition target="error" cond="children_count != 0" />
+        <!-- The condition here will be the negation of the one above -->
+        <transition target="idle" />
+    </state>
+
+    <state id="idle">
         <!-- React to an incoming BT Tick -->
         <bt_tick target="idle">
             <bt_return_status status="SUCCESS" />

diff --git a/src/as2fm/resources/bt_control_nodes/fallback.scxml b/src/as2fm/resources/bt_control_nodes/fallback.scxml
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <scxml
     xmlns="http://www.w3.org/2005/07/scxml"
-    initial="wait_for_tick"
+    initial="init"
     version="1.0"
     name="Fallback"
     model_src="https://raw.githubusercontent.com/BehaviorTree/BehaviorTree.CPP/refs/heads/v3.8/src/controls/fallback_node.cpp">
@@ -18,9 +18,14 @@
         </data>
     </datamodel>
 
-    <state id="wait_for_tick">
-        <!-- Check if the state is valid. If not, go to error and stop -->
+    <state id="init">
+        <!-- Check if the config is valid. If not, go to error and stop -->
         <transition target="error" cond="children_count &lt; 1" />
+        <!-- The condition here will be the negation of the one above -->
+        <transition target="wait_for_tick" />
+    </state>
+
+    <state id="wait_for_tick">
         <!-- React to an incoming BT Tick -->
         <bt_tick target="tick_children"/>
     </state>

diff --git a/src/as2fm/resources/bt_control_nodes/if_then_else.scxml b/src/as2fm/resources/bt_control_nodes/if_then_else.scxml
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <scxml
     xmlns="http://www.w3.org/2005/07/scxml"
-    initial="wait_for_tick"
+    initial="init"
     version="1.0"
     name="IfThenElse"
     model_src="https://raw.githubusercontent.com/BehaviorTree/BehaviorTree.CPP/refs/heads/v3.8/src/controls/if_then_else_node.cpp">
@@ -18,9 +18,14 @@
         </data>
     </datamodel>
 
-    <state id="wait_for_tick">
-        <!-- Check if the state is valid. If not, go to error and stop -->
+    <state id="init">
+        <!-- Check if the config is valid. If not, go to error and stop -->
         <transition target="error" cond="children_count &lt; 2 || children_count > 3" />
+        <!-- The condition here will be the negation of the one above -->
+        <transition target="wait_for_tick" />
+    </state>
+
+    <state id="wait_for_tick">
         <!-- React to an incoming BT Tick -->
         <bt_tick cond="child_idx == 0" target="tick_condition_child" />
         <bt_tick cond="child_idx > 0" target="tick_exec_child" />

diff --git a/src/as2fm/resources/bt_control_nodes/inverter.scxml b/src/as2fm/resources/bt_control_nodes/inverter.scxml
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <scxml
     xmlns="http://www.w3.org/2005/07/scxml"
-    initial="wait_for_tick"
+    initial="init"
     version="1.0"
     name="Inverter"
     model_src="https://raw.githubusercontent.com/BehaviorTree/BehaviorTree.CPP/refs/heads/v3.8/src/decorators/inverter_node.cpp">
@@ -17,9 +17,14 @@
         </data>
     </datamodel>
 
-    <state id="wait_for_tick">
-        <!-- Check if the state is valid. If not, go to error and stop -->
+    <state id="init">
+        <!-- Check if the config is valid. If not, go to error and stop -->
         <transition target="error" cond="children_count != 1" />
+        <!-- The condition here will be the negation of the one above -->
+        <transition target="wait_for_tick" />
+    </state>
+
+    <state id="wait_for_tick">
         <!-- React to an incoming BT Tick -->
         <bt_tick target="tick_child" />
     </state>

diff --git a/src/as2fm/resources/bt_control_nodes/reactive_fallback.scxml b/src/as2fm/resources/bt_control_nodes/reactive_fallback.scxml
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <scxml
     xmlns="http://www.w3.org/2005/07/scxml"
-    initial="wait_for_tick"
+    initial="init"
     version="1.0"
     name="ReactiveFallback"
     model_src="https://raw.githubusercontent.com/BehaviorTree/BehaviorTree.CPP/refs/heads/v3.8/src/controls/reactive_fallback.cpp">
@@ -19,9 +19,14 @@
         </data>
     </datamodel>
 
-    <state id="wait_for_tick">
-        <!-- Check if the state is valid. If not, go to error and stop -->
+    <state id="init">
+        <!-- Check if the config is valid. If not, go to error and stop -->
         <transition target="error" cond="children_count &lt; 1" />
+        <!-- The condition here will be the negation of the one above -->
+        <transition target="wait_for_tick" />
+    </state>
+
+    <state id="wait_for_tick">
         <!-- React to an incoming BT Tick -->
         <bt_tick target="tick_children">
             <assign location="failure_count" expr="0" />

diff --git a/src/as2fm/resources/bt_control_nodes/reactive_sequence.scxml b/src/as2fm/resources/bt_control_nodes/reactive_sequence.scxml
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <scxml
     xmlns="http://www.w3.org/2005/07/scxml"
-    initial="wait_for_tick"
+    initial="init"
     version="1.0"
     name="ReactiveSequence"
     model_src="https://raw.githubusercontent.com/BehaviorTree/BehaviorTree.CPP/refs/heads/v3.8/src/controls/reactive_sequence.cpp">
@@ -19,9 +19,14 @@
         </data>
     </datamodel>
 
-    <state id="wait_for_tick">
-        <!-- Check if the state is valid. If not, go to error and stop -->
+    <state id="init">
+        <!-- Check if the config is valid. If not, go to error and stop -->
         <transition target="error" cond="children_count &lt; 1" />
+        <!-- The condition here will be the negation of the one above -->
+        <transition target="wait_for_tick" />
+    </state>
+
+    <state id="wait_for_tick">
         <!-- React to an incoming BT Tick -->
         <bt_tick target="tick_children">
             <assign location="success_count" expr="0" />

diff --git a/src/as2fm/resources/bt_control_nodes/sequence.scxml b/src/as2fm/resources/bt_control_nodes/sequence.scxml
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <scxml
     xmlns="http://www.w3.org/2005/07/scxml"
-    initial="wait_for_tick"
+    initial="init"
     version="1.0"
     name="Sequence"
     model_src="https://raw.githubusercontent.com/BehaviorTree/BehaviorTree.CPP/refs/heads/v3.8/src/controls/sequence_node.cpp">
@@ -18,9 +18,14 @@
         </data>
     </datamodel>
 
-    <state id="wait_for_tick">
-        <!-- Check if the state is valid. If not, go to error and stop -->
+    <state id="init">
+        <!-- Check if the config is valid. If not, go to error and stop -->
         <transition target="error" cond="children_count &lt; 1" />
+        <!-- The condition here will be the negation of the one above -->
+        <transition target="wait_for_tick" />
+    </state>
+
+    <state id="wait_for_tick">
         <!-- React to an incoming BT Tick -->
         <bt_tick target="tick_children"/>
     </state>

diff --git a/test/jani_generator/_test_data/.gitignore b/test/jani_generator/_test_data/.gitignore
@@ -3,5 +3,6 @@
 */**/main*.jani
 */**/*.csv
 */**/*.ods
+*/**/*.pu
 */output.jani
 */out
diff --git a/test/jani_generator/_test_data/uc2_assembly/Plugins/IsBlockFell.scxml b/test/jani_generator/_test_data/uc2_assembly/Plugins/IsBlockFell.scxml
@@ -25,11 +25,7 @@
     </state>
 
     <state id="wait_answer">
-        <!--<onentry>
-            <bt_return_status status="RUNNING" />
-        </onentry>
-        <bt_tick target="wait_answer" />-->
-        <ros_service_handle_response name="get_block_status" target="idle"> <!--target="answer">-->
+        <ros_service_handle_response name="get_block_status" target="idle">
             <assign location="block_down" expr="_res.block_down" />
             <if cond="block_down > 0">
                 <bt_return_status status="SUCCESS" />
@@ -39,14 +35,4 @@
         </ros_service_handle_response>
     </state>
 
-    <!--<state id="answer">
-        <bt_tick target="idle">
-            <if cond="block_down > 0">
-                <bt_return_status status="SUCCESS" />
-                <else />
-                <bt_return_status status="FAILURE" />
-            </if>
-        </bt_tick>
-    </state>-->
-
 </scxml>
diff --git a/test/jani_generator/test_systemtest_scxml_to_jani.py b/test/jani_generator/test_systemtest_scxml_to_jani.py
@@ -416,7 +416,6 @@ def test_uc1_docking_bugged(self):
             success=False,
         )
 
-    @pytest.mark.skip(reason="Result is 0.98, need to find the 2 percent failures.")
     def test_uc2_assembly(self):
         """Test the UC2 BT example."""
         self._test_with_main(

diff --git a/test/scxml_converter/_test_data/battery_drainer_w_bt/gt_bt_scxml/1000_ReactiveSequence.scxml b/test/scxml_converter/_test_data/battery_drainer_w_bt/gt_bt_scxml/1000_ReactiveSequence.scxml
@@ -1,11 +1,14 @@
-<scxml name="1000_ReactiveSequence" version="1.0" model_src="" initial="wait_for_tick" xmlns="http://www.w3.org/2005/07/scxml">
+<scxml name="1000_ReactiveSequence" version="1.0" model_src="" initial="init" xmlns="http://www.w3.org/2005/07/scxml">
     <datamodel>
         <data id="success_count" expr="0" type="int8"/>
         <data id="child_idx" expr="0" type="int8"/>
         <data id="children_count" expr="2" type="int8"/>
     </datamodel>
+    <state id="init">
+        <transition cond="children_count &lt; 1" target="error"/>
+        <transition target="wait_for_tick"/>
+    </state>
     <state id="wait_for_tick">
-        <transition target="error" cond="children_count &lt; 1"/>
         <transition target="tick_children" event="bt_1000_tick">
             <assign location="success_count" expr="0"/>
             <assign location="child_idx" expr="0"/>

diff --git a/test/scxml_converter/_test_data/battery_drainer_w_bt/gt_bt_scxml/1001_Inverter.scxml b/test/scxml_converter/_test_data/battery_drainer_w_bt/gt_bt_scxml/1001_Inverter.scxml
@@ -1,9 +1,12 @@
-<scxml name="1001_Inverter" version="1.0" model_src="" initial="wait_for_tick" xmlns="http://www.w3.org/2005/07/scxml">
+<scxml name="1001_Inverter" version="1.0" model_src="" initial="init" xmlns="http://www.w3.org/2005/07/scxml">
     <datamodel>
         <data id="children_count" expr="1" type="int8"/>
     </datamodel>
+    <state id="init">
+        <transition cond="children_count != 1" target="error"/>
+        <transition target="wait_for_tick"/>
+    </state>
     <state id="wait_for_tick">
-        <transition target="error" cond="children_count != 1"/>
         <transition target="tick_child" event="bt_1001_tick"/>
     </state>
     <state id="tick_child">

diff --git a/test/scxml_converter/_test_data/bt_ports_only/gt_bt_scxml/1000_ReactiveSequence.scxml b/test/scxml_converter/_test_data/bt_ports_only/gt_bt_scxml/1000_ReactiveSequence.scxml
@@ -1,11 +1,14 @@
-<scxml name="1000_ReactiveSequence" version="1.0" model_src="" initial="wait_for_tick" xmlns="http://www.w3.org/2005/07/scxml">
+<scxml name="1000_ReactiveSequence" version="1.0" model_src="" initial="init" xmlns="http://www.w3.org/2005/07/scxml">
     <datamodel>
         <data id="success_count" expr="0" type="int8"/>
         <data id="child_idx" expr="0" type="int8"/>
         <data id="children_count" expr="2" type="int8"/>
     </datamodel>
+    <state id="init">
+        <transition cond="children_count &lt; 1" target="error"/>
+        <transition target="wait_for_tick"/>
+    </state>
     <state id="wait_for_tick">
-        <transition target="error" cond="children_count &lt; 1"/>
         <transition target="tick_children" event="bt_1000_tick">
             <assign location="success_count" expr="0"/>
             <assign location="child_idx" expr="0"/>