Add setter/getter for Pose's each element - Issue 35#125
Add setter/getter for Pose's each element - Issue 35#125chapulina merged 15 commits intogazebosim:ign-math6from
Conversation
|
@chapulina Here we go! first PR!! \o/ |
chapulina
left a comment
There was a problem hiding this comment.
Thanks for the PR! ✌️ It's looking good. My only suggestion would be to also add tests for the mutable functions. I think the current tests are only covering the const ones.
Here's an example:
|
are you able to merge this branch with |
|
[DO NOT MERGE] @chapulina @scpeters That is expected because the Quaternion.Roll() , Pitch() and Yaw() are How should I solve this point? I thought in some ways but, all of them I need to edit the library Quaternion.hh and, also, there is pattern and architecture questions, so I decided to ask |
|
humm nice catch In fact, this reminds me that we've been deprecating the mutable accessors in favor of So I think the solution is to substitute the mutable functions with |
|
Humm, nice, so, maybe, stop working with this issue and help with this functionality of the Quaternion.hh ? After that continue to working here? The idea is, also, to use this code style to Pose.hh and Vector3.hh ? |
|
Oh I don't think you need to address the entire issue. I'd just change the new functions you're adding. Remove all the mutable functions and add: |
|
Ok, good, should I remove Pos() and Rot() mutable, as well, and change for SetPos() and SetRot(), or is it better to do this in another PR? |
Good call, yes, please! |
those API's are already released in ign-math6, so I wouldn't remove them in this PR |
Ah sorry, thank you Steve. Let's only remove the ones being added by this PR. |
Quaternion doesn't allow modification of Euler angles independently; it only allows them to all be set at once. I would suggest simply adding |
Ok, It was I thought, and also using the Euler method would not be a good idea now since it is deprecated, so I'll do your suggestion! |
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
Signed-off-by: pxalcantara <pxalcantara@gmail.com>
|
@scpeters @chapulina sorry for the delay. Is this the way that you suggested? About the failure of the CI test, I couldn't understand the error, what should I do to fix it? |
|
it looks like the build machines had an error; I'll start the jobs again |
|
@osrf-jenkins run tests please |
|
Thank you @scpeters! |
chapulina
left a comment
There was a problem hiding this comment.
Looks great, thanks for iterating, @pxalcantara !

Solved #35
Add methods to Pose3.hh to get X,Y,Z,Roll,Pitch and Yaw directly, as suggested in the issue description :