-
Notifications
You must be signed in to change notification settings - Fork 48
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 External Wrenches in Gazebo #293
Multiple External Wrenches in Gazebo #293
Conversation
@traversaro I finished testing and everything runs smooth. Please review it(especially handling dynamically created objects) and let me know if something is wrong. Thank you |
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.
Sorry for the late review!
this->m_reply.addString ( "[force]: (double x, y, z) Force components in N w.r.t. world reference frame" ); | ||
this->m_reply.addString ( "[torque]: (double x, y, z) Torque components in N.m w.r.t world reference frame" ); | ||
this->m_reply.addString ( "[duration]: (double) Duration of the applied force in seconds" ); | ||
this->m_reply.addString ( "Note: The reference frame is the base/root robot frame with x pointing backwards and z upwards."); |
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 orientation of the base frame robot frame seems to be something model-specific, that you can't know in advance. Furthermore, the documentation for the force and torque vector seem to suggest that (at least for the orientation part) force and torque are expressed with respect to the world
frame, rather then the base model frame.
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.
@traversaro I agree with you, the reference is gazebo world frame. Also a better additional note, in my opinion, is to indicate that external wrench is applied at the center of mass of the link(not at the link frame).
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.
Perfect, feel free to update the docs.
|
||
//yInfo() << "Creating new instance of external wrench"; | ||
//Creating new instances of external wrenches | ||
//newWrench = std::unique_ptr<ExternalWrench>(new ExternalWrench()); |
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.
Leftover?
{ | ||
wrenchesVectorPtr->push_back(newWrench); | ||
} | ||
else yError() << "Failed to set new wrech values!"; |
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.
Typo: wrech
--> wrench
.
m_rpcPort.read ( command,true ); | ||
if(command.get(0).asString() == "help") | ||
{ | ||
this->m_reply.addVocab ( yarp::os::Vocab::encode ( "many" ) ); |
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.
This help seems to be equivalent to the one of the externalwrench
plugin. Is this intended?
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.
@traversaro yes, if needed a line can be added stating this plugin lets the user apply multiple wrenches. The limitation of externalWrench plugin is that user cannot apply a wrench until a previously applied wrench time duration has finished
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 see. If this is the case and this plugin just adds a feature in a backward compatible way, perhaps we can just merge this plugin in the externalwrench
one instead of having two plugins that do almost the same thing?
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.
@traversaro sure, I will make the changes needed in externalWrench
plugin and open a PR
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.
Perfect!
@Yeshasvitvs Did you see my comments on this PR? |
@traversaro sorry missed the update on comments, will check soon and resolve |
Closing as discussed in #293 (comment) . |
Currently "externalwrench" plugin lets user to issue only one wrench at a time. This new plugin can be used to issue multiple wrench commands with varying durations. The visualization for each wrench is colored randomly.
@traversaro I am doing some more tests and hopefully we can close this PR by tomorrow. Thank you