include fixed tfs to siblings in links #912
Conversation
It does not compute *all* fixed transforms, but only those *below* the link in the kinematic tree.
If the robot structure contains y-paths made up of fixed transforms, the previous implementation could miss associations. The structure - a - b - c \ - d only added associationed fixed transforms for a and b to c, but missed d.
|
Sorry this took so long for me to look at, but LGTM. I added a test that I though I'd be able to add to this PR, but I can't; would you mind cherry-picking commit 76366f126 off my copy of this PR? Minor comment, but the |
No reason to break ABI. Added a comment instead
|
@BryceStevenWilley Thanks for providing an initial unit test for this. I cleaned this up and augmented some more tests. |
| getLinkModel(it->first->getName())->addAssociatedFixedTransform(link_model_vector_[i], it->second.inverse()); | ||
| link_model_vector_[i]->addAssociatedFixedTransform(it->first, it->second); | ||
| // this const-cast is equivalent to | ||
| // associated_transforms[tf_base.first->getLinkIndex()] |
There was a problem hiding this comment.
This doesn't make sense at all, does it?
associated_transforms is probably way smaller than tf_base.first->getLinkIndex().
You just want to regain write access to the "base" link, don't you?
I will add a commit to fix this comment.
…moveit#912) Old implementation only computes those transforms *below* the link in the kinematic tree. If the robot structure contains y-paths made up of fixed transforms, the previous implementation could miss associations. The structure - a - b - c \ - d only added associationed fixed transforms for a and b to c, but missed d.
…moveit#912) Old implementation only computes those transforms *below* the link in the kinematic tree. If the robot structure contains y-paths made up of fixed transforms, the previous implementation could miss associations. The structure - a - b - c \ - d only added associationed fixed transforms for a and b to c, but missed d.
If the robot structure contains y-paths made up of fixed transforms, the
previous implementation missed these associations in the associated_transforms.
The structure
\
only added associationed fixed transforms for a and b to c, but missed d.
I missed to submit this request when I wrote it two months ago
and it just popped up again here.