diff --git a/docs/ports.rst b/docs/ports.rst index 2942d608..bcd7d17f 100644 --- a/docs/ports.rst +++ b/docs/ports.rst @@ -227,6 +227,80 @@ Example:: +.. _ports-xml-default-inputs-label: + +Setting default inputs for subtrees +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +A ```` definition can carry attributes of its own. +Every attribute other than ``ID`` and ``name`` is a **default input** for that tree: +a value used for the matching key whenever an instance of the tree does not supply one itself. +This lets a reusable subtree ship with sensible values baked in, so callers only have to override what they actually care about. + +.. code-block:: xml + + + + + + + + + + + + + + + + + +Resolution is per key: an attribute on the ```` element wins, and every key it leaves out +falls back to the default on the ```` definition. +Defaults are written exactly like any other XML attribute value. +A literal constant is type-converted according to the declared port type +(so ``volume="5"`` reaches an ``int`` port as ``5``), and a ``{curly_key}`` reference is wired through the remapping table as usual. +Note that a ``{curly_key}`` default is resolved in the scope of whoever *instantiates* the subtree, +not inside the subtree itself, so it behaves like a remapping the caller would have written by hand. +Literal defaults are usually the clearer choice. + +Defaults for the top-level tree +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The same mechanism applies to the tree named by ``main_tree_to_execute``. +Attributes on *its* ```` element are the defaults for the tree as a whole. +Since there is no enclosing ```` element to override them from, those defaults are overridden +from Python instead, via the ``input_mappings`` argument of +:func:`~py_trees.parsers.behaviour_tree_xml.parse_behaviour_tree_xml`: + +.. code-block:: xml + + + + + + + + + +.. code-block:: python + + root = parse_behaviour_tree_xml( + "my_tree.xml", + input_mappings={"greeting": "Howdy", "volume": "11"}, + ) + +This makes the top-level tree parameterisable from the calling application without having to +edit (or template) the XML for each run. + +A few rules to be aware of: + +* ``input_mappings`` values must be **strings**, matching how they would have been written as + XML attributes (pass ``"11"``, not ``11``). A non-string value raises ``ValueError`` at parse time. +* ``input_mappings`` both overrides existing defaults and supplies keys the XML never defaulted at all (``volume`` in the example above). +* A required input port left with neither a default nor a mapping is not an error at parse time. + It raises :class:`~py_trees.ports.NoDataAvailable` if and when the node first reads it during a tick. + .. _ports-xml-includes-label: Composing trees from multiple files diff --git a/py_trees/parsers/behaviour_tree_xml.py b/py_trees/parsers/behaviour_tree_xml.py index f9a59154..88cd3676 100644 --- a/py_trees/parsers/behaviour_tree_xml.py +++ b/py_trees/parsers/behaviour_tree_xml.py @@ -301,6 +301,7 @@ def parse_behaviour_tree_xml( node_registry: dict | str = "auto", logger: PortsLogger | None = None, search_paths: list[str] | None = None, + input_mappings: dict[str, str] | None = None, ) -> py_trees.behaviour.Behaviour: """ Parse the XML file and build the behavior tree. @@ -335,6 +336,9 @@ def parse_behaviour_tree_xml( ``{tag: class/partial}`` dict to use exclusively. Defaults to ``"auto"``. logger (PortsLogger | None): Optional logger (NoOp if None). search_paths (list[str] | None): Optional extra directories to resolve imports. + input_mappings (dict[str, str]): Optional input key-value mappings that can override default + top-level behaviour tree port values. Note that the values must be strings, to match how + they would be defined in XML. Returns: The root py_trees.behaviour.Behaviour for the requested tree. @@ -387,9 +391,17 @@ def parse_behaviour_tree_xml( logger.debug(f"[DEBUG] Starting parse of main tree ID='{main_tree_id}'") bt_elem = bt_index[main_tree_id] + # Apply any input mappings by just modifying the XML element. + if input_mappings is not None: + for k, v in input_mappings.items(): + if not isinstance(v, str): + raise ValueError(f"Value in input mapping {k} -> {v} must be a string.") + bt_elem.attrib[k] = v + + remapping_table = build_subtree_remapping(bt_elem, bt_index, {}, "/", logger) tree = build_tree_from_xml( bt_elem, - remapping_table={}, + remapping_table=remapping_table, node_registry=node_registry, bt_index=bt_index, logger=logger, @@ -444,6 +456,7 @@ def add_new_key_to_remapping_table(value: str, remapping_table: dict[str, str], def build_subtree_remapping( elem: ET.Element, + bt_index: dict[str, ET.Element], remapping_table: dict[str, str], parent_namespace: str, logger: PortsLogger = NOOP_LOGGER, @@ -451,11 +464,13 @@ def build_subtree_remapping( """ Process the XML element. - The remapping table is updated to include the remappings from the or element. + The remapping table is updated to include the remappings from the or element, + as well as any default port values inherited from the underlying definition. The subtree is then instantiated with the new remapping table. Args: elem: The (or ) XML element. + bt_index (dict[str, ET.Element]): dictionary {ID: BehaviorTree element} for subtree lookup. remapping_table: Parent remapping table (logical name -> absolute key). parent_namespace: Absolute namespace of the parent tree. logger: Optional logger. @@ -478,6 +493,15 @@ def build_subtree_remapping( continue logger.debug(f"Checking to add new key for SubTree attribute: {k} -> {v}") add_new_key_to_remapping_table(v, remapping_table=remapping_table, subtree_namespace=parent_namespace) + + # Also include any default args in the behavior tree definition that were not remapped in the subtree instance. + bt_elem = bt_index[elem.attrib["ID"]] + for k, v in bt_elem.attrib.items(): + if k in ("ID", "name"): + continue + logger.debug(f"Checking to add new key for default BehaviorTree attribute: {k} -> {v}") + add_new_key_to_remapping_table(v, remapping_table=remapping_table, subtree_namespace=parent_namespace) + logger.debug(f"Updated remapping table: {remapping_table}") # Build the new remapping table for this subtree. We build a new remapping table because we need to ensure that @@ -489,7 +513,9 @@ def build_subtree_remapping( # If the value is a key and it is already in the remapping table, we resolve it to its absolute path. # Example: `` - we need resolve {other_key} to its absolute # path (using the parent remapping) and then add `in -> resolved({other_key})` to the new remapping table. - for k, v in elem.attrib.items(): + attrib_dict = bt_elem.attrib.copy() # the copy is necessary! + attrib_dict.update(elem.attrib) + for k, v in attrib_dict.items(): if k in ("ID", "name"): continue logger.debug(f"Processing SubTree attribute: {k} -> {v}") @@ -506,6 +532,7 @@ def build_subtree_remapping( v = resolve_direct_value_remapping(v, remapping_table) logger.debug(f"[Direct value] Adding remapping from parent remapping table: {k} -> {v}") new_remapping[k] = v + logger.debug(f"Subtree '{elem.attrib['ID']}' new remapping table: {new_remapping}") return new_remapping @@ -678,7 +705,7 @@ def build_tree_from_xml( elem: ET.Element, remapping_table: dict[str, str], node_registry: dict, - bt_index: dict, + bt_index: dict[str, ET.Element], logger: PortsLogger = NOOP_LOGGER, subtree_namespace: str = "/", parent_names_str: str = "", @@ -691,7 +718,7 @@ def build_tree_from_xml( remapping_table (dict[str, str]): Remapping table. node_registry (dict): Mapping from class names (str) to callables (constructors or partials) that return ``PortsMixin``-derived instances. - bt_index (dict[str, BehaviorTree]): dictionary {ID: BehaviorTree element} for subtree lookup. + bt_index (dict[str, ET.Element]): dictionary {ID: BehaviorTree element} for subtree lookup. subtree_namespace (str): current blackboard namespace. logger: Optional logger-like object. parent_names_str (str): Dot-separated string of parent names for logging context and generating node names. @@ -880,6 +907,7 @@ def build_tree_from_xml( f" must have exactly one child (the root node), " f"but found {len(child_elems)} children." ) + return build_tree_from_xml( child_elems[0], remapping_table, @@ -904,7 +932,7 @@ def build_tree_from_xml( # Build the new namespace by prepending the subtree ID new_namespace = get_absolute_reference(subtree_name, subtree_namespace) # Build the new remapping table for this subtree. - new_remapping = build_subtree_remapping(elem, remapping_table, subtree_namespace, logger) + new_remapping = build_subtree_remapping(elem, bt_index, remapping_table, subtree_namespace, logger) # Recursively build the subtree with the new remapping table return build_tree_from_xml( subtree_elem, diff --git a/tests/test_ports_xml_parser.py b/tests/test_ports_xml_parser.py index 0e62b0b2..b6167dab 100644 --- a/tests/test_ports_xml_parser.py +++ b/tests/test_ports_xml_parser.py @@ -19,7 +19,13 @@ import py_trees from py_trees.parsers.behaviour_tree_xml import is_key, parse_behaviour_tree_xml -from py_trees.ports import BehaviourWithPorts, PortInformation, PortsMixin, get_ports_registry +from py_trees.ports import ( + BehaviourWithPorts, + NoDataAvailable, + PortInformation, + PortsMixin, + get_ports_registry, +) from py_trees.ports_utils import ( find_node_by_class, find_node_by_name, @@ -27,7 +33,7 @@ strip_trailing_uuid4, ) -from .test_ports_helpers import Consumer, Producer +from .test_ports_helpers import Consumer, FloatConsumer, Producer class StdoutLogger: @@ -418,6 +424,95 @@ def test_subtree_parsing_direct_values_to_XML(self) -> None: self.assertEqual(node.consumed_value, expected_value) + def test_subtree_default_ports(self) -> None: + """Verify that subtree default ports take effect as intended.""" + self.xml = """ + + + + + + + + + + + + + + + + """ + + self.tempfile = tempfile.NamedTemporaryFile(delete=False, mode="w", suffix=".xml") + self.tempfile.write(self.xml) + self.tempfile.close() + + root_node = parse_behaviour_tree_xml(self.tempfile.name, logger=StdoutLogger()) + btree = py_trees.trees.BehaviourTree(root_node) + btree.tick() + + for subtree_name, expected_str, expected_float in ( + ("AllDefaults", "default_str", 42), + ("FirstInputSet", "override_str", 42), + ("SecondInputSet", "default_str", 67), + ("BothInputsSet", "another_str", 9001), + ): + node = find_node_by_name(root_node, f"{subtree_name}.Consumer") + assert isinstance(node, Consumer) + self.assertEqual(node.consumed_value, expected_str) + + node = find_node_by_name(root_node, f"{subtree_name}.FloatConsumer") + assert isinstance(node, FloatConsumer) + self.assertEqual(node.consumed_value, expected_float) + + def test_top_level_default_ports(self) -> None: + """Verify that top-level behaviour default ports take effect as intended.""" + # In this tree, input1 has a default value but input2 does not. + self.xml = """ + + + + + + + """ + + self.tempfile = tempfile.NamedTemporaryFile(delete=False, mode="w", suffix=".xml") + self.tempfile.write(self.xml) + self.tempfile.close() + + # Run without mappings. This should fail in FloatConsumer because no port was set. + root_node = parse_behaviour_tree_xml(self.tempfile.name, logger=StdoutLogger()) + btree = py_trees.trees.BehaviourTree(root_node) + btree.tick() + + node = find_node_by_name(root_node, "Consumer", strip_prefix=True) + assert isinstance(node, Consumer) + self.assertEqual(node.consumed_value, "default_str") + + float_node = find_node_by_name(root_node, "FloatConsumer", strip_prefix=True) + assert isinstance(float_node, FloatConsumer) + with self.assertRaises(NoDataAvailable): + self.assertEqual(float_node.consumed_value, 42) + + # Setting input values should override the inputs appropriately. + root_node = parse_behaviour_tree_xml( + self.tempfile.name, + logger=StdoutLogger(), + input_mappings={"input1": "override_str", "input2": "67"}, + ) + btree = py_trees.trees.BehaviourTree(root_node) + btree.tick() + + node = find_node_by_name(root_node, "Consumer", strip_prefix=True) + assert isinstance(node, Consumer) + self.assertEqual(node.consumed_value, "override_str") + + float_node = find_node_by_name(root_node, "FloatConsumer", strip_prefix=True) + assert isinstance(float_node, FloatConsumer) + self.assertEqual(float_node.consumed_value, 67) + def test_wait_node(self) -> None: """Verify that the duration value is successfully used by the Wait node.""" wait_duration_ms = 500