Moving refactoring changes to this earlier MR

roslibrust/examples/native_ros1.rs

@@ -5,7 +5,7 @@
 #[cfg(feature = "ros1")]
 #[tokio::main]
 async fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
-    use roslibrust::NodeHandle;
+    use roslibrust::ros1::NodeHandle;
 
     simple_logger::SimpleLogger::new()
         .with_level(log::LevelFilter::Debug)

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<dyn std::error::Error + Send + Sync>> {
-    use roslibrust::NodeHandle;
+    use roslibrust::ros1::NodeHandle;
 
     simple_logger::SimpleLogger::new()
         .with_level(log::LevelFilter::Debug)

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<dyn std::error::Error + Send + Sync>> {
-    use roslibrust::NodeHandle;
+    use roslibrust::ros1::NodeHandle;
 
     simple_logger::SimpleLogger::new()
         .with_level(log::LevelFilter::Debug)

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<T> = Result<T, RosLibRustError>;

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;

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<String>) -> Option<Self> {
+    pub fn new(name: impl Into<String>) -> RosLibRustResult<Self> {
         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::*;

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<T: RosMessageType>(
         &self,
         topic: &str,
-        topic_type: &str,
         queue_size: usize,
     ) -> Result<mpsc::Sender<Vec<u8>>, Box<dyn std::error::Error + Send + Sync>> {
         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 {

roslibrust/src/ros1/node/handle.rs

@@ -41,7 +41,7 @@ impl NodeHandle {
     ) -> Result<Publisher<T>, Box<dyn std::error::Error + Send + Sync>> {
         let sender = self
             .inner
-            .register_publisher::<T>(topic_name, T::ROS_TYPE_NAME, queue_size)
+            .register_publisher::<T>(topic_name, queue_size)
             .await?;
         Ok(Publisher::new(topic_name, sender))
     }

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;

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::{

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

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<tokio_tungstenite::tungstenite::Error> 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<T> = Result<T, RosLibRustError>;
+// 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<dyn Fn(&str) + Send + Sync>;