From ed9d8a75603808b9c9cc1ae33172f4a6ab39d531 Mon Sep 17 00:00:00 2001 From: Carter12s Date: Thu, 18 Jul 2024 08:38:50 -0600 Subject: [PATCH 1/8] Fix topic provider --- roslibrust/src/rosbridge/topic_provider.rs | 39 ++++++++++++++-------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/roslibrust/src/rosbridge/topic_provider.rs b/roslibrust/src/rosbridge/topic_provider.rs index b80d10aa..eb518bf8 100644 --- a/roslibrust/src/rosbridge/topic_provider.rs +++ b/roslibrust/src/rosbridge/topic_provider.rs @@ -10,7 +10,7 @@ use crate::RosLibRustResult; /// It assumes topics only carry one data type, but is not expected to enforce that. /// It assumes that all actions can fail due to a variety of causes, and by network interruption specifically. #[async_trait] -trait TopicProvider { +pub trait TopicProvider { // These associated types makeup the other half of the API // They are expected to be "self-deregistering", where dropping them results in unadvertise or unsubscribe operations as appropriate type Publisher; @@ -33,14 +33,19 @@ trait TopicProvider { request: Req, ) -> RosLibRustResult; - async fn advertise_service( + async fn advertise_service( &self, topic: &str, - server: fn( - T::Request, - ) - -> Result>, - ) -> RosLibRustResult; + server: F, + ) -> RosLibRustResult + where + F: Fn( + T::Request, + ) + -> Result> + + Send + + Sync + + 'static; } #[async_trait] @@ -71,15 +76,21 @@ impl TopicProvider for crate::ClientHandle { self.call_service(topic, request).await } - async fn advertise_service( + async fn advertise_service( &self, topic: &str, - server: fn( - T::Request, - ) - -> Result>, - ) -> RosLibRustResult { - self.advertise_service::(topic, server).await + server: F, + ) -> RosLibRustResult + where + F: Fn( + T::Request, + ) + -> Result> + + Send + + Sync + + 'static, + { + self.advertise_service::(topic, server).await } } From 365834052fdae4538ce418b6a5feab6bce319b83 Mon Sep 17 00:00:00 2001 From: Carter12s Date: Thu, 18 Jul 2024 11:32:09 -0600 Subject: [PATCH 2/8] Cleanup use of complex types with some aliases and auto impls --- roslibrust/src/lib.rs | 24 ++++++++++++++++++++++ roslibrust/src/ros1/mod.rs | 7 +++++++ roslibrust/src/ros1/node/actor.rs | 21 +++++-------------- roslibrust/src/ros1/node/handle.rs | 14 ++++++------- roslibrust/src/ros1/service_server.rs | 12 +---------- roslibrust/src/rosbridge/client.rs | 10 ++------- roslibrust/src/rosbridge/topic_provider.rs | 18 +++------------- 7 files changed, 49 insertions(+), 57 deletions(-) diff --git a/roslibrust/src/lib.rs b/roslibrust/src/lib.rs index 9278e650..aca3713e 100644 --- a/roslibrust/src/lib.rs +++ b/roslibrust/src/lib.rs @@ -100,6 +100,7 @@ mod rosbridge; pub use rosbridge::*; +use roslibrust_codegen::RosServiceType; #[cfg(feature = "rosapi")] pub mod rosapi; @@ -139,3 +140,26 @@ pub enum RosLibRustError { /// 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; + +// Note: service Fn is currently defined here as it used by ros1 and roslibrust impls +/// This trait describes a function which can validly act as a ROS service +/// server with roslibrust. We're really just using this as a trait alias +/// as the full definition is overly verbose and trait aliases are unstable. +pub trait ServiceFn: + Fn(T::Request) -> Result> + + Send + + Sync + + 'static +{ +} + +/// Automatic implementation of ServiceFn for Fn +impl ServiceFn for F +where + T: RosServiceType, + F: Fn(T::Request) -> Result> + + Send + + Sync + + 'static, +{ +} diff --git a/roslibrust/src/ros1/mod.rs b/roslibrust/src/ros1/mod.rs index 60486dde..e65b60d2 100644 --- a/roslibrust/src/ros1/mod.rs +++ b/roslibrust/src/ros1/mod.rs @@ -19,3 +19,10 @@ pub use subscriber::Subscriber; mod service_server; pub use service_server::ServiceServer; mod tcpros; + +/// Provides a common type alias for type erased service server functions. +/// Internally we use this type to store collections of server functions. +pub(crate) type TypeErasedCallback = dyn Fn(Vec) -> Result, Box> + + Send + + Sync + + 'static; diff --git a/roslibrust/src/ros1/node/actor.rs b/roslibrust/src/ros1/node/actor.rs index 562d8a54..21367059 100644 --- a/roslibrust/src/ros1/node/actor.rs +++ b/roslibrust/src/ros1/node/actor.rs @@ -6,9 +6,9 @@ use crate::{ service_client::ServiceClientLink, service_server::ServiceServerLink, subscriber::Subscription, - MasterClient, NodeError, ProtocolParams, ServiceClient, + MasterClient, NodeError, ProtocolParams, ServiceClient, TypeErasedCallback, }, - RosLibRustError, + RosLibRustError, ServiceFn, }; use abort_on_drop::ChildTask; use log::warn; @@ -68,11 +68,7 @@ pub enum NodeMsg { service: Name, service_type: String, srv_definition: String, - server: Box< - dyn Fn(Vec) -> Result, Box> - + Send - + Sync, - >, + server: Box, md5sum: String, }, UnregisterServiceServer { @@ -213,10 +209,7 @@ impl NodeServerHandle { ) -> Result<(), NodeError> where T: RosServiceType, - F: Fn(T::Request) -> Result> - + Send - + Sync - + 'static, + F: ServiceFn, { let (sender, receiver) = oneshot::channel(); @@ -685,11 +678,7 @@ impl Node { service: &Name, service_type: &str, srv_definition: &str, - server: Box< - dyn Fn(Vec) -> Result, Box> - + Send - + Sync, - >, + server: Box, md5sum: &str, ) -> Result<(), Box> { let found = self.service_servers.get_mut(service_type); diff --git a/roslibrust/src/ros1/node/handle.rs b/roslibrust/src/ros1/node/handle.rs index 86f36201..75e09cc6 100644 --- a/roslibrust/src/ros1/node/handle.rs +++ b/roslibrust/src/ros1/node/handle.rs @@ -1,7 +1,10 @@ use super::actor::{Node, NodeServerHandle}; -use crate::ros1::{ - names::Name, publisher::Publisher, service_client::ServiceClient, subscriber::Subscriber, - NodeError, ServiceServer, +use crate::{ + ros1::{ + names::Name, publisher::Publisher, service_client::ServiceClient, subscriber::Subscriber, + NodeError, ServiceServer, + }, + ServiceFn, }; /// Represents a handle to an underlying [Node]. NodeHandle's can be freely cloned, moved, copied, etc. @@ -97,10 +100,7 @@ impl NodeHandle { ) -> Result where T: roslibrust_codegen::RosServiceType, - F: Fn(T::Request) -> Result> - + Send - + Sync - + 'static, + F: ServiceFn, { let service_name = Name::new(service_name)?; let _response = self diff --git a/roslibrust/src/ros1/service_server.rs b/roslibrust/src/ros1/service_server.rs index 326abb5e..3e12b72a 100644 --- a/roslibrust/src/ros1/service_server.rs +++ b/roslibrust/src/ros1/service_server.rs @@ -9,17 +9,7 @@ use tokio::io::{AsyncReadExt, AsyncWriteExt}; use crate::ros1::tcpros::{self, ConnectionHeader}; -use super::{names::Name, NodeHandle}; - -// TODO: someday I'd like to define a trait alias here for a ServerFunction -// Currently unstable: -// https://doc.rust-lang.org/beta/unstable-book/language-features/trait-alias.html -// trait ServerFunction = Fn(T::Request) -> Err(T::Response, Box) + Send + Sync + 'static; - -type TypeErasedCallback = dyn Fn(Vec) -> Result, Box> - + Send - + Sync - + 'static; +use super::{names::Name, NodeHandle, TypeErasedCallback}; /// ServiceServer is simply a lifetime control /// The underlying ServiceServer is kept alive while object is kept alive. diff --git a/roslibrust/src/rosbridge/client.rs b/roslibrust/src/rosbridge/client.rs index 7643695f..28b33713 100644 --- a/roslibrust/src/rosbridge/client.rs +++ b/roslibrust/src/rosbridge/client.rs @@ -1,6 +1,6 @@ use crate::{ rosbridge::comm::{self, RosBridgeComm}, - Publisher, RosLibRustError, RosLibRustResult, ServiceHandle, Subscriber, + Publisher, RosLibRustError, RosLibRustResult, ServiceFn, ServiceHandle, Subscriber, }; use anyhow::anyhow; use dashmap::DashMap; @@ -427,13 +427,7 @@ impl ClientHandle { ) -> RosLibRustResult where T: RosServiceType, - F: Fn( - T::Request, - ) - -> Result> - + Send - + Sync - + 'static, + F: ServiceFn, { self.check_for_disconnect()?; { diff --git a/roslibrust/src/rosbridge/topic_provider.rs b/roslibrust/src/rosbridge/topic_provider.rs index eb518bf8..d0e9be71 100644 --- a/roslibrust/src/rosbridge/topic_provider.rs +++ b/roslibrust/src/rosbridge/topic_provider.rs @@ -1,7 +1,7 @@ use async_trait::async_trait; use roslibrust_codegen::{RosMessageType, RosServiceType}; -use crate::RosLibRustResult; +use crate::{RosLibRustResult, ServiceFn}; /// This trait generically describes the capability of something to act as an async interface to a set of topics /// @@ -39,13 +39,7 @@ pub trait TopicProvider { server: F, ) -> RosLibRustResult where - F: Fn( - T::Request, - ) - -> Result> - + Send - + Sync - + 'static; + F: ServiceFn; } #[async_trait] @@ -82,13 +76,7 @@ impl TopicProvider for crate::ClientHandle { server: F, ) -> RosLibRustResult where - F: Fn( - T::Request, - ) - -> Result> - + Send - + Sync - + 'static, + F: ServiceFn, { self.advertise_service::(topic, server).await } From 269cea794c6263b2ed2f49900ca37dc47658ff78 Mon Sep 17 00:00:00 2001 From: Carter12s Date: Thu, 18 Jul 2024 15:55:19 -0600 Subject: [PATCH 3/8] progress from plane --- roslibrust/examples/generic_client.rs | 38 ++++++++++ roslibrust/src/lib.rs | 7 ++ roslibrust/src/ros1/node/mod.rs | 22 ++++++ roslibrust/src/rosbridge/mod.rs | 4 -- .../src/{rosbridge => }/topic_provider.rs | 71 ++++++++++++++++++- 5 files changed, 135 insertions(+), 7 deletions(-) create mode 100644 roslibrust/examples/generic_client.rs rename roslibrust/src/{rosbridge => }/topic_provider.rs (63%) diff --git a/roslibrust/examples/generic_client.rs b/roslibrust/examples/generic_client.rs new file mode 100644 index 00000000..a96ac0c8 --- /dev/null +++ b/roslibrust/examples/generic_client.rs @@ -0,0 +1,38 @@ +//! Purpose of this example is to show how the TopicProvider trait can be use +//! to create code that is generic of which communication backend it will use. + +#[cfg(feature = "topic_provider")] +fn main() { + use roslibrust::topic_provider::TopicProvider; + + roslibrust_codegen_macro::find_and_generate_ros_messages!( + "assets/ros1_common_interfaces/std_msgs" + ); + // TopicProvider cannot be an "Object Safe Trait" due to its generic parameters + // This means we can't do: + // let x: Box = ros1::NodeHandle; + + // Which specific TopicProvider you are going to use must be known at + // compile time! We can use features to build multiple copies of our + // executable with different backends. Or mix and match within a + // single application. The critical part is to make TopicProvider a + // generic type on you Node. + + struct MyNode { + ros: T, + } + + impl MyNode { + async fn run(ros: T) { + let publisher = ros.advertise::("/chatter").await.unwrap(); + + loop {} + } + } + + // Start the node + +} + +#[cfg(not(feature = "topic_provider"))] +fn main() {} diff --git a/roslibrust/src/lib.rs b/roslibrust/src/lib.rs index aca3713e..69fc065b 100644 --- a/roslibrust/src/lib.rs +++ b/roslibrust/src/lib.rs @@ -108,6 +108,13 @@ pub mod rosapi; #[cfg(feature = "ros1")] pub mod ros1; +// Topic provider is locked behind a feature until it is stabalized +// additionally because of its use of generic associated types, it requires rust >1.65 +#[cfg(feature = "topic_provider")] +/// Provides a generic trait for building clients / against either the rosbridge, +/// ros1, or a mock backend +pub mod topic_provider; + /// For now starting with a central error type, may break this up more in future #[derive(thiserror::Error, Debug)] pub enum RosLibRustError { diff --git a/roslibrust/src/ros1/node/mod.rs b/roslibrust/src/ros1/node/mod.rs index bcc40ca6..e4d1432d 100644 --- a/roslibrust/src/ros1/node/mod.rs +++ b/roslibrust/src/ros1/node/mod.rs @@ -1,6 +1,8 @@ //! 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::RosLibRustError; + use super::{names::InvalidNameError, RosMasterError}; use std::{ io, @@ -11,6 +13,7 @@ mod actor; mod handle; mod xmlrpc; use actor::*; +use anyhow::anyhow; pub use handle::NodeHandle; use tokio::sync::{mpsc, oneshot}; use xmlrpc::*; @@ -98,3 +101,22 @@ impl From> for NodeError { Self::ChannelClosedError } } + +// TODO MAJOR: this is kinda messy +// but for historic development reasons (not having a design for errors) +// we produced two different error types for the two different impls (ros1, rosbridge) +// This allows fusing the two error types together so that TopicProider can work +// but we should just better design all the error handling +impl From for RosLibRustError { + fn from(value: NodeError) -> Self { + match value { + NodeError::RosMasterError(e) => RosLibRustError::ServerError(e.to_string()), + NodeError::ChannelClosedError => { + RosLibRustError::Unexpected(anyhow!("Channel closed, something was dropped?")) + } + NodeError::InvalidName(e) => RosLibRustError::InvalidName(e.to_string()), + NodeError::XmlRpcError(e) => RosLibRustError::SerializationError(e.to_string().into()), + NodeError::IoError(e) => RosLibRustError::IoError(e), + } + } +} diff --git a/roslibrust/src/rosbridge/mod.rs b/roslibrust/src/rosbridge/mod.rs index b21edea4..8b4b7e03 100644 --- a/roslibrust/src/rosbridge/mod.rs +++ b/roslibrust/src/rosbridge/mod.rs @@ -21,10 +21,6 @@ mod integration_tests; #[allow(dead_code)] type TestResult = Result<(), anyhow::Error>; -// Topic provider is locked behind a feature until it is stabalized -// additionally because of its use of generic associated types, it requires rust >1.65 -#[cfg(feature = "topic_provider")] -mod topic_provider; /// Communication primitives for the rosbridge_suite protocol mod comm; diff --git a/roslibrust/src/rosbridge/topic_provider.rs b/roslibrust/src/topic_provider.rs similarity index 63% rename from roslibrust/src/rosbridge/topic_provider.rs rename to roslibrust/src/topic_provider.rs index d0e9be71..be6b94fa 100644 --- a/roslibrust/src/rosbridge/topic_provider.rs +++ b/roslibrust/src/topic_provider.rs @@ -42,6 +42,7 @@ pub trait TopicProvider { F: ServiceFn; } +// Implementation of TopicProvider trait for rosbridge client #[async_trait] impl TopicProvider for crate::ClientHandle { type Publisher = crate::Publisher; @@ -82,10 +83,57 @@ impl TopicProvider for crate::ClientHandle { } } +#[cfg(feature = "ros1")] +#[async_trait] +impl TopicProvider for crate::ros1::NodeHandle { + type Publisher = crate::ros1::Publisher; + type Subscriber = crate::ros1::Subscriber; + type ServiceHandle = crate::ros1::ServiceServer; + + async fn advertise( + &self, + topic: &str, + ) -> RosLibRustResult> { + // TODO MAJOR: consider promoting queue size, making unlimited default + self.advertise::(topic.as_ref(), 10) + .await + .map_err(|e| e.into()) + } + + async fn subscribe( + &self, + topic: &str, + ) -> RosLibRustResult> { + // TODO MAJOR: consider promoting queue size, making unlimited default + self.subscribe(topic, 10).await.map_err(|e| e.into()) + } + + async fn call_service( + &self, + topic: &str, + request: Req, + ) -> RosLibRustResult { + self.call_service(topic, request).await + } + + async fn advertise_service( + &self, + topic: &str, + server: F, + ) -> RosLibRustResult + where + F: ServiceFn, + { + self.advertise_service::(topic, server) + .await + .map_err(|e| e.into()) + } +} + #[cfg(test)] mod test { use super::TopicProvider; - use crate::ClientHandle; + use crate::{ros1::NodeHandle, ClientHandle}; // This test specifically fails because TopicProvider is not object safe // Traits that have methods with generic parameters cannot be object safe in rust (currently) @@ -98,7 +146,7 @@ mod test { // This tests proves that you could use topic provider in a compile time api, but not object safe... #[test_log::test] #[should_panic] - fn topic_proivder_can_be_used_at_compile_time() { + fn topic_provider_can_be_used_at_compile_time() { struct MyClient { _client: T, } @@ -107,8 +155,25 @@ mod test { // constructing one at runtime let new_mock: Result = Err(anyhow::anyhow!("Expected error")); + + let _x = MyClient { + _client: new_mock.unwrap(), // panic + }; + } + + #[test_log::test] + #[should_panic] + fn topic_provider_can_be_used_with_ros1() { + struct MyClient { + _client: T, + } + + // Kinda a hack way to make the compiler prove it could construct a MyClient with out actually + // constructing one at runtime + let new_mock: Result = Err(anyhow::anyhow!("Expected error")); + let _x = MyClient { - _client: new_mock.unwrap(), + _client: new_mock.unwrap(), // panic }; } } From 473767cb3d80296c70fb2d246b566d5c354e3958 Mon Sep 17 00:00:00 2001 From: Carter12s Date: Fri, 19 Jul 2024 10:14:40 -0600 Subject: [PATCH 4/8] Topic provider shenanigan progress --- roslibrust/examples/generic_client.rs | 14 ++++++++++---- roslibrust/src/topic_provider.rs | 22 +++++++++++++++++++++- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/roslibrust/examples/generic_client.rs b/roslibrust/examples/generic_client.rs index a96ac0c8..f3088bf4 100644 --- a/roslibrust/examples/generic_client.rs +++ b/roslibrust/examples/generic_client.rs @@ -3,7 +3,7 @@ #[cfg(feature = "topic_provider")] fn main() { - use roslibrust::topic_provider::TopicProvider; + use roslibrust::topic_provider::*; roslibrust_codegen_macro::find_and_generate_ros_messages!( "assets/ros1_common_interfaces/std_msgs" @@ -26,12 +26,18 @@ fn main() { async fn run(ros: T) { let publisher = ros.advertise::("/chatter").await.unwrap(); - loop {} + loop { + let msg = std_msgs::String { + data: "Hello World!".to_string(), + }; + publisher.publish(&msg).await.unwrap(); + } } } - // Start the node - + // create a rosbridge handle and start node + + let } #[cfg(not(feature = "topic_provider"))] diff --git a/roslibrust/src/topic_provider.rs b/roslibrust/src/topic_provider.rs index be6b94fa..932e551f 100644 --- a/roslibrust/src/topic_provider.rs +++ b/roslibrust/src/topic_provider.rs @@ -3,6 +3,26 @@ use roslibrust_codegen::{RosMessageType, RosServiceType}; use crate::{RosLibRustResult, ServiceFn}; +// Indicates that something is a publisher and has our expected publish +// function +pub trait Publish { + async fn publish(&self, data: &T) -> RosLibRustResult<()>; +} + +impl Publish for crate::Publisher { + async fn publish(&self, data: &T) -> RosLibRustResult<()> { + // TODO clone here is bad and we should standardized on ownership of publish + self.publish(data.clone()).await + } +} + +impl Publish for crate::ros1::Publisher { + async fn publish(&self, data: &T) -> RosLibRustResult<()> { + // TODO error type conversion here is terrible and we need to standardize error stuff badly + self.publish(data).await.map_err(|e| crate::RosLibRustError::SerializationError(e.to_string())) + } +} + /// This trait generically describes the capability of something to act as an async interface to a set of topics /// /// This trait is largely based on ROS concepts, but could be extended to other protocols / concepts. @@ -13,7 +33,7 @@ use crate::{RosLibRustResult, ServiceFn}; pub trait TopicProvider { // These associated types makeup the other half of the API // They are expected to be "self-deregistering", where dropping them results in unadvertise or unsubscribe operations as appropriate - type Publisher; + type Publisher: Publish; type Subscriber; type ServiceHandle; From 0e47f55458fc7eafff82912005253c728bd41513 Mon Sep 17 00:00:00 2001 From: carter Date: Tue, 30 Jul 2024 16:33:00 -0600 Subject: [PATCH 5/8] Random work --- Cargo.lock | 12 --------- roslibrust/Cargo.toml | 1 - roslibrust/examples/generic_client.rs | 35 ++++++++++++++++++++---- roslibrust/src/rosbridge/comm.rs | 3 --- roslibrust/src/topic_provider.rs | 39 +++++++++++++++------------ 5 files changed, 52 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 63c102cd..763ecc16 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -114,17 +114,6 @@ version = "1.0.70" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7de8ce5e0f9f8d88245311066a578d72b7af3e7088f32783804676302df237e4" -[[package]] -name = "async-trait" -version = "0.1.68" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b9ccdd8f2a161be9bd5c023df56f1b2a0bd1d83872ae53b71a84a12c9bf6e842" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.15", -] - [[package]] name = "atty" version = "0.2.14" @@ -1280,7 +1269,6 @@ version = "0.10.1" dependencies = [ "abort-on-drop", "anyhow", - "async-trait", "byteorder", "dashmap", "deadqueue", diff --git a/roslibrust/Cargo.toml b/roslibrust/Cargo.toml index 2380188d..0a83cca7 100644 --- a/roslibrust/Cargo.toml +++ b/roslibrust/Cargo.toml @@ -13,7 +13,6 @@ categories = ["science::robotics"] [dependencies] abort-on-drop = "0.2" anyhow = "1.0" -async-trait = "0.1" byteorder = "1.4" dashmap = "5.3" deadqueue = "0.2.4" # .4+ is required to fix bug with missing tokio dep diff --git a/roslibrust/examples/generic_client.rs b/roslibrust/examples/generic_client.rs index f3088bf4..c4a796ef 100644 --- a/roslibrust/examples/generic_client.rs +++ b/roslibrust/examples/generic_client.rs @@ -2,7 +2,14 @@ //! to create code that is generic of which communication backend it will use. #[cfg(feature = "topic_provider")] -fn main() { +#[tokio::main] +async fn main() { + simple_logger::SimpleLogger::new() + .with_level(log::LevelFilter::Debug) + .without_timestamps() // required for running wsl2 + .init() + .unwrap(); + use roslibrust::topic_provider::*; roslibrust_codegen_macro::find_and_generate_ros_messages!( @@ -20,24 +27,42 @@ fn main() { struct MyNode { ros: T, + name: String, } impl MyNode { - async fn run(ros: T) { - let publisher = ros.advertise::("/chatter").await.unwrap(); + async fn run(self) { + let publisher = self.ros.advertise::("/chatter").await.unwrap(); loop { let msg = std_msgs::String { - data: "Hello World!".to_string(), + data: format!("Hello world from {}", self.name), }; publisher.publish(&msg).await.unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(500)).await; } } } // create a rosbridge handle and start node + let ros = roslibrust::ClientHandle::new("ws://localhost:9090").await.unwrap(); + let node = MyNode { ros, name: "rosbridge_node".to_string() }; + tokio::spawn(async move { node.run().await }); + + // create a ros1 handle and start node + let ros = roslibrust::ros1::NodeHandle::new("http://localhost:11311", "/my_node").await.unwrap(); + let node = MyNode { ros, name: "ros1_node".to_string() }; + tokio::spawn(async move { node.run().await }); + + loop { + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + println!("sleeping"); + } - let + // With this executable running + // RUST_LOG=debug cargo run --features ros1,topic_provider --example generic_client + // You should be able to run `rostopic echo /chatter` and see the two nodes print out their names. + // Note: this will not run without rosbridge running } #[cfg(not(feature = "topic_provider"))] diff --git a/roslibrust/src/rosbridge/comm.rs b/roslibrust/src/rosbridge/comm.rs index 212d8b3e..2f66edb4 100644 --- a/roslibrust/src/rosbridge/comm.rs +++ b/roslibrust/src/rosbridge/comm.rs @@ -1,6 +1,5 @@ use crate::{rosbridge::Writer, RosLibRustResult}; use anyhow::bail; -use async_trait::async_trait; use futures_util::SinkExt; use log::debug; use roslibrust_codegen::RosMessageType; @@ -88,7 +87,6 @@ impl FromStr for Ops { /// So we're defining this trait on a foreign type, since we didn't end up /// using this trait for mocking. I'm inclined to replace it, and move the /// impls directly into some wrapper around [Writer] -#[async_trait] pub(crate) trait RosBridgeComm { async fn subscribe(&mut self, topic: &str, msg_type: &str) -> RosLibRustResult<()>; async fn unsubscribe(&mut self, topic: &str) -> RosLibRustResult<()>; @@ -113,7 +111,6 @@ pub(crate) trait RosBridgeComm { ) -> RosLibRustResult<()>; } -#[async_trait] impl RosBridgeComm for Writer { async fn subscribe(&mut self, topic: &str, msg_type: &str) -> RosLibRustResult<()> { let msg = json!( diff --git a/roslibrust/src/topic_provider.rs b/roslibrust/src/topic_provider.rs index 932e551f..4500cd77 100644 --- a/roslibrust/src/topic_provider.rs +++ b/roslibrust/src/topic_provider.rs @@ -1,12 +1,15 @@ -use async_trait::async_trait; use roslibrust_codegen::{RosMessageType, RosServiceType}; use crate::{RosLibRustResult, ServiceFn}; // Indicates that something is a publisher and has our expected publish -// function +// Implementors of this trait are expected to auto-cleanup the publisher when dropped pub trait Publish { - async fn publish(&self, data: &T) -> RosLibRustResult<()>; + // Note: this is really just syntactic de-sugared `async fn` + // However see: https://blog.rust-lang.org/2023/12/21/async-fn-rpit-in-traits.html + // This generates a warning is rust as of writing due to ambiguity around the "Send-ness" of the return type + // We only plan to work with multi-threaded work stealing executors (e.g. tokio) so we're manually specifying Send + fn publish(&self, data: &T) -> impl futures::Future> + Send; } impl Publish for crate::Publisher { @@ -19,7 +22,9 @@ impl Publish for crate::Publisher { impl Publish for crate::ros1::Publisher { async fn publish(&self, data: &T) -> RosLibRustResult<()> { // TODO error type conversion here is terrible and we need to standardize error stuff badly - self.publish(data).await.map_err(|e| crate::RosLibRustError::SerializationError(e.to_string())) + self.publish(data) + .await + .map_err(|e| crate::RosLibRustError::SerializationError(e.to_string())) } } @@ -29,7 +34,7 @@ impl Publish for crate::ros1::Publisher { /// Fundamentally, it assumes that topics are uniquely identified by a string name (likely an ASCII assumption is buried in here...). /// It assumes topics only carry one data type, but is not expected to enforce that. /// It assumes that all actions can fail due to a variety of causes, and by network interruption specifically. -#[async_trait] +// #[async_trait] pub trait TopicProvider { // These associated types makeup the other half of the API // They are expected to be "self-deregistering", where dropping them results in unadvertise or unsubscribe operations as appropriate @@ -37,33 +42,32 @@ pub trait TopicProvider { type Subscriber; type ServiceHandle; - async fn advertise( + fn advertise( &self, topic: &str, - ) -> RosLibRustResult>; + ) -> impl futures::Future>> + Send; - async fn subscribe( + fn subscribe( &self, topic: &str, - ) -> RosLibRustResult>; + ) -> impl futures::Future>> + Send; - async fn call_service( + fn call_service( &self, topic: &str, request: Req, - ) -> RosLibRustResult; + ) -> impl std::future::Future> + Send; - async fn advertise_service( + fn advertise_service( &self, topic: &str, server: F, - ) -> RosLibRustResult + ) -> impl futures::Future> + Send where F: ServiceFn; } // Implementation of TopicProvider trait for rosbridge client -#[async_trait] impl TopicProvider for crate::ClientHandle { type Publisher = crate::Publisher; type Subscriber = crate::Subscriber; @@ -104,7 +108,6 @@ impl TopicProvider for crate::ClientHandle { } #[cfg(feature = "ros1")] -#[async_trait] impl TopicProvider for crate::ros1::NodeHandle { type Publisher = crate::ros1::Publisher; type Subscriber = crate::ros1::Subscriber; @@ -133,7 +136,10 @@ impl TopicProvider for crate::ros1::NodeHandle { topic: &str, request: Req, ) -> RosLibRustResult { - self.call_service(topic, request).await + // TODO this is a problem + // service_client for Ros1 wants the service type, not the req and res types + // We should change top level API of TopicProvider and probably of rosbridge + unimplemented!(); } async fn advertise_service( @@ -175,7 +181,6 @@ mod test { // constructing one at runtime let new_mock: Result = Err(anyhow::anyhow!("Expected error")); - let _x = MyClient { _client: new_mock.unwrap(), // panic }; From 830b461f8dece3bb585fb0853457878e0e42a41e Mon Sep 17 00:00:00 2001 From: carter Date: Tue, 6 Aug 2024 09:51:18 -0600 Subject: [PATCH 6/8] Fmt and update changelog --- CHANGELOG.md | 8 ++++-- README.md | 35 +++++++++++++-------------- roslibrust/examples/generic_client.rs | 27 +++++++++++++++------ roslibrust/src/lib.rs | 2 +- roslibrust/src/rosbridge/mod.rs | 1 - roslibrust/src/topic_provider.rs | 2 +- 6 files changed, 45 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e27a42c..5c39fd56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ All notable changes to this project will be documented in this file. -## Release Instructions: +## Release Instructions Steps: @@ -22,14 +22,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Added + - ROS1 Native Publishers now support latching behavior - The XML RPC client for interacting directly with the rosmaster server has been exposed as a public API +- Experimental: Initial support for writing generic clients that can be compile time specialized for rosbridge or ros1 ### Fixed + - ROS1 Native Publishers correctly call unadvertise when dropped ### Changed -- ROS1 Node Handle's advertise() now requies a latching argument + +- ROS1 Node Handle's advertise() now requires a latching argument ## 0.10.2 - August 3rd, 2024 diff --git a/README.md b/README.md index f8763c6d..3cd62c1f 100644 --- a/README.md +++ b/README.md @@ -40,31 +40,30 @@ Rough overview of the features planned to built for this crate in what order: | Feature | rosbridge | ROS1 | ROS2 | |------------------------------|-------------------------------------------------------------|------|------| -| examples | ✅ | ✅ | x | -| message_gen | ✅ | ✅ | ✅ | -| advertise / publish | ✅ | ✅ | x | -| unadvertise | ✅ | ✅ | x | -| subscribe | ✅ | ✅ | x | -| unsubscribe | ✅ | ✅ | x | -| services | ✅ | x | x | -| rosapi | ✅ (ROS1 only for now) | N/A | N/A | +| examples | ✅ | ✅ | x | +| message_gen | ✅ | ✅ | ✅ | +| advertise / publish | ✅ | ✅ | x | +| unadvertise | ✅ | ✅ | x | +| subscribe | ✅ | ✅ | x | +| unsubscribe | ✅ | ✅ | x | +| services | ✅ | ✅ | x | +| actions | (codgen of messages only) | +| rosapi | ✅ | x | x | | TLS / wss:// | Should be working, untested | N/A | N/A | -| ROS2 msgs length limits | Planned | N/A | x | -| cbor | Planned | N/A | N/A | -| rosbridge status access | Planned | N/A | N/A | -| rosout logger | Planned | x | x | -| auth | Planned | N/A | N/A | -| fragment / png | Uncertain if this package will support | x | N/A | -| cbor-raw | Uncertain if this package will support | N/A | N/A | - ## Contributing Contribution through reporting of issues encountered and implementation in PRs is welcome! Before landing a large PR with lots of code implemented, please open an issue if there isn't a relevant one already available and chat with a maintainer to make sure the design fits well with all supported platforms and any in-progress implementation efforts. -### Rust Version for Development +### Minimum Supported Rust Version / MSRV + +We don't have an official MSRV yet. + +Due to cargo 1.72 enabling "doctest-in-workspace" by default it is recommended to use Rust 1.72+ for development. +Previous rust versions are support but will require some incantations when executing doctests. -Due to cargo 1.72 enabling "doctest-in-workspace" by default it is reccomended to use Rust 1.72+ for develop. Previous rust versions are support but will require some incantations when executing doctests. +The experimental topic_provider feature currently relies on `async fn` in traits from Rust 1.75. +When that feature standardizes that will likely become our MSRV. ### Running Tests diff --git a/roslibrust/examples/generic_client.rs b/roslibrust/examples/generic_client.rs index c4a796ef..e850485e 100644 --- a/roslibrust/examples/generic_client.rs +++ b/roslibrust/examples/generic_client.rs @@ -17,7 +17,6 @@ async fn main() { ); // TopicProvider cannot be an "Object Safe Trait" due to its generic parameters // This means we can't do: - // let x: Box = ros1::NodeHandle; // Which specific TopicProvider you are going to use must be known at // compile time! We can use features to build multiple copies of our @@ -32,7 +31,11 @@ async fn main() { impl MyNode { async fn run(self) { - let publisher = self.ros.advertise::("/chatter").await.unwrap(); + let publisher = self + .ros + .advertise::("/chatter") + .await + .unwrap(); loop { let msg = std_msgs::String { @@ -45,13 +48,23 @@ async fn main() { } // create a rosbridge handle and start node - let ros = roslibrust::ClientHandle::new("ws://localhost:9090").await.unwrap(); - let node = MyNode { ros, name: "rosbridge_node".to_string() }; + let ros = roslibrust::ClientHandle::new("ws://localhost:9090") + .await + .unwrap(); + let node = MyNode { + ros, + name: "rosbridge_node".to_string(), + }; tokio::spawn(async move { node.run().await }); // create a ros1 handle and start node - let ros = roslibrust::ros1::NodeHandle::new("http://localhost:11311", "/my_node").await.unwrap(); - let node = MyNode { ros, name: "ros1_node".to_string() }; + let ros = roslibrust::ros1::NodeHandle::new("http://localhost:11311", "/my_node") + .await + .unwrap(); + let node = MyNode { + ros, + name: "ros1_node".to_string(), + }; tokio::spawn(async move { node.run().await }); loop { @@ -62,7 +75,7 @@ async fn main() { // With this executable running // RUST_LOG=debug cargo run --features ros1,topic_provider --example generic_client // You should be able to run `rostopic echo /chatter` and see the two nodes print out their names. - // Note: this will not run without rosbridge running + // Note: this will not run without rosbridge running } #[cfg(not(feature = "topic_provider"))] diff --git a/roslibrust/src/lib.rs b/roslibrust/src/lib.rs index 69fc065b..78927e8c 100644 --- a/roslibrust/src/lib.rs +++ b/roslibrust/src/lib.rs @@ -112,7 +112,7 @@ pub mod ros1; // additionally because of its use of generic associated types, it requires rust >1.65 #[cfg(feature = "topic_provider")] /// Provides a generic trait for building clients / against either the rosbridge, -/// ros1, or a mock backend +/// ros1, or a mock backend pub mod topic_provider; /// For now starting with a central error type, may break this up more in future diff --git a/roslibrust/src/rosbridge/mod.rs b/roslibrust/src/rosbridge/mod.rs index 8b4b7e03..a7948b7e 100644 --- a/roslibrust/src/rosbridge/mod.rs +++ b/roslibrust/src/rosbridge/mod.rs @@ -21,7 +21,6 @@ mod integration_tests; #[allow(dead_code)] type TestResult = Result<(), anyhow::Error>; - /// Communication primitives for the rosbridge_suite protocol mod comm; diff --git a/roslibrust/src/topic_provider.rs b/roslibrust/src/topic_provider.rs index 4500cd77..ced81f56 100644 --- a/roslibrust/src/topic_provider.rs +++ b/roslibrust/src/topic_provider.rs @@ -118,7 +118,7 @@ impl TopicProvider for crate::ros1::NodeHandle { topic: &str, ) -> RosLibRustResult> { // TODO MAJOR: consider promoting queue size, making unlimited default - self.advertise::(topic.as_ref(), 10) + self.advertise::(topic.as_ref(), 10, false) .await .map_err(|e| e.into()) } From 77e9e0a2b770abd12383a70d6d60296ed02b817b Mon Sep 17 00:00:00 2001 From: carter Date: Tue, 6 Aug 2024 09:59:19 -0600 Subject: [PATCH 7/8] Update README.md --- README.md | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 3cd62c1f..08b81d8b 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,9 @@ [![Iron](https://github.com/Carter12s/roslibrust/actions/workflows/iron.yml/badge.svg)](https://github.com/Carter12s/roslibrust/actions/workflows/iron.yml) [![License:MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT) -This package aims to provide a convenient intermediary between ROS1's rosbridge and Rust similar to roslibpy and roslibjs. +This package aims to provide a convenient "async first" library for interacting with ROS. + +Currently this packaged provides support for both ROS1 native communication (TCPROS) and rosbridge's protocol which provides support for both ROS1 and ROS2 albeit with some overhead. Information about the protocol can be found [here](https://github.com/RobotWebTools/rosbridge_suite). @@ -13,7 +15,7 @@ Note on documentation: All information about the crate itself (examples, documentation, tutorials, etc.) lives in the source code and can be viewed on [docs.rs](https://docs.rs/roslibrust). This readme is for "Meta" information about developing for the crate. -Fully Supported via rosbridge: Noetic, Galactic, Humble. +Fully Supported via rosbridge: Noetic, Galactic, Humble, Iron, ## Code Generation of ROS Messages @@ -24,20 +26,14 @@ It's used like this: roslibrust_codegen_macro::find_and_generate_ros_messages!("assets/ros1_common_interfaces/std_msgs"); ``` -Code generation can also be done with a script using the same code generation backend called by the macro. See the contents of `example_package` for a detailed example of how this can be done. While the proc_macros are extremely convenient for getting started +Code generation can also be done with a build.rs script using the same code generation backend called by the macro. See the contents of `example_package` for a detailed example of how this can be done. While the proc_macros are extremely convenient for getting started there is currently no (good) way for a proc_macro to inform the compiler that it needs to be re-generated when an external file changes. Using a build script requires more setup, but can correctly handling re-building when message files are edited. -## Experimental Support for ROS1 Native - -If built with the `ros1` feature, `roslibrust` exports some experimental support for implementing nodes which talk to other ROS1 nodes using the TCPROS protocol without the need for the rosbridge as an intermediary. See `ros1_talker.rs` and `ros1_listener.rs` under `roslibrust/examples` to see usage. This implementation is relatively new, incomplete, and untested. Filing issues on bugs encountered is very appreciated! - -See this issue filter for known issues: https://github.com/Carter12s/roslibrust/labels/ros1 +Generated message types are compatible with both the ROS1 native and RosBridge backends. ## Roadmap -Rough overview of the features planned to built for this crate in what order: - | Feature | rosbridge | ROS1 | ROS2 | |------------------------------|-------------------------------------------------------------|------|------| | examples | ✅ | ✅ | x | @@ -47,10 +43,16 @@ Rough overview of the features planned to built for this crate in what order: | subscribe | ✅ | ✅ | x | | unsubscribe | ✅ | ✅ | x | | services | ✅ | ✅ | x | -| actions | (codgen of messages only) | +| actions | (codgen of message types only) | | rosapi | ✅ | x | x | | TLS / wss:// | Should be working, untested | N/A | N/A | +Upcoming features in rough order: + +- Ability to write generic clients via ROS trait +- In memory backend that can be used for testing +- Support for parameter server + ## Contributing Contribution through reporting of issues encountered and implementation in PRs is welcome! Before landing a large PR with lots of code implemented, please open an issue if there isn't a relevant one already available and chat with a maintainer to make sure the design fits well with all supported platforms and any in-progress implementation efforts. From d7a673a6358de15542a09edfd13da2b19ab5facd Mon Sep 17 00:00:00 2001 From: carter Date: Tue, 6 Aug 2024 10:12:42 -0600 Subject: [PATCH 8/8] Fix rosapi for no-async-trait --- roslibrust/src/rosapi/mod.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/roslibrust/src/rosapi/mod.rs b/roslibrust/src/rosapi/mod.rs index a8daf3d0..37160bed 100644 --- a/roslibrust/src/rosapi/mod.rs +++ b/roslibrust/src/rosapi/mod.rs @@ -4,7 +4,6 @@ //! Ensure rosapi is running on your target system before attempting to utilize these features! use crate::{ClientHandle, RosLibRustResult}; -use async_trait::async_trait; // TODO major issue here for folks who actually try to use rosapi in their project // This macro isn't going to expand correctly when not used from this crate's workspace @@ -14,7 +13,6 @@ roslibrust_codegen_macro::find_and_generate_ros_messages!("assets/ros1_common_in /// Represents the ability to interact with the interfaces provided by the rosapi node. /// This trait is implemented for ClientHandle when the `rosapi` feature is enabled. -#[async_trait] trait RosApi { async fn get_time(&self) -> RosLibRustResult; async fn topics(&self) -> RosLibRustResult; @@ -99,7 +97,6 @@ trait RosApi { async fn get_services(&self) -> RosLibRustResult; } -#[async_trait] impl RosApi for ClientHandle { /// Get the current time async fn get_time(&self) -> RosLibRustResult {