Add friction direction unit vector and frame parameters #1427

scpeters · 2019-11-26T05:22:06Z

Currently the ContactConstraint uses a friction pyramid with 2 directions. In #1424, we added an additional parameter to allow the friction coefficient to be specified separately in each direction. This pull request allows the friction direction to be specified with a unit vector FirstFrictionDirection and pointer to a frame FirstFrictionDirectionFrame in which the unit vector is expressed. If FirstFrictionDirection is a vector of zeros (which is its default value), it is ignored and the default friction directions are used. If the frame pointer is nullptr (which is its default value), the body-fixed frame of that ShapeNode is used.

I've added two bodies to test_Friction with friction direction set using the default body-fixed frame and also using the world frame. In these tests, the friction is set to zero along one direction, and the lateral gravity component pushes along a diagonal.

Before creating a pull request

Document new methods and classes
Format new code files using clang-format

Before merging a pull request

Set version target by selecting a milestone on the right side
Summarize this change in CHANGELOG.md
Add unit test(s) for this change

scpeters · 2019-11-26T05:22:53Z

cc @azeey

codecov · 2019-11-26T06:02:09Z

Codecov Report

Merging #1427 into master will increase coverage by <.01%.
The diff coverage is 78%.

@@            Coverage Diff             @@
##           master    #1427      +/-   ##
==========================================
+ Coverage   57.99%   57.99%   +<.01%     
==========================================
  Files         412      412              
  Lines       29832    29861      +29     
==========================================
+ Hits        17302    17319      +17     
- Misses      12530    12542      +12

Impacted Files	Coverage Δ
dart/constraint/ContactConstraint.hpp	`100% <ø> (ø)`	⬆️
dart/dynamics/detail/ShapeFrameAspect.hpp	`88.88% <ø> (ø)`	⬆️
dart/dynamics/ShapeFrame.hpp	`83.33% <100%> (+1.51%)`	⬆️
dart/constraint/ContactConstraint.cpp	`68.89% <75.6%> (-0.43%)`	⬇️
dart/dynamics/ShapeFrame.cpp	`72.22% <87.5%> (+1.06%)`	⬆️
dart/dynamics/Skeleton.cpp	`66.16% <0%> (-0.17%)`	⬇️

scpeters · 2019-12-02T19:41:32Z

@mxgrey and @jslee02 we are getting close to a release of ignition citadel, and we'd really like to get these changes from our fork into dartsim

there is another pull request scheduled to come after this too that adds slip parameters

any time you could spend on a review would be greatly appreciated

jslee02

Thank you for this set of changes and sorry for taking so long to review this PR. Overall, it looks good to me. I left several comments.

dart/constraint/ContactConstraint.cpp

jslee02 · 2019-12-02T20:26:18Z

dart/constraint/ContactConstraint.cpp

+  // The first column is the same as mFirstFrictionalDirection unless
+  // mFirstFrictionalDirection is parallel to the normal
+  T.col(0) = Eigen::Quaterniond(Eigen::AngleAxisd(0.5_pi, n)) * tangent;
+  T.col(1) = tangent;


It's not the part of this change, but I would like to negate the direction of tangent so that T.col(0).cross(T.col(1)) == n (currently, it's T.col(1).cross(T.col(0)) == n) because I believe it'd be more intuitive. This change wouldn't affect the behavior (but just flipping the sign of impulse for the second tangential friction).

It can be done by changing the order of all the cross products in this function. For example, line 813 should be changed to:

Eigen::Vector3d tangent = n.cross(mFirstFrictionalDirection);

or simply negating the final tangent. I'd prefer changing the order of the cross products though.

I'm not opposed to that, but I'll bring @azeey into the conversation since he made this specific change

Negating the direction of tangent sounds fine to me if it doesn't affect the behavior. I think the main change here is that T.col(0) has to be parallel to the first frictional direction.

reversed in 5fe40b7

jslee02 · 2019-12-02T20:28:39Z

dart/dynamics/ShapeFrame.cpp

@@ -57,7 +57,9 @@ DynamicsAspectProperties::DynamicsAspectProperties(
    const double frictionCoeff, const double restitutionCoeff)
  : mFrictionCoeff(frictionCoeff),
    mRestitutionCoeff(restitutionCoeff),
-    mSecondaryFrictionCoeff(frictionCoeff)
+    mSecondaryFrictionCoeff(frictionCoeff),
+    mFirstFrictionDirection(Eigen::Vector3d::Zero()),


Currently, the first friction direction will be set to z-direction in world coordinates (unless it's parallel to the contact normal). Maybe we want to set the default direction to non-zero direction (e.g., Eigen::Vector3d::UnitZ()) so that the default behavior is more consistent in terms of the friction direction?

my intent was that an unset vector with zero length would use the global friction direction, and that only friction directions that are explicitly set to have a nonzero length would use their custom direction and frame. See the following logic in ContactConstraint.cpp

dart/dart/constraint/ContactConstraint.cpp

Lines 148 to 172 in a932d87

// only consider custom friction direction if one has nonzero length

if (nonzeroDirA || nonzeroDirB)

{

// if A and B are both set, choose one with smaller friction coefficient

// since it's friction properties will dominate

if (nonzeroDirA && nonzeroDirB)

{

if (primaryFrictionCoeffA <= primaryFrictionCoeffB)

{

mFirstFrictionalDirection = frictionDirA.normalized();

}

else

{

mFirstFrictionalDirection = frictionDirB.normalized();

}

}

else if (nonzeroDirA)

{

mFirstFrictionalDirection = frictionDirA.normalized();

}

else

{

mFirstFrictionalDirection = frictionDirB.normalized();

}

}

does that make sense?

Yeah, I read the logic but was suggesting changing the default behavior to using local friction direction (by setting mFirstFrictionDirection to non-zero vector intentionally) because first/second friction coefficients would make more sense when they're aligned with local directions (if they have different values).

Downside of it is that it changes the default simulation behavior (from using global friction direction to local friction direction) though. I'm okay to leave as it is as long as the logic is documented.

a tricky aspect of the friction directions is choosing which to use when custom directions are set for both bodies. I've added logic in this PR to choose the direction from the body with the smaller primary friction coefficient, since that would be the dominant parameter, which makes sense to me, but I haven't tested it, and I'd be wary of making a big behavior change like that without testing.

Is the logic sufficiently documented? If not, I'll add the necessary documentation.

Agreed to not changing the default behavior.

It's well documented in the implementation, but it'd be great if it's also documented in the header as well (maybe on the constructor?).

how does cddef7a look to you?

Looks awesome! Thank you for adding it.

Co-Authored-By: Jeongseok Lee <[email protected]>

scpeters and others added 4 commits November 21, 2019 18:27

Add friction direction vector and frame parameters

ed3a488

compute body-fixed friction directions

d1d8073

Fix handling of first frictional direction

0195b7a

test_Friction: test friction direction parameters

843b40b

scpeters requested review from jslee02 and mxgrey November 26, 2019 05:22

jslee02 requested changes Dec 2, 2019

View reviewed changes

scpeters and others added 5 commits December 3, 2019 17:29

ContactConstraint.cpp use const &

ed85373

Co-Authored-By: Jeongseok Lee <[email protected]>

ContactConstraint.cpp: print transpose

1962ef3

Co-Authored-By: Jeongseok Lee <[email protected]>

ContactConstraint.cpp: update comment

a932d87

ContactConstraint: reverse tangent direction

5fe40b7

add documentation of friction direction params

cddef7a

scpeters added this to the DART 6.10.0 milestone Dec 6, 2019

Jeongseok Lee added 3 commits December 6, 2019 12:36

Format code

b3438cb

Update changelog

691fa12

Fix typo

d2b9e0d

jslee02 approved these changes Dec 6, 2019

View reviewed changes

jslee02 merged commit 31a8a0f into dartsim:master Dec 7, 2019

scpeters deleted the friction_direction branch December 7, 2019 08:38

diegoferigo mentioned this pull request Jul 21, 2020

Understand the role of mu and mu2 parameters robotology/gym-ignition-models#23

Closed

scpeters mentioned this pull request Jan 15, 2021

Custom friction directions not implemented #141

Closed

scpeters mentioned this pull request Jan 23, 2021

Demo for edifice: mecanum wheels gazebosim/gz-sim#579

Closed

scpeters mentioned this pull request Dec 10, 2022

Support for Omni-wheels missing? bulletphysics/bullet3#2849

Closed

azeey mentioned this pull request Jun 13, 2023

Tracked vehicles go backwards. gazebosim/gz-sim#2008

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add friction direction unit vector and frame parameters #1427

Add friction direction unit vector and frame parameters #1427

scpeters commented Nov 26, 2019 •

edited by jslee02

Loading

scpeters commented Nov 26, 2019

codecov bot commented Nov 26, 2019 •

edited

Loading

scpeters commented Dec 2, 2019

jslee02 left a comment

jslee02 Dec 2, 2019

scpeters Dec 4, 2019

azeey Dec 5, 2019

scpeters Dec 6, 2019

jslee02 Dec 2, 2019 •

edited

Loading

scpeters Dec 4, 2019

jslee02 Dec 4, 2019

scpeters Dec 4, 2019

jslee02 Dec 6, 2019

scpeters Dec 6, 2019

jslee02 Dec 6, 2019

	// only consider custom friction direction if one has nonzero length
	if (nonzeroDirA \|\| nonzeroDirB)
	{
	// if A and B are both set, choose one with smaller friction coefficient
	// since it's friction properties will dominate
	if (nonzeroDirA && nonzeroDirB)
	{
	if (primaryFrictionCoeffA <= primaryFrictionCoeffB)
	{
	mFirstFrictionalDirection = frictionDirA.normalized();
	}
	else
	{
	mFirstFrictionalDirection = frictionDirB.normalized();
	}
	}
	else if (nonzeroDirA)
	{
	mFirstFrictionalDirection = frictionDirA.normalized();
	}
	else
	{
	mFirstFrictionalDirection = frictionDirB.normalized();
	}
	}

Add friction direction unit vector and frame parameters #1427

Add friction direction unit vector and frame parameters #1427

Conversation

scpeters commented Nov 26, 2019 • edited by jslee02 Loading

scpeters commented Nov 26, 2019

codecov bot commented Nov 26, 2019 • edited Loading

Codecov Report

scpeters commented Dec 2, 2019

jslee02 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jslee02 Dec 2, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scpeters commented Nov 26, 2019 •

edited by jslee02

Loading

codecov bot commented Nov 26, 2019 •

edited

Loading

jslee02 Dec 2, 2019 •

edited

Loading