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

Updated SDF examples #573

Merged
merged 1 commit into from
Jan 21, 2021
Merged

Updated SDF examples #573

merged 1 commit into from
Jan 21, 2021

Conversation

jennuine
Copy link
Contributor

Added <size> element to ground_plane collisions since it is a required spec of SDFormat in all example and test SDF files.

This also provides collision visualization for the ground plane: #531

@jennuine jennuine requested a review from iche033 January 20, 2021 14:13
@jennuine jennuine requested a review from chapulina as a code owner January 20, 2021 14:13
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Jan 20, 2021
@codecov
Copy link

codecov bot commented Jan 20, 2021

Codecov Report

Merging #573 (5e18bb2) into ign-gazebo3 (ddd7fcd) will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-gazebo3     #573      +/-   ##
===============================================
+ Coverage        77.66%   77.71%   +0.05%     
===============================================
  Files              208      208              
  Lines            11574    11574              
===============================================
+ Hits              8989     8995       +6     
+ Misses            2585     2579       -6     
Impacted Files Coverage Δ
src/SimulationRunner.cc 94.53% <0.00%> (+1.12%) ⬆️

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 ddd7fcd...5e18bb2. Read the comment docs.

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Nice! Even though DART is ignoring the size and creating an infinite plane, we should still specify it because it is required. Other physics engines may use it.

@chapulina chapulina merged commit 8998740 into ign-gazebo3 Jan 21, 2021
@chapulina chapulina deleted the jennuine/update_examples branch January 21, 2021 02:29
@azeey
Copy link
Contributor

azeey commented Jan 21, 2021

Isn't the size of the plane only meaningful for visualization? i.e, I thought all physics engines should create an infinite plane. If a plane is not infinite, why is it needed? You can just use a box shape.

@jennuine
Copy link
Contributor Author

@azeey when trying to view collisions (i.e., visuals) of the ground plane (#531) when the geometry is a //plane, the collision visual could not be viewed because it was missing the //size tag. I did notice in some of the examples used a box shape instead of a plane.

@chapulina
Copy link
Contributor

I thought all physics engines should create an infinite plane. If a plane is not infinite, why is it needed? You can just use a box shape.

I think that's a fair question, but I'm not sure we can make this assumption. A box has 6 sides, while a plane can be interpreted to have 1 or 2 at most. So I can imagine a physics engine that implements a finite 2-sided plane without sides, for example.

That could be something interesting to iterate on the SDF spec.

the collision visual could not be viewed because it was missing the //size tag

That's an interesting use case, because the collision visual may be misleading. Currently, even though the visual will be 100x100, the actual collision created by DART is infinite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants