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

Apply mimic constraint to joints #1838

Merged
merged 18 commits into from
Aug 31, 2023
Merged

Apply mimic constraint to joints #1838

merged 18 commits into from
Aug 31, 2023

Conversation

adityapande-1995
Copy link
Contributor

@adityapande-1995 adityapande-1995 commented Dec 13, 2022

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

🎉 New feature

Summary

This PR picks up the mimic joint tag from sdf::JointAxis, and aplies it to the relevant gz::physics joint.
Work in progress !

Test it

Test with the example world included in this PR:

gz sim -v 4 mimic_fast_slow_pendulums_world.sdf \
  --physics-engine gz-physics-bullet-featherstone-plugin

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.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

Signed-off-by: Aditya <aditya050995@gmail.com>
Signed-off-by: Aditya <aditya050995@gmail.com>
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Dec 13, 2022
@osrf-triage osrf-triage added this to Inbox in Core development Dec 13, 2022
@adityapande-1995 adityapande-1995 changed the title Aditya/mimic constraint apply Apply mimic constraint to joints Dec 13, 2022
@azeey azeey moved this from Inbox to In progress in Core development Dec 19, 2022
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters
Copy link
Member

After finally testing this pull request, I've noticed that dartsim's MimicMotorConstraint is not behaving the same way as the Gearbox joint in gazebo-classic. While the gearbox joint creates a bilateral constraint that applies equal and opposite impulses on the connected joints, the Mimic constraint appears to be unilateral, in that it only applies impulses to the follower joint and does not affect the leader joint. This can be seen in the following animation from the mimic_pendulum_world.sdf that I added to this branch in 554f99d

In this example world, there are three pairs of pendulums. Each pair of pendulums have different lengths, and thus different oscillation frequencies. The middle pair of pendulums are uncoupled, so you can see how they oscillate at different frequencies, and their motions diverge over time. On the left side, the large pendulum mimics the small pendulum with a multiplier of 1.0 and follows it exactly. On the right side, the small pendulum follows the large pendulum with a multiplier of 1.0 and also follows it exactly. With a gearbox joint, I would expect the pendulums to both oscillate at an in-between frequency.

mimic_pendulum.mp4

The behavior of dartsim's MimicMotorConstraint is not wrong, but we need a slightly different constraint to replace the gearbox functionality from gazebo-classic.

@scpeters
Copy link
Member

I've opened a feature request in dartsim/dart#1756 to add support for a bilateral linear constraint between joints

Based on changes made in gz-physics test world.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@azeey azeey added the beta Targeting beta release of upcoming collection label Jul 31, 2023
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters scpeters force-pushed the aditya/mimic_constraint_apply branch from 23c64da to 1678861 Compare August 10, 2023 18:12
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters scpeters marked this pull request as ready for review August 10, 2023 20:20
@scpeters
Copy link
Member

I think this is ready for review

@scpeters scpeters changed the base branch from gz-sim7 to main August 25, 2023 06:05
@scpeters scpeters added 🎵 harmonic Gazebo Harmonic and removed 🌱 garden Ignition Garden labels Aug 25, 2023
@scpeters
Copy link
Member

retargeted to harmonic / main branch

@codecov
Copy link

codecov bot commented Aug 25, 2023

Codecov Report

Merging #1838 (3fcad36) into main (5846393) will decrease coverage by 0.04%.
Report is 1 commits behind head on main.
The diff coverage is 43.47%.

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

@@            Coverage Diff             @@
##             main    #1838      +/-   ##
==========================================
- Coverage   65.32%   65.28%   -0.04%     
==========================================
  Files         321      321              
  Lines       30303    30326      +23     
==========================================
+ Hits        19795    19799       +4     
- Misses      10508    10527      +19     
Files Changed Coverage Δ
src/systems/physics/Physics.cc 71.84% <43.47%> (-0.41%) ⬇️

... and 1 file with indirect coverage changes

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

Works for me! Just one minor comment

@@ -0,0 +1,655 @@
<?xml version="1.0" ?>
<!--
Gazebo Mimic constraint demo
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this currently only works with bullet-featherstone, can you provide the command to run here?

Copy link
Member

Choose a reason for hiding this comment

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

I added the example command in e9b22fc

I noticed that lots of our other example worlds are using command-line strings with -- inside XML comment blocks, which is invalid XML, so I put the example command inside a CDATA block. Now xmllint is happy with the file, and I believe it still works.

Put example command in CDATA block because -- is not allowed
in <!-- --> comment blocks. Put the CDATA and comment block
just inside the <sdf> tag.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters
Copy link
Member

I just merged with main to get the gz-msgs fix. Fingers crossed for good CI...

@scpeters
Copy link
Member

I just merged with main to get the gz-msgs fix. Fingers crossed for good CI...

I think we still need #2100 to land

@azeey
Copy link
Contributor

azeey commented Aug 31, 2023

I just merged with main to get the gz-msgs fix. Fingers crossed for good CI...

I think we still need #2100 to land

Yup

@azeey azeey merged commit 1ae10f7 into main Aug 31, 2023
5 of 10 checks passed
@azeey azeey deleted the aditya/mimic_constraint_apply branch August 31, 2023 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection 🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Core development
In progress
Development

Successfully merging this pull request may close these issues.

None yet

3 participants