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

Document Pose::operator*() #170

Merged
merged 2 commits into from
Nov 4, 2020

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Oct 23, 2020

Adds some documentation for Pose::operator*(). If I understand correctly, it looks like B + A is the same as A * B, but operator+ is discouraged.

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@chapulina
Copy link
Contributor

How about adding a couple of illustrative test cases? I don't see that operator being explicitly tested here:

https://github.com/ignitionrobotics/ign-math/blob/main/src/Pose_TEST.cc

Maybe it's tested indirectly from somewhere else.

@chapulina chapulina added this to Inbox in Core development via automation Oct 23, 2020
@scpeters
Copy link
Member

for the life of me, I have the toughest time verbalizing how these transforms work; I'm sure @azeey remembers how much trouble I had when we were writing the following SDFormat tutorial:

I'll let him speak up if he has any concrete suggestions for syntax

@mjcarroll mjcarroll moved this from Inbox to In review in Core development Oct 26, 2020
@azeey
Copy link
Contributor

azeey commented Oct 27, 2020

Thanks for adding the documentation @sloretz. In most robotics books (eg. http://hades.mech.northwestern.edu/images/2/25/MR-v2.pdf), the * operator is used for composing transforms rather than +. The notation given in these books uses two frames to describe a transform/pose. In http://sdformat.org/tutorials?tut=specify_pose&cat=specification&, we have used X_BA to indicate the transform from B to A. This is also the same quantity as the pose of frame A (relative to and expressed in) frame B.

Given two Pose objects, the order of arguments when using Pose::operator* is the same as if the Poses were first converted to 4x4 homogeneous matrices and the product of the matrices was taken. So if we have frames O, P, and Q and we have the poses X_OP (frame P relative to O) and X_PQ (frame Q relative to P), then X_OQ (frame Q relative to O) would be X_OQ = X_OP * X_PQ

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
Copy link
Contributor Author

sloretz commented Nov 2, 2020

Given two Pose objects, the order of arguments when using Pose::operator* is the same as if the Poses were first converted to 4x4 homogeneous matrices and the product of the matrices was taken. So if we have frames O, P, and Q and we have the poses X_OP (frame P relative to O) and X_PQ (frame Q relative to P), then X_OQ (frame Q relative to O) would be X_OQ = X_OP * X_PQ

Uses this notation in 975a50c

Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

LGTM

@sloretz
Copy link
Contributor Author

sloretz commented Nov 3, 2020

Does this need a second reviewer, or is it good to merge? If the latter, would someone mind merging it? I don't have write access to the ignitionrobotics repos.

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

👌

@chapulina chapulina merged commit 3338263 into gazebosim:main Nov 4, 2020
Core development automation moved this from In review to Done Nov 4, 2020
scpeters pushed a commit to scpeters/ign-math that referenced this pull request Nov 4, 2020
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
chapulina pushed a commit that referenced this pull request Nov 4, 2020
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@j-rivero j-rivero removed this from Done in Core development May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants