search

moveit / moveit_ros

THIS REPO HAS MOVED TO https://github.com/ros-planning/moveit
69 stars 118 forks source link

add ApplyPlanningSceneService capability #686

Closed v4hn closed 8 years ago

v4hn commented 8 years ago

See https://groups.google.com/forum/#!topic/moveit-users/D7MWZEUSKLc for a longer discussion on why this makes sense to have around.

The only thing I'm unsure about is whether the service should be ~-namespaced or not. There might be multiple monitored scenes around, so I moved it to the namespace. On the other hand, the ClearOctomap service is not namespaced...

v4hn commented 8 years ago

This obviously depends on https://github.com/ros-planning/moveit_msgs/pull/21

v4hn commented 8 years ago

@davetcoleman: now this request also spans 4 repos :-) Full +1 to merge most moveit repositories...

I do care quite a lot about a compatible ABI within ros distros until we add sonames to our libraries to explicitly mark updates as ABI-breaking. As @mikeferguson pointed out last week it's pretty horrible to debug random segfaults due to ABI changes without sonames. Last year I spent more than 12 hours debugging one of these issues in the PCL, although it was a particularly horrible one.

Any comments on the question of namespacing?

davetcoleman commented 8 years ago

i don't have an opinion on the namespacing, other than we should do it as the other services have been done. i don't know of any current uses cases needing more than one planning scene

v4hn commented 8 years ago

Ok, I renamed all the result variables, added the comment you proposed, and decided to move the service away from ~. Most of the other services don't use it so no reason to start with it for now.

davetcoleman commented 8 years ago

I'll merge these as soon as @mikeferguson gives me a +1 on the moveit_msgs change. Until then, could you add a section to the documentation here mentioning this new way to update the PS? It doesn't need to be much.

mikeferguson commented 8 years ago

@davetcoleman -- we need to merge the moveit_msgs and then release it as Debs so that this can properly build, else the CI will be broken

davetcoleman commented 8 years ago

just restarted build after merging msgs and core