Refs #21096: Applied Edus review

Signed-off-by: Mario Dominguez <[email protected]>

include/fastdds/dds/core/Types.hpp

@@ -23,7 +23,7 @@ namespace dds {
 
 typedef uint32_t DomainId_t;
 
-const DomainId_t c_DomainId_t_Unknown = 0xFFFFFFFF;
+const DomainId_t DOMAIN_ID_UNKNOWN = 0xFFFFFFFF;
 
 const int32_t LENGTH_UNLIMITED = -1;
 

include/fastdds/dds/core/policy/ParameterTypes.hpp

@@ -27,6 +27,7 @@
 
 #include <fastcdr/cdr/fixed_size_string.hpp>
 
+#include <fastdds/dds/core/Types.hpp>
 #include <fastdds/rtps/common/InstanceHandle.h>
 #include <fastdds/rtps/common/Locator.h>
 #include <fastdds/rtps/common/SampleIdentity.h>
@@ -532,14 +533,14 @@ class ParameterDomainId_t : public Parameter_t
 {
 public:
 
-    //!Domain ID. <br> By default, 0.
+    //!Domain ID. <br> By default, DOMAIN_ID_UNKNOWN.
     uint32_t domain_id;
 
     /**
      * @brief Constructor without parameters
      */
     ParameterDomainId_t()
-        : domain_id(0)
+        : domain_id(DOMAIN_ID_UNKNOWN)
     {
     }
 
@@ -553,9 +554,9 @@ class ParameterDomainId_t : public Parameter_t
             ParameterId_t pid,
             uint16_t in_length)
         : Parameter_t(pid, in_length)
-        , domain_id(0)
+        , domain_id(DOMAIN_ID_UNKNOWN)
     {
-        domain_id = 0;
+        domain_id = DOMAIN_ID_UNKNOWN;
     }
 
 };

src/cpp/fastdds/core/policy/ParameterSerializer.hpp

@@ -438,8 +438,7 @@ inline bool ParameterSerializer<ParameterDomainId_t>::add_content_to_cdr_message
         const ParameterDomainId_t& parameter,
         fastrtps::rtps::CDRMessage_t* cdr_message)
 {
-    bool valid = fastrtps::rtps::CDRMessage::addUInt32(cdr_message, parameter.domain_id);
-    return valid;
+    return fastrtps::rtps::CDRMessage::addUInt32(cdr_message, parameter.domain_id);
 }
 
 template<>
@@ -448,14 +447,12 @@ inline bool ParameterSerializer<ParameterDomainId_t>::read_content_from_cdr_mess
         fastrtps::rtps::CDRMessage_t* cdr_message,
         const uint16_t parameter_length)
 {
-    if (parameter_length != PARAMETER_VENDOR_LENGTH)
+    if (parameter_length != PARAMETER_DOMAINID_LENGTH)
     {
         return false;
     }
     parameter.length = parameter_length;
-    bool valid = fastrtps::rtps::CDRMessage::readUInt32(cdr_message, &parameter.domain_id);
-    cdr_message->pos += 2; //padding
-    return valid;
+    return fastrtps::rtps::CDRMessage::readUInt32(cdr_message, &parameter.domain_id);
 }
 
 template<>

src/cpp/rtps/builtin/data/ParticipantProxyData.cpp

@@ -52,7 +52,7 @@ ParticipantProxyData::ParticipantProxyData(
         const RTPSParticipantAllocationAttributes& allocation)
     : m_protocolVersion(c_ProtocolVersion)
     , m_VendorId(c_VendorId_Unknown)
-    , m_domain_id(fastdds::dds::c_DomainId_t_Unknown)
+    , m_domain_id(fastdds::dds::DOMAIN_ID_UNKNOWN)
     , m_expectsInlineQos(false)
     , m_availableBuiltinEndpoints(0)
     , m_networkConfiguration(0)
@@ -154,7 +154,7 @@ uint32_t ParticipantProxyData::get_serialized_size(
     ret_val += 4 + 4;
 
     // PID_DOMAIN_ID
-    ret_val += 4 + 4;
+    ret_val += 4 + PARAMETER_DOMAINID_LENGTH;
 
     if (m_expectsInlineQos)
     {
@@ -765,7 +765,7 @@ void ParticipantProxyData::clear()
     m_guid = GUID_t();
     //set_VendorId_Unknown(m_VendorId);
     m_VendorId = c_VendorId_Unknown;
-    m_domain_id = fastdds::dds::c_DomainId_t_Unknown;
+    m_domain_id = fastdds::dds::DOMAIN_ID_UNKNOWN;
     m_expectsInlineQos = false;
     m_availableBuiltinEndpoints = 0;
     m_networkConfiguration = 0;

src/cpp/rtps/builtin/discovery/participant/PDPClientListener.hpp

@@ -19,7 +19,6 @@
 
 #ifndef _FASTDDS_RTPS_PDPCLIENTLISTENER_H_
 #define _FASTDDS_RTPS_PDPCLIENTLISTENER_H_
-#ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC
 
 #include <rtps/builtin/discovery/participant/PDPListener.h>
 
@@ -58,5 +57,4 @@ class PDPClientListener : public PDPListener
 } /* namespace fastrtps */
 } /* namespace eprosima */
 
-#endif // ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC
 #endif /* _FASTDDS_RTPS_PDPCLIENTLISTENER_H_ */

src/cpp/rtps/builtin/discovery/participant/PDPListener.cpp

@@ -114,9 +114,7 @@ void PDPListener::onNewCacheChangeAdded(
                 return;
             }
 
-            void* empty_extra_data = nullptr;
-            static_cast<void>(empty_extra_data);
-            if (!check_discovery_conditions(temp_participant_data_, empty_extra_data))
+            if (!check_discovery_conditions(temp_participant_data_, nullptr))
             {
                 return;
             }
@@ -291,7 +289,7 @@ bool PDPListener::check_discovery_conditions(
     // In PDPSimple, do not match if the participant is from a different domain.
     // If the domain id is unknown, it is assumed to be the same domain
     if (remote_participant_domain_id != parent_pdp_->getRTPSParticipant()->get_domain_id() &&
-            remote_participant_domain_id != fastdds::dds::c_DomainId_t_Unknown)
+            remote_participant_domain_id != fastdds::dds::DOMAIN_ID_UNKNOWN)
     {
         EPROSIMA_LOG_INFO(RTPS_PDP_DISCOVERY, "Received participant with different domain id ("
                 << remote_participant_domain_id << ") than ours ("

src/cpp/rtps/builtin/discovery/participant/PDPServerListener.cpp

@@ -148,11 +148,12 @@ void PDPServerListener::onNewCacheChangeAdded(
                 return;
             }
 
-            bool is_client = true;
-            if (!check_discovery_conditions(participant_data, &is_client))
+            auto ret = check_server_discovery_conditions(participant_data);
+            if (!ret.first)
             {
                 return;
             }
+            bool is_client = ret.second;
 
             const auto& pattr = pdp_server()->getRTPSParticipant()->getAttributes();
             fastdds::rtps::network::external_locators::filter_remote_locators(participant_data,
@@ -361,18 +362,18 @@ void PDPServerListener::onNewCacheChangeAdded(
     EPROSIMA_LOG_INFO(RTPS_PDP_LISTENER, "");
 }
 
-bool PDPServerListener::check_discovery_conditions(
-        ParticipantProxyData& participant_data,
-        void* extra_data /* bool is_client*/)
+std::pair<bool, bool> PDPServerListener::check_server_discovery_conditions(
+        ParticipantProxyData& participant_data)
 {
-    bool ret = true;
+    // is_valid, is_client
+    std::pair<bool, bool> ret{true, true};
 
     /* Check PID_VENDOR_ID */
     if (participant_data.m_VendorId != fastrtps::rtps::c_VendorId_eProsima)
     {
         EPROSIMA_LOG_INFO(RTPS_PDP_LISTENER,
                 "DATA(p|Up) from different vendor is not supported for Discover-Server operation");
-        ret = false;
+        ret.first = false;
     }
 
     // In Discovery Server we don't impose
@@ -382,7 +383,7 @@ bool PDPServerListener::check_discovery_conditions(
     fastdds::dds::ParameterPropertyList_t properties = participant_data.m_properties;
 
     /* Check DS_VERSION */
-    if (ret)
+    if (ret.first)
     {
         auto ds_version = std::find_if(
             properties.begin(),
@@ -398,7 +399,7 @@ bool PDPServerListener::check_discovery_conditions(
             {
                 EPROSIMA_LOG_ERROR(RTPS_PDP_LISTENER, "Minimum " << dds::parameter_property_ds_version
                                                                  << " is 1.0, found: " << ds_version->second());
-                ret = false;
+                ret.first = false;
             }
             EPROSIMA_LOG_INFO(RTPS_PDP_LISTENER, "Participant " << dds::parameter_property_ds_version << ": "
                                                                 << ds_version->second());
@@ -410,7 +411,7 @@ bool PDPServerListener::check_discovery_conditions(
     }
 
     /* Check PARTICIPANT_TYPE */
-    if (ret)
+    if (ret.first)
     {
         auto participant_type = std::find_if(
             properties.begin(),
@@ -426,19 +427,19 @@ bool PDPServerListener::check_discovery_conditions(
                     participant_type->second() == ParticipantType::BACKUP ||
                     participant_type->second() == ParticipantType::SUPER_CLIENT)
             {
-                *static_cast<bool*>(extra_data) = false;
+                ret.second = false;
             }
             else if (participant_type->second() == ParticipantType::SIMPLE)
             {
                 EPROSIMA_LOG_INFO(RTPS_PDP_LISTENER, "Ignoring " << dds::parameter_property_participant_type << ": "
                                                                  << participant_type->second());
-                ret = false;
+                ret.first = false;
             }
             else if (participant_type->second() != ParticipantType::CLIENT)
             {
                 EPROSIMA_LOG_ERROR(RTPS_PDP_LISTENER, "Wrong " << dds::parameter_property_participant_type << ": "
                                                                << participant_type->second());
-                ret = false;
+                ret.first = false;
             }
             EPROSIMA_LOG_INFO(RTPS_PDP_LISTENER, "Participant type " << participant_type->second());
         }
@@ -458,11 +459,10 @@ bool PDPServerListener::check_discovery_conditions(
             // as persistent will have this property.
             if (persistence_guid != properties.end())
             {
-                // is_client = false
-                *static_cast<bool*>(extra_data) = false;
+                ret.second = false;
             }
             EPROSIMA_LOG_INFO(RTPS_PDP_LISTENER,
-                    "Participant is client: " << std::boolalpha << *static_cast<bool*>(extra_data));
+                    "Participant is client: " << std::boolalpha << ret.second);
         }
     }
 

src/cpp/rtps/builtin/discovery/participant/PDPServerListener.hpp

@@ -61,9 +61,16 @@ class PDPServerListener : public fastrtps::rtps::PDPListener
 
 protected:
 
-    bool check_discovery_conditions(
-            fastrtps::rtps::ParticipantProxyData& participant_data,
-            void* extra_data) override;
+    /**
+     * Checks discovery conditions on a discovery server entity.
+     * Essentially, it checks for incoming PIDS of remote proxy datas.
+     * @param participant_data Remote participant data to check.
+     * @return A pair of booleans.
+     * The first one indicates if the remote participant data is valid.
+     * The second one indicates if the remote participant data is a client.
+     */
+    std::pair<bool, bool> check_server_discovery_conditions(
+            fastrtps::rtps::ParticipantProxyData& participant_data);
 };
 
 

src/cpp/rtps/participant/RTPSParticipantImpl.cpp

@@ -277,6 +277,12 @@ RTPSParticipantImpl::RTPSParticipantImpl(
     , has_shm_transport_(false)
     , match_local_endpoints_(should_match_local_endpoints(PParam))
 {
+    if (domain_id_ == fastdds::dds::DOMAIN_ID_UNKNOWN)
+    {
+        EPROSIMA_LOG_ERROR(RTPS_PARTICIPANT, "Domain ID has to be set to a correct value");
+        return;
+    }
+
     if (c_GuidPrefix_Unknown != persistence_guid)
     {
         m_persistence_guid = GUID_t(persistence_guid, c_EntityId_RTPSParticipant);

test/blackbox/common/DDSBlackboxTestsDiscovery.cpp

@@ -2034,7 +2034,6 @@ TEST(DDSDiscovery, pdp_simple_participants_exchange_same_pid_domain_id_and_disco
  * do not discover each other despite the first one is initial peer of the second.
  *
  */
-
 TEST(DDSDiscovery, pdp_simple_initial_peer_participants_with_different_domain_ids_do_not_discover)
 {
     PubSubWriter<HelloWorldPubSubType> writer_domain_1(TEST_TOPIC_NAME);
@@ -2064,7 +2063,6 @@ TEST(DDSDiscovery, pdp_simple_initial_peer_participants_with_different_domain_id
  * discover each other.
  *
  */
-
 TEST(DDSDiscovery, client_server_participants_with_different_domain_ids_discover)
 {
     PubSubReader<HelloWorldPubSubType> server_domain_1(TEST_TOPIC_NAME);

test/blackbox/discovery_participants_initial_peers_profile.xml

@@ -25,7 +25,7 @@
                         <locator>
                             <udpv4>
                                 <address>127.0.0.1</address>
-                                <port>7412</port>
+                                <port>11500</port>
                             </udpv4>
                         </locator>
                     </initialPeersList>
@@ -50,14 +50,14 @@
                         <locator>
                             <udpv4>
                                 <address>127.0.0.1</address>
-                                <port>7412</port>
+                                <port>11500</port>
                             </udpv4>
                         </locator>
                     </metatrafficUnicastLocatorList>
                     <metatrafficMulticastLocatorList>
                         <locator>
                             <udpv4>
-                                <port>7400</port>
+                                <port>11600</port>
                                 <address>239.255.0.1</address>
                             </udpv4>
                         </locator>