From b26a411baecdb1b3f0575830f0df99e91d98d24b Mon Sep 17 00:00:00 2001
From: Marco Lampacrescia <65171491+MarcoLm993@users.noreply.github.com>
Date: Mon, 25 Nov 2024 14:19:23 +0100
Subject: [PATCH] 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 <marco.lampacrescia@de.bosch.com>
---
 examples                                         |  1 +
 graphics                                         |  1 -
 .../jani_generator/scxml_helpers/scxml_tags.py   | 16 +++++++++++++++-
 .../bt_control_nodes/always_failure.scxml        | 11 ++++++++---
 .../bt_control_nodes/always_success.scxml        | 11 ++++++++---
 .../resources/bt_control_nodes/fallback.scxml    | 11 ++++++++---
 .../bt_control_nodes/if_then_else.scxml          | 11 ++++++++---
 .../resources/bt_control_nodes/inverter.scxml    | 11 ++++++++---
 .../bt_control_nodes/reactive_fallback.scxml     | 11 ++++++++---
 .../bt_control_nodes/reactive_sequence.scxml     | 11 ++++++++---
 .../resources/bt_control_nodes/sequence.scxml    | 11 ++++++++---
 test/jani_generator/_test_data/.gitignore        |  1 +
 .../uc2_assembly/Plugins/IsBlockFell.scxml       | 16 +---------------
 .../test_systemtest_scxml_to_jani.py             |  1 -
 .../gt_bt_scxml/1000_ReactiveSequence.scxml      |  7 +++++--
 .../gt_bt_scxml/1001_Inverter.scxml              |  7 +++++--
 .../gt_bt_scxml/1000_ReactiveSequence.scxml      |  7 +++++--
 17 files changed, 97 insertions(+), 48 deletions(-)
 create mode 120000 examples
 delete mode 120000 graphics

diff --git a/examples b/examples
new file mode 120000
index 00000000..d3851334
--- /dev/null
+++ b/examples
@@ -0,0 +1 @@
+test/jani_generator/_test_data/
\ No newline at end of file
diff --git a/graphics b/graphics
deleted file mode 120000
index 1183e149..00000000
--- a/graphics
+++ /dev/null
@@ -1 +0,0 @@
-docs/source/graphics/
\ No newline at end of file
diff --git a/src/as2fm/jani_generator/scxml_helpers/scxml_tags.py b/src/as2fm/jani_generator/scxml_helpers/scxml_tags.py
index 699fc79a..c8a710d8 100644
--- 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
index ad8f88db..3303ff32 100644
--- 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
index 533298bd..f131fe8c 100644
--- 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
index eb521dc1..71e23fb8 100644
--- 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
index 7014fa3d..0eb0b28f 100644
--- 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
index 96456b77..5234a592 100644
--- 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
index e944075e..65ba22a5 100644
--- 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
index dd886cd2..3a0bec9a 100644
--- 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
index be076a5f..7900e1e3 100644
--- 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
index 1df0e5bc..b3d9c27d 100644
--- 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
index 1cdbc9a9..f081b82b 100644
--- 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
index dfbb684e..5a69f6d8 100644
--- 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
index d8e5aeab..12e4fa4e 100644
--- 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
index 75f06330..d1624be0 100644
--- 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
index 8d61abce..ab21e083 100644
--- 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"/>