Skip to content

Rename variables of rcl types #193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions rclrs/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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<Mutex<rcl_context_t>>,
pub(crate) rcl_context_mtx: Arc<Mutex<rcl_context_t>>,
}

impl Context {
Expand Down Expand Up @@ -77,31 +77,31 @@ 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() {
std::ptr::null()
} 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)),
})
}

Expand Down Expand Up @@ -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) }
}
}
6 changes: 3 additions & 3 deletions rclrs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use std::time::Duration;
pub fn spin_once(node: &Node, timeout: Option<Duration>) -> 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)?;

Expand All @@ -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) {
Expand Down
14 changes: 7 additions & 7 deletions rclrs/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,16 @@ unsafe impl Send for rcl_node_t {}
/// [3]: crate::NodeBuilder::new
/// [4]: crate::NodeBuilder::namespace
pub struct Node {
handle: Arc<Mutex<rcl_node_t>>,
pub(crate) context: Arc<Mutex<rcl_context_t>>,
rcl_node_mtx: Arc<Mutex<rcl_node_t>>,
pub(crate) rcl_context_mtx: Arc<Mutex<rcl_context_t>>,
pub(crate) subscriptions: Vec<Weak<dyn SubscriptionBase>>,
}

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)
}
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
20 changes: 10 additions & 10 deletions rclrs/src/node/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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![],
})
}
Expand Down
32 changes: 16 additions & 16 deletions rclrs/src/node/publisher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,23 @@ use rosidl_runtime_rs::{Message, RmwMessage};
unsafe impl Send for rcl_publisher_t {}

pub(crate) struct PublisherHandle {
handle: Mutex<rcl_publisher_t>,
node_handle: Arc<Mutex<rcl_node_t>>,
rcl_publisher_mtx: Mutex<rcl_publisher_t>,
rcl_node_mtx: Arc<Mutex<rcl_node_t>>,
}

impl PublisherHandle {
fn lock(&self) -> MutexGuard<rcl_publisher_t> {
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 _);
}
}
}
Expand Down Expand Up @@ -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 =
<T as Message>::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,
Expand All @@ -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 {
Expand All @@ -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 <T as Message>::RmwMsg as *mut _,
std::ptr::null_mut(),
)
Expand Down
35 changes: 17 additions & 18 deletions rclrs/src/node/subscription.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -19,23 +18,23 @@ unsafe impl Send for rcl_subscription_t {}

/// Internal struct used by subscriptions.
pub struct SubscriptionHandle {
handle: Mutex<rcl_subscription_t>,
node_handle: Arc<Mutex<rcl_node_t>>,
rcl_subscription_mtx: Mutex<rcl_subscription_t>,
rcl_node_mtx: Arc<Mutex<rcl_node_t>>,
}

impl SubscriptionHandle {
pub(crate) fn lock(&self) -> MutexGuard<rcl_subscription_t> {
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);
}
}
}
Expand Down Expand Up @@ -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 =
<T as Message>::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,
Expand All @@ -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 {
Expand Down Expand Up @@ -149,13 +148,13 @@ where
// ```
pub fn take(&self) -> Result<T, RclrsError> {
let mut rmw_message = <T as 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 <T as Message>::RmwMsg as *mut _,
std::ptr::null_mut(),
std::ptr::null_mut(),
Expand All @@ -171,7 +170,7 @@ where
T: Message,
{
fn handle(&self) -> &SubscriptionHandle {
self.handle.borrow()
&self.handle
}

fn execute(&self) -> Result<(), RclrsError> {
Expand Down
Loading