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.

Sign up for GitHub

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

Jump to bottom

use hardcoded QoS (keep all, transient local) for /tf_static topic in dynamic_bridge #282

Merged
merged 7 commits into from
Aug 19, 2020
Merged

use hardcoded QoS (keep all, transient local) for /tf_static topic in dynamic_bridge #282

merged 7 commits into from
Aug 19, 2020

Conversation

dirk-thomas
Copy link
Member

Otherwise the latched ROS 1 message won't be "latched" on the ROS 2 side and commonly ROS 2 subscribers of the /tf_static topic will use the QoS transient_local and therefore won't be matched with volatile publishers.

Build Status

@dirk-thomas
add factory API to create ROS 2 pubs/subs with rclcpp QoS
1d416ce 
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas
add API to create bridges with rclcpp QoS
59330b3 
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas
use hardcoded QoS (keep all, transient local) for /tf_static topic
57e8bee 
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas dirk-thomas added the bug Something isn't working label Aug 19, 2020
@dirk-thomas dirk-thomas self-assigned this Aug 19, 2020
wjwwood
wjwwood reviewed Aug 19, 2020
View reviewed changes
src/bridge.cpp Outdated Show resolved Hide resolved
@dirk-thomas
feedback cleaner code
5b2dcab 
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas
fix compile error of previous commit
6c25657 
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas
update existing code the same way
e3ca885 
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
wjwwood
wjwwood approved these changes Aug 19, 2020
View reviewed changes
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

It's good as-is or you can take my suggested change.

@dirk-thomas
simplify qos construction
b5c4c5c 
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas
Copy link
Member Author

Build Status

@dirk-thomas dirk-thomas merged commit 2102d8c into master Aug 19, 2020
@dirk-thomas dirk-thomas deleted the dirk-thomas/custom-qos-for-tf-static-topic branch August 19, 2020 19:39
renehoelbling pushed a commit to Intermodalics/ros1_bridge that referenced this pull request Aug 27, 2020
@dirk-thomas @renehoelbling
use hardcoded QoS (keep all, transient local) for /tf_static topic in…
cd276be 
… dynamic_bridge (ros2#282)

* add factory API to create ROS 2 pubs/subs with rclcpp QoS

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* add API to create bridges with rclcpp QoS

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* use hardcoded QoS (keep all, transient local) for /tf_static topic

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* feedback cleaner code

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* fix compile error of previous commit

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* update existing code the same way

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* simplify qos construction

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@renehoelbling renehoelbling mentioned this pull request Aug 28, 2020
Merged
@vmanchal1
Copy link

Is this fix going to be available in any of the patches to Foxy?

@wjwwood
Copy link
Member

wjwwood commented Sep 8, 2020

@vmanchal1 there was a new release recently, it should be in the next patch release of Foxy I guess: ros/rosdistro#26380

@dirk-thomas
Copy link
Member Author

there was a new release recently, it should be in the next patch release of Foxy I guess

I don't think that is the case. The patch has only landed on master. Neither has it been backported to foxy nor has the foxy branch been fast forwarded to the state of the master branch.

@jacobperron Are you ok with releasing new patch release into rolling as well as fast forwarding the foxy branch?

@jacobperron
Copy link
Member

Are you ok with releasing new patch release into rolling as well as fast forwarding the foxy branch?

SGTM (I didn't look too closely at binary compatibility, but I'm not sure if we should worry about that for this leafy package).

@dirk-thomas
Copy link
Member Author

I didn't look too closely at binary compatibility

This patch only adds API.

Is this fix going to be available in any of the patches to Foxy?

I just made and upstream release of 0.9.4 on master and also fast forwarded foxy. The bloom release is still pending though since the release repo has some incompatible patches... @jacobperron ?

@dirk-thomas
Copy link
Member Author

The Foxy release: ros/rosdistro#26519.

@dirk-thomas
Copy link
Member Author

And the same release into Rolling: ros/rosdistro#26520.

hsd-dev pushed a commit to hsd-dev/ros1_bridge that referenced this pull request Nov 17, 2020
@dirk-thomas @hsd-dev
use hardcoded QoS (keep all, transient local) for /tf_static topic in…
1c57eea 
… dynamic_bridge (ros2#282)

* add factory API to create ROS 2 pubs/subs with rclcpp QoS

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* add API to create bridges with rclcpp QoS

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* use hardcoded QoS (keep all, transient local) for /tf_static topic

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* feedback cleaner code

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* fix compile error of previous commit

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* update existing code the same way

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* simplify qos construction

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Signed-off-by: Harsh Deshpande <harshavardhan.deshpande@ipa.fraunhofer.de>
@lFatality lFatality mentioned this pull request Mar 27, 2021
Closed
@royito55
Copy link

Is this change implemented in the parameter_bridge?

If it is not, does it mean I have to install it from source and make the changes to be able to recompile?
Or is there a way to edit the binary? (I know there probably isn't)

Finally, would this post address the issue of not all transforms appearing while using the parameter_bridge, even though they appear using the dynamic_bridge?
https://answers.ros.org/question/349230/problem-passing-tf_static-through-ros1_bridge-to-ros2/

@LoyVanBeek LoyVanBeek mentioned this pull request Oct 14, 2021
Merged
@Timple Timple mentioned this pull request Mar 30, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wjwwood wjwwood wjwwood approved these changes

Assignees

@dirk-thomas dirk-thomas

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dirk-thomas @vmanchal1 @wjwwood @jacobperron @royito55