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

use static tf broadcaster for ros2 #112

Conversation

Aposhian
Copy link

@Aposhian Aposhian commented Apr 27, 2023

Related Issues & PRs

#105

Summary of Changes

  • send static transforms only once with a static transform broadcaster

Validation

In progress

@Aposhian
Copy link
Author

Aposhian commented May 1, 2023

Ok @Samahu I have updated this to include the latest merged PR, so now this only adds the static broadcaster

@Aposhian Aposhian changed the title Ros2 tf update use static tf broadcaster for ros2 May 1, 2023
@Samahu
Copy link
Contributor

Samahu commented May 2, 2023

If static broadcast is the appropriate (for ROS2) then we don't need to add the static publisher in the C++ code.. we could just utilize the static publishers that are created when RVIZ is launched:

Basically all we need to do is enable these static publisher regardless if RVIZ is launched or not.

@Samahu
Copy link
Contributor

Samahu commented May 2, 2023

I take that back we need the C++ code since we don't have the transform until metadata is received.

@Samahu Samahu added the enhancement New feature or request label May 2, 2023
Copy link
Contributor

@Samahu Samahu left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@Samahu Samahu marked this pull request as ready for review May 2, 2023 14:30
@Samahu
Copy link
Contributor

Samahu commented May 2, 2023

@Aposhian if you don't have any other changes I am ready to merge

@Aposhian
Copy link
Author

Aposhian commented May 2, 2023

I don't have any more changes, but I am still testing to validate that it works as expected.

@Samahu
Copy link
Contributor

Samahu commented May 2, 2023

I don't have any more changes, but I am still testing to validate that it works as expected.

I appreciate any help!

@Aposhian
Copy link
Author

Aposhian commented May 4, 2023

I've confirmed this works, although it gave me a little bit of unexpected results due to #33. This PR should be good to merge!

@Samahu Samahu changed the base branch from ros2 to SW-4972-merge-switching-to-static-transform-publisher-contribution May 5, 2023 17:03
@Samahu Samahu merged commit a8b643c into ouster-lidar:SW-4972-merge-switching-to-static-transform-publisher-contribution May 5, 2023
2 checks passed
@Samahu
Copy link
Contributor

Samahu commented May 5, 2023

@Aposhian I went ahead and merged your PR into a branch so I would be able to document your changes and also disable the static publishers that I had for the rivz node before merging into the main ros2 branch.

@Aposhian
Copy link
Author

Aposhian commented May 5, 2023

Sounds good!

Samahu added a commit that referenced this pull request May 5, 2023
…124)

* use static tf broadcaster for ros2 (#112)
* use separate params for tf frames
* send static transforms once
* Disable static transform publishers and update changelog and package version
* Disable rviz static transform publisher
* Remove rviz static transform publisher hack
* Remove left out variables
---------
Co-authored-by: Adam Aposhian <[email protected]>
Samahu added a commit that referenced this pull request May 5, 2023
…oxy (#125)

* use static tf broadcaster for ros2 (#112)
* use separate params for tf frames
* send static transforms once
* Disable static transform publishers and update changelog and package version
* Disable rviz static transform  publisher
* Remove rviz static transform publishers hack
---------
Co-authored-by: Adam Aposhian <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants