Upstream changes from our DART fork

[azeey]

We've been using a fork of DART on Ignition. This was originally done to add support for slip parameters and we used a fork because we had some lack of clarity on the units for the parameters and how they should be implemented in DART to behave the same way as they do in ODE.

Since the fork we have added the following changes:

• Fix issue with removing skeletons without shapes gazebo-forks/dart#4
• Fix FreeJoint velocity and position integration for articulated bodies Copy of dartsim PR #1437 gazebo-forks/dart#5
• Fix grouping of contact constraints gazebo-forks/dart#6
• Force dependent slip gazebo-forks/dart#9
• Generalize the way properties of a contact constraints are computed, allow for custom implementations gazebo-forks/dart#22

We are starting to diverge a bit from upstream, so we should start upstreaming these changes soon.

More context:

• Debs for our fork of DART are hosted on packages.osrfoundation.org and have versions with the pattern 6.10.0~osrf<version>~<date>...

• We have already upstreamed a change that was part of the slip parameters work (Add secondary friction coefficient parameter #1424).

• If an official release of DART 6.10 lands without slip parameter, it could potentially break ign-physics depending on whether the official version gets installed on a system.

• We have added #ifdefs in ign-physics2 to allow using ign-physics with debs from the dartsim ppa, which are currently at v6.9.

[chapulina]

How's this going? Does it look like we'll be able to use upstream DART for Dome?

[azeey]

I haven't looked at this for a while. I think I need to work on making some tests pass for dartsim/dart#1437 to be approved. I'll see if I can get some movement on that this/next week.

[simonbogh]

Any news on this? Would very much like to see the fix for grouping of contact constraints since more complex simulations with many objects will make the simulator crash.

[scpeters]

some of the slip-related changes are submitted upstream in dartsim/dart#1505

[scpeters]

I just made branch to see how much new content is on our fork that is missing from master:

• dartsim/dart@master...ignition-forks:scpeters/merge_from_upstream

[chapulina]

DART 6.10.0 has been released and includes some of these changes: https://github.com/dartsim/dart/releases/tag/v6.10.0

As far as I understand, upstream's 6.10 isn't ABI compatible with OSRF's 6.10.

[traversaro]

Related comment by @stefanbuettner: #23 (comment) . From that, it seems that even the API is different?

[traversaro]

Related comment by @stefanbuettner: #23 (comment) . From that, it seems that even the API is different?

Apparently this is being fixed in #249 .

[stefanbuettner]

Great, thanks for the pointers 👍

[peci1]

Are there any guidelines on how to work on the upstreaming? Which branch of DART should be targeted? Does Open robotics want to take care of it all (at once?), or are community efforts welocome?

[chapulina]

Does Open robotics want to take care of it all (at once?), or are community efforts welocome?

We've been low on time to work on this, so we'd very much appreciate help from the community 🙇‍♀️

I think @azeey and @scpeters can talk better about the specific next steps.

[scpeters]

Are there any guidelines on how to work on the upstreaming? Which branch of DART should be targeted? Does Open robotics want to take care of it all (at once?), or are community efforts welocome?

we have one pull request open at dartsim/dart#1437 that is awaiting review. If you are able to contribute a review that may assist their process

[azeey]

dartsim/dart#1437 has been merged! @peci1 If you could help with upstreaming gazebo-forks/dart#22, that would be awesome!

[peci1]

Congrats, is that just a coincidence? Helping with gazebo-forks/dart#22 was my main target when asking for the correct procedure. I'll try to start the PR.

[peci1]

Here we go: dartsim/dart#1626 .

Unfortunately, there was some renaming in upstream DART and I wanted to be consistent with it. This could be problematic in ign-physics - two members of ContactSurfaceParams were renamed: mFrictionCoeff->mPrimaryFrictionCoeff and mSlipCompliance->mPrimarySlipCompliance. So what's the best approach to get it compatible with both ignition dart and upstream dart? Should I post the renaming as a PR to ignition-forks, so that at least the names of the struct members are the same in both versions of DART? That would be technically API breakage, but only in ign-physics plugin internals. The other option is adding some ignition-forks-specific preprocessor define and #ifing around it.

[scpeters]

Here we go: dartsim/dart#1626 .

Unfortunately, there was some renaming in upstream DART and I wanted to be consistent with it. This could be problematic in ign-physics - two members of ContactSurfaceParams were renamed: mFrictionCoeff->mPrimaryFrictionCoeff and mSlipCompliance->mPrimarySlipCompliance. So what's the best approach to get it compatible with both ignition dart and upstream dart? Should I post the renaming as a PR to ignition-forks, so that at least the names of the struct members are the same in both versions of DART? That would be technically API breakage, but only in ign-physics plugin internals. The other option is adding some ignition-forks-specific preprocessor define and #ifing around it.

that dart pull request is targeting 6.13. I think we can handle it with #if macros in ign-physics

[peci1]

dartsim/dart#1626 has landed. So is that all regarding this issue?

[azeey]

Awesome!! The only thing left is making changes to ign-physics to support upstream as well as our fork since some of the API changed during code review.

[peci1]

Ok, I could help with the contact surface part. Just to have a clearer idea about what's gonna happen - what are current plans with DART distribution? From what I got:

• On bionic, the ignition-forks fork distributed via packages.osrfoundation.org will be used reporting version 6.10?
• On focal, upstream version 6.13 will be distributed via packages.osrfoundation.org (or???)
• On jammy, ...?

[Bi0T1N]

Ok, I could help with the contact surface part. Just to have a clearer idea about what's gonna happen - what are current plans with DART distribution? From what I got:

* On bionic, the ignition-forks fork distributed via packages.osrfoundation.org will be used reporting version 6.10?

* On focal, upstream version 6.13 will be distributed via packages.osrfoundation.org (or???)

* On jammy, ...?

To add a reference for a real-world problem: there is this issue (gazebosim/gz-sim#1662) related to an outdated DART 6.9 (Focal) / 6.12 (Jammy) version in the default Ubuntu repository.

[peci1]

My suggestion would be to keep the fork(s) of DART for use with Gazebo, while still trying to upstream as much as possible. The reasoning is that there might be similar cases in the future when Gazebo will need to add/fix something in DART, and waiting for all the roundtrip via DART upstreaming and Ubuntu offering the new version will be years...

So, to allow agile development of features involving DART changes, it would make sense to me to always distribute DART from Gazebo PPAs, even if it's identical to upstream. However, what would need to be changed is to distribute the correct version for each Ubuntu release, so that the one from the PPA will "win" during APT installs over the Ubuntu-distributed one. This probably means having a branch of DART for each supported Ubuntu release.