Skip to content
This repository has been archived by the owner on Nov 13, 2017. It is now read-only.

KinematicsBase support for multiple tip frames and IK requests with multiple poses #149

Merged
merged 19 commits into from
Apr 29, 2014
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
f8911d8
KinematicsBase support for multiple tip frames and IK requests with m…
davetcoleman Feb 20, 2014
870fd23
Cleaned up commit
davetcoleman Feb 20, 2014
f5183b4
Feedback and cleaned up comment lengths
davetcoleman Feb 21, 2014
1ebd587
Fixed missing removeSlash to setValues()
davetcoleman Feb 21, 2014
e38970f
Merge branch 'hydro-devel' of https://github.com/ros-planning/moveit_…
davetcoleman Feb 25, 2014
ea77b6b
Merge branch 'hydro-devel' of https://github.com/ros-planning/moveit_…
davetcoleman Feb 26, 2014
1cb4e16
Authorship
davetcoleman Feb 27, 2014
a35382a
fix for reverting PR #148
Feb 25, 2014
6aa705a
Kinematic base minor change
davetcoleman Feb 27, 2014
61e3646
Merge branch 'hydro-devel' into kinematic_base
davetcoleman Feb 28, 2014
09cca0c
Merge branch 'hydro-devel' of https://github.com/ros-planning/moveit_…
davetcoleman Feb 28, 2014
738b8ab
Merge branch 'hydro-devel' into kinematic_base
davetcoleman Feb 28, 2014
c70a3bb
Merge branch 'hydro-devel' of https://github.com/ros-planning/moveit_…
davetcoleman Mar 7, 2014
dcb42ab
Created supportsGroup() test for IK solvers
davetcoleman Mar 7, 2014
cad032e
Merge branch 'hydro-devel' of https://github.com/ros-planning/moveit_…
davetcoleman Mar 18, 2014
d369b53
Changed KinematicsBase::supportsGroup() to use a more standard call s…
Mar 18, 2014
27eb2dc
Merge branch 'hydro-devel' into kinematic_base
davetcoleman Mar 18, 2014
d0c93af
Added support for legacy IK calls without solution_callback
davetcoleman Mar 18, 2014
e74b40b
Merge pull request #3 from ros-planning/supports-group-change
davetcoleman Mar 19, 2014
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 111 additions & 5 deletions kinematics_base/include/moveit/kinematics_base/kinematics_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include <geometry_msgs/PoseStamped.h>
#include <moveit_msgs/MoveItErrorCodes.h>
#include <boost/function.hpp>
#include <console_bridge/console.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this console_bridge/console.h include needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added several logError messages to alert user when kinematic base is attempted to be used as a multi or single tip-frame IK solver when it shouldn't be. See use cases below in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.

#include <string>

/** @brief API for forward and inverse kinematics */
Expand Down Expand Up @@ -178,6 +179,50 @@ class KinematicsBase
moveit_msgs::MoveItErrorCodes &error_code,
const kinematics::KinematicsQueryOptions &options = kinematics::KinematicsQueryOptions()) const = 0;

/**
* @brief Given a set of desired poses for a planning group with multiple end-effectors, search for the joint angles
* required to reach it them. This is useful for e.g. biped robots that need to perform whole-body IK.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo. Also try to limit lines to 120 chars max.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any misspellings here?
The lines I actually edited from the original function descriptions are within 120 characters I believe

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "required to reach it them" mean?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

man it was late, thanks

* Not necessary for most robots that have kinematic chains.
* This particular method is intended for "searching" for a solutions by stepping through the redundancy
* (or other numerical routines).
* @param ik_poses the desired pose of each tip link
* @param ik_seed_state an initial guess solution for the inverse kinematics
* @param timeout The amount of time (in seconds) available to the solver
* @param consistency_limits the distance that any joint in the solution can be from the corresponding joints in the current seed state
* @param solution the solution vector
* @param desired_pose_callback A callback function for the desired link pose - could be used, e.g. to check for collisions for the end-effector
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line needs to go away, there is no such parameter in this function.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Note: those incorrect comments were already there / are in hydro-devel

* @param solution_callback A callback solution for the IK solution
* @param error_code an error code that encodes the reason for failure or success
* @param lock_redundant_joints if setRedundantJoints() was previously called, keep the values of the joints marked as redundant the same as in the seed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lock_redundant_joints does not match any parameters in the function.

The KinematicsQueryOptions parameter is not documented.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the original file, I see that these same function parameter comment problems exist for all the functions. I'll merge this now and fix those in a separate pull request.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I removed them then just added them back when I saw your second comment.

* @return True if a valid solution was found, false otherwise
*/
virtual bool searchPositionIK(const std::vector<geometry_msgs::Pose> &ik_poses,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have index of the actually moving tip in the parameter list.
Like this:

virtual bool searchPositionIK(const std::vector<geometry_msgs::Pose> &ik_poses,
                              int search_tip_index,
                              const std::vector<double> &ik_seed_state,
                              ...);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davetcoleman can correct me, but I believe the idea is that all the tips in the ik_poses vector are supposed to be moving, not just one. The size and indexing of the ik_poses array should match that of the tip_frames vector sent to initialize() or setValues(). If there are more "tips" in the group than in tip_frames, the solver might be programmed to keep those other tips at their same poses.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hersh is correct, the idea is that you can have more than one tip (or end effector) moving at the same time and your IK solver needs to be able to solve for all of them at once.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But now function searchPositionIK is called frommoveit :: core :: RobotState :: setFromIK and only one "tip" at the input does not correspond ik_seed_state. Am I wrong? In the future this will change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request will be merged once KinematicBase is modified, and will fix this issue.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is incorrect, but I want to use this functionality to control collaborative manipulators. I move one 'tip' and others move together, so as not to drop the payload.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this new functionality will enable you to do that - your 'master' tip will be used to calculate the 'slave' tip pose (using reference frame conversions), then you will send both of these poses to the IK solver and it will return the valid joint positions for this non-kinematic chain

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thank you.

const std::vector<double> &ik_seed_state,
double timeout,
const std::vector<double> &consistency_limits,
std::vector<double> &solution,
const IKCallbackFn &solution_callback,
moveit_msgs::MoveItErrorCodes &error_code,
const kinematics::KinematicsQueryOptions &options = kinematics::KinematicsQueryOptions()) const
{
// For IK solvers that do not support multiple poses, fall back to single pose call
if (ik_poses.size() == 1)
{
return searchPositionIK(ik_poses[0],
ik_seed_state,
timeout,
consistency_limits,
solution,
solution_callback,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not always call the version of searchPositionIK() which takes a solution_callback function. Instead it should check if solution_callback is defined; if so, call the version with, if not, call the version without. This fixes a "todo" in pull request #155.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks. Fixed.

error_code,
options);
}

// Otherwise throw error because this function should have been implemented
logError("moveit.kinematics_base: This kinematic solver does not support searchPositionIK with multiple poses");
return false;
}

/**
* @brief Given a set of joint angles and a set of links, compute their pose
* @param link_names A set of links for which FK needs to be computed
Expand All @@ -190,7 +235,7 @@ class KinematicsBase
std::vector<geometry_msgs::Pose> &poses) const = 0;

/**
* @brief Set the parameters for the solver
* @brief Set the parameters for the solver, for use with kinematic chain IK solvers
* @param robot_description This parameter can be used as an identifier for the robot kinematics is computed for; For example, rhe name of the ROS parameter that contains the robot description;
* @param group_name The group for which this solver is being configured
* @param base_frame The base frame in which all input poses are expected.
Expand All @@ -205,7 +250,22 @@ class KinematicsBase
double search_discretization);

/**
* @brief Initialization function for the kinematics
* @brief Set the parameters for the solver, for use with non-chain IK solvers
* @param robot_description This parameter can be used as an identifier for the robot kinematics is computed for; For example, rhe name of the ROS parameter that contains the robot description;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, though this typo is was already there!

* @param group_name The group for which this solver is being configured
* @param base_frame The base frame in which all input poses are expected.
* This may (or may not) be the root frame of the chain that the solver operates on
* @param tip_frames A vector of tips of the kinematic tree
* @param search_discretization The discretization of the search when the solver steps through the redundancy
*/
virtual void setValues(const std::string& robot_description,
const std::string& group_name,
const std::string& base_frame,
const std::vector<std::string>& tip_frames,
double search_discretization);

/**
* @brief Initialization function for the kinematics, for use with kinematic chain IK solvers
* @param robot_description This parameter can be used as an identifier for the robot kinematics is computed for; For example, rhe name of the ROS parameter that contains the robot description;
* @param group_name The group for which this solver is being configured
* @param base_frame The base frame in which all input poses are expected.
Expand All @@ -220,6 +280,36 @@ class KinematicsBase
const std::string& tip_frame,
double search_discretization) = 0;

/**
* @brief Initialization function for the kinematics, for use with non-chain IK solvers
* @param robot_description This parameter can be used as an identifier for the robot kinematics is computed for; For example, rhe name of the ROS parameter that contains the robot description;
* @param group_name The group for which this solver is being configured
* @param base_frame The base frame in which all input poses are expected.
* This may (or may not) be the root frame of the chain that the solver operates on
* @param tip_frames A vector of tips of the kinematic tree
* @param search_discretization The discretization of the search when the solver steps through the redundancy
* @return True if initialization was successful, false otherwise
*/
virtual bool initialize(const std::string& robot_description,
const std::string& group_name,
const std::string& base_frame,
const std::vector<std::string>& tip_frames,
double search_discretization)
{
// For IK solvers that do not support multiple tip frames, fall back to single pose call
if (tip_frames.size() == 1)
{
return initialize(robot_description,
group_name,
base_frame,
tip_frames[0],
search_discretization);
}

logError("moveit.kinematics_base: This kinematic solver does not support initialization with more than one tip frames");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think instead of the error here you can just always set tip_frames_ to the array specified in the argument and not fail.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the default implementation for initialize() with multiple tip frames, and it is intended to be overridden by the inheriting class if that class supports multiple tip frames. This error message exists for backwards compatibility, to allow older IK solvers like IKFast to be able to accept an initialize() function with an array of tips, so long as that array contains only one tip entry.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add a comment about this in the code if you agree.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine.

return false;
}

/**
* @brief Return the name of the group that the solver is operating on
* @return The string name of the group that the solver is operating on
Expand All @@ -240,11 +330,25 @@ class KinematicsBase

/**
* @brief Return the name of the tip frame of the chain on which the solver is operating. This is usually a link name. No namespacing (e.g., no "/" prefix) should be used.
* Deprecated in favor of getTipFrames(), but will remain for foreseeable future for backwards compatibility
* @return The string name of the tip frame of the chain on which the solver is operating
*/
virtual const std::string& getTipFrame() const
{
return tip_frame_;
if (tip_frames_.size() > 1)
logError("moveit.kinematics_base: This kinematic solver has more than one tip frame, do not call getTipFrame()");

return tip_frame_; // for backwards-compatibility. should actually use tip_frames_[0]
}

/**
* @brief Return the names of the tip frames of the kinematic tree on which the solver is operating.
* This is usually a link name. No namespacing (e.g., no "/" prefix) should be used.
* @return The vector of names of the tip frames of the kinematic tree on which the solver is operating
*/
virtual const std::vector<std::string>& getTipFrames() const
{
return tip_frames_;
}

/**
Expand Down Expand Up @@ -319,15 +423,17 @@ class KinematicsBase

KinematicsBase() :
search_discretization_(DEFAULT_SEARCH_DISCRETIZATION),
default_timeout_(DEFAULT_TIMEOUT)
default_timeout_(DEFAULT_TIMEOUT),
tip_frame_("DEPRECATED") // help users understand why this variable might not be set (if multiple tip frames provided, this variable will be unset)
{}

protected:

std::string robot_description_;
std::string group_name_;
std::string base_frame_;
std::string tip_frame_;
std::vector<std::string> tip_frames_;
std::string tip_frame_; // DEPRECATED - this variable only still exists for backwards compatibility with previously generated custom ik solvers like IKFast
double search_discretization_;
double default_timeout_;
std::vector<unsigned int> redundant_joint_indices_;
Expand Down
21 changes: 20 additions & 1 deletion kinematics_base/src/kinematics_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,29 @@ void kinematics::KinematicsBase::setValues(const std::string& robot_description,
robot_description_ = robot_description;
group_name_ = group_name;
base_frame_ = removeSlash(base_frame);
tip_frame_ = removeSlash(tip_frame);
tip_frame_ = removeSlash(tip_frame); // for backwards compatibility
tip_frames_.push_back(removeSlash(tip_frame));
search_discretization_ = search_discretization;
}

void kinematics::KinematicsBase::setValues(const std::string& robot_description,
const std::string& group_name,
const std::string& base_frame,
const std::vector<std::string>& tip_frames,
double search_discretization)
{
robot_description_ = robot_description;
group_name_ = group_name;
base_frame_ = removeSlash(base_frame);
search_discretization_ = search_discretization;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably tip_frames_.clear() here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

// Copy tip frames to local vector after stripping slashes
for (std::size_t i = 0; i < tip_frames.size(); ++i)
tip_frames_.push_back(removeSlash(tip_frames[i]));

// Note that we do not set tip_frame_ variable here, because it should never be used in combination with multiple tip frames

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably set tip_frame_ here if tip_frames.size()==1. (Eventually perhaps we can get rid of the tip_frame_ variable and then this won't matter.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point

}

bool kinematics::KinematicsBase::setRedundantJoints(const std::vector<unsigned int> &redundant_joint_indices)
{
for(std::size_t i = 0; i < redundant_joint_indices.size(); ++i)
Expand Down