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

[Citadel] [TPE] Add GetContactsFromLastStepFeature #61

Merged
merged 6 commits into from
Jun 9, 2020

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented May 29, 2020

depends on pull request #59 and pull request #60

Add TPE implementation of GetContactsFromLastStepFeature. It returns contact points generated by the CollisionDetector introduced in pull request #60

Since contacts are associated with collisions in ign-physics but TPE checks contacts on the model level, a workaround is implemented to return the first collision in the canonical link of the model.

The test added caught a bug in EntityManagementFeatures in which incorrect entity names were returned so I had to remove the static variables.

iche033 added 4 commits May 28, 2020 17:39
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@iche033 iche033 requested a review from mxgrey as a code owner May 29, 2020 01:03
@iche033 iche033 requested a review from claireyywang May 29, 2020 01:04
@codecov
Copy link

codecov bot commented May 29, 2020

Codecov Report

Merging #61 into ign-physics2 will increase coverage by 0.35%.
The diff coverage is 94.96%.

Impacted file tree graph

@@               Coverage Diff                @@
##           ign-physics2      #61      +/-   ##
================================================
+ Coverage         82.10%   82.45%   +0.35%     
================================================
  Files               106      106              
  Lines              3559     3585      +26     
================================================
+ Hits               2922     2956      +34     
+ Misses              637      629       -8     
Impacted Files Coverage Δ
tpe/lib/src/Entity.hh 0.00% <ø> (ø)
tpe/lib/src/Shape.hh 100.00% <ø> (ø)
tpe/lib/src/World.hh 100.00% <ø> (ø)
tpe/plugin/src/EntityManagementFeatures.cc 81.32% <50.00%> (+3.09%) ⬆️
tpe/lib/src/Collision.cc 76.31% <75.00%> (ø)
tpe/plugin/src/SimulationFeatures.cc 70.00% <88.88%> (+30.86%) ⬆️
tpe/lib/src/Entity.cc 82.97% <91.66%> (-0.36%) ⬇️
tpe/lib/src/CollisionDetector.cc 100.00% <100.00%> (ø)
tpe/lib/src/CollisionDetector.hh 100.00% <100.00%> (ø)
tpe/lib/src/Utils.cc 100.00% <100.00%> (ø)
... and 10 more

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 12e7299...4be03d2. Read the comment docs.

@iche033 iche033 requested a review from chapulina May 29, 2020 03:34
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Base automatically changed from collision_detector to ign-physics2 June 4, 2020 19:39
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Copy link
Contributor

@claireyywang claireyywang left a comment

Choose a reason for hiding this comment

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

when I tested it locally, I got the following error:

~/citadel_ws $ ign gazebo -v 2 shapes.sdf --physics-engine ignition-physics-tpe-plugin
[Wrn] [Component.hh:144] Trying to serialize component with data type [N3sdf2v95WorldE], which doesn't have `operator<<`. Component will not be serialized.
[Wrn] [Component.hh:144] Trying to serialize component with data type [N3sdf2v95ModelE], which doesn't have `operator<<`. Component will not be serialized.
ign gazebo server: symbol lookup error: /home/claire/citadel_ws/install/lib/libignition-physics2-tpelib.so.2: undefined symbol: _ZN8ignition7physics6tpelib17CollisionDetector15CheckCollisionsERKSt3mapImSt10shared_ptrINS1_6EntityEESt4lessImESaISt4pairIKmS6_EEEb

@iche033
Copy link
Contributor Author

iche033 commented Jun 8, 2020

I just rebuilt ign-physics and ign-gazebo in two colcon worksapces and I can't reproduce the unresolved symbol error. I do see the component warning msgs.

Could you try clean the build dir and rebuild to see if the problem still occurs?

@claireyywang
Copy link
Contributor

I just rebuilt ign-physics and ign-gazebo in two colcon worksapces and I can't reproduce the unresolved symbol error. I do see the component warning msgs.

Could you try clean the build dir and rebuild to see if the problem still occurs?

I did a clean build and the error went away. Thanks for double checking!

Copy link
Contributor

@claireyywang claireyywang left a comment

Choose a reason for hiding this comment

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

LGTM!

@iche033 iche033 merged commit 2894ef4 into ign-physics2 Jun 9, 2020
@iche033 iche033 deleted the tpe_contact_feature branch June 9, 2020 23:23
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