Use latest stable Rust CI + Fix Test Errors (#420)

* Use latest stable Rust CI Signed-off-by: Michael X. Grey <[email protected]> * Fix quotation in doc Signed-off-by: Michael X. Grey <[email protected]> * Fix serde feature for vendored messages Signed-off-by: Michael X. Grey <[email protected]> * Fix formatting of lists in docs Signed-off-by: Michael X. Grey <[email protected]> * Update minimum supported Rust version based on the currently used language features Signed-off-by: Michael X. Grey <[email protected]> * Conform to clippy style guide Signed-off-by: Michael X. Grey <[email protected]> * Add rust toolchain as a matrix dimension Signed-off-by: Michael X. Grey <[email protected]> * Bump declaration of minimum supported rust version Signed-off-by: Michael X. Grey <[email protected]> * Limit the scheduled runs to once a week Signed-off-by: Michael X. Grey <[email protected]> * Define separate stable and minimal workflows because matrix does not work with reusable actions Signed-off-by: Michael X. Grey <[email protected]> * Reduce minimum version to 1.75 to target Noble Signed-off-by: Michael X. Grey <[email protected]> --------- Signed-off-by: Michael X. Grey <[email protected]> Signed-off-by: Michael X. Grey <[email protected]>
ros2-rust · Nov 15, 2024 · b4e975c · b4e975c
1 parent f45a66f
commit b4e975c
Show file tree

Hide file tree

Showing 8 changed files with 178 additions and 36 deletions.
diff --git a/.github/workflows/rust.yml → .github/workflows/rust-minimal.yml b/.github/workflows/rust.yml → .github/workflows/rust-minimal.yml
@@ -1,12 +1,15 @@
-name: Rust
+name: Rust Minimal
 
 on:
   push:
     branches: [ main ]
   pull_request:
     branches: [ main ]
   schedule:
-    - cron: '0 0 * * *'
+    # Run the CI at 02:22 UTC every Tuesday
+    # We pick an arbitrary time outside of most of the world's work hours
+    # to minimize the likelihood of running alongside a heavy workload.
+    - cron: '22 2 * * 2'
 
 env:
   CARGO_TERM_COLOR: always
@@ -51,7 +54,7 @@ jobs:
         use-ros2-testing: ${{ matrix.ros_distribution == 'rolling' }}
 
     - name: Setup Rust
-      uses: dtolnay/rust-toolchain@1.74.0
+      uses: dtolnay/rust-toolchain@1.75
       with:
         components: clippy, rustfmt
 

diff --git a/.github/workflows/rust-stable.yml b/.github/workflows/rust-stable.yml
@@ -0,0 +1,130 @@
+name: Rust Stable
+
+on:
+  push:
+    branches: [ main ]
+  pull_request:
+    branches: [ main ]
+  schedule:
+    # Run the CI at 02:22 UTC every Tuesday
+    # We pick an arbitrary time outside of most of the world's work hours
+    # to minimize the likelihood of running alongside a heavy workload.
+    - cron: '22 2 * * 2'
+
+env:
+  CARGO_TERM_COLOR: always
+
+jobs:
+  build:
+    strategy:
+      matrix:
+        ros_distribution:
+          - humble
+          - iron
+          - rolling
+        include:
+          # Humble Hawksbill (May 2022 - May 2027)
+          - docker_image: rostooling/setup-ros-docker:ubuntu-jammy-ros-humble-ros-base-latest
+            ros_distribution: humble
+            ros_version: 2
+          # Iron Irwini (May 2023 - November 2024)
+          - docker_image: rostooling/setup-ros-docker:ubuntu-jammy-ros-iron-ros-base-latest
+            ros_distribution: iron
+            ros_version: 2
+          # Rolling Ridley  (June 2020 - Present)
+          - docker_image: rostooling/setup-ros-docker:ubuntu-jammy-ros-rolling-ros-base-latest
+            ros_distribution: rolling
+            ros_version: 2
+    runs-on: ubuntu-latest
+    continue-on-error: ${{ matrix.ros_distribution == 'rolling' }}
+    container:
+      image: ${{ matrix.docker_image }}
+    steps:
+    - uses: actions/checkout@v4
+
+    - name: Search packages in this repository
+      id: list_packages
+      run: |
+        echo ::set-output name=package_list::$(colcon list --names-only)
+
+    - name: Setup ROS environment
+      uses: ros-tooling/[email protected]
+      with:
+        required-ros-distributions: ${{ matrix.ros_distribution }}
+        use-ros2-testing: ${{ matrix.ros_distribution == 'rolling' }}
+
+    - name: Setup Rust
+      uses: dtolnay/rust-toolchain@stable
+      with:
+        components: clippy, rustfmt
+
+    - name: Install colcon-cargo and colcon-ros-cargo
+      run: |
+        sudo pip3 install git+https://github.com/colcon/colcon-cargo.git
+        sudo pip3 install git+https://github.com/colcon/colcon-ros-cargo.git
+
+    - name: Check formatting of Rust packages
+      run: |
+        for path in $(colcon list | awk '$3 == "(ament_cargo)" { print $2 }'); do
+        cd $path
+        rustup toolchain install nightly
+        cargo +nightly fmt -- --check
+        cd -
+        done
+
+    - name: Install cargo-ament-build
+      run: |
+        cargo install --debug cargo-ament-build
+
+    - name: Build and test
+      id: build
+      uses: ros-tooling/[email protected]
+      with:
+        package-name: ${{ steps.list_packages.outputs.package_list }}
+        target-ros2-distro: ${{ matrix.ros_distribution }}
+        vcs-repo-file-url: ros2_rust_${{ matrix.ros_distribution }}.repos
+
+    - name: Run clippy on Rust packages
+      run: |
+        cd ${{ steps.build.outputs.ros-workspace-directory-name }}
+        . /opt/ros/${{ matrix.ros_distribution }}/setup.sh
+        for path in $(colcon list | awk '$3 == "(ament_cargo)" { print $2 }'); do
+        cd $path
+        echo "Running clippy in $path"
+        # Run clippy for all features except generate_docs (needed for docs.rs)
+        if [ "$(basename $path)" = "rclrs" ]; then
+          cargo clippy --all-targets -F default,dyn_msg -- -D warnings
+        else
+          cargo clippy --all-targets --all-features -- -D warnings
+        fi
+        cd -
+        done
+
+    - name: Run cargo test on Rust packages
+      run: |
+        cd ${{ steps.build.outputs.ros-workspace-directory-name }}
+        . install/setup.sh
+        for path in $(colcon list | awk '$3 == "(ament_cargo)" && $1 != "examples_rclrs_minimal_pub_sub" && $1 != "examples_rclrs_minimal_client_service" && $1 != "rust_pubsub" { print $2 }'); do
+        cd $path
+        echo "Running cargo test in $path"
+        # Run cargo test for all features except generate_docs (needed for docs.rs)
+        if [ "$(basename $path)" = "rclrs" ]; then
+          cargo test -F default,dyn_msg
+        elif [ "$(basename $path)" = "rosidl_runtime_rs" ]; then
+          cargo test -F default
+        else
+          cargo test --all-features
+        fi
+        cd -
+        done
+
+    - name: Rustdoc check
+      run: |
+        cd ${{ steps.build.outputs.ros-workspace-directory-name }}
+        . /opt/ros/${{ matrix.ros_distribution }}/setup.sh
+        for path in $(colcon list | awk '$3 == "(ament_cargo)" && $1 != "examples_rclrs_minimal_pub_sub" && $1 != "examples_rclrs_minimal_client_service" && $1 != "rust_pubsub" { print $2 }'); do
+        cd $path
+        echo "Running rustdoc check in $path"
+        cargo rustdoc -- -D warnings
+        cd -
+        done
diff --git a/rclrs/Cargo.toml b/rclrs/Cargo.toml
@@ -6,7 +6,7 @@ authors = ["Esteve Fernandez <[email protected]>", "Nikolai Morin <nnmmgit@gmail
 edition = "2021"
 license = "Apache-2.0"
 description = "A ROS 2 client library for developing robotics applications in Rust"
-rust-version = "1.63"
+rust-version = "1.75"
 
 [lib]
 path = "src/lib.rs"
@@ -29,6 +29,10 @@ libloading = { version = "0.8", optional = true }
 # Needed for the Message trait, among others
 rosidl_runtime_rs = "0.4"
 
+# Needed for serliazation and deserialization of vendored messages
+serde = { version = "1", optional = true, features = ["derive"] }
+serde-big-array = { version = "0.5.1", optional = true }
+
 [dev-dependencies]
 # Needed for e.g. writing yaml files in tests
 tempfile = "3.3.0"
@@ -46,6 +50,7 @@ cfg-if = "1.0.0"
 [features]
 default = []
 dyn_msg = ["ament_rs", "libloading"]
+serde = ["dep:serde", "dep:serde-big-array", "rosidl_runtime_rs/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"]

diff --git a/rclrs/src/parameter.rs b/rclrs/src/parameter.rs
@@ -25,18 +25,18 @@ use std::{
 // Among the most relevant ones:
 //
 // * Parameter declaration returns an object which will be the main accessor to the parameter,
-// providing getters and, except for read only parameters, setters. Object destruction will
-// undeclare the parameter.
+//   providing getters and, except for read only parameters, setters. Object destruction will
+//   undeclare the parameter.
 // * Declaration uses a builder pattern to specify ranges, description, human readable constraints
-// instead of an ParameterDescriptor argument.
+//   instead of an ParameterDescriptor argument.
 // * Parameters properties of read only and dynamic are embedded in their type rather than being a
-// boolean parameter.
+//   boolean parameter.
 // * There are no runtime exceptions for common cases such as undeclared parameter, already
-// declared, or uninitialized.
+//   declared, or uninitialized.
 // * There is no "parameter not set" type, users can instead decide to have a `Mandatory` parameter
-// that must always have a value or `Optional` parameter that can be unset.
+//   that must always have a value or `Optional` parameter that can be unset.
 // * Explicit API for access to undeclared parameters by having a
-// `node.use_undeclared_parameters()` API that allows access to all parameters.
+//   `node.use_undeclared_parameters()` API that allows access to all parameters.
 
 #[derive(Clone, Debug)]
 struct ParameterOptionsStorage {
@@ -701,9 +701,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<T: ParameterVariant>(
         &self,

diff --git 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.rs b/rclrs/src/subscription.rs
@@ -272,9 +272,7 @@ where
     }
 
     fn execute(&self) -> Result<(), RclrsError> {
-        // Immediately evaluated closure, to handle SubscriptionTakeFailed
-        // outside this match
-        match (|| {
+        let evaluate = || {
             match &mut *self.callback.lock().unwrap() {
                 AnySubscriptionCallback::Regular(cb) => {
                     let (msg, _) = self.take()?;
@@ -302,7 +300,11 @@ where
                 }
             }
             Ok(())
-        })() {
+        };
+
+        // Immediately evaluated closure, to handle SubscriptionTakeFailed
+        // outside this match
+        match evaluate() {
             Err(RclrsError::RclError {
                 code: RclReturnCode::SubscriptionTakeFailed,
                 ..

diff --git a/rclrs/src/subscription/message_info.rs b/rclrs/src/subscription/message_info.rs
@@ -4,26 +4,28 @@ use crate::rcl_bindings::*;
 
 /// An identifier for a publisher in the local context.
 ///
-/// To quote the `rmw` documentation:
+/// To quote the [`rmw` documentation][1]:
 ///
 /// > 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.
+///
+/// [1]: https://docs.ros.org/en/rolling/p/rmw/generated/structrmw__message__info__s.html#_CPPv4N18rmw_message_info_s13publisher_gidE
 #[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
@@ -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.
     ///