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

Deprecate tf for tf2 #1236

Merged
merged 16 commits into from
May 10, 2018
Merged

Deprecate tf for tf2 #1236

merged 16 commits into from
May 10, 2018

Conversation

wjwwood
Copy link
Member

@wjwwood wjwwood commented May 9, 2018

This pr deprecates all of the interfaces in rviz which I believe need to be so that in a future version we can drop the dependency on tf completely.

The two main places where interfaces are disabled are:

I migrated the MessageFilterDisplay which is a base class that many display plugins inherit from and therefore most of them will require no changes at all. I also migrated a couple other displays to show how it can be done, but I just suppressed the rest of the displays that produced deprecation warnings because I wanted to give an example of how to do that and because the work to convert them can be deferred until we actually remove the dependency on tf completely.

Examples of migrating to tf2:

  • MessageFilterDisplay: 0f227e5
  • Image related displays (image and camera): ffdfad0
  • DepthCloudDisplay: 2dc400d

Examples of suppressing the deprecation warnings:

There are a few unrelated style/warnings related changes at the end, which I could separate if desired, but they're small and mostly only affect macOS.

@wjwwood wjwwood self-assigned this May 9, 2018
@wjwwood wjwwood requested a review from dhood May 9, 2018 02:33
@wjwwood
Copy link
Member Author

wjwwood commented May 9, 2018

@IanTheEngineer FYI

I plan on closing the other pr. Also, this will probably introduce a deprecation warning in moveit (sorry about that), but I'm happy to make a pr for that if you can point me to where you needed to change things there. The warnings are easy to either address or suppress. There are some examples above in the issue description.

@wjwwood
Copy link
Member Author

wjwwood commented May 9, 2018

I'll also mention that for the changes to the Constructor of VisualizationManager and FrameManager which optionally take a tf::TransformListener, I deprecated them but could not offer the alternative right now. The reason is that if someone gives us a tf2 equivalent I cannot provide the tf version of the client when someone asks for it via getTFClient(). So they'll have to suppress the warning and then when we completely remove tf I can provide a new constructor that can take the tf2 equivalent.

@IanTheEngineer
Copy link

That makes sense. I’m out on vacation this week, but I’ll point you to the right spot to help suppress the deprecation warnings next week when I return (around 5/16). There’s only two lines in all of MoveIt using the FrameManager

@wjwwood
Copy link
Member Author

wjwwood commented May 10, 2018

The CI looks good to me, the remaining failure is in robot_model_display.cpp and should be addressed separately, see: #1235

@wjwwood
Copy link
Member Author

wjwwood commented May 10, 2018

I'll add an note on this wiki page when this is merged:

https://wiki.ros.org/melodic/Migration

@wjwwood wjwwood added this to the melodic release milestone May 10, 2018
@wjwwood wjwwood merged commit 57325fa into melodic-devel May 10, 2018
@ghost ghost removed the review label May 10, 2018
@wjwwood wjwwood deleted the deprecate_tf_for_tf2 branch May 10, 2018 23:13
@wjwwood
Copy link
Member Author

wjwwood commented May 10, 2018

Migration guide updated: https://wiki.ros.org/melodic/Migration#rviz

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

Successfully merging this pull request may close these issues.

3 participants