-
Notifications
You must be signed in to change notification settings - Fork 424
Implement new PoseBroadcaster controller #1311
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
Implement new PoseBroadcaster controller #1311
Conversation
Co-authored-by: Felix Exner (fexner) <felix_mauch@web.de>
Co-authored-by: Felix Exner (fexner) <felix_mauch@web.de>
fmauch
left a comment
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.
Looks good to me! As @RobertWilbrandt wrote, we tested this in the UR driver already.
|
Would you see use of this being a chainable controller at the same time? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1311 +/- ##
==========================================
+ Coverage 80.37% 80.58% +0.21%
==========================================
Files 105 109 +4
Lines 9387 9551 +164
Branches 824 830 +6
==========================================
+ Hits 7545 7697 +152
- Misses 1570 1578 +8
- Partials 272 276 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
I don't have much experience with chaining controllers, but this sounds like a good extension. I can work on that and extend this PR. |
|
@bmagyar This should probably be backported? The semantic component has already been backported. |
|
@Mergifyio backport humble iron |
✅ Backports have been created
|
(cherry picked from commit 7c89c17) # Conflicts: # doc/controllers_index.rst
(cherry picked from commit 7c89c17)
* Implement new PoseBroadcaster controller (#1311) (cherry picked from commit 7c89c17) # Conflicts: # doc/controllers_index.rst * Update controllers_index.rst * Fix API in tests * Add hardware_interface_testing dependency --------- Co-authored-by: RobertWilbrandt <wilbrandt@fzi.de> Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com> Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com> Co-authored-by: Christoph Froehlich <christoph.froehlich@ait.ac.at>
This PR implements a new broadcasting controller based on the
PoseSensorsemantic component introduced in ros-controls/ros2_control#1775.As described in the
PoseSensorPR, my primary use-case for this is the publishing of tcp poses from robot arms. This can be more accurate than calculating forward kinematics, as robot calibrations etc. can be considered by the robot. Settings on the robot side concerning tool offsets etc. can also be included by this (at least for UR and kuka RSI). The ROS 1ur_robot_driverused to publish this transform directly from within the hardware interface, but in ROS 2 there is not yet a standard way for providing this information. A PR for the UR ROS 2 driver already exists at UniversalRobots/Universal_Robots_ROS2_Driver#1108.This controller publishes poses directly as
geometry_msgs::msg::PoseStampeds and as tf transforms (can be disabled).