From 13fcc0dc74c7ff484e44d5583539cd11e594b6dc Mon Sep 17 00:00:00 2001 From: Marco Boneberger Date: Thu, 14 Mar 2024 11:24:47 +0100 Subject: [PATCH 1/3] make services FnMut --- rclrs/src/node.rs | 2 +- rclrs/src/service.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rclrs/src/node.rs b/rclrs/src/node.rs index 87702ab0f..bde45f38f 100644 --- a/rclrs/src/node.rs +++ b/rclrs/src/node.rs @@ -270,7 +270,7 @@ impl Node { ) -> Result>, RclrsError> where T: rosidl_runtime_rs::Service, - F: Fn(&rmw_request_id_t, T::Request) -> T::Response + 'static + Send, + F: FnMut(&rmw_request_id_t, T::Request) -> T::Response + 'static + Send, { let service = Arc::new(Service::::new( Arc::clone(&self.rcl_node_mtx), diff --git a/rclrs/src/service.rs b/rclrs/src/service.rs index 8cac799b1..d2e0de3ea 100644 --- a/rclrs/src/service.rs +++ b/rclrs/src/service.rs @@ -47,7 +47,7 @@ pub trait ServiceBase: Send + Sync { } type ServiceCallback = - Box Response + 'static + Send>; + Box Response + 'static + Send>; /// Main class responsible for responding to requests sent by ROS clients. /// @@ -79,7 +79,7 @@ where // [`Node::create_service`], see the struct's documentation for the rationale where T: rosidl_runtime_rs::Service, - F: Fn(&rmw_request_id_t, T::Request) -> T::Response + 'static + Send, + F: FnMut(&rmw_request_id_t, T::Request) -> T::Response + 'static + Send, { // SAFETY: Getting a zero-initialized value is always safe. let mut rcl_service = unsafe { rcl_get_zero_initialized_service() }; From 07bcfe92dd8e991790386b1aa512edd6d7ef5378 Mon Sep 17 00:00:00 2001 From: Marco Boneberger Date: Fri, 15 Mar 2024 11:55:39 +0100 Subject: [PATCH 2/3] remove second generic parameter for create_service method --- examples/minimal_client_service/src/minimal_service.rs | 2 +- rclrs/src/node.rs | 5 ++--- rclrs_tests/src/graph_tests.rs | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/examples/minimal_client_service/src/minimal_service.rs b/examples/minimal_client_service/src/minimal_service.rs index b4149c817..4e5617ef0 100644 --- a/examples/minimal_client_service/src/minimal_service.rs +++ b/examples/minimal_client_service/src/minimal_service.rs @@ -18,7 +18,7 @@ fn main() -> Result<(), Error> { let node = rclrs::create_node(&context, "minimal_service")?; let _server = node - .create_service::("add_two_ints", handle_service)?; + .create_service::("add_two_ints", handle_service)?; println!("Starting server"); rclrs::spin(node).map_err(|err| err.into()) diff --git a/rclrs/src/node.rs b/rclrs/src/node.rs index bde45f38f..3ab2dae10 100644 --- a/rclrs/src/node.rs +++ b/rclrs/src/node.rs @@ -263,14 +263,13 @@ impl Node { /// /// [1]: crate::Service // TODO: make service's lifetime depend on node's lifetime - pub fn create_service( + pub fn create_service( &self, topic: &str, - callback: F, + callback: impl FnMut(&rmw_request_id_t, T::Request) -> T::Response + 'static + Send, ) -> Result>, RclrsError> where T: rosidl_runtime_rs::Service, - F: FnMut(&rmw_request_id_t, T::Request) -> T::Response + 'static + Send, { let service = Arc::new(Service::::new( Arc::clone(&self.rcl_node_mtx), diff --git a/rclrs_tests/src/graph_tests.rs b/rclrs_tests/src/graph_tests.rs index 11072e09c..aede3413c 100644 --- a/rclrs_tests/src/graph_tests.rs +++ b/rclrs_tests/src/graph_tests.rs @@ -203,7 +203,7 @@ fn test_services() -> Result<(), RclrsError> { let _node_1_empty_service = graph .node1 - .create_service::("graph_test_topic_4", |_, _| srv::Empty_Response { + .create_service::("graph_test_topic_4", |_, _| srv::Empty_Response { structure_needs_at_least_one_member: 0, })?; let _node_2_empty_client = graph From 68b0ef5b75b3802b3b250a123c96bc80ea312666 Mon Sep 17 00:00:00 2001 From: Marco Boneberger Date: Fri, 15 Mar 2024 14:30:08 +0100 Subject: [PATCH 3/3] allow service creation without handling the request header --- .../src/minimal_service.rs | 5 ++--- rclrs/src/node.rs | 19 +++++++++++++++++++ rclrs/src/service.rs | 8 +++++--- rclrs_tests/src/graph_tests.rs | 2 +- 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/examples/minimal_client_service/src/minimal_service.rs b/examples/minimal_client_service/src/minimal_service.rs index 4e5617ef0..312fc03e8 100644 --- a/examples/minimal_client_service/src/minimal_service.rs +++ b/examples/minimal_client_service/src/minimal_service.rs @@ -3,7 +3,6 @@ use std::env; use anyhow::{Error, Result}; fn handle_service( - _request_header: &rclrs::rmw_request_id_t, request: example_interfaces::srv::AddTwoInts_Request, ) -> example_interfaces::srv::AddTwoInts_Response { println!("request: {} + {}", request.a, request.b); @@ -17,8 +16,8 @@ fn main() -> Result<(), Error> { let node = rclrs::create_node(&context, "minimal_service")?; - let _server = node - .create_service::("add_two_ints", handle_service)?; + let _server = + node.create_service::("add_two_ints", handle_service)?; println!("Starting server"); rclrs::spin(node).map_err(|err| err.into()) diff --git a/rclrs/src/node.rs b/rclrs/src/node.rs index 3ab2dae10..a335d2156 100644 --- a/rclrs/src/node.rs +++ b/rclrs/src/node.rs @@ -264,6 +264,25 @@ impl Node { /// [1]: crate::Service // TODO: make service's lifetime depend on node's lifetime pub fn create_service( + &self, + topic: &str, + mut callback: impl FnMut(T::Request) -> T::Response + 'static + Send, + ) -> Result>, RclrsError> + where + T: rosidl_runtime_rs::Service, + { + let callback = + move |_request_header: &rmw_request_id_t, request: T::Request| callback(request); + self.create_service_with_header(topic, callback) + } + + /// Creates a [`Service`][1]. Same as [`create_service`][2] but the callback + /// also has access to the ID of the service request. + /// + /// [1]: crate::Service + /// [2]: Self::create_service + // TODO: make service's lifetime depend on node's lifetime + pub fn create_service_with_header( &self, topic: &str, callback: impl FnMut(&rmw_request_id_t, T::Request) -> T::Response + 'static + Send, diff --git a/rclrs/src/service.rs b/rclrs/src/service.rs index d2e0de3ea..ddbf470c9 100644 --- a/rclrs/src/service.rs +++ b/rclrs/src/service.rs @@ -51,11 +51,13 @@ type ServiceCallback = /// Main class responsible for responding to requests sent by ROS clients. /// -/// The only available way to instantiate services is via [`Node::create_service()`][1], this is to -/// ensure that [`Node`][2]s can track all the services that have been created. +/// The only available way to instantiate services is via [`Node::create_service()`][1] and +/// [`Node::create_service_with_header()`][2], this is to +/// ensure that [`Node`][3]s can track all the services that have been created. /// /// [1]: crate::Node::create_service -/// [2]: crate::Node +/// [2]: crate::Node::create_service_with_header +/// [3]: crate::Node pub struct Service where T: rosidl_runtime_rs::Service, diff --git a/rclrs_tests/src/graph_tests.rs b/rclrs_tests/src/graph_tests.rs index aede3413c..47e456138 100644 --- a/rclrs_tests/src/graph_tests.rs +++ b/rclrs_tests/src/graph_tests.rs @@ -203,7 +203,7 @@ fn test_services() -> Result<(), RclrsError> { let _node_1_empty_service = graph .node1 - .create_service::("graph_test_topic_4", |_, _| srv::Empty_Response { + .create_service::("graph_test_topic_4", |_| srv::Empty_Response { structure_needs_at_least_one_member: 0, })?; let _node_2_empty_client = graph