-
Notifications
You must be signed in to change notification settings - Fork 569
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
Make moveit_servo
listen to Octomap updates
#2601
Make moveit_servo
listen to Octomap updates
#2601
Conversation
Please target the |
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.
Thanks! Please address the comment above
Please target the main branch for development, we will backport the changes to humble for you if approved and if they don't break API.
This looks fine to me. @AndyZe is there any reason not to activate the WorldGeometryMonitor?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2601 +/- ##
==========================================
+ Coverage 50.33% 50.73% +0.41%
==========================================
Files 386 386
Lines 32086 32087 +1
==========================================
+ Hits 16147 16276 +129
+ Misses 15939 15811 -128 ☔ View full report in Codecov by Sentry. |
4c8439e
to
b2c3db7
Compare
Done; I rebased onto Thanks for the prompt review! |
Thanks for your contribution! |
Head branch was pushed to by a user without write access
@sjahr any idea what might be causing The only error relevant to my change is that no sensor is configured for the Octomap (log here) although I believe that is a non-fatal error. The only pointer I have for differences between Humble and Rolling is that in Humble, the correct line is |
I don't think that error is related to your change since it occurred on main too https://github.com/ros-planning/moveit2/actions/runs/7198483518/job/19608037907. |
* Start servo's world geometry monitor * Typo fix --------- Co-authored-by: Amal Nanavati <amaln@cs.washington.edu> (cherry picked from commit 87d5945)
* Start servo's world geometry monitor * Typo fix --------- Co-authored-by: Amal Nanavati <amaln@cs.washington.edu> (cherry picked from commit 87d5945) # Conflicts: # moveit_ros/moveit_servo/src/utils/common.cpp
Co-authored-by: Amal Nanavati <amaln@cs.washington.edu> (cherry picked from commit 87d5945) Co-authored-by: Amal Nanavati <amaln@uw.edu>
--------- Co-authored-by: Amal Nanavati <amaln@cs.washington.edu> (cherry picked from commit 87d5945)
Description
As explained in #2548, the current
moveit_servo
does not respect Octomap collisions. Responding to that, this PR makes the following one-line changes:(Instead of enabling the world geometry monitor by default, I'm happy to change it to being controlled by a boolean parameter. Maintainers, let me know if you'd prefer that.)
Testing
I have tested this code on our local robot setup in ROS2 Humble, where a
move_group
node is the primary planning scene monitor, and aservo_node
subscribes to its updates over the "monitored_planning_scene" topic. As expected, with these changes the robot treats octomap collisions as any other collision in the planning scene, and without these changes it ignores octomap collisions.Importantly, I have not tested the code in cases where the
servo_node
is the primary planning scene monitor. I imagine it should work as long as developers pass in the save octomap andsensors_3d.yaml
configuration parameters as they would to amove_group
with an octomap.Checklist