Migrate create_client to use impl Into<ClientOptions>

Signed-off-by: Michael X. Grey <[email protected]>

Author: mxgrey

rclrs/src/client.rs

@@ -11,7 +11,7 @@ use rosidl_runtime_rs::Message;
 use crate::{
     error::{RclReturnCode, ToResult},
     rcl_bindings::*,
-    MessageCow, NodeHandle, RclrsError, ENTITY_LIFECYCLE_MUTEX,
+    IntoPrimitiveOptions, MessageCow, NodeHandle, RclrsError, ENTITY_LIFECYCLE_MUTEX, QoSProfile,
 };
 
 // SAFETY: The functions accessing this type, including drop(), shouldn't care about the thread
@@ -83,23 +83,28 @@ where
     T: rosidl_runtime_rs::Service,
 {
     /// Creates a new client.
-    pub(crate) fn new(node_handle: Arc<NodeHandle>, topic: &str) -> Result<Self, RclrsError>
+    pub(crate) fn new<'a>(
+        node_handle: Arc<NodeHandle>,
+        options: impl Into<ClientOptions<'a>>,
+    ) -> Result<Self, RclrsError>
     // This uses pub(crate) visibility to avoid instantiating this struct outside
     // [`Node::create_client`], see the struct's documentation for the rationale
     where
         T: rosidl_runtime_rs::Service,
     {
+        let ClientOptions { service_name, qos } = options.into();
         // SAFETY: Getting a zero-initialized value is always safe.
         let mut rcl_client = unsafe { rcl_get_zero_initialized_client() };
         let type_support = <T as rosidl_runtime_rs::Service>::get_type_support()
             as *const rosidl_service_type_support_t;
-        let topic_c_string = CString::new(topic).map_err(|err| RclrsError::StringContainsNul {
+        let topic_c_string = CString::new(service_name).map_err(|err| RclrsError::StringContainsNul {
             err,
-            s: topic.into(),
+            s: service_name.into(),
         })?;
 
         // SAFETY: No preconditions for this function.
-        let client_options = unsafe { rcl_client_get_default_options() };
+        let mut client_options = unsafe { rcl_client_get_default_options() };
+        client_options.qos = qos.into();
 
         {
             let rcl_node = node_handle.rcl_node.lock().unwrap();
@@ -275,6 +280,28 @@ where
     }
 }
 
+/// `ClientOptions` are used by [`Node::create_client`][1] to initialize a
+/// [`Client`] for a service.
+///
+/// [1]: crate::NodeState::create_client
+#[derive(Debug, Clone)]
+#[non_exhaustive]
+pub struct ClientOptions<'a> {
+    /// The name of the service that this client will send requests to
+    pub service_name: &'a str,
+    /// The quality of the service profile for this client
+    pub qos: QoSProfile,
+}
+
+impl<'a, T: IntoPrimitiveOptions<'a>> From<T> for ClientOptions<'a> {
+    fn from(value: T) -> Self {
+        let options = value.into_primitive_options();
+        let mut qos = QoSProfile::services_default();
+        options.apply(&mut qos);
+        Self { service_name: options.name, qos }
+    }
+}
+
 impl<T> ClientBase for Client<T>
 where
     T: rosidl_runtime_rs::Service,

rclrs/src/node.rs

@@ -19,7 +19,7 @@ use std::{
 use rosidl_runtime_rs::Message;
 
 use crate::{
-    rcl_bindings::*, Client, ClientBase, Clock, ContextHandle, GuardCondition,
+    rcl_bindings::*, Client, ClientBase, ClientOptions, Clock, ContextHandle, GuardCondition,
     ParameterBuilder, ParameterInterface, ParameterVariant, Parameters, Publisher, PublisherOptions,
     RclrsError, Service, ServiceBase, ServiceOptions, Subscription, SubscriptionBase, SubscriptionCallback,
     SubscriptionOptions, TimeSource, ENTITY_LIFECYCLE_MUTEX,
@@ -215,13 +215,47 @@ impl NodeState {
 
     /// Creates a [`Client`][1].
     ///
+    /// Pass in only the service name for the `options` argument to use all default client options:
+    /// ```
+    /// # use rclrs::*;
+    /// # let executor = Context::default().create_basic_executor();
+    /// # let node = executor.create_node("my_node").unwrap();
+    /// let client = node.create_client::<test_msgs::srv::Empty>(
+    ///     "my_service"
+    /// )
+    /// .unwrap();
+    /// ```
+    ///
+    /// Take advantage of the [`IntoPrimitiveOptions`] API to easily build up the
+    /// client options:
+    ///
+    /// ```
+    /// # use rclrs::*;
+    /// # let executor = Context::default().create_basic_executor();
+    /// # let node = executor.create_node("my_node").unwrap();
+    /// let client = node.create_client::<test_msgs::srv::Empty>(
+    ///     "my_service"
+    ///     .keep_all()
+    ///     .transient_local()
+    /// )
+    /// .unwrap();
+    /// ```
+    ///
+    /// Any quality of service options that you explicitly specify will override
+    /// the default service options. Any that you do not explicitly specify will
+    /// remain the default service options. Note that clients are generally
+    /// expected to use [reliable][2], so it's best not to change the reliability
+    /// setting unless you know what you are doing.
     /// [1]: crate::Client
     // TODO: make client's lifetime depend on node's lifetime
-    pub fn create_client<T>(&self, topic: &str) -> Result<Arc<Client<T>>, RclrsError>
+    pub fn create_client<'a, T>(
+        &self,
+        options: impl Into<ClientOptions<'a>>,
+    ) -> Result<Arc<Client<T>>, RclrsError>
     where
         T: rosidl_runtime_rs::Service,
     {
-        let client = Arc::new(Client::<T>::new(Arc::clone(&self.handle), topic)?);
+        let client = Arc::new(Client::<T>::new(Arc::clone(&self.handle), options)?);
         { self.clients_mtx.lock().unwrap() }.push(Arc::downgrade(&client) as Weak<dyn ClientBase>);
         Ok(client)
     }
@@ -352,7 +386,7 @@ impl NodeState {
     /// Any quality of service options that you explicitly specify will override
     /// the default service options. Any that you do not explicitly specify will
     /// remain the default service options. Note that services are generally
-    /// expected to use [reliable][2], so is best not to change the reliability
+    /// expected to use [reliable][2], so it's best not to change the reliability
     /// setting unless you know what you are doing.
     ///
     /// [1]: crate::Service

rclrs/src/service.rs

@@ -185,6 +185,8 @@ where
 
 /// `ServiceOptions are used by [`Node::create_service`][1] to initialize a
 /// [`Service`] provider.
+///
+/// [1]: crate::NodeState::create_service
 #[derive(Debug, Clone)]
 #[non_exhaustive]
 pub struct ServiceOptions<'a> {