Tracked vehicles go backwards.

[rodericktaylor]

Environment

• OS Version: Ubuntu 22.04
• Source or binary build?
  Source 3bde4b5 (gz-sim7 branch on garden)

Description

• Expected: tracked_vehicle_simple.sdf, pressing W should make the robot go forward, pressing D should make the robot turn clockwise.
• Actual: pressing W makes the robot go backwards and pressing D makes the robot go anti-clockwise.

The TrackedVehicle plugin does not take into account that TrackController's moving in the positive direction make the part of the track that touches ground go the opposite direction.

Steps to reproduce

1. gz sim -r tracked_vehicle_simple.sdf
2. Press W, then press S to stop. The robot moves backwards.
3. Press D, then press S to stop. The robot turns anti-clockwise.

[Bi0T1N]

I've an environment setup as written in #1662 for Fortress (@jrutgeer for Garden) and there it works as expected. W moves the robot forward and D moves it clockwise. Thus this must be a recent regression in gz-sim or DART if it is reproducible by others.

[azeey]

I was able to reproduce this. It looks like there's a sign convention mismatch between our fork of DART and upstream. @peci1 Do you recall this being the case?

[jrutgeer]

I can confirm it is inversed in Garden + Dart 6.13, i.e. W moves as indicated in the image below.

@Bi0T1N Be aware that the 3D model is (imo) confusing: the top part is shifted to the back, which can easily be mistaken for the front:

It is also inversed in the conveyor demo:

[peci1]

I have no idea why this is happening.

Could you compare the contents of params in

gz-sim/include/gz/sim/physics/Events.hh

Line 47 in a65c4c9

using CollectContactSurfaceProperties = gz::common::EventT<

in Fortress and Garden? I.e. to figure out whether the problem lies on Gazebo side or DART side?

[azeey]

I'm using just Garden but with DART 6.10 (fork) vs DART 6.13.0 from upstream.

Here's the print out from setting <debug>1</debug> on one of the TrackController plugins and pressing W:

With DART 6.13

[Dbg] [TrackController.cc:512] Link: left_track
[Dbg] [TrackController.cc:513] - is collision 1 track 1
[Dbg] [TrackController.cc:515] - velocity cmd         1
[Dbg] [TrackController.cc:516] - limited velocity cmd 1
[Dbg] [TrackController.cc:517] - friction direction   -1 1.5088e-11 0
[Dbg] [TrackController.cc:518] - surface motion       -1
[Dbg] [TrackController.cc:519] - contact point             2.58091
    0.247139
-0.000164322
[Dbg] [TrackController.cc:520] - contact normal       0 0 1
[Dbg] [TrackController.cc:521] - track rot            -1.66882e-08 -9.13289e-07 -1.50728e-11
[Dbg] [TrackController.cc:522] - track Y              1.5088e-11 1 -1.66882e-08
[Dbg] [TrackController.cc:523] - belt direction       -1 1.5088e-11 0

With DART 6.10 from our fork

[Dbg] [TrackController.cc:512] Link: left_track
[Dbg] [TrackController.cc:513] - is collision 1 track 1
[Dbg] [TrackController.cc:515] - velocity cmd         1
[Dbg] [TrackController.cc:516] - limited velocity cmd 1
[Dbg] [TrackController.cc:517] - friction direction   -1 -1.50255e-11 0
[Dbg] [TrackController.cc:518] - surface motion       -1
[Dbg] [TrackController.cc:519] - contact point             3.25509
    0.247139
-3.71957e-05
[Dbg] [TrackController.cc:520] - contact normal       0 0 1
[Dbg] [TrackController.cc:521] - track rot            -3.08715e-08 -1.52132e-06 1.50724e-11
[Dbg] [TrackController.cc:522] - track Y              -1.50255e-11 1 -3.08715e-08
[Dbg] [TrackController.cc:523] - belt direction       -1 -1.50255e-11 0

They look very similar to me, so I'm inclined to think it's a difference in implementation between our fork of DART and upstream.

[azeey]

I've tracked this down to the way the TangentBasisMatrix is calculated in DART. When we upstreamed our changes in dartsim/dart#1427, there was a review comment that caused us to flip the direction of the tangent. At the time, the change didn't seem to break any behavior, so we didn't think to sync our fork with the upstream version.

@peci1 Do you think we can simply negate the sign of the contactSurfaceMotionVelocity vector in gz-physics for DART versions 6.13.0 and later?

[peci1]

Good investigation. It seems negating the velocity vector in gz-physics would be the cleanest way.

The only thing I fear is whether it will be sufficient to distinguish the fork and upstream based on the version number. I remember times where there were the same versions upstream and in the fork with different behaviors.

[Bi0T1N]

I've updated my Fortress version (still compiled against same DART PPA) to the new release from 2023-05-31 and the buttons are inverted now. I'm pretty sure this wasn't the case before (working on different robots, not the ones from the Gazebo example) but can't easily verify it...

[rodericktaylor]

I'm building from source. Is the fork available for me to include in my build?

In the interest of accelerating a solution, may I suggest you define something like. #define DART_HAS_POSITIVE_SURFACE_MOTION_VELOCITY 1. and negate based on #ifndef instead of checking version numbers. When or if your fork is not used, the code will still work... forever.

[jrutgeer]

@rodericktaylor

I'm building from source. Is the fork available for me to include in my build?

Refer to #1662 (comment)

[azeey]

The only thing I fear is whether it will be sufficient to distinguish the fork and upstream based on the version number. I remember times where there were the same versions upstream and in the fork with different behaviors.

Our fork is currently at 6.10, so we can easily distinguish between the two. If we ever update our fork to be >= 6.13, we'll need to change the TangentBasisMatrix to match what's in upstream. In addition to opening an issue on our fork, we should add a test in gz-physics to make sure we don't forget.

In the interest of accelerating a solution, may I suggest you define something like. #define DART_HAS_POSITIVE_SURFACE_MOTION_VELOCITY 1. and negate based on #ifndef instead of checking version numbers. When or if your fork is not used, the code will still work... forever.

Where are you suggesting we put this #define? If we put it in our fork, and update gz-physics to check for it, I'm concerned that users that aren't following the developments in our fork will have a silent change of behavior. We already make assumptions that >=6.13 is upstream, so I think fixing it that way is our best option. /cc @scpeters

[rodericktaylor]

This looks like it is fixed in the harmonic release when compiled from source with DART 6.13. Ta heaps!

[azeey]

Yes, I closed #1662, but forgot to close this.