-
Notifications
You must be signed in to change notification settings - Fork 70
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
Document Pose::operator*() #170
Conversation
Signed-off-by: Shane Loretz <[email protected]>
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. |
we added tests for
|
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 |
Thanks for adding the documentation @sloretz. In most robotics books (eg. http://hades.mech.northwestern.edu/images/2/25/MR-v2.pdf), the Given two |
Signed-off-by: Shane Loretz <[email protected]>
Uses this notation in 975a50c |
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
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. |
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.
👌
Signed-off-by: Shane Loretz <[email protected]>
Signed-off-by: Shane Loretz <[email protected]>
Adds some documentation for
Pose::operator*()
. If I understand correctly, it looks likeB + A
is the same asA * B
, but operator+ is discouraged.