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

Joint axis mimic constraints: add sdf element #1166

Merged
merged 65 commits into from
Aug 27, 2023
Merged

Joint axis mimic constraints: add sdf element #1166

merged 65 commits into from
Aug 27, 2023

Conversation

adityapande-1995
Copy link
Contributor

@adityapande-1995 adityapande-1995 commented Sep 20, 2022

🎉 New feature

Summary

This PR adds a <mimic joint="some_joint" multiplier="1" offset="0"> to joint axes i.e. <axis>, meant to expose a way to mimic some other joint. Part of feature : gazebosim/sdf_tutorials#62, gazebosim/sdf_tutorials#80

Test it

Added test cases to JointAxis_TEST.cc

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>
Signed-off-by: Aditya <aditya050995@gmail.com>
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Sep 20, 2022
@osrf-triage osrf-triage added this to Inbox in Core development Sep 20, 2022
Signed-off-by: Aditya <aditya050995@gmail.com>
src/JointAxis.cc Outdated Show resolved Hide resolved
src/JointAxis.cc Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 26, 2022

Codecov Report

Merging #1166 (aefe58e) into main (a1027c3) will increase coverage by 0.02%.
The diff coverage is 90.36%.

❗ Current head aefe58e differs from pull request most recent head 1e4624c. Consider uploading reports for the commit 1e4624c to get more accurate results

@@            Coverage Diff             @@
##             main    #1166      +/-   ##
==========================================
+ Coverage   87.42%   87.45%   +0.02%     
==========================================
  Files         133      133              
  Lines       17295    17452     +157     
==========================================
+ Hits        15121    15262     +141     
- Misses       2174     2190      +16     
Files Changed Coverage Δ
python/src/sdf/pyJointAxis.cc 59.22% <55.55%> (-1.98%) ⬇️
python/src/sdf/_gz_sdformat_pybind11.cc 100.00% <100.00%> (ø)
python/src/sdf/pyError.cc 82.71% <100.00%> (+0.21%) ⬆️
src/Joint.cc 91.60% <100.00%> (+0.38%) ⬆️
src/JointAxis.cc 99.13% <100.00%> (+0.32%) ⬆️
src/Root.cc 95.23% <100.00%> (+0.02%) ⬆️
src/parser.cc 87.30% <100.00%> (+0.44%) ⬆️

@azeey azeey moved this from Inbox to In progress in Core development Sep 26, 2022
Signed-off-by: Aditya <aditya050995@gmail.com>
Signed-off-by: Aditya <aditya050995@gmail.com>
@adityapande-1995
Copy link
Contributor Author

Updated PR according to the proposal to include an axis attribute, and changed multiplier and offset to elements rather than attributes.

Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

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

I updated the proposal in gazebosim/sdf_tutorials@829aa86, so I commented on how the specification could be updated to reflect that.

sdf/1.10/joint.sdf Outdated Show resolved Hide resolved
sdf/1.10/joint.sdf Outdated Show resolved Hide resolved
sdf/1.10/joint.sdf Outdated Show resolved Hide resolved
@scpeters
Copy link
Member

scpeters commented Oct 8, 2022

I just added a //mimic/reference tag to the proposal in gazebosim/sdf_tutorials@93a8fad

Signed-off-by: Aditya <aditya050995@gmail.com>
Signed-off-by: Aditya <aditya050995@gmail.com>
@adityapande-1995
Copy link
Contributor Author

We can have an edge case where a joint tries to mimic itself. The JointAxis sdf element does not have access to the parent Joint element's name (or element pointer), and hence that check cannot be implemented here. I've implemented it here : gazebosim/gz-physics@4dd1b21

Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

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

We can have an edge case where a joint tries to mimic itself. The JointAxis sdf element does not have access to the parent Joint element's name (or element pointer), and hence that check cannot be implemented here. I've implemented it here : gazebosim/gz-physics@4dd1b21

We could also check in Joint.cc just after loading the JointAxis objects if each axis has a mimic and confirm that the mimic's joint name doesn't match its own name. There are more extensive tests that could be done to confirm that it does match the name of an existing joint, but that shouldn't happen in Joint::Load since not all joint will have been loaded yet.

include/sdf/JointAxis.hh Outdated Show resolved Hide resolved
src/JointAxis_TEST.cc Outdated Show resolved Hide resolved
Check that the specified leader axis name is valid
and matches an existing JointAxis.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
This is needed to reduce the literal string size
in Embedded.cc and fix the windows build.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
include/sdf/Error.hh Show resolved Hide resolved
include/sdf/JointAxis.hh Show resolved Hide resolved
Core development automation moved this from In progress to In review May 12, 2023
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>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters scpeters changed the title Mimic joints: Add sdf element Joint axis mimic constraints: add sdf element Aug 23, 2023
@scpeters scpeters changed the base branch from sdf13 to main August 23, 2023 20:19
@scpeters
Copy link
Member

retargeted to harmonic / main branch

@scpeters scpeters added 🎵 harmonic Gazebo Harmonic and removed 🌱 garden Ignition Garden labels Aug 23, 2023
src/Joint.cc Outdated Show resolved Hide resolved
src/Joint.cc Outdated Show resolved Hide resolved
src/JointAxis_TEST.cc Outdated Show resolved Hide resolved
src/parser.cc Outdated Show resolved Hide resolved
sdf/1.11/mimic.sdf Show resolved Hide resolved
* Initialize variables directly in range-based for loop
* Use local axis reference to simplify code
* Use FindElement in test and assert result is non nullptr

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Joint::Load already checks for invalid self-mimics.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters scpeters requested a review from ahcorde August 25, 2023 05:55
@scpeters scpeters dismissed ahcorde’s stale review August 25, 2023 06:17

comments have been addressed

sdf/1.11/mimic.sdf Show resolved Hide resolved
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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants