diff --git a/rclrs/src/context.rs b/rclrs/src/context.rs index ebcdfe59c..0e7e250c5 100644 --- a/rclrs/src/context.rs +++ b/rclrs/src/context.rs @@ -16,7 +16,7 @@ impl Drop for rcl_context_t { // line arguments. // SAFETY: No preconditions for this function. if rcl_context_is_valid(self) { - // SAFETY: These functions have no preconditions besides a valid handle + // SAFETY: These functions have no preconditions besides a valid rcl_context rcl_shutdown(self); rcl_context_fini(self); } @@ -41,7 +41,7 @@ unsafe impl Send for rcl_context_t {} /// - the allocator used (left as the default by `rclrs`) /// pub struct Context { - pub(crate) handle: Arc>, + pub(crate) rcl_context_mtx: Arc>, } impl Context { @@ -77,12 +77,12 @@ impl Context { // SAFETY: No preconditions for this function. let allocator = rcutils_get_default_allocator(); // SAFETY: Getting a zero-initialized value is always safe. - let mut init_options = rcl_get_zero_initialized_init_options(); + 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 init_options, allocator).ok()?; + 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 handle is expected. + // pointers. Passing in a zero-initialized rcl_context is expected. let ret = rcl_init( c_args.len() as i32, if c_args.is_empty() { @@ -90,18 +90,18 @@ impl Context { } else { c_args.as_ptr() }, - &init_options, + &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 init_options).ok()?; + rcl_init_options_fini(&mut rcl_init_options).ok()?; // Move the check after the last fini() ret?; } Ok(Self { - handle: Arc::new(Mutex::new(rcl_context)), + rcl_context_mtx: Arc::new(Mutex::new(rcl_context)), }) } @@ -153,8 +153,8 @@ impl Context { pub fn ok(&self) -> bool { // This will currently always return true, but once we have a signal handler, the signal // handler could call `rcl_shutdown()`, hence making the context invalid. - let handle = &mut *self.handle.lock(); + let rcl_context = &mut *self.rcl_context_mtx.lock(); // SAFETY: No preconditions for this function. - unsafe { rcl_context_is_valid(handle) } + unsafe { rcl_context_is_valid(rcl_context) } } } diff --git a/rclrs/src/lib.rs b/rclrs/src/lib.rs index 424b84ded..df6800812 100644 --- a/rclrs/src/lib.rs +++ b/rclrs/src/lib.rs @@ -34,7 +34,7 @@ use std::time::Duration; pub fn spin_once(node: &Node, timeout: Option) -> Result<(), RclrsError> { let live_subscriptions = node.live_subscriptions(); let ctx = Context { - handle: node.context.clone(), + rcl_context_mtx: node.rcl_context_mtx.clone(), }; let mut wait_set = WaitSet::new(live_subscriptions.len(), &ctx)?; @@ -57,10 +57,10 @@ pub fn spin(node: &Node) -> Result<(), RclrsError> { // The context_is_valid functions exists only to abstract away ROS distro differences #[cfg(ros_distro = "foxy")] // SAFETY: No preconditions for this function. - let context_is_valid = || unsafe { rcl_context_is_valid(&mut *node.context.lock()) }; + let context_is_valid = || unsafe { rcl_context_is_valid(&mut *node.rcl_context_mtx.lock()) }; #[cfg(not(ros_distro = "foxy"))] // SAFETY: No preconditions for this function. - let context_is_valid = || unsafe { rcl_context_is_valid(&*node.context.lock()) }; + let context_is_valid = || unsafe { rcl_context_is_valid(&*node.rcl_context_mtx.lock()) }; while context_is_valid() { match spin_once(node, None) { diff --git a/rclrs/src/node.rs b/rclrs/src/node.rs index da0428abf..7c63750d1 100644 --- a/rclrs/src/node.rs +++ b/rclrs/src/node.rs @@ -66,8 +66,8 @@ unsafe impl Send for rcl_node_t {} /// [3]: crate::NodeBuilder::new /// [4]: crate::NodeBuilder::namespace pub struct Node { - handle: Arc>, - pub(crate) context: Arc>, + rcl_node_mtx: Arc>, + pub(crate) rcl_context_mtx: Arc>, pub(crate) subscriptions: Vec>, } @@ -75,7 +75,7 @@ impl Eq for Node {} impl PartialEq for Node { fn eq(&self, other: &Self) -> bool { - Arc::ptr_eq(&self.handle, &other.handle) + Arc::ptr_eq(&self.rcl_node_mtx, &other.rcl_node_mtx) } } @@ -171,8 +171,8 @@ impl Node { getter: unsafe extern "C" fn(*const rcl_node_t) -> *const c_char, ) -> String { let char_ptr = unsafe { - // SAFETY: The node handle is valid. - getter(&*self.handle.lock()) + // SAFETY: The rcl_node is valid. + getter(&*self.rcl_node_mtx.lock()) }; debug_assert!(!char_ptr.is_null()); let cstr = unsafe { @@ -249,11 +249,11 @@ impl Node { // add description about this function is for getting actual domain_id // and about override of domain_id via node option pub fn domain_id(&self) -> usize { - let handle = &*self.handle.lock(); + let rcl_node = &*self.rcl_node_mtx.lock(); let mut domain_id: usize = 0; let ret = unsafe { // SAFETY: No preconditions for this function. - rcl_node_get_domain_id(handle, &mut domain_id) + rcl_node_get_domain_id(rcl_node, &mut domain_id) }; debug_assert_eq!(ret, 0); diff --git a/rclrs/src/node/builder.rs b/rclrs/src/node/builder.rs index b94812ead..bcc154b2a 100644 --- a/rclrs/src/node/builder.rs +++ b/rclrs/src/node/builder.rs @@ -86,7 +86,7 @@ impl NodeBuilder { /// [3]: NodeBuilder::build pub fn new(context: &Context, name: &str) -> NodeBuilder { NodeBuilder { - context: context.handle.clone(), + context: context.rcl_context_mtx.clone(), name: name.to_string(), namespace: "/".to_string(), use_global_arguments: true, @@ -244,30 +244,30 @@ impl NodeBuilder { s: self.namespace.clone(), })?; let node_options = self.create_node_options()?; - let context_handle = &mut *self.context.lock(); + let rcl_context = &mut *self.context.lock(); // SAFETY: Getting a zero-initialized value is always safe. - let mut node_handle = unsafe { rcl_get_zero_initialized_node() }; + let mut rcl_node = unsafe { rcl_get_zero_initialized_node() }; unsafe { - // SAFETY: The node handle is zero-initialized as expected by this function. + // SAFETY: The rcl_node is zero-initialized as expected by this function. // The strings and node options are copied by this function, so we don't need // to keep them alive. - // The context handle has to be kept alive because it is co-owned by the node. + // The rcl_context has to be kept alive because it is co-owned by the node. rcl_node_init( - &mut node_handle, + &mut rcl_node, node_name.as_ptr(), node_namespace.as_ptr(), - context_handle, + rcl_context, &node_options, ) .ok()?; }; - let handle = Arc::new(Mutex::new(node_handle)); + let rcl_node_mtx = Arc::new(Mutex::new(rcl_node)); Ok(Node { - handle, - context: self.context.clone(), + rcl_node_mtx, + rcl_context_mtx: self.context.clone(), subscriptions: std::vec![], }) } diff --git a/rclrs/src/node/publisher.rs b/rclrs/src/node/publisher.rs index f6329eb2b..79cdab2c4 100644 --- a/rclrs/src/node/publisher.rs +++ b/rclrs/src/node/publisher.rs @@ -17,23 +17,23 @@ use rosidl_runtime_rs::{Message, RmwMessage}; unsafe impl Send for rcl_publisher_t {} pub(crate) struct PublisherHandle { - handle: Mutex, - node_handle: Arc>, + rcl_publisher_mtx: Mutex, + rcl_node_mtx: Arc>, } impl PublisherHandle { fn lock(&self) -> MutexGuard { - self.handle.lock() + self.rcl_publisher_mtx.lock() } } impl Drop for PublisherHandle { fn drop(&mut self) { - let handle = self.handle.get_mut(); - let node_handle = &mut *self.node_handle.lock(); + let rcl_publisher = self.rcl_publisher_mtx.get_mut(); + let rcl_node = &mut *self.rcl_node_mtx.lock(); // SAFETY: No preconditions for this function (besides the arguments being valid). unsafe { - rcl_publisher_fini(handle as *mut _, node_handle as *mut _); + rcl_publisher_fini(rcl_publisher as *mut _, rcl_node as *mut _); } } } @@ -68,27 +68,27 @@ where T: Message, { // SAFETY: Getting a zero-initialized value is always safe. - let mut publisher_handle = unsafe { rcl_get_zero_initialized_publisher() }; + let mut rcl_publisher = unsafe { rcl_get_zero_initialized_publisher() }; let type_support = ::RmwMsg::get_type_support() as *const rosidl_message_type_support_t; let topic_c_string = CString::new(topic).map_err(|err| RclrsError::StringContainsNul { err, s: topic.into(), })?; - let node_handle = &mut *node.handle.lock(); + let rcl_node = &mut *node.rcl_node_mtx.lock(); // SAFETY: No preconditions for this function. let mut publisher_options = unsafe { rcl_publisher_get_default_options() }; publisher_options.qos = qos.into(); unsafe { - // SAFETY: The publisher handle is zero-initialized as expected by this function. - // The node handle is kept alive because it is co-owned by the subscription. + // SAFETY: The rcl_publisher is zero-initialized as expected by this function. + // The rcl_node is kept alive because it is co-owned by the subscription. // The topic name and the options are copied by this function, so they can be dropped // afterwards. // TODO: type support? rcl_publisher_init( - &mut publisher_handle, - node_handle, + &mut rcl_publisher, + rcl_node, type_support, topic_c_string.as_ptr(), &publisher_options, @@ -97,8 +97,8 @@ where } let handle = Arc::new(PublisherHandle { - handle: Mutex::new(publisher_handle), - node_handle: node.handle.clone(), + rcl_publisher_mtx: Mutex::new(rcl_publisher), + rcl_node_mtx: node.rcl_node_mtx.clone(), }); Ok(Self { @@ -125,13 +125,13 @@ where /// [1]: https://github.com/ros2/ros2/issues/255 pub fn publish<'a, M: MessageCow<'a, T>>(&self, message: M) -> Result<(), RclrsError> { let rmw_message = T::into_rmw_message(message.into_cow()); - let handle = &mut *self.handle.lock(); + let rcl_publisher = &mut *self.handle.lock(); let ret = unsafe { // SAFETY: The message type is guaranteed to match the publisher type by the type system. // The message does not need to be valid beyond the duration of this function call. // The third argument is explictly allowed to be NULL. rcl_publish( - handle, + rcl_publisher, rmw_message.as_ref() as *const ::RmwMsg as *mut _, std::ptr::null_mut(), ) diff --git a/rclrs/src/node/subscription.rs b/rclrs/src/node/subscription.rs index 178825737..3999effd2 100644 --- a/rclrs/src/node/subscription.rs +++ b/rclrs/src/node/subscription.rs @@ -3,7 +3,6 @@ use crate::qos::QoSProfile; use crate::Node; use crate::{rcl_bindings::*, RclrsError}; -use std::borrow::Borrow; use std::boxed::Box; use std::ffi::CString; use std::marker::PhantomData; @@ -19,23 +18,23 @@ unsafe impl Send for rcl_subscription_t {} /// Internal struct used by subscriptions. pub struct SubscriptionHandle { - handle: Mutex, - node_handle: Arc>, + rcl_subscription_mtx: Mutex, + rcl_node_mtx: Arc>, } impl SubscriptionHandle { pub(crate) fn lock(&self) -> MutexGuard { - self.handle.lock() + self.rcl_subscription_mtx.lock() } } impl Drop for SubscriptionHandle { fn drop(&mut self) { - let handle = self.handle.get_mut(); - let node_handle = &mut *self.node_handle.lock(); + let rcl_subscription = self.rcl_subscription_mtx.get_mut(); + let rcl_node = &mut *self.rcl_node_mtx.lock(); // SAFETY: No preconditions for this function (besides the arguments being valid). unsafe { - rcl_subscription_fini(handle, node_handle); + rcl_subscription_fini(rcl_subscription, rcl_node); } } } @@ -85,27 +84,27 @@ where F: FnMut(T) + 'static + Send, { // SAFETY: Getting a zero-initialized value is always safe. - let mut subscription_handle = unsafe { rcl_get_zero_initialized_subscription() }; + let mut rcl_subscription = unsafe { rcl_get_zero_initialized_subscription() }; let type_support = ::RmwMsg::get_type_support() as *const rosidl_message_type_support_t; let topic_c_string = CString::new(topic).map_err(|err| RclrsError::StringContainsNul { err, s: topic.into(), })?; - let node_handle = &mut *node.handle.lock(); + let rcl_node = &mut *node.rcl_node_mtx.lock(); // SAFETY: No preconditions for this function. let mut subscription_options = unsafe { rcl_subscription_get_default_options() }; subscription_options.qos = qos.into(); unsafe { - // SAFETY: The subscription handle is zero-initialized as expected by this function. - // The node handle is kept alive because it is co-owned by the subscription. + // SAFETY: The rcl_subscription is zero-initialized as expected by this function. + // The rcl_node is kept alive because it is co-owned by the subscription. // The topic name and the options are copied by this function, so they can be dropped // afterwards. // TODO: type support? rcl_subscription_init( - &mut subscription_handle, - node_handle, + &mut rcl_subscription, + rcl_node, type_support, topic_c_string.as_ptr(), &subscription_options, @@ -114,8 +113,8 @@ where } let handle = Arc::new(SubscriptionHandle { - handle: Mutex::new(subscription_handle), - node_handle: node.handle.clone(), + rcl_subscription_mtx: Mutex::new(rcl_subscription), + rcl_node_mtx: node.rcl_node_mtx.clone(), }); Ok(Self { @@ -149,13 +148,13 @@ where // ``` pub fn take(&self) -> Result { let mut rmw_message = ::RmwMsg::default(); - let handle = &mut *self.handle.lock(); + let rcl_subscription = &mut *self.handle.lock(); let ret = unsafe { // SAFETY: The first two pointers are valid/initialized, and do not need to be valid // beyond the function call. // The latter two pointers are explicitly allowed to be NULL. rcl_take( - handle, + rcl_subscription, &mut rmw_message as *mut ::RmwMsg as *mut _, std::ptr::null_mut(), std::ptr::null_mut(), @@ -171,7 +170,7 @@ where T: Message, { fn handle(&self) -> &SubscriptionHandle { - self.handle.borrow() + &self.handle } fn execute(&self) -> Result<(), RclrsError> { diff --git a/rclrs/src/wait.rs b/rclrs/src/wait.rs index 776d79138..062c57026 100644 --- a/rclrs/src/wait.rs +++ b/rclrs/src/wait.rs @@ -27,9 +27,9 @@ use parking_lot::Mutex; /// A struct for waiting on subscriptions and other waitable entities to become ready. pub struct WaitSet { - handle: rcl_wait_set_t, + rcl_wait_set: rcl_wait_set_t, // Used to ensure the context is alive while the wait set is alive. - _context_handle: Arc>, + _rcl_context_mtx: Arc>, // The subscriptions that are currently registered in the wait set. // This correspondence is an invariant that must be maintained by all functions, // even in the error case. @@ -71,15 +71,15 @@ impl WaitSet { 0, 0, 0, - &mut *context.handle.lock(), + &mut *context.rcl_context_mtx.lock(), rcutils_get_default_allocator(), ) .ok()?; rcl_wait_set }; Ok(Self { - handle: rcl_wait_set, - _context_handle: context.handle.clone(), + rcl_wait_set, + _rcl_context_mtx: context.rcl_context_mtx.clone(), subscriptions: Vec::new(), }) } @@ -94,7 +94,7 @@ impl WaitSet { // valid, which it always is in our case. Hence, only debug_assert instead of returning // Result. // SAFETY: No preconditions for this function (besides passing in a valid wait set). - let ret = unsafe { rcl_wait_set_clear(&mut self.handle) }; + let ret = unsafe { rcl_wait_set_clear(&mut self.rcl_wait_set) }; debug_assert_eq!(ret, 0); } @@ -116,7 +116,7 @@ impl WaitSet { // for as long as the wait set exists, because it's stored in self.subscriptions. // Passing in a null pointer for the third argument is explicitly allowed. rcl_wait_set_add_subscription( - &mut self.handle, + &mut self.rcl_wait_set, &*subscription.handle().lock(), std::ptr::null_mut(), ) @@ -164,8 +164,8 @@ impl WaitSet { // in multiple threads, and the wait sets may not share content." // We cannot currently guarantee that the wait sets may not share content, but it is // mentioned in the doc comment for `add_subscription`. - // Also, the handle is obviously valid. - unsafe { rcl_wait(&mut self.handle, timeout_ns) }.ok()?; + // Also, the rcl_wait_set is obviously valid. + unsafe { rcl_wait(&mut self.rcl_wait_set, timeout_ns) }.ok()?; let mut ready_entities = ReadyEntities { subscriptions: Vec::new(), }; @@ -173,7 +173,7 @@ impl WaitSet { // SAFETY: The `subscriptions` entry is an array of pointers, and this dereferencing is // equivalent to // https://github.com/ros2/rcl/blob/35a31b00a12f259d492bf53c0701003bd7f1745c/rcl/include/rcl/wait.h#L419 - let wait_set_entry = unsafe { *self.handle.subscriptions.add(i) }; + let wait_set_entry = unsafe { *self.rcl_wait_set.subscriptions.add(i) }; if !wait_set_entry.is_null() { ready_entities.subscriptions.push(subscription.clone()); }