From c00ff00b14cdcc5a9fb487af6d335aa46fd68d44 Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 3 Jul 2024 15:57:47 +0200 Subject: [PATCH 01/10] adjustation to toolchain --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 3fd48dba0..96fbd0404 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -51,7 +51,7 @@ jobs: use-ros2-testing: ${{ matrix.ros_distribution == 'rolling' }} - name: Setup Rust - uses: dtolnay/rust-toolchain@1.74.0 + uses: dtolnay/rust-toolchain@1.78.0 with: components: clippy, rustfmt From e342cecb79e3ec96be8e0e8d44bfdd73f438e0fb Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 3 Jul 2024 16:00:12 +0200 Subject: [PATCH 02/10] fixation --- rclrs/src/wait.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 abbdce45fca609ce51b360786fe9d32eec097766 Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 3 Jul 2024 16:00:25 +0200 Subject: [PATCH 03/10] rclrs rust version change --- rclrs/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclrs/Cargo.toml b/rclrs/Cargo.toml index f489ea3f0..f1500852f 100644 --- a/rclrs/Cargo.toml +++ b/rclrs/Cargo.toml @@ -6,7 +6,7 @@ authors = ["Esteve Fernandez ", "Nikolai Morin Date: Wed, 3 Jul 2024 16:01:34 +0200 Subject: [PATCH 04/10] formatting errors --- rclrs/src/subscription/message_info.rs | 30 +++++++++++++------------- 1 file changed, 15 insertions(+), 15 deletions(-) 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. From 12a335bb502efc654f2f3cf77cb876c3d7aeb186 Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 3 Jul 2024 16:02:24 +0200 Subject: [PATCH 05/10] formatting errors --- rclrs/src/parameter.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rclrs/src/parameter.rs b/rclrs/src/parameter.rs index c8a710eeb..7ee642dfe 100644 --- a/rclrs/src/parameter.rs +++ b/rclrs/src/parameter.rs @@ -668,9 +668,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, From d466e5c2d3ad3be2ba50dd4efcf481d06faf4286 Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 3 Jul 2024 16:16:33 +0200 Subject: [PATCH 06/10] approved --- rclrs/src/parameter/range.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rclrs/src/parameter/range.rs b/rclrs/src/parameter/range.rs index 96f66d6e3..e0c738d31 100644 --- a/rclrs/src/parameter/range.rs +++ b/rclrs/src/parameter/range.rs @@ -1,3 +1,4 @@ + use crate::{ vendor::rcl_interfaces::msg::rmw::{FloatingPointRange, IntegerRange}, DeclarationError, ParameterValue, ParameterVariant, @@ -32,7 +33,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 { From 7c7b61bb8e99b6cf82a53898ea112efae8f7cc15 Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 3 Jul 2024 16:27:19 +0200 Subject: [PATCH 07/10] approved --- rclrs/src/parameter/range.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/rclrs/src/parameter/range.rs b/rclrs/src/parameter/range.rs index e0c738d31..6a46d2ff0 100644 --- a/rclrs/src/parameter/range.rs +++ b/rclrs/src/parameter/range.rs @@ -1,4 +1,3 @@ - use crate::{ vendor::rcl_interfaces::msg::rmw::{FloatingPointRange, IntegerRange}, DeclarationError, ParameterValue, ParameterVariant, From 63e182e64b45643f9b369476fbae1fc232072009 Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 3 Jul 2024 17:21:43 +0200 Subject: [PATCH 08/10] fixes --- rclrs/src/subscription.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rclrs/src/subscription.rs b/rclrs/src/subscription.rs index 36c241d19..7f3a26d01 100644 --- a/rclrs/src/subscription.rs +++ b/rclrs/src/subscription.rs @@ -274,7 +274,7 @@ where fn execute(&self) -> Result<(), RclrsError> { // Immediately evaluated closure, to handle SubscriptionTakeFailed // outside this match - match (|| { + let closure_result = || { match &mut *self.callback.lock().unwrap() { AnySubscriptionCallback::Regular(cb) => { let (msg, _) = self.take()?; @@ -302,7 +302,8 @@ where } } Ok(()) - })() { + }; + match closure_result() { Err(RclrsError::RclError { code: RclReturnCode::SubscriptionTakeFailed, .. From a6e7e38775e3c1f878a99342c1e9cdb00ff68563 Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 3 Jul 2024 17:35:37 +0200 Subject: [PATCH 09/10] added rust backtrace --- .github/workflows/rust.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 96fbd0404..121cab915 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -55,6 +55,11 @@ jobs: with: components: clippy, rustfmt + - name: Set Rust backtrace + run: | + echo "RUST_BACKTRACE=1" >> $GITHUB_ENV + + - name: Install colcon-cargo and colcon-ros-cargo run: | sudo pip3 install git+https://github.com/colcon/colcon-cargo.git From a431f7c342ca991bbd47348d5a9853523c169e8d Mon Sep 17 00:00:00 2001 From: GueLaKais Date: Wed, 3 Jul 2024 19:45:03 +0200 Subject: [PATCH 10/10] more if statements --- rclrs/src/node/graph.rs | 60 ++++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/rclrs/src/node/graph.rs b/rclrs/src/node/graph.rs index c8079b32a..be12ab72e 100644 --- a/rclrs/src/node/graph.rs +++ b/rclrs/src/node/graph.rs @@ -419,38 +419,42 @@ fn convert_names_and_types( rcl_names_and_types: rmw_names_and_types_t, ) -> HashMap> { let mut names_and_types: TopicNamesAndTypes = HashMap::new(); - - // SAFETY: Safe if the rcl_names_and_types arg has been initialized by the caller - let name_slice = unsafe { - slice::from_raw_parts( - rcl_names_and_types.names.data, - rcl_names_and_types.names.size, - ) - }; - - for (idx, name) in name_slice.iter().enumerate() { - // SAFETY: The slice contains valid C string pointers if it was populated by the caller - let name: String = unsafe { - let cstr = CStr::from_ptr(*name); - cstr.to_string_lossy().into_owned() + // Check if the names.data pointer is not null + if rcl_names_and_types.names.data.is_null() { + panic!("Invalid names.data pointer"); + } else { + // SAFETY: Safe if the rcl_names_and_types arg has been initialized by the caller + let name_slice = unsafe { + slice::from_raw_parts( + rcl_names_and_types.names.data, + rcl_names_and_types.names.size, + ) }; - // SAFETY: Safe as long as rcl_names_and_types was populated by the caller - let types: Vec = unsafe { - let p = rcl_names_and_types.types.add(idx); - slice::from_raw_parts((*p).data, (*p).size) - .iter() - .map(|s| { - let cstr = CStr::from_ptr(*s); - cstr.to_string_lossy().into_owned() - }) - .collect() - }; + for (idx, name) in name_slice.iter().enumerate() { + // SAFETY: The slice contains valid C string pointers if it was populated by the caller + let name: String = unsafe { + let cstr = CStr::from_ptr(*name); + cstr.to_string_lossy().into_owned() + }; + + // SAFETY: Safe as long as rcl_names_and_types was populated by the caller + let types: Vec = unsafe { + let p = rcl_names_and_types.types.add(idx); + slice::from_raw_parts((*p).data, (*p).size) + .iter() + .map(|s| { + let cstr = CStr::from_ptr(*s); + cstr.to_string_lossy().into_owned() + }) + .collect() + }; + + names_and_types.insert(name, types); + } - names_and_types.insert(name, types); + names_and_types } - - names_and_types } #[cfg(test)]