Skip to content

Add new node name-related functions #143

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
May 2, 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
21 changes: 21 additions & 0 deletions rclrs/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,27 @@ impl Context {
Node::new(node_name, self)
}

/// Creates a node in a namespace.
///
/// Convenience function equivalent to [`Node::new_with_namespace`][1].
///
/// [1]: crate::Node::new_with_namespace
///
/// # Example
/// ```
/// # use rclrs::Context;
/// let ctx = Context::new([]).unwrap();
/// let node = ctx.create_node_with_namespace("/my/nested/namespace", "my_node");
/// assert!(node.is_ok());
/// ```
pub fn create_node_with_namespace(
&self,
node_namespace: &str,
node_name: &str,
) -> Result<Node, RclReturnCode> {
Node::new_with_namespace(node_namespace, node_name, self)
}

/// Checks if the context is still valid.
///
/// This will return `false` when a signal has caused the context to shut down (currently
Expand Down
88 changes: 85 additions & 3 deletions rclrs/src/node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ mod subscription;
pub use self::publisher::*;
pub use self::subscription::*;

use std::ffi::CString;
use std::ffi::{CStr, CString};
use std::sync::{Arc, Weak};
use std::vec::Vec;

use libc::c_char;
use parking_lot::Mutex;

use rosidl_runtime_rs::Message;
Expand Down Expand Up @@ -43,16 +44,16 @@ impl Node {
/// Creates a new node in the empty namespace.
#[allow(clippy::new_ret_no_self)]
pub fn new(node_name: &str, context: &Context) -> Result<Node, RclReturnCode> {
Self::new_with_namespace(node_name, "", context)
Self::new_with_namespace("", node_name, context)
}

/// Creates a new node in a namespace.
///
/// A namespace without a leading forward slash is automatically changed to have a leading
/// forward slash.
pub fn new_with_namespace(
node_name: &str,
node_ns: &str,
node_name: &str,
context: &Context,
) -> Result<Node, RclReturnCode> {
let raw_node_name = CString::new(node_name).unwrap();
Expand Down Expand Up @@ -88,6 +89,87 @@ impl Node {
})
}

/// Returns the name of the node.
///
/// This returns the name after remapping, so it is not necessarily the same as the name that
/// was used when creating the node.
///
/// # Example
/// ```
/// # use rclrs::{Context, RclReturnCode};
/// // Without remapping
/// let context = Context::new([])?;
/// let node = context.create_node("my_node")?;
/// assert_eq!(node.name(), String::from("my_node"));
/// // With remapping
/// let remapping = ["--ros-args", "-r", "__node:=your_node"].map(String::from);
/// let context_r = Context::new(remapping)?;
/// let node_r = context_r.create_node("my_node")?;
/// assert_eq!(node_r.name(), String::from("your_node"));
/// # Ok::<(), RclReturnCode>(())
/// ```
pub fn name(&self) -> String {
self.get_string(rcl_node_get_name)
}

/// Returns the namespace of the node.
///
/// This returns the namespace after remapping, so it is not necessarily the same as the
/// namespace that was used when creating the node.
///
/// # Example
/// ```
/// # use rclrs::{Context, RclReturnCode};
/// // Without remapping
/// let context = Context::new([])?;
/// let node = context.create_node_with_namespace("/my/namespace", "my_node")?;
/// assert_eq!(node.namespace(), String::from("/my/namespace"));
/// // With remapping
/// let remapping = ["--ros-args", "-r", "__ns:=/your_namespace"].map(String::from);
/// let context_r = Context::new(remapping)?;
/// let node_r = context_r.create_node("my_node")?;
/// assert_eq!(node_r.namespace(), String::from("/your_namespace"));
/// # Ok::<(), RclReturnCode>(())
/// ```
pub fn namespace(&self) -> String {
self.get_string(rcl_node_get_namespace)
}

/// Returns the fully qualified name of the node.
///
/// The fully qualified name of the node is the node namespace combined with the node name.
/// It is subject to the remappings shown in [`Node::name`] and [`Node::namespace`].
///
/// # Example
/// ```
/// # use rclrs::{Context, RclReturnCode};
/// let context = Context::new([])?;
/// let node = context.create_node_with_namespace("/my/namespace", "my_node")?;
/// assert_eq!(node.fully_qualified_name(), String::from("/my/namespace/my_node"));
/// # Ok::<(), RclReturnCode>(())
/// ```
pub fn fully_qualified_name(&self) -> String {
self.get_string(rcl_node_get_fully_qualified_name)
}

fn get_string(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good candidate for a macro, IMHO

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't care much either way in this instance, but the approach using a function could be said to have a few advantages:

  • Less code size (obviously doesn't matter much for such a small function)
  • No need to know macro_rules!
  • Better IDE support
  • You can see the type of the function supported by the helper
  • Definition of the helper is closer to the call site (since macro would typically be at the top level)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little less efficient though, but I think for these functions that won't matter.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense. I was just thinking of #141 and how to integrate it with this, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's something we'll need to think about. Maybe call_ffi!(getter, …) will already work if the right token type is used.

&self,
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())
};
debug_assert!(!char_ptr.is_null());
let cstr = unsafe {
// SAFETY: The returned CStr is immediately converted to an owned string,
// so the lifetime is no issue. The ptr is valid as per the documentation
// of rcl_node_get_name.
CStr::from_ptr(char_ptr)
};
cstr.to_string_lossy().into_owned()
}

/// Creates a [`Publisher`][1].
///
/// [1]: crate::Publisher
Expand Down