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 ROS style api #3

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

felixvd
Copy link

@felixvd felixvd commented Apr 26, 2019

This PR adds a Transformer class with similar interfaces to the original ROS TransformListener. This should allow much easier switching between the ROS tf library and tiny_tf.

ToDo:

  • Simplify the test_transformer.cpp so it can be used as a demo
  • Clean up the geometry_msgs folder

@felixvd
Copy link
Author

felixvd commented Apr 29, 2019

I made test_transformer.py a bit nicer to read for humans, with more useful example cases. However, I got this error, but I'm not sure it's related to my changes. They seem simple enough. Maybe you can have a look @safijari ?

Robot tooltip position in the world frame:
0.10000000000000003, 0.0, 1.0
0.0, 0.7071067811865476, 0.0, 0.7071067811865475
In Euler angles: -0.0, 1.5707963267948961, 0.0
Traceback (most recent call last):
  File "tiny_tf/test_transformer.py", line 98, in <module>
    test_transformations(tree)
  File "tiny_tf/test_transformer.py", line 67, in test_transformations
    ps_new = tree.transformPose("robot_camera", ps)
  File "/home/felix/tiny_tf/transformer.py", line 49, in transformPose
    t = self.lookup_transform(pose_stamped.header.frame_id, target_frame)
  File "/home/felix/tiny_tf/tf.py", line 76, in lookup_transform
    frame_transform_to_common_parent = get_inverse_xform_for_path(frame_path_to_parent)
  File "/home/felix/tiny_tf/tf.py", line 73, in get_inverse_xform_for_path
    transform_to_parent = np.dot(node.transformation_matrix, transform_to_parent)
  File "/home/felix/tiny_tf/tf.py", line 141, in transformation_matrix
    return self.transform.matrix
  File "/home/felix/tiny_tf/tf.py", line 174, in matrix
    out[0, -1] = self.x
TypeError: float() argument must be a string or a number, not 'Point'

@safijari
Copy link
Owner

The problem seems to come from lines 30 and 38 of the test_transformer.py file.

ts.transform.translation.x = geometry_msgs.msg.Point(x=-0.1, y=0.0, z=0.05)

This makes me wish for mypy. No system I use runs python2 anymore but I assume you'd still want python2 compatibility otherwise I can upgrade the codebase to be python3 and add type hints. Then type checks can be added in CI.

@felixvd
Copy link
Author

felixvd commented Apr 30, 2019

ROS itself uses it, so python2 compatibility is probably smart. I doubt type checking or even CI will be necessary for this. It's supposed to be tiny, right?

This last commit should work, but I can't check until tomorrow. I also forgot thatstd_msgs/Header needs to be included, too, and that all the messages still depend on genpy. Adding it and taking out the serialization should be pretty quick. I'll do it tomorrow, too.

@felixvd
Copy link
Author

felixvd commented May 1, 2019

Well. I assumed that removing the genpy.Message dependance would work fine, but I guess I was mistaken. Not being able to parse from a list of arguments would make the code a lot uglier:

ts.transform.rotation = geometry_msgs.msg.Quaternion(*tf_transformations.quaternion_from_euler(0, pi*90/180, 0)) would become:

q = tf_transformations.quaternion_from_euler(0, pi*90/180, 0)
ts.transform.rotation.x = q[0]
ts.transform.rotation.y = q[1]
ts.transform.rotation.z = q[2]
ts.transform.rotation.w = q[3]

I guess including genpy.Message as well could do the trick, but it's starting to get awkward. Thoughts?

@felixvd felixvd force-pushed the add-ros-style-api branch from 3542528 to 1b36629 Compare May 7, 2019 12:03
@felixvd
Copy link
Author

felixvd commented May 7, 2019

Ok. It's super ugly, but it finally works out of the box on Mac OS X on both python3 and python2 (after installing numpy and pyyaml). Feel free to merge and make it not embarrassing.

@safijari
Copy link
Owner

safijari commented May 8, 2019

Do you mind if I split the messages into a separate repo? I can release it as a separate package and make it a dependency for tiny_tf.

@felixvd
Copy link
Author

felixvd commented May 9, 2019 via email

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.

2 participants