Skip to content
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

Add convenience function to compute forward kinematics #251

Merged
merged 9 commits into from
Feb 4, 2016

Conversation

DavidB-CMU
Copy link
Contributor

I can add a simple unit test that checks if a specific configuration returns the expected end effector transform?

@DavidB-CMU DavidB-CMU self-assigned this Jan 7, 2016

# Save the robot state
sp = openravepy.Robot.SaveParameters
with robot.CreateRobotStateSaver():
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to only pass the LinkTransformations option here. Otherwise we'll save a bunch of extra state that we don't modify (e.g. joint limits, active DOFs, etc).

@DavidB-CMU
Copy link
Contributor Author

GetForwardKinematics() returns the end-effector transform w.r.t. the "/world" frame.

I'm thinking about adding an optional argument to specify the desired reference frame,
for example to get the transform w.r.t. the base link of the WAM manipulator:

...

# Example:  frame = '/right/wam_base'
from tf import transformations
if frame != None:
    T_ref_frame = robot.GetLink(frame).GetTransform()
    T_ee = transformations.concatenate_matrices(numpy.linalg.inv(T_ref_frame), T_ee)

@mkoval
Copy link
Member

mkoval commented Jan 14, 2016

I noticed that a more general version of this functionality exists in BodyPointsStatesFromJointState. That code optionally supports velocities (derivatives=[0,1]) and accelerations (derivatives=[0,1,2]). It also allows you to query the state of any point, instead of only the end-effector frame. See #166 for some discussion about this API.

Is this sufficient for your needs? Or, if not, it would be nice to implement the new function in terms of the existing ones. Thoughts?

@DavidB-CMU
Copy link
Contributor Author

The GetForwardKinematics() function has 4 lines of code and is very easy to understand. I think we should add it.

I don't think GetForwardKinematics() should wrap a complicated function that is 53 lines and more difficult to understand.


if frame != None:
T_ref_frame = robot.GetLink(frame).GetTransform()
T_ee = transformations.concatenate_matrices(numpy.linalg.inv(T_ref_frame), T_ee)
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to not add a tf dependency for this. Could you use numpy.dot instead?

@mkoval
Copy link
Member

mkoval commented Jan 22, 2016

I am fine with merging this if you: (1) remove the tf dependency, see above, and (2) fix the failed build on Travis.

@DavidB-CMU
Copy link
Contributor Author

  • Removed tf dependency
  • Added an extra unit test
  • Rebased the commits to make Travis happy

mkoval added a commit that referenced this pull request Feb 4, 2016
Add convenience function to compute forward kinematics
@mkoval mkoval merged commit 18f1dc4 into personalrobotics:master Feb 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants