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 static transform (/tf_static) to 3DViz Panel #336

Closed

Conversation

lwohlhart
Copy link

Summary

The visualisation of the TF tree currently doesn't include static transformations published at the /tf_static topic.
Since many robot descriptions (and also environment models) are built using this concept it is necessary to include these transformations.

Therefore I simply added the messages received on /tf_static to the transform consumer in the ThreeDimensionalViz panel.

Test plan

The change can be tested and verified replaying the example.bag file from packages/webviz-core/public/fixtures/
The 'turtle1' frame actually has a 'carrot' frame statically attach to it at 1 meter in x direction.

Prior to the change the carrot frame doesn't show up at all.
Now the frame is available an correctly linked to its parent.

Versioning impact

Most probably this can be handled as a patch update. Or maybe a minor if your versioning policy requires a minor for a new 'feature'.

Thank you for the nice tool!

Changelog:
- Parse messages commonly published at the /tf_static topic  to include them in the transform tree
- Thereby correctly building tf tree for most robot descriptions (sensor placements, fixed joints, ...)
@lwohlhart lwohlhart requested a review from janpaul123 January 30, 2020 17:29
if (tfs_static) {
for (const msg of tfs_static) {
for (const tf of msg.message.transforms) {
if (tf.child_frame_id !== getGlobalHooks().perPanelHooks().ThreeDimensionalViz.skipTranformFrame) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't need this line, since we only need this for /tf for our internal version. Better to keep this complication out of this new code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I'll get rid of it in a moment
Anyways if I look at it correctly there's a typo in the other line ^^ (skipTranformFrame is missing an 's' I guess) so this should be undefined either way

Copy link
Contributor

@janpaul123 janpaul123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cool! My only concern is that in the new topic tree (which you can enable by going to "Experimental features" these transforms will show up under "/tf", since currently it's hardcoded that way. That's probably not a super big deal but it might be mildly confusing. It might be good to at least add some comments around where we hardcode that, to say that these also include transforms from "/tf_static", e.g. at

(And by the way, it might be good for us to use TRANSFORM_TOPIC there as well, instead of just writing "/tf" 😱 cc @vidaaudrey)

Anyway, this looks good to me after making those changes. 👍 Thanks for the contribution!

@janpaul123
Copy link
Contributor

@lwohlhart ah I totally forgot about this. CI seems to be failing because it's a PR to a fork, but that's OK. I'll pull this into our internal repo and then I'll soon do a sync to push this out to this open source repo again. Thanks!

janpaul123 pushed a commit that referenced this pull request Apr 9, 2020
Changelog:
- Added support for streaming a second bag using the `remote-bag-url-2` query parameter in the URL. #374
- Fixed crashing on bags with topics that don't have a message definition. #373
- Various performance improvements.
- Improved caching of deserialized messages should result in fewer browser crashes.
- Added `unlimitedMemoryCache` experimental feature to load everything in memory without regard for caching limits. Useful for when you have a machine with lots of memory and you're explicitly OK with Webviz taking up a lot of it. Use at your own risk; this might crash your browser!
- Included an "Add Topics" option for adding markers to the Image panel.
- Made performance improvements to the Plot and State Transitions panels. If you notice any issues, these performance improvements can be disabled in the Experimental Features menu, under "Use a web worker to render the Plot panel".
- In the Image panel, we again filter available marker topics by the namespace of the currently selected camera.
- Added support for publishing messages over the Websocket connection using a Publish panel. #323
- Added layout undo/redo shortcuts.
- Added support for transforms from [`/tf_static`](http://wiki.ros.org/tf2/Tutorials/Writing%20a%20tf2%20static%20broadcaster%20%28C%2B%2B%29). #336
- Deployed a faster format for displaying text in the 3D panel. You can now use `ctrl-f` (or Mac equivalent) to physically move the camera to matched text. If you notice any issues, this change can be disabled in the Experimental Features menu, under "Faster 3D Text".
- Fixed Webviz getting stuck in a reloading loop.
- Added support for streaming a second bag using the `remote-bag-url-2` query parameter in the URL. #374
- Fixed crashing on bags with topics that don't have a message definition. #373
- Fixed not always loading messages when subscribing to a new topic when paused ("backfilling").
- Improved caching of deserialized messages should result in fewer browser crashes.
- Improved startup time by not making multiple requests with different topics when loading the page.
- Fix some cases in which "syncing" 3d panels could cause panels to display a blank screen.
- Fixed not always loading messages when subscribing to a new topic when paused ("backfilling").
- Switched Websocket message encoding to [cbor-raw](RobotWebTools/rosbridge_suite#452). #361
janpaul123 added a commit that referenced this pull request Apr 9, 2020
Changelog:
- Added support for streaming a second bag using the `remote-bag-url-2` query parameter in the URL. #374
- Fixed crashing on bags with topics that don't have a message definition. #373
- Various performance improvements.
- Improved caching of deserialized messages should result in fewer browser crashes.
- Added `unlimitedMemoryCache` experimental feature to load everything in memory without regard for caching limits. Useful for when you have a machine with lots of memory and you're explicitly OK with Webviz taking up a lot of it. Use at your own risk; this might crash your browser!
- Included an "Add Topics" option for adding markers to the Image panel.
- Made performance improvements to the Plot and State Transitions panels. If you notice any issues, these performance improvements can be disabled in the Experimental Features menu, under "Use a web worker to render the Plot panel".
- In the Image panel, we again filter available marker topics by the namespace of the currently selected camera.
- Added support for publishing messages over the Websocket connection using a Publish panel. #323
- Added layout undo/redo shortcuts.
- Added support for transforms from [`/tf_static`](http://wiki.ros.org/tf2/Tutorials/Writing%20a%20tf2%20static%20broadcaster%20%28C%2B%2B%29). #336
- Deployed a faster format for displaying text in the 3D panel. You can now use `ctrl-f` (or Mac equivalent) to physically move the camera to matched text. If you notice any issues, this change can be disabled in the Experimental Features menu, under "Faster 3D Text".
- Fixed Webviz getting stuck in a reloading loop.
- Fixed not always loading messages when subscribing to a new topic when paused ("backfilling").
- Improved startup time by not making multiple requests with different topics when loading the page.
- Fix some cases in which "syncing" 3d panels could cause panels to display a blank screen.
- Switched Websocket message encoding to [cbor-raw](RobotWebTools/rosbridge_suite#452). #361
@janpaul123
Copy link
Contributor

I included these changes in #396. Thanks again!

@janpaul123 janpaul123 closed this Apr 9, 2020
@lwohlhart
Copy link
Author

yay thx !
😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants