From 69c6f98d7758a82eb057119c88f9267d85e48072 Mon Sep 17 00:00:00 2001 From: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> Date: Thu, 20 Aug 2026 02:17:15 +0100 Subject: [PATCH] Avoid incompatible ros2 daemon discovery state Expose the daemon discovery configuration and verify it before using daemon-backed graph queries. Fall back to direct discovery when the running daemon uses a different or unverifiable configuration. Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> --- ros2cli/ros2cli/daemon/__init__.py | 11 +++++- ros2cli/ros2cli/node/daemon.py | 12 +++++++ ros2cli/ros2cli/node/strategy.py | 9 +++++ ros2cli/test/test_strategy.py | 56 ++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 1 deletion(-) diff --git a/ros2cli/ros2cli/daemon/__init__.py b/ros2cli/ros2cli/daemon/__init__.py index ffcfda99f..fb221182d 100644 --- a/ros2cli/ros2cli/daemon/__init__.py +++ b/ros2cli/ros2cli/daemon/__init__.py @@ -61,6 +61,14 @@ def make_xmlrpc_server() -> LocalXMLRPCServer: ) +def get_daemon_configuration(): + """Get discovery settings that affect the ros2cli daemon's ROS graph.""" + return { + 'ros_domain_id': get_ros_domain_id(), + 'rmw_implementation': rclpy.get_rmw_implementation_identifier(), + } + + def serve(server: LocalXMLRPCServer, *, timeout: float = 2 * 60 * 60): """ Serve the ros2cli daemon API using the given `server`. @@ -68,7 +76,7 @@ def serve(server: LocalXMLRPCServer, *, timeout: float = 2 * 60 * 60): :param server: an XMLRPC server instance :param timeout: how long, in seconds, to wait before shutting down the server due to inactivity. A negative value disables - the timeout (it becomes ``float('inf')``), so the server runs + the timeout (it becomes ``float('inf')``), so the daemon runs until explicitly stopped. """ # A negative timeout means "never time out"; mirror the convention @@ -83,6 +91,7 @@ def serve(server: LocalXMLRPCServer, *, timeout: float = 2 * 60 * 60): with NetworkAwareNode(node_args) as node: daemon_logger = node.get_logger() functions = [ + get_daemon_configuration, node.get_name, node.get_namespace, node.get_node_names_and_namespaces, diff --git a/ros2cli/ros2cli/node/daemon.py b/ros2cli/ros2cli/node/daemon.py index 7dbca380e..2008f2bee 100644 --- a/ros2cli/ros2cli/node/daemon.py +++ b/ros2cli/ros2cli/node/daemon.py @@ -54,6 +54,18 @@ def connected(self): def methods(self): return self._methods + @property + def configuration_matches(self): + """Return whether the daemon uses this process's ROS discovery configuration.""" + if 'get_daemon_configuration' not in self._methods: + return False + configuration = self._proxy.get_daemon_configuration() + return ( + configuration.get('ros_domain_id') == get_ros_domain_id() and + configuration.get('rmw_implementation') == + rclpy.get_rmw_implementation_identifier() + ) + def __enter__(self): self._proxy.__enter__() return self diff --git a/ros2cli/ros2cli/node/strategy.py b/ros2cli/ros2cli/node/strategy.py index 2a52508e1..fd6cbf4dd 100644 --- a/ros2cli/ros2cli/node/strategy.py +++ b/ros2cli/ros2cli/node/strategy.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import sys from typing import Optional from ros2cli.helpers import check_discovery_configuration @@ -36,6 +37,14 @@ def __init__(self, args, *, node_name: Optional[str] = None): if not self._daemon_node.connected: self._direct_node = DirectNode(args, node_name=node_name) self._daemon_node = None + elif not self._daemon_node.configuration_matches: + print( + 'WARNING: the running ROS 2 daemon uses a different or unverifiable ' + 'discovery configuration. Falling back to direct discovery.', + file=sys.stderr + ) + self._direct_node = DirectNode(args, node_name=node_name) + self._daemon_node = None else: if use_daemon: spawn_daemon(args) diff --git a/ros2cli/test/test_strategy.py b/ros2cli/test/test_strategy.py index baaa414b4..b0fad5c89 100644 --- a/ros2cli/test/test_strategy.py +++ b/ros2cli/test/test_strategy.py @@ -13,12 +13,16 @@ # limitations under the License. import argparse +from types import SimpleNamespace import pytest +import ros2cli.daemon as daemon +from ros2cli.node.daemon import DaemonNode from ros2cli.node.daemon import is_daemon_running from ros2cli.node.daemon import shutdown_daemon from ros2cli.node.daemon import spawn_daemon +import ros2cli.node.strategy as strategy_module from ros2cli.node.strategy import NodeStrategy @@ -66,3 +70,55 @@ def test_enforce_no_daemon(enforce_daemon_is_running): with NodeStrategy(args=args) as node: assert node._daemon_node is None assert node._direct_node is not None + + +def test_daemon_configuration_matches_current_process(): + node = DaemonNode(args=[]) + node._methods = ['get_daemon_configuration'] + node._proxy = SimpleNamespace( + get_daemon_configuration=daemon.get_daemon_configuration + ) + + assert node.configuration_matches + + +def test_daemon_configuration_rejects_different_rmw(): + configuration = daemon.get_daemon_configuration() + configuration['rmw_implementation'] += '_different' + node = DaemonNode(args=[]) + node._methods = ['get_daemon_configuration'] + node._proxy = SimpleNamespace( + get_daemon_configuration=lambda: configuration + ) + + assert not node.configuration_matches + + +def test_daemon_configuration_rejects_unverifiable_daemon(): + node = DaemonNode(args=[]) + node._methods = [] + + assert not node.configuration_matches + + +def test_strategy_falls_back_for_mismatched_daemon(monkeypatch, capsys): + daemon_node = SimpleNamespace( + connected=True, + configuration_matches=False, + ) + direct_node = object() + + monkeypatch.setattr(strategy_module, 'check_discovery_configuration', lambda: None) + monkeypatch.setattr(strategy_module, 'is_daemon_running', lambda args: True) + monkeypatch.setattr(strategy_module, 'DaemonNode', lambda args: daemon_node) + monkeypatch.setattr( + strategy_module, + 'DirectNode', + lambda args, node_name=None: direct_node, + ) + + strategy = NodeStrategy(args=[]) + + assert strategy._daemon_node is None + assert strategy._direct_node is direct_node + assert 'Falling back to direct discovery' in capsys.readouterr().err