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

Allow bi-directional topic bridging #39

Merged
merged 6 commits into from
Jul 15, 2021
Merged

Allow bi-directional topic bridging #39

merged 6 commits into from
Jul 15, 2021

Conversation

rebecca-butler
Copy link
Contributor

Closes #38

Signed-off-by: Rebecca Butler rebecca@openrobotics.org

@rebecca-butler
Create reverse option to enable bidirectional bridging
f6a58f2 
Signed-off-by: Rebecca Butler <rebecca@openrobotics.org>
ivanpauno
ivanpauno reviewed Jul 13, 2021
View reviewed changes
Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

LGTM, I have left some minor comments.

jacobperron
jacobperron reviewed Jul 13, 2021
View reviewed changes
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

I agree that we should rename the YAML key to bidirectional.

As a follow-up PR, I think we also want a reverse option to create a single bridge swapping the from and to domains.

@jacobperron
Copy link
Member

Please add a brief test exercising the new option.

@rebecca-butler
Rename reverse option to bidirectional
bc3461a 
Signed-off-by: Rebecca Butler <rebecca@openrobotics.org>
@rebecca-butler
Add tests for bidirectional bridge
c3e3a3a 
Signed-off-by: Rebecca Butler <rebecca@openrobotics.org>
@rebecca-butler
Copy link
Contributor Author

Please add a brief test exercising the new option.

I've added a couple tests in c3e3a3a.

jacobperron
jacobperron approved these changes Jul 14, 2021
View reviewed changes
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

LGTM, feel free to ignore my latest comment.

@rebecca-butler
Move test to test_domain_bridge_end_to_end.cpp
359ea21 
Signed-off-by: Rebecca Butler <rebecca@openrobotics.org>
@rebecca-butler
Update test to wait for publisher on both domains
90e7647 
Signed-off-by: Rebecca Butler <rebecca@openrobotics.org>
@rebecca-butler rebecca-butler merged commit 2dcb49c into ros2:main Jul 15, 2021
@rebecca-butler rebecca-butler deleted the bidirectional_bridge branch July 15, 2021 19:05
@rebecca-butler rebecca-butler mentioned this pull request Jul 16, 2021
Merged
@smnogar smnogar mentioned this pull request Jan 4, 2022
Open
bart02 pushed a commit to bart02/domain_bridge that referenced this pull request Oct 22, 2022
@rebecca-butler @bart02
Allow bi-directional topic bridging (ros2#39)
8f7a062 
* Create `reverse` option to enable bidirectional bridging

Signed-off-by: Rebecca Butler <rebecca@openrobotics.org>

* Rename `reverse` option to `bidirectional`

Signed-off-by: Rebecca Butler <rebecca@openrobotics.org>

* Add tests for bidirectional bridge

Signed-off-by: Rebecca Butler <rebecca@openrobotics.org>

* Move test to test_domain_bridge_end_to_end.cpp

Signed-off-by: Rebecca Butler <rebecca@openrobotics.org>

* Update test to wait for publisher on both domains

Signed-off-by: Rebecca Butler <rebecca@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobperron jacobperron jacobperron approved these changes

@ivanpauno ivanpauno ivanpauno approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow bi-directional topic bridging
3 participants
@rebecca-butler @jacobperron @ivanpauno