From eb4cffe4e421b1100f812732a28ec935a240a59d Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Thu, 3 Oct 2024 14:04:05 +0000 Subject: [PATCH 01/11] Use latest stable Rust CI Signed-off-by: Michael X. Grey --- .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..afc0eda62 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@stable with: components: clippy, rustfmt From 6abc295ef73247077e8e4953c3c3b1def1ba99d2 Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Mon, 7 Oct 2024 16:07:55 +0800 Subject: [PATCH 02/11] Fix quotation in doc Signed-off-by: Michael X. Grey --- rclrs/src/subscription/message_info.rs | 36 ++++++++++++++------------ 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/rclrs/src/subscription/message_info.rs b/rclrs/src/subscription/message_info.rs index 010bf28ec..1adecd3ec 100644 --- 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. From 8978d61ac6069445a526e0f6a091bb400e223570 Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Mon, 7 Oct 2024 16:11:10 +0800 Subject: [PATCH 03/11] Fix serde feature for vendored messages Signed-off-by: Michael X. Grey --- rclrs/Cargo.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rclrs/Cargo.toml b/rclrs/Cargo.toml index f489ea3f0..6cfd453f0 100644 --- a/rclrs/Cargo.toml +++ b/rclrs/Cargo.toml @@ -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"] From 8e18f1e927d0a06e354b4f3ece2b75fb2eb65583 Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Mon, 7 Oct 2024 16:14:14 +0800 Subject: [PATCH 04/11] Fix formatting of lists in docs Signed-off-by: Michael X. Grey --- rclrs/src/parameter.rs | 18 +++++++++--------- rclrs/src/parameter/range.rs | 2 +- rclrs/src/wait.rs | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/rclrs/src/parameter.rs b/rclrs/src/parameter.rs index c8a710eeb..200b42433 100644 --- 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 { @@ -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, 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/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 20e0524ad7a902d2dd6f1c5b94739322feadbf59 Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Mon, 7 Oct 2024 16:16:36 +0800 Subject: [PATCH 05/11] Update minimum supported Rust version based on the currently used language features Signed-off-by: Michael X. Grey --- rclrs/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclrs/Cargo.toml b/rclrs/Cargo.toml index 6cfd453f0..f61356888 100644 --- a/rclrs/Cargo.toml +++ b/rclrs/Cargo.toml @@ -6,7 +6,7 @@ authors = ["Esteve Fernandez ", "Nikolai Morin Date: Mon, 7 Oct 2024 16:25:49 +0800 Subject: [PATCH 06/11] Conform to clippy style guide Signed-off-by: Michael X. Grey --- rclrs/src/subscription.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/rclrs/src/subscription.rs b/rclrs/src/subscription.rs index 36c241d19..fbd518c21 100644 --- 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, .. From aeef0924a2c811f2c1a6b0805047e3b8af29d7c5 Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Tue, 22 Oct 2024 13:09:49 +0000 Subject: [PATCH 07/11] Add rust toolchain as a matrix dimension Signed-off-by: Michael X. Grey --- .github/workflows/rust.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index afc0eda62..86a2c7948 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -19,6 +19,9 @@ jobs: - humble - iron - rolling + rust_toolchain: + - stable + - v1_78 include: # Humble Hawksbill (May 2022 - May 2027) - docker_image: rostooling/setup-ros-docker:ubuntu-jammy-ros-humble-ros-base-latest @@ -32,6 +35,10 @@ jobs: - docker_image: rostooling/setup-ros-docker:ubuntu-jammy-ros-rolling-ros-base-latest ros_distribution: rolling ros_version: 2 + - rust_toolchain: stable + rust_toolchain_action: dtolnay/rust-toolchain@stable + - rust_toolchain: v1_78 + rust_toolchain_action: dtolnay/rust-toolchain@1.78 runs-on: ubuntu-latest continue-on-error: ${{ matrix.ros_distribution == 'rolling' }} container: @@ -51,7 +58,7 @@ jobs: use-ros2-testing: ${{ matrix.ros_distribution == 'rolling' }} - name: Setup Rust - uses: dtolnay/rust-toolchain@stable + uses: ${{ matrix.rust_toolchain_action }} with: components: clippy, rustfmt From 6e472fceb9bfa1fa34c4c38e5b32b6ec6e502b33 Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Tue, 22 Oct 2024 13:26:59 +0000 Subject: [PATCH 08/11] Bump declaration of minimum supported rust version Signed-off-by: Michael X. Grey --- rclrs/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclrs/Cargo.toml b/rclrs/Cargo.toml index f61356888..6370a192f 100644 --- a/rclrs/Cargo.toml +++ b/rclrs/Cargo.toml @@ -6,7 +6,7 @@ authors = ["Esteve Fernandez ", "Nikolai Morin Date: Tue, 22 Oct 2024 13:37:35 +0000 Subject: [PATCH 09/11] Limit the scheduled runs to once a week Signed-off-by: Michael X. Grey --- .github/workflows/rust.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 86a2c7948..78b04d538 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -6,7 +6,10 @@ on: 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 From e7ff2da19603eff011c2c0e95c8d8f4f13c15521 Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Tue, 22 Oct 2024 13:53:48 +0000 Subject: [PATCH 10/11] Define separate stable and minimal workflows because matrix does not work with reusable actions Signed-off-by: Michael X. Grey --- .../workflows/{rust.yml => rust-minimal.yml} | 11 +- .github/workflows/rust-stable.yml | 130 ++++++++++++++++++ 2 files changed, 132 insertions(+), 9 deletions(-) rename .github/workflows/{rust.yml => rust-minimal.yml} (93%) create mode 100644 .github/workflows/rust-stable.yml diff --git a/.github/workflows/rust.yml b/.github/workflows/rust-minimal.yml similarity index 93% rename from .github/workflows/rust.yml rename to .github/workflows/rust-minimal.yml index 78b04d538..dcf37a080 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust-minimal.yml @@ -1,4 +1,4 @@ -name: Rust +name: Rust Minimal on: push: @@ -22,9 +22,6 @@ jobs: - humble - iron - rolling - rust_toolchain: - - stable - - v1_78 include: # Humble Hawksbill (May 2022 - May 2027) - docker_image: rostooling/setup-ros-docker:ubuntu-jammy-ros-humble-ros-base-latest @@ -38,10 +35,6 @@ jobs: - docker_image: rostooling/setup-ros-docker:ubuntu-jammy-ros-rolling-ros-base-latest ros_distribution: rolling ros_version: 2 - - rust_toolchain: stable - rust_toolchain_action: dtolnay/rust-toolchain@stable - - rust_toolchain: v1_78 - rust_toolchain_action: dtolnay/rust-toolchain@1.78 runs-on: ubuntu-latest continue-on-error: ${{ matrix.ros_distribution == 'rolling' }} container: @@ -61,7 +54,7 @@ jobs: use-ros2-testing: ${{ matrix.ros_distribution == 'rolling' }} - name: Setup Rust - uses: ${{ matrix.rust_toolchain_action }} + uses: dtolnay/rust-toolchain@1.78 with: components: clippy, rustfmt diff --git a/.github/workflows/rust-stable.yml b/.github/workflows/rust-stable.yml new file mode 100644 index 000000000..6dc395496 --- /dev/null +++ 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/setup-ros@v0.7 + 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/action-ros-ci@v0.3 + 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 From 56ead26d91c76e3c631c178e3f96db22036e799c Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Mon, 28 Oct 2024 16:39:22 +0800 Subject: [PATCH 11/11] Reduce minimum version to 1.75 to target Noble Signed-off-by: Michael X. Grey --- .github/workflows/rust-minimal.yml | 2 +- rclrs/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust-minimal.yml b/.github/workflows/rust-minimal.yml index dcf37a080..9b1a5bb49 100644 --- a/.github/workflows/rust-minimal.yml +++ b/.github/workflows/rust-minimal.yml @@ -54,7 +54,7 @@ jobs: use-ros2-testing: ${{ matrix.ros_distribution == 'rolling' }} - name: Setup Rust - uses: dtolnay/rust-toolchain@1.78 + uses: dtolnay/rust-toolchain@1.75 with: components: clippy, rustfmt diff --git a/rclrs/Cargo.toml b/rclrs/Cargo.toml index 6370a192f..fe17cc990 100644 --- a/rclrs/Cargo.toml +++ b/rclrs/Cargo.toml @@ -6,7 +6,7 @@ authors = ["Esteve Fernandez ", "Nikolai Morin