Refactoring Of Dependency usage

examples/message_demo/src/message_demo.rs

@@ -1,6 +1,4 @@
-use std::convert::TryInto;
-use std::env;
-use std::sync::Arc;
+use std::{convert::TryInto,env,sync::Arc};
 
 use anyhow::{Error, Result};
 use rosidl_runtime_rs::{seq, BoundedSequence, Message, Sequence};

examples/minimal_pub_sub/src/minimal_two_nodes.rs

@@ -1,6 +1,10 @@
-use std::env;
-use std::sync::atomic::{AtomicU32, Ordering};
-use std::sync::{Arc, Mutex};
+use std::{
+    env,
+    sync::{
+        atomic::{AtomicU32, Ordering},
+        {Arc, Mutex}
+    }
+};
 
 use anyhow::{Error, Result};
 

rclrs/.rustfmt.toml

@@ -0,0 +1,7 @@
+imports_granularity = "Crate"
+array_width = 60
+attr_fn_like_width = 70
+brace_style = "SameLineWhere"
+combine_control_expr = false
+comment_width = 80
+wrap_comments = true

rclrs/build.rs

@@ -1,6 +1,8 @@
-use std::env;
-use std::fs::read_dir;
-use std::path::{Path, PathBuf};
+use std::{
+    env,
+    fs::read_dir,
+    path::{Path, PathBuf},
+};
 
 const AMENT_PREFIX_PATH: &str = "AMENT_PREFIX_PATH";
 const ROS_DISTRO: &str = "ROS_DISTRO";
@@ -65,8 +67,9 @@ fn main() {
     // #############
     //
     // For each prefix in ${AMENT_PREFIX_PATH}:
-    // Search through ament index at ${prefix}/share/ament_index/resource_index/packages/ to find packages to include
-    // The include root will be located at either:
+    // Search through ament index at
+    // ${prefix}/share/ament_index/resource_index/packages/ to find packages to
+    // include The include root will be located at either:
     // - ${prefix}/include/ (old style)
     // - ${prefix}/include/${package_name} (new style)
     // - ${prefix}/include/CycloneDDS (special case, match for this)
@@ -97,9 +100,10 @@ fn main() {
             if package_include_dir.is_dir() {
                 let new_style_include_dir = package_include_dir.join(&package);
 
-                // CycloneDDS is a special case - it needs to be included as if it were a new-style path, but
-                // doesn't actually have a secondary folder within it called "CycloneDDS"
-                // TODO(jhdcs): if this changes in future, remove this check
+                // CycloneDDS is a special case - it needs to be included as if it were a
+                // new-style path, but doesn't actually have a secondary folder
+                // within it called "CycloneDDS" TODO(jhdcs): if this changes in
+                // future, remove this check
                 if package == "CycloneDDS" || new_style_include_dir.is_dir() {
                     builder =
                         builder.clang_arg(format!("-isystem{}", package_include_dir.display()));

rclrs/src/arguments.rs

@@ -1,20 +1,22 @@
-use std::ffi::CString;
-use std::os::raw::c_char;
-use std::os::raw::c_void;
-use std::ptr::null_mut;
+use std::{
+    ffi::CString,
+    os::raw::{c_char, c_void},
+    ptr::null_mut,
+};
 
-use crate::error::*;
-use crate::rcl_bindings::*;
+use crate::{error::*, rcl_bindings::*};
 
 /// Extract non-ROS arguments from program's input arguments.
 ///
-/// `args` is expected to be the input arguments of the program (e.g. [`std::env::args()`]),
-/// which are expected to contain at least one element - the executable name.
+/// `args` is expected to be the input arguments of the program (e.g.
+/// [`std::env::args()`]), which are expected to contain at least one element -
+/// the executable name.
 ///
-/// ROS arguments are arguments between `--ros-args` and `--`, with the final `--` being optional.
-/// Everything else is considered as non-ROS arguments and will be left unparsed by
-/// [`Context::new()`][1] etc.
-/// Extracted non-ROS arguments are returned in the order that they appear by this function.
+/// ROS arguments are arguments between `--ros-args` and `--`, with the final
+/// `--` being optional. Everything else is considered as non-ROS arguments and
+/// will be left unparsed by [`Context::new()`][1] etc.
+/// Extracted non-ROS arguments are returned in the order that they appear by
+/// this function.
 ///
 /// # Example
 /// ```
@@ -79,14 +81,16 @@ pub fn extract_non_ros_args(
     ret
 }
 
-/// Returns arguments type held by `rcl_arguments` basing on `rcl_get_count` and `rcl_get_indices` function pointers.
+/// Returns arguments type held by `rcl_arguments` basing on `rcl_get_count` and
+/// `rcl_get_indices` function pointers.
 ///
-/// This function must be called after `rcl_arguments` was initialized. `args` must be array of input arguments passed to node/program.
+/// This function must be called after `rcl_arguments` was initialized. `args`
+/// must be array of input arguments passed to node/program.
 ///
-/// SAFETY: `rcl_get_count` and `rcl_get_indices` has to be corresponding rcl API functions, e.g.:
-/// `rcl_arguments_get_count_unparsed` -> `rcl_arguments_get_unparsed`
-/// `rcl_arguments_get_count_unparsed_ros` -> `rcl_arguments_get_count_ros`
-/// ...
+/// SAFETY: `rcl_get_count` and `rcl_get_indices` has to be corresponding rcl
+/// API functions, e.g.: `rcl_arguments_get_count_unparsed` ->
+/// `rcl_arguments_get_unparsed` `rcl_arguments_get_count_unparsed_ros` ->
+/// `rcl_arguments_get_count_ros` ...
 pub(crate) fn get_rcl_arguments(
     rcl_get_count: unsafe extern "C" fn(*const rcl_arguments_t) -> std::os::raw::c_int,
     rcl_get_indices: unsafe extern "C" fn(
@@ -110,13 +114,14 @@ pub(crate) fn get_rcl_arguments(
     unsafe {
         // SAFETY: No preconditions for this function.
         let allocator = rcutils_get_default_allocator();
-        // SAFETY: The indices_ptr is an output parameter, so it is expected that it contains null.
-        // The indices_ptr will need to be freed by the caller, which happens later in this function.
+        // SAFETY: The indices_ptr is an output parameter, so it is expected that it
+        // contains null. The indices_ptr will need to be freed by the caller,
+        // which happens later in this function.
         rcl_get_indices(rcl_arguments, allocator, &mut indices_ptr).ok()?;
 
         for i in 0..args_count {
-            // SAFETY: rcl_get_indices finished with success and rcl_get_count is matching function
-            // according to documentation of this function
+            // SAFETY: rcl_get_indices finished with success and rcl_get_count is matching
+            // function according to documentation of this function
             let index = *(indices_ptr.add(i));
             // SAFETY: rcl_get_indices and rcl_get_count are matching functions according
             // to documentation of this function
@@ -137,7 +142,8 @@ mod tests {
 
     #[test]
     fn test_non_ros_arguments() -> Result<(), String> {
-        // ROS args are expected to be between '--ros-args' and '--'. Everything beside that is 'non-ROS'.
+        // ROS args are expected to be between '--ros-args' and '--'. Everything beside
+        // that is 'non-ROS'.
         let input_args: [String; 6] = [
             "non-ros1",
             "--ros-args",

rclrs/src/client.rs

@@ -1,18 +1,22 @@
-use std::boxed::Box;
-use std::collections::HashMap;
-use std::ffi::CString;
-use std::sync::atomic::AtomicBool;
-use std::sync::{Arc, Mutex, MutexGuard};
+use std::{
+    boxed::Box,
+    collections::HashMap,
+    ffi::CString,
+    sync::{atomic::AtomicBool, Arc, Mutex, MutexGuard},
+};
 
 use futures::channel::oneshot;
 use rosidl_runtime_rs::Message;
+use crate::{
+    error::{RclReturnCode, ToResult},
+    rcl_bindings::*,
+    MessageCow, RclrsError,
+};
 
-use crate::error::{RclReturnCode, ToResult};
-use crate::MessageCow;
-use crate::{rcl_bindings::*, NodeHandle, RclrsError, ENTITY_LIFECYCLE_MUTEX};
+// SAFETY: The functions accessing this type, including drop(), shouldn't care
+// about the thread they are running in. Therefore, this type can be safely sent
+// to another thread.
 
-// SAFETY: The functions accessing this type, including drop(), shouldn't care about the thread
-// they are running in. Therefore, this type can be safely sent to another thread.
 unsafe impl Send for rcl_client_t {}
 
 /// Manage the lifecycle of an `rcl_client_t`, including managing its dependencies
@@ -61,8 +65,9 @@ type RequestId = i64;
 
 /// Main class responsible for sending requests to a ROS service.
 ///
-/// The only available way to instantiate clients is via [`Node::create_client`][1], this is to
-/// ensure that [`Node`][2]s can track all the clients that have been created.
+/// The only available way to instantiate clients is via
+/// [`Node::create_client`][1], this is to ensure that [`Node`][2]s can track
+/// all the clients that have been created.
 ///
 /// [1]: crate::Node::create_client
 /// [2]: crate::Node
@@ -98,27 +103,19 @@ where
         // SAFETY: No preconditions for this function.
         let client_options = unsafe { rcl_client_get_default_options() };
 
-        {
-            let rcl_node = node_handle.rcl_node.lock().unwrap();
-            let _lifecycle_lock = ENTITY_LIFECYCLE_MUTEX.lock().unwrap();
-
-            // SAFETY:
-            // * The rcl_client was zero-initialized as expected by this function.
-            // * The rcl_node is kept alive by the NodeHandle because it is a dependency of the client.
-            // * The topic name and the options are copied by this function, so they can be dropped
-            //   afterwards.
-            // * The entity lifecycle mutex is locked to protect against the risk of global
-            //   variables in the rmw implementation being unsafely modified during initialization.
-            unsafe {
-                rcl_client_init(
-                    &mut rcl_client,
-                    &*rcl_node,
-                    type_support,
-                    topic_c_string.as_ptr(),
-                    &client_options,
-                )
-                .ok()?;
-            }
+        unsafe {
+            // SAFETY: The rcl_client is zero-initialized as expected by this function.
+            // The rcl_node is kept alive because it is co-owned by the client.
+            // The topic name and the options are copied by this function, so they can be
+            // dropped afterwards.
+            rcl_client_init(
+                &mut rcl_client,
+                &*rcl_node_mtx.lock().unwrap(),
+                type_support,
+                topic_c_string.as_ptr(),
+                &client_options,
+            )
+            .ok()?;
         }
 
         let handle = Arc::new(ClientHandle {
@@ -141,13 +138,14 @@ where
     /// The [`MessageCow`] trait is implemented by any
     /// [`Message`] as well as any reference to a `Message`.
     ///
-    /// The reason for allowing owned messages is that publishing owned messages can be more
-    /// efficient in the case of idiomatic messages[^note].
+    /// The reason for allowing owned messages is that publishing owned messages
+    /// can be more efficient in the case of idiomatic messages[^note].
     ///
     /// [^note]: See the [`Message`] trait for an explanation of "idiomatic".
     ///
-    /// Hence, when a message will not be needed anymore after publishing, pass it by value.
-    /// When a message will be needed again after publishing, pass it by reference, instead of cloning and passing by value.
+    /// Hence, when a message will not be needed anymore after publishing, pass
+    /// it by value. When a message will be needed again after publishing,
+    /// pass it by reference, instead of cloning and passing by value.
     pub fn async_send_request_with_callback<'a, M: MessageCow<'a, T::Request>, F>(
         &self,
         message: M,
@@ -159,7 +157,8 @@ where
         let rmw_message = T::Request::into_rmw_message(message.into_cow());
         let mut sequence_number = -1;
         unsafe {
-            // SAFETY: The request type is guaranteed to match the client type by the type system.
+            // SAFETY: The request type is guaranteed to match the client type by the type
+            // system.
             rcl_send_request(
                 &*self.handle.lock() as *const _,
                 rmw_message.as_ref() as *const <T::Request as Message>::RmwMsg as *mut _,
@@ -177,13 +176,14 @@ where
     /// The [`MessageCow`] trait is implemented by any
     /// [`Message`] as well as any reference to a `Message`.
     ///
-    /// The reason for allowing owned messages is that publishing owned messages can be more
-    /// efficient in the case of idiomatic messages[^note].
+    /// The reason for allowing owned messages is that publishing owned messages
+    /// can be more efficient in the case of idiomatic messages[^note].
     ///
     /// [^note]: See the [`Message`] trait for an explanation of "idiomatic".
     ///
-    /// Hence, when a message will not be needed anymore after publishing, pass it by value.
-    /// When a message will be needed again after publishing, pass it by reference, instead of cloning and passing by value.
+    /// Hence, when a message will not be needed anymore after publishing, pass
+    /// it by value. When a message will be needed again after publishing,
+    /// pass it by reference, instead of cloning and passing by value.
     pub async fn call_async<'a, R: MessageCow<'a, T::Request>>(
         &self,
         request: R,
@@ -194,7 +194,8 @@ where
         let rmw_message = T::Request::into_rmw_message(request.into_cow());
         let mut sequence_number = -1;
         unsafe {
-            // SAFETY: The request type is guaranteed to match the client type by the type system.
+            // SAFETY: The request type is guaranteed to match the client type by the type
+            // system.
             rcl_send_request(
                 &*self.handle.lock() as *const _,
                 rmw_message.as_ref() as *const <T::Request as Message>::RmwMsg as *mut _,
@@ -204,8 +205,8 @@ where
         .ok()?;
         let (tx, rx) = oneshot::channel::<T::Response>();
         self.futures.lock().unwrap().insert(sequence_number, tx);
-        // It is safe to call unwrap() here since the `Canceled` error will only happen when the
-        // `Sender` is dropped
+        // It is safe to call unwrap() here since the `Canceled` error will only happen
+        // when the `Sender` is dropped
         // https://docs.rs/futures/latest/futures/channel/oneshot/struct.Canceled.html
         Ok(rx.await.unwrap())
     }
@@ -255,8 +256,8 @@ where
 
     /// Check if a service server is available.
     ///
-    /// Will return true if there is a service server available, false if unavailable.
-    ///
+    /// Will return true if there is a service server available, false if
+    /// unavailable.
     pub fn service_is_ready(&self) -> Result<bool, RclrsError> {
         let mut is_ready = false;
         let client = &mut *self.handle.rcl_client.lock().unwrap();