search

ros / geometry

Packages for common geometric calculations including the ROS transform library, "tf". Also includes ROS bindings for "bullet" physics engine and "kdl" kinematics/dynamics package.
173 stars 274 forks source link

tf_echo behavior when echo rate <= 0 #159

Closed lucasw closed 6 years ago

lucasw commented 6 years ago

The parameter is passed straight into rate()

If echo_rate is 0 there is a core dump (doesn't matter if frame1 is valid):

rosrun tf tf_echo map frame1 0
terminate called after throwing an instance of 'std::runtime_error'
  what():  Duration is out of dual 32-bit range
Aborted (core dumped)

That is undesirable. What I was actually hoping to find is a way to print a single transform then exit. Would 0 be acceptable for that, or should it be a different parameter for a single output?

If rate is negative then the rate.sleep() doesn't sleep and a lot of stdout results, eating up a lot of cpu:

rosrun tf tf_echo map frame1 -1
At time 1518933331.795
- Translation: [1.500, 0.000, 0.000]
- Rotation: in Quaternion [0.149, 0.000, 0.000, 0.989]
            in RPY (radian) [0.300, -0.000, 0.000]
            in RPY (degree) [17.189, -0.000, 0.000]
At time 1518933331.795
- Translation: [1.500, 0.000, 0.000]
- Rotation: in Quaternion [0.149, 0.000, 0.000, 0.989]
            in RPY (radian) [0.300, -0.000, 0.000]
            in RPY (degree) [17.189, -0.000, 0.000]
At time 1518933331.795
- Translation: [1.500, 0.000, 0.000]
- Rotation: in Quaternion [0.149, 0.000, 0.000, 0.989]
            in RPY (radian) [0.300, -0.000, 0.000]
            in RPY (degree) [17.189, -0.000, 0.000]
At time 1518933331.795
- Translation: [1.500, 0.000, 0.000]
- Rotation: in Quaternion [0.149, 0.000, 0.000, 0.989]
            in RPY (radian) [0.300, -0.000, 0.000]
            in RPY (degree) [17.189, -0.000, 0.000]
...

The max speed updating is possibly desirable, but ought to be documented. Though if the tf hasn't actually changed when the loop loops it shouldn't bother printing it out again, unless additional information about when the query was made was also printed? I'll make a pr for one or both possibilities.

I'm interested in tf_echo now, but does Rate deserve an issue for this also?

tfoote commented 6 years ago

Certainly adding a range check to give a better error if a zero or negative rate is provided to clearly state that it's outside of a valid range.

For the single output and only updating on change that also makes sense. But I'd suggest that these would be best implemented as arguments similar to vcs logs. Like -l, --limit=1 and --changed-values-only. And to do that I would suggest that reimplementing this in python might make sense. Obviously this would be a big change, but it would be a good reason to migrate from tf to tf2_tools where the new python implementation could live. Using argparse and python will make this much simpler to implement.

For the rate it could throw it's own runtime_error with the message that a zero rate causes a divide by zero error, instead of having it fail at the next step where it gets a nan as the duration.

tfoote commented 6 years ago

Patched here, and discussion open on ros/geometry2#287 so I'm going to close.