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.

Sign up for GitHub

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

Jump to bottom

Add offset to link and collision pose #79

Merged
merged 6 commits into from
Jul 13, 2020
Merged

Add offset to link and collision pose #79

merged 6 commits into from
Jul 13, 2020

Conversation

claireyywang
Copy link
Contributor

@claireyywang claireyywang commented Jul 9, 2020 •
edited
Loading

Use pose represent relative transformation to parent entity. GetWorldPose represents pose relative to world. This PR takes account of link and collision relative pose to compute their worldPose.

This is the first step to implement #57

@claireyywang claireyywang added the TPE Trivial Physics Engine label Jul 9, 2020
@claireyywang claireyywang requested a review from mxgrey as a code owner July 9, 2020 20:04
@claireyywang claireyywang self-assigned this Jul 9, 2020
@claireyywang claireyywang marked this pull request as draft July 9, 2020 20:04
@claireyywang claireyywang removed the request for review from mxgrey July 9, 2020 20:04
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Jul 9, 2020
@claireyywang
update link pose in model
0ab41ec 
Signed-off-by: claireyywang <[email protected]>
@codecov
Copy link

codecov bot commented Jul 9, 2020 •
edited
Loading

Codecov Report

Merging #79 into ign-physics2 will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff                @@
##           ign-physics2      #79      +/-   ##
================================================
+ Coverage         82.80%   82.82%   +0.01%     
================================================
  Files               106      106              
  Lines              3659     3663       +4     
================================================
+ Hits               3030     3034       +4     
  Misses              629      629
Impacted Files Coverage Δ
tpe/lib/src/Entity.hh 0.00% <ø> (ø)
tpe/lib/src/Entity.cc 85.83% <100.00%> (+0.48%) ⬆️

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 d8f9e0b...f0dcf08. Read the comment docs.

scpeters
scpeters reviewed Jul 10, 2020
View reviewed changes
tpe/lib/src/Link.hh Outdated Show resolved Hide resolved
@claireyywang @iche033
testing world pose function (#80)
aa0763a 
Signed-off-by: Ian Chen <[email protected]>

Co-authored-by: Ian Chen <[email protected]>
Signed-off-by: claireyywang <[email protected]>
@iche033
Copy link
Contributor

iche033 commented Jul 11, 2020 •
edited
Loading

The changes look good to me. Is this PR still a draft?

Btw, I started looking at the bounding box issue when there is a link offset. I think with the latest change of keeping GetPose and adding a new GetWorldPose function, the bounding box calculation should now be fine. The GetBoundingBox function takes into account the child entity's pose relative to parent and the test case also includes an example model with link offset. Let me know if you still see an issue.

@claireyywang claireyywang marked this pull request as ready for review July 13, 2020 06:04
@claireyywang claireyywang requested a review from iche033 July 13, 2020 19:54
@claireyywang
Copy link
Contributor Author

@iche033 awesome, thanks for looking into it. I think this PR is ready to go. I have updates in tpeplugin but that is on a separate branch and doesn't interfere with this one.

iche033
iche033 approved these changes Jul 13, 2020
View reviewed changes
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

just couple of minor comments, otherwise this is good to go

tpe/lib/src/Collision.hh Outdated Show resolved Hide resolved
tpe/lib/src/Link.hh Outdated Show resolved Hide resolved
@claireyywang claireyywang merged commit 1207fe0 into ign-physics2 Jul 13, 2020
@claireyywang claireyywang deleted the claire/add-tf branch July 13, 2020 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scpeters scpeters scpeters left review comments

@iche033 iche033 iche033 approved these changes

Assignees

@claireyywang claireyywang

Labels
🏰 citadel Ignition Citadel TPE Trivial Physics Engine
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@claireyywang @iche033 @scpeters