search

ros / geometry2

A set of ROS packages for keeping track of coordinate transforms.
189 stars 275 forks source link

Revert "do not add TF with redundant timestamp" #475

Closed Hugal31 closed 3 years ago

Hugal31 commented 4 years ago

Allow one to send a TF with a redundant timestamp to replace it.

Closes #467

SteveMacenski commented 3 years ago

@tfoote in that case, we should probably just close this. For the initial ticket #467 there are suggestions to throttle the logging so its more manageable or just log the warning once on the first instance. Would you merge either of those?

Looking at the ticket there's a total of 13 +1's from people asking for this to change. The current state of things is really nasty for users.

tfoote commented 3 years ago

Yeah, closing this makes sense. The "nasty" state for the users is generating complaints because it's raising the warnings about unexpected behavior. We can throttle it back a little but the warning should be loud. This is a very dangerous behavior.

The insidiousness of the issue is that it only fails to lookup sometimes and as a consequence there's multiple implementations making false assumptions about the results. And I believe that it's quite likely that the attempts to "improve" the past history are actually degrading the quality of the information for some use cases. Especially for any use cases which are sensitive to consistency or the slope since the old behavior will cause data to look like a sawtooth signal.

For comparison we've had a similar issue with extrapolation in the past. It's "easy" to mathematically allow extrapolation so you don't have to wait for all data to come in. But then it turns out that you amplify any errors in your system, and consequently all control algorithms just get worse performance. Nothing seemed wrong and we had much better latency, but after analyzing the performance with and without extrapolation what seemed like extra power was actually worse in every use case we tested.