forked from MaxiMaerz/rustros_tf
-
Notifications
You must be signed in to change notification settings - Fork 6
Roslibrust tf support #55
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
Draft
lucasw
wants to merge
128
commits into
smilerobotics:main
Choose a base branch
from
lucasw:roslibrust
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains hidden or 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
…ons on Time and Duration but need to test them, and need cmp()
…and Duration, this compiles but untested
…t not compiling here
…ooking up and printing the transform, need to pass in frames as args and use static transforms
…en static and dynamic properly, but maybe python and C++ listeners are the same
…he same stamp across the full tf chain
… get the most recent stamp- though should get rid of the is_zero check and just pass in a flag to get most recent instead of using a sentinel value
…g the transforms between them, because the inverted transforms aren't getting added to the same data structure more logic is needed - maybe store the inverse transforms adjacent to the normal ones, but clearly distinguished as inverse?
…a function and make loops an error type
…nsforms and different tf tree leaf frames- but can't say for certain that number are right under dynamic conditions, need to test
…f2_tools echo reports
…me instances of it getting locked up where next() on both subscribers appear to almost always return something (not sure what, is it the last message over and over again?)
…y sorted, but the cursor functionality isn't available yet so in the process of using another function
… tokio selects in echo, this looks to work fine but need to update other applications using tf listener
…rust_codegen for Time but maybe roslibrust_util could wrap that
…, which will be useful for a tf_mcap tool to find gaps in tf buffers
…lice (but my local version didn't)
… toml file of any number of commands to run
…get the same sigint passed into the running commands
…but need to try more intensive processes
…es, but need to integrate clap to be able to override them on the command line
…t, but ended up just cloning the arc mutex
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.
I made a version of this that works with https://github.com/Carter12s/roslibrust based on the rosrust version here.
There are a lot of changes made to the underlying tf system that may not have been necessary but I wanted to experiment:
chrono TimeDelta is used in place of the roslibrust Time in most places except for the Transform message because the latter doesn't have comparison or addition or subtraction support.
It was previously inverting every received transforms and adding those with parent-child reversed alongside the originals I think to avoid having to invert later in a lookup, but this made the tf tree harder to traverse to find the chain between any two frames. WIth a directed graph any two frames can traverse the tree upwards until they hit a root frame or a frame that the other traversal has already found, and then there is a tf chain (or disconnected trees if no frames in common).
Also the transform inversion on every incoming frame seemed unnecessary given that tf lookups are probably much lower frequency than incoming transforms, 10s of Hz vs. possibly hundreds of Hz, and there are many incoming transforms that are not part of the chain the node instantiating the tf listener cares about (it would be nice if those unneeded transforms could be dropped on reception, but that only works with a non-dynamic tree). There could be more improvements there where the found tf chain in a lookup knows exactly which transforms need inverting, currently it tries to find a given parent-child transform and fails and then looks up the reversed pair and does the inversion.
Loop detection is only done at lookup time, not for the entire received graph, which is less of a burden.
There are other changes made to handling static transforms and the tree dynamically changing that I made trying to debug lookup issues (which I may have created making other changes). I haven't looked at documentation or compared to behavior of the rospy/roscpp transform libraries, but I don't typically change parents of big parts of the tree or change a transform from static to non-static live, but I could see how some applications would want to do that. Rviz and other tools seem bad at handling big tree changes (maybe just those involving static tf changes?) and require restarts to flush old contradictory transforms, so I won't be surprised if it isn't handled well in the original C++ (or python).
I think the expiration of old transforms ought to be done across the entire buffer, the newest transform stamp - oldest transform stamp should be < cache duration for every parent-child, but currently individual transform pairs expire old transforms based on the time span of just that particular parent-child.
There's an echo.rs executable that mostly duplicates the output of
tf2_tools echo.py
but isn't yet doing any quaternion to euler conversions, and other command line options aren't supported.It's up to the executable to update tf_listener subscribers alongside whatever subscribers and timed events they are doing, that could be improved. I wouldn't want to completely take control away by spawning threads behind the scenes, but maybe that should be optionally available.