Fixes related to downstream migrations (#37)

Co-authored-by: Jenny Barry <[email protected]>

bdai_ros2_wrappers/bdai_ros2_wrappers/process.py

@@ -6,11 +6,15 @@
 import sys
 import threading
 import typing
+import warnings
 
 import rclpy
 import rclpy.executors
 import rclpy.logging
 import rclpy.node
+from rclpy.exceptions import InvalidNamespaceException, InvalidNodeNameException
+from rclpy.validate_namespace import validate_namespace
+from rclpy.validate_node_name import validate_node_name
 
 import bdai_ros2_wrappers.context as context
 import bdai_ros2_wrappers.scope as scope
@@ -41,7 +45,7 @@ def __init__(
         self,
         func: MainCallable,
         *,
-        prebaked: bool = True,
+        prebaked: typing.Union[bool, str] = True,
         autospin: typing.Optional[bool] = None,
         forward_logging: typing.Optional[bool] = None,
         namespace: typing.Optional[typing.Union[typing.Literal[True], str]] = None,
@@ -55,8 +59,11 @@ def __init__(
             func: a ``main``-like function to wrap ie. a callable taking a sequence of strings,
             an `argparse.Namespace` (if a CLI is specified), or nothing, and returning a integer
             exit code or nothing.
-            prebaked: whether to instantiate a prebaked or a bare process. Bare processes do not bear
-            a node nor spin an executor, which brings them closest to standard ROS 2 idioms.
+            prebaked: whether to instantiate a prebaked or a bare process i.e. a process bearing
+            an implicit node and executor, or not. May also specificy the exact name for the implicit
+            node, or, if True, the current executable basename without its extension (or CLI program
+            name if one is specified) will be used. Note that completely bare processes do not bear a
+            node nor spin an executor, which brings them closest to standard ROS 2 idioms.
             autospin: whether to automatically equip the underlying scope with a background executor
             or not. Defaults to True for prebaked processes and to False for bare processes.
             forward_logging: whether to forward `logging` logs to the ROS 2 logging system or not.
@@ -70,18 +77,27 @@ def __init__(
         Raises:
             ValueError: if a prebaked process is configured without autospin.
         """
+        if cli is None:
+            program_name = os.path.basename(sys.argv[0])
+        else:
+            program_name = cli.prog
+        name, _ = os.path.splitext(program_name)
+        if prebaked is True:
+            try:
+                validate_node_name(name)
+                prebaked = name
+            except InvalidNodeNameException:
+                warnings.warn(f"'{name}' cannot be used as node name, using scope default")
+        if namespace is True:
+            try:
+                validate_namespace(name)
+                namespace = name
+            except InvalidNamespaceException:
+                warnings.warn(f"'{name}' cannot be used as namespace, using scope default")
         if forward_logging is None:
-            forward_logging = prebaked
+            forward_logging = bool(prebaked)
         if autospin is None:
-            autospin = prebaked
-        if namespace is None and prebaked:
-            namespace = True
-        if namespace is True:
-            if cli is None:
-                program_name = os.path.basename(sys.argv[0])
-            else:
-                program_name = cli.prog
-            namespace, _ = os.path.splitext(program_name)
+            autospin = bool(prebaked)
         self._func = func
         self._cli = cli
         self._scope_kwargs = dict(
@@ -134,9 +150,16 @@ def __call__(self, argv: typing.Optional[typing.Sequence[str]] = None) -> typing
         self._lock.acquire()
         try:
             args = None
+            scope_kwargs = dict(self._scope_kwargs)
             if self._cli is not None:
                 args = self._cli.parse_args(rclpy.utilities.remove_ros_args(argv)[1:])
-            scope_kwargs = either_or(args, "process_args", self._scope_kwargs)
+                scope_kwargs.update(either_or(args, "process_args", {}))
+                prebaked = scope_kwargs.get("prebaked")
+                if isinstance(prebaked, str):
+                    scope_kwargs["prebaked"] = prebaked.format(**vars(args))
+                namespace = scope_kwargs.get("namespace")
+                if isinstance(namespace, str):
+                    scope_kwargs["namespace"] = namespace.format(**vars(args))
             with scope.top(argv, global_=True, **scope_kwargs) as self._scope:
                 ROSAwareProcess.current = self
                 self._lock.release()

bdai_ros2_wrappers/bdai_ros2_wrappers/scope.py

@@ -37,8 +37,8 @@ class LocalStack(threading.local):
     def __init__(
         self,
         *,
-        prebaked: bool = True,
         global_: bool = False,
+        prebaked: typing.Union[bool, str] = True,
         autospin: typing.Optional[bool] = None,
         forward_logging: bool = False,
         namespace: typing.Optional[typing.Union[typing.Literal[True], str]] = None,
@@ -49,7 +49,7 @@ def __init__(
 
         Args:
             prebaked: whether to include an implicit main node in the scope graph or not,
-            for convenience.
+            for convenience. May also specify the exact name for the implicit node.
             global_: whether to make this scope global (ie. accessible from all threads).
             Only one, outermost global scope can be entered at any given time. Global
             scopes can only be entered from the main thread.
@@ -62,7 +62,9 @@ def __init__(
             context: optional context for the underlying executor and nodes.
         """
         if autospin is None:
-            autospin = prebaked
+            autospin = bool(prebaked)
+        if prebaked is True:
+            prebaked = "node"
         if namespace is True:
             namespace = f"_ros_aware_scope_{os.getpid()}_{id(self)}"
         self._namespace = namespace
@@ -120,7 +122,7 @@ def __enter__(self) -> "ROSAwareScope":
                     self._executor = self._stack.enter_context(foreground(executor))
 
                 if self._prebaked:
-                    node = Node("node", namespace=self._namespace, context=self._context)
+                    node = Node(self._prebaked, namespace=self._namespace, context=self._context)
                     self._executor.add_node(node)
                     self._graph.append(node)
                     if self._forward_logging:
@@ -315,7 +317,7 @@ def load(
                 raise RuntimeError("scope has not been entered (or was already exited)")
             if self._executor is None:
                 raise RuntimeError("scope executor has not been set")
-            if namespace is not None:
+            if namespace:
                 namespace = namespace_with(self._namespace, namespace)
             else:
                 namespace = self._namespace

bdai_ros2_wrappers/bdai_ros2_wrappers/tf_listener_wrapper.py

@@ -1,7 +1,7 @@
 # Copyright (c) 2023 Boston Dynamics AI Institute Inc.  All rights reserved.
 
 import warnings
-from typing import Optional
+from typing import Optional, Protocol, Union, runtime_checkable
 
 from geometry_msgs.msg import TransformStamped
 from rclpy.clock import ClockType
@@ -16,6 +16,26 @@
 from bdai_ros2_wrappers.futures import wait_for_future
 
 
+@runtime_checkable
+class StampLike(Protocol):
+    sec: int
+    nanosec: int
+
+
+@runtime_checkable
+class TimeLike(Protocol):
+    nanoseconds: int
+
+
+def from_time_like(obj: Union[StampLike, TimeLike]) -> Time:
+    """Convert an object that exhibits a Time-like interface to a proper Time instance."""
+    if isinstance(obj, StampLike):
+        return Time.from_msg(obj, clock_type=ClockType.SYSTEM_TIME)
+    if isinstance(obj, TimeLike):
+        return Time(nanoseconds=obj.nanoseconds, clock_type=ClockType.SYSTEM_TIME)
+    raise TypeError(f"{obj} does not define a point in time")
+
+
 class TFListenerWrapper:
     """
     A `tf2_ros` lookup device, wrapping both a buffer and a listener.
@@ -67,7 +87,9 @@ def buffer(self) -> Buffer:
     def shutdown(self) -> None:
         self._tf_listener.unregister()
 
-    def wait_for_a_tform_b_async(self, frame_a: str, frame_b: str, transform_time: Optional[Time] = None) -> Future:
+    def wait_for_a_tform_b_async(
+        self, frame_a: str, frame_b: str, transform_time: Optional[Union[StampLike, TimeLike]] = None
+    ) -> Future:
         """
         Wait asynchronously for the transform from from_frame to to_frame to become available.
 
@@ -84,7 +106,11 @@ def wait_for_a_tform_b_async(self, frame_a: str, frame_b: str, transform_time: O
         return self._tf_buffer.wait_for_transform_async(frame_a, frame_b, transform_time)
 
     def wait_for_a_tform_b(
-        self, frame_a: str, frame_b: str, transform_time: Optional[Time] = None, timeout_sec: Optional[float] = None
+        self,
+        frame_a: str,
+        frame_b: str,
+        transform_time: Optional[Union[StampLike, TimeLike]] = None,
+        timeout_sec: Optional[float] = None,
     ) -> bool:
         """
         Wait for a transform from frame_a to frame_b to become available.
@@ -117,7 +143,7 @@ def lookup_a_tform_b(
         self,
         frame_a: str,
         frame_b: str,
-        transform_time: Optional[Time] = None,
+        transform_time: Optional[Union[StampLike, TimeLike]] = None,
         timeout_sec: Optional[float] = None,
         wait_for_frames: bool = False,
     ) -> TransformStamped:
@@ -147,11 +173,12 @@ def lookup_a_tform_b(
                 # tf2 Python bindings yield system clock timestamps rather than ROS clock
                 # timestamps, regardless of where these timestamps came from (as there is no
                 # clock notion in tf2)
-                latest_time = Time(nanoseconds=latest_time.nanoseconds, clock_type=ClockType.ROS_TIME)
-                if transform_time is not None and latest_time > transform_time:
-                    # no need to wait, either lookup can be satisfied or an extrapolation
-                    # to the past exception will ensue
-                    timeout_sec = 0.0
+                if transform_time is not None:
+                    transform_time = from_time_like(transform_time)
+                    if latest_time > transform_time:
+                        # no need to wait, either lookup can be satisfied or an extrapolation
+                        # to the past exception will ensue
+                        timeout_sec = 0.0
             except TransformException:
                 # either frames do not exist or a path between them doesn't, do not wait
                 timeout_sec = 0.0

bdai_ros2_wrappers/test/test_process.py

@@ -1,5 +1,6 @@
 # Copyright (c) 2023 Boston Dynamics AI Institute Inc.  All rights reserved.
 import argparse
+import unittest.mock as mock
 
 from std_srvs.srv import Trigger
 
@@ -39,8 +40,30 @@ def cli() -> argparse.ArgumentParser:
     @process.main(cli())
     def main(args: argparse.Namespace) -> int:
         assert main.node is not None
-        assert main.node.get_fully_qualified_name() == "/test_command/node"
+        assert main.node.get_fully_qualified_name() == "/test_command"
         assert args.robot == "spot"
         return 0
 
     assert main(["test_command", "spot"]) == 0
+
+
+def test_cli_configuration() -> None:  # type: ignore
+    """Asserts that CLI can affect process configuration."""
+
+    def cli() -> argparse.ArgumentParser:
+        parser = argparse.ArgumentParser("test_command")
+        parser.add_argument("robot")
+        parser.add_argument("-q", "--quiet", action="store_true")
+        parser.set_defaults(process_args=lambda args: dict(forward_logging=not args.quiet))
+        return parser
+
+    @process.main(cli(), namespace="{robot}")
+    def main(args: argparse.Namespace) -> int:
+        assert main.node is not None
+        assert main.node.get_fully_qualified_name() == "/spot/test_command"
+        assert args.robot == "spot"
+        return 0
+
+    with mock.patch("bdai_ros2_wrappers.scope.logs_to_ros") as logs_to_ros:
+        assert main(["test_command", "spot", "--quiet"]) == 0
+    assert not logs_to_ros.called