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 getYaw helper function #110

Closed
tfoote opened this issue May 17, 2015 · 9 comments
Closed

add getYaw helper function #110

tfoote opened this issue May 17, 2015 · 9 comments

Comments

@tfoote
Copy link
Member

tfoote commented May 17, 2015

It saves several lines of code which could easily be done wrong.

https://github.com/ros-perception/slam_gmapping/pull/31/files#diff-ee79bfdef4f6452c01f74517ee41c050R371

@vrabaud
Copy link
Member

vrabaud commented May 17, 2015

I can make a PR but where do you see that ? a member function of the tf2::Quaternion object ? A standalone function ? (that could work with tf2::Matrix3x3 and tf2::QUaternion)

@tfoote
Copy link
Member Author

tfoote commented May 17, 2015

I like the bare function. It would be nice to support extendable datatypes kind of like convert does with the inclusion of the appropriate headers. It doesn't need to be templated as the datatypes can just be overloaded. As a bare function we can also provide it for the message datatypes.

It should probably be something like getHeading for a clearer scope of what it's providing.

I guess it should get it's own header file for clarity. Are there other similar methods missing? Common, simple, but painful?

@vrabaud
Copy link
Member

vrabaud commented May 17, 2015

well, there are a few other functions but they are member functions (like setIdentity for a transform).
Should that function be put in https://github.com/ros/geometry_experimental/blob/indigo-devel/tf2/include/tf2/convert.h ? It's already a bit of everything in there and those functions are supposed to work for any datatypes.

@tfoote
Copy link
Member Author

tfoote commented May 17, 2015

convert.h does seem like a reasonable place. And it can be added side by side with the toMsg fromMsg in the other packages too.

@vrabaud
Copy link
Member

vrabaud commented May 17, 2015

#112 might be a solution

@mikeferguson
Copy link

+1 to having a getYaw -- especially one that accepts the orientation of geometry_msgs/Pose directly

@vrabaud
Copy link
Member

vrabaud commented May 17, 2015

hmm, with #112, it only gives the yaw of anything that can be converted to a tf2::Quaternion. Hence, not a pose. You can just call getYaw(pose.orientation). We could also specialize it for a Pose. I just feel that we should keep it simple. If you feel it should work for several types, please let me know which. I see Transform and Pose. Anything else ?

Just saw that my PR would not work for Stamped messages, will fix that.

@mikeferguson
Copy link

pose.orientation is fine -- I really just want to avoid converting from message to TF to yaw (something I find is an often repeated calculation)

@tfoote
Copy link
Member Author

tfoote commented Jun 8, 2015

Fixed in #112

@tfoote tfoote closed this as completed Jun 8, 2015
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

3 participants