Don't insert a TF frame if one of the same timestamp already exists, instead just overwrite it. #425
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
October 18, 2019 20:33
…instead just overwrite it.
pbeeson
changed the title
…nd normal transform was frame 'b' instead of frame 'a', thus creating a problem
3 tasks
ooeygui
pushed a commit
to ms-iot/geometry2
that referenced
this pull request
Oct 12, 2022
* [tf2_kdl] pep8 formatting * [tf2_kdl] fix pep257 * [tf2_kdl] fix copyright * tf2_kdl header guard and copyright * more flake8 fixes Co-authored-by: Bjar Ne <gleichdick@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So, we ran into an issue with ROS on a project (still on kinetic due to legacy robot issues, but this applies to melodic too, so I'll make a similar PR there).
Generally, if a node is broadcasting TF data based on other incoming frames and/or other information, and something goes wrong, it will stop publishing data. But we ran into a scenario where something went wrong in a node, and it kept broadcasting the same TF data with the same timestamp, forever. When this happened, we noticed that all nodes with a tf listener started immediately consuming more and more memory.
In looking into this, it is clear that when storing an incoming TF in the tf2 listener code, that if a frame between parent and child exists with the same timestamp, that the incoming data is not ignored, nor does it overwrite the previous frame, but it gets inserted next to the existing frame with the same timestamp, so that now there are multiple transforms between the same two frames with the same timestamp.
I added some debug console outputs and a small publisher node and a node that just makes a tf listener and spins. This proved that the storage for tf listener does indeed grow infinitely so long as no newer data comes in that is more than max_duration (10 seconds) to force these to be pruned.
The fix is VERY simple, and has been tested with the stand alone programs. where previously the size of storage_ grew without bound when sending the exact same message in a loop, but now remains size 1 for that parent/child transform.