From fc89816e95af5b2c0b87a7c7c071f96e1abe1288 Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Wed, 17 May 2023 17:17:36 +0900 Subject: [PATCH 01/18] Add context builder Signed-off-by: Taehun Lim --- rclrs/src/context.rs | 119 ++++++++++++++--------------- rclrs/src/context/builder.rs | 144 +++++++++++++++++++++++++++++++++++ 2 files changed, 202 insertions(+), 61 deletions(-) create mode 100644 rclrs/src/context/builder.rs diff --git a/rclrs/src/context.rs b/rclrs/src/context.rs index 83ef17c56..d1b15d33f 100644 --- a/rclrs/src/context.rs +++ b/rclrs/src/context.rs @@ -1,11 +1,10 @@ -use std::ffi::CString; -use std::os::raw::c_char; +mod builder; use std::string::String; use std::sync::{Arc, Mutex}; -use std::vec::Vec; +pub use self::builder::*; use crate::rcl_bindings::*; -use crate::{RclrsError, ToResult}; +use crate::{RclrsError}; impl Drop for rcl_context_t { fn drop(&mut self) { @@ -43,64 +42,10 @@ pub struct Context { } impl Context { - /// Creates a new context. - /// - /// Usually, this would be called with `std::env::args()`, analogously to `rclcpp::init()`. - /// See also the official "Passing ROS arguments to nodes via the command-line" tutorial. - /// - /// Creating a context can fail in case the args contain invalid ROS arguments. - /// - /// # Example - /// ``` - /// # use rclrs::Context; - /// assert!(Context::new([]).is_ok()); - /// let invalid_remapping = ["--ros-args", "-r", ":=:*/]"].map(String::from); - /// assert!(Context::new(invalid_remapping).is_err()); - /// ``` + /// See [`ContextBuilder::new()`] for documentation. + #[allow(clippy::new_ret_no_self)] pub fn new(args: impl IntoIterator) -> Result { - // SAFETY: Getting a zero-initialized value is always safe - let mut rcl_context = unsafe { rcl_get_zero_initialized_context() }; - let cstring_args: Vec = args - .into_iter() - .map(|arg| { - CString::new(arg.as_str()).map_err(|err| RclrsError::StringContainsNul { - err, - s: arg.clone(), - }) - }) - .collect::>()?; - // Vector of pointers into cstring_args - let c_args: Vec<*const c_char> = cstring_args.iter().map(|arg| arg.as_ptr()).collect(); - unsafe { - // SAFETY: No preconditions for this function. - let allocator = rcutils_get_default_allocator(); - // SAFETY: Getting a zero-initialized value is always safe. - let mut rcl_init_options = rcl_get_zero_initialized_init_options(); - // SAFETY: Passing in a zero-initialized value is expected. - // In the case where this returns not ok, there's nothing to clean up. - rcl_init_options_init(&mut rcl_init_options, allocator).ok()?; - // SAFETY: This function does not store the ephemeral init_options and c_args - // pointers. Passing in a zero-initialized rcl_context is expected. - let ret = rcl_init( - c_args.len() as i32, - if c_args.is_empty() { - std::ptr::null() - } else { - c_args.as_ptr() - }, - &rcl_init_options, - &mut rcl_context, - ) - .ok(); - // SAFETY: It's safe to pass in an initialized object. - // Early return will not leak memory, because this is the last fini function. - rcl_init_options_fini(&mut rcl_init_options).ok()?; - // Move the check after the last fini() - ret?; - } - Ok(Self { - rcl_context_mtx: Arc::new(Mutex::new(rcl_context)), - }) + Self::builder(args).build() } /// Checks if the context is still valid. @@ -114,6 +59,58 @@ impl Context { // SAFETY: No preconditions for this function. unsafe { rcl_context_is_valid(rcl_context) } } + + /// Returns the context domain id. + /// + /// The domain ID controls which nodes can send messages to each other, see the [ROS 2 concept article][1]. + /// It can be set through the `ROS_DOMAIN_ID` environment variable + /// or [`ContextBuilder`][2] + /// + /// [1]: https://docs.ros.org/en/rolling/Concepts/About-Domain-ID.html + /// [2]: crate::ContextBuilder + /// + /// # Example + /// ``` + /// # use rclrs::{Context, RclrsError}; + /// // Set default ROS domain ID to 10 here + /// std::env::set_var("ROS_DOMAIN_ID", "10"); + /// let context = Context::new([])?; + /// let domain_id = context.domain_id(); + /// assert_eq!(domain_id, 10); + /// let context = Context::builder([]).domain_id(11).build()?; + /// let domain_id = context.domain_id(); + /// assert_eq!(domain_id, 11); + /// # Ok::<(), RclrsError>(()) + /// ``` + pub fn domain_id(&self) -> usize { + let mut rcl_context = self.rcl_context_mtx.lock().unwrap(); + let mut domain_id: usize = 0; + let ret = unsafe { + // SAFETY: No preconditions for this function. + rcl_context_get_domain_id(&mut *rcl_context, &mut domain_id) + }; + + debug_assert_eq!(ret, 0); + domain_id + } + + /// Creates a [`ContextBuilder`][1] with the given name. + /// + /// Convenience function equivalent to [`ContextBuilder::new()`][2]. + /// + /// [1]: crate::ContextBuilder + /// [2]: crate::ContextBuilder::new + /// + /// # Example + /// ``` + /// # use rclrs::{Context, RclrsError}; + /// let context_builder = Context::builder([]); + /// assert!(context_builder.build().is_ok()); + /// # Ok::<(), RclrsError>(()) + /// ``` + pub fn builder(args: impl IntoIterator) -> ContextBuilder { + ContextBuilder::new(args) + } } #[cfg(test)] diff --git a/rclrs/src/context/builder.rs b/rclrs/src/context/builder.rs new file mode 100644 index 000000000..2cfbb871f --- /dev/null +++ b/rclrs/src/context/builder.rs @@ -0,0 +1,144 @@ +use std::ffi::CString; +use std::os::raw::c_char; +use std::sync::{Arc, Mutex}; + +use crate::rcl_bindings::*; +use crate::{Context, RclrsError, ToResult}; + +/// A builder for creating a [`Context`][1]. +/// +/// The builder pattern allows selectively setting some fields, and leaving all others at their default values. +/// This struct instance can be created via [`Context::builder()`][2]. +/// +/// # Example +/// ``` +/// # use rclrs::{Context, ContextBuilder, RclrsError}; +/// // Building a context in a single expression +/// let args = ["ROS 1 ROS 2"].map(String::from); +/// let context = ContextBuilder::new(args.clone()).domain_id(1).build()?; +/// assert_eq!(context.domain_id(), 1); +/// // Building a context via Context::builder() +/// let context = Context::builder(args.clone()).domain_id(2).build()?; +/// assert_eq!(context.domain_id(), 2); +/// // Building a context step-by-step +/// let mut builder = Context::builder(args.clone()); +/// builder = builder.domain_id(3); +/// let context = builder.build()?; +/// assert_eq!(context.domain_id(), 3); +/// # Ok::<(), RclrsError>(()) +/// ``` +/// +/// [1]: crate::Context +/// [2]: crate::Context::builder +pub struct ContextBuilder { + arguments: Vec, + domain_id: usize, +} + +impl ContextBuilder { + /// Creates a builder for a context with arguments. + /// + /// Usually, this would be called with `std::env::args()`, analogously to `rclcpp::init()`. + /// See also the official "Passing ROS arguments to nodes via the command-line" tutorial. + /// + /// Creating a context can fail in case the args contain invalid ROS arguments. + /// + /// # Example + /// ``` + /// # use rclrs::ContextBuilder; + /// let invalid_remapping = ["--ros-args", "-r", ":=:*/]"].map(String::from); + /// assert!(ContextBuilder::new(invalid_remapping).build().is_err()); + /// let valid_remapping = ["--ros-args", "--remap", "__node:=my_node"].map(String::from); + /// assert!(ContextBuilder::new(valid_remapping).build().is_ok()); + /// ``` + pub fn new(args: impl IntoIterator) -> ContextBuilder { + let mut domain_id = 0; + // SAFETY: Getting the default domain ID, based on the environment + let ret = unsafe { rcl_get_default_domain_id(&mut domain_id) }; + debug_assert_eq!(ret, 0); + + ContextBuilder { + arguments: args.into_iter().collect(), + domain_id + } + } + + /// Sets the context domain id. + /// + /// The domain ID controls which nodes can send messages to each other, see the [ROS 2 concept article][1]. + /// + /// [1]: https://docs.ros.org/en/rolling/Concepts/About-Domain-ID.html + pub fn domain_id(mut self, domain_id: usize) -> Self { + self.domain_id = domain_id; + self + } + + /// Builds the context instance and `rcl_init_options` in order to initialize rcl + /// + /// For example usage, see the [`ContextBuilder`][1] docs. + /// + /// [1]: crate::ContextBuilder + pub fn build(&self) -> Result { + // SAFETY: Getting a zero-initialized value is always safe + let mut rcl_context = unsafe { rcl_get_zero_initialized_context() }; + let cstring_args: Vec = self.arguments + .iter() + .map(|arg| { + CString::new(arg.as_str()).map_err(|err| RclrsError::StringContainsNul { + err, + s: arg.clone(), + }) + }) + .collect::>()?; + // Vector of pointers into cstring_args + let c_args: Vec<*const c_char> = cstring_args.iter().map(|arg| arg.as_ptr()).collect(); + let rcl_init_options = self.create_rcl_init_options()?; + unsafe { + // SAFETY: This function does not store the ephemeral init_options and c_args + // pointers. Passing in a zero-initialized rcl_context is expected. + rcl_init( + c_args.len() as i32, + if c_args.is_empty() { + std::ptr::null() + } else { + c_args.as_ptr() + }, + &rcl_init_options, + &mut rcl_context, + ) + .ok()?; + } + Ok(Context { + rcl_context_mtx: Arc::new(Mutex::new(rcl_context)), + }) + } + + /// Creates a rcl_init_options_t struct from this builder. + /// + /// domain id validation is performed in this method. + fn create_rcl_init_options(&self) -> Result { + unsafe { + // SAFETY: No preconditions for this function. + let allocator = rcutils_get_default_allocator(); + // SAFETY: Getting a zero-initialized value is always safe. + let mut rcl_init_options = rcl_get_zero_initialized_init_options(); + // SAFETY: Passing in a zero-initialized value is expected. + // In the case where this returns not ok, there's nothing to clean up. + rcl_init_options_init(&mut rcl_init_options, allocator).ok()?; + // SAFETY: Setting domain id in the init options provided. + // In the case where this returns not ok, the domain id is invalid. + rcl_init_options_set_domain_id(&mut rcl_init_options, self.domain_id).ok()?; + + Ok(rcl_init_options) + } + } +} + +impl Drop for rcl_init_options_t { + fn drop(&mut self) { + // SAFETY: Do not finish this struct except here. + unsafe { + rcl_init_options_fini(self).ok().unwrap(); + } + } +} From 935d9d8a6b074c6a5e6644b7711de544ba2c93d1 Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Sun, 21 May 2023 21:49:40 +0900 Subject: [PATCH 02/18] Fix typo Signed-off-by: Taehun Lim --- rclrs/src/context/builder.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rclrs/src/context/builder.rs b/rclrs/src/context/builder.rs index 2cfbb871f..8c24bf5eb 100644 --- a/rclrs/src/context/builder.rs +++ b/rclrs/src/context/builder.rs @@ -59,7 +59,7 @@ impl ContextBuilder { ContextBuilder { arguments: args.into_iter().collect(), - domain_id + domain_id, } } @@ -81,7 +81,8 @@ impl ContextBuilder { pub fn build(&self) -> Result { // SAFETY: Getting a zero-initialized value is always safe let mut rcl_context = unsafe { rcl_get_zero_initialized_context() }; - let cstring_args: Vec = self.arguments + let cstring_args: Vec = self + .arguments .iter() .map(|arg| { CString::new(arg.as_str()).map_err(|err| RclrsError::StringContainsNul { From 98eb2f6805577081f00684f3b0755c066fe66322 Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Sun, 21 May 2023 21:54:57 +0900 Subject: [PATCH 03/18] Delete brackets Signed-off-by: Taehun Lim --- rclrs/src/context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclrs/src/context.rs b/rclrs/src/context.rs index d1b15d33f..b9b8e5fdd 100644 --- a/rclrs/src/context.rs +++ b/rclrs/src/context.rs @@ -4,7 +4,7 @@ use std::sync::{Arc, Mutex}; pub use self::builder::*; use crate::rcl_bindings::*; -use crate::{RclrsError}; +use crate::RclrsError; impl Drop for rcl_context_t { fn drop(&mut self) { From fd7323a6cd6d042eb575a93f8382863aa3e3f446 Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Sun, 21 May 2023 22:14:14 +0900 Subject: [PATCH 04/18] Add cfg to avoid compile error at old distro Signed-off-by: Taehun Lim --- rclrs/src/context.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rclrs/src/context.rs b/rclrs/src/context.rs index b9b8e5fdd..3e58aea67 100644 --- a/rclrs/src/context.rs +++ b/rclrs/src/context.rs @@ -82,6 +82,7 @@ impl Context { /// assert_eq!(domain_id, 11); /// # Ok::<(), RclrsError>(()) /// ``` + #[cfg(not(ros_distro = "foxy"))] pub fn domain_id(&self) -> usize { let mut rcl_context = self.rcl_context_mtx.lock().unwrap(); let mut domain_id: usize = 0; From 729fd3c68f8d48bc7fe5da3e33a44283bfb1a8dd Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Sun, 21 May 2023 23:06:43 +0900 Subject: [PATCH 05/18] Split implementation about get domain id according to ros distro Signed-off-by: Taehun Lim --- rclrs/src/context.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rclrs/src/context.rs b/rclrs/src/context.rs index 3e58aea67..1c6bf7841 100644 --- a/rclrs/src/context.rs +++ b/rclrs/src/context.rs @@ -82,14 +82,19 @@ impl Context { /// assert_eq!(domain_id, 11); /// # Ok::<(), RclrsError>(()) /// ``` - #[cfg(not(ros_distro = "foxy"))] pub fn domain_id(&self) -> usize { let mut rcl_context = self.rcl_context_mtx.lock().unwrap(); let mut domain_id: usize = 0; + #[cfg(not(ros_distro = "foxy"))] let ret = unsafe { // SAFETY: No preconditions for this function. rcl_context_get_domain_id(&mut *rcl_context, &mut domain_id) }; + #[cfg(ros_distro = "foxy")] + let ret = unsafe { + // SAFETY: Getting the default domain ID, based on the environment + rcl_get_default_domain_id(&mut domain_id) + }; debug_assert_eq!(ret, 0); domain_id From 93b10c5379eb43b2c1e3972b2392bd271f612bce Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Mon, 22 May 2023 00:03:19 +0900 Subject: [PATCH 06/18] Modify cfg Signed-off-by: Taehun Lim --- rclrs/src/context.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rclrs/src/context.rs b/rclrs/src/context.rs index 1c6bf7841..f3c8986da 100644 --- a/rclrs/src/context.rs +++ b/rclrs/src/context.rs @@ -83,10 +83,11 @@ impl Context { /// # Ok::<(), RclrsError>(()) /// ``` pub fn domain_id(&self) -> usize { - let mut rcl_context = self.rcl_context_mtx.lock().unwrap(); let mut domain_id: usize = 0; + #[cfg(not(ros_distro = "foxy"))] let ret = unsafe { + let mut rcl_context = self.rcl_context_mtx.lock().unwrap(); // SAFETY: No preconditions for this function. rcl_context_get_domain_id(&mut *rcl_context, &mut domain_id) }; From 4e1b46dd7265bcee349ef92541e1a76ccd837d3d Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Mon, 22 May 2023 10:30:28 +0900 Subject: [PATCH 07/18] Add set_env() to change domain_id at foxy Signed-off-by: Taehun Lim --- rclrs/src/context/builder.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rclrs/src/context/builder.rs b/rclrs/src/context/builder.rs index 8c24bf5eb..33bd01609 100644 --- a/rclrs/src/context/builder.rs +++ b/rclrs/src/context/builder.rs @@ -69,6 +69,9 @@ impl ContextBuilder { /// /// [1]: https://docs.ros.org/en/rolling/Concepts/About-Domain-ID.html pub fn domain_id(mut self, domain_id: usize) -> Self { + #[cfg(ros_distro = "foxy")] + std::env::set_var("ROS_DOMAIN_ID", domain_id.to_string()); + self.domain_id = domain_id; self } From b647fe963e81fcd5244c793395e7b2779c6db08d Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Sun, 28 May 2023 11:54:47 +0900 Subject: [PATCH 08/18] Apply advices from clippy Signed-off-by: Taehun Lim --- rclrs/src/dynamic_message.rs | 2 +- rclrs/src/error.rs | 2 +- rclrs/src/node.rs | 4 ++-- rclrs/src/parameter/value.rs | 4 ++-- rclrs/src/subscription.rs | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/rclrs/src/dynamic_message.rs b/rclrs/src/dynamic_message.rs index 3997ad6e7..048c2a36f 100644 --- a/rclrs/src/dynamic_message.rs +++ b/rclrs/src/dynamic_message.rs @@ -75,7 +75,7 @@ fn get_type_support_library( let ament = ament_rs::Ament::new().map_err(|_| RequiredPrefixNotSourced { package: package_name.to_owned(), })?; - let prefix = PathBuf::from(ament.find_package(&package_name).ok_or( + let prefix = PathBuf::from(ament.find_package(package_name).ok_or( RequiredPrefixNotSourced { package: package_name.to_owned(), }, diff --git a/rclrs/src/error.rs b/rclrs/src/error.rs index b84f4d4d3..84bc602b1 100644 --- a/rclrs/src/error.rs +++ b/rclrs/src/error.rs @@ -347,6 +347,6 @@ pub(crate) trait ToResult { impl ToResult for rcl_ret_t { fn ok(&self) -> Result<(), RclrsError> { - to_rclrs_result(*self as i32) + to_rclrs_result(*self) } } diff --git a/rclrs/src/node.rs b/rclrs/src/node.rs index 4df07ba90..e29009758 100644 --- a/rclrs/src/node.rs +++ b/rclrs/src/node.rs @@ -173,7 +173,7 @@ impl Node { &self, getter: unsafe extern "C" fn(*const rcl_node_t) -> *const c_char, ) -> String { - unsafe { call_string_getter_with_handle(&*self.rcl_node_mtx.lock().unwrap(), getter) } + unsafe { call_string_getter_with_handle(&self.rcl_node_mtx.lock().unwrap(), getter) } } /// Creates a [`Client`][1]. @@ -317,7 +317,7 @@ impl Node { } /// Returns the ROS domain ID that the node is using. - /// + /// /// The domain ID controls which nodes can send messages to each other, see the [ROS 2 concept article][1]. /// It can be set through the `ROS_DOMAIN_ID` environment variable. /// diff --git a/rclrs/src/parameter/value.rs b/rclrs/src/parameter/value.rs index 04b64322a..b49108a6d 100644 --- a/rclrs/src/parameter/value.rs +++ b/rclrs/src/parameter/value.rs @@ -163,8 +163,8 @@ mod tests { assert!(!rcl_params.is_null()); assert_eq!(unsafe { (*rcl_params).num_nodes }, 1); let rcl_node_params = unsafe { &(*(*rcl_params).params) }; - assert_eq!((*rcl_node_params).num_params, 1); - let rcl_variant = unsafe { &(*(*rcl_node_params).parameter_values) }; + assert_eq!(rcl_node_params.num_params, 1); + let rcl_variant = unsafe { &(*rcl_node_params.parameter_values) }; let param_value = unsafe { ParameterValue::from_rcl_variant(rcl_variant) }; assert_eq!(param_value, pair.1); unsafe { rcl_yaml_node_struct_fini(rcl_params) }; diff --git a/rclrs/src/subscription.rs b/rclrs/src/subscription.rs index 9e551becd..e684866c2 100644 --- a/rclrs/src/subscription.rs +++ b/rclrs/src/subscription.rs @@ -183,7 +183,7 @@ where /// /// This can be more efficient for messages containing large arrays. pub fn take_boxed(&self) -> Result<(Box, MessageInfo), RclrsError> { - let mut rmw_message = Box::new(::RmwMsg::default()); + let mut rmw_message = Box::<::RmwMsg>::default(); let message_info = self.take_inner(&mut *rmw_message)?; // TODO: This will still use the stack in general. Change signature of // from_rmw_message to allow placing the result in a Box directly. From 4481f99c22dbd1e50e604170bcf0aa9062a73f45 Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Sun, 28 May 2023 11:55:29 +0900 Subject: [PATCH 09/18] Add blocklist to rosidl_dynamic_support Signed-off-by: Taehun Lim --- rclrs/build.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rclrs/build.rs b/rclrs/build.rs index 4f8458db0..28417993d 100644 --- a/rclrs/build.rs +++ b/rclrs/build.rs @@ -36,6 +36,10 @@ fn main() { .allowlist_var("rmw_.*") .allowlist_var("rcutils_.*") .allowlist_var("rosidl_.*") + .blocklist_function("rosidl_dynamic_.*") + .blocklist_function("rmw_take_dynamic_.*") + .blocklist_function("rcl_take_dynamic_.*") + .blocklist_function("rmw_serialization_support_init") .layout_tests(false) .size_t_is_usize(true) .default_enum_style(bindgen::EnumVariation::Rust { From 58e4d75b94e186d33543c2f1b6d3697af327a2a6 Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Sun, 28 May 2023 13:30:13 +0900 Subject: [PATCH 10/18] Modify orders Signed-off-by: Taehun Lim --- rclrs/build.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rclrs/build.rs b/rclrs/build.rs index 28417993d..37f80e64e 100644 --- a/rclrs/build.rs +++ b/rclrs/build.rs @@ -36,10 +36,10 @@ fn main() { .allowlist_var("rmw_.*") .allowlist_var("rcutils_.*") .allowlist_var("rosidl_.*") - .blocklist_function("rosidl_dynamic_.*") - .blocklist_function("rmw_take_dynamic_.*") .blocklist_function("rcl_take_dynamic_.*") + .blocklist_function("rmw_take_dynamic_.*") .blocklist_function("rmw_serialization_support_init") + .blocklist_function("rosidl_dynamic_.*") .layout_tests(false) .size_t_is_usize(true) .default_enum_style(bindgen::EnumVariation::Rust { From ad7049268f349ab5770142d10596983d3cb8b970 Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Wed, 14 Jun 2023 11:38:40 +0900 Subject: [PATCH 11/18] Move domain_id func to node builder in Foxy Signed-off-by: Taehun Lim --- rclrs/src/context/builder.rs | 4 +--- rclrs/src/node/builder.rs | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/rclrs/src/context/builder.rs b/rclrs/src/context/builder.rs index 33bd01609..5d72a6df0 100644 --- a/rclrs/src/context/builder.rs +++ b/rclrs/src/context/builder.rs @@ -68,10 +68,8 @@ impl ContextBuilder { /// The domain ID controls which nodes can send messages to each other, see the [ROS 2 concept article][1]. /// /// [1]: https://docs.ros.org/en/rolling/Concepts/About-Domain-ID.html + #[cfg(not(ros_distro = "foxy"))] pub fn domain_id(mut self, domain_id: usize) -> Self { - #[cfg(ros_distro = "foxy")] - std::env::set_var("ROS_DOMAIN_ID", domain_id.to_string()); - self.domain_id = domain_id; self } diff --git a/rclrs/src/node/builder.rs b/rclrs/src/node/builder.rs index 91cd6fbe3..b2af28a76 100644 --- a/rclrs/src/node/builder.rs +++ b/rclrs/src/node/builder.rs @@ -224,6 +224,24 @@ impl NodeBuilder { self } + /// Sets the node domain id. + /// + /// The domain ID controls which nodes can send messages to each other, see the [ROS 2 concept article][1]. + /// + /// [1]: https://docs.ros.org/en/rolling/Concepts/About-Domain-ID.html + /// + /// # Example + /// ``` + /// # use rclrs::{Context, Node, NodeBuilder, RclrsError}; + /// let context = Context::new([])?; + /// let node = Node::builder(&context, "my_node").domain_id(1); + /// assert_eq!(node.domain_id(), 1); + #[cfg(ros_distro = "foxy")] + pub fn domain_id(mut self, domain_id: usize) -> Self { + std::env::set_var("ROS_DOMAIN_ID", domain_id.to_string()); + self + } + /// Builds the node instance. /// /// Node name and namespace validation is performed in this method. From 9c501984c75506649ba1488778d81e03281df2bf Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Wed, 14 Jun 2023 12:03:15 +0900 Subject: [PATCH 12/18] Delete unnecessary mut Signed-off-by: Taehun Lim --- rclrs/src/node/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclrs/src/node/builder.rs b/rclrs/src/node/builder.rs index b2af28a76..1605a879a 100644 --- a/rclrs/src/node/builder.rs +++ b/rclrs/src/node/builder.rs @@ -237,7 +237,7 @@ impl NodeBuilder { /// let node = Node::builder(&context, "my_node").domain_id(1); /// assert_eq!(node.domain_id(), 1); #[cfg(ros_distro = "foxy")] - pub fn domain_id(mut self, domain_id: usize) -> Self { + pub fn domain_id(self, domain_id: usize) -> Self { std::env::set_var("ROS_DOMAIN_ID", domain_id.to_string()); self } From 47a4b516c9e19dfc2400567bebd08b4578eab38e Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Wed, 14 Jun 2023 15:48:07 +0900 Subject: [PATCH 13/18] Add rcl_init_options_t field in context builder Signed-off-by: Taehun Lim --- rclrs/src/context.rs | 2 +- rclrs/src/context/builder.rs | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/rclrs/src/context.rs b/rclrs/src/context.rs index f3c8986da..79044e8af 100644 --- a/rclrs/src/context.rs +++ b/rclrs/src/context.rs @@ -111,7 +111,7 @@ impl Context { /// # Example /// ``` /// # use rclrs::{Context, RclrsError}; - /// let context_builder = Context::builder([]); + /// let mut context_builder = Context::builder([]); /// assert!(context_builder.build().is_ok()); /// # Ok::<(), RclrsError>(()) /// ``` diff --git a/rclrs/src/context/builder.rs b/rclrs/src/context/builder.rs index 5d72a6df0..6d19e7b9d 100644 --- a/rclrs/src/context/builder.rs +++ b/rclrs/src/context/builder.rs @@ -33,6 +33,7 @@ use crate::{Context, RclrsError, ToResult}; pub struct ContextBuilder { arguments: Vec, domain_id: usize, + rcl_init_options: rcl_init_options_t, } impl ContextBuilder { @@ -56,10 +57,18 @@ impl ContextBuilder { // SAFETY: Getting the default domain ID, based on the environment let ret = unsafe { rcl_get_default_domain_id(&mut domain_id) }; debug_assert_eq!(ret, 0); + // SAFETY: No preconditions for this function. + let allocator = unsafe { rcutils_get_default_allocator() }; + // SAFETY: Getting a zero-initialized value is always safe. + let mut rcl_init_options = unsafe { rcl_get_zero_initialized_init_options() }; + // SAFETY: Passing in a zero-initialized value is expected. + // In the case where this returns not ok, there's nothing to clean up. + unsafe { rcl_init_options_init(&mut rcl_init_options, allocator).ok().unwrap() }; ContextBuilder { arguments: args.into_iter().collect(), domain_id, + rcl_init_options, } } @@ -79,7 +88,7 @@ impl ContextBuilder { /// For example usage, see the [`ContextBuilder`][1] docs. /// /// [1]: crate::ContextBuilder - pub fn build(&self) -> Result { + pub fn build(&mut self) -> Result { // SAFETY: Getting a zero-initialized value is always safe let mut rcl_context = unsafe { rcl_get_zero_initialized_context() }; let cstring_args: Vec = self @@ -94,7 +103,7 @@ impl ContextBuilder { .collect::>()?; // Vector of pointers into cstring_args let c_args: Vec<*const c_char> = cstring_args.iter().map(|arg| arg.as_ptr()).collect(); - let rcl_init_options = self.create_rcl_init_options()?; + self.rcl_init_options = self.create_rcl_init_options()?; unsafe { // SAFETY: This function does not store the ephemeral init_options and c_args // pointers. Passing in a zero-initialized rcl_context is expected. @@ -105,7 +114,7 @@ impl ContextBuilder { } else { c_args.as_ptr() }, - &rcl_init_options, + &self.rcl_init_options, &mut rcl_context, ) .ok()?; From 169edd130c3e5e6a02d711e7028bad67a2fa4677 Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Wed, 14 Jun 2023 15:55:31 +0900 Subject: [PATCH 14/18] Change formatting Signed-off-by: Taehun Lim --- rclrs/src/context/builder.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rclrs/src/context/builder.rs b/rclrs/src/context/builder.rs index 6d19e7b9d..82b2569fa 100644 --- a/rclrs/src/context/builder.rs +++ b/rclrs/src/context/builder.rs @@ -63,7 +63,9 @@ impl ContextBuilder { let mut rcl_init_options = unsafe { rcl_get_zero_initialized_init_options() }; // SAFETY: Passing in a zero-initialized value is expected. // In the case where this returns not ok, there's nothing to clean up. - unsafe { rcl_init_options_init(&mut rcl_init_options, allocator).ok().unwrap() }; + unsafe { + rcl_init_options_init(&mut rcl_init_options, allocator).ok().unwrap() + }; ContextBuilder { arguments: args.into_iter().collect(), From caf0f331f9e7d7d9f340fb60dda98881885dd1ca Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Wed, 14 Jun 2023 16:04:40 +0900 Subject: [PATCH 15/18] Modify formats Signed-off-by: Taehun Lim --- rclrs/src/context/builder.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rclrs/src/context/builder.rs b/rclrs/src/context/builder.rs index 82b2569fa..6f6d373d4 100644 --- a/rclrs/src/context/builder.rs +++ b/rclrs/src/context/builder.rs @@ -64,7 +64,9 @@ impl ContextBuilder { // SAFETY: Passing in a zero-initialized value is expected. // In the case where this returns not ok, there's nothing to clean up. unsafe { - rcl_init_options_init(&mut rcl_init_options, allocator).ok().unwrap() + rcl_init_options_init(&mut rcl_init_options, allocator) + .ok() + .unwrap() }; ContextBuilder { From 7c200b34c9f9630b2bd0286d904af6aed9933d9c Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Fri, 16 Jun 2023 11:06:25 +0900 Subject: [PATCH 16/18] Modify test of domain_id func in each ros distro Signed-off-by: Taehun Lim --- rclrs/src/context.rs | 36 ++++++++++++++++++++++++++++++++++-- rclrs/src/context/builder.rs | 21 +++++++++++++-------- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/rclrs/src/context.rs b/rclrs/src/context.rs index 79044e8af..45d16c2a2 100644 --- a/rclrs/src/context.rs +++ b/rclrs/src/context.rs @@ -77,21 +77,53 @@ impl Context { /// let context = Context::new([])?; /// let domain_id = context.domain_id(); /// assert_eq!(domain_id, 10); + /// // Set ROS domain ID by builder /// let context = Context::builder([]).domain_id(11).build()?; /// let domain_id = context.domain_id(); /// assert_eq!(domain_id, 11); /// # Ok::<(), RclrsError>(()) /// ``` + #[cfg(not(ros_distro = "foxy"))] pub fn domain_id(&self) -> usize { let mut domain_id: usize = 0; - #[cfg(not(ros_distro = "foxy"))] let ret = unsafe { let mut rcl_context = self.rcl_context_mtx.lock().unwrap(); // SAFETY: No preconditions for this function. rcl_context_get_domain_id(&mut *rcl_context, &mut domain_id) }; - #[cfg(ros_distro = "foxy")] + + debug_assert_eq!(ret, 0); + domain_id + } + + /// Returns the context domain id. + /// + /// The domain ID controls which nodes can send messages to each other, see the [ROS 2 concept article][1]. + /// It can be set through the `ROS_DOMAIN_ID` environment variable + /// or [`ContextBuilder`][2] + /// + /// [1]: https://docs.ros.org/en/rolling/Concepts/About-Domain-ID.html + /// [2]: crate::ContextBuilder + /// + /// # Example + /// ``` + /// # use rclrs::{Context, RclrsError}; + /// // Set default ROS domain ID to 10 here + /// std::env::set_var("ROS_DOMAIN_ID", "10"); + /// let context = Context::new([])?; + /// let domain_id = context.domain_id(); + /// assert_eq!(domain_id, 10); + /// // Set ROS domain ID by builder + /// let context = Context::builder([]).domain_id(11).build()?; + /// let domain_id = context.domain_id(); + /// sassert_eq!(domain_id, 11); + /// # Ok::<(), RclrsError>(()) + /// ``` + #[cfg(ros_distro = "foxy")] + pub fn domain_id(&self) -> usize { + let mut domain_id: usize = 0; + let ret = unsafe { // SAFETY: Getting the default domain ID, based on the environment rcl_get_default_domain_id(&mut domain_id) diff --git a/rclrs/src/context/builder.rs b/rclrs/src/context/builder.rs index 6f6d373d4..75d53685c 100644 --- a/rclrs/src/context/builder.rs +++ b/rclrs/src/context/builder.rs @@ -15,16 +15,12 @@ use crate::{Context, RclrsError, ToResult}; /// # use rclrs::{Context, ContextBuilder, RclrsError}; /// // Building a context in a single expression /// let args = ["ROS 1 ROS 2"].map(String::from); -/// let context = ContextBuilder::new(args.clone()).domain_id(1).build()?; -/// assert_eq!(context.domain_id(), 1); +/// assert!(ContextBuilder::new(args.clone()).build().is_ok()); /// // Building a context via Context::builder() -/// let context = Context::builder(args.clone()).domain_id(2).build()?; -/// assert_eq!(context.domain_id(), 2); +/// assert!(Context::builder(args.clone()).build().is_ok()); /// // Building a context step-by-step /// let mut builder = Context::builder(args.clone()); -/// builder = builder.domain_id(3); -/// let context = builder.build()?; -/// assert_eq!(context.domain_id(), 3); +/// assert!(builder.build().is_ok()); /// # Ok::<(), RclrsError>(()) /// ``` /// @@ -46,11 +42,12 @@ impl ContextBuilder { /// /// # Example /// ``` - /// # use rclrs::ContextBuilder; + /// # use rclrs::{ContextBuilder, RclrsError}; /// let invalid_remapping = ["--ros-args", "-r", ":=:*/]"].map(String::from); /// assert!(ContextBuilder::new(invalid_remapping).build().is_err()); /// let valid_remapping = ["--ros-args", "--remap", "__node:=my_node"].map(String::from); /// assert!(ContextBuilder::new(valid_remapping).build().is_ok()); + /// # Ok::<(), RclrsError>(()) /// ``` pub fn new(args: impl IntoIterator) -> ContextBuilder { let mut domain_id = 0; @@ -81,6 +78,14 @@ impl ContextBuilder { /// The domain ID controls which nodes can send messages to each other, see the [ROS 2 concept article][1]. /// /// [1]: https://docs.ros.org/en/rolling/Concepts/About-Domain-ID.html + /// + /// # Example + /// ``` + /// # use rclrs::{Context, ContextBuilder, RclrsError}; + /// let context = ContextBuilder::new([]).domain_id(1).build()?; + /// assert_eq!(context.domain_id(), 1); + /// # Ok::<(), RclrsError>(()) + /// ``` #[cfg(not(ros_distro = "foxy"))] pub fn domain_id(mut self, domain_id: usize) -> Self { self.domain_id = domain_id; From 82e8f3aeea7136ae339bb6806660c37d8bb09c6e Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Fri, 16 Jun 2023 12:25:45 +0900 Subject: [PATCH 17/18] Modify wrong tests Signed-off-by: Taehun Lim --- rclrs/src/context.rs | 4 ---- rclrs/src/node/builder.rs | 11 ++++++----- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/rclrs/src/context.rs b/rclrs/src/context.rs index 45d16c2a2..a20daa705 100644 --- a/rclrs/src/context.rs +++ b/rclrs/src/context.rs @@ -114,10 +114,6 @@ impl Context { /// let context = Context::new([])?; /// let domain_id = context.domain_id(); /// assert_eq!(domain_id, 10); - /// // Set ROS domain ID by builder - /// let context = Context::builder([]).domain_id(11).build()?; - /// let domain_id = context.domain_id(); - /// sassert_eq!(domain_id, 11); /// # Ok::<(), RclrsError>(()) /// ``` #[cfg(ros_distro = "foxy")] diff --git a/rclrs/src/node/builder.rs b/rclrs/src/node/builder.rs index 1605a879a..4b9dd389b 100644 --- a/rclrs/src/node/builder.rs +++ b/rclrs/src/node/builder.rs @@ -79,8 +79,8 @@ impl NodeBuilder { /// RclrsError::RclError { code: RclReturnCode::NodeInvalidName, .. } /// )); /// # Ok::<(), RclrsError>(()) - /// ``` - /// + /// ``` + /// /// [1]: crate::Node#naming /// [2]: https://docs.ros2.org/latest/api/rmw/validate__node__name_8h.html#a5690a285aed9735f89ef11950b6e39e3 /// [3]: NodeBuilder::build @@ -187,7 +187,7 @@ impl NodeBuilder { /// used in creating the context. /// /// For more details about command line arguments, see [here][2]. - /// + /// /// # Example /// ``` /// # use rclrs::{Context, Node, NodeBuilder, RclrsError}; @@ -233,9 +233,10 @@ impl NodeBuilder { /// # Example /// ``` /// # use rclrs::{Context, Node, NodeBuilder, RclrsError}; - /// let context = Context::new([])?; + /// let context = Context::new([]); /// let node = Node::builder(&context, "my_node").domain_id(1); - /// assert_eq!(node.domain_id(), 1); + /// let domain_id = node.domain_id(); + /// assert_eq!(domain_id, 1); #[cfg(ros_distro = "foxy")] pub fn domain_id(self, domain_id: usize) -> Self { std::env::set_var("ROS_DOMAIN_ID", domain_id.to_string()); From 4f1af024f2d80c8fed6e48a35e8022d47f2d1ea3 Mon Sep 17 00:00:00 2001 From: Taehun Lim Date: Fri, 16 Jun 2023 12:52:18 +0900 Subject: [PATCH 18/18] Modify failed tests Signed-off-by: Taehun Lim --- rclrs/src/node/builder.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rclrs/src/node/builder.rs b/rclrs/src/node/builder.rs index 4b9dd389b..3ac2f34b4 100644 --- a/rclrs/src/node/builder.rs +++ b/rclrs/src/node/builder.rs @@ -233,10 +233,12 @@ impl NodeBuilder { /// # Example /// ``` /// # use rclrs::{Context, Node, NodeBuilder, RclrsError}; - /// let context = Context::new([]); - /// let node = Node::builder(&context, "my_node").domain_id(1); + /// let context = Context::new([])?; + /// let node = Node::builder(&context, "my_node").domain_id(1).build()?; /// let domain_id = node.domain_id(); /// assert_eq!(domain_id, 1); + /// # Ok::<(), RclrsError>(()) + /// ``` #[cfg(ros_distro = "foxy")] pub fn domain_id(self, domain_id: usize) -> Self { std::env::set_var("ROS_DOMAIN_ID", domain_id.to_string());