diff --git a/.github/workflows/rust.yml b/.github/workflows/rust-minimal.yml similarity index 94% rename from .github/workflows/rust.yml rename to .github/workflows/rust-minimal.yml index 3fd48dba0..9b1a5bb49 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust-minimal.yml @@ -1,4 +1,4 @@ -name: Rust +name: Rust Minimal on: push: @@ -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 @@ -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 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 diff --git a/rclrs/Cargo.toml b/rclrs/Cargo.toml index f489ea3f0..fe17cc990 100644 --- a/rclrs/Cargo.toml +++ b/rclrs/Cargo.toml @@ -6,7 +6,7 @@ authors = ["Esteve Fernandez ", "Nikolai Morin 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.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, .. 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. 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. ///