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

Migrate rtt_tf to tf2 #121

Merged
merged 13 commits into from
May 15, 2019
Merged

Migrate rtt_tf to tf2 #121

merged 13 commits into from
May 15, 2019

Conversation

francisco-miguel-almeida
Copy link
Collaborator

This branch ports rtt_tf to be now based on tf2. It also allows subscribing to static transforms as input.
As requested in issue #68.

Copy link
Member

@meyerj meyerj 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. I only have a few comments and requests:

  • Static transforms have to be added as such to tf2::BufferCore::setTransform().
  • I suggest to add support for broadcasting static transforms, too.
  • Remove commented/obsolete code

Not covered by comments below:

  • rtt_tf needs to depend on the new typekit package rtt_tf2_msgs. Include the generated header rtt_tf2_msgs/typekit/Types.hpp instead of tf2_msgs/TFMessage.h, which declares the template instantiations for RTT::InputPort<tf2_msgs::TFMessage> and RTT::OutputPort<tf2_msgs::TFMessage> as external. This pattern should always be used for types that have a typekit to reduce binary sizes and save compilation time.
  • Update rtt_tf/package.xml and rtt_tf/CMakeLists.txt. At the moment it only works because tf depends on tf2_ros internally. We can keep the dependency to tf and still use tf::resolve() instead of copying the method.

When adding new or updating operations of the component, please do not forget to also update the public tf_interface.h, too.

rtt_tf/README.md Outdated Show resolved Hide resolved
rtt_tf/rtt_tf-component.cpp Outdated Show resolved Hide resolved
rtt_tf/rtt_tf-component.cpp Outdated Show resolved Hide resolved
rtt_tf/rtt_tf-component.cpp Show resolved Hide resolved
rtt_tf/rtt_tf-component.cpp Outdated Show resolved Hide resolved
typekits/rtt_tf2_msgs/package.xml Outdated Show resolved Hide resolved
rtt_tf/rtt_tf-component.cpp Outdated Show resolved Hide resolved
rtt_tf/rtt_tf-component.cpp Outdated Show resolved Hide resolved
rtt_tf/rtt_tf-component.cpp Show resolved Hide resolved
rtt_tf/rtt_tf-component.hpp Outdated Show resolved Hide resolved
meyerj and others added 2 commits May 8, 2019 02:08
…loads a component that allows importing ROS packages following the dependency tree.
@francisco-miguel-almeida
Copy link
Collaborator Author

francisco-miguel-almeida commented May 13, 2019

Addressed most issues, but as we discussed earlier, some bigger changes should be addressed as a separate PR. See issue created to discuss this: #126

@meyerj meyerj merged commit eaf056f into toolchain-2.9 May 15, 2019
@meyerj meyerj deleted the rtt-migrate-tf2 branch May 15, 2019 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants