From 4516be0af2e2c49b09e2f5a9f0a470528cc360d3 Mon Sep 17 00:00:00 2001 From: Florian Vahl Date: Fri, 5 Apr 2024 00:09:27 +0000 Subject: [PATCH 1/5] Initialize sequence member actions on the fly. This fixes issues where one expects the action to be initialized when it enters the top of the stack --- .../dynamic_stack_decider/dsd.py | 7 +-- .../dynamic_stack_decider/sequence_element.py | 46 +++++++++++++------ .../dsd_follower.py | 4 +- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/dynamic_stack_decider/dynamic_stack_decider/dsd.py b/dynamic_stack_decider/dynamic_stack_decider/dsd.py index e08f999..25807cf 100644 --- a/dynamic_stack_decider/dynamic_stack_decider/dsd.py +++ b/dynamic_stack_decider/dynamic_stack_decider/dsd.py @@ -201,13 +201,10 @@ def _bind_modules(self, element): else: raise ValueError(f'Unknown parser tree element type "{type(element)}" for element "{element}"!') - def _init_element(self, element: AbstractTreeElement): + def _init_element(self, element: AbstractTreeElement) -> AbstractStackElement: """Initializes the module belonging to the given element.""" if isinstance(element, SequenceTreeElement): - initialized_actions = list() - for action in element.action_elements: - initialized_actions.append(action.module(self.blackboard, self, action.parameters)) - return SequenceElement(self.blackboard, self, initialized_actions) + return SequenceElement(self.blackboard, self, element.action_elements, self._init_element) else: return element.module(self.blackboard, self, element.parameters) diff --git a/dynamic_stack_decider/dynamic_stack_decider/sequence_element.py b/dynamic_stack_decider/dynamic_stack_decider/sequence_element.py index 167fb61..f07505e 100644 --- a/dynamic_stack_decider/dynamic_stack_decider/sequence_element.py +++ b/dynamic_stack_decider/dynamic_stack_decider/sequence_element.py @@ -2,6 +2,7 @@ from dynamic_stack_decider.abstract_action_element import AbstractActionElement from dynamic_stack_decider.abstract_stack_element import AbstractStackElement +from dynamic_stack_decider.tree import AbstractTreeElement, ActionTreeElement if TYPE_CHECKING: from dynamic_stack_decider.dsd import DSD @@ -16,13 +17,30 @@ class SequenceElement(AbstractStackElement): This is not an abstract class to inherit from. """ - def __init__(self, blackboard, dsd: "DSD", actions: list[AbstractActionElement]): + def __init__( + self, + blackboard, + dsd: "DSD", + actions: list[ActionTreeElement], + init_function: callable[[AbstractTreeElement], AbstractStackElement], + ): """ + :param blackboard: Shared blackboard for data exchange and code reuse between elements + :param dsd: The stack decider which has this element on its stack. :param actions: list of initialized action elements + :param init_function: A function that initializes an action element creating a stack element from a tree element """ super().__init__(blackboard, dsd, dict()) + # Here we store the 'blueprints' of the actions self.actions = actions - self.current_action_index = 0 + # We store a reference to the function that initializes the action elements based on the tree + self._init_function = init_function + # Here we store the actual instances of the active action + # The action is only initialized when it is the current action + assert len(actions) > 0, "A sequence element must contain at least one action" + self.current_action: AbstractActionElement = self._init_function(actions[0]) + # Where we are in the sequence + self.current_action_index: int = 0 def perform(self, reevaluate=False): """ @@ -30,7 +48,13 @@ def perform(self, reevaluate=False): :param reevaluate: Ignored for SequenceElements """ + # Log the active element + self.publish_debug_data("Active Element", self.current_action.name) + # Pass the perform call to the current action self.current_action.perform() + # If the action had debug data, publish it + if self.current_action._debug_data: + self.publish_debug_data("Corresponding debug data", self.current_action._debug_data) def pop_one(self): """ @@ -39,30 +63,24 @@ def pop_one(self): assert not self.in_last_element(), ( "It is not possible to pop a single element when" "the last element of the sequence is active" ) + # Increment the index to the next action and initialize it self.current_action_index += 1 + # We initilize the current action here to avoid the problem described in + # https://github.com/bit-bots/dynamic_stack_decider/issues/107 + self.current_action = self._init_function(self.actions[self.current_action_index]) def in_last_element(self): """Returns if the current element is the last element of the sequence""" return self.current_action_index == len(self.actions) - 1 - @property - def current_action(self) -> AbstractActionElement: - """ - Returns the currently executed action of the sequence element - """ - return self.actions[self.current_action_index] - def repr_dict(self) -> dict: """ Represent this stack element as dictionary which is JSON encodable """ - self.publish_debug_data("Active Element", self.current_action.name) - if self.current_action._debug_data: - self.publish_debug_data("Corresponding debug data", self.current_action._debug_data) data = { "type": "sequence", - "current_action_id": self.current_action_index, - "content": [elem.repr_dict() for elem in self.actions], + "current_action_index": self.current_action_index, + "current_action": self.current_action.repr_dict(), "debug_data": self._debug_data, } self.clear_debug_data() diff --git a/dynamic_stack_decider_visualization/dynamic_stack_decider_visualization/dsd_follower.py b/dynamic_stack_decider_visualization/dynamic_stack_decider_visualization/dsd_follower.py index 442666e..9b73fa8 100644 --- a/dynamic_stack_decider_visualization/dynamic_stack_decider_visualization/dsd_follower.py +++ b/dynamic_stack_decider_visualization/dynamic_stack_decider_visualization/dsd_follower.py @@ -182,7 +182,7 @@ def param_string(params: dict) -> str: # Spaces for indentation action_label = " " # Mark current element (if this sequence is on the stack) - if stack_element is not None and i == stack_element["current_action_id"]: + if stack_element is not None and i == stack_element["current_action_index"]: action_label += "--> " action_label += action["name"] + param_string(action["parameters"]) label.append(action_label) @@ -327,7 +327,7 @@ def to_q_item_model(self): # Set the text of the item if stack_element["type"] == "sequence": # Get the names of all actions - action_names = [element["name"] for element in stack_element["content"]] + action_names = [element["name"] for element in stack_element["current_action"]] # Join them together and set the text elem_item.setText("Sequence: " + ", ".join(action_names)) else: From c008a7685b5f54807a46ba7d2fa71315cab58beb Mon Sep 17 00:00:00 2001 From: Florian Vahl Date: Fri, 5 Apr 2024 00:24:28 +0000 Subject: [PATCH 2/5] Remove legacy cmake file --- dynamic_stack_decider/CMakeLists.txt | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 dynamic_stack_decider/CMakeLists.txt diff --git a/dynamic_stack_decider/CMakeLists.txt b/dynamic_stack_decider/CMakeLists.txt deleted file mode 100644 index 7c09fca..0000000 --- a/dynamic_stack_decider/CMakeLists.txt +++ /dev/null @@ -1,7 +0,0 @@ -cmake_minimum_required(VERSION 3.5) -project(dynamic_stack_decider) - -find_package(ament_cmake_python REQUIRED) - -ament_python_install_package(${PROJECT_NAME}) -ament_package() From 43e637b5e5818ff0a1f77f8764e1392045622967 Mon Sep 17 00:00:00 2001 From: Florian Vahl Date: Fri, 5 Apr 2024 00:25:06 +0000 Subject: [PATCH 3/5] Fix typing for callables --- .../dynamic_stack_decider/sequence_element.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dynamic_stack_decider/dynamic_stack_decider/sequence_element.py b/dynamic_stack_decider/dynamic_stack_decider/sequence_element.py index f07505e..fcaf51d 100644 --- a/dynamic_stack_decider/dynamic_stack_decider/sequence_element.py +++ b/dynamic_stack_decider/dynamic_stack_decider/sequence_element.py @@ -1,4 +1,4 @@ -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Callable from dynamic_stack_decider.abstract_action_element import AbstractActionElement from dynamic_stack_decider.abstract_stack_element import AbstractStackElement @@ -22,7 +22,7 @@ def __init__( blackboard, dsd: "DSD", actions: list[ActionTreeElement], - init_function: callable[[AbstractTreeElement], AbstractStackElement], + init_function: Callable[[AbstractTreeElement], AbstractStackElement], ): """ :param blackboard: Shared blackboard for data exchange and code reuse between elements From 8464396f7237e5155a3f070edd1f92a37061a189 Mon Sep 17 00:00:00 2001 From: Florian Vahl Date: Fri, 5 Apr 2024 10:42:33 +0000 Subject: [PATCH 4/5] Fix comment --- dynamic_stack_decider/dynamic_stack_decider/sequence_element.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dynamic_stack_decider/dynamic_stack_decider/sequence_element.py b/dynamic_stack_decider/dynamic_stack_decider/sequence_element.py index fcaf51d..045518a 100644 --- a/dynamic_stack_decider/dynamic_stack_decider/sequence_element.py +++ b/dynamic_stack_decider/dynamic_stack_decider/sequence_element.py @@ -27,7 +27,7 @@ def __init__( """ :param blackboard: Shared blackboard for data exchange and code reuse between elements :param dsd: The stack decider which has this element on its stack. - :param actions: list of initialized action elements + :param actions: list of of action tree elements / blueprints for the actions :param init_function: A function that initializes an action element creating a stack element from a tree element """ super().__init__(blackboard, dsd, dict()) From 3ea11a7657e50197460639f930e87a59690e45fa Mon Sep 17 00:00:00 2001 From: Florian Vahl Date: Fri, 5 Apr 2024 16:35:32 +0200 Subject: [PATCH 5/5] Use tree instead of stack for the names of the uninit actions --- .../dynamic_stack_decider_visualization/dsd_follower.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dynamic_stack_decider_visualization/dynamic_stack_decider_visualization/dsd_follower.py b/dynamic_stack_decider_visualization/dynamic_stack_decider_visualization/dsd_follower.py index 9b73fa8..34623da 100644 --- a/dynamic_stack_decider_visualization/dynamic_stack_decider_visualization/dsd_follower.py +++ b/dynamic_stack_decider_visualization/dynamic_stack_decider_visualization/dsd_follower.py @@ -312,6 +312,9 @@ def to_q_item_model(self): # Start with the root/bottom of the stack stack_element = self.stack + # Store the corresponding tree element + tree_element = self.tree + # Go through all stack elements while stack_element is not None: # Sanity check @@ -327,7 +330,7 @@ def to_q_item_model(self): # Set the text of the item if stack_element["type"] == "sequence": # Get the names of all actions - action_names = [element["name"] for element in stack_element["current_action"]] + action_names = [action["name"] for action in tree_element["action_elements"]] # Join them together and set the text elem_item.setText("Sequence: " + ", ".join(action_names)) else: @@ -351,6 +354,9 @@ def to_q_item_model(self): # Go to next element stack_element = stack_element["next"] + # Also select the corresponding tree element if there are any + if "children" in tree_element and stack_element is not None: + tree_element = tree_element["children"][stack_element["activation_reason"]] return model