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

Increment body node version after it has been moved to a new skeleton #1445

Merged
merged 3 commits into from
Apr 20, 2020

Conversation

mxgrey
Copy link
Member

@mxgrey mxgrey commented Apr 20, 2020

An issue with auto-updating collisions was found in ign-physics. This PR ports the fix into DART itself.

I will be pushing a regression test for this issue shortly.


Before creating a pull request

  • Document new methods and classes
  • Format new code files using clang-format

Before merging a pull request

  • Set version target by selecting a milestone on the right side
  • Summarize this change in CHANGELOG.md
  • Add unit test(s) for this change

@mxgrey mxgrey added this to the DART 6.9.3 milestone Apr 20, 2020
Copy link
Member

@jslee02 jslee02 left a comment

Choose a reason for hiding this comment

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

Could you change the target branch to release-6.9 if this patch needs to go into to v6.9.3?

I will be pushing a regression test for this issue shortly.

👍

@mxgrey mxgrey changed the base branch from master to release-6.9 April 20, 2020 04:08
@mxgrey
Copy link
Member Author

mxgrey commented Apr 20, 2020

Sorry, that's what I meant for the target branch to be. It's updated now.

@mxgrey
Copy link
Member Author

mxgrey commented Apr 20, 2020

I've added a unit test that fails if this line gets removed.

Copy link
Member

@jslee02 jslee02 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

It seems CI tests are failing due to issues that are unrelated to this change. release-6.9 has been failing: https://dev.azure.com/dartsim/dart/_build/results?buildId=2053&view=results

@codecov
Copy link

codecov bot commented Apr 20, 2020

Codecov Report

Merging #1445 into release-6.9 will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@               Coverage Diff               @@
##           release-6.9    #1445      +/-   ##
===============================================
+ Coverage        56.96%   56.99%   +0.02%     
===============================================
  Files              366      366              
  Lines            27145    27146       +1     
===============================================
+ Hits             15464    15472       +8     
+ Misses           11681    11674       -7     
Impacted Files Coverage Δ
dart/dynamics/Skeleton.cpp 66.14% <100.00%> (+0.01%) ⬆️
dart/dynamics/EulerJoint.cpp 67.68% <0.00%> (-3.05%) ⬇️
dart/collision/fcl/FCLCollisionDetector.cpp 83.33% <0.00%> (+0.85%) ⬆️
dart/dynamics/ZeroDofJoint.cpp 21.73% <0.00%> (+3.38%) ⬆️

@mxgrey mxgrey merged commit a42c205 into release-6.9 Apr 20, 2020
@mxgrey mxgrey deleted the bugfix/increment_version branch April 20, 2020 09:18
azeey added a commit to gazebosim/gz-physics that referenced this pull request Jun 24, 2021
…268)

When a joint is created between BodyNodes in different skeletons, the child BodyNode is moved to the skeleton of the parent BodyNode. When this happens, the BodyNode version of the child needs to be incremented. This is actually fixed by dartsim/dart#1445, but we don't have that merged into our fork. So in the meantime, we call incrementVersion after moveTo is called similar to #31.

Signed-off-by: Addisu Z. Taddese <[email protected]>

Co-authored-by: Louise Poubel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants