-
Notifications
You must be signed in to change notification settings - Fork 111
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
Allow setting joint values for end-effector marker #21
Allow setting joint values for end-effector marker #21
Conversation
currently end-effector markers are based on the default robot state (ie all zero), this allows passing a vector of doubles for all active joint in the end-effector group Passing a new set of joint values will invalidate the cache but for our use-case this is neglectible and could be optimized later
Looks like the CI is broken, attempting fix: |
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.
LGTM
* \return true if it is successful | ||
*/ | ||
bool loadEEMarker(const robot_model::JointModelGroup* ee_jmg); | ||
bool loadEEMarker(const robot_model::JointModelGroup* ee_jmg, const std::vector<double>& ee_joint_pos); |
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.
Would you like to have a default value here? just noticed that I changed a public api in a non backwards compatible manner here. Would you like a default value " const std::vector& ee_joint_pos = {} " here?
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.
ping?
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.
yes please, or provide two versions of the function
CI is unfortunately still broken, but it should be API compatible again now |
@davetcoleman anything else I should do? |
The Travis issue seems very unrelated to this PR, will merge:
|
Thanks, yes, those travis errors come from rviz visual tools instead |
@davetcoleman could you please forward port/merge this to melodic? |
* \param color to display the collision object with | ||
* \return true on success | ||
*/ | ||
bool publishEEMarkers(const Eigen::Affine3d& pose, const robot_model::JointModelGroup* ee_jmg, | ||
const std::vector<double>& ee_joint_pos, | ||
const rviz_visual_tools::colors& color = rviz_visual_tools::CLEAR, |
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.
@simonschmeisser why did you choose CLEAR here? I just debugged why moveit_grasps didn't visualize animated grasps anymore, and it took a long time for me to realize alpha was set to 0 by default! I'm going to make the default something else...
} | ||
bool publishEEMarkers(const geometry_msgs::Pose& pose, const robot_model::JointModelGroup* ee_jmg, | ||
const std::vector<double>& ee_joint_pos, | ||
const rviz_visual_tools::colors& color = rviz_visual_tools::CLEAR, |
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.
@davetcoleman wasn't me 😜 I just copy pasted this from 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.
hmmm, my bad!
currently end-effector markers are based on the default robot state (ie all zero),
this allows passing a vector of doubles for all active joint in the end-effector group
Passing a new set of joint values will invalidate the cache but for our use-case this is
neglectible and could be optimized later
implements #18