-
Notifications
You must be signed in to change notification settings - Fork 690
Add --world flag to publish_scene_from_text #3512
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
base: main
Are you sure you want to change the base?
Conversation
|
I was not able to find any existing documentation nor tests for this file. I am also unable to run this myself at this moment. I made this change because I was working with the ros1 version (where I needed to publish to both scene and world) and saw a bug in the ros2 version that I could fix at the same time. |
rhaschke
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.
Just a small nitpick...
moveit_ros/planning/planning_components_tools/src/publish_scene_from_text.cpp
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3512 +/- ##
==========================================
+ Coverage 46.19% 46.22% +0.03%
==========================================
Files 720 720
Lines 62714 62734 +20
Branches 7590 7595 +5
==========================================
+ Hits 28962 28990 +28
+ Misses 33585 33578 -7
+ Partials 167 166 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Robert Haschke <[email protected]>
|
Are there any additional action items? |
|
This needs approval from an official MoveIt2 maintainer. |
|
This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete. |
EzraBrooks
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.
This seems like a pretty substantial change to be making without adding any tests. Could you add at least one demonstrating the successful case?
Description
Adds a --world flag for use in conjunction with --scene flag. Publishing to the topics
DEFAULT_PLANNING_SCENE_TOPIC(pub_scene), andDEFAULT_PLANNING_SCENE_WORLD_TOPIC(pub_world_scene) are no longer mutually exclusive with this new optional flag. To be backwards compatible, the absence of both flags defaults topub_world_scenejust like it did previously.Examples:
--world--scene--scene --worldFixes #2104 where the absence of --scene failed to publish to
pub_world_scenein a timely manner.Checklist
Document API changes relevant to the user in the MIGRATION.md notesInclude a screenshot if changing a GUI