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

Add transport interface to concatenate transports [12290] #2103

Merged
merged 16 commits into from
Dec 16, 2021
Merged

Add transport interface to concatenate transports [12290] #2103

merged 16 commits into from
Dec 16, 2021

Conversation

richiware
Copy link
Member

@richiware richiware commented Jul 29, 2021 •
edited
Loading

Transports can be implemented inheriting from this interface and getting the feature of concatenate them. A typical use case is writing a transport that perform an action or modification on the output UDP datagram before being sent to UDPv4 Transport.

auto udp_transport = std::make_shared<fastdds::rtps::UDPv4TransportDescriptor>();

auto my_transport = std::make_shared<fastdds::rtps::MyTransportDescriptor>(
    udp_transport);

participant_qos.transport().use_builtin_transports = false;
participant_qos.transport().user_transports.push_back(my_transport);

@richiware richiware changed the title New feature: chaining transports New feature: chaining transports [12290] Jul 29, 2021
@richiware richiware force-pushed the feature/chaining-transports branch 2 times, most recently from b735b5d to d520b99 Compare July 29, 2021 09:49
@richiware richiware force-pushed the feature/chaining-transports branch from 9070752 to 05c5f11 Compare September 2, 2021 09:33
@richiware richiware added this to the v2.5.0 milestone Nov 24, 2021
@richiware richiware changed the title New feature: chaining transports [12290] Add transport interface to concatenate transports [12290] Nov 24, 2021
@richiware richiware force-pushed the feature/chaining-transports branch from 3cdc61c to 789085f Compare November 24, 2021 15:01
@richiware
Copy link
Member Author

@richiprosima Please test this

This was referenced Dec 1, 2021
Closed
Merged
MiguelCompany and others added 10 commits December 9, 2021 10:52
@MiguelCompany @JLBuenoLopez
Squash of low-bandwidth-plugins development:
791c3cd 
Refs #2731. Initial version of DataTransformTransport

Refs #2731. DataTransformTransport implementation complete + unit test

Refs #2731. Refactor of DataTransformTransport with middle class ChainingTransport

Refs #2752. Refactor to pass participant properties on transport creation

Refs #2691. Added some utils to StringMatching class

Refs #2693. Headers reduction transport implementation

Refs #2759. Blackbox test and bug fixing

Refs #2759. Fixing compiler warnings

Refs #2692. Added Payload Compression Transport (ZLIB only)

Refs #2692. Added BlackBox test for ZLib transport

Refs #2692. Added BZIP2 compression

Refs #2692. Added BlackBox test for BZip2 transport

Refs #2692. Fixing compiler warnings

Refs #2692. Added automatic compression implementation

Refs #2692. Added blackbox test for automatic compression

Refs #2692. Only compiling compression transport when compression libraries are found

Refs #2727. Added source timestamp transport

Refs #2727. Added blackbox test for source timestamp transport

Refs #2693. Removing unsupported entity ids

Refs #2693. Fix of bug in HeaderReduction_Recover

Refs #2769. Documenting interfaces and some optimizations

Refs #2691. Added blackbox test chaining several transports

Better doc on some transports

Refs #2787. Fixed bug in SourceTimestamp transport

Refs #2787. Added examples to test interoperability

Refs #2787. Interop example improved

Refs #2787. Added dump capabiliy to HeaderReductionTransport

Refs #2787. Added dump to test participants

Refs #2787. Fixed bug in SourceTimestampTransport

Refs #2787. Improving tests and dump

Refs #2787. Improving example

Refs #2787. Fixing entiy id packing

Refs #2787. Message sent improved

Refs #2787. Added dump of low level data

Refs #2787. Testing improvements

Refs #2787. Changing data type

Refs #2787. Test data

Refs #2787. Adding parameters to endpoints

Refs #2787. Conditional compilation of debug dump

Refs #2691. Changing example name

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@richiware @JLBuenoLopez
Fixing compilation errors after rebase.
0584e36 
Signed-off-by: Ricardo González Moreno <correoricky@gmail.com>

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
New features
4da1c94 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@richiware @JLBuenoLopez
Refs #10121. Fix error creating output channel.
1597f32 
Signed-off-by: Ricardo González Moreno <correoricky@gmail.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@richiware @JLBuenoLopez
Refs #12290. Uncrustify
fecd76e 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@richiware @JLBuenoLopez
Moved example to other repo
653fbab 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@richiware @JLBuenoLopez
Refs #12290. Fix warning on windows
4687dd9 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@richiware @JLBuenoLopez
Refs #12290. Fix warning on windows
2bac11a 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@richiware @JLBuenoLopez
Refs #12290. Fix warnings on Mac
cfee2f5 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@richiware @JLBuenoLopez
Refs 12290. Fix old uncrustify
a4382a9 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez JLBuenoLopez force-pushed the feature/chaining-transports branch from 2070a34 to a4382a9 Compare December 9, 2021 09:52
JLBuenoLopez
JLBuenoLopez reviewed Dec 10, 2021
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

Partial review: implementation

include/fastdds/rtps/network/SenderResource.h Outdated Show resolved Hide resolved
include/fastdds/rtps/transport/ChainingTransport.h Outdated Show resolved Hide resolved
include/fastdds/rtps/transport/ChainingTransport.h Outdated Show resolved Hide resolved
include/fastdds/rtps/transport/ChainingTransport.h Outdated Show resolved Hide resolved
include/fastdds/rtps/transport/ChainingTransport.h Show resolved Hide resolved
src/cpp/rtps/transport/ChainingSenderResource.hpp Outdated Show resolved Hide resolved
src/cpp/rtps/transport/ChainingSenderResource.hpp Outdated Show resolved Hide resolved
src/cpp/rtps/transport/ChainingTransport.cpp Show resolved Hide resolved
src/cpp/rtps/transport/ChainingTransport.cpp Show resolved Hide resolved
include/fastrtps/utils/StringMatching.h Outdated Show resolved Hide resolved
JLBuenoLopez
JLBuenoLopez suggested changes Dec 13, 2021
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

This feature is not being tested. Tests should be included before considering merging.

There are some additions that are not required for this feature. Some code cleaning is strongly suggested.

test/blackbox/BlackboxTestsTransportCustom.cpp Outdated Show resolved Hide resolved
test/blackbox/BlackboxTestsTransportCustom.cpp Outdated Show resolved Hide resolved
test/blackbox/BlackboxTestsTransportCustom.cpp Outdated Show resolved Hide resolved
test/blackbox/BlackboxTestsTransportCustom.cpp Outdated Show resolved Hide resolved
include/fastdds/rtps/transport/ChainingTransport.h Outdated Show resolved Hide resolved
richiware and others added 5 commits December 13, 2021 10:38
@richiware @JLBuenoLopez
Apply suggestions from code review
5d2a325 
Co-authored-by: José Luis Bueno López <69244257+JLBuenoLopez-eProsima@users.noreply.github.com>
@richiware
Refs #12290. Apply suggestions.
5e0863e 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #12290. Apply suggestions
5f11b50 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #12290. Add blackbox test.
0219013 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #12290. Fix license
372d7da 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
JLBuenoLopez
JLBuenoLopez approved these changes Dec 14, 2021
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

LGTM with greenish CI

JLBuenoLopez
JLBuenoLopez suggested changes Dec 15, 2021
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

Please, fix Clang warning in MacOS platform

'get_configuration' overrides a member function but is not marked 'override'

@richiware
Refs #12290. Fix warning on mac
373a503 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Copy link
Member Author

Please, fix Clang warning in MacOS platform

'get_configuration' overrides a member function but is not marked 'override'

Fixed

@JLBuenoLopez
Copy link
Contributor

@richiprosima Please test mac

JLBuenoLopez
JLBuenoLopez approved these changes Dec 16, 2021
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

LGTM. CI failures are unrelated

@JLBuenoLopez JLBuenoLopez added the ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. label Dec 16, 2021
@MiguelCompany MiguelCompany merged commit 7b97f19 into master Dec 16, 2021
@MiguelCompany MiguelCompany deleted the feature/chaining-transports branch December 16, 2021 07:48
@JLBuenoLopez JLBuenoLopez mentioned this pull request Dec 16, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JLBuenoLopez JLBuenoLopez JLBuenoLopez approved these changes

Assignees
No one assigned
Labels
ready-to-merge Ready to be merged. CI and changes have been reviewed and approved.
Projects
None yet
Milestone
v2.5.0
Development

Successfully merging this pull request may close these issues.
3 participants
@richiware @JLBuenoLopez @MiguelCompany