From 2b19fe3fcc4a420591f3215f09fd6e45a8a8b125 Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Tue, 1 Oct 2024 15:57:57 +0200 Subject: [PATCH 01/10] added more interfaces --- rclrs/src/parameter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclrs/src/parameter.rs b/rclrs/src/parameter.rs index c8a710eeb..78f9f0f7a 100644 --- a/rclrs/src/parameter.rs +++ b/rclrs/src/parameter.rs @@ -13,7 +13,7 @@ use crate::vendor::rcl_interfaces::msg::rmw::{ParameterType, ParameterValue as R use crate::{call_string_getter_with_rcl_node, rcl_bindings::*, Node, RclrsError}; use std::{ collections::{btree_map::Entry, BTreeMap}, - fmt::Debug, + fmt::{self, Debug, Display}, marker::PhantomData, sync::{Arc, Mutex, RwLock, Weak}, }; From adba623cd3811b56a871ea2cba248c88df549a4e Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Tue, 1 Oct 2024 15:58:22 +0200 Subject: [PATCH 02/10] added rudimentary display implementation --- rclrs/src/parameter.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/rclrs/src/parameter.rs b/rclrs/src/parameter.rs index 78f9f0f7a..1ba17263b 100644 --- a/rclrs/src/parameter.rs +++ b/rclrs/src/parameter.rs @@ -643,6 +643,34 @@ pub enum DeclarationError { /// An invalid range was provided to a parameter declaration (i.e. lower bound > higher bound). InvalidRange, } +impl Display for DeclarationError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + DeclarationError::AlreadyDeclared => write!( + f, + "Parameter was already declared and a new declaration was attempted" + ), + DeclarationError::NoValueAvailable => write!( + f, + "Parameter was declared as non optional but no value was available" + ), + DeclarationError::OverrideValueTypeMismatch => { + write!(f, "The override value that was provided has the wrong type") + } + DeclarationError::PriorValueTypeMismatch => write!( + f, + "The value that the parameter was already set to has the wrong type" + ), + DeclarationError::InitialValueOutOfRange => { + write!(f, "The initial value that was selected is out of range") + } + DeclarationError::InvalidRange => write!( + f, + "An invalid range was provided to a parameter declaration" + ), + } + } +} impl<'a> Parameters<'a> { /// Tries to read a parameter of the requested type. From ad65fff693869a649e6cc886f63ec4103b28e9d9 Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 2 Oct 2024 11:49:57 +0200 Subject: [PATCH 03/10] added new important import --- rclrs/src/node/graph.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rclrs/src/node/graph.rs b/rclrs/src/node/graph.rs index c8079b32a..42c68953b 100644 --- a/rclrs/src/node/graph.rs +++ b/rclrs/src/node/graph.rs @@ -1,5 +1,6 @@ use std::{ collections::HashMap, + env, ffi::{CStr, CString}, slice, }; From 05a131242a378903ca83a4cbe2a6f97ffeb13ebf Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 2 Oct 2024 11:50:16 +0200 Subject: [PATCH 04/10] tryage to make error handling cleaner --- rclrs/src/node/graph.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/rclrs/src/node/graph.rs b/rclrs/src/node/graph.rs index 42c68953b..27781e4e7 100644 --- a/rclrs/src/node/graph.rs +++ b/rclrs/src/node/graph.rs @@ -479,15 +479,11 @@ mod tests { // // 99 and 98 are just chosen as arbitrary valid domain ID values. There is // otherwise nothing special about either value. - let domain_id: usize = std::env::var("ROS_DOMAIN_ID") - .ok() - .and_then(|value| value.parse().ok()) - .map(|value: usize| if value != 99 { 99 } else { 98 }) - .unwrap_or(99); - - let context = - Context::new_with_options([], InitOptions::new().with_domain_id(Some(domain_id))) - .unwrap(); + let domain_id = env::var("ROS_DOMAIN_ID") + .map_err(|e| format!("Failed to parse ROS_DOMAIN_ID: {}", e)) + .and_then(|val| val.parse::().map_err(|e| format!("{}", e))) + .map(|id| if id != 99 { 99 } else { 98 }) + .expect("Error setting domain_id"); let node_name = "test_publisher_names_and_types"; let node = Node::new(&context, node_name).unwrap(); // Test that the graph has no publishers From 1bdb9ae2f6182a0ace0b8f28fba5bd2de8a7f07d Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 2 Oct 2024 11:51:34 +0200 Subject: [PATCH 05/10] Get rid of documentation warnings. Documentation warnings are annoying. So i've get rid of them. --- rclrs/src/parameter.rs | 4 ++-- rclrs/src/parameter/range.rs | 2 +- rclrs/src/subscription/message_info.rs | 30 +++++++++++++------------- rclrs/src/wait.rs | 2 +- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/rclrs/src/parameter.rs b/rclrs/src/parameter.rs index 1ba17263b..6cbc35bf7 100644 --- a/rclrs/src/parameter.rs +++ b/rclrs/src/parameter.rs @@ -696,9 +696,9 @@ impl<'a> Parameters<'a> { /// Returns: /// * `Ok(())` if setting was successful. /// * [`Err(DeclarationError::TypeMismatch)`] if the type of the requested value is different - /// from the parameter's type. + /// from the parameter's type. /// * [`Err(DeclarationError::OutOfRange)`] if the requested value is out of the parameter's - /// range. + /// range. /// * [`Err(DeclarationError::ReadOnly)`] if the parameter is read only. pub fn set( &self, diff --git a/rclrs/src/parameter/range.rs b/rclrs/src/parameter/range.rs index 96f66d6e3..6a46d2ff0 100644 --- a/rclrs/src/parameter/range.rs +++ b/rclrs/src/parameter/range.rs @@ -32,7 +32,7 @@ impl From<()> for ParameterRanges { /// Usually only one of these ranges will be applied, but all have to be stored since: /// /// * A dynamic parameter can change its type at runtime, in which case a different range could be -/// applied. +/// applied. /// * Introspection through service calls requires all the ranges to be reported to the user. #[derive(Clone, Debug, Default)] pub struct ParameterRanges { diff --git a/rclrs/src/subscription/message_info.rs b/rclrs/src/subscription/message_info.rs index 010bf28ec..b36421d8e 100644 --- a/rclrs/src/subscription/message_info.rs +++ b/rclrs/src/subscription/message_info.rs @@ -7,23 +7,23 @@ use crate::rcl_bindings::*; /// To quote the `rmw` documentation: /// /// > The identifier uniquely identifies the publisher for the local context, but -/// it will not necessarily be the same identifier given in other contexts or processes -/// for the same publisher. -/// Therefore the identifier will uniquely identify the publisher within your application -/// but may disagree about the identifier for that publisher when compared to another -/// application. -/// Even with this limitation, when combined with the publisher sequence number it can -/// uniquely identify a message within your local context. -/// Publisher GIDs generated by the RMW implementation could collide at some point, in which -/// case it is not possible to distinguish which publisher sent the message. -/// The details of how GIDs are generated are RMW implementation dependent. +/// > it will not necessarily be the same identifier given in other contexts or processes +/// > for the same publisher. +/// > Therefore the identifier will uniquely identify the publisher within your application +/// > but may disagree about the identifier for that publisher when compared to another +/// > application. +/// > Even with this limitation, when combined with the publisher sequence number it can +/// > uniquely identify a message within your local context. +/// > Publisher GIDs generated by the RMW implementation could collide at some point, in which +/// > case it is not possible to distinguish which publisher sent the message. +/// > The details of how GIDs are generated are RMW implementation dependent. /// /// > It is possible the the RMW implementation needs to reuse a publisher GID, -/// due to running out of unique identifiers or some other constraint, in which case -/// the RMW implementation may document what happens in that case, but that -/// behavior is not defined here. -/// However, this should be avoided, if at all possible, by the RMW implementation, -/// and should be unlikely to happen in practice. +/// > due to running out of unique identifiers or some other constraint, in which case +/// > the RMW implementation may document what happens in that case, but that +/// > behavior is not defined here. +/// > However, this should be avoided, if at all possible, by the RMW implementation, +/// > and should be unlikely to happen in practice. #[derive(Clone, Debug, PartialEq, Eq)] pub struct PublisherGid { /// Bytes identifying a publisher in the RMW implementation. diff --git a/rclrs/src/wait.rs b/rclrs/src/wait.rs index 2ef99c026..243c9d857 100644 --- a/rclrs/src/wait.rs +++ b/rclrs/src/wait.rs @@ -329,7 +329,7 @@ impl WaitSet { /// /// - Passing a wait set with no wait-able items in it will return an error. /// - The timeout must not be so large so as to overflow an `i64` with its nanosecond - /// representation, or an error will occur. + /// representation, or an error will occur. /// /// This list is not comprehensive, since further errors may occur in the `rmw` or `rcl` layers. /// From 8827064f89a42c60e950fa48af97e8ccc4bc0b62 Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 2 Oct 2024 11:52:25 +0200 Subject: [PATCH 06/10] get rid of rust version the rust version causes errors. You'll need at least rust version 1.70 to have stable compiling --- rclrs/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/rclrs/Cargo.toml b/rclrs/Cargo.toml index f489ea3f0..4ef3cbbf8 100644 --- a/rclrs/Cargo.toml +++ b/rclrs/Cargo.toml @@ -6,7 +6,6 @@ authors = ["Esteve Fernandez ", "Nikolai Morin Date: Wed, 2 Oct 2024 11:53:27 +0200 Subject: [PATCH 07/10] added serde as optional dependecy I mean for real: How patient do you'll need to be to not get annoyed by the whole serde warnings? --- rclrs/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/rclrs/Cargo.toml b/rclrs/Cargo.toml index 4ef3cbbf8..615c0ff4b 100644 --- a/rclrs/Cargo.toml +++ b/rclrs/Cargo.toml @@ -27,6 +27,7 @@ libloading = { version = "0.8", optional = true } # Needed for the Message trait, among others rosidl_runtime_rs = "0.4" +serde = { version = "1.0.210", optional = true } [dev-dependencies] # Needed for e.g. writing yaml files in tests From 552e2d4697c60f24cdead259329ec88697d058a9 Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 2 Oct 2024 11:53:39 +0200 Subject: [PATCH 08/10] formatting improvement --- rclrs/Cargo.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rclrs/Cargo.toml b/rclrs/Cargo.toml index 615c0ff4b..93907a0f1 100644 --- a/rclrs/Cargo.toml +++ b/rclrs/Cargo.toml @@ -33,7 +33,7 @@ serde = { version = "1.0.210", optional = true } # Needed for e.g. writing yaml files in tests tempfile = "3.3.0" # Needed for publisher and subscriber tests -test_msgs = {version = "*"} +test_msgs = { version = "*" } # Needed for parameter service tests tokio = { version = "*", features = ["rt", "time", "macros"] } @@ -46,6 +46,7 @@ cfg-if = "1.0.0" [features] default = [] dyn_msg = ["ament_rs", "libloading"] +serde = ["dep:serde"] # This feature is solely for the purpose of being able to generate documetation without a ROS installation # The only intended usage of this feature is for docs.rs builders to work, and is not intended to be used by end users generate_docs = ["rosidl_runtime_rs/generate_docs"] From 02539ffd9da053293a07b4fc7dde2ae37a5e8f04 Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 2 Oct 2024 11:56:48 +0200 Subject: [PATCH 09/10] tryage to make error handling cleaner --- rclrs/src/node/graph.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rclrs/src/node/graph.rs b/rclrs/src/node/graph.rs index 27781e4e7..98057a8b9 100644 --- a/rclrs/src/node/graph.rs +++ b/rclrs/src/node/graph.rs @@ -484,6 +484,10 @@ mod tests { .and_then(|val| val.parse::().map_err(|e| format!("{}", e))) .map(|id| if id != 99 { 99 } else { 98 }) .expect("Error setting domain_id"); + let context: Context = + Context::new_with_options([], InitOptions::new().with_domain_id(Some(domain_id))) + .unwrap_or_else(|error| panic!("Failed to create context: {}", error)); + let node_name = "test_publisher_names_and_types"; let node = Node::new(&context, node_name).unwrap(); // Test that the graph has no publishers From f2a553be9630ca7ab832dee84aad4d19ae18d22b Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 2 Oct 2024 12:29:15 +0200 Subject: [PATCH 10/10] tryage to make error handling cleaner --- rclrs/src/node/graph.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rclrs/src/node/graph.rs b/rclrs/src/node/graph.rs index 98057a8b9..17124b4b8 100644 --- a/rclrs/src/node/graph.rs +++ b/rclrs/src/node/graph.rs @@ -493,8 +493,7 @@ mod tests { // Test that the graph has no publishers let names_and_topics = node .get_publisher_names_and_types_by_node(node_name, "") - .unwrap(); - + .unwrap_or_else(|error| panic!("Failed to get publisher names and types: {}", error)); assert_eq!(names_and_topics.len(), 0); let num_publishers = node.count_publishers("/test").unwrap();