Skip to content
Open
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
101 changes: 65 additions & 36 deletions .github/workflows/rust-minimal.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ on:
env:
CARGO_TERM_COLOR: always

defaults:
run:
shell: bash

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?


jobs:
build:
strategy:
Expand Down Expand Up @@ -82,6 +86,20 @@ jobs:
sudo pip3 install git+https://github.com/colcon/colcon-cargo.git
sudo pip3 install git+https://github.com/colcon/colcon-ros-cargo.git

- name: Search Rust packages in this repository
id: list_repo_rust_packages
run: |
mapfile -t package_paths < <(colcon list | awk '$3 == "(ros.ament_cargo)" { print $2 }')
if (( ${#package_paths[@]} == 0 )); then
echo "::error::No ros.ament_cargo packages found in the repository"
exit 1
fi
{
echo 'package_paths<<EOF'
printf '%s\n' "${package_paths[@]}"
echo EOF
} >> "$GITHUB_OUTPUT"

# test_msgs recently added ament_mypy as a test dependency, but rosdep
# may fail to install it when building from source on rolling
- name: Install ament_cmake_mypy for rolling
Expand All @@ -91,13 +109,16 @@ jobs:
sudo apt-get install -y ros-rolling-ament-cmake-mypy

- name: Check formatting of Rust packages
env:
PACKAGE_PATHS: ${{ steps.list_repo_rust_packages.outputs.package_paths }}
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
while IFS= read -r path; do
[ -n "$path" ] || continue
cd "$path"
cargo +nightly fmt -- --check
cd - > /dev/null
done <<< "$PACKAGE_PATHS"

- name: Build and test
id: build
Expand All @@ -107,45 +128,53 @@ jobs:
target-ros2-distro: ${{ matrix.ros_distribution }}
vcs-repo-file-url: ros2_rust_${{ matrix.ros_distribution }}.repos

- name: Run clippy on Rust packages
- name: Search target Rust packages in the built workspace
id: list_workspace_rust_packages
env:
PACKAGE_NAMES: ${{ steps.list_packages.outputs.package_list }}
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 use_ros_shim (needed for docs.rs)
if [ "$(basename $path)" = "rclrs" ]; then
cargo clippy --no-deps --all-targets -F default -- -D warnings
else
cargo clippy --no-deps --all-targets --all-features -- -D warnings
mapfile -t package_names <<< "$PACKAGE_NAMES"
mapfile -t package_paths < <(colcon list --packages-select "${package_names[@]}" | awk '$3 == "(ros.ament_cargo)" { print $2 }')
if (( ${#package_paths[@]} == 0 )); then
echo "::error::No target ros.ament_cargo packages found in the built workspace"
exit 1
fi
cd -
done
{
echo 'package_paths<<EOF'
printf '%s\n' "${package_paths[@]}"
echo EOF
} >> "$GITHUB_OUTPUT"

- name: Run cargo test on Rust packages
- name: Run clippy on Rust packages
env:
PACKAGE_PATHS: ${{ steps.list_workspace_rust_packages.outputs.package_paths }}
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 use_ros_shim (needed for docs.rs)
if [ "$(basename $path)" = "rclrs" ]; then
cargo test -F default,serde
else
cargo test --all-features
fi
cd -
done
while IFS= read -r path; do
[ -n "$path" ] || continue
cd "$path"
echo "Running clippy in $path"
# Run clippy for all features except use_ros_shim (needed for docs.rs)
if [ "$(basename "$path")" = "rclrs" ]; then
cargo clippy --no-deps --all-targets -F default -- -D warnings
else
cargo clippy --no-deps --all-targets --all-features -- -D warnings
fi
cd - > /dev/null
done <<< "$PACKAGE_PATHS"

- name: Rustdoc check
env:
PACKAGE_PATHS: ${{ steps.list_workspace_rust_packages.outputs.package_paths }}
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
. install/setup.sh
while IFS= read -r path; do
[ -n "$path" ] || continue
cd "$path"
echo "Running rustdoc check in $path"
cargo rustdoc -- -D warnings
cd - > /dev/null
done <<< "$PACKAGE_PATHS"
101 changes: 65 additions & 36 deletions .github/workflows/rust-stable.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ on:
env:
CARGO_TERM_COLOR: always

defaults:
run:
shell: bash

jobs:
build:
strategy:
Expand Down Expand Up @@ -82,6 +86,20 @@ jobs:
sudo pip3 install git+https://github.com/colcon/colcon-cargo.git
sudo pip3 install git+https://github.com/colcon/colcon-ros-cargo.git

- name: Search Rust packages in this repository
id: list_repo_rust_packages
run: |
mapfile -t package_paths < <(colcon list | awk '$3 == "(ros.ament_cargo)" { print $2 }')
if (( ${#package_paths[@]} == 0 )); then
echo "::error::No ros.ament_cargo packages found in the repository"
exit 1
fi
{
echo 'package_paths<<EOF'
printf '%s\n' "${package_paths[@]}"
echo EOF
} >> "$GITHUB_OUTPUT"

# test_msgs recently added ament_mypy as a test dependency, but rosdep
# may fail to install it when building from source on rolling
- name: Install ament_cmake_mypy for rolling
Expand All @@ -91,13 +109,16 @@ jobs:
sudo apt-get install -y ros-rolling-ament-cmake-mypy

- name: Check formatting of Rust packages
env:
PACKAGE_PATHS: ${{ steps.list_repo_rust_packages.outputs.package_paths }}
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
while IFS= read -r path; do
[ -n "$path" ] || continue
cd "$path"
cargo +nightly fmt -- --check
cd - > /dev/null
done <<< "$PACKAGE_PATHS"

- name: Build and test
id: build
Expand All @@ -107,45 +128,53 @@ jobs:
target-ros2-distro: ${{ matrix.ros_distribution }}
vcs-repo-file-url: ros2_rust_${{ matrix.ros_distribution }}.repos

- name: Run clippy on Rust packages
- name: Search target Rust packages in the built workspace
id: list_workspace_rust_packages
env:
PACKAGE_NAMES: ${{ steps.list_packages.outputs.package_list }}
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 use_ros_shim (needed for docs.rs)
if [ "$(basename $path)" = "rclrs" ]; then
cargo clippy --no-deps --all-targets -F default -- -D warnings
else
cargo clippy --no-deps --all-targets --all-features -- -D warnings
mapfile -t package_names <<< "$PACKAGE_NAMES"
mapfile -t package_paths < <(colcon list --packages-select "${package_names[@]}" | awk '$3 == "(ros.ament_cargo)" { print $2 }')
if (( ${#package_paths[@]} == 0 )); then
echo "::error::No target ros.ament_cargo packages found in the built workspace"
exit 1
fi
cd -
done
{
echo 'package_paths<<EOF'
printf '%s\n' "${package_paths[@]}"
echo EOF
} >> "$GITHUB_OUTPUT"

- name: Run cargo test on Rust packages
- name: Run clippy on Rust packages
env:
PACKAGE_PATHS: ${{ steps.list_workspace_rust_packages.outputs.package_paths }}
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 use_ros_shim (needed for docs.rs)
if [ "$(basename $path)" = "rclrs" ]; then
cargo test -F default,serde
else
cargo test --all-features
fi
cd -
done
while IFS= read -r path; do
[ -n "$path" ] || continue
cd "$path"
echo "Running clippy in $path"
# Run clippy for all features except use_ros_shim (needed for docs.rs)
if [ "$(basename "$path")" = "rclrs" ]; then
cargo clippy --no-deps --all-targets -F default -- -D warnings
else
cargo clippy --no-deps --all-targets --all-features -- -D warnings
fi
cd - > /dev/null
done <<< "$PACKAGE_PATHS"

- name: Rustdoc check
env:
PACKAGE_PATHS: ${{ steps.list_workspace_rust_packages.outputs.package_paths }}
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
. install/setup.sh
while IFS= read -r path; do
[ -n "$path" ] || continue
cd "$path"
echo "Running rustdoc check in $path"
cargo rustdoc -- -D warnings
cd - > /dev/null
done <<< "$PACKAGE_PATHS"
13 changes: 10 additions & 3 deletions .github/workflows/rust-win.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,12 @@ jobs:
run: |
call C:\pixi_ws\ros2-windows\setup.bat
cd C:\workspace
set /a rust_package_count=0
for /f "tokens=1,2,3" %%A in ('pixi run --manifest-path C:\pixi_ws\pixi.toml colcon list') do (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the logs, cargo test fails but the step still succeeds and the failure is silently ignored

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 71537a4: the Windows batch loop now checks errorlevel immediately after each cargo test, returns a failing step on any package failure, and also fails if the selector matches zero target packages.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in : the Windows batch loop now checks immediately after each , returns a failing step on any package failure, and also fails if the selector matches zero target packages.

if "%%C"=="(ament_cargo)" (
if "%%C"=="(ros.ament_cargo)" (
if /I not "%%A"=="examples_rclrs_minimal_pub_sub" if /I not "%%A"=="examples_rclrs_minimal_client_service" if /I not "%%A"=="rust_pubsub" (
cd %%B
set /a rust_package_count+=1
cd /d %%B
echo Running cargo test in %%B
if /I "%%~nxB"=="rclrs" (
cargo test -F default
Expand All @@ -87,9 +89,14 @@ jobs:
) else (
cargo test --all-features
)
cd ..
if errorlevel 1 exit /b 1
cd /d C:\workspace
)
)
)
if %rust_package_count% EQU 0 (
echo No target ros.ament_cargo packages found 1>&2
exit /b 1
)
shell: cmd
working-directory: C:\workspace
6 changes: 6 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,10 @@
members = [
"rclrs",
]
# action-ros-ci builds the colcon workspace as `ros_ws/` inside this checkout. Without this
# exclusion every package under it inherits this manifest as its workspace root and cargo
# refuses to build them ("current package believes it's in a workspace when it's not").
exclude = [
"ros_ws",
]
resolver = "2"
2 changes: 1 addition & 1 deletion docs/building.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ colcon list
without errors and see a line like this in the output:

```
rclrs src/ros2_rust/rclrs (ament_cargo)
rclrs src/ros2_rust/rclrs (ros.ament_cargo)
```

The build type `ament_cargo` means that the `colcon-ros-cargo` plugin works as expected.
Expand Down
10 changes: 5 additions & 5 deletions rclrs/src/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,12 @@ impl CancelResponseCode {

impl From<i8> for CancelResponseCode {
fn from(value: i8) -> Self {
if 0 <= value && value <= 3 {
if (0..=3).contains(&value) {
unsafe {
// SAFETY: We have already ensured that the integer value is
// within the acceptable range for the enum, so transmuting is
// safe.
return std::mem::transmute(value);
return std::mem::transmute::<i8, CancelResponseCode>(value);
}
}

Expand Down Expand Up @@ -205,12 +205,12 @@ impl GoalStatusCode {

impl From<i8> for GoalStatusCode {
fn from(value: i8) -> Self {
if 0 <= value && value <= 6 {
if (0..=6).contains(&value) {
unsafe {
// SAFETY: We have already ensured that the integer value is
// within the acceptable range for the enum, so transmuting is
// safe.
return std::mem::transmute(value);
return std::mem::transmute::<i8, GoalStatusCode>(value);
}
}

Expand Down Expand Up @@ -579,7 +579,7 @@ mod tests {
let mut current = 1;

for _ in 0..goal_order {
if let Err(_) = sender.send(current) {
if sender.send(current).is_err() {
// The action has been cancelled early, so just drop this thread.
return;
}
Expand Down
Loading
Loading