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 group_name to RobotTrajectory.msg related to MoveIt #2810 #133

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

cambel
Copy link

@cambel cambel commented Sep 20, 2021

Related to MoveIt draft PR #2810
Adding the group_name to the RobotTrajectories to keep track of them during simultaneous execution of multiple trajectories from different groups.

@v4hn
Copy link
Contributor

v4hn commented Sep 20, 2021

why don't you just collect the set of joints specified in the joint trajectory method?
I would prefer to get further away from the requirement to specify joint model groups everywhere in MoveIt and this somewhat counteracts the effort...
Also, you still have to consider overlapping JMGs during checking, so I don't see the advantage of having the string here. 😕

@felixvd
Copy link
Contributor

felixvd commented Sep 20, 2021

That might be better discussed in the original PR, but in a nutshell, we did it this way because when we check for collisions between the two trajectories here, we need to give the group_name to the PlanningScene's isPathValid in order to limit the collision checking to that group. That API continues via isStateColliding and checkCollision all the way to the CollisionRequest in collision_common.h, which does not accept a set of joints. Opening that up is definitely out of scope for this PR.

Copy link
Contributor

@felixvd felixvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking

msg/RobotTrajectory.msg Outdated Show resolved Hide resolved
@v4hn
Copy link
Contributor

v4hn commented Sep 20, 2021 via email

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.

3 participants