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

Propagate servers list updates to PDPServer and PDPClient [12278] #2138

Merged
merged 12 commits into from
Aug 12, 2021
Merged

Propagate servers list updates to PDPServer and PDPClient [12278] #2138

merged 12 commits into from
Aug 12, 2021

Conversation

EduPonz
Copy link

@EduPonz EduPonz commented Aug 10, 2021

This PR brings the following functionality:

  1. DomainParticipantQos::wire_protocol().builtin.discovery_config.m_DiscoveryServers can only be modified to add servers, but not to remove them.
  2. Propagate new servers to PDPServer and PDPClient so that discovery can take place
  3. Add simple discovery test using discovery server to check that servers added both to a SERVER and a CLIENT have the expected effect.

EduPonz added 8 commits August 10, 2021 16:34
@EduPonz
Refs 12278: Only add new servers
757a892 
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz
Refs 12278: Update Discovery Server with new servers
70b0f7c 
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz
Refs 12278: Check that old server list is a subset of the new one whe…
5445459 
…n using DDS layer

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz
Refs 12278: Test now checks that servers cannot be removed from list
f361f74 
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz
Refs 12278: Manually match new server' PDP endpoints
6911c58 
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz
Refs 12278: Discovery test for adding servers
2cd20fa 
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz
Refs 12278: Randomize test parameters
e819801 
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz
Refs 12278: Uncrustify
22287fe 
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz EduPonz requested a review from JLBuenoLopez August 10, 2021 18:58
@EduPonz
Refs 12278: Address Windows warning
d19aeb9 
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
JLBuenoLopez
JLBuenoLopez suggested changes Aug 11, 2021
View reviewed changes
src/cpp/rtps/participant/RTPSParticipantImpl.cpp Show resolved Hide resolved
src/cpp/fastdds/domain/DomainParticipantImpl.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/builtin/discovery/participant/PDPClient.h Outdated Show resolved Hide resolved
src/cpp/rtps/builtin/discovery/participant/PDPClient.h Outdated Show resolved Hide resolved
src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp Show resolved Hide resolved
src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp Show resolved Hide resolved
test/blackbox/common/DDSBlackboxTestsDiscovery.cpp Outdated Show resolved Hide resolved
test/blackbox/common/DDSBlackboxTestsDiscovery.cpp Outdated Show resolved Hide resolved
test/blackbox/common/DDSBlackboxTestsDiscovery.cpp Outdated Show resolved Hide resolved
EduPonz added 2 commits August 11, 2021 11:57
@EduPonz
Refs 12278: Apply suggestions
31e2678 
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz
Refs 12278: Use nullptr and add headers
e4bbd50 
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
JLBuenoLopez
JLBuenoLopez approved these changes Aug 11, 2021
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment •
edited
Loading

Choose a reason for hiding this comment

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

👍 LGTM with greenish CI

@EduPonz
Refs 12278: Add SystemInfo source to CMake
ec1b395 
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@JLBuenoLopez JLBuenoLopez mentioned this pull request Aug 12, 2021
Merged
3 tasks
JLBuenoLopez
JLBuenoLopez approved these changes Aug 12, 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.

Discovery server test failure is not one of the common ones but I have reproduce it in master. It seems that it is flaky and fails from time to time (but really sparingly).

@EduPonz EduPonz merged this pull request into feature/rtps-participant-qos-updates Aug 12, 2021
@EduPonz EduPonz deleted the feature/update_server_list branch August 12, 2021 07:45
JLBuenoLopez pushed a commit that referenced this pull request Aug 12, 2021
@EduPonz @JLBuenoLopez
Propagate servers list updates to PDPServer and PDPClient (#2138)
818a419 
* Refs 12278: Only add new servers

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Update Discovery Server with new servers

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Check that old server list is a subset of the new one when using DDS layer

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Test now checks that servers cannot be removed from list

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Manually match new server' PDP endpoints

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Discovery test for adding servers

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Randomize test parameters

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Uncrustify

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Address Windows warning

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Apply suggestions

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Use nullptr and add headers

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Add SystemInfo source to CMake

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
EduPonz added a commit that referenced this pull request Aug 12, 2021
@JLBuenoLopez @EduPonz
Allow DomainParticipantQos updates for UserDataQosPolicy and adding n…
f96903c 
…ew servers to the remote server list (#2142)

* Add empty api (#2111)

* Refs 12196: add empty api

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: apply review suggestions

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* DomainParticipantQos propagation to the RTPS layer implementation (#2113)

* Refs #12196: link dds and rtps layers

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: modify and announce change in user data QoS

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: update remote servers list

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: add update_attributes method to RTPSParticipant mock class

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: protect remote servers list with PDP mutex

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: fix ParticipantTests.CheckDomainParticipantQos test. The first time the QoS was set it was calling to the update_attributes method

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs 12196: apply review suggestions

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: uncrustify

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: reset hasChanged flag

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: refactor to check if Qos has changed both in DDS and RTPS layers

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: apply review suggestions

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: fix ParticipantTests

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: bug fix

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* WireProtocolConfigQos update through DomainParticipant::set_qos (#2131)

* Refs 12197: Check that the only modification in wire_protocol is the list of servers

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12196: Unit test for changing WireProtocolQosPolicy

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12196: Apply suggestions

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12196: Uncrustify

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Add UserDataQoS blackbox test (#2130)

* Refs #12196: modify Blackbox PubSubParticipant API to include UserDataQosPolicy

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: add discovery participant API to PubSubParticipant

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: add blackbox test that checks user data QoS update

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: uncrustify

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: fix deadlock in Blackbox test

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: apply review suggestions

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Propagate servers list updates to PDPServer and PDPClient (#2138)

* Refs 12278: Only add new servers

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Update Discovery Server with new servers

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Check that old server list is a subset of the new one when using DDS layer

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Test now checks that servers cannot be removed from list

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Manually match new server' PDP endpoints

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Discovery test for adding servers

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Randomize test parameters

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Uncrustify

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Address Windows warning

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Apply suggestions

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Use nullptr and add headers

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Add SystemInfo source to CMake

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

Co-authored-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
SamuelWHalodi pushed a commit to SamuelWHalodi/Fast-DDS that referenced this pull request Oct 4, 2021
@JLBuenoLopez @EduPonz @SamuelWHalodi
Allow DomainParticipantQos updates for UserDataQosPolicy and adding n…
5fdfb29 
…ew servers to the remote server list (eProsima#2142)

* Add empty api (eProsima#2111)

* Refs 12196: add empty api

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: apply review suggestions

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* DomainParticipantQos propagation to the RTPS layer implementation (eProsima#2113)

* Refs #12196: link dds and rtps layers

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: modify and announce change in user data QoS

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: update remote servers list

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: add update_attributes method to RTPSParticipant mock class

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: protect remote servers list with PDP mutex

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: fix ParticipantTests.CheckDomainParticipantQos test. The first time the QoS was set it was calling to the update_attributes method

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs 12196: apply review suggestions

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: uncrustify

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: reset hasChanged flag

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: refactor to check if Qos has changed both in DDS and RTPS layers

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: apply review suggestions

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: fix ParticipantTests

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: bug fix

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* WireProtocolConfigQos update through DomainParticipant::set_qos (eProsima#2131)

* Refs 12197: Check that the only modification in wire_protocol is the list of servers

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12196: Unit test for changing WireProtocolQosPolicy

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12196: Apply suggestions

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12196: Uncrustify

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Add UserDataQoS blackbox test (eProsima#2130)

* Refs #12196: modify Blackbox PubSubParticipant API to include UserDataQosPolicy

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: add discovery participant API to PubSubParticipant

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: add blackbox test that checks user data QoS update

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: uncrustify

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: fix deadlock in Blackbox test

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #12196: apply review suggestions

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Propagate servers list updates to PDPServer and PDPClient (eProsima#2138)

* Refs 12278: Only add new servers

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Update Discovery Server with new servers

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Check that old server list is a subset of the new one when using DDS layer

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Test now checks that servers cannot be removed from list

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Manually match new server' PDP endpoints

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Discovery test for adding servers

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Randomize test parameters

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Uncrustify

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Address Windows warning

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Apply suggestions

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Use nullptr and add headers

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12278: Add SystemInfo source to CMake

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

Co-authored-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
Signed-off-by: Samuel Wilhelmsson <samuel@halodi.com>
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
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@EduPonz @JLBuenoLopez