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

update link transforms in UnionConstraintSampler::project too #384

Conversation

v4hn
Copy link
Contributor

@v4hn v4hn commented Dec 20, 2016

This extends #186 (0119d58).

Scott Paulin found that the UnionConstraintSampler should explicitly
update the LinkTransforms of the returned samples, because one of the
other samplers could require them to be valid.

Back then, we missed that the sampler also provides a project method
that has the same limitation. So I update the transforms there too.

This should be picked to j/k.

I triggered this today by adding a JointConstraint and an OrientationConstraint
as PathConstraints to a planning request.

Update: The JointConstraint alone is enough to trigger this, according to a mail on the mailing list.

Here's a stack-trace to reveal the calling code:

    quat=..., ks=..., max_attempts=4)
    at /home/v4hn/ros/indigo/moveit/src/moveit/moveit_core/constraint_samplers/src/default_constraint_samplers.cpp:416
    reference_state=..., max_attempts=4, project=true)
    at /home/v4hn/ros/indigo/moveit/src/moveit/moveit_core/constraint_samplers/src/default_constraint_samplers.cpp:562
    max_attempts=4)
    at /home/v4hn/ros/indigo/moveit/src/moveit/moveit_core/constraint_samplers/src/default_constraint_samplers.cpp:586
    max_attempts=4)
    at /home/v4hn/ros/indigo/moveit/src/moveit/moveit_core/constraint_samplers/src/union_constraint_sampler.cpp:159
    gls=0x7fffac051690, new_goal=0x7fffa4005450)

This extends moveit#186 (0119d58).

Scott Paulin found that the UnionConstraintSampler should explicitly
update the LinkTransforms of the returned samples, because one of the
other samplers could require them to be valid.

Back then, we missed that the sampler also provides a `project` method
that has the same limitation. So I update the transforms there too.
@v4hn
Copy link
Contributor Author

v4hn commented Dec 23, 2016

@scottpaulin do you have a moment to verify this patch?

@scottpaulin
Copy link
Contributor

Looks good! I'm surprised that this can be triggered with only a joint constraint, since the joint constraint sampler does not use the robot state's link transforms.

@v4hn
Copy link
Contributor Author

v4hn commented Dec 24, 2016 via email

@davetcoleman davetcoleman merged commit f4a8db4 into moveit:indigo-devel Dec 24, 2016
@davetcoleman
Copy link
Member

Thanks!

davetcoleman pushed a commit that referenced this pull request Dec 24, 2016
This extends #186 (0119d58).

Scott Paulin found that the UnionConstraintSampler should explicitly
update the LinkTransforms of the returned samples, because one of the
other samplers could require them to be valid.

Back then, we missed that the sampler also provides a `project` method
that has the same limitation. So I update the transforms there too.
davetcoleman pushed a commit that referenced this pull request Dec 24, 2016
This extends #186 (0119d58).

Scott Paulin found that the UnionConstraintSampler should explicitly
update the LinkTransforms of the returned samples, because one of the
other samplers could require them to be valid.

Back then, we missed that the sampler also provides a `project` method
that has the same limitation. So I update the transforms there too.
davetcoleman pushed a commit to davetcoleman/moveit that referenced this pull request Jan 2, 2017
This extends moveit#186 (0119d58).

Scott Paulin found that the UnionConstraintSampler should explicitly
update the LinkTransforms of the returned samples, because one of the
other samplers could require them to be valid.

Back then, we missed that the sampler also provides a `project` method
that has the same limitation. So I update the transforms there too.
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