Make ROS 2 RobotInterface joint-state waiting safe and fresh#743
Merged
Conversation
iory
force-pushed
the
improve-ros2-joint-state-waiting
branch
3 times, most recently
from
f3e3cba to
f972a69
Compare
wait_until_update_all_joints, update_robot_state, and wait_interpolation in skrobot.interfaces.ros2.base previously had three sharp edges that already had matching fixes on the ROS 1 side: - wait_until_update_all_joints had no timeout and no diagnostic, so it waited forever for any URDF joint that was never published (e.g. gripper finger joints when only an arm controller is up). - update_robot_state did not return a usable bool, so callers could not distinguish no joint_states yet from updated successfully. - wait_interpolation returned as soon as the action result arrived, but the final joint_state_broadcaster sample lands ~one cycle later, so an immediate update_robot_state read a frame from before motion completion. This patch backports the ROS 1 base improvements: - _received_joint_names tracks names actually seen on the joint_states topic so the wait logic only waits for joints being published. - robot_state slots are pre-allocated from robot.joint_list, so late publishers (e.g. a gripper node started after controllers) get their slots ready in advance. - wait_until_update_all_joints accepts a timeout, returns bool, and populates _timeout_reason / _not_updated_joints for diagnosis. - update_robot_state returns True/False matching the ROS 1 contract. - wait_interpolation now waits for a fresh joint_state after the action result by default (wait_for_state_update=True, state_update_timeout=1.0), eliminating the post-motion stale-read race. - ROS2RobotInterfaceBase.__init__ gains opt-in wait_for_joint_states / joint_states_timeout to fail fast on misconfigured topics. 10 unit tests cover the new bookkeeping, timeout behaviour, bool returns, and the constructor wait option. A new ros2-tests job is added to .github/workflows/test.yml that installs ROS 2 jazzy (Ubuntu 24.04) via ros-tooling/setup-ros and runs the new test_base.py with pytest --timeout=60 so that any future regression of the wait_until_update_all_joints hang surfaces as a per-test timeout instead of a stuck CI run.
iory
force-pushed
the
improve-ros2-joint-state-waiting
branch
from
f972a69 to
7a8de4a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
wait_until_update_all_joints, update_robot_state, and wait_interpolation in skrobot.interfaces.ros2.base previously had three sharp edges that already had matching fixes on the ROS 1 side:
This patch backports the ROS 1 base improvements:
10 unit tests cover the new bookkeeping, timeout behaviour, bool returns, and the constructor wait option.
A new ros2-tests job is added to .github/workflows/test.yml that installs ROS 2 humble (Ubuntu 22.04) and jazzy (Ubuntu 24.04) via ros-tooling/setup-ros and runs the new test_base.py with pytest --timeout=60 so that any future regression of the wait_until_update_all_joints hang surfaces as a per-test timeout instead of a stuck CI run.