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

Only create readers, writers, and tracks when they are going to be used [19421] #57

Merged
merged 34 commits into from
Oct 9, 2023
Merged

Only create readers, writers, and tracks when they are going to be used [19421] #57

merged 34 commits into from
Oct 9, 2023

Conversation

Tempate
Copy link
Contributor

@Tempate Tempate commented Aug 31, 2023 •
edited by rsanchez15
Loading

In the previous version, the DdsBridge created a reader and a writer for every participant. Then, for each reader, the DdsBridge created a track towards every writer (except the participant's writer unless the participant was configured as a repeater).

In this version, the DdsBridge creates the readers and the writers dynamically.

  • Whenever a participant discovers an external subscriber, the DdsBridge:

    1. Creates a writer in the participant who discovered the subscriber.
    2. Creates readers in the participants who have a route towards the participant who discovered the subscriber.

  • Whenever an external subscriber drops or disconnects, the DdsBridge:

    1. Removes its associated writer from every track it was on.
    2. Removes the tracks that don't have any writers in them.

Merge after

@Tempate Tempate marked this pull request as ready for review September 15, 2023 06:23
@juanlofer-eprosima juanlofer-eprosima changed the title Only create readers, writers, and tracks when they are going to be used Only create readers, writers, and tracks when they are going to be used [19421] Sep 25, 2023
@rsanchez15 rsanchez15 mentioned this pull request Sep 27, 2023
Merged
juanlofer-eprosima
juanlofer-eprosima requested changes Sep 27, 2023
View reviewed changes
Copy link
Contributor

@juanlofer-eprosima juanlofer-eprosima left a comment

Choose a reason for hiding this comment

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

Brave PR, keep it up!

ddspipe_core/include/ddspipe_core/core/DdsPipe.hpp Outdated Show resolved Hide resolved
ddspipe_core/include/ddspipe_core/communication/dds/DdsBridge.hpp Outdated Show resolved Hide resolved
ddspipe_core/include/ddspipe_core/communication/dds/DdsBridge.hpp Outdated Show resolved Hide resolved
ddspipe_core/include/ddspipe_core/communication/dds/DdsBridge.hpp Outdated Show resolved Hide resolved
ddspipe_core/include/ddspipe_core/core/DdsPipe.hpp Outdated Show resolved Hide resolved
ddspipe_core/src/cpp/communication/dds/DdsBridge.cpp Outdated Show resolved Hide resolved
ddspipe_core/src/cpp/communication/dds/DdsBridge.cpp Outdated Show resolved Hide resolved
ddspipe_core/src/cpp/communication/dds/DdsBridge.cpp Outdated Show resolved Hide resolved
ddspipe_core/src/cpp/communication/dds/DdsBridge.cpp Outdated Show resolved Hide resolved
ddspipe_core/src/cpp/communication/dds/DdsBridge.cpp Outdated Show resolved Hide resolved
juanlofer-eprosima
juanlofer-eprosima reviewed Oct 2, 2023
View reviewed changes
ddspipe_core/src/cpp/core/DdsPipe.cpp Outdated Show resolved Hide resolved
ddspipe_core/include/ddspipe_core/core/DdsPipe.hpp Outdated Show resolved Hide resolved
ddspipe_core/src/cpp/core/DdsPipe.cpp Outdated Show resolved Hide resolved
@Tempate Tempate changed the title Only create readers, writers, and tracks when they are going to be used [19421] Only create readers, writers, and tracks when they are needed [19421] Oct 3, 2023
@Tempate Tempate changed the title Only create readers, writers, and tracks when they are needed [19421] Only create readers, writers, and tracks when they are going to be used [19421] Oct 3, 2023
@Tempate Tempate mentioned this pull request Oct 6, 2023
Merged
juanlofer-eprosima
juanlofer-eprosima requested changes Oct 9, 2023
View reviewed changes
Copy link
Contributor

@juanlofer-eprosima juanlofer-eprosima left a comment

Choose a reason for hiding this comment

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

Super NIT

ddspipe_core/include/ddspipe_core/configuration/DdsPipeConfiguration.hpp Outdated Show resolved Hide resolved
@Tempate
Save the discoverer's id when creating a DdsBridge
fa2619f 
Signed-off-by: tempate <[email protected]>
@Tempate
Only create the necessary readers and writers
d91fd37 
Signed-off-by: tempate <[email protected]>
@Tempate
Improve readability
ef0043d 
Signed-off-by: tempate <[email protected]>
@Tempate
Add a new subscriber to the tracks in a bridge
49cd062 
Signed-off-by: tempate <[email protected]>
@Tempate
Remove a dropped subscriber from its bridge
f866319 
Signed-off-by: tempate <[email protected]>
@Tempate
Remove unnecessary removed_topic_nts function
a8976a6 
Signed-off-by: tempate <[email protected]>
@Tempate
Delete the tracks that don't have any writers
02844a9 
Signed-off-by: tempate <[email protected]>
@Tempate
Improved readability
ad0638f 
Signed-off-by: tempate <[email protected]>
@Tempate
Uncrustify
bd8ca33 
Signed-off-by: tempate <[email protected]>
@Tempate
Allow participants to have multiple subscribers
b1f6795 
Signed-off-by: tempate <[email protected]>
@Tempate
Doxygen, Windows support, and improved readability
ff007a4 
Signed-off-by: tempate <[email protected]>
@Tempate
Uncrustify
c2a9127 
Signed-off-by: tempate <[email protected]>
@Tempate
Make the deletion of unused entities configurable
69651e0 
Signed-off-by: tempate <[email protected]>
@Tempate
Don't delete unused entities by default
4a8a226 
Signed-off-by: tempate <[email protected]>
@Tempate
Make dynamic tracks configurable
3917182 
Signed-off-by: tempate <[email protected]>
@Tempate
Three small patches to pass the tests
58876a7 
Signed-off-by: tempate <[email protected]>
@Tempate
Move dynamic_tracks's init to avoid warning
b75a064 
Signed-off-by: tempate <[email protected]>
@Tempate
Fix bug in forwarding routes
0c4db75 
Signed-off-by: tempate <[email protected]>
@Tempate
Fix bug by removing debugging comments
7831e29 
Signed-off-by: tempate <[email protected]>
@Tempate
Rename dynamic tracks to remove unused entities
8bb6475 
Signed-off-by: tempate <[email protected]>
@Tempate
Fix segfault when removing elements from tracks
bf70c40 
Signed-off-by: tempate <[email protected]>
@Tempate
Rebase fix
667adcb 
Signed-off-by: tempate <[email protected]>
@Tempate
Apply suggestions
48972d1 
Signed-off-by: tempate <[email protected]>
@Tempate
Remove commented code
c2daf07 
Signed-off-by: tempate <[email protected]>
@Tempate
Apply suggestions
69b2613 
Signed-off-by: tempate <[email protected]>
@Tempate
Get endpoints with a filter
6e14556 
Signed-off-by: tempate <[email protected]>
@Tempate
Set builtin participants' discoverer
c31c57f 
Signed-off-by: tempate <[email protected]>
@Tempate
Apply suggestions
d71917e 
Signed-off-by: tempate <[email protected]>
@Tempate
Apply NIT suggestions
e5fc216 
Signed-off-by: tempate <[email protected]>
@Tempate
Uncrustify
5fca9c7 
Signed-off-by: tempate <[email protected]>
@Tempate
Check that the conf. is valid inside the DdsPipe
3dd87fa 
Signed-off-by: tempate <[email protected]>
@Tempate
Fix compilation error
266673e 
Signed-off-by: tempate <[email protected]>
@Tempate
Apply suggestions
49525e9 
Signed-off-by: tempate <[email protected]>
juanlofer-eprosima
juanlofer-eprosima approved these changes Oct 9, 2023
View reviewed changes
Copy link
Contributor

@juanlofer-eprosima juanlofer-eprosima left a comment

Choose a reason for hiding this comment

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

LGTM

@Tempate Tempate merged commit 47856ff into main Oct 9, 2023
12 checks passed
@Tempate Tempate deleted the feature/dynamic_tracks branch October 9, 2023 12:07
@Tempate Tempate mentioned this pull request Oct 16, 2023
Merged
@juanlofer-eprosima juanlofer-eprosima mentioned this pull request Nov 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juanlofer-eprosima juanlofer-eprosima juanlofer-eprosima approved these changes

@rsanchez15 rsanchez15 Awaiting requested review from rsanchez15

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

Successfully merging this pull request may close these issues.
2 participants
@Tempate @juanlofer-eprosima