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

RigidBodySystem::spatialForceInFrameToJointTorque() Needs to be Moved to RigidBodyTree. #2323

Closed
liangfok opened this issue May 11, 2016 · 9 comments

Comments

@liangfok
Copy link
Contributor

See the following review comment for context:

https://reviewable.io/reviews/robotlocomotion/drake/2303#-KHROOYoXMVoJcMHqdJA

@liangfok
Copy link
Contributor Author

liangfok commented May 11, 2016

Also consider renaming the method to be ComputeGeneralizedJointForcesFromWrenchOnRigidBodyFrame().

or:

ConvertSpatialForceToGeneralizedForces().

See: https://reviewable.io/reviews/robotlocomotion/drake/2303#-KHRMPUfS_OpOwvf4itk

@amcastro-tri
Copy link
Contributor

Ok, I can take this one. So I assume your plan is then to leave spatialForceInFrameToJointTorque in RigidBodySystem.h as it was originally? will you revert that change in #2303?

@liangfok
Copy link
Contributor Author

liangfok commented May 12, 2016

I moved the implementation of spatialForceInFrameToJointTorque() from the .h file to the .cpp file. Since I believe this is an improvement, would it be OK to keep that refactor in #2303? If not, I would be happy to revert that change as well.

@amcastro-tri
Copy link
Contributor

You mean because it would make the build faster? in that case I agree

@liangfok
Copy link
Contributor Author

Yes, to make the build faster and de-clutter the .h file. OK I will keep the refactor in #2303.

@amcastro-tri
Copy link
Contributor

sounds good.

@sherm1
Copy link
Member

sherm1 commented May 12, 2016

Copied from reviewable for preservation:

@amcastro-tri:

We need proper Doxygen documentation here. My try:
"Given a wrench described in the given RigidBodyFrame frame, return the (scalar) generalized force at each joint required to achieve this wrench".
@sherm1. What do you think?, because if a joint is prismatic (and if I understand correctly) the value of tau will be that of the (scalar) force along the axis of action. Therefore, the name of this method is misleading. It should contain the term "generalized force", not "torque".
My try: rename to ComputeGeneralizedJointForcesFromWrenchOnRigidBodyFrame().

@sherm1:

It looks like Drake is using the term "spatial force" rather than "wrench". Those are synonyms. Usually I would prefer the shorter term, but this terminology has the advantage that it can be used for other "spatial vector" (6 element) quantities like spatial velocity and spatial acceleration. We ought to pick a terminology and stick with it.

Also, I would reword the proposed description a little. An applied spatial force (or wrench) produces a set of generalized forces at the joints that will produce the same motion. But generalized forces cannot be said to "achieve" that wrench; it just achieves the same acceleration. Also, a single joint may have several generalized forces (up to 6). So how about: "Given a spatial force (wrench) applied at the origin of the given frame, and expressed in that frame, return the vector of generalized forces induced by the spatial force."

Could be ConvertSpatialForceToGeneralizedForces(). I'm not sure the "OnFrame" part is adding anything because a spatial force must inherently be applied to a point (for the force half), and must be expressed in a frame (since both force and torque components are vectors). The method is a little odd because it combines the application point and expressed-in frame. Usually spatial force are most naturally computed in the World frame and then applied to some point on a body.

@amcastro-tri
Copy link
Contributor

That's great! thank you @sherm1. It would've been difficult to find that again

@jwnimmer-tri
Copy link
Collaborator

I assume this issue is WONTFIX. Please reopen in case not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants