-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add support for ROS 2 (Humble) #18
Conversation
* Fix library install destination * Set include directories in CMakeLists.txt * Use generalized plugin path for app * Delete duplicated-instanciated plugin * Update ros2 to humble in build test workflow * Update build dependencies (ref: latest install-requisites-ubuntu-22.04.sh) * Do apt update/upgrade in build workflow * Use osrf/ros:humble-desktop-jammy as build workflow base image * Update ros-tooling/action-ros-ci * Add libzip-dev to install dependencies * Fix plugin/CMakeLists.txt * Clean up build test workflow * Skip test on build test workflow * Add python3-colcon-lcov-result / python3-colcon-coveragepy-result to dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your fantastic development. I leave some comments for improvement and clarification of the codes.
While we still need to work for further implementation, we now get a foot in the door of ROS 2 with your PR!
|
||
bool BodyROS2Item::store(Archive &archive) | ||
{ | ||
archive.write("body_ros_version", 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter body_ros_version
is no longer used as not restored in the next function.
|
||
jointStatePublisher | ||
= rosNode->create_publisher<sensor_msgs::msg::JointState>(getROS2Name("joint_states"), | ||
1000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just my interest: is 1000 a good choice in ROS 2 publication?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The history depth for publisher should be comprehensively judged based on the cycle of publication, the importance of the content, the environment, etc.
I don't think 1000 (which can withstand 10 seconds of communication loss) is appropriate.
The old joint state has almost no value, so if a communication breakdown occurs, the old data should be dropped and new data published.
This is the same between ROS 1 and 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much!
abstruct
I imported derverbles from choreonoid/choreonoid_ros2 and added some fixes.
With this pull-request, this repository is configured to be buildable on both ROS 1 and ROS 2.
The major differences between ROS 1 version and ROS 2 version are as follows.
ROS configuration
ROS 1
ROS 2
ros_control support
ROS 1
ros_control support is available now.
ROS 2
ros_control support is not available now.
However, we plan to support it in the future.
Build instruction for ROS 2 (Humble, Ubuntu22.04)
Note: Please ignore the error from choreonoid with catkin, when running rosdep (Fixed in choreonoid/choreonoid#36)