Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple frictional properties per BodyNode #200

Closed
jslee02 opened this issue Jun 24, 2014 · 6 comments · Fixed by #1369
Closed

Multiple frictional properties per BodyNode #200

jslee02 opened this issue Jun 24, 2014 · 6 comments · Fixed by #1369

Comments

@jslee02
Copy link
Member

jslee02 commented Jun 24, 2014

BodyNode stores one friction coefficient for the all collision shapes in the BodyNode. Multiple friction coefficient per body would be useful in case of a body consist of different materials.

One possible way to do this is that stores the frictional properties in collision object. Currently, DART uses Shape classes for both of collision object and visualization object so we may need to consider split the class.

In addition, multiple frictional directions should be considered too.

@jslee02 jslee02 added this to the Release DART 5.0 milestone Jun 24, 2014
@jslee02 jslee02 modified the milestones: Release DART (15.06), Release DART 4.3 Jan 2, 2015
@jslee02 jslee02 removed this from the Release DART 5.0 milestone Mar 22, 2015
@stale stale bot added the stale label Feb 13, 2018
@stale stale bot removed the stale label Feb 13, 2018
@scpeters
Copy link
Collaborator

It looks like each ShapeNode with a DynamicsAspect now has its own friction properties, but the ContactConstraint doesn't use them. It only seems to use the friction parameters stored in the body node. To demonstrate this, I modified the hello_world example in scpeters@dbdcdec to have a floor with inclined gravity, so that when setting the box ShapeNode friction to 0.0, it should slide along the floor. It doesn't slide though, unless you modify the ContactConstraint (as in scpeters@f8b5378).

@scpeters
Copy link
Collaborator

ShapeNode friction coefficient defined in detail/ShapeFrameAspect.hpp

@mxgrey
Copy link
Member

mxgrey commented Jul 10, 2019

Thanks, that seems like a bug. Feel free to open a PR.

I wonder how we should handle the full transition from BodyNode-specified friction to Shape-specified friction? We shouldn't have the BodyNode and ShapeNodes both store friction data, because they'll easily conflict with each other.

It would be easy enough to change BodyNode::setFrictionCoeff(~) so that it just changes the friction coefficient of all shapes currently in the BodyNode. However that won't help for new shapes that could get added later.

Also, BodyNode::getFrictionCoeff() doesn't make very much sense if friction information is stored per ShapeNode. At best we could return some kind of average friction?

I guess both of those functions should be deprecated and we should recommend to users that they focus on setting friction per ShapeNode.

@dartsim dartsim deleted a comment from stale bot Jul 11, 2019
@jslee02
Copy link
Member Author

jslee02 commented Jul 11, 2019

Alternatively, we could introduce a friction coefficient mode to ShapeNode to decide which coefficient should be used. The mode would have several modes something like INHERIT, OVERRIDE, BLEND, MIN, and MAX. In this way, newly added shapes could use the same coefficient if the mode is INHERIT, and we could use INHERIT mode as the default not to break backward compatibility.

Here (d912dc7) is a quick implementation based on @scpeters's example.

@jslee02
Copy link
Member Author

jslee02 commented Jul 12, 2019

On second thought, having the friction property in both of BodyNode and DynamicAspect wouldn't have many benefits but introduces confusions. So just deprecated them from BodyNode and changed the constraint solver to use of DynamicAspect in #1369. Also, applied this to the restitution coefficient as well.

@scpeters
Copy link
Collaborator

yeah, it's simpler to just have the parameters in the ShapeNode, rather than also being in the BodyNode

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants