Set a cloned joint's parent/child link names to the cloned parent/child link names

[adlarkin]

Signed-off-by: Ashton Larkin [email protected]

🦟 Bug fix

Summary

When a joint is cloned, the cloned joint's parent and child links need to be set to the cloned parent and child links (currently, the cloned joint's parent/child links are the original joint's parent/child links). Otherwise, links will not be connected to joints when cloning things like models that have links and joints.

The approach taken here is similar to the approach taken for updating a cloned canonical link in #959, where internal maps are used to keep track of a joint's original links and cloned links. As discussed in #959 (comment), we shouldn't give special treatment to certain components. This is just a temporary workaround until we find a better solution.

To test this, do the following:

1. Start gazebo with a double pendulum, since this model has links and joints:

ign gazebo -r pendulum_links.sdf

2. In another terminal window, clone the double pendulum:

ign service -s /world/double_pendulum/create --reqtype ignition.msgs.EntityFactory --reptype ignition.msgs.Boolean --timeout 300 --req 'clone_name: "double_pendulum_with_base", pose: {position: {y: 3}}, name: "pendulum_copy"'

3. You should now see two pendulums operating as expected:

If you repeat the steps above without this PR, you'll notice that the links on the cloned pendulum fall to the ground because the cloned pendulum does not have proper joints:

Checklist

• Signed all commits for DCO
• Added tests
• Updated documentation (as needed)
• Updated migration guide (as needed)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

[adlarkin]

There's a similar TODO note here: https://github.com/ignitionrobotics/ign-gazebo/blob/8f083d6d212c5867cc18cd45162ba3e4807643c4/src/EntityComponentManager.cc#L248-L250

I don't think we have an issue for tracking the idea of "generalizable component cloning" yet, do we? Should we create one?

[azeey]

Detachable joints also contain information about parent and child links. Should we add those as well or state that they're not supported?

[codecov]

Codecov Report

Merging #1075 (cb681b8) into ign-gazebo6 (30c6df0) will decrease coverage by 0.02%.
The diff coverage is 67.34%.

❗ Current head cb681b8 differs from pull request most recent head 3dbbf5a. Consider uploading reports for the commit 3dbbf5a to get more accurate results

@@               Coverage Diff               @@
##           ign-gazebo6    #1075      +/-   ##
===============================================
- Coverage        64.09%   64.07%   -0.03%     
===============================================
  Files              255      255              
  Lines            20002    20051      +49     
===============================================
+ Hits             12821    12847      +26     
- Misses            7181     7204      +23     

Impacted Files	Coverage Δ
include/ignition/gazebo/EntityComponentManager.hh	100.00% <ø> (ø)
src/EntityComponentManager.cc	84.22% <67.34%> (-1.13%)	⬇️
src/gui/plugins/plot_3d/Plot3D.cc	43.47% <0.00%> (-4.35%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30c6df0...3dbbf5a. Read the comment docs.

[adlarkin]

Detachable joints also contain information about parent and child links. Should we add those as well or state that they're not supported?

As we discussed offline, adding support for cloning detachable joints would require cloning the DetachableJoint system. So, for now, I have made a note in the documentation that cloning detachable joints is not supported: b557328

I've also created an issue for adding detachable joint cloning support so that we can re-visit this in the future: #1080

[nkoenig]

Just one minor comment.