Skip to content

Fix ROS 2 controller bounds and shutdown - #4

Open
wee733 wants to merge 1 commit into
ARXroboticsX:masterfrom
wee733:fix/ros2-controller-bounds-shutdown
Open

Fix ROS 2 controller bounds and shutdown#4
wee733 wants to merge 1 commit into
ARXroboticsX:masterfrom
wee733:fix/ros2-controller-bounds-shutdown

Conversation

@wee733

@wee733 wee733 commented Jul 11, 2026

Copy link
Copy Markdown

Summary

  • Fix out-of-bounds writes when publishing the seven-element joint position, velocity, and current arrays.
  • Bound copies by both the ROS message array size and the SDK vector size.
  • Avoid releasing the hardware interface from a global shutdown callback while the 1 kHz state timer may still be active.

Problem

RobotStatus.msg defines seven-element joint arrays, but the controller loops previously used i <= 7, which writes index 7 and invokes undefined behavior.

The controller also registered an on_shutdown callback that immediately reset interfaces_ptr_. During a Ctrl+C shutdown this could race with PubState(), which accesses the same pointer every millisecond. On the tested system, the motor and CAN cleanup ran, but the process then exited with code -11.

Relying on normal node/member destruction stops executor callbacks before releasing the hardware interface. After this change, the receiver thread and CAN socket terminate cleanly and the process exits normally.

Validation

  • Ubuntu 24.04, ROS 2 Jazzy, x86_64
  • Real ARX R5 connected through can1
  • colcon build --symlink-install --packages-select arx_r5_controller
  • /arm_status received at approximately 1000 Hz
  • SocketCAN reported zero bus errors and dropped frames
  • Repeated launch and Ctrl+C shutdown completed cleanly

The official documentation targets ROS 2 Humble; maintainers should also verify the lifecycle change on Humble.

Signed-off-by: 李博钊 <18622332179@163.com>
Copilot AI review requested due to automatic review settings July 11, 2026 21:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes undefined behavior in the ROS 2 R5 controller’s status publishing by preventing out-of-bounds writes to the 7-element joint arrays, and it removes a shutdown callback that could race with the 1 kHz publish timer and cause crashes during Ctrl+C shutdown.

Changes:

  • Replace incorrect joint array loops (previously iterating with i <= 7) with bounds-checked loops using both the ROS message array size and the SDK vector size.
  • Remove rclcpp::on_shutdown(...) callback that reset interfaces_ptr_, avoiding a potential race with timer callbacks during shutdown.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants