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

Add support for animation tension #256

Merged
merged 2 commits into from
Oct 14, 2021
Merged

Add support for animation tension #256

merged 2 commits into from
Oct 14, 2021

Conversation

nkoenig
Copy link
Contributor

@nkoenig nkoenig commented Oct 6, 2021

Signed-off-by: Nate Koenig [email protected]

🎉 New feature

Summary

PoseAnimation was missing the tension parameter, which was implemented in gazebo classic.

I didn't put this in ign-common3 because the Animation classes were not PIMPL-ized.

Test it

See: gazebosim/gz-sim#1091

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • 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

@codecov
Copy link

codecov bot commented Oct 6, 2021

Codecov Report

Merging #256 (fb1b612) into ign-common4 (2af6895) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-common4     #256      +/-   ##
===============================================
+ Coverage        77.00%   77.05%   +0.04%     
===============================================
  Files               75       75              
  Lines            10655    10665      +10     
===============================================
+ Hits              8205     8218      +13     
+ Misses            2450     2447       -3     
Impacted Files Coverage Δ
graphics/src/Animation.cc 95.73% <100.00%> (+1.70%) ⬆️

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 2af6895...fb1b612. Read the comment docs.

@chapulina chapulina added the graphics Graphics component label Oct 6, 2021
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

One though on a todo for forward porting.

Is there any integration/unit testing that we can add here?

graphics/include/ignition/common/Animation.hh Show resolved Hide resolved
@mjcarroll
Copy link
Contributor

@osrf-jenkins retest this please

Signed-off-by: Nate Koenig <[email protected]>
@nkoenig
Copy link
Contributor Author

nkoenig commented Oct 14, 2021

Added a test in fb1b612

@nkoenig nkoenig enabled auto-merge (squash) October 14, 2021 20:38
@nkoenig nkoenig merged commit 773a808 into ign-common4 Oct 14, 2021
@nkoenig nkoenig deleted the animation_tension branch October 14, 2021 20:51
@nkoenig nkoenig mentioned this pull request Oct 15, 2021
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress graphics Graphics component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants