diff --git a/roslibrust/examples/native_ros1.rs b/roslibrust/examples/native_ros1.rs index ddceef8b..5e4e3673 100644 --- a/roslibrust/examples/native_ros1.rs +++ b/roslibrust/examples/native_ros1.rs @@ -5,7 +5,7 @@ #[cfg(feature = "ros1")] #[tokio::main] async fn main() -> Result<(), Box> { - use roslibrust::NodeHandle; + use roslibrust::ros1::NodeHandle; simple_logger::SimpleLogger::new() .with_level(log::LevelFilter::Debug) diff --git a/roslibrust/examples/ros1_listener.rs b/roslibrust/examples/ros1_listener.rs index 3d94b3e5..cdbc6826 100644 --- a/roslibrust/examples/ros1_listener.rs +++ b/roslibrust/examples/ros1_listener.rs @@ -3,7 +3,7 @@ roslibrust_codegen_macro::find_and_generate_ros_messages!("assets/ros1_common_in #[cfg(feature = "ros1")] #[tokio::main] async fn main() -> Result<(), Box> { - use roslibrust::NodeHandle; + use roslibrust::ros1::NodeHandle; simple_logger::SimpleLogger::new() .with_level(log::LevelFilter::Debug) diff --git a/roslibrust/examples/ros1_talker.rs b/roslibrust/examples/ros1_talker.rs index 3d40d479..934a19cf 100644 --- a/roslibrust/examples/ros1_talker.rs +++ b/roslibrust/examples/ros1_talker.rs @@ -3,7 +3,7 @@ roslibrust_codegen_macro::find_and_generate_ros_messages!("assets/ros1_common_in #[cfg(feature = "ros1")] #[tokio::main] async fn main() -> Result<(), Box> { - use roslibrust::NodeHandle; + use roslibrust::ros1::NodeHandle; simple_logger::SimpleLogger::new() .with_level(log::LevelFilter::Debug) diff --git a/roslibrust/src/lib.rs b/roslibrust/src/lib.rs index 8932461d..e3044431 100644 --- a/roslibrust/src/lib.rs +++ b/roslibrust/src/lib.rs @@ -105,6 +105,33 @@ pub use rosbridge::*; pub mod rosapi; #[cfg(feature = "ros1")] -mod ros1; -#[cfg(feature = "ros1")] -pub use ros1::*; +pub mod ros1; + +/// For now starting with a central error type, may break this up more in future +#[derive(thiserror::Error, Debug)] +pub enum RosLibRustError { + #[error("Not currently connected to ros master / bridge")] + Disconnected, + // TODO we probably want to eliminate tungstenite from this and hide our + // underlying websocket implementation from the API + // currently we "technically" break the API when we change tungstenite verisons + #[error("Websocket communication error: {0}")] + CommFailure(#[from] tokio_tungstenite::tungstenite::Error), + #[error("Operation timed out: {0}")] + Timeout(#[from] tokio::time::error::Elapsed), + #[error("Failed to parse message from JSON: {0}")] + InvalidMessage(#[from] serde_json::Error), + #[error("Rosbridge server reported an error: {0}")] + ServerError(String), + #[error("Name does not meet ROS requirements: {0}")] + InvalidName(String), + // Generic catch-all error type for not-yet-handled errors + // TODO ultimately this type will be removed from API of library + #[error(transparent)] + Unexpected(#[from] anyhow::Error), +} + +/// Generic result type used as standard throughout library. +/// Note: many functions which currently return this will be updated to provide specific error +/// types in the future instead of the generic error here. +pub type RosLibRustResult = Result; diff --git a/roslibrust/src/ros1/mod.rs b/roslibrust/src/ros1/mod.rs index f8c2e514..2ae9fdb5 100644 --- a/roslibrust/src/ros1/mod.rs +++ b/roslibrust/src/ros1/mod.rs @@ -11,5 +11,7 @@ mod node; pub use node::*; mod publisher; +pub use publisher::Publisher; mod subscriber; +pub use subscriber::Subscriber; mod tcpros; diff --git a/roslibrust/src/ros1/names.rs b/roslibrust/src/ros1/names.rs index ebf9a120..d61a3d93 100644 --- a/roslibrust/src/ros1/names.rs +++ b/roslibrust/src/ros1/names.rs @@ -1,3 +1,6 @@ +use crate::{RosLibRustError, RosLibRustResult}; +use std::fmt::Display; + lazy_static::lazy_static! { static ref GRAPH_NAME_REGEX: regex::Regex = regex::Regex::new(r"^([/~a-zA-Z]){1}([a-zA-Z0-9_/])*([A-z0-9_])$").unwrap(); } @@ -8,12 +11,12 @@ pub struct Name { } impl Name { - pub fn new(name: impl Into) -> Option { + pub fn new(name: impl Into) -> RosLibRustResult { let name: String = name.into(); if is_valid(&name) { - Some(Self { inner: name }) + Ok(Self { inner: name }) } else { - None + Err(RosLibRustError::InvalidName(name)) } } @@ -54,6 +57,12 @@ fn is_valid(name: &str) -> bool { GRAPH_NAME_REGEX.is_match(name) } +impl Display for Name { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.inner.fmt(f) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/roslibrust/src/ros1/node/actor.rs b/roslibrust/src/ros1/node/actor.rs index 47276bfa..c72258a3 100644 --- a/roslibrust/src/ros1/node/actor.rs +++ b/roslibrust/src/ros1/node/actor.rs @@ -5,8 +5,9 @@ use crate::{ node::{XmlRpcServer, XmlRpcServerHandle}, publisher::Publication, subscriber::Subscription, + MasterClient, }, - MasterClient, ServiceCallback, + ServiceCallback, }; use abort_on_drop::ChildTask; use roslibrust_codegen::RosMessageType; @@ -142,14 +143,13 @@ impl NodeServerHandle { pub async fn register_publisher( &self, topic: &str, - topic_type: &str, queue_size: usize, ) -> Result>, Box> { let (sender, receiver) = oneshot::channel(); match self.node_server_sender.send(NodeMsg::RegisterPublisher { reply: sender, topic: topic.to_owned(), - topic_type: topic_type.to_owned(), + topic_type: T::ROS_TYPE_NAME.to_owned(), queue_size, msg_definition: T::DEFINITION.to_owned(), md5sum: T::MD5SUM.to_owned(), @@ -254,12 +254,7 @@ impl Node { let xmlrpc_server = XmlRpcServer::new(addr, xml_server_handle)?; let client_uri = format!("http://{hostname}:{}", xmlrpc_server.port()); - if let None = Name::new(node_name) { - log::error!("Node name {node_name} is not valid"); - return Err(Box::new(std::io::Error::from( - std::io::ErrorKind::InvalidInput, - ))); - } + let _ = Name::new(node_name)?; let rosmaster_client = MasterClient::new(master_uri, client_uri, node_name).await?; let mut node = Self { diff --git a/roslibrust/src/ros1/node/handle.rs b/roslibrust/src/ros1/node/handle.rs index 493ca3a4..75888ec8 100644 --- a/roslibrust/src/ros1/node/handle.rs +++ b/roslibrust/src/ros1/node/handle.rs @@ -41,7 +41,7 @@ impl NodeHandle { ) -> Result, Box> { let sender = self .inner - .register_publisher::(topic_name, T::ROS_TYPE_NAME, queue_size) + .register_publisher::(topic_name, queue_size) .await?; Ok(Publisher::new(topic_name, sender)) } diff --git a/roslibrust/src/ros1/node/mod.rs b/roslibrust/src/ros1/node/mod.rs index 4fa4a1d3..1a167764 100644 --- a/roslibrust/src/ros1/node/mod.rs +++ b/roslibrust/src/ros1/node/mod.rs @@ -1,7 +1,7 @@ //! This module contains the top level Node and NodeHandle classes. //! These wrap the lower level management of a ROS Node connection into a higher level and thread safe API. -use crate::RosMasterError; +use super::RosMasterError; use std::net::{IpAddr, Ipv4Addr}; mod actor; diff --git a/roslibrust/src/ros1/publisher.rs b/roslibrust/src/ros1/publisher.rs index bc1db0c6..b25abcca 100644 --- a/roslibrust/src/ros1/publisher.rs +++ b/roslibrust/src/ros1/publisher.rs @@ -1,6 +1,4 @@ -use crate::RosLibRustError; - -use super::tcpros::ConnectionHeader; +use crate::{ros1::tcpros::ConnectionHeader, RosLibRustError}; use abort_on_drop::ChildTask; use roslibrust_codegen::RosMessageType; use std::{ diff --git a/roslibrust/src/ros1/subscriber.rs b/roslibrust/src/ros1/subscriber.rs index 7d821a4d..7416dca1 100644 --- a/roslibrust/src/ros1/subscriber.rs +++ b/roslibrust/src/ros1/subscriber.rs @@ -1,4 +1,4 @@ -use super::tcpros::ConnectionHeader; +use crate::ros1::tcpros::ConnectionHeader; use abort_on_drop::ChildTask; use roslibrust_codegen::RosMessageType; use std::{marker::PhantomData, sync::Arc}; diff --git a/roslibrust/src/rosbridge/mod.rs b/roslibrust/src/rosbridge/mod.rs index 284b9dec..258c8602 100644 --- a/roslibrust/src/rosbridge/mod.rs +++ b/roslibrust/src/rosbridge/mod.rs @@ -30,46 +30,13 @@ mod topic_provider; mod comm; use futures_util::stream::{SplitSink, SplitStream}; -use log::*; use std::collections::HashMap; use tokio::net::TcpStream; use tokio_tungstenite::*; use tungstenite::Message; -/// For now starting with a central error type, may break this up more in future -#[derive(thiserror::Error, Debug)] -pub enum RosLibRustError { - #[error("Not currently connected to ros master / bridge")] - Disconnected, - // TODO we probably want to eliminate tungstenite from this and hide our - // underlying websocket implementation from the API - // currently we "technically" break the API when we change tungstenite verisons - #[error("Websocket communication error: {0}")] - CommFailure(tokio_tungstenite::tungstenite::Error), - #[error("Operation timed out: {0}")] - Timeout(#[from] tokio::time::error::Elapsed), - #[error("Failed to parse message from JSON: {0}")] - InvalidMessage(#[from] serde_json::Error), - #[error("Rosbridge server reported an error: {0}")] - ServerError(String), - // Generic catch-all error type for not-yet-handled errors - // TODO ultimately this type will be removed from API of library - #[error(transparent)] - Unexpected(#[from] anyhow::Error), -} - -/// Provides an implementation tranlating the underlying websocket error into our error type -impl From for RosLibRustError { - fn from(e: tokio_tungstenite::tungstenite::Error) -> Self { - // TODO we probably want to expand this type and do some matching here - RosLibRustError::CommFailure(e) - } -} - -/// Generic result type used as standard throughout library. -/// Note: many functions which currently return this will be updated to provide specific error -/// types in the future instead of the generic error here. -pub type RosLibRustResult = Result; +// Doing this to maintain backwards compatibilities like `use roslibrust::rosbridge::RosLibRustError` +pub use super::{RosLibRustError, RosLibRustResult}; /// Used for type erasure of message type so that we can store arbitrary handles type Callback = Box;