Skip to content

Commit a506f60

Browse files
authored
Merge pull request #299 from RosLibRust/consistent-topic-name-handling
Fix topic names flexibility
2 parents 5da01a3 + e94cdaf commit a506f60

File tree

19 files changed

+513
-244
lines changed

19 files changed

+513
-244
lines changed

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@ They also correctly fallback to using the IP address of the first interface in t
1616

1717
### Changed
1818

19+
- Reorganized some of the module structure of roslibrust_common. Users pulling things from `roslibrust` will see no change.
20+
- All API calls that were taking a topic name as "&str" now take a "impl ToGlobalTopicName" instead.
21+
This allows for more ergonomic usage of the API, and allows us to extend the API to support substitutions in the future.
22+
The API is now more strict about the format of topic names. See the documentation for GlobalTopicName for more details.
23+
Previously names like "chatter" could be handled differently be different backends that may or may not have added a leading slash automatically.
24+
Now all names **MUST** be a fully resolved name.
25+
1926
## 0.18.0 - November 25th, 2025
2027

2128
### Added

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ This allows writing generic behaviors like:
2323
use roslibrust::{TopicProvider, Publish, Subscribe};
2424

2525
async fn relay<T: TopicProvider>(ros: T) -> roslibrust::Result<()> {
26-
let mut subscriber = ros.subscribe::<std_msgs::String>("in").await?;
27-
let mut publisher = ros.advertise::<std_msgs::String>("out").await?;
26+
let mut subscriber = ros.subscribe::<std_msgs::String>("/in").await?;
27+
let mut publisher = ros.advertise::<std_msgs::String>("/out").await?;
2828
while let Ok(msg) = subscriber.next().await {
2929
println!("Got message: {}", msg.data);
3030
publisher.publish(&msg).await?;

example_package/src/main.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use roslibrust::Publish;
1313
// and the generated types from the macro above.
1414
async fn pub_counter(ros: impl roslibrust::Ros) {
1515
let publisher = ros
16-
.advertise::<std_msgs::Int16>("example_counter")
16+
.advertise::<std_msgs::Int16>("/example_counter")
1717
.await
1818
.unwrap();
1919
let mut counter = 0;
@@ -60,7 +60,7 @@ mod test {
6060

6161
// Subscribe to the topic we're publishing to
6262
let mut subscriber = ros
63-
.subscribe::<std_msgs::Int16>("example_counter")
63+
.subscribe::<std_msgs::Int16>("/example_counter")
6464
.await
6565
.unwrap();
6666

example_package_macro/src/main.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ roslibrust::find_and_generate_ros_messages!(
1414
// and the generated types from the macro above.
1515
async fn pub_counter(ros: impl roslibrust::Ros) {
1616
let publisher = ros
17-
.advertise::<std_msgs::Int16>("example_counter")
17+
.advertise::<std_msgs::Int16>("/example_counter")
1818
.await
1919
.unwrap();
2020
let mut counter = 0;
@@ -61,7 +61,7 @@ mod test {
6161

6262
// Subscribe to the topic we're publishing to
6363
let mut subscriber = ros
64-
.subscribe::<std_msgs::Int16>("example_counter")
64+
.subscribe::<std_msgs::Int16>("/example_counter")
6565
.await
6666
.unwrap();
6767

roslibrust_codegen/src/ros2_hashing.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//! Module for calculating the ROS2 hash of a message definition via https://github.com/ros-infrastructure/rep/pull/381/files
1+
//! Module for calculating the ROS2 hash of a message definition via <https://github.com/ros-infrastructure/rep/pull/381/files>
22
33
use anyhow::bail;
44
use log::*;

roslibrust_codegen/src/serde_rosmsg_bytes.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
use base64::{engine::general_purpose::STANDARD, Engine};
1212
use serde::{Deserialize, Deserializer, Serializer};
1313

14-
/// Serialize a Vec<u8> as a base64 string
14+
/// Serialize a `Vec<u8>` as a base64 string
1515
///
1616
/// This is compatible with rosbridge's protocol which expects uint8[] as base64.
1717
pub fn serialize<S>(bytes: &Vec<u8>, serializer: S) -> Result<S::Ok, S::Error>
@@ -29,7 +29,7 @@ where
2929
}
3030
}
3131

32-
/// Deserialize a Vec<u8> from either a base64 string or binary format
32+
/// Deserialize a `Vec<u8>` from either a base64 string or binary format
3333
///
3434
/// This allows the same generated code to work with:
3535
/// - Rosbridge (which sends base64 strings)

roslibrust_common/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,5 @@ md5 = "0.7"
2020
futures-core = "0.3"
2121
# Used for implementation of into_stream for subscribers
2222
async-stream = "0.3"
23+
# Used for validation of topic names
24+
regex = "1.9"

roslibrust_common/src/lib.rs

Lines changed: 9 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -46,82 +46,11 @@ pub enum Error {
4646
/// Generic result type used throughout roslibrust.
4747
pub type Result<T> = std::result::Result<T, Error>;
4848

49-
/// Fundamental traits for message types this crate works with
50-
/// This trait will be satisfied for any types generated with this crate's message_gen functionality
51-
pub trait RosMessageType:
52-
'static + serde::de::DeserializeOwned + Send + serde::Serialize + Sync + Clone + std::fmt::Debug
53-
{
54-
/// Expected to be the combination pkg_name/type_name string describing the type to ros
55-
/// Example: std_msgs/Header
56-
const ROS_TYPE_NAME: &'static str;
57-
/// The computed md5sum of the message file and its dependencies
58-
/// This field is optional, and only needed when using ros1 native communication
59-
const MD5SUM: &'static str = "";
60-
/// The definition from the msg, srv, or action file
61-
/// This field is optional, and only needed when using ros1 native communication
62-
const DEFINITION: &'static str = "";
63-
/// The fully qualified type name we need to work with ROS2 zenoh
64-
/// e.g. std_msgs::msg::dds_::String_
65-
/// This field is optional, and only needed when using ros2 native communication
66-
const ROS2_TYPE_NAME: &'static str = "";
67-
/// The computed ROS2 hash of the message file and its dependencies
68-
/// This field is optional, and only needed when using ros2 native communication
69-
const ROS2_HASH: &'static [u8; 32] = &[0; 32];
70-
}
71-
72-
// This special impl allows for services with no args / returns
73-
impl RosMessageType for () {
74-
const ROS_TYPE_NAME: &'static str = "";
75-
const MD5SUM: &'static str = "";
76-
const DEFINITION: &'static str = "";
77-
}
78-
79-
/// Represents a ROS service type definition corresponding to a `.srv` file.
80-
///
81-
/// Typically this trait will not be implemented by hand but instead be generated by using [roslibrust's codegen functionality][<https://docs.rs/roslibrust/latest/roslibrust/codegen>].
82-
/// This trait is used by the [ServiceProvider] trait to define types that can be used with [ServiceProvider::call_service] and [ServiceProvider::advertise_service]
83-
pub trait RosServiceType: 'static + Send + Sync {
84-
/// Name of the ros service e.g. `rospy_tutorials/AddTwoInts` in ROS1
85-
const ROS_SERVICE_NAME: &'static str;
86-
/// The computed md5sum of the message file and its dependencies
87-
/// This field is optional, and only needed when using ros1 native communication
88-
const MD5SUM: &'static str = "";
89-
/// The computed ROS2 hash of the message file and its dependencies
90-
/// This field is optional, and only needed when using ros2 native communication
91-
const ROS2_HASH: &'static [u8; 32] = &[0; 32];
92-
/// The fully qualified type name we need to work with ROS2 zenoh
93-
/// e.g. std_srvs::srv::dds_::SetBool_
94-
const ROS2_TYPE_NAME: &'static str = "";
95-
/// The type of data being sent in the request
96-
type Request: RosMessageType;
97-
/// The type of the data
98-
type Response: RosMessageType;
99-
}
100-
10149
/// The error type used by [ServiceFn]
10250
///
10351
/// When writing service callbacks this is the error type that should be returned.
10452
pub type ServiceError = anyhow::Error;
10553

106-
/// This trait describes a function which can validly act as a ROS service
107-
/// server with roslibrust. We're really just using this as a trait alias
108-
/// as the full definition is overly verbose and trait aliases are unstable.
109-
///
110-
/// Note: The error type intentionally does NOT have a 'static bound to allow
111-
/// for more flexible lifetime inference when used with tokio::spawn and similar.
112-
pub trait ServiceFn<T: RosServiceType>:
113-
Fn(T::Request) -> std::result::Result<T::Response, ServiceError> + Send + Sync + 'static
114-
{
115-
}
116-
117-
/// Automatic implementation of ServiceFn for Fn
118-
impl<T, F> ServiceFn<T> for F
119-
where
120-
T: RosServiceType,
121-
F: Fn(T::Request) -> std::result::Result<T::Response, ServiceError> + Send + Sync + 'static,
122-
{
123-
}
124-
12554
/// A generic message type used by some implementations to provide a generic subscriber / publisher without serialization
12655
#[derive(:: serde :: Deserialize, :: serde :: Serialize, Debug, Default, Clone, PartialEq)]
12756
pub struct ShapeShifter(Vec<u8>);
@@ -133,11 +62,17 @@ impl RosMessageType for ShapeShifter {
13362
const DEFINITION: &'static str = "";
13463
}
13564

136-
/// Contains functions for calculating md5sums of message definitions
65+
/// Contains functions for calculating md5sums of message definitions.
66+
///
13767
/// These functions are needed both in roslibrust_ros1 and roslibrust_codegen so they're in this crate
68+
/// for the moment.
13869
pub mod md5sum;
13970

140-
/// Contains the generic traits represent a pubsub system and service system
141-
/// These traits will be implemented for specific backends to provides access to "ROS Like" functionality
71+
/// Contains the generic traits represent a pubsub system and service system.
72+
/// These traits will be implemented for specific backends to provides access to "ROS Like" functionality.
14273
pub mod traits;
14374
pub use traits::*; // Bring topic provider traits into root namespace
75+
76+
/// Contains the validation logic for topic, service, and action names.
77+
pub mod topic_name;
78+
pub use topic_name::*; // Bring topic name validation into root namespace
Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
use crate::Error as RError;
2+
3+
/// A unified representation for the names of topics, services, and actions.
4+
///
5+
/// This type tries to conform to the naming conventions of both ROS1 and ROS2,
6+
/// but only supports a subset of the valid names as a result.
7+
///
8+
/// For the time being roslibrust has decided to perform no automatic substitutions of name
9+
/// elements for users, so all names must be "Global" and be fully resolved and start with a `/`.
10+
/// ROS2 refers to these names a "fully qualified name".
11+
///
12+
/// Examples of valid global names (to roslibrust's standards are):
13+
/// * `/chatter`
14+
/// * `/foo/bar/baz`
15+
/// * `/foo_bar_baz`
16+
/// * `/abc123/def_456`
17+
///
18+
/// Examples of invalid Global names:
19+
/// * `chatter` - ROS1 / ROS2 may namespace this differently
20+
/// * `~chatter` - ROS2 rejects this without a '/' between `~` and the name
21+
/// * `~/chatter` - Even with the '/' RosLibRust will reject this as we don't support the '~'
22+
///
23+
///
24+
// For developers of this area look here for ROS1 documentation https://wiki.ros.org/Names
25+
// and here for ROS2 documentation https://design.ros2.org/articles/topic_and_service_names.html
26+
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
27+
pub struct GlobalTopicName {
28+
inner: String,
29+
}
30+
31+
impl GlobalTopicName {
32+
pub fn new(name: impl Into<String>) -> Result<GlobalTopicName, RError> {
33+
let name: String = name.into();
34+
match validate_global_name(&name) {
35+
Ok(()) => Ok(Self { inner: name }),
36+
Err(failures) => Err(RError::InvalidName(format!(
37+
"Invalid topic name: {name}, reasons: {failures:?}"
38+
))),
39+
}
40+
}
41+
}
42+
43+
/// Can print the name with `{}` syntax as it has a canonical string representation
44+
impl std::fmt::Display for GlobalTopicName {
45+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
46+
self.inner.fmt(f)
47+
}
48+
}
49+
50+
// Conversion into a String is allowed as it is the canonical representation
51+
impl From<GlobalTopicName> for String {
52+
fn from(name: GlobalTopicName) -> Self {
53+
name.inner
54+
}
55+
}
56+
57+
// Allow GlobalTopicName to be used as &str
58+
impl AsRef<str> for GlobalTopicName {
59+
fn as_ref(&self) -> &str {
60+
&self.inner
61+
}
62+
}
63+
64+
/// This trait represents types that can be converted into a GlobalTopicName
65+
///
66+
/// This trait is in use within roslibrust because we have APIs that want to take either
67+
/// &str, String, GlobalTopicName, or &GlobalTopicName, but we don't want to use TryFrom
68+
/// as the error types are not compatible, and would force additional boilerplate on users.
69+
pub trait ToGlobalTopicName: Send {
70+
fn to_global_name(self) -> Result<GlobalTopicName, RError>;
71+
}
72+
73+
impl ToGlobalTopicName for GlobalTopicName {
74+
fn to_global_name(self) -> Result<GlobalTopicName, RError> {
75+
Ok(self)
76+
}
77+
}
78+
79+
impl ToGlobalTopicName for &GlobalTopicName {
80+
fn to_global_name(self) -> Result<GlobalTopicName, RError> {
81+
Ok(self.clone())
82+
}
83+
}
84+
85+
impl ToGlobalTopicName for String {
86+
fn to_global_name(self) -> Result<GlobalTopicName, RError> {
87+
GlobalTopicName::new(self)
88+
}
89+
}
90+
91+
impl ToGlobalTopicName for &str {
92+
fn to_global_name(self) -> Result<GlobalTopicName, RError> {
93+
GlobalTopicName::new(self)
94+
}
95+
}
96+
97+
static GLOBAL_NAME_REGEX: std::sync::LazyLock<regex::Regex> = std::sync::LazyLock::new(|| {
98+
// Best attempt at a regex that matches both ROS1 and ROS2 naming conventions
99+
regex::Regex::new(r"(?-u)^\/([A-Za-z][A-Za-z0-9_]*)(\/[A-Za-z][A-Za-z0-9_]*)*$").unwrap()
100+
});
101+
102+
/// Check the name against our set of rules for validity
103+
/// Returns a list of reasons the name is invalid
104+
fn validate_global_name(name: &str) -> Result<(), Vec<String>> {
105+
// First character must be a '/'
106+
let mut failures = vec![];
107+
if !name.starts_with('/') {
108+
failures.push("Name must start with a '/'".to_string());
109+
}
110+
111+
// Name must not contain any whitespace
112+
if name.contains(char::is_whitespace) {
113+
failures.push("Name must not contain whitespace".to_string());
114+
}
115+
116+
// Name must not contain characters other than alphanumeric, underscore, and forward slash
117+
if !name
118+
.chars()
119+
.all(|c| c.is_alphanumeric() || c == '_' || c == '/')
120+
{
121+
failures.push(
122+
"Name must only contain alphanumeric characters, underscores, and forward slashes"
123+
.to_string(),
124+
);
125+
}
126+
127+
// Name must not end with a '/'
128+
if name.ends_with('/') {
129+
failures.push("Name must not end with a '/'".to_string());
130+
}
131+
132+
// Use the ROS1 validation regex for a final check (in a lazy cell)
133+
if !GLOBAL_NAME_REGEX.is_match(name) {
134+
failures.push("Name must match the ROS1 name validation regex".to_string());
135+
}
136+
137+
if failures.is_empty() {
138+
Ok(())
139+
} else {
140+
Err(failures)
141+
}
142+
}
143+
144+
#[cfg(test)]
145+
mod tests {
146+
use super::*;
147+
148+
#[test]
149+
fn test_valid_global_names() {
150+
assert!(GlobalTopicName::new("/chatter").is_ok());
151+
assert!(GlobalTopicName::new("/foo/bar/baz").is_ok());
152+
assert!(GlobalTopicName::new("/foo_bar_baz").is_ok());
153+
assert!(GlobalTopicName::new("/abc123/def_456").is_ok());
154+
155+
assert!(GlobalTopicName::new("chatter").is_err());
156+
assert!(GlobalTopicName::new("chatter/").is_err());
157+
assert!(GlobalTopicName::new("/chatter/").is_err());
158+
assert!(GlobalTopicName::new("/chatter ").is_err());
159+
assert!(GlobalTopicName::new("/chatter space").is_err());
160+
assert!(GlobalTopicName::new("/chatter#").is_err());
161+
assert!(GlobalTopicName::new("~chatter").is_err());
162+
assert!(GlobalTopicName::new("/chatter/{ros2}").is_err());
163+
assert!(GlobalTopicName::new("/chatter-").is_err());
164+
assert!(GlobalTopicName::new("/chatter/with space").is_err());
165+
assert!(GlobalTopicName::new("/chatter/with#hash").is_err());
166+
assert!(GlobalTopicName::new("/empty//bad").is_err());
167+
168+
// It is unclear for the ROS documentation if this should be valid or not
169+
// assert!(GlobalTopicName::new("/123/_456/").is_err());
170+
// assert!(GlobalTopicName::new("/123").is_err());
171+
}
172+
173+
#[test]
174+
fn type_conversions_exist_and_behave() {
175+
// Test for the ToGlobalTopicName trait - this is how roslibrust traits work!
176+
fn generic_with_to_global<MsgType>(name: impl ToGlobalTopicName) {
177+
let name: GlobalTopicName = name.to_global_name().unwrap();
178+
assert_eq!(name.to_string(), "/chatter".to_string());
179+
}
180+
181+
// Works with String
182+
generic_with_to_global::<String>("/chatter".to_string());
183+
// Works with &str
184+
generic_with_to_global::<String>("/chatter");
185+
// Works with GlobalTopicName
186+
generic_with_to_global::<String>(GlobalTopicName::new("/chatter").unwrap());
187+
// Works with &GlobalTopicName
188+
generic_with_to_global::<String>(&GlobalTopicName::new("/chatter").unwrap());
189+
}
190+
}

0 commit comments

Comments
 (0)