PSI: add apply* functions that use ApplyPlanningScene.srv #381
Conversation
v4hn
force-pushed
the
pr-kinetic-apply-in-interface
branch
from
fa60586
to
2712d25
Compare
| /** \brief Apply attached collision objects to the planning scene of the move_group node. | ||
| Other PlanningSceneMonitors will NOT receive the update unless they subscribe to move_group's monitored scene */ | ||
| bool | ||
| applyAttachedCollisionObjects(const std::vector<moveit_msgs::AttachedCollisionObject> &attached_collision_objects); |
There was a problem hiding this comment.
what do you think of abbreviating collision_objects to collision_objs for line length?
| Other PlanningSceneMonitors will NOT receive the update unless they subscribe to move_group's monitored scene */ | ||
| bool applyPlanningScene(const moveit_msgs::PlanningScene &ps); | ||
|
|
||
| /** \brief Add collision objects to the world via /planning_scene. | ||
| Make sure object.operation is set to object.ADD. */ |
There was a problem hiding this comment.
Add comment that this and the following function are the old asynchronous approach that usually require arbitrary sleep timers?
| request.scene = planning_scene; | ||
| if (!apply_planning_scene_service_.call(request, response)) | ||
| { | ||
| ROS_WARN("Failed to call ApplyPlanningScene service"); |
| @@ -235,6 +251,7 @@ class PlanningSceneInterface::PlanningSceneInterfaceImpl | |||
| private: | |||
| ros::NodeHandle node_handle_; | |||
| ros::ServiceClient planning_scene_service_; | |||
| mutable ros::ServiceClient apply_planning_scene_service_; | |||
There was a problem hiding this comment.
I know mutable is for ABI, but it seems dirty and we have sonames that will force everyone to recompile anyway. I vote to remove this.
|
On Thu, Dec 15, 2016 at 11:35:02AM -0800, Dave Coleman wrote:
what do you think of abbreviating ``collision_objects`` to ``collision_objs`` for line length?
I kept it the same as in the other functions and I would like to keep it that way.
Add comment that this and the following function are the *old* asynchronous approach that usually require arbitrary sleep timers?
Well, they are not "old" as in "obsolete". I added comments on the synchronous/asynchronous behavior.
``_NAMESPACE``
done. `_NAMED` was missing throughout the class.
I know ``mutable`` is for ABI, but it seems dirty and we have sonames that will force everyone to recompile anyway. I vote to remove this.
Actually, this is not at all required anymore, I just forgot to remove it... Thanks for pointing it out!
|
| @@ -210,6 +214,19 @@ class PlanningSceneInterface::PlanningSceneInterfaceImpl | |||
| return result; | |||
| } | |||
|
|
|||
| bool applyPlanningScene(const moveit_msgs::PlanningScene &planning_scene) const | |||
There was a problem hiding this comment.
no const here? Travis is not building
There was a problem hiding this comment.
Yes, another neglected specifier that was shadowed by the mutable above.
I adjusted it. Let's see whether it passes now.
|
Your comment in reference to Travis of "Let's see whether it passes now." worries me that this PR has not been tested on a live system at all. Is this ready to be merged? |
Added helper function to call ApplyPlanningScene service with fallback to asynchronous processing via "planning_scene" topic.
to update move_group's PlanningScene
This was only required for the proposal to change the behavior of the add/remove functions. Now that I added the apply* interfaces, this is not relevant anymore.
v4hn
force-pushed
the
pr-kinetic-apply-in-interface
branch
from
65194bf
to
1092232
Compare
Sorry to make you worry :) I just rebased the request to clean up the history. |
* Use ApplyPlanningSceneService in planning_scene_interface Added helper function to call ApplyPlanningScene service with fallback to asynchronous processing via "planning_scene" topic. * PSI: add apply* functions that use ApplyPlanningScene.srv to update move_group's PlanningScene * remove obsolete mutable declaration This was only required for the proposal to change the behavior of the add/remove functions. Now that I added the apply* interfaces, this is not relevant anymore. * PSI: point out synchronicity behavior of apply* and add/removeObjects interfaces * PSI: make all logs _NAMED
cherry-pick apply* functions in PSI (#381)
cherry-pick apply* functions in PSI (#381)
This implements my comment #128 (comment)
and supersedes #128.