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

tf2_eigen: Should Eigen::Affine3d be converted to tf2::Transform? #358

Open
peci1 opened this issue Feb 11, 2019 · 5 comments
Open

tf2_eigen: Should Eigen::Affine3d be converted to tf2::Transform? #358

peci1 opened this issue Feb 11, 2019 · 5 comments

Comments

@peci1
Copy link
Contributor

peci1 commented Feb 11, 2019

Affine transforms are a superset of ROS transforms (which are isometries). So should there really be the conversion function from Affine3d to Transform? Shouldn't the user be required to convert the Affine3d to Isometry3d in Eigen first (doing all the needed checks)? Or should the tf2 conversion function issue a warning/error when the given affine transform is not an isometry?

@tfoote
Copy link
Member

tfoote commented Feb 14, 2019

It would probably be good to deprecate that part of the API. It makes more sense since it's not an isomorphic conversion. When first written the Affine3d was the only transform available and Isometry3d was added later and support added here recently (#335). Though we will want to target that for the next distro to not break people in the current distro.

@peci1
Copy link
Contributor Author

peci1 commented Feb 15, 2019

Do you think a build time deprecation/warning or rather a runtime warning would be suitable?

It might be nice to actually check if the given transform is an isometry or not, and issue a runtime warning when it isn't, but I can imagine that could have impact on performance. And in most cases the transform will be an isometry (because it probably comes from a tf transform).

@tfoote
Copy link
Member

tfoote commented Feb 15, 2019

A build time deprecation warning is appropriate. This can only be fixed by changing code. Spamming the user will just be annoying.

@peci1
Copy link
Contributor Author

peci1 commented Mar 5, 2019

Deprecation warnings added in #371 .

@peci1
Copy link
Contributor Author

peci1 commented Feb 3, 2024

Noetic continuation in #561.

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

2 participants