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

Get yaw #112

Merged
merged 2 commits into from
Jun 8, 2015
Merged

Get yaw #112

merged 2 commits into from
Jun 8, 2015

Conversation

vrabaud
Copy link
Member

@vrabaud vrabaud commented May 17, 2015

This needs #111 to be in but it provides a few useful functions when using tf2. Maybe we should have a new header like utils.h or something.

@vrabaud
Copy link
Member Author

vrabaud commented May 17, 2015

@Karsten1987, you must be so happy to have such a boss :)

@vrabaud
Copy link
Member Author

vrabaud commented May 17, 2015

TODO: get it to work with Stamped version of the data. Will probably wait for #111 to be merged before updating.

@tfoote
Copy link
Member

tfoote commented May 18, 2015

Thanks @vrabaud that looks like a good approach. Maybe euler.h would be a good file name? Keep it relatively clearly scoped.

@vrabaud
Copy link
Member Author

vrabaud commented May 18, 2015

well, there is also getIdentity in there.

@vrabaud
Copy link
Member Author

vrabaud commented May 18, 2015

how about a utils.h that would contain getTimestamp, getFrameId, getYaw, getEulerYPR, getIdentity and have convert.h include it (for backward compatiblity) and only contain doTransform, toMsg, fromMsg and convert ?

@vrabaud
Copy link
Member Author

vrabaud commented May 31, 2015

ok, updated. @tfoote , @mikeferguson I need some advice before validating that:

  • I restricted getEulerYPR and get Yaw to work with anything that can be converted to a quaternion. hence, it would not work for a Pose and such. Should I specialize for things that contain a rotation ? (I personnally think it would be wrong and it's not such a hassle to write getEulerYPR(t.quaternion)
  • getIdentity only works for anything that can be converted to a tf2::Transform. Should I name it differently ? Should I specialize it for anything that has some identity (like Vector3, Quaternion) ?

@mikeferguson
Copy link

@vrabaud on the first point, that is exactly what I would expect, getEulerYPR(t.quaternion) is very clear what the intent is, and is not much more to write.

On the getIdentity() -- I don't really have a strong opinion either way. I think it is fine as-is, and if it becomes a problem later, we can add additional specializations.

@tfoote
Copy link
Member

tfoote commented Jun 1, 2015

I agree with @mikeferguson .

This looks pretty good. Anything else you're thinking @vrabaud ?

@vrabaud
Copy link
Member Author

vrabaud commented Jun 7, 2015

all I did was rename getIdentity to getTransformIdentity as I believe we might need getQuaternionIdentity and a few other ones.

So, all good to me.

tfoote added a commit that referenced this pull request Jun 8, 2015
@tfoote tfoote merged commit 88cdf22 into ros:indigo-devel Jun 8, 2015
@tfoote tfoote removed the in progress label Jun 8, 2015
@vrabaud vrabaud deleted the getYaw branch June 13, 2015 04:39
@vrabaud
Copy link
Member Author

vrabaud commented Jun 29, 2015

that will be released after the indigo and jade syncs I guess ?

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.

3 participants