Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use latest stable Rust CI + Fix Test Errors #420

Merged
merged 13 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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

Expand Down
130 changes: 130 additions & 0 deletions .github/workflows/rust-stable.yml
Original file line number Diff line number Diff line change
@@ -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
7 changes: 6 additions & 1 deletion rclrs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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"]
Expand Down
18 changes: 9 additions & 9 deletions rclrs/src/parameter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<T: ParameterVariant>(
&self,
Expand Down
2 changes: 1 addition & 1 deletion rclrs/src/parameter/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 6 additions & 4 deletions rclrs/src/subscription.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down Expand Up @@ -302,7 +300,11 @@ where
}
}
Ok(())
})() {
};

// Immediately evaluated closure, to handle SubscriptionTakeFailed
// outside this match
match evaluate() {
Err(RclrsError::RclError {
code: RclReturnCode::SubscriptionTakeFailed,
..
Expand Down
36 changes: 19 additions & 17 deletions rclrs/src/subscription/message_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion rclrs/src/wait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down
Loading