-
Notifications
You must be signed in to change notification settings - Fork 97
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
Use //link/inertial/density for auto-inertials #1335
Conversation
The density precedence for Collisions is now: 1. Density explicitly set in //collision/density or by Collision::SetDensity. 2. Density explicitly set in //link/inertial/density or by Link::SetDensity. 3. Default density in Collision::DensityDefault(). Signed-off-by: Steve Peters <[email protected]>
f5803b7
to
ed6f2d5
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## sdf14 #1335 +/- ##
==========================================
+ Coverage 92.40% 92.42% +0.01%
==========================================
Files 134 134
Lines 17674 17707 +33
==========================================
+ Hits 16332 16365 +33
Misses 1342 1342 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Steve Peters <[email protected]>
Per #1330 (comment), and #1330 (comment) could we pass along the |
Signed-off-by: Steve Peters <[email protected]>
Pass //link/inertial/auto_inertia_params ElementPtr to Collision::CalculateInertial and use it if the Collision does not have auto_inertia_params of its own. Signed-off-by: Steve Peters <[email protected]>
good call, thanks for the reminder I've started work in scpeters/inertial_density...scpeters/inertial_density_auto_params, but I didn't write the test yet, since I think it will need a mock Geometry class in order to confirm which ElementPtr is passed to CalculateInertial. |
Instead of a mock Geometry class, would it be easier to provide a mesh inertia calculator callback in the test that will receive the Line 341 in 11205c9
autoInertiaParams passed to Geometry .
|
The FilePath refers to the SDFormat file that the Mesh object was parsed from, not the Mesh file itself, so remove the unrelated warning. Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
I adapted the lambda callback from Geometry_TEST.cc for an |
Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
I fixed a compiler warning and coverage; CI is happy now |
Signed-off-by: Steve Peters <[email protected]>
A density element was written unconditionally, even if it was not originally set by the user. This checks the optional density value before setting via ToElement(). Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
🎉 New feature
Closes #1329. cc @jasmeet0915
Summary
This adds support for specifying density and
auto_inertia_params
in the//link/inertial/density
and//link/inertial/auto_inertia_params
elements respectively, which will apply to all collisions within a link. The following APIs are added:static double Collision::DensityDefault()
: accessor for default density valueCollision::CalculateInertial
with astd::optional<double>
input to represent the value from//link/inertial/density
if it has been set and ansdf::ElementPtr
to the<auto_inertia_params>
element ornullptr
if it has not been setdouble Link::Density() const
void Link::SetDensity(double)
sdf::ElementPtr AutoInertiaParams() const
void SetAutoInertiaParams(const sdf::ElementPtr)
The density precedence for Collisions is now:
//collision/density
or byCollision::SetDensity
.//link/inertial/density
or byLink::SetDensity
.Collision::DensityDefault()
.The following table summarizes this behavior:
The precedence rules for AutoInertiaParams match the rules for density.
Test it
A test has has been added in
UNIT_Link_TEST
with the 4 combinations of link / collision density being set or not from the table above. Also thetest/sdf/inertial_stats_auto.sdf
file, which is tested withgz sdf --inertial-stats
inUNIT_gz_TEST
, is modified so that a//collision/density
value is converted to//link/inertial/density
, and the test still passes.Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-by
messages.