Skip to content
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

add new srv ApplyPlanningScene #21

Merged
merged 2 commits into from
Jun 14, 2016

Conversation

v4hn
Copy link
Contributor

@v4hn v4hn commented Jun 13, 2016

This service takes a PlanningScene message and applies it to the monitored
scene.

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

Ideally it should include a bool success field, but it is not possible to
apply the scene and check for success without ABI changes, so leave it out for
now to get this change pushed to indigo.

With kinetic we might want to think about adding a bool success field here
and make newPlanningSceneMessage return bool instead of void.

This service takes a PlanningScene message
and applies it to the monitored scene.

Ideally it should include a `bool success` field,
but it is not possible to apply the scene and check for success
without ABI changes, so leave it out for now. To get this change
pushed to indigo.
@davetcoleman
Copy link
Member

why not go ahead and add the success field to the message and we can connect it to something other than true in the future?

i hope you'll make newPlanningSceneMessage() return true in jade/kinetic

This will be set to true in indigo,
but might return false in kinetic and upcoming
after we broke the underlying API to get that information.
@v4hn
Copy link
Contributor Author

v4hn commented Jun 14, 2016

I thought it's probably confusing to have a constantly true
success field in the action, but you're probably right.
There's no need to change the action spec later on.

I adjusted the spec accordingly.
On Mon, Jun 13, 2016 at 03:39:23PM -0700, Dave Coleman wrote:

why not go ahead and add the success field to the message and we can connect it to something other than true in the future?

i hope you'll make newPlanningSceneMessage() return true in jade/kinetic

@davetcoleman
Copy link
Member

@mikeferguson you're our ROS messages expert, think its ok to go ahead and merge this now or do I need to wait for something?

@mikeferguson
Copy link
Contributor

@davetcoleman -- it is an entirely new message, right? That is pretty much always fine, it is just changing messages that is problematic.

@davetcoleman davetcoleman merged commit 93d3ef9 into moveit:indigo-devel Jun 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants