-
Notifications
You must be signed in to change notification settings - Fork 38
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
Implement FeasibleContactWrenchTask for TSID component #369
Conversation
* Enable the task. I.e. a contact wrench different from zero is allowed. | ||
*/ | ||
void enable(); | ||
|
||
/** | ||
* Disable the task. I.e. the contact wrench is forced to be equal to zero. | ||
*/ | ||
void disable(); |
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.
There is some potential confusion on whether it is the task to be disabled (hence the wrench is not constrained), or if the wrench cannot be used to control the robot. A possibility could be to allow setting directly the maximum normal component. Another possibility could be to have a method like setContactState
with an enum. Otherwise also a different naming could do the trick, although I did not find any good alternatives myself.
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 like setContactState
! the enum could be Active
Inactive
, what do you think?
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.
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.
That would be nice. On the other hand, we should try to be coherent and use that enum also inside the class. Right now it is a boolean https://github.com/dic-iit/bipedal-locomotion-framework/blob/d81b390683127a451669b0e09ff06da199f9490a/src/Contacts/include/BipedalLocomotion/Contacts/Contact.h#L102
and only in the EstimatedContact
.
For the time being, we could simply add the boolean and start using it in separate PRs.
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.
In 9b8662b I implemented the function we discussed.
Co-authored-by: Stefano Dafarra <stefano.dafarra@gmail.com>
@S-Dafarra let mIf I can merge the PR 😃 |
* Disable the task. I.e. the contact wrench is forced to be equal to zero. | ||
*/ | ||
void disable(); | ||
void isContactActive(bool isActive); |
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 realized later that isContactActive
is more in the form of a question, so I would expect a return value. Maybe setContactActive
would suggest more that you are changing an internal state. Does it make sense?
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.
Fixed in 1277a62
c59fc2a
to
1277a62
Compare
1277a62
to
7ffb525
Compare
Most of the jobs failed, did you check the output? |
This PR implements a task for the TSID component ensuring a feasible contract wrench