Fix build errors and warnings for DART 6.13.0

[azeey]

🦟 Bug fix

Summary

DART 6.13.0 was tagged yesterday and is already available on homebrew. This PR fixes a compilation errors related to ContactSurfaceParams, which was a feature added in our fork first and upstreamed. Variable names were changed during the upstreaming process, which now has to be resolved.

This also fixes a compilation warning with using BodyNode::getShapeNodes which is now deprecated and as been replaced by BodyNode::eachShapeNode.

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 #465 (51a7fed) into ign-physics2 (5e99521) will not change coverage.
The diff coverage is n/a.

@@              Coverage Diff              @@
##           ign-physics2     #465   +/-   ##
=============================================
  Coverage         83.15%   83.15%           
=============================================
  Files               108      108           
  Lines              4210     4210           
=============================================
  Hits               3501     3501           
  Misses              709      709           

Impacted Files	Coverage Δ
dartsim/src/Base.hh	99.01% <ø> (ø)
dartsim/src/SimulationFeatures.cc	82.90% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[mjcarroll]

I think my preference would be use macros that are characterized by a feature name, rather than a version number. We can then set those in a single header. Similar to how libav does it: https://github.com/FFmpeg/FFmpeg/blob/master/libavutil/version.h

I think it makes it clearer from a readability and maintenance point of view, but I would be open to arguments the other way.

[azeey]

I would agree with that if this was based on the presence of a feature, but it really is based on whether we're using a newer API (> 6.13.0) or not.

[ahcorde]

I would agree with that if this was based on the presence of a feature, but it really is based on whether we're using a newer API (> 6.13.0) or not.

I already have this fix in my local setup. LGTM

[mjcarroll]

@osrf-jenkins retest this please

[scpeters]

I think my preference would be use macros that are characterized by a feature name, rather than a version number. We can then set those in a single header. Similar to how libav does it: https://github.com/FFmpeg/FFmpeg/blob/master/libavutil/version.h

I think it makes it clearer from a readability and maintenance point of view, but I would be open to arguments the other way.

I would agree with that if this was based on the presence of a feature, but it really is based on whether we're using a newer API (> 6.13.0) or not.

I think @mjcarroll's concern can be addressed in a cosmetic way with some extra macros with verbose names:

in SimulationFeatures.cc:

#if DART_VERSION_AT_LEAST(6, 13, 0)
// comments with relevant links to pull request / issue
#define DART_HAS_NEW_FRICTION_VARIABLE_NAMES
#endif

in Base.hh:

#if DART_VERSION_AT_LEAST(6, 13, 0)
// comments with relevant links to pull request / issue
#define DART_HAS_EACH_SHAPE_NODE_API
#endif

and then using the presence of those macros in the relevant code

[scpeters]

I think @mjcarroll's concern can be addressed in a cosmetic way with some extra macros with verbose names:

I've implemented these verbose macro names in a separate branch in 51a7fed. If you'd like to merge it into this branch with your own changes, that works, or I can submit it as a separate pull request if you prefer

[azeey]

I think @mjcarroll's concern can be addressed in a cosmetic way with some extra macros with verbose names:

I've implemented these verbose macro names in a separate branch in 51a7fed. If you'd like to merge it into this branch with your own changes, that works, or I can submit it as a separate pull request if you prefer

Thanks for the changes. I've gone ahead and merged them to this branch.

[Crola1702]

@azeey @scpeters, this is pending the forward port right?

Please let me know when you are going to do so, I would like to have a peer programming session and see how to do a proper forward port.