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

Set collision detector and solver for DART #225

Merged
merged 11 commits into from
Mar 24, 2021

Conversation

chapulina
Copy link
Contributor

@chapulina chapulina commented Mar 16, 2021

🎉 New feature

Summary

Add a feature that allows setting and getting a couple of physics options: the collision detector and the solver. DART offers a few options for each of these. The motivation is to make it easier for end users to test different options (which I want to do on #218).

It can be seen on the commit history that my first implementation was getting the options directly from SDF files. However, the sdf::World object received from ign-gazebo doesn't contain the Element() pointer, and the sdf::Physics object doesn't have collision detector and solver information yet due to gazebosim/sdformat#508. That's why I went for the feature approach instead.

Test it

The API's usage can be seen on the added test. It's also used on gazebosim/gz-sim#684.

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

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina added enhancement New feature or request close the gap Features from Gazebo-classic DART DART engine labels Mar 16, 2021
@chapulina chapulina requested a review from mxgrey as a code owner March 16, 2021 03:21
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Mar 16, 2021
Signed-off-by: Louise Poubel <[email protected]>
@codecov
Copy link

codecov bot commented Mar 16, 2021

Codecov Report

Merging #225 (dde6786) into main (f3342ee) will increase coverage by 0.06%.
The diff coverage is 86.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #225      +/-   ##
==========================================
+ Coverage   82.22%   82.28%   +0.06%     
==========================================
  Files         107      109       +2     
  Lines        4292     4353      +61     
==========================================
+ Hits         3529     3582      +53     
- Misses        763      771       +8     
Impacted Files Coverage Δ
dartsim/src/plugin.cc 100.00% <ø> (ø)
dartsim/src/WorldFeatures.cc 82.97% <82.97%> (ø)
include/ignition/physics/detail/World.hh 100.00% <100.00%> (ø)

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 f3342ee...dde6786. Read the comment docs.

@scpeters
Copy link
Member

@osrf-jenkins run tests please

@chapulina chapulina added the beta Targeting beta release of upcoming collection label Mar 17, 2021
@chapulina chapulina mentioned this pull request Mar 17, 2021
8 tasks
@chapulina chapulina self-assigned this Mar 19, 2021
@chapulina chapulina requested a review from scpeters March 19, 2021 01:39
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

Looks great! Just a few minor comments.

dartsim/src/WorldFeatures.cc Outdated Show resolved Hide resolved
dartsim/src/WorldFeatures.cc Outdated Show resolved Hide resolved
dartsim/src/WorldFeatures.cc Outdated Show resolved Hide resolved
@chapulina chapulina enabled auto-merge (squash) March 24, 2021 06:34
@chapulina chapulina merged commit ab50cc5 into main Mar 24, 2021
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 close the gap Features from Gazebo-classic DART DART engine 🏢 edifice Ignition Edifice enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants