-
Notifications
You must be signed in to change notification settings - Fork 41
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
Mimic constraint feature #431
Conversation
Signed-off-by: Aditya <[email protected]>
Signed-off-by: Aditya <[email protected]>
Signed-off-by: Aditya <[email protected]>
Signed-off-by: Aditya <[email protected]>
Signed-off-by: Aditya <[email protected]>
Signed-off-by: Aditya <[email protected]>
Signed-off-by: Aditya <[email protected]>
Codecov Report
@@ Coverage Diff @@
## gz-physics6 #431 +/- ##
===============================================
+ Coverage 76.18% 76.24% +0.06%
===============================================
Files 142 142
Lines 7251 7279 +28
===============================================
+ Hits 5524 5550 +26
- Misses 1727 1729 +2
|
Signed-off-by: Aditya <[email protected]>
Signed-off-by: Aditya <[email protected]>
Signed-off-by: Aditya <[email protected]>
TODO ChecklistChecklist for tests (parent - child joint) :
Constructing using sdf - will be done in gz-sim now:
|
Signed-off-by: Aditya <[email protected]>
Signed-off-by: Aditya <[email protected]>
Signed-off-by: Aditya <[email protected]>
Signed-off-by: Aditya <[email protected]>
Signed-off-by: Aditya <[email protected]>
Based on an offline discussion with @azeey , applying to mimic constraint using the sdf tags would be done in the Physics system inside |
Signed-off-by: Aditya <[email protected]>
17a078c
to
1fbeed2
Compare
include/gz/physics/Joint.hh
Outdated
public: using Scalar = typename PolicyT::Scalar; | ||
|
||
public: void SetMimicConstraint( | ||
const std::string _mimicJoint, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's add the axis name string here to match the SDFormat API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 6a0294b
include/gz/physics/Joint.hh
Outdated
{ | ||
/// \brief The Joint API for setting the mimic joint constraint. | ||
/// \param[in] _mimicJoint | ||
/// name of the joint to be mimicked, i.e. the parent joint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use Mimic constraint instead of joint and follower / leader terminology
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 0b4f490
pinging @azeey for an opinion: should we add support for the Mimic constraints to the |
…straint() Signed-off-by: Aditya Pande <[email protected]>
Signed-off-by: Aditya Pande <[email protected]>
{ | ||
public: using Scalar = typename PolicyT::Scalar; | ||
|
||
public: void SetMimicConstraint( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should add a std::size_t _dof
argument (similar to the GetBasicJointState APIs) to specify the follower joint axis
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include/gz/physics/Joint.hh
Outdated
/// \param[in] _joint | ||
/// name of the joint to be mimicked, i.e. the leader joint. | ||
/// \param[in] _multiplier | ||
/// The multiplier to be applied to poistion of leader joint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// The multiplier to be applied to poistion of leader joint. | |
/// The multiplier to be applied to position of leader joint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
// Testing with different (multiplier, offset, reference) combinations. | ||
testMimicFcn(1, 0, 0); | ||
testMimicFcn(-1, 0, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
calling testMimicFcn
sequentially results in multiple calls to SetMimicConstraint
for a given joint, which appears to work with the dartsim implementation in this PR, but it causes issues with the bullet-featherstone implementation in #517 that adds a new constraint for each call to SetMimicConstraint
. Probably I need to update #517 to clear any existing mimic constraints when SetMimicConstraint
is called. We should document in the Joint.hh
header what the expected behavior is for multiple subsequent calls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated #517 to clear existing mimic constraints. I made some adjustments to the test expectations, but I've resolved the concern in this comment
Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
include/gz/physics/Joint.hh
Outdated
public: void SetMimicConstraint( | ||
const std::size_t _dof, | ||
const std::string _joint, | ||
const std::string _axis, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use const references for _joint
and _axis
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: Steve Peters <[email protected]>
I've retargeted #517 directly at |
closing until dartsim/dart#1756 is addressed |
🎉 New feature
Summary
This PR adds a
SetMimicConstraint(std::string joint_name, double multiplier, double offset )
feature to joints, and is currently implemented when using dartsim.Test it
Added a bunch of test cases to
joint_features.cc
, between different joint combinations and offsets, multipliers.Pendulum test :
Checklist
codecheck
passed (See contributing)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.🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸