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

Change action name path -> trajectory for better name #27

Merged
merged 1 commit into from
Aug 20, 2016

Conversation

wkentaro
Copy link
Contributor

@rhaschke rhaschke merged commit c79df9f into moveit:indigo-devel Aug 20, 2016
@rhaschke
Copy link
Contributor

Thanks @wkentaro. Cherry-picked to Jade.

@rhaschke
Copy link
Contributor

I suggest, to drop this new feature (of TrajectoryExecution action) for Indigo for the following reason:
The MoveGroupInterface tries to connect to the new action for trajectory execution. However, as existing moveit configs don't define this new capability yet, we need to issue a deprecation warning. However, this shouldn't happen for an LTS release like Indigo. I suggest to deprecate beginning with Jade. The new feature is functionally not required.
@davetcoleman @v4hn: What do you think?

@v4hn
Copy link
Contributor

v4hn commented Aug 20, 2016

@rhaschke Could you have a look at the solution I proposed in moveit/moveit#38 . Maybe something similar works for this patch? (we could even silently support the action in indigo to enable users to use the same code from indigo to kinetic).
Note though, that the change there is a medium safety issue and should be addressed in indigo.

If the new action is not functionally required in indigo to allow for proper cancelling of trajectories,
then yes, it would be ok to have it only starting in jade.

@rhaschke
Copy link
Contributor

@v4hn That's exactly, what I implemented. However, I think we shouldn't bother people with a deprecation warning in the middle of the lifetime of Indigo, should we?

@130s
Copy link
Contributor

130s commented Aug 20, 2016

ros/rosdistro#12423 I made a release request for indigo that includes this patch but closed it before it's accepted, waiting for the discussion to be settled here.

@v4hn
Copy link
Contributor

v4hn commented Aug 20, 2016

On Sat, Aug 20, 2016 at 08:11:06AM -0700, Robert Haschke wrote:

@v4hn That's exactly, what I implemented. However, I think we shouldn't bother people with a deprecation warning in the middle of the lifetime of Indigo, should we?

So why not silently fall back to the current version if the new one is not available?
We could add the deprecation warning starting with jade.
This would allow to use the feature in indigo but wouldn't bother anyone.

@rhaschke
Copy link
Contributor

So why not silently fall back to the current version if the new one is not available?

That would be an option.

We could add the deprecation warning starting with jade.

+1 That's what I suggested.

This would allow to use the feature in indigo but wouldn't bother anyone.

Agreed.

@davetcoleman
Copy link
Member

+1

@v4hn
Copy link
Contributor

v4hn commented Aug 22, 2016

So this looks like having the action spec released in indigo-devel is fine.
@130s I believe the discussion "is settled" :)

@130s
Copy link
Contributor

130s commented Aug 22, 2016

Aye.

(I'll actually have to make a new release for IJK, instead of re-opening ros/rosdistro#12423, to include #28)

@130s
Copy link
Contributor

130s commented Aug 22, 2016

New release request for 0.7.4 for Indigo ros/rosdistro#12451

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

5 participants