Allow re-attaching detached joint

[liamhan0905]

Signed-off-by: Liam Han liam@openrobotics.org
🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

🎉 New feature

Summary

Extending the feature for detachable_joint plugin to support re-attaching the joint again. By publishing to a topic, the child link can be easily reattached with a fixed joint to its parent link. The demo below shows detaching and reattaching B1 and B2 models.

This PR needs to be addressed after this current PR

re-attach.webm

Once detached, it can be useful to move the child model to a desired position through a service call and then reattach the joint. The demo below shows detaching the B1 model, moving to a desired location, and then re-attaching the joint.

attach+reset.webm

Test it

checkout this branch and build it. To run the test, run the following command in gz workspace.

./build/ignition-gazebo6/bin/INTEGRATION_detachable_joint

Checklist

• Signed all commits for DCO
• Added tests
• Added example and/or tutorial
• Updated documentation (as needed)
• Updated migration guide (as needed)
• Consider updating Python bindings (if the library has them)
• 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 and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

[codecov]

Codecov Report

Merging #1687 (87764b9) into ign-gazebo6 (e37ff8f) will decrease coverage by 0.02%.
The diff coverage is 66.66%.

❗ Current head 87764b9 differs from pull request most recent head 5bf2a93. Consider uploading reports for the commit 5bf2a93 to get more accurate results

@@               Coverage Diff               @@
##           ign-gazebo6    #1687      +/-   ##
===============================================
- Coverage        65.38%   65.37%   -0.02%     
===============================================
  Files              327      327              
  Lines            26932    26996      +64     
===============================================
+ Hits             17610    17649      +39     
- Misses            9322     9347      +25     

Impacted Files	Coverage Δ
src/systems/detachable_joint/DetachableJoint.hh	100.00% <ø> (ø)
src/systems/detachable_joint/DetachableJoint.cc	66.18% <66.66%> (-4.48%)	⬇️

... and 2 files with indirect coverage changes

[bperseghetti]

This looks like a really great new feature add.

[liamhan0905]

[STATUS]

• ready for review
• created a new PR that changes the <topic> tag to <detach_topic> tag

[QUESTIONS]

• currently I'm using ingition.msgs.StringMsg to publish the state of the joint, e.g. whether it is detached or attached. These states can be useful to subscribe. I wanted to initially use ignition.msgs.Boolean but since false value can't be used, I decided to use StringMsg type. Let me know if you have any good suggestions

[liamhan0905]

@ahcorde thanks for the feedback. Left some comments and addressed other things

• rename ignition -> gazebo
• consolidated the other minor PR into this PR
  ready for another review

[liamhan0905]

Note to self:

Just occurred to me that this will potentially have a backwards compatibility issue with <topic> tag being switched to <detach_topic> tag. I'll address this by 11/14/22

Update:
Addressed

[jrutgeer]

@liamhan0905

I wanted to initially use ignition.msgs.Boolean but since false value can't be used, I decided to use StringMsg type.

What do you mean with "false can't be used"?

[liamhan0905]

@jrutgeer
If you check this link, it states

Note that for scalar message fields, once a message is parsed there’s no way of telling whether a field was explicitly set to the default value (for example whether a boolean was set to false) or just not set at all: you should bear this in mind when defining your message types. For example, don’t have a boolean that switches on some behavior when set to false if you don’t want that behavior to also happen by default. Also note that if a scalar message field is set to its default, the value will not be serialized on the wire.

Hence, the boolean false can't be used to check the state. That's why I resorted to using stringMsg type. But there's probably a better way of tackling things like this. Open for suggestions!

[azeey]

This looks great! I just have a few minor comments. The main issue is what we should do about <topic>.

[azeey]

Just minor comments. The tutorial mentions that objects should not be in contact in their initial (attached) state. I think we should emphasize this more now that we allow attaching detached joints. In the detachable_joint.sdf example, if you do the following in sequence you will get a crash:

ign topic -t "/model/vehicle_blue/cmd_vel" -m ignition.msgs.Twist -p "linear: {x: -0.25}"
ign topic -t "/B1/detach" -m ignition.msgs.Empty -p "unused: true"
# wait a couple seconds until the vehicle starts dragging B1.
ign topic -t "/B1/attach" -m ignition.msgs.Empty -p "unused: true"

[liamhan0905]

I added some comments regarding reattaching while the child and parent models are in contact but let me know if you'd prefer more explanation or any clarification shown here. I didn't try this before so this was a mild surprise haha

[azeey]

This looks ready to me. @ahcorde any remaining issues?

All comments have been addressed