From 9bee1f2236c321d73f20a623da4bbe4946f950e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Luis=20Bueno=20L=C3=B3pez?= <69244257+JLBuenoLopez-eProsima@users.noreply.github.com> Date: Tue, 3 Aug 2021 08:50:34 +0200 Subject: [PATCH 1/5] Add empty api (#2111) * Refs 12196: add empty api Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: apply review suggestions Signed-off-by: JLBuenoLopez-eProsima --- include/fastdds/rtps/participant/RTPSParticipant.h | 8 ++++++++ src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp | 5 +++++ src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp | 5 +++++ src/cpp/rtps/participant/RTPSParticipant.cpp | 6 ++++++ src/cpp/rtps/participant/RTPSParticipantImpl.cpp | 7 +++++++ src/cpp/rtps/participant/RTPSParticipantImpl.h | 8 ++++++++ 6 files changed, 39 insertions(+) diff --git a/include/fastdds/rtps/participant/RTPSParticipant.h b/include/fastdds/rtps/participant/RTPSParticipant.h index 7a0d87b14ad..c47cc717244 100644 --- a/include/fastdds/rtps/participant/RTPSParticipant.h +++ b/include/fastdds/rtps/participant/RTPSParticipant.h @@ -148,6 +148,14 @@ class RTPS_DllAPI RTPSParticipant const TopicAttributes& topicAtt, const ReaderQos& rqos); + /** + * Update participant attributes. + * @param patt New participant attributes. + * @return True on success, false otherwise. + */ + bool update_attributes( + const RTPSParticipantAttributes& patt); + /** * Update writer QOS * @param Writer to update diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp index ec5f01e64d5..0747115fa16 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp @@ -891,6 +891,11 @@ bool PDPServer::server_update_routine() return pending_work && discovery_db_.is_enabled(); } +void PDPServer::update_remote_servers_list() +{ + return; +} + bool PDPServer::process_writers_acknowledgements() { logInfo(RTPS_PDP_SERVER, "process_writers_acknowledgements start"); diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp index d8f50103fda..94dd48b9db6 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp @@ -164,6 +164,11 @@ class PDPServer : public fastrtps::rtps::PDP */ bool server_update_routine(); + /* + * Update the list of remote servers + */ + void update_remote_servers_list(); + fastdds::rtps::ddb::DiscoveryDataBase& discovery_db(); const RemoteServerList_t& servers(); diff --git a/src/cpp/rtps/participant/RTPSParticipant.cpp b/src/cpp/rtps/participant/RTPSParticipant.cpp index 2f17b2fe930..0b66d46f53a 100644 --- a/src/cpp/rtps/participant/RTPSParticipant.cpp +++ b/src/cpp/rtps/participant/RTPSParticipant.cpp @@ -91,6 +91,12 @@ bool RTPSParticipant::registerReader( return mp_impl->registerReader(Reader, topicAtt, rqos); } +bool RTPSParticipant::update_attributes( + const RTPSParticipantAttributes& patt) +{ + return mp_impl->update_attributes(patt); +} + bool RTPSParticipant::updateWriter( RTPSWriter* Writer, const TopicAttributes& topicAtt, diff --git a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp index bd79b132187..e047ee4387b 100644 --- a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp +++ b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp @@ -1170,6 +1170,13 @@ bool RTPSParticipantImpl::registerReader( return this->mp_builtinProtocols->addLocalReader(reader, topicAtt, rqos); } +bool RTPSParticipantImpl::update_attributes( + const RTPSParticipantAttributes& patt) +{ + static_cast(patt); + return false; +} + bool RTPSParticipantImpl::updateLocalWriter( RTPSWriter* Writer, const TopicAttributes& topicAtt, diff --git a/src/cpp/rtps/participant/RTPSParticipantImpl.h b/src/cpp/rtps/participant/RTPSParticipantImpl.h index 032fe0bcc2c..a29a2d5125a 100644 --- a/src/cpp/rtps/participant/RTPSParticipantImpl.h +++ b/src/cpp/rtps/participant/RTPSParticipantImpl.h @@ -797,6 +797,14 @@ class RTPSParticipantImpl const TopicAttributes& topicAtt, const ReaderQos& rqos); + /** + * Update participant attributes. + * @param patt New participant attributes. + * @return True on success, false otherwise. + */ + bool update_attributes( + const RTPSParticipantAttributes& patt); + /** * Update local writer QoS * @param Writer Writer to update From 96ed77a7093c7fd811e639080fbca47994bac359 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Luis=20Bueno=20L=C3=B3pez?= <69244257+JLBuenoLopez-eProsima@users.noreply.github.com> Date: Tue, 10 Aug 2021 09:04:41 +0200 Subject: [PATCH 2/5] DomainParticipantQos propagation to the RTPS layer implementation (#2113) * Refs #12196: link dds and rtps layers Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: modify and announce change in user data QoS Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: update remote servers list Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: add update_attributes method to RTPSParticipant mock class Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: protect remote servers list with PDP mutex Signed-off-by: JLBuenoLopez-eProsima * 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 * Refs 12196: apply review suggestions Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: uncrustify Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: reset hasChanged flag Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: refactor to check if Qos has changed both in DDS and RTPS layers Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: apply review suggestions Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: fix ParticipantTests Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: bug fix Signed-off-by: JLBuenoLopez-eProsima --- .../fastdds/dds/core/policy/QosPolicies.hpp | 2 +- .../rtps/participant/RTPSParticipant.h | 2 +- .../fastdds/domain/DomainParticipantImpl.cpp | 21 ++++- .../fastdds/domain/DomainParticipantImpl.hpp | 12 ++- src/cpp/rtps/RTPSDomain.cpp | 1 + src/cpp/rtps/builtin/BuiltinProtocols.cpp | 5 ++ .../builtin/discovery/participant/PDP.cpp | 1 + .../discovery/participant/PDPClient.cpp | 46 +++++----- .../discovery/participant/PDPServer.cpp | 87 +++++++++++-------- .../discovery/participant/PDPServer.hpp | 6 ++ src/cpp/rtps/participant/RTPSParticipant.cpp | 4 +- .../rtps/participant/RTPSParticipantImpl.cpp | 43 ++++++++- .../rtps/participant/RTPSParticipantImpl.h | 2 +- .../fastdds/domain/DomainParticipantImpl.hpp | 2 +- .../rtps/participant/RTPSParticipant.h | 7 ++ 15 files changed, 174 insertions(+), 67 deletions(-) diff --git a/include/fastdds/dds/core/policy/QosPolicies.hpp b/include/fastdds/dds/core/policy/QosPolicies.hpp index 8093e1b9c86..d021dfd8567 100644 --- a/include/fastdds/dds/core/policy/QosPolicies.hpp +++ b/include/fastdds/dds/core/policy/QosPolicies.hpp @@ -105,7 +105,7 @@ class QosPolicy { public: - //! Boolean that indicates if the Qos has been changed + //! Boolean that indicates if the Qos has been changed with respect to the default Qos. bool hasChanged; /** diff --git a/include/fastdds/rtps/participant/RTPSParticipant.h b/include/fastdds/rtps/participant/RTPSParticipant.h index c47cc717244..fb7dfabeae0 100644 --- a/include/fastdds/rtps/participant/RTPSParticipant.h +++ b/include/fastdds/rtps/participant/RTPSParticipant.h @@ -153,7 +153,7 @@ class RTPS_DllAPI RTPSParticipant * @param patt New participant attributes. * @return True on success, false otherwise. */ - bool update_attributes( + void update_attributes( const RTPSParticipantAttributes& patt); /** diff --git a/src/cpp/fastdds/domain/DomainParticipantImpl.cpp b/src/cpp/fastdds/domain/DomainParticipantImpl.cpp index f13850030f9..9395c4930da 100644 --- a/src/cpp/fastdds/domain/DomainParticipantImpl.cpp +++ b/src/cpp/fastdds/domain/DomainParticipantImpl.cpp @@ -29,6 +29,7 @@ #include #include +#include #include #include #include @@ -343,7 +344,15 @@ ReturnCode_t DomainParticipantImpl::set_qos( { return ReturnCode_t::RETCODE_IMMUTABLE_POLICY; } - set_qos(qos_, qos_to_set, !enabled); + + if (set_qos(qos_, qos_to_set, !enabled) && enabled) + { + // Notify the participant that there is a QoS update + fastrtps::rtps::RTPSParticipantAttributes patt; + set_attributes_from_qos(patt, qos_); + rtps_participant_->update_attributes(patt); + } + return ReturnCode_t::RETCODE_OK; } @@ -1733,11 +1742,13 @@ bool DomainParticipantImpl::has_active_entities() return false; } -void DomainParticipantImpl::set_qos( +bool DomainParticipantImpl::set_qos( DomainParticipantQos& to, const DomainParticipantQos& from, bool first_time) { + bool qos_should_be_updated = false; + if (!(to.entity_factory() == from.entity_factory())) { to.entity_factory() = from.entity_factory(); @@ -1746,6 +1757,10 @@ void DomainParticipantImpl::set_qos( { to.user_data() = from.user_data(); to.user_data().hasChanged = true; + if (!first_time) + { + qos_should_be_updated = true; + } } if (first_time && !(to.allocation() == from.allocation())) { @@ -1767,6 +1782,8 @@ void DomainParticipantImpl::set_qos( { to.name() = from.name(); } + + return qos_should_be_updated; } fastrtps::types::ReturnCode_t DomainParticipantImpl::check_qos( diff --git a/src/cpp/fastdds/domain/DomainParticipantImpl.hpp b/src/cpp/fastdds/domain/DomainParticipantImpl.hpp index 13773b0c4b4..ae11c5f4885 100644 --- a/src/cpp/fastdds/domain/DomainParticipantImpl.hpp +++ b/src/cpp/fastdds/domain/DomainParticipantImpl.hpp @@ -559,7 +559,17 @@ class DomainParticipantImpl std::string get_inner_type_name( const fastrtps::rtps::SampleIdentity& id) const; - static void set_qos( + /** + * Set the DomainParticipantQos checking if the Qos can be updated or not + * + * @param to DomainParticipantQos to be updated + * @param from DomainParticipantQos desired + * @param first_time Whether the DomainParticipant has been already initialized or not + * + * @return true if there has been a changed in one of the attributes that can be updated. + * false otherwise. + */ + static bool set_qos( DomainParticipantQos& to, const DomainParticipantQos& from, bool first_time); diff --git a/src/cpp/rtps/RTPSDomain.cpp b/src/cpp/rtps/RTPSDomain.cpp index f586103b80b..312b74738fb 100644 --- a/src/cpp/rtps/RTPSDomain.cpp +++ b/src/cpp/rtps/RTPSDomain.cpp @@ -394,6 +394,7 @@ RTPSParticipant* RTPSDomain::clientServerEnvironmentCreationOverride( RTPSParticipantAttributes client_att(att); // Retrieve the info from the environment variable + // TODO(jlbueno) This should be protected with the PDP mutex. if (!load_environment_server_info(client_att.builtin.discovery_config.m_DiscoveryServers)) { // it's not an error, the environment variable may not be set. Any issue with environment diff --git a/src/cpp/rtps/builtin/BuiltinProtocols.cpp b/src/cpp/rtps/builtin/BuiltinProtocols.cpp index 834e35b3992..be088e77d59 100644 --- a/src/cpp/rtps/builtin/BuiltinProtocols.cpp +++ b/src/cpp/rtps/builtin/BuiltinProtocols.cpp @@ -79,6 +79,9 @@ bool BuiltinProtocols::initBuiltinProtocols( m_metatrafficUnicastLocatorList = m_att.metatrafficUnicastLocatorList; m_metatrafficMulticastLocatorList = m_att.metatrafficMulticastLocatorList; m_initialPeersList = m_att.initialPeersList; + + // TODO(jlbueno) The access to the list should be protected with the PDP mutex but requires a refactor on PDPClient + // and PDPServer: read the remote servers list after the initialization of PDP. m_DiscoveryServers = m_att.discovery_config.m_DiscoveryServers; transform_server_remote_locators(p_part->network_factory()); @@ -160,6 +163,8 @@ bool BuiltinProtocols::updateMetatrafficLocators( void BuiltinProtocols::transform_server_remote_locators( NetworkFactory& nf) { + // TODO(jlbueno) The access to the list should be protected with the PDP mutex but requires a refactor on PDPClient + // and PDPServer: read the remote servers list after the initialization of PDP. for (eprosima::fastdds::rtps::RemoteServerAttributes& rs : m_DiscoveryServers) { for (Locator_t& loc : rs.metatrafficUnicastLocatorList) diff --git a/src/cpp/rtps/builtin/discovery/participant/PDP.cpp b/src/cpp/rtps/builtin/discovery/participant/PDP.cpp index 3988d9515ef..cba52305b2b 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDP.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDP.cpp @@ -1073,6 +1073,7 @@ ParticipantProxyData* PDP::get_participant_proxy_data( std::list& PDP::remote_server_attributes() { + std::unique_lock lock(*getMutex()); return mp_builtin->m_DiscoveryServers; } diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp index 74ba1bb5c50..e1269ed353a 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp @@ -215,19 +215,22 @@ bool PDPClient::createPDPEndpoints() // mp_RTPSParticipant->set_endpoint_rtps_protection_supports(rout, false); //#endif // Initial peer list doesn't make sense in server scenario. Client should match its server list - for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) { - std::lock_guard data_guard(temp_data_lock_); - temp_writer_data_.clear(); - temp_writer_data_.guid(it.GetPDPWriter()); - temp_writer_data_.set_multicast_locators(it.metatrafficMulticastLocatorList, network); - temp_writer_data_.set_remote_unicast_locators(it.metatrafficUnicastLocatorList, network); - temp_writer_data_.m_qos.m_durability.kind = TRANSIENT_DURABILITY_QOS; // Server Information must be persistent - temp_writer_data_.m_qos.m_reliability.kind = RELIABLE_RELIABILITY_QOS; + std::lock_guard lock(*getMutex()); - mp_PDPReader->matched_writer_add(temp_writer_data_); + for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) + { + std::lock_guard data_guard(temp_data_lock_); + temp_writer_data_.clear(); + temp_writer_data_.guid(it.GetPDPWriter()); + temp_writer_data_.set_multicast_locators(it.metatrafficMulticastLocatorList, network); + temp_writer_data_.set_remote_unicast_locators(it.metatrafficUnicastLocatorList, network); + temp_writer_data_.m_qos.m_durability.kind = TRANSIENT_DURABILITY_QOS; // Server Information must be persistent + temp_writer_data_.m_qos.m_reliability.kind = RELIABLE_RELIABILITY_QOS; + + mp_PDPReader->matched_writer_add(temp_writer_data_); + } } - } else { @@ -267,19 +270,22 @@ bool PDPClient::createPDPEndpoints() //#if HAVE_SECURITY // mp_RTPSParticipant->set_endpoint_rtps_protection_supports(wout, false); //#endif - for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) { - std::lock_guard data_guard(temp_data_lock_); - temp_reader_data_.clear(); - temp_reader_data_.guid(it.GetPDPReader()); - temp_reader_data_.set_multicast_locators(it.metatrafficMulticastLocatorList, network); - temp_reader_data_.set_remote_unicast_locators(it.metatrafficUnicastLocatorList, network); - temp_reader_data_.m_qos.m_durability.kind = TRANSIENT_LOCAL_DURABILITY_QOS; - temp_reader_data_.m_qos.m_reliability.kind = RELIABLE_RELIABILITY_QOS; + std::lock_guard lock(*getMutex()); - mp_PDPWriter->matched_reader_add(temp_reader_data_); + for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) + { + std::lock_guard data_guard(temp_data_lock_); + temp_reader_data_.clear(); + temp_reader_data_.guid(it.GetPDPReader()); + temp_reader_data_.set_multicast_locators(it.metatrafficMulticastLocatorList, network); + temp_reader_data_.set_remote_unicast_locators(it.metatrafficUnicastLocatorList, network); + temp_reader_data_.m_qos.m_durability.kind = TRANSIENT_LOCAL_DURABILITY_QOS; + temp_reader_data_.m_qos.m_reliability.kind = RELIABLE_RELIABILITY_QOS; + + mp_PDPWriter->matched_reader_add(temp_reader_data_); + } } - } else { diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp index 0747115fa16..1ab61903f0a 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp @@ -69,13 +69,17 @@ PDPServer::PDPServer( { // Add remote servers from environment variable RemoteServerList_t env_servers; - if (load_environment_server_info(env_servers)) { - for (auto server : env_servers) + std::lock_guard lock(*getMutex()); + + if (load_environment_server_info(env_servers)) { - mp_builtin->m_DiscoveryServers.push_back(server); - m_discovery.discovery_config.m_DiscoveryServers.push_back(server); - discovery_db_.add_server(server.guidPrefix); + for (auto server : env_servers) + { + mp_builtin->m_DiscoveryServers.push_back(server); + m_discovery.discovery_config.m_DiscoveryServers.push_back(server); + discovery_db_.add_server(server.guidPrefix); + } } } } @@ -260,20 +264,24 @@ bool PDPServer::createPDPEndpoints() // Enable unknown clients to reach this reader mp_PDPReader->enableMessagesFromUnkownWriters(true); - // Initial peer list doesn't make sense in server scenario. Client should match its server list - for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) { - std::lock_guard data_guard(temp_data_lock_); - temp_writer_data_.clear(); - temp_writer_data_.guid(it.GetPDPWriter()); - temp_writer_data_.set_multicast_locators(it.metatrafficMulticastLocatorList, network); - temp_writer_data_.set_remote_unicast_locators(it.metatrafficUnicastLocatorList, network); - // TODO check if this is correct, it is equal as PDPServer, but we do not know like this the durKind of the - // other server - temp_writer_data_.m_qos.m_durability.durabilityKind(durability_); - temp_writer_data_.m_qos.m_reliability.kind = fastrtps::RELIABLE_RELIABILITY_QOS; - - mp_PDPReader->matched_writer_add(temp_writer_data_); + std::lock_guard lock(*getMutex()); + + // Initial peer list doesn't make sense in server scenario. Client should match its server list + for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) + { + std::lock_guard data_guard(temp_data_lock_); + temp_writer_data_.clear(); + temp_writer_data_.guid(it.GetPDPWriter()); + temp_writer_data_.set_multicast_locators(it.metatrafficMulticastLocatorList, network); + temp_writer_data_.set_remote_unicast_locators(it.metatrafficUnicastLocatorList, network); + // TODO check if this is correct, it is equal as PDPServer, but we do not know like this the durKind of the + // other server + temp_writer_data_.m_qos.m_durability.durabilityKind(durability_); + temp_writer_data_.m_qos.m_reliability.kind = fastrtps::RELIABLE_RELIABILITY_QOS; + + mp_PDPReader->matched_writer_add(temp_writer_data_); + } } } // Could not create PDP Reader, so return false @@ -330,17 +338,21 @@ bool PDPServer::createPDPEndpoints() // Enable separate sending so the filter can be called for each change and reader proxy mp_PDPWriter->set_separate_sending(true); - for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) { - std::lock_guard data_guard(temp_data_lock_); - temp_reader_data_.clear(); - temp_reader_data_.guid(it.GetPDPReader()); - temp_reader_data_.set_multicast_locators(it.metatrafficMulticastLocatorList, network); - temp_reader_data_.set_remote_unicast_locators(it.metatrafficUnicastLocatorList, network); - temp_reader_data_.m_qos.m_durability.kind = fastrtps::TRANSIENT_LOCAL_DURABILITY_QOS; - temp_reader_data_.m_qos.m_reliability.kind = fastrtps::RELIABLE_RELIABILITY_QOS; - - mp_PDPWriter->matched_reader_add(temp_reader_data_); + std::lock_guard lock(*getMutex()); + + for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) + { + std::lock_guard data_guard(temp_data_lock_); + temp_reader_data_.clear(); + temp_reader_data_.guid(it.GetPDPReader()); + temp_reader_data_.set_multicast_locators(it.metatrafficMulticastLocatorList, network); + temp_reader_data_.set_remote_unicast_locators(it.metatrafficUnicastLocatorList, network); + temp_reader_data_.m_qos.m_durability.kind = fastrtps::TRANSIENT_LOCAL_DURABILITY_QOS; + temp_reader_data_.m_qos.m_reliability.kind = fastrtps::RELIABLE_RELIABILITY_QOS; + + mp_PDPWriter->matched_reader_add(temp_reader_data_); + } } } // Could not create PDP Writer, so return false @@ -1339,6 +1351,7 @@ bool PDPServer::pending_ack() std::vector PDPServer::servers_prefixes() { + std::lock_guard lock(*getMutex()); std::vector servers; for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) { @@ -1368,16 +1381,20 @@ void PDPServer::ping_remote_servers() LocatorList locators; // Iterate over the list of servers - for (auto& server : mp_builtin->m_DiscoveryServers) { + std::lock_guard lock(*getMutex()); - // If the server is the the ack_pending list, then add its GUID and locator to send the announcement - auto server_it = std::find(ack_pending_servers.begin(), ack_pending_servers.end(), server.guidPrefix); - if (server_it != ack_pending_servers.end()) + for (auto& server : mp_builtin->m_DiscoveryServers) { - // get the info to send to this already known locators - remote_readers.push_back(GUID_t(server.guidPrefix, c_EntityId_SPDPReader)); - locators.push_back(server.metatrafficUnicastLocatorList); + + // If the server is the the ack_pending list, then add its GUID and locator to send the announcement + auto server_it = std::find(ack_pending_servers.begin(), ack_pending_servers.end(), server.guidPrefix); + if (server_it != ack_pending_servers.end()) + { + // get the info to send to this already known locators + remote_readers.push_back(GUID_t(server.guidPrefix, c_EntityId_SPDPReader)); + locators.push_back(server.metatrafficUnicastLocatorList); + } } } send_announcement(discovery_db().cache_change_own_participant(), remote_readers, locators); diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp index 94dd48b9db6..5f4b896d327 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp @@ -171,6 +171,12 @@ class PDPServer : public fastrtps::rtps::PDP fastdds::rtps::ddb::DiscoveryDataBase& discovery_db(); + /** + * Access to the remote servers list + * This method is not thread safe. + * The return reference may be invalidated if the user modifies simultaneously the remote server list. + * @return constant reference to the remote servers list + */ const RemoteServerList_t& servers(); protected: diff --git a/src/cpp/rtps/participant/RTPSParticipant.cpp b/src/cpp/rtps/participant/RTPSParticipant.cpp index 0b66d46f53a..bc40bdb57b1 100644 --- a/src/cpp/rtps/participant/RTPSParticipant.cpp +++ b/src/cpp/rtps/participant/RTPSParticipant.cpp @@ -91,10 +91,10 @@ bool RTPSParticipant::registerReader( return mp_impl->registerReader(Reader, topicAtt, rqos); } -bool RTPSParticipant::update_attributes( +void RTPSParticipant::update_attributes( const RTPSParticipantAttributes& patt) { - return mp_impl->update_attributes(patt); + mp_impl->update_attributes(patt); } bool RTPSParticipant::updateWriter( diff --git a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp index e047ee4387b..898817a4e13 100644 --- a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp +++ b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp @@ -63,6 +63,8 @@ #include #include +#include + #include namespace eprosima { @@ -1170,11 +1172,46 @@ bool RTPSParticipantImpl::registerReader( return this->mp_builtinProtocols->addLocalReader(reader, topicAtt, rqos); } -bool RTPSParticipantImpl::update_attributes( +void RTPSParticipantImpl::update_attributes( const RTPSParticipantAttributes& patt) { - static_cast(patt); - return false; + // Check if there are changes + if (patt.builtin.discovery_config.m_DiscoveryServers == m_att.builtin.discovery_config.m_DiscoveryServers + && patt.userData == m_att.userData) + { + return; + } + + // Update RTPSParticipantAttributes member + m_att.builtin.discovery_config.m_DiscoveryServers = patt.builtin.discovery_config.m_DiscoveryServers; + m_att.userData = patt.userData; + + auto pdp = mp_builtinProtocols->mp_PDP; + { + std::unique_lock lock(*pdp->getMutex()); + + // Update user data + auto local_participant_proxy_data = pdp->getLocalParticipantProxyData(); + local_participant_proxy_data->m_userData.data_vec(m_att.userData); + + // Update remote servers list + if (m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::CLIENT || + m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::SUPER_CLIENT || + m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::SERVER || + m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::BACKUP) + { + mp_builtinProtocols->m_DiscoveryServers = m_att.builtin.discovery_config.m_DiscoveryServers; + if (m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::SERVER || + m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::BACKUP) + { + fastdds::rtps::PDPServer* pdp_server = static_cast(pdp); + pdp_server->update_remote_servers_list(); + } + } + } + + // Send DATA(P) + pdp->announceParticipantState(true); } bool RTPSParticipantImpl::updateLocalWriter( diff --git a/src/cpp/rtps/participant/RTPSParticipantImpl.h b/src/cpp/rtps/participant/RTPSParticipantImpl.h index a29a2d5125a..c9e6771a416 100644 --- a/src/cpp/rtps/participant/RTPSParticipantImpl.h +++ b/src/cpp/rtps/participant/RTPSParticipantImpl.h @@ -802,7 +802,7 @@ class RTPSParticipantImpl * @param patt New participant attributes. * @return True on success, false otherwise. */ - bool update_attributes( + void update_attributes( const RTPSParticipantAttributes& patt); /** diff --git a/test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp b/test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp index 1b2a8febe0a..74638d76b00 100644 --- a/test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp +++ b/test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp @@ -592,7 +592,7 @@ class DomainParticipantImpl return new SubscriberImpl(this, qos, listener); } - static void set_qos( + static bool set_qos( DomainParticipantQos& /*to*/, const DomainParticipantQos& /*from*/, bool /*first_time*/) diff --git a/test/mock/rtps/RTPSParticipant/fastdds/rtps/participant/RTPSParticipant.h b/test/mock/rtps/RTPSParticipant/fastdds/rtps/participant/RTPSParticipant.h index 0a9d88d5c69..d2ffba35678 100644 --- a/test/mock/rtps/RTPSParticipant/fastdds/rtps/participant/RTPSParticipant.h +++ b/test/mock/rtps/RTPSParticipant/fastdds/rtps/participant/RTPSParticipant.h @@ -163,6 +163,13 @@ class RTPS_DllAPI RTPSParticipant return attributes_; } + bool update_attributes( + const RTPSParticipantAttributes& patt) + { + static_cast(patt); + return true; + } + #if HAVE_SECURITY MOCK_METHOD1(is_security_enabled_for_writer, bool( From b3f2ab584772b422be5db437bf340eee0b0a728c Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Segrelles Date: Tue, 10 Aug 2021 10:55:05 +0200 Subject: [PATCH 3/5] 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 * Refs 12196: Unit test for changing WireProtocolQosPolicy Signed-off-by: Eduardo Ponz Segrelles * Refs 12196: Apply suggestions Signed-off-by: Eduardo Ponz Segrelles * Refs 12196: Uncrustify Signed-off-by: Eduardo Ponz Segrelles --- .../fastdds/domain/DomainParticipantImpl.cpp | 70 ++++- .../fastdds/domain/DomainParticipantImpl.hpp | 1 + .../dds/participant/ParticipantTests.cpp | 269 ++++++++++++++++++ 3 files changed, 337 insertions(+), 3 deletions(-) diff --git a/src/cpp/fastdds/domain/DomainParticipantImpl.cpp b/src/cpp/fastdds/domain/DomainParticipantImpl.cpp index 9395c4930da..7e65972d513 100644 --- a/src/cpp/fastdds/domain/DomainParticipantImpl.cpp +++ b/src/cpp/fastdds/domain/DomainParticipantImpl.cpp @@ -1770,9 +1770,14 @@ bool DomainParticipantImpl::set_qos( { to.properties() = from.properties(); } - if (first_time && !(to.wire_protocol() == from.wire_protocol())) + if (!(to.wire_protocol() == from.wire_protocol())) { to.wire_protocol() = from.wire_protocol(); + to.wire_protocol().hasChanged = true; + if (!first_time) + { + qos_should_be_updated = true; + } } if (first_time && !(to.transport() == from.transport())) { @@ -1814,8 +1819,67 @@ bool DomainParticipantImpl::can_qos_be_updated( } if (!(to.wire_protocol() == from.wire_protocol())) { - updatable = false; - logWarning(RTPS_QOS_CHECK, "WireProtocolConfigQos cannot be changed after the participant is enabled"); + // Check that the only modification was in wire_protocol().discovery_config.m_DiscoveryServers + if ((to.wire_protocol().builtin.discovery_config.m_DiscoveryServers == + from.wire_protocol().builtin.discovery_config.m_DiscoveryServers) || + (!(to.wire_protocol().builtin.discovery_config.m_DiscoveryServers == + from.wire_protocol().builtin.discovery_config.m_DiscoveryServers) && + (!(to.wire_protocol().prefix == from.wire_protocol().prefix) || + !(to.wire_protocol().participant_id == from.wire_protocol().participant_id) || + !(to.wire_protocol().port == from.wire_protocol().port) || + !(to.wire_protocol().throughput_controller == from.wire_protocol().throughput_controller) || + !(to.wire_protocol().default_unicast_locator_list == + from.wire_protocol().default_unicast_locator_list) || + !(to.wire_protocol().default_multicast_locator_list == + from.wire_protocol().default_multicast_locator_list) || + !(to.wire_protocol().builtin.use_WriterLivelinessProtocol == + from.wire_protocol().builtin.use_WriterLivelinessProtocol) || + !(to.wire_protocol().builtin.typelookup_config.use_client == + from.wire_protocol().builtin.typelookup_config.use_client) || + !(to.wire_protocol().builtin.typelookup_config.use_server == + from.wire_protocol().builtin.typelookup_config.use_server) || + !(to.wire_protocol().builtin.metatrafficUnicastLocatorList == + from.wire_protocol().builtin.metatrafficUnicastLocatorList) || + !(to.wire_protocol().builtin.metatrafficMulticastLocatorList == + from.wire_protocol().builtin.metatrafficMulticastLocatorList) || + !(to.wire_protocol().builtin.initialPeersList == from.wire_protocol().builtin.initialPeersList) || + !(to.wire_protocol().builtin.readerHistoryMemoryPolicy == + from.wire_protocol().builtin.readerHistoryMemoryPolicy) || + !(to.wire_protocol().builtin.readerPayloadSize == from.wire_protocol().builtin.readerPayloadSize) || + !(to.wire_protocol().builtin.writerHistoryMemoryPolicy == + from.wire_protocol().builtin.writerHistoryMemoryPolicy) || + !(to.wire_protocol().builtin.writerPayloadSize == from.wire_protocol().builtin.writerPayloadSize) || + !(to.wire_protocol().builtin.mutation_tries == from.wire_protocol().builtin.mutation_tries) || + !(to.wire_protocol().builtin.avoid_builtin_multicast == + from.wire_protocol().builtin.avoid_builtin_multicast) || + !(to.wire_protocol().builtin.discovery_config.discoveryProtocol == + from.wire_protocol().builtin.discovery_config.discoveryProtocol) || + !(to.wire_protocol().builtin.discovery_config.use_SIMPLE_EndpointDiscoveryProtocol == + from.wire_protocol().builtin.discovery_config.use_SIMPLE_EndpointDiscoveryProtocol) || + !(to.wire_protocol().builtin.discovery_config.use_STATIC_EndpointDiscoveryProtocol == + from.wire_protocol().builtin.discovery_config.use_STATIC_EndpointDiscoveryProtocol) || + !(to.wire_protocol().builtin.discovery_config.discoveryServer_client_syncperiod == + from.wire_protocol().builtin.discovery_config.discoveryServer_client_syncperiod) || + !(to.wire_protocol().builtin.discovery_config.m_PDPfactory == + from.wire_protocol().builtin.discovery_config.m_PDPfactory) || + !(to.wire_protocol().builtin.discovery_config.leaseDuration == + from.wire_protocol().builtin.discovery_config.leaseDuration) || + !(to.wire_protocol().builtin.discovery_config.leaseDuration_announcementperiod == + from.wire_protocol().builtin.discovery_config.leaseDuration_announcementperiod) || + !(to.wire_protocol().builtin.discovery_config.initial_announcements == + from.wire_protocol().builtin.discovery_config.initial_announcements) || + !(to.wire_protocol().builtin.discovery_config.m_simpleEDP == + from.wire_protocol().builtin.discovery_config.m_simpleEDP) || + !(strcmp(to.wire_protocol().builtin.discovery_config.static_edp_xml_config(), + from.wire_protocol().builtin.discovery_config.static_edp_xml_config()) == 0) || + !(to.wire_protocol().builtin.discovery_config.ignoreParticipantFlags == + from.wire_protocol().builtin.discovery_config.ignoreParticipantFlags)))) + { + updatable = false; + logWarning(RTPS_QOS_CHECK, "WireProtocolConfigQos cannot be changed after the participant is enabled, " + << "with the exception of builtin.discovery_config.m_DiscoveryServers"); + } + } if (!(to.transport() == from.transport())) { diff --git a/test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp b/test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp index 74638d76b00..46d457e66e0 100644 --- a/test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp +++ b/test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp @@ -597,6 +597,7 @@ class DomainParticipantImpl const DomainParticipantQos& /*from*/, bool /*first_time*/) { + return false; } static ReturnCode_t check_qos( diff --git a/test/unittest/dds/participant/ParticipantTests.cpp b/test/unittest/dds/participant/ParticipantTests.cpp index d45b34eb3cd..9f4e97d7310 100644 --- a/test/unittest/dds/participant/ParticipantTests.cpp +++ b/test/unittest/dds/participant/ParticipantTests.cpp @@ -376,6 +376,275 @@ TEST(ParticipantTests, ChangePSMDomainParticipantQos) } +/** This test checks that the only mutable element in WireProtocolQosPolicy is the list of remote servers. + * The checks exclude: + * 1. wire_protocol().port since its data member cannot be neither initialized nor get + * 2. wire_protocol().builtin.discovery_config.m_PDPFactory since it is a deprecated interface for RTPS + * applications to implement a different discovery mechanism. + */ +TEST(ParticipantTests, ChangeWireProtocolQos) +{ + DomainParticipant* participant = + DomainParticipantFactory::get_instance()->create_participant(0, PARTICIPANT_QOS_DEFAULT); + DomainParticipantQos qos; + participant->get_qos(qos); + + ASSERT_EQ(qos, PARTICIPANT_QOS_DEFAULT); + + // Check that just adding two servers is OK + rtps::RemoteServerAttributes server; + server.ReadguidPrefix("44.53.00.5f.45.50.52.4f.53.49.4d.41"); + fastrtps::rtps::Locator_t locator; + fastrtps::rtps::IPLocator::setIPv4(locator, 192, 168, 1, 133); + locator.port = 64863; + server.metatrafficUnicastLocatorList.push_back(locator); + qos.wire_protocol().builtin.discovery_config.m_DiscoveryServers.push_back(server); + + rtps::RemoteServerAttributes server_2; + server_2.ReadguidPrefix("44.53.00.5f.45.50.52.4f.53.49.4d.42"); + fastrtps::rtps::Locator_t locator_2; + fastrtps::rtps::IPLocator::setIPv4(locator_2, 192, 168, 1, 134); + locator_2.port = 64862; + server_2.metatrafficUnicastLocatorList.push_back(locator_2); + qos.wire_protocol().builtin.discovery_config.m_DiscoveryServers.push_back(server_2); + + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_OK); + DomainParticipantQos set_qos; + participant->get_qos(set_qos); + ASSERT_EQ(set_qos, qos); + + // Check that removing one server is OK + qos.wire_protocol().builtin.discovery_config.m_DiscoveryServers.pop_front(); + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_OK); + participant->get_qos(set_qos); + ASSERT_EQ(set_qos, qos); + + // Check that removing all servers is OK + fastdds::rtps::RemoteServerList_t servers; + qos.wire_protocol().builtin.discovery_config.m_DiscoveryServers = servers; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_OK); + participant->get_qos(set_qos); + ASSERT_EQ(set_qos, qos); + + // Check changing wire_protocol().prefix is NOT OK + participant->get_qos(qos); + std::istringstream("44.53.00.5f.45.50.52.4f.53.49.4d.41") >> qos.wire_protocol().prefix; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().participant_id is NOT OK + participant->get_qos(qos); + qos.wire_protocol().participant_id = 7; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().throughput_controller is NOT OK + participant->get_qos(qos); + fastrtps::rtps::ThroughputControllerDescriptor controller{300000, 1000}; + qos.wire_protocol().throughput_controller = controller; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().default_unicast_locator_list is NOT OK + participant->get_qos(qos); + fastrtps::rtps::Locator_t loc; + fastrtps::rtps::IPLocator::setIPv4(loc, "192.0.0.0"); + loc.port = static_cast(12); + qos.wire_protocol().default_unicast_locator_list.push_back(loc); + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().default_multicast_locator_list is NOT OK + participant->get_qos(qos); + qos.wire_protocol().default_multicast_locator_list.push_back(loc); + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.use_WriterLivelinessProtocol is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.use_WriterLivelinessProtocol ^= true; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.typelookup_config.use_client is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.typelookup_config.use_client ^= true; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.typelookup_config.use_server is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.typelookup_config.use_server ^= true; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.metatrafficUnicastLocatorList is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.metatrafficUnicastLocatorList.push_back(loc); + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.metatrafficMulticastLocatorList is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.metatrafficMulticastLocatorList.push_back(loc); + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.initialPeersList is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.initialPeersList.push_back(loc); + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.readerHistoryMemoryPolicy is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.readerHistoryMemoryPolicy = + fastrtps::rtps::MemoryManagementPolicy_t::DYNAMIC_RESERVE_MEMORY_MODE; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.readerPayloadSize is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.readerPayloadSize = 27; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.writerHistoryMemoryPolicy is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.writerHistoryMemoryPolicy = + fastrtps::rtps::MemoryManagementPolicy_t::DYNAMIC_RESERVE_MEMORY_MODE; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.writerPayloadSize is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.writerPayloadSize = 27; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.mutation_tries is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.mutation_tries = 27; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.avoid_builtin_multicast is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.avoid_builtin_multicast ^= true; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.discovery_config.discoveryProtocol is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.discovery_config.discoveryProtocol = fastrtps::rtps::DiscoveryProtocol_t::NONE; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.discovery_config.use_SIMPLE_EndpointDiscoveryProtocol is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.discovery_config.use_SIMPLE_EndpointDiscoveryProtocol ^= true; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.discovery_config.use_STATIC_EndpointDiscoveryProtocol is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.discovery_config.use_STATIC_EndpointDiscoveryProtocol ^= true; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.discovery_config.discoveryServer_client_syncperiod is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.discovery_config.discoveryServer_client_syncperiod = { 27, 27}; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.discovery_config.leaseDuration is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.discovery_config.leaseDuration = { 27, 27}; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.discovery_config.leaseDuration_announcementperiod is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.discovery_config.leaseDuration_announcementperiod = { 27, 27}; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.discovery_config.initial_announcements is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.discovery_config.initial_announcements.count = 27; + qos.wire_protocol().builtin.discovery_config.initial_announcements.period = {27, 27}; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.discovery_config.m_simpleEDP is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.discovery_config.m_simpleEDP.use_PublicationWriterANDSubscriptionReader ^= true; + qos.wire_protocol().builtin.discovery_config.m_simpleEDP.use_PublicationReaderANDSubscriptionWriter ^= true; +#if HAVE_SECURITY + qos.wire_protocol().builtin.discovery_config.m_simpleEDP. + enable_builtin_secure_publications_writer_and_subscriptions_reader ^= true; + qos.wire_protocol().builtin.discovery_config.m_simpleEDP. + enable_builtin_secure_subscriptions_writer_and_publications_reader ^= true; +#endif // if HAVE_SECURITY + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.discovery_config.static_edp_xml_config() is NOT OK + participant->get_qos(qos); + std::string static_xml = "data://" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "STATIC" \ + "" \ + "" \ + "" \ + "" \ + ""; + qos.wire_protocol().builtin.discovery_config.static_edp_xml_config(static_xml.c_str()); + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + // Check changing wire_protocol().builtin.discovery_config.ignoreParticipantFlags is NOT OK + participant->get_qos(qos); + qos.wire_protocol().builtin.discovery_config.ignoreParticipantFlags = + fastrtps::rtps::ParticipantFilteringFlags::FILTER_DIFFERENT_HOST; + ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + participant->get_qos(set_qos); + ASSERT_FALSE(set_qos == qos); + + ASSERT_TRUE(DomainParticipantFactory::get_instance()->delete_participant(participant) == ReturnCode_t::RETCODE_OK); +} + TEST(ParticipantTests, EntityFactoryBehavior) { DomainParticipantFactory* factory = DomainParticipantFactory::get_instance(); From eb33c83ee40d922fae4dfc2c3e9cbc1a3b21a616 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Luis=20Bueno=20L=C3=B3pez?= <69244257+JLBuenoLopez-eProsima@users.noreply.github.com> Date: Tue, 10 Aug 2021 16:27:51 +0200 Subject: [PATCH 4/5] Add UserDataQoS blackbox test (#2130) * Refs #12196: modify Blackbox PubSubParticipant API to include UserDataQosPolicy Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: add discovery participant API to PubSubParticipant Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: add blackbox test that checks user data QoS update Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: uncrustify Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: fix deadlock in Blackbox test Signed-off-by: JLBuenoLopez-eProsima * Refs #12196: apply review suggestions Signed-off-by: JLBuenoLopez-eProsima --- .../api/dds-pim/PubSubParticipant.hpp | 169 +++++++++++++++++- .../common/DDSBlackboxTestsUserDataQos.cpp | 144 +++++++++++++++ 2 files changed, 312 insertions(+), 1 deletion(-) create mode 100644 test/blackbox/common/DDSBlackboxTestsUserDataQos.cpp diff --git a/test/blackbox/api/dds-pim/PubSubParticipant.hpp b/test/blackbox/api/dds-pim/PubSubParticipant.hpp index bb209e358ae..7e7427888b5 100644 --- a/test/blackbox/api/dds-pim/PubSubParticipant.hpp +++ b/test/blackbox/api/dds-pim/PubSubParticipant.hpp @@ -20,6 +20,7 @@ #ifndef _TEST_BLACKBOX_PUBSUBPARTICIPANT_HPP_ #define _TEST_BLACKBOX_PUBSUBPARTICIPANT_HPP_ +#include #include #include #include @@ -39,6 +40,7 @@ #include #include #include +#include /** * @brief A class with one participant that can have multiple publishers and subscribers @@ -144,6 +146,58 @@ class PubSubParticipant PubSubParticipant* participant_; }; + class ParticipantListener : public eprosima::fastdds::dds::DomainParticipantListener + { + friend class PubSubParticipant; + + public: + + ParticipantListener( + PubSubParticipant* participant) + : participant_(participant) + { + } + + ~ParticipantListener() = default; + + void on_participant_discovery( + eprosima::fastdds::dds::DomainParticipant*, + eprosima::fastrtps::rtps::ParticipantDiscoveryInfo&& info) + { + bool expected = false; + if (info.status == eprosima::fastrtps::rtps::ParticipantDiscoveryInfo::DISCOVERED_PARTICIPANT) + { + ++participant_->matched_; + if (nullptr != participant_->on_discovery_) + { + participant_->discovery_result_.compare_exchange_strong(expected, + participant_->on_discovery_(info)); + } + participant_->cv_discovery_.notify_one(); + } + else if (participant_->on_participant_qos_update_ != nullptr && + info.status == eprosima::fastrtps::rtps::ParticipantDiscoveryInfo::CHANGED_QOS_PARTICIPANT) + { + participant_->participant_qos_updated_.compare_exchange_strong(expected, + participant_->on_participant_qos_update_(info)); + participant_->cv_discovery_.notify_one(); + } + else if (info.status == eprosima::fastrtps::rtps::ParticipantDiscoveryInfo::REMOVED_PARTICIPANT || + info.status == eprosima::fastrtps::rtps::ParticipantDiscoveryInfo::DROPPED_PARTICIPANT) + { + --participant_->matched_; + participant_->cv_discovery_.notify_one(); + } + } + + private: + + ParticipantListener& operator =( + const ParticipantListener&) = delete; + PubSubParticipant* participant_; + + }; + public: PubSubParticipant( @@ -158,8 +212,14 @@ class PubSubParticipant , num_expected_publishers_(num_expected_publishers) , publishers_(num_publishers) , subscribers_(num_subscribers) + , participant_listener_(this) , pub_listener_(this) , sub_listener_(this) + , matched_(0) + , on_discovery_(nullptr) + , on_participant_qos_update_(nullptr) + , discovery_result_(false) + , participant_qos_updated_(false) , pub_matched_(0) , sub_matched_(0) , pub_times_liveliness_lost_(0) @@ -242,9 +302,13 @@ class PubSubParticipant bool init_participant() { + matched_ = 0; + participant_ = eprosima::fastdds::dds::DomainParticipantFactory::get_instance()->create_participant( (uint32_t)GET_PID() % 230, - participant_qos_); + participant_qos_, + &participant_listener_, + eprosima::fastdds::dds::StatusMask::none()); if (participant_ != nullptr) { @@ -360,6 +424,44 @@ class PubSubParticipant std::get<2>(publishers_[index])->assert_liveliness(); } + bool wait_discovery( + std::chrono::seconds timeout = std::chrono::seconds::zero()) + { + bool ret_value = true; + std::unique_lock lock(mutex_discovery_); + + std::cout << "Participant is waiting discovery..." << std::endl; + + if (timeout == std::chrono::seconds::zero()) + { + cv_discovery_.wait(lock, [&]() + { + return matched_ != 0; + }); + } + else + { + if (!cv_discovery_.wait_for(lock, timeout, [&]() + { + return matched_ != 0; + })) + { + ret_value = false; + } + } + + if (ret_value) + { + std::cout << "Participant discovery finished successfully..." << std::endl; + } + else + { + std::cout << "Participant discovery finished unsuccessfully..." << std::endl; + } + + return ret_value; + } + void pub_wait_discovery( std::chrono::seconds timeout = std::chrono::seconds::zero()) { @@ -512,6 +614,20 @@ class PubSubParticipant return *this; } + PubSubParticipant& user_data( + const std::vector& user_data) + { + participant_qos_.user_data().data_vec(user_data); + return *this; + } + + bool update_user_data( + const std::vector& user_data) + { + participant_qos_.user_data().data_vec(user_data); + return ReturnCode_t::RETCODE_OK == participant_->set_qos(participant_qos_); + } + PubSubParticipant& pub_property_policy( const eprosima::fastrtps::rtps::PropertyPolicy property_policy) { @@ -669,6 +785,46 @@ class PubSubParticipant return sub_times_liveliness_recovered_; } + void wait_discovery_result() + { + std::unique_lock lock(mutex_discovery_); + + std::cout << "Participant is waiting discovery result..." << std::endl; + + cv_discovery_.wait(lock, [&]() -> bool + { + return discovery_result_; + }); + + std::cout << "Participant gets discovery result..." << std::endl; + } + + void wait_qos_update() + { + std::unique_lock lock(mutex_discovery_); + + std::cout << "Participant is waiting QoS update..." << std::endl; + + cv_discovery_.wait(lock, [&]() -> bool + { + return participant_qos_updated_; + }); + + std::cout << "Participant gets QoS update..." << std::endl; + } + + void set_on_discovery_function( + std::function f) + { + on_discovery_ = f; + } + + void set_on_participant_qos_update_function( + std::function f) + { + on_participant_qos_update_ = f; + } + private: PubSubParticipant& operator =( @@ -728,6 +884,8 @@ class PubSubParticipant eprosima::fastdds::dds::DataWriterQos datawriter_qos_; //! Subscriber attributes eprosima::fastdds::dds::DataReaderQos datareader_qos_; + //! A listener for participants + ParticipantListener participant_listener_; //! A listener for publishers PubListener pub_listener_; //! A listener for subscribers @@ -735,6 +893,15 @@ class PubSubParticipant std::string publisher_topicname_; std::string subscriber_topicname_; + //! Discovery + std::mutex mutex_discovery_; + std::condition_variable cv_discovery_; + std::atomic matched_; + std::function on_discovery_; + std::function on_participant_qos_update_; + std::atomic_bool discovery_result_; + std::atomic_bool participant_qos_updated_; + std::mutex pub_mutex_; std::mutex sub_mutex_; std::condition_variable pub_cv_; diff --git a/test/blackbox/common/DDSBlackboxTestsUserDataQos.cpp b/test/blackbox/common/DDSBlackboxTestsUserDataQos.cpp new file mode 100644 index 00000000000..571845352e1 --- /dev/null +++ b/test/blackbox/common/DDSBlackboxTestsUserDataQos.cpp @@ -0,0 +1,144 @@ +// Copyright 2021 Proyectos y Sistemas de Mantenimiento SL (eProsima). +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "BlackboxTests.hpp" + +#include "PubSubParticipant.hpp" + +#include +#include +#include +#include + +using namespace eprosima::fastrtps; + +enum communication_type +{ + TRANSPORT, + INTRAPROCESS, + DATASHARING +}; + +class UserDataQos : public testing::TestWithParam +{ +public: + + void SetUp() override + { + LibrarySettingsAttributes library_settings; + switch (GetParam()) + { + case INTRAPROCESS: + library_settings.intraprocess_delivery = IntraprocessDeliveryType::INTRAPROCESS_FULL; + xmlparser::XMLProfileManager::library_settings(library_settings); + break; + case DATASHARING: + enable_datasharing = true; + break; + case TRANSPORT: + default: + break; + } + } + + void TearDown() override + { + LibrarySettingsAttributes library_settings; + switch (GetParam()) + { + case INTRAPROCESS: + library_settings.intraprocess_delivery = IntraprocessDeliveryType::INTRAPROCESS_OFF; + xmlparser::XMLProfileManager::library_settings(library_settings); + break; + case DATASHARING: + enable_datasharing = false; + break; + case TRANSPORT: + default: + break; + } + } + +}; + +/** + * This test checks that the user data updates once the participant is initialized are correctly applied + * In order to check that the user data is correctly updated, tow participants are created and the discovery info is + * checked. + */ +TEST_P(UserDataQos, update_user_data_qos) +{ + PubSubParticipant participant_1(0u, 0u, 0u, 0u); + ASSERT_TRUE(participant_1.user_data({'a', 'b', 'c', 'd', 'e'}).init_participant()); + + PubSubParticipant participant_2(0u, 0u, 0u, 0u); + + participant_2.set_on_discovery_function([&](const rtps::ParticipantDiscoveryInfo& info) -> bool + { + std::cout << "Received USER_DATA: "; + for (auto i : info.info.m_userData) + { + std::cout << i << ' '; + } + std::cout << std::endl; + return info.info.m_userData == std::vector({'a', 'b', 'c', 'd', 'e'}); + }); + participant_2.set_on_participant_qos_update_function([&](const rtps::ParticipantDiscoveryInfo& info) -> bool + { + std::cout << "Received USER_DATA: "; + for (auto i : info.info.m_userData) + { + std::cout << i << ' '; + } + std::cout << std::endl; + return info.info.m_userData == std::vector({'f', 'g'}); + }); + + ASSERT_TRUE(participant_2.init_participant()); + + participant_1.wait_discovery(); + participant_2.wait_discovery_result(); + + // Update user data + ASSERT_TRUE(participant_1.update_user_data({'f', 'g'})); + + participant_2.wait_qos_update(); +} + +#ifdef INSTANTIATE_TEST_SUITE_P +#define GTEST_INSTANTIATE_TEST_MACRO(x, y, z, w) INSTANTIATE_TEST_SUITE_P(x, y, z, w) +#else +#define GTEST_INSTANTIATE_TEST_MACRO(x, y, z, w) INSTANTIATE_TEST_CASE_P(x, y, z, w) +#endif // INSTANTIATE_TEST_SUITE_P + +GTEST_INSTANTIATE_TEST_MACRO(UserDataQos, + UserDataQos, + testing::Values(TRANSPORT, INTRAPROCESS, DATASHARING), + [](const testing::TestParamInfo& info) + { + switch (info.param) + { + case INTRAPROCESS: + return "Intraprocess"; + break; + case DATASHARING: + return "Datasharing"; + break; + case TRANSPORT: + default: + return "Transport"; + } + }); From 818a4196fd3a27585364c71ef3a2dbbceb0b6f29 Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Segrelles Date: Thu, 12 Aug 2021 09:45:48 +0200 Subject: [PATCH 5/5] Propagate servers list updates to PDPServer and PDPClient (#2138) * Refs 12278: Only add new servers Signed-off-by: Eduardo Ponz Segrelles * Refs 12278: Update Discovery Server with new servers Signed-off-by: Eduardo Ponz Segrelles * Refs 12278: Check that old server list is a subset of the new one when using DDS layer Signed-off-by: Eduardo Ponz Segrelles * Refs 12278: Test now checks that servers cannot be removed from list Signed-off-by: Eduardo Ponz Segrelles * Refs 12278: Manually match new server' PDP endpoints Signed-off-by: Eduardo Ponz Segrelles * Refs 12278: Discovery test for adding servers Signed-off-by: Eduardo Ponz Segrelles * Refs 12278: Randomize test parameters Signed-off-by: Eduardo Ponz Segrelles * Refs 12278: Uncrustify Signed-off-by: Eduardo Ponz Segrelles * Refs 12278: Address Windows warning Signed-off-by: Eduardo Ponz Segrelles * Refs 12278: Apply suggestions Signed-off-by: Eduardo Ponz Segrelles * Refs 12278: Use nullptr and add headers Signed-off-by: Eduardo Ponz Segrelles * Refs 12278: Add SystemInfo source to CMake Signed-off-by: Eduardo Ponz Segrelles --- .../fastdds/domain/DomainParticipantImpl.cpp | 27 +++- .../discovery/database/DiscoveryDataBase.cpp | 7 +- .../discovery/database/DiscoveryDataBase.hpp | 11 +- .../discovery/participant/PDPClient.cpp | 78 +++++++++--- .../builtin/discovery/participant/PDPClient.h | 23 ++++ .../discovery/participant/PDPServer.cpp | 91 ++++++++++---- .../discovery/participant/PDPServer.hpp | 28 ++++- .../rtps/participant/RTPSParticipantImpl.cpp | 30 ++++- test/blackbox/CMakeLists.txt | 1 + .../api/dds-pim/PubSubParticipant.hpp | 45 ++++++- .../common/DDSBlackboxTestsDiscovery.cpp | 119 +++++++++++++++++- .../dds/participant/ParticipantTests.cpp | 12 +- 12 files changed, 399 insertions(+), 73 deletions(-) diff --git a/src/cpp/fastdds/domain/DomainParticipantImpl.cpp b/src/cpp/fastdds/domain/DomainParticipantImpl.cpp index 7e65972d513..d37575e57a0 100644 --- a/src/cpp/fastdds/domain/DomainParticipantImpl.cpp +++ b/src/cpp/fastdds/domain/DomainParticipantImpl.cpp @@ -1879,7 +1879,32 @@ bool DomainParticipantImpl::can_qos_be_updated( logWarning(RTPS_QOS_CHECK, "WireProtocolConfigQos cannot be changed after the participant is enabled, " << "with the exception of builtin.discovery_config.m_DiscoveryServers"); } - + else + { + // This means that the only change is in wire_protocol().builtin.discovery_config.m_DiscoveryServers + // In that case, we need to ensure that the current list (to) is strictly contained in the incoming + // list (from). For that, we check that every server in the current list (to) is also in the incoming one + // (from) + for (auto existing_server : to.wire_protocol().builtin.discovery_config.m_DiscoveryServers) + { + bool contained = false; + for (auto incoming_server : from.wire_protocol().builtin.discovery_config.m_DiscoveryServers) + { + if (existing_server.guidPrefix == incoming_server.guidPrefix) + { + contained = true; + break; + } + } + if (!contained) + { + updatable = false; + logWarning(RTPS_QOS_CHECK, + "Discovery Servers cannot be removed from the list; they can only be added"); + break; + } + } + } } if (!(to.transport() == from.transport())) { diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp index 67304f6c4ab..e8ca3e6e2eb 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp @@ -18,6 +18,7 @@ */ #include +#include #include #include @@ -37,7 +38,7 @@ namespace ddb { DiscoveryDataBase::DiscoveryDataBase( fastrtps::rtps::GuidPrefix_t server_guid_prefix, - std::vector servers) + std::set servers) : server_guid_prefix_(server_guid_prefix) , server_acked_by_all_(servers.size() == 0) , servers_(servers) @@ -65,7 +66,7 @@ void DiscoveryDataBase::add_server( fastrtps::rtps::GuidPrefix_t server) { logInfo(DISCOVERY_DATABASE, "Server " << server << " added"); - servers_.push_back(server); + servers_.insert(server); } std::vector DiscoveryDataBase::clear() @@ -1716,7 +1717,7 @@ void DiscoveryDataBase::AckedFunctor::operator () ( { // If the reader proxy is from a server that we are pinging, we may not want to wait // for it to be acked as the routine will not stop - for (auto it = db_->servers_.begin(); it < db_->servers_.end(); ++it) + for (auto it = db_->servers_.begin(); it != db_->servers_.end(); ++it) { if (reader_proxy->guid().guidPrefix == *it) { diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp index bbb5f46c672..54a2c9164e3 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp @@ -20,11 +20,12 @@ #ifndef _FASTDDS_RTPS_DISCOVERY_DATABASE_H_ #define _FASTDDS_RTPS_DISCOVERY_DATABASE_H_ -#include +#include +#include #include #include -#include -#include +#include +#include #include #include @@ -117,7 +118,7 @@ class DiscoveryDataBase DiscoveryDataBase( fastrtps::rtps::GuidPrefix_t server_guid_prefix, - std::vector servers); + std::set servers); ~DiscoveryDataBase(); @@ -562,7 +563,7 @@ class DiscoveryDataBase std::atomic server_acked_by_all_; //! List of GUID prefixes of the remote servers - std::vector servers_; + std::set servers_; // The virtual topic associated with virtual writers and readers const std::string virtual_topic_ = "eprosima_server_virtual_topic"; diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp index e1269ed353a..4dd9b0bb510 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp @@ -188,8 +188,6 @@ bool PDPClient::createPDPEndpoints() { logInfo(RTPS_PDP, "Beginning PDPClient Endpoints creation"); - const NetworkFactory& network = mp_RTPSParticipant->network_factory(); - HistoryAttributes hatt; hatt.payloadMaxSize = mp_builtin->m_att.readerPayloadSize; hatt.initialReservedCaches = pdp_initial_reserved_caches; @@ -220,15 +218,7 @@ bool PDPClient::createPDPEndpoints() for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) { - std::lock_guard data_guard(temp_data_lock_); - temp_writer_data_.clear(); - temp_writer_data_.guid(it.GetPDPWriter()); - temp_writer_data_.set_multicast_locators(it.metatrafficMulticastLocatorList, network); - temp_writer_data_.set_remote_unicast_locators(it.metatrafficUnicastLocatorList, network); - temp_writer_data_.m_qos.m_durability.kind = TRANSIENT_DURABILITY_QOS; // Server Information must be persistent - temp_writer_data_.m_qos.m_reliability.kind = RELIABLE_RELIABILITY_QOS; - - mp_PDPReader->matched_writer_add(temp_writer_data_); + match_pdp_writer_nts_(it); } } } @@ -275,15 +265,7 @@ bool PDPClient::createPDPEndpoints() for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) { - std::lock_guard data_guard(temp_data_lock_); - temp_reader_data_.clear(); - temp_reader_data_.guid(it.GetPDPReader()); - temp_reader_data_.set_multicast_locators(it.metatrafficMulticastLocatorList, network); - temp_reader_data_.set_remote_unicast_locators(it.metatrafficUnicastLocatorList, network); - temp_reader_data_.m_qos.m_durability.kind = TRANSIENT_LOCAL_DURABILITY_QOS; - temp_reader_data_.m_qos.m_reliability.kind = RELIABLE_RELIABILITY_QOS; - - mp_PDPWriter->matched_reader_add(temp_reader_data_); + match_pdp_reader_nts_(it); } } } @@ -587,6 +569,62 @@ bool PDPClient::match_servers_EDP_endpoints() return all; } +void PDPClient::update_remote_servers_list() +{ + if (!mp_PDPReader || !mp_PDPWriter) + { + logError(SERVER_CLIENT_DISCOVERY, "Cannot update server list within an uninitialized Client"); + return; + } + + std::lock_guard lock(*getMutex()); + + for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) + { + if (mp_PDPReader->matched_writer_is_matched(it.GetPDPWriter())) + { + continue; + } + + match_pdp_writer_nts_(it); + + if (mp_PDPWriter->matched_reader_is_matched(it.GetPDPReader())) + { + continue; + } + + match_pdp_reader_nts_(it); + } +} + +void PDPClient::match_pdp_writer_nts_( + const eprosima::fastdds::rtps::RemoteServerAttributes& server_att) +{ + std::lock_guard data_guard(temp_data_lock_); + const NetworkFactory& network = mp_RTPSParticipant->network_factory(); + temp_writer_data_.clear(); + temp_writer_data_.guid(server_att.GetPDPWriter()); + temp_writer_data_.set_multicast_locators(server_att.metatrafficMulticastLocatorList, network); + temp_writer_data_.set_remote_unicast_locators(server_att.metatrafficUnicastLocatorList, network); + temp_writer_data_.m_qos.m_durability.kind = TRANSIENT_DURABILITY_QOS; + temp_writer_data_.m_qos.m_reliability.kind = RELIABLE_RELIABILITY_QOS; + mp_PDPReader->matched_writer_add(temp_writer_data_); +} + +void PDPClient::match_pdp_reader_nts_( + const eprosima::fastdds::rtps::RemoteServerAttributes& server_att) +{ + std::lock_guard data_guard(temp_data_lock_); + const NetworkFactory& network = mp_RTPSParticipant->network_factory(); + temp_reader_data_.clear(); + temp_reader_data_.guid(server_att.GetPDPReader()); + temp_reader_data_.set_multicast_locators(server_att.metatrafficMulticastLocatorList, network); + temp_reader_data_.set_remote_unicast_locators(server_att.metatrafficUnicastLocatorList, network); + temp_reader_data_.m_qos.m_durability.kind = TRANSIENT_LOCAL_DURABILITY_QOS; + temp_reader_data_.m_qos.m_reliability.kind = RELIABLE_RELIABILITY_QOS; + mp_PDPWriter->matched_reader_add(temp_reader_data_); +} + const std::string& ros_discovery_server_env() { static std::string servers; diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPClient.h b/src/cpp/rtps/builtin/discovery/participant/PDPClient.h index d2620444b67..925e0ac070b 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPClient.h +++ b/src/cpp/rtps/builtin/discovery/participant/PDPClient.h @@ -128,6 +128,29 @@ class PDPClient : public PDP */ bool match_servers_EDP_endpoints(); + /* + * Update the list of remote servers + */ + void update_remote_servers_list(); + +protected: + + /** + * Manually match the local PDP reader with the PDP writer of a given server. The function is + * not thread safe (nts) in the sense that it does not take the PDP mutex. It does however take + * temp_data_lock_ + */ + void match_pdp_writer_nts_( + const eprosima::fastdds::rtps::RemoteServerAttributes& server_att); + + /** + * Manually match the local PDP writer with the PDP reader of a given server. The function is + * not thread safe (nts) in the sense that it does not take the PDP mutex. It does however take + * temp_data_lock_ + */ + void match_pdp_reader_nts_( + const eprosima::fastdds::rtps::RemoteServerAttributes& server_att); + private: /** diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp index 1ab61903f0a..d1a10fb4524 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp @@ -17,9 +17,10 @@ * */ -#include #include +#include #include +#include #include @@ -224,8 +225,6 @@ bool PDPServer::createPDPEndpoints() { logInfo(RTPS_PDP_SERVER, "Beginning PDPServer Endpoints creation"); - const NetworkFactory& network = mp_RTPSParticipant->network_factory(); - /*********************************** * PDP READER ***********************************/ @@ -270,17 +269,7 @@ bool PDPServer::createPDPEndpoints() // Initial peer list doesn't make sense in server scenario. Client should match its server list for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) { - std::lock_guard data_guard(temp_data_lock_); - temp_writer_data_.clear(); - temp_writer_data_.guid(it.GetPDPWriter()); - temp_writer_data_.set_multicast_locators(it.metatrafficMulticastLocatorList, network); - temp_writer_data_.set_remote_unicast_locators(it.metatrafficUnicastLocatorList, network); - // TODO check if this is correct, it is equal as PDPServer, but we do not know like this the durKind of the - // other server - temp_writer_data_.m_qos.m_durability.durabilityKind(durability_); - temp_writer_data_.m_qos.m_reliability.kind = fastrtps::RELIABLE_RELIABILITY_QOS; - - mp_PDPReader->matched_writer_add(temp_writer_data_); + match_pdp_writer_nts_(it); } } } @@ -343,15 +332,7 @@ bool PDPServer::createPDPEndpoints() for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) { - std::lock_guard data_guard(temp_data_lock_); - temp_reader_data_.clear(); - temp_reader_data_.guid(it.GetPDPReader()); - temp_reader_data_.set_multicast_locators(it.metatrafficMulticastLocatorList, network); - temp_reader_data_.set_remote_unicast_locators(it.metatrafficUnicastLocatorList, network); - temp_reader_data_.m_qos.m_durability.kind = fastrtps::TRANSIENT_LOCAL_DURABILITY_QOS; - temp_reader_data_.m_qos.m_reliability.kind = fastrtps::RELIABLE_RELIABILITY_QOS; - - mp_PDPWriter->matched_reader_add(temp_reader_data_); + match_pdp_reader_nts_(it); } } } @@ -905,7 +886,35 @@ bool PDPServer::server_update_routine() void PDPServer::update_remote_servers_list() { - return; + if (!mp_PDPReader || !mp_PDPWriter) + { + logError(RTPS_PDP_SERVER, "Cannot update server list within an uninitialized Server"); + return; + } + + std::lock_guard lock(*getMutex()); + + for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) + { + if (mp_PDPReader->matched_writer_is_matched(it.GetPDPWriter())) + { + continue; + } + + match_pdp_writer_nts_(it); + + if (mp_PDPWriter->matched_reader_is_matched(it.GetPDPReader())) + { + continue; + } + + match_pdp_reader_nts_(it); + } + + for (auto server : mp_builtin->m_DiscoveryServers) + { + discovery_db_.add_server(server.guidPrefix); + } } bool PDPServer::process_writers_acknowledgements() @@ -1349,13 +1358,13 @@ bool PDPServer::pending_ack() return ret; } -std::vector PDPServer::servers_prefixes() +std::set PDPServer::servers_prefixes() { std::lock_guard lock(*getMutex()); - std::vector servers; + std::set servers; for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) { - servers.push_back(it.guidPrefix); + servers.insert(it.guidPrefix); } return servers; } @@ -1768,6 +1777,34 @@ void PDPServer::process_backup_store() discovery_db_.clean_backup(); } +void PDPServer::match_pdp_writer_nts_( + const eprosima::fastdds::rtps::RemoteServerAttributes& server_att) +{ + std::lock_guard data_guard(temp_data_lock_); + const NetworkFactory& network = mp_RTPSParticipant->network_factory(); + temp_writer_data_.clear(); + temp_writer_data_.guid(server_att.GetPDPWriter()); + temp_writer_data_.set_multicast_locators(server_att.metatrafficMulticastLocatorList, network); + temp_writer_data_.set_remote_unicast_locators(server_att.metatrafficUnicastLocatorList, network); + temp_writer_data_.m_qos.m_durability.durabilityKind(durability_); + temp_writer_data_.m_qos.m_reliability.kind = dds::RELIABLE_RELIABILITY_QOS; + mp_PDPReader->matched_writer_add(temp_writer_data_); +} + +void PDPServer::match_pdp_reader_nts_( + const eprosima::fastdds::rtps::RemoteServerAttributes& server_att) +{ + std::lock_guard data_guard(temp_data_lock_); + const NetworkFactory& network = mp_RTPSParticipant->network_factory(); + temp_reader_data_.clear(); + temp_reader_data_.guid(server_att.GetPDPReader()); + temp_reader_data_.set_multicast_locators(server_att.metatrafficMulticastLocatorList, network); + temp_reader_data_.set_remote_unicast_locators(server_att.metatrafficUnicastLocatorList, network); + temp_reader_data_.m_qos.m_durability.kind = dds::TRANSIENT_LOCAL_DURABILITY_QOS; + temp_reader_data_.m_qos.m_reliability.kind = dds::RELIABLE_RELIABILITY_QOS; + mp_PDPWriter->matched_reader_add(temp_reader_data_); +} + } // namespace rtps } // namespace fastdds } // namespace eprosima diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp index 5f4b896d327..05b65d4d817 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp @@ -22,11 +22,17 @@ #ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC #include + +#include +#include +#include +#include + +#include #include #include - -#include #include +#include #include namespace eprosima { @@ -260,7 +266,7 @@ class PDPServer : public fastrtps::rtps::PDP nlohmann::json& ddb_json, std::vector& new_changes); - std::vector servers_prefixes(); + std::set servers_prefixes(); // General file name for the prefix of every backup file std::ostringstream get_persistence_file_name_() const; @@ -272,6 +278,22 @@ class PDPServer : public fastrtps::rtps::PDP // from DDB must be called during this process void process_backup_store(); + /** + * Manually match the local PDP reader with the PDP writer of a given server. The function is + * not thread safe (nts) in the sense that it does not take the PDP mutex. It does however take + * temp_data_lock_ + */ + void match_pdp_writer_nts_( + const eprosima::fastdds::rtps::RemoteServerAttributes& server_att); + + /** + * Manually match the local PDP writer with the PDP reader of a given server. The function is + * not thread safe (nts) in the sense that it does not take the PDP mutex. It does however take + * temp_data_lock_ + */ + void match_pdp_reader_nts_( + const eprosima::fastdds::rtps::RemoteServerAttributes& server_att); + private: //! Server thread diff --git a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp index 898817a4e13..a9b4a8eb626 100644 --- a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp +++ b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp @@ -64,6 +64,7 @@ #include #include +#include #include @@ -1183,7 +1184,6 @@ void RTPSParticipantImpl::update_attributes( } // Update RTPSParticipantAttributes member - m_att.builtin.discovery_config.m_DiscoveryServers = patt.builtin.discovery_config.m_DiscoveryServers; m_att.userData = patt.userData; auto pdp = mp_builtinProtocols->mp_PDP; @@ -1200,13 +1200,41 @@ void RTPSParticipantImpl::update_attributes( m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::SERVER || m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::BACKUP) { + // Add incoming servers iff we don't know about them already + for (auto incoming_server : patt.builtin.discovery_config.m_DiscoveryServers) + { + eprosima::fastdds::rtps::RemoteServerList_t::iterator server_it; + for (server_it = m_att.builtin.discovery_config.m_DiscoveryServers.begin(); + server_it != m_att.builtin.discovery_config.m_DiscoveryServers.end(); server_it++) + { + if (server_it->guidPrefix == incoming_server.guidPrefix) + { + break; + } + } + if (server_it == m_att.builtin.discovery_config.m_DiscoveryServers.end()) + { + m_att.builtin.discovery_config.m_DiscoveryServers.push_back(incoming_server); + } + } + + // Update the servers list in builtin protocols mp_builtinProtocols->m_DiscoveryServers = m_att.builtin.discovery_config.m_DiscoveryServers; + + // Notify PDPServer if (m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::SERVER || m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::BACKUP) { fastdds::rtps::PDPServer* pdp_server = static_cast(pdp); pdp_server->update_remote_servers_list(); } + // Notify PDPClient + else if (m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::CLIENT || + m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::SUPER_CLIENT) + { + fastdds::rtps::PDPClient* pdp_client = static_cast(pdp); + pdp_client->update_remote_servers_list(); + } } } diff --git a/test/blackbox/CMakeLists.txt b/test/blackbox/CMakeLists.txt index 01e8e7a2583..29783e5f4e3 100644 --- a/test/blackbox/CMakeLists.txt +++ b/test/blackbox/CMakeLists.txt @@ -227,6 +227,7 @@ if(NOT ((MSVC OR MSVC_IDE) AND EPROSIMA_INSTALLER) AND fastcdr_FOUND) set(DDS_BLACKBOXTESTS_SOURCE ${DDS_BLACKBOXTESTS_TEST_SOURCE} ${BLACKBOXTESTS_SOURCE} + ${PROJECT_SOURCE_DIR}/src/cpp/utils/SystemInfo.cpp ) # Prepare static discovery xml file for blackbox tests. diff --git a/test/blackbox/api/dds-pim/PubSubParticipant.hpp b/test/blackbox/api/dds-pim/PubSubParticipant.hpp index 7e7427888b5..534a6e1e52a 100644 --- a/test/blackbox/api/dds-pim/PubSubParticipant.hpp +++ b/test/blackbox/api/dds-pim/PubSubParticipant.hpp @@ -425,25 +425,40 @@ class PubSubParticipant } bool wait_discovery( - std::chrono::seconds timeout = std::chrono::seconds::zero()) + std::chrono::seconds timeout = std::chrono::seconds::zero(), + uint8_t matched = 0, + bool exact = false) { - bool ret_value = true; - std::unique_lock lock(mutex_discovery_); + // No need to wait in this case + if (exact && matched == matched_) + { + return true; + } + std::unique_lock lock(mutex_discovery_); + bool ret_value = true; std::cout << "Participant is waiting discovery..." << std::endl; if (timeout == std::chrono::seconds::zero()) { cv_discovery_.wait(lock, [&]() { - return matched_ != 0; + if (exact) + { + return matched_ == matched; + } + return matched_ >= matched; }); } else { if (!cv_discovery_.wait_for(lock, timeout, [&]() { - return matched_ != 0; + if (exact) + { + return matched_ == matched; + } + return matched_ >= matched; })) { ret_value = false; @@ -628,6 +643,26 @@ class PubSubParticipant return ReturnCode_t::RETCODE_OK == participant_->set_qos(participant_qos_); } + PubSubParticipant& wire_protocol( + const eprosima::fastdds::dds::WireProtocolConfigQos& wire_protocol) + { + participant_qos_.wire_protocol() = wire_protocol; + return *this; + } + + bool update_wire_protocol( + const eprosima::fastdds::dds::WireProtocolConfigQos& wire_protocol) + { + eprosima::fastdds::dds::DomainParticipantQos participant_qos = participant_qos_; + participant_qos.wire_protocol() = wire_protocol; + if (ReturnCode_t::RETCODE_OK == participant_->set_qos(participant_qos)) + { + participant_qos_ = participant_qos; + return true; + } + return false; + } + PubSubParticipant& pub_property_policy( const eprosima::fastrtps::rtps::PropertyPolicy property_policy) { diff --git a/test/blackbox/common/DDSBlackboxTestsDiscovery.cpp b/test/blackbox/common/DDSBlackboxTestsDiscovery.cpp index 470d3562aa9..197c9d67cb7 100644 --- a/test/blackbox/common/DDSBlackboxTestsDiscovery.cpp +++ b/test/blackbox/common/DDSBlackboxTestsDiscovery.cpp @@ -12,12 +12,21 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include +#include +#include +#include + #include #include "BlackboxTests.hpp" - +#include "PubSubParticipant.hpp" #include "PubSubReader.hpp" +#include +#include +#include + // Regression test for redmine issue 11857 TEST(DDSDiscovery, IgnoreParticipantFlags) { @@ -52,4 +61,110 @@ TEST(DDSDiscovery, IgnoreParticipantFlags) p3.init(); EXPECT_TRUE(p1.wait_participant_discovery()); EXPECT_TRUE(p3.wait_participant_discovery()); -} \ No newline at end of file +} + +/** + * This test checks that adding servers to the Discovery Server list results in discovering those participants. + * It does so by: + * 1. Creating two servers and one client that is only connected to one server. At this point check discovery + * state. + * 2. Then, connect the client to the other server and check discovery again. + * 3. Finally connect the two servers by adding one of them to the others list + */ +TEST(DDSDiscovery, AddDiscoveryServerToList) +{ + using namespace eprosima; + using namespace eprosima::fastdds::dds; + using namespace eprosima::fastrtps::rtps; + + /* Get random port from the environment */ + const char* value; + if (eprosima::ReturnCode_t::RETCODE_OK != SystemInfo::instance().get_env("W_UNICAST_PORT_RANDOM_NUMBER", &value)) + { + value = &std::string("11811")[0]; + } + + /* Create first server */ + PubSubParticipant server_1(0u, 0u, 0u, 0u); + // Set participant as server + WireProtocolConfigQos server_1_qos; + server_1_qos.builtin.discovery_config.discoveryProtocol = DiscoveryProtocol_t::SERVER; + // Generate random GUID prefix + srand(static_cast(time(nullptr))); + GuidPrefix_t server_1_prefix; + for (auto i = 0; i < 12; i++) + { + server_1_prefix.value[i] = eprosima::fastrtps::rtps::octet(rand() % 254); + } + server_1_qos.prefix = server_1_prefix; + // Generate server's listening locator + Locator_t locator_server_1; + IPLocator::setIPv4(locator_server_1, 127, 0, 0, 1); + uint32_t server_1_port = atol(value); + locator_server_1.port = server_1_port; + server_1_qos.builtin.metatrafficUnicastLocatorList.push_back(locator_server_1); + // Init server + ASSERT_TRUE(server_1.wire_protocol(server_1_qos).init_participant()); + + /* Create second server */ + PubSubParticipant server_2(0u, 0u, 0u, 0u); + // Set participant as server + WireProtocolConfigQos server_2_qos; + server_2_qos.builtin.discovery_config.discoveryProtocol = DiscoveryProtocol_t::SERVER; + // Generate random GUID prefix + GuidPrefix_t server_2_prefix = server_1_prefix; + server_2_prefix.value[11]++; + server_2_qos.prefix = server_2_prefix; + // Generate server's listening locator + Locator_t locator_server_2; + IPLocator::setIPv4(locator_server_2, 127, 0, 0, 1); + uint32_t server_2_port = atol(value) + 1; + locator_server_2.port = server_2_port; + server_2_qos.builtin.metatrafficUnicastLocatorList.push_back(locator_server_2); + // Init server + ASSERT_TRUE(server_2.wire_protocol(server_2_qos).init_participant()); + + /* Create a client that connects to the first server from the beginning */ + PubSubParticipant client(0u, 0u, 0u, 0u); + // Set participant as client + WireProtocolConfigQos client_qos; + client_qos.builtin.discovery_config.discoveryProtocol = DiscoveryProtocol_t::CLIENT; + // Connect to first server + RemoteServerAttributes server_1_att; + server_1_att.guidPrefix = server_1_prefix; + server_1_att.metatrafficUnicastLocatorList.push_back(Locator_t(locator_server_1)); + client_qos.builtin.discovery_config.m_DiscoveryServers.push_back(server_1_att); + // Init client + ASSERT_TRUE(client.wire_protocol(client_qos).init_participant()); + + /** + * Check that server_1 and client_1 only know each other, and that server_2 does has not + * discovered anyone + */ + server_1.wait_discovery(std::chrono::seconds::zero(), 1, true); + client.wait_discovery(std::chrono::seconds::zero(), 1, true); + server_2.wait_discovery(std::chrono::seconds::zero(), 0, true); + + /* Add server_2 to client */ + RemoteServerAttributes server_2_att; + server_2_att.guidPrefix = server_2_prefix; + server_2_att.metatrafficUnicastLocatorList.push_back(Locator_t(locator_server_2)); + client_qos.builtin.discovery_config.m_DiscoveryServers.push_back(server_2_att); + // Update client's servers list + ASSERT_TRUE(client.update_wire_protocol(client_qos)); + + /* Check that the servers only know about the client, and that the client known about both servers */ + server_1.wait_discovery(std::chrono::seconds::zero(), 1, true); + client.wait_discovery(std::chrono::seconds::zero(), 2, true); + server_2.wait_discovery(std::chrono::seconds::zero(), 1, true); + + /* Add server_2 to server_1 */ + server_1_qos.builtin.discovery_config.m_DiscoveryServers.push_back(server_2_att); + ASSERT_TRUE(server_1.update_wire_protocol(server_1_qos)); + + /* Check that they all know each other */ + server_1.wait_discovery(std::chrono::seconds::zero(), 2, true); + client.wait_discovery(std::chrono::seconds::zero(), 2, true); + server_2.wait_discovery(std::chrono::seconds::zero(), 2, true); +} + diff --git a/test/unittest/dds/participant/ParticipantTests.cpp b/test/unittest/dds/participant/ParticipantTests.cpp index 9f4e97d7310..f3064597d67 100644 --- a/test/unittest/dds/participant/ParticipantTests.cpp +++ b/test/unittest/dds/participant/ParticipantTests.cpp @@ -413,18 +413,18 @@ TEST(ParticipantTests, ChangeWireProtocolQos) participant->get_qos(set_qos); ASSERT_EQ(set_qos, qos); - // Check that removing one server is OK + // Check that removing one server is NOT OK qos.wire_protocol().builtin.discovery_config.m_DiscoveryServers.pop_front(); - ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_OK); + ASSERT_FALSE(participant->set_qos(qos) == ReturnCode_t::RETCODE_OK); participant->get_qos(set_qos); - ASSERT_EQ(set_qos, qos); + ASSERT_FALSE(set_qos == qos); - // Check that removing all servers is OK + // Check that removing all servers is NOT OK fastdds::rtps::RemoteServerList_t servers; qos.wire_protocol().builtin.discovery_config.m_DiscoveryServers = servers; - ASSERT_TRUE(participant->set_qos(qos) == ReturnCode_t::RETCODE_OK); + ASSERT_FALSE(participant->set_qos(qos) == ReturnCode_t::RETCODE_OK); participant->get_qos(set_qos); - ASSERT_EQ(set_qos, qos); + ASSERT_FALSE(set_qos == qos); // Check changing wire_protocol().prefix is NOT OK participant->get_qos(qos);