ParserConfig: save auto inertial values by default

[scpeters]

🦟 Bug fix

Follow-up to #1299 (comment)

Summary

While reviewing #1299, I suggested adding a parameter to ParserConfig so that inertial values could be computed by default, but the calculation could be skipped by setting the appropriate parameter before loading, which is needed by gz-sim. At least, my intent was to point out that saving auto-inertial values should be the default, but I didn't articulate it explicitly, and the implementation skips by default instead.

This changes the default configuration of ParserConfig to save auto inertial values instead of skipping. Without this change gz sdf --inertial-stats doesn't resolve auto-inertial values (confirm by building ab438f3 and observing the failure of UNIT_gz_TEST).

Minor changes:

• 36de542: fix xml syntax in test/sdf/inertial_stats.sdf
• ab438f3: add test/sdf/inertial_stats_auto.sdf based on test/sdf/inertial_stats.sdf with //inertial/@auto=true and //collision/density values that yield equivalent inertial values
• 4b1112e: minor changes to error messages

Checklist

• Signed all commits for DCO
• Added tests
• Updated documentation (as needed)
• Updated migration guide (as needed)
• Consider updating Python bindings (if the library has them)
• 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 and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

[codecov]

Codecov Report

Merging #1325 (512cea7) into sdf14 (2ba6470) will increase coverage by 0.01%.
The diff coverage is 90.00%.

❗ Current head 512cea7 differs from pull request most recent head d6d8e7e. Consider uploading reports for the commit d6d8e7e to get more accurate results

@@            Coverage Diff             @@
##            sdf14    #1325      +/-   ##
==========================================
+ Coverage   87.47%   87.49%   +0.01%     
==========================================
  Files         134      134              
  Lines       17751    17759       +8     
==========================================
+ Hits        15528    15538      +10     
+ Misses       2223     2221       -2     

Files	Coverage Δ
src/Geometry.cc	100.00% <ø> (ø)
src/Mesh.cc	98.90% <100.00%> (+0.07%)	⬆️
src/ParserConfig.cc	100.00% <ø> (ø)
src/Link.cc	98.09% <75.00%> (+<0.01%)	⬆️

... and 2 files with indirect coverage changes

[azeey]

I added a test in c550e3d that fails because a MOI calculator callback is not set in sdf::ParserConfig. Mesh::CalculateInertial throws an exception because the callback is used without checking if it's set. I tried adding

  if (!customCalculator)
  {
    _errors.push_back(
        {sdf::ErrorCode::WARNING,
         "Custom moment of inertia calculator for meshes not set via "
         "sdf::ParserConfig::RegisterCustomInertiaCalc"});
    return std::nullopt;
  }

but I got another error from Collision.cc saying the "Inertia Calculated for collision is invalid" :

sdformat/src/Collision.cc

Lines 277 to 282 in c550e3d

	if (!geomInertial)
	{
	_errors.push_back({ErrorCode::LINK_INERTIA_INVALID,
	"Inertia Calculated for collision: " +
	this->dataPtr->name + " is invalid."});
	}

Do we want it to just use the default inertial if a callback is not set?

[scpeters]

I added a test in c550e3d that fails because a MOI calculator callback is not set in sdf::ParserConfig. Mesh::CalculateInertial throws an exception because the callback is used without checking if it's set. I tried adding

but I got another error from Collision.cc saying the "Inertia Calculated for collision is invalid" :

sdformat/src/Collision.cc

Lines 277 to 282 in c550e3d

	if (!geomInertial)
	{
	_errors.push_back({ErrorCode::LINK_INERTIA_INVALID,
	"Inertia Calculated for collision: " +
	this->dataPtr->name + " is invalid."});
	}

Do we want it to just use the default inertial if a callback is not set?

I think we could use the default inertial with a warning that could be upgraded to an error with the right ParserConfig setting

[scpeters]

updated in d6d8e7e

[azeey]

Returning the default Inertial instead of a nullopt makes more sense. Now, we only get the error from Mesh.cc.