From 25fe5705f1c2a6b464a47ad4425fc4049b51027c Mon Sep 17 00:00:00 2001 From: Sebastian Castro <4603398+sea-bass@users.noreply.github.com> Date: Wed, 25 Dec 2024 09:19:06 -0500 Subject: [PATCH 1/5] Drop support for ROS 2 Iron (#981) --- .github/workflows/ci.yml | 2 -- README.md | 6 ++---- .../test/capabilities/test_action_capabilities.py | 13 ++----------- 3 files changed, 4 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 85c92aa8..0c8dd8bf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,8 +22,6 @@ jobs: # Test supported ROS 2 distributions # https://docs.ros.org/en/rolling/Releases.html # NOTE: Humble does not work on the `ros2` branch, so it is tested in its own branch. - - ros: iron - os: ubuntu-22.04 - ros: jazzy os: ubuntu-24.04 - ros: rolling diff --git a/README.md b/README.md index fe42d9d7..9bcc970d 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,6 @@ rosbridge_suite =============== [![ROS Humble version](https://img.shields.io/ros/v/humble/rosbridge_suite)](https://index.ros.org/p/rosbridge_suite/github-RobotWebTools-rosbridge_suite/#humble) -[![ROS Iron version](https://img.shields.io/ros/v/iron/rosbridge_suite)](https://index.ros.org/p/rosbridge_suite/github-RobotWebTools-rosbridge_suite/#iron) [![ROS Jazzy version](https://img.shields.io/ros/v/jazzy/rosbridge_suite)](https://index.ros.org/p/rosbridge_suite/github-RobotWebTools-rosbridge_suite/#jazzy) [![ROS Rolling version](https://img.shields.io/ros/v/rolling/rosbridge_suite)](https://index.ros.org/p/rosbridge_suite/github-RobotWebTools-rosbridge_suite/#rolling) @@ -56,11 +55,10 @@ Releasing requires push access to [RobotWebTools/rosbridge_suite](https://github 3. Run `catkin_prepare_release --bump [major/minor/patch]` to bump versions in package.xml and push changes to origin. 4. Run bloom-release commands to create PRs to update rosdistro: - `bloom-release --rosdistro humble rosbridge_suite` - - `bloom-release --rosdistro iron rosbridge_suite` - `bloom-release --rosdistro jazzy rosbridge_suite` - `bloom-release --rosdistro rolling rosbridge_suite` -Note that right now, the Humble release is tracked in the `humble` branch, while Iron and later are tracked in the `ros2` branch. +Note that right now, the Humble release is tracked in the `humble` branch, while Jazzy and later are tracked in the `ros2` branch. Once the PRs are merged, packages will be available for each distro after the next sync. -Build/sync status can be viewed at: [humble](http://repo.ros2.org/status_page/ros_humble_default.html), [iron](http://repo.ros2.org/status_page/ros_iron_default.html), [jazzy](http://repo.ros2.org/status_page/ros_jazzy_default.html), and [rolling](http://repo.ros2.org/status_page/ros_rolling_default.html). +Build/sync status can be viewed at: [humble](http://repo.ros2.org/status_page/ros_humble_default.html), [jazzy](http://repo.ros2.org/status_page/ros_jazzy_default.html), and [rolling](http://repo.ros2.org/status_page/ros_rolling_default.html). diff --git a/rosbridge_library/test/capabilities/test_action_capabilities.py b/rosbridge_library/test/capabilities/test_action_capabilities.py index 61804f34..f1752c16 100755 --- a/rosbridge_library/test/capabilities/test_action_capabilities.py +++ b/rosbridge_library/test/capabilities/test_action_capabilities.py @@ -7,7 +7,7 @@ import rclpy from action_msgs.msg import GoalStatus from example_interfaces.action._fibonacci import Fibonacci_FeedbackMessage -from rclpy.executors import SingleThreadedExecutor +from rclpy.executors import MultiThreadedExecutor from rclpy.node import Node from rclpy.qos import DurabilityPolicy, QoSProfile, ReliabilityPolicy from rosbridge_library.capabilities.action_feedback import ActionFeedback @@ -25,7 +25,7 @@ class TestActionCapabilities(unittest.TestCase): def setUp(self): rclpy.init() - self.executor = SingleThreadedExecutor() + self.executor = MultiThreadedExecutor() self.node = Node("test_action_capabilities") self.executor.add_node(self.node) @@ -97,9 +97,6 @@ def test_advertise_action(self): ) self.advertise.advertise_action(advertise_msg) - @unittest.skip( - reason="Currently fails in Iron due to https://github.com/ros2/rclpy/issues/1195. Unskip when Iron is EOL in Nov 2024." - ) def test_execute_advertised_action(self): # Advertise the action action_path = "/fibonacci_action_2" @@ -205,9 +202,6 @@ def test_execute_advertised_action(self): self.assertEqual(self.received_message["values"]["sequence"], [0, 1, 1, 2, 3, 5]) self.assertEqual(self.received_message["status"], GoalStatus.STATUS_SUCCEEDED) - @unittest.skip( - reason="Currently fails in due to https://github.com/ros2/rclpy/issues/1195, need to fix this" - ) def test_cancel_advertised_action(self): # Advertise the action action_path = "/fibonacci_action_3" @@ -301,7 +295,6 @@ def test_cancel_advertised_action(self): self.assertEqual(self.received_message["values"]["sequence"], []) self.assertEqual(self.received_message["status"], GoalStatus.STATUS_CANCELED) - @unittest.skip("Currently raises an exception not catchable by unittest, need to fix this") def test_unadvertise_action(self): # Advertise the action action_path = "/fibonacci_action_4" @@ -346,8 +339,6 @@ def test_unadvertise_action(self): self.assertTrue("id" in self.received_message) # Now unadvertise the action - # TODO: This raises an exception, likely because of the following rclpy issue: - # https://github.com/ros2/rclpy/issues/1098 unadvertise_msg = loads(dumps({"op": "unadvertise_action", "action": action_path})) self.received_message = None self.unadvertise.unadvertise_action(unadvertise_msg) From 5444d9b7e801af061dd479044a2e0e01b496e0e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C5=82a=C5=BCej=20Sowa?= Date: Wed, 25 Dec 2024 15:26:46 +0100 Subject: [PATCH 2/5] Add ament_mypy test and fix all mypy errors (#980) --- rosapi/CMakeLists.txt | 3 ++ rosapi/package.xml | 1 + rosbridge_library/CMakeLists.txt | 3 ++ rosbridge_library/package.xml | 1 + .../capabilities/advertise_action.py | 12 +++---- .../capabilities/defragmentation.py | 2 +- .../capabilities/send_action_goal.py | 5 +-- .../capabilities/subscribe.py | 4 +-- .../src/rosbridge_library/internal/actions.py | 16 ++++----- .../rosbridge_library/internal/ros_loader.py | 10 +++--- .../rosbridge_library/internal/services.py | 16 ++++----- .../src/rosbridge_library/protocol.py | 5 +-- .../src/rosbridge_library/util/__init__.py | 4 +-- ...test_non-ros_service_client_complex-srv.py | 16 ++++----- .../test_non-ros_service_client_fragmented.py | 19 ++++++----- rosbridge_server/CMakeLists.txt | 3 ++ rosbridge_server/package.xml | 1 + .../test/websocket/advertise_action.test.py | 7 ++++ .../advertise_action_feedback.test.py | 8 ++++- .../test/websocket/advertise_service.test.py | 1 + .../advertise_service_duplicate.test.py | 1 + .../websocket/best_effort_publisher.test.py | 1 + .../test/websocket/call_service.test.py | 1 + rosbridge_server/test/websocket/common.py | 34 +++++++++++-------- .../websocket/multiple_subscribers.test.py | 1 + .../multiple_subscribers_raw.test.py | 1 + .../test/websocket/send_action_goal.test.py | 1 + rosbridge_server/test/websocket/smoke.test.py | 1 + .../transient_local_publisher.test.py | 1 + 29 files changed, 111 insertions(+), 68 deletions(-) diff --git a/rosapi/CMakeLists.txt b/rosapi/CMakeLists.txt index c9d5f7b8..923abe8d 100644 --- a/rosapi/CMakeLists.txt +++ b/rosapi/CMakeLists.txt @@ -25,4 +25,7 @@ if(BUILD_TESTING) find_package(ament_cmake_pytest REQUIRED) ament_add_pytest_test(${PROJECT_NAME}_test_stringify_field_types test/test_stringify_field_types.py) ament_add_pytest_test(${PROJECT_NAME}_test_typedefs test/test_typedefs.py) + + find_package(ament_cmake_mypy REQUIRED) + ament_mypy() endif() diff --git a/rosapi/package.xml b/rosapi/package.xml index 3457970c..f5317efd 100644 --- a/rosapi/package.xml +++ b/rosapi/package.xml @@ -34,6 +34,7 @@ rosgraph --> + ament_cmake_mypy ament_cmake_pytest sensor_msgs shape_msgs diff --git a/rosbridge_library/CMakeLists.txt b/rosbridge_library/CMakeLists.txt index dce17cda..7373b0c5 100644 --- a/rosbridge_library/CMakeLists.txt +++ b/rosbridge_library/CMakeLists.txt @@ -14,4 +14,7 @@ if (BUILD_TESTING) find_package(ament_cmake_pytest REQUIRED) ament_add_pytest_test(test_capabilities "test/capabilities/") ament_add_pytest_test(test_internal "test/internal/") + + find_package(ament_cmake_mypy REQUIRED) + ament_mypy() endif() diff --git a/rosbridge_library/package.xml b/rosbridge_library/package.xml index 1b4f38d7..a1991536 100644 --- a/rosbridge_library/package.xml +++ b/rosbridge_library/package.xml @@ -31,6 +31,7 @@ rosbridge_test_msgs action_msgs + ament_cmake_mypy ament_cmake_pytest builtin_interfaces diagnostic_msgs diff --git a/rosbridge_library/src/rosbridge_library/capabilities/advertise_action.py b/rosbridge_library/src/rosbridge_library/capabilities/advertise_action.py index 9cff54d8..02042537 100644 --- a/rosbridge_library/src/rosbridge_library/capabilities/advertise_action.py +++ b/rosbridge_library/src/rosbridge_library/capabilities/advertise_action.py @@ -33,11 +33,11 @@ import fnmatch from typing import Any -import rclpy from action_msgs.msg import GoalStatus from rclpy.action import ActionServer from rclpy.action.server import CancelResponse, ServerGoalHandle from rclpy.callback_groups import ReentrantCallbackGroup +from rclpy.task import Future from rosbridge_library.capability import Capability from rosbridge_library.internal import message_conversion from rosbridge_library.internal.ros_loader import get_action_class @@ -51,9 +51,9 @@ class AdvertisedActionHandler: def __init__( self, action_name: str, action_type: str, protocol: Protocol, sleep_time: float = 0.001 ) -> None: - self.goal_futures = {} - self.goal_handles = {} - self.goal_statuses = {} + self.goal_futures: dict[str, Future] = {} + self.goal_handles: dict[str, Any] = {} + self.goal_statuses: dict[str, GoalStatus] = {} self.action_name = action_name self.action_type = action_type @@ -79,7 +79,7 @@ async def execute_callback(self, goal: Any) -> Any: # generate a unique ID goal_id = f"action_goal:{self.action_name}:{self.next_id()}" - def done_callback(fut: rclpy.task.Future) -> None: + def done_callback(fut: Future) -> None: if fut.cancelled(): goal.abort() self.protocol.log("info", f"Aborted goal {goal_id}") @@ -94,7 +94,7 @@ def done_callback(fut: rclpy.task.Future) -> None: else: goal.abort() - future = rclpy.task.Future() + future: Future = Future() future.add_done_callback(done_callback) self.goal_handles[goal_id] = goal self.goal_futures[goal_id] = future diff --git a/rosbridge_library/src/rosbridge_library/capabilities/defragmentation.py b/rosbridge_library/src/rosbridge_library/capabilities/defragmentation.py index 1757a117..7809622a 100644 --- a/rosbridge_library/src/rosbridge_library/capabilities/defragmentation.py +++ b/rosbridge_library/src/rosbridge_library/capabilities/defragmentation.py @@ -30,7 +30,7 @@ def spam(self): # } # }, # ... - lists = {} + lists: dict[str, dict] = {} def __init__(self): """Create singleton instance""" diff --git a/rosbridge_library/src/rosbridge_library/capabilities/send_action_goal.py b/rosbridge_library/src/rosbridge_library/capabilities/send_action_goal.py index 089d8456..db74333d 100644 --- a/rosbridge_library/src/rosbridge_library/capabilities/send_action_goal.py +++ b/rosbridge_library/src/rosbridge_library/capabilities/send_action_goal.py @@ -33,6 +33,7 @@ import fnmatch from functools import partial from threading import Thread +from typing import Any from action_msgs.msg import GoalStatus from rosbridge_library.capability import Capability @@ -52,7 +53,7 @@ class SendActionGoal(Capability): cancel_action_goal_msg_fields = [(True, "action", str)] actions_glob = None - client_handler_list = {} + client_handler_list: dict[str, ActionClientHandler] = {} def __init__(self, protocol: Protocol) -> None: # Call superclass constructor @@ -182,7 +183,7 @@ def _failure(self, cid: str, action: str, exc: Exception) -> None: outgoing_message["id"] = cid self.protocol.send(outgoing_message) - def _feedback(self, cid: str, action: str, message: dict) -> None: + def _feedback(self, cid: str, action: str, message: Any) -> None: outgoing_message = { "op": "action_feedback", "action": action, diff --git a/rosbridge_library/src/rosbridge_library/capabilities/subscribe.py b/rosbridge_library/src/rosbridge_library/capabilities/subscribe.py index c54e8009..0f523ab2 100644 --- a/rosbridge_library/src/rosbridge_library/capabilities/subscribe.py +++ b/rosbridge_library/src/rosbridge_library/capabilities/subscribe.py @@ -40,10 +40,10 @@ from rosbridge_library.internal.subscription_modifiers import MessageHandler try: - from ujson import dumps as encode_json + from ujson import dumps as encode_json # type: ignore[import-untyped] except ImportError: try: - from simplejson import dumps as encode_json + from simplejson import dumps as encode_json # type: ignore[import-untyped] except ImportError: from json import dumps as encode_json diff --git a/rosbridge_library/src/rosbridge_library/internal/actions.py b/rosbridge_library/src/rosbridge_library/internal/actions.py index efc9211c..8e3a0273 100644 --- a/rosbridge_library/src/rosbridge_library/internal/actions.py +++ b/rosbridge_library/src/rosbridge_library/internal/actions.py @@ -32,7 +32,7 @@ import time from threading import Thread -from typing import Any, Callable, Optional, Union +from typing import Any, Callable, Optional from rclpy.action import ActionClient from rclpy.expand_topic_name import expand_topic_name @@ -59,9 +59,9 @@ def __init__( action: str, action_type: str, args: dict, - success_callback: Callable[[str, str, int, bool, dict], None], - error_callback: Callable[[str, str, Exception], None], - feedback_callback: Callable[[str, str, dict], None], + success_callback: Callable[[dict], None], + error_callback: Callable[[Exception], None], + feedback_callback: Optional[Callable[[dict], None]], node_handle: Node, ) -> None: """ @@ -90,12 +90,11 @@ def __init__( self.error = error_callback self.feedback = feedback_callback self.node_handle = node_handle - self.send_goal_helper = None + self.send_goal_helper = SendGoal() def run(self) -> None: try: # Call the service and pass the result to the success handler - self.send_goal_helper = SendGoal() self.success( self.send_goal_helper.send_goal( self.node_handle, @@ -110,7 +109,7 @@ def run(self) -> None: self.error(e) -def args_to_action_goal_instance(action: str, inst: Any, args: Union[list, dict]) -> Any: +def args_to_action_goal_instance(action: str, inst: Any, args: list | dict | None) -> Any: """ " Populate an action goal instance with the provided args @@ -142,6 +141,7 @@ def get_result_cb(self, future: Future) -> None: def goal_response_cb(self, future: Future) -> None: self.goal_handle = future.result() + assert self.goal_handle is not None if not self.goal_handle.accepted: raise Exception("Action goal was rejected") result_future = self.goal_handle.get_result_async() @@ -156,7 +156,7 @@ def send_goal( action: str, action_type: str, args: Optional[dict] = None, - feedback_cb: Optional[Callable[[str, str, dict], None]] = None, + feedback_cb: Optional[Callable[[dict], None]] = None, ) -> dict: # Given the action name and type, fetch a request instance action_name = expand_topic_name(action, node_handle.get_name(), node_handle.get_namespace()) diff --git a/rosbridge_library/src/rosbridge_library/internal/ros_loader.py b/rosbridge_library/src/rosbridge_library/internal/ros_loader.py index c45abb89..d3efa166 100644 --- a/rosbridge_library/src/rosbridge_library/internal/ros_loader.py +++ b/rosbridge_library/src/rosbridge_library/internal/ros_loader.py @@ -43,9 +43,9 @@ """ # Variable containing the loaded classes -_loaded_msgs = {} -_loaded_srvs = {} -_loaded_actions = {} +_loaded_msgs: dict[str, Any] = {} +_loaded_srvs: dict[str, Any] = {} +_loaded_actions: dict[str, Any] = {} _msgs_lock = Lock() _srvs_lock = Lock() _actions_lock = Lock() @@ -185,7 +185,7 @@ def _get_class(typestring: str, subname: str, cache: Dict[str, Any], lock: Lock) return cls -def _load_class(modname: str, subname: str, classname: str) -> None: +def _load_class(modname: str, subname: str, classname: str) -> Any: """Loads the manifest and imports the module that contains the specified type. @@ -220,7 +220,7 @@ def _splittype(typestring: str) -> Tuple[str, str]: raise InvalidTypeStringException(typestring) -def _add_to_cache(cache: Dict[str, Any], lock: Lock, key: str, value: any) -> None: +def _add_to_cache(cache: Dict[str, Any], lock: Lock, key: str, value: Any) -> None: lock.acquire() cache[key] = value lock.release() diff --git a/rosbridge_library/src/rosbridge_library/internal/services.py b/rosbridge_library/src/rosbridge_library/internal/services.py index 59a76da7..547877ef 100644 --- a/rosbridge_library/src/rosbridge_library/internal/services.py +++ b/rosbridge_library/src/rosbridge_library/internal/services.py @@ -58,8 +58,8 @@ def __init__( self, service: str, args: dict, - success_callback: Callable[[str, str, int, bool, Any], None], - error_callback: Callable[[str, str, Exception], None], + success_callback: Callable[[dict], None], + error_callback: Callable[[Exception], None], node_handle: Node, ) -> None: """Create a service caller for the specified service. Use start() @@ -94,7 +94,7 @@ def run(self) -> None: self.error(e) -def args_to_service_request_instance(service: str, inst: Any, args: dict) -> Any: +def args_to_service_request_instance(service: str, inst: Any, args: list | dict | None) -> Any: """Populate a service request instance with the provided args args can be a dictionary of values, or a list, or None @@ -122,14 +122,14 @@ def call_service( service = expand_topic_name(service, node_handle.get_name(), node_handle.get_namespace()) service_names_and_types = dict(node_handle.get_service_names_and_types()) - service_type = service_names_and_types.get(service) - if service_type is None: + service_types = service_names_and_types.get(service) + if service_types is None: raise InvalidServiceException(service) # service_type is a tuple of types at this point; only one type is supported. - if len(service_type) > 1: - node_handle.get_logger().warning(f"More than one service type detected: {service_type}") - service_type = service_type[0] + if len(service_types) > 1: + node_handle.get_logger().warning(f"More than one service type detected: {service_types}") + service_type = service_types[0] service_class = get_service_class(service_type) inst = get_service_request_instance(service_type) diff --git a/rosbridge_library/src/rosbridge_library/protocol.py b/rosbridge_library/src/rosbridge_library/protocol.py index 9d91076c..9d6e54f6 100644 --- a/rosbridge_library/src/rosbridge_library/protocol.py +++ b/rosbridge_library/src/rosbridge_library/protocol.py @@ -31,6 +31,7 @@ # POSSIBILITY OF SUCH DAMAGE. import time +from typing import Any from rosbridge_library.capabilities.fragmentation import Fragmentation from rosbridge_library.util import bson, json @@ -81,9 +82,9 @@ class Protocol: # !! this might be related to (or even be avoided by using) throttle_rate !! delay_between_messages = 0 # global list of non-ros advertised services - external_service_list = {} + external_service_list: dict[str, Any] = {} # global list of non-ros advertised actions - external_action_list = {} + external_action_list: dict[str, Any] = {} # Use only BSON for the whole communication if the server has been started with bson_only_mode:=True bson_only_mode = False diff --git a/rosbridge_library/src/rosbridge_library/util/__init__.py b/rosbridge_library/src/rosbridge_library/util/__init__.py index 8b04567e..359021ae 100644 --- a/rosbridge_library/src/rosbridge_library/util/__init__.py +++ b/rosbridge_library/src/rosbridge_library/util/__init__.py @@ -1,9 +1,9 @@ # try to import json-lib: 1st try ujson, 2nd try simplejson, else import standard Python json try: - import ujson as json + import ujson as json # type: ignore[import-untyped] except ImportError: try: - import simplejson as json + import simplejson as json # type: ignore[import-untyped] except ImportError: import json # noqa: F401 diff --git a/rosbridge_library/test/experimental/complex_srv+tcp/test_non-ros_service_client_complex-srv.py b/rosbridge_library/test/experimental/complex_srv+tcp/test_non-ros_service_client_complex-srv.py index eefc39f2..24f39f3c 100755 --- a/rosbridge_library/test/experimental/complex_srv+tcp/test_non-ros_service_client_complex-srv.py +++ b/rosbridge_library/test/experimental/complex_srv+tcp/test_non-ros_service_client_complex-srv.py @@ -67,13 +67,13 @@ def request_service(): try: incoming = sock.recv(max_msg_length) # receive service_response from rosbridge if buffer == "": - buffer = incoming + buffer = incoming.decode("utf-8") if incoming == "": print("closing socket") sock.close() break else: - buffer = buffer + incoming + buffer = buffer + incoming.decode("utf-8") # print "buffer-length:", len(buffer) try: # try to access service_request directly (not fragmented) data_object = json.loads(buffer) @@ -90,14 +90,14 @@ def request_service(): "}{" ) # split buffer into fragments and re-fill curly brackets result = [] - for fragment in result_string: - if fragment[0] != "{": - fragment = "{" + fragment - if fragment[len(fragment) - 1] != "}": - fragment = fragment + "}" + for fragment_str in result_string: + if fragment_str[0] != "{": + fragment_str = "{" + fragment_str + if fragment_str[len(fragment_str) - 1] != "}": + fragment_str = fragment_str + "}" try: result.append( - json.loads(fragment) + json.loads(fragment_str) ) # try to parse json from string, and append if successful except Exception: # print(e) diff --git a/rosbridge_library/test/experimental/fragmentation+srv+tcp/test_non-ros_service_client_fragmented.py b/rosbridge_library/test/experimental/fragmentation+srv+tcp/test_non-ros_service_client_fragmented.py index 368d6c04..e80ec962 100755 --- a/rosbridge_library/test/experimental/fragmentation+srv+tcp/test_non-ros_service_client_fragmented.py +++ b/rosbridge_library/test/experimental/fragmentation+srv+tcp/test_non-ros_service_client_fragmented.py @@ -1,5 +1,6 @@ #!/usr/bin/python import socket +from typing import Any from rosbridge_library.util import json @@ -63,13 +64,13 @@ def request_service(): try: incoming = sock.recv(max_msg_length) # receive service_response from rosbridge if buffer == "": - buffer = incoming + buffer = incoming.decode("utf-8") if incoming == "": print("closing socket") sock.close() break else: - buffer = buffer + incoming + buffer = buffer + incoming.decode("utf-8") # print "buffer-length:", len(buffer) try: # try to access service_request directly (not fragmented) data_object = json.loads(buffer) @@ -86,14 +87,14 @@ def request_service(): "}{" ) # split buffer into fragments and re-fill curly brackets result = [] - for fragment in result_string: - if fragment[0] != "{": - fragment = "{" + fragment - if fragment[len(fragment) - 1] != "}": - fragment = fragment + "}" + for fragment_str in result_string: + if fragment_str[0] != "{": + fragment_str = "{" + fragment_str + if fragment_str[len(fragment_str) - 1] != "}": + fragment_str = fragment_str + "}" try: result.append( - json.loads(fragment) + json.loads(fragment_str) ) # try to parse json from string, and append if successful except Exception: # print(e) @@ -105,7 +106,7 @@ def request_service(): announced = int(result[0]["total"]) if fragment_count == announced: # if all fragments received --> sort and defragment # sort fragments - sorted_result = [None] * fragment_count + sorted_result: list[Any] = [None] * fragment_count unsorted_result = [] for fragment in result: unsorted_result.append(fragment) diff --git a/rosbridge_server/CMakeLists.txt b/rosbridge_server/CMakeLists.txt index b979bc35..048f6cbd 100644 --- a/rosbridge_server/CMakeLists.txt +++ b/rosbridge_server/CMakeLists.txt @@ -32,4 +32,7 @@ if(BUILD_TESTING) add_launch_test(test/websocket/transient_local_publisher.test.py) add_launch_test(test/websocket/best_effort_publisher.test.py) add_launch_test(test/websocket/multiple_subscribers_raw.test.py) + + find_package(ament_cmake_mypy REQUIRED) + ament_mypy() endif() diff --git a/rosbridge_server/package.xml b/rosbridge_server/package.xml index 4520a09b..31bdd65c 100644 --- a/rosbridge_server/package.xml +++ b/rosbridge_server/package.xml @@ -24,6 +24,7 @@ rosbridge_msgs rosapi + ament_cmake_mypy example_interfaces python3-autobahn launch diff --git a/rosbridge_server/test/websocket/advertise_action.test.py b/rosbridge_server/test/websocket/advertise_action.test.py index 5021aa63..84997a69 100644 --- a/rosbridge_server/test/websocket/advertise_action.test.py +++ b/rosbridge_server/test/websocket/advertise_action.test.py @@ -7,6 +7,7 @@ from example_interfaces.action import Fibonacci from rclpy.action import ActionClient from rclpy.node import Node +from rclpy.task import Future from twisted.python import log sys.path.append(os.path.dirname(__file__)) # enable importing from common.py in this directory @@ -20,6 +21,9 @@ class TestAdvertiseAction(unittest.TestCase): + goal1_result_future: Future | None + goal2_result_future: Future | None + def goal1_response_callback(self, future): goal_handle = future.result() if not goal_handle.accepted: @@ -48,6 +52,7 @@ async def test_two_concurrent_calls(self, node: Node, make_client): requests_future, ws_client.message_handler = expect_messages( 2, "WebSocket", node.get_logger() ) + assert node.executor is not None requests_future.add_done_callback(lambda _: node.executor.wake()) self.goal1_result_future = None @@ -90,8 +95,10 @@ async def test_two_concurrent_calls(self, node: Node, make_client): } ) + assert self.goal1_result_future is not None result1 = await self.goal1_result_future self.assertEqual(result1.result, Fibonacci.Result(sequence=[0, 1, 1, 2])) + assert self.goal2_result_future is not None result2 = await self.goal2_result_future self.assertEqual(result2.result, Fibonacci.Result(sequence=[0, 1, 1, 2, 3, 5])) diff --git a/rosbridge_server/test/websocket/advertise_action_feedback.test.py b/rosbridge_server/test/websocket/advertise_action_feedback.test.py index 69b2c686..af3f983c 100644 --- a/rosbridge_server/test/websocket/advertise_action_feedback.test.py +++ b/rosbridge_server/test/websocket/advertise_action_feedback.test.py @@ -7,6 +7,7 @@ from example_interfaces.action import Fibonacci from rclpy.action import ActionClient from rclpy.node import Node +from rclpy.task import Future from twisted.python import log sys.path.append(os.path.dirname(__file__)) # enable importing from common.py in this directory @@ -20,8 +21,11 @@ class TestActionFeedback(unittest.TestCase): - def goal_response_callback(self, future): + goal_result_future: Future | None + + def goal_response_callback(self, future: Future): goal_handle = future.result() + assert goal_handle is not None if not goal_handle.accepted: return self.goal_result_future = goal_handle.get_result_async() @@ -45,6 +49,7 @@ async def test_feedback(self, node: Node, make_client): requests_future, ws_client.message_handler = expect_messages( 1, "WebSocket", node.get_logger() ) + assert node.executor is not None requests_future.add_done_callback(lambda _: node.executor.wake()) self.goal_result_future = None @@ -81,6 +86,7 @@ async def test_feedback(self, node: Node, make_client): } ) + assert self.goal_result_future is not None result = await self.goal_result_future self.assertIsNotNone(self.latest_feedback) self.assertEqual(self.latest_feedback.feedback, Fibonacci.Feedback(sequence=[0, 1, 1, 2])) diff --git a/rosbridge_server/test/websocket/advertise_service.test.py b/rosbridge_server/test/websocket/advertise_service.test.py index 6097dc3f..724ffdf7 100644 --- a/rosbridge_server/test/websocket/advertise_service.test.py +++ b/rosbridge_server/test/websocket/advertise_service.test.py @@ -34,6 +34,7 @@ async def test_two_concurrent_calls(self, node: Node, make_client): requests_future, ws_client.message_handler = expect_messages( 2, "WebSocket", node.get_logger() ) + assert node.executor is not None requests_future.add_done_callback(lambda _: node.executor.wake()) response1_future = client.call_async(SetBool.Request(data=True)) diff --git a/rosbridge_server/test/websocket/advertise_service_duplicate.test.py b/rosbridge_server/test/websocket/advertise_service_duplicate.test.py index 466e4435..08ba0779 100644 --- a/rosbridge_server/test/websocket/advertise_service_duplicate.test.py +++ b/rosbridge_server/test/websocket/advertise_service_duplicate.test.py @@ -33,6 +33,7 @@ async def test_double_advertise(self, node: Node, make_client): requests1_future, ws_client1.message_handler = expect_messages( 1, "WebSocket 1", node.get_logger() ) + assert node.executor is not None requests1_future.add_done_callback(lambda _: node.executor.wake()) client.call_async(SetBool.Request(data=True)) diff --git a/rosbridge_server/test/websocket/best_effort_publisher.test.py b/rosbridge_server/test/websocket/best_effort_publisher.test.py index 2c84e690..f011b885 100644 --- a/rosbridge_server/test/websocket/best_effort_publisher.test.py +++ b/rosbridge_server/test/websocket/best_effort_publisher.test.py @@ -44,6 +44,7 @@ async def test_best_effort_publisher(self, node: Node, make_client): ws1_completed_future, ws_client1.message_handler = expect_messages( 1, "WebSocket 1", node.get_logger() ) + assert node.executor is not None ws1_completed_future.add_done_callback(lambda _: node.executor.wake()) self.assertEqual( diff --git a/rosbridge_server/test/websocket/call_service.test.py b/rosbridge_server/test/websocket/call_service.test.py index c8e04181..b6e8948b 100644 --- a/rosbridge_server/test/websocket/call_service.test.py +++ b/rosbridge_server/test/websocket/call_service.test.py @@ -35,6 +35,7 @@ def service_cb(req, res): responses_future, ws_client.message_handler = expect_messages( 1, "WebSocket", node.get_logger() ) + assert node.executor is not None responses_future.add_done_callback(lambda _: node.executor.wake()) ws_client.sendJson( diff --git a/rosbridge_server/test/websocket/common.py b/rosbridge_server/test/websocket/common.py index 450534ae..4cc2f7ec 100644 --- a/rosbridge_server/test/websocket/common.py +++ b/rosbridge_server/test/websocket/common.py @@ -7,9 +7,11 @@ import rclpy import rclpy.task from autobahn.twisted.websocket import WebSocketClientFactory, WebSocketClientProtocol +from launch.launch_description import LaunchDescription from rcl_interfaces.srv import GetParameters from rclpy.executors import SingleThreadedExecutor from rclpy.node import Node +from rclpy.task import Future from twisted.internet import reactor from twisted.internet.endpoints import TCP4ClientEndpoint @@ -23,7 +25,7 @@ class TestClientProtocol(WebSocketClientProtocol): def __init__(self, *args, **kwargs): self.received = [] - self.connected_future = rclpy.task.Future() + self.connected_future = Future() self.message_handler = lambda _: None super().__init__(*args, **kwargs) @@ -60,19 +62,19 @@ def _generate_node(): try: from launch_testing.actions import ReadyToTest - def generate_test_description() -> launch.LaunchDescription: + def generate_test_description() -> LaunchDescription: """ Generate a launch description that runs the websocket server. Re-export this from a test file and use add_launch_test() to run the test. """ - return launch.LaunchDescription([_generate_node(), ReadyToTest()]) + return LaunchDescription([_generate_node(), ReadyToTest()]) except ImportError: - def generate_test_description(ready_fn) -> launch.LaunchDescription: + def generate_test_description(ready_fn) -> LaunchDescription: # type: ignore[misc] """ Generate a launch description that runs the websocket server. Re-export this from a test file and use add_launch_test() to run the test. """ - return launch.LaunchDescription( + return LaunchDescription( [_generate_node(), launch.actions.OpaqueFunction(function=lambda context: ready_fn())] ) @@ -86,6 +88,7 @@ async def get_server_port(node: Node) -> int: if not client.wait_for_service(5): raise RuntimeError("GetParameters service not available") port_param = await client.call_async(GetParameters.Request(names=["actual_port"])) + assert port_param is not None return port_param.values[0].integer_value finally: node.destroy_client(client) @@ -96,18 +99,21 @@ async def connect_to_server(node: Node) -> TestClientProtocol: factory = WebSocketClientFactory("ws://127.0.0.1:" + str(port)) factory.protocol = TestClientProtocol - future = rclpy.task.Future() - future.add_done_callback(lambda _: node.executor.wake()) + future: Future = Future() + executor = node.executor + assert executor is not None + future.add_done_callback(lambda _: executor.wake()) def connect(): TCP4ClientEndpoint(reactor, "127.0.0.1", port).connect(factory).addCallback( future.set_result ) - reactor.callFromThread(connect) + reactor.callFromThread(connect) # type: ignore[attr-defined] - protocol = await future - protocol.connected_future.add_done_callback(lambda _: node.executor.wake()) + protocol: TestClientProtocol | None = await future + assert protocol is not None + protocol.connected_future.add_done_callback(lambda _: executor.wake()) await protocol.connected_future # wait for onOpen before proceeding return protocol @@ -128,8 +134,8 @@ async def task(): future = executor.create_task(task) - reactor.callInThread(executor.spin_until_future_complete, future) - reactor.run(installSignalHandlers=False) + reactor.callInThread(executor.spin_until_future_complete, future) # type: ignore[attr-defined] + reactor.run(installSignalHandlers=False) # type: ignore[attr-defined] executor.remove_node(node) node.destroy_node() @@ -140,7 +146,7 @@ def sleep(node: Node, duration: float) -> Awaitable[None]: """ Async-compatible delay function based on a ROS timer. """ - future = rclpy.task.Future() + future: Future = Future() def callback(): future.set_result(None) @@ -169,7 +175,7 @@ def expect_messages(count: int, description: str, logger): Convenience function to create a Future and a message handler function which gathers results into a list and waits for the list to have the expected number of items. """ - future = rclpy.Future() + future: Future = Future() results = [] def handler(msg): diff --git a/rosbridge_server/test/websocket/multiple_subscribers.test.py b/rosbridge_server/test/websocket/multiple_subscribers.test.py index 58aaadb1..f4908048 100644 --- a/rosbridge_server/test/websocket/multiple_subscribers.test.py +++ b/rosbridge_server/test/websocket/multiple_subscribers.test.py @@ -40,6 +40,7 @@ async def test_multiple_subscribers(self, node: Node, make_client): ws1_completed_future, ws_client1.message_handler = expect_messages( 1, "WebSocket 1", node.get_logger() ) + assert node.executor is not None ws1_completed_future.add_done_callback(lambda _: node.executor.wake()) ws2_completed_future, ws_client2.message_handler = expect_messages( 1, "WebSocket 2", node.get_logger() diff --git a/rosbridge_server/test/websocket/multiple_subscribers_raw.test.py b/rosbridge_server/test/websocket/multiple_subscribers_raw.test.py index a1f5c1f3..ddfe653c 100644 --- a/rosbridge_server/test/websocket/multiple_subscribers_raw.test.py +++ b/rosbridge_server/test/websocket/multiple_subscribers_raw.test.py @@ -33,6 +33,7 @@ async def test_multiple_subscribers(self, node: Node, make_client): ws1_completed_future, ws_client1.message_handler = expect_messages( 1, "WebSocket 1", node.get_logger() ) + assert node.executor is not None ws1_completed_future.add_done_callback(lambda _: node.executor.wake()) ws2_completed_future, ws_client2.message_handler = expect_messages( 1, "WebSocket 2", node.get_logger() diff --git a/rosbridge_server/test/websocket/send_action_goal.test.py b/rosbridge_server/test/websocket/send_action_goal.test.py index 1db33ab5..bd4b9b77 100644 --- a/rosbridge_server/test/websocket/send_action_goal.test.py +++ b/rosbridge_server/test/websocket/send_action_goal.test.py @@ -56,6 +56,7 @@ async def test_one_call(self, node: Node, make_client): responses_future, ws_client.message_handler = expect_messages( 5, "WebSocket", node.get_logger() ) + assert node.executor is not None responses_future.add_done_callback(lambda _: node.executor.wake()) ws_client.sendJson( diff --git a/rosbridge_server/test/websocket/smoke.test.py b/rosbridge_server/test/websocket/smoke.test.py index 2447ed42..f8959fa4 100644 --- a/rosbridge_server/test/websocket/smoke.test.py +++ b/rosbridge_server/test/websocket/smoke.test.py @@ -37,6 +37,7 @@ async def test_smoke(self, node: Node, make_client): ws_completed_future, ws_client.message_handler = expect_messages( NUM_MSGS, "WebSocket", node.get_logger() ) + assert node.executor is not None ws_completed_future.add_done_callback(lambda _: node.executor.wake()) sub_a = node.create_subscription(String, A_TOPIC, sub_handler, NUM_MSGS) diff --git a/rosbridge_server/test/websocket/transient_local_publisher.test.py b/rosbridge_server/test/websocket/transient_local_publisher.test.py index d492350f..1ad4ae1a 100644 --- a/rosbridge_server/test/websocket/transient_local_publisher.test.py +++ b/rosbridge_server/test/websocket/transient_local_publisher.test.py @@ -46,6 +46,7 @@ async def test_transient_local_publisher(self, node: Node, make_client): ws_completed_future, ws_client.message_handler = expect_messages( 1, "WebSocket " + str(num), node.get_logger() ) + assert node.executor is not None ws_completed_future.add_done_callback(lambda _: node.executor.wake()) self.assertEqual( From 3b30f441144bdc19dd2e577fd32caaa28b7e2676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C5=82a=C5=BCej=20Sowa?= Date: Wed, 25 Dec 2024 16:04:15 +0100 Subject: [PATCH 3/5] Use monotonic clock for time measuring (#982) --- .../rosbridge_library/capabilities/defragmentation.py | 10 +++++----- .../internal/subscription_modifiers.py | 6 +++--- .../subscribers/test_subscription_modifiers.py | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/rosbridge_library/src/rosbridge_library/capabilities/defragmentation.py b/rosbridge_library/src/rosbridge_library/capabilities/defragmentation.py index 7809622a..85187e01 100644 --- a/rosbridge_library/src/rosbridge_library/capabilities/defragmentation.py +++ b/rosbridge_library/src/rosbridge_library/capabilities/defragmentation.py @@ -1,5 +1,5 @@ import threading -from datetime import datetime +import time from rosbridge_library.capability import Capability @@ -84,13 +84,13 @@ def __init__(self, protocol): # 4.b) pass the reconstructed message string to protocol.incoming() # protocol.incoming is checking message fields by itself, so no need to do this before passing the reconstructed message to protocol # 4.c) remove the fragment list to free up memory def defragment(self, message): - now = datetime.now() + now = time.monotonic() if self.received_fragments is not None: for id in self.received_fragments.keys(): time_diff = now - self.received_fragments[id]["timestamp_last_append"] if ( - time_diff.total_seconds() > self.fragment_timeout + time_diff > self.fragment_timeout and not self.received_fragments[id]["is_reconstructing"] ): log_msg = ["fragment list ", str(id), " timed out.."] @@ -188,7 +188,7 @@ def defragment(self, message): log_msg = "".join(log_msg) self.protocol.log("debug", log_msg) - duration = datetime.now() - now + duration = time.monotonic() - now # Pass the reconstructed message to rosbridge self.protocol.incoming(reconstructed_msg) @@ -196,7 +196,7 @@ def defragment(self, message): log_msg.extend([str(msg_total), " fragments. "]) # cannot access msg.data if message is a service_response or else! # log_msg += "[message length: " + str(len(str(json.loads(reconstructed_msg)["msg"]["data"]))) +"]" - log_msg.extend(["[duration: ", str(duration.total_seconds()), " s]"]) + log_msg.extend(["[duration: ", str(duration), " s]"]) log_msg = "".join(log_msg) self.protocol.log("info", log_msg) diff --git a/rosbridge_library/src/rosbridge_library/internal/subscription_modifiers.py b/rosbridge_library/src/rosbridge_library/internal/subscription_modifiers.py index e68faee4..ac129c5c 100644 --- a/rosbridge_library/src/rosbridge_library/internal/subscription_modifiers.py +++ b/rosbridge_library/src/rosbridge_library/internal/subscription_modifiers.py @@ -31,10 +31,10 @@ # POSSIBILITY OF SUCH DAMAGE. import sys +import time import traceback from collections import deque from threading import Condition, Thread -from time import time """ Sits between incoming messages from a subscription, and the outgoing publish method. Provides throttling / buffering capabilities. @@ -66,10 +66,10 @@ def set_queue_length(self, queue_length): return self.transition() def time_remaining(self): - return max((self.last_publish + self.throttle_rate) - time(), 0) + return max((self.last_publish + self.throttle_rate) - time.monotonic(), 0) def handle_message(self, msg): - self.last_publish = time() + self.last_publish = time.monotonic() self.publish(msg) def transition(self): diff --git a/rosbridge_library/test/internal/subscribers/test_subscription_modifiers.py b/rosbridge_library/test/internal/subscribers/test_subscription_modifiers.py index 27d149b8..597961df 100755 --- a/rosbridge_library/test/internal/subscribers/test_subscription_modifiers.py +++ b/rosbridge_library/test/internal/subscribers/test_subscription_modifiers.py @@ -146,9 +146,9 @@ def cb(msg): handler.publish = cb self.assertTrue(handler.time_remaining() == 0) - t1 = time.time() + t1 = time.monotonic() handler.handle_message(msg) - t2 = time.time() + t2 = time.monotonic() self.assertEqual(received["msg"], msg) self.assertLessEqual(t1, handler.last_publish) From ffa267500eb1076f04b30bc52b6f350a5627f24f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C5=82a=C5=BCej=20Sowa?= Date: Wed, 25 Dec 2024 16:17:02 +0100 Subject: [PATCH 4/5] Fix infinite loop in QueueMessageHandler (#983) Co-authored-by: Daisuke Sato --- .../src/rosbridge_library/internal/subscription_modifiers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rosbridge_library/src/rosbridge_library/internal/subscription_modifiers.py b/rosbridge_library/src/rosbridge_library/internal/subscription_modifiers.py index ac129c5c..f05231ed 100644 --- a/rosbridge_library/src/rosbridge_library/internal/subscription_modifiers.py +++ b/rosbridge_library/src/rosbridge_library/internal/subscription_modifiers.py @@ -163,6 +163,7 @@ def run(self): traceback.print_exc(file=sys.stderr) while self.time_remaining() == 0 and len(self.queue) > 0: try: - MessageHandler.handle_message(self, self.queue[0]) + msg = self.queue.popleft() + MessageHandler.handle_message(self, msg) except Exception: traceback.print_exc(file=sys.stderr) From a00022648c92b8dab0284d19b0141bfbc8b3bd03 Mon Sep 17 00:00:00 2001 From: Lebecque Florian Date: Sat, 4 Jan 2025 20:06:39 +0100 Subject: [PATCH 5/5] Add new service to retrieve the different interfaces in the ROS Network (#988) --- rosapi/scripts/rosapi_node | 7 +++++++ rosapi/src/rosapi/proxy.py | 6 ++++++ rosapi_msgs/CMakeLists.txt | 1 + rosapi_msgs/srv/Interfaces.srv | 2 ++ 4 files changed, 16 insertions(+) create mode 100644 rosapi_msgs/srv/Interfaces.srv diff --git a/rosapi/scripts/rosapi_node b/rosapi/scripts/rosapi_node index af478ee5..013b0729 100755 --- a/rosapi/scripts/rosapi_node +++ b/rosapi/scripts/rosapi_node @@ -48,6 +48,7 @@ from rosapi_msgs.srv import ( GetROSVersion, GetTime, HasParam, + Interfaces, MessageDetails, NodeDetails, Nodes, @@ -89,6 +90,7 @@ class Rosapi(Node): full_name = self.get_namespace() + "/" + self.get_name() params.init(full_name) self.create_service(Topics, "/rosapi/topics", self.get_topics) + self.create_service(Interfaces, "/rosapi/interfaces", self.get_interfaces) self.create_service(TopicsForType, "/rosapi/topics_for_type", self.get_topics_for_type) self.create_service( TopicsAndRawTypes, @@ -137,6 +139,11 @@ class Rosapi(Node): response.topics, response.types = proxy.get_topics_and_types(self.globs.topics) return response + def get_interfaces(self, request, response): + """Called by the rosapi/Types service. Returns a list of all the types in the system.""" + response.interfaces = proxy.get_interfaces() + return response + def get_topics_for_type(self, request, response): """Called by the rosapi/TopicsForType service. Returns a list of all the topics that are publishing a given type""" response.topics = proxy.get_topics_for_type(request.type, self.globs.topics) diff --git a/rosapi/src/rosapi/proxy.py b/rosapi/src/rosapi/proxy.py index c3bdbb4a..48ed7835 100644 --- a/rosapi/src/rosapi/proxy.py +++ b/rosapi/src/rosapi/proxy.py @@ -31,6 +31,7 @@ # ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE # POSSIBILITY OF SUCH DAMAGE. +from ros2interface.api import type_completer from ros2node.api import ( get_node_names, get_publisher_info, @@ -60,6 +61,11 @@ def get_topics(topics_glob, include_hidden=False): return filter_globs(topics_glob, topic_names) +def get_interfaces(): + """Returns a list of all the types in the ROS system""" + return type_completer() + + def get_topics_and_types(topics_glob, include_hidden=False): return get_publications_and_types( topics_glob, get_topic_names_and_types, include_hidden_topics=include_hidden diff --git a/rosapi_msgs/CMakeLists.txt b/rosapi_msgs/CMakeLists.txt index 477ab48e..8764e26c 100644 --- a/rosapi_msgs/CMakeLists.txt +++ b/rosapi_msgs/CMakeLists.txt @@ -14,6 +14,7 @@ rosidl_generate_interfaces(${PROJECT_NAME} srv/GetROSVersion.srv srv/GetTime.srv srv/HasParam.srv + srv/Interfaces.srv srv/MessageDetails.srv srv/Nodes.srv srv/NodeDetails.srv diff --git a/rosapi_msgs/srv/Interfaces.srv b/rosapi_msgs/srv/Interfaces.srv new file mode 100644 index 00000000..53355592 --- /dev/null +++ b/rosapi_msgs/srv/Interfaces.srv @@ -0,0 +1,2 @@ +--- +string[] interfaces