Refs #20307: Appy Miguel's suggestions

Signed-off-by: EduPonz <[email protected]>
eProsima · Jan 30, 2024 · 5931f7a · 5931f7a
1 parent 841dfab
commit 5931f7a
Show file tree

Hide file tree

Showing 11 changed files with 273 additions and 29 deletions.
diff --git a/src/cpp/fastdds/core/policy/ParameterList.cpp b/src/cpp/fastdds/core/policy/ParameterList.cpp
@@ -68,7 +68,7 @@ bool ParameterList::updateCacheChangeFromInlineQos(
                     {
                         // TODO(eduponz): This check is done here because an implicit fall through rises a warning.
                         // C++17 included a [[fallthrough]] attribute to avoid this kind of warning.
-                        if (pid == PID_RELATED_ENTITY_GUID)
+                        if (pid == PID_CUSTOM_RELATED_SAMPLE_IDENTITY)
                         {
                             // Ignore custom PID when coming from other vendors except RTI Connext
                             if ((rtps::c_VendorId_eProsima != change.vendor_id) &&
@@ -87,6 +87,13 @@ bool ParameterList::updateCacheChangeFromInlineQos(
                                 return false;
                             }
 
+                            /*
+                             * TODO(eduponz): The data from this PID should be used to filled the
+                             * related_sample_identity field, not the sample_identity one.
+                             * Changing this here implies a behaviour change in the
+                             * RTPS layer, so it is postponed until the next major release.
+                             */
+                            FASTDDS_TODO_BEFORE(3, 0, "Fill related sample identity instead");
                             change.write_params.sample_identity(p.sample_id);
                         }
                         break;

diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp
@@ -238,9 +238,17 @@ struct ReadTakeCommand
         info.reception_timestamp = item->reader_info.receptionTimestamp;
         info.instance_handle = item->instanceHandle;
         info.publication_handle = InstanceHandle_t(item->writerGUID);
+
+        /*
+         * TODO(eduponz): The sample identity should be taken from the sample identity parameter.
+         * More importantly, the related sample identity should be taken from the related sample identity
+         * in write_params.
+         */
+        FASTDDS_TODO_BEFORE(3, 0, "Fill both sample_identity and related_sample_identity with write_params");
         info.sample_identity.writer_guid(item->writerGUID);
         info.sample_identity.sequence_number(item->sequenceNumber);
         info.related_sample_identity = item->write_params.sample_identity();
+
         info.valid_data = true;
 
         switch (item->kind)

diff --git a/src/cpp/rtps/DataSharing/DataSharingListener.cpp b/src/cpp/rtps/DataSharing/DataSharingListener.cpp
@@ -173,7 +173,8 @@ void DataSharingListener::process_new_data ()
                 {
                     EPROSIMA_LOG_WARNING(RTPS_READER, "GAP (" << last_sequence + 1 << " - " << ch.sequenceNumber - 1 << ")"
                                                               << " detected on datasharing writer " << pool->writer());
-                    reader_->processGapMsg(pool->writer(), last_sequence + 1, SequenceNumberSet_t(ch.sequenceNumber));
+                    reader_->processGapMsg(pool->writer(), last_sequence + 1, SequenceNumberSet_t(
+                                ch.sequenceNumber), c_VendorId_eProsima);
                 }
 
                 if (last_sequence == c_SequenceNumber_Unknown && ch.sequenceNumber > SequenceNumber_t(0, 1))
@@ -182,7 +183,7 @@ void DataSharingListener::process_new_data ()
                                                                            << " detected on datasharing writer " <<
                             pool->writer());
                     reader_->processGapMsg(pool->writer(), SequenceNumber_t(0, 1), SequenceNumberSet_t(
-                                ch.sequenceNumber));
+                                ch.sequenceNumber), c_VendorId_eProsima);
                 }
 
                 EPROSIMA_LOG_INFO(RTPS_READER, "New data found on writer " << pool->writer()

diff --git a/src/cpp/rtps/builtin/data/ParticipantProxyData.cpp b/src/cpp/rtps/builtin/data/ParticipantProxyData.cpp
@@ -394,7 +394,7 @@ bool ParticipantProxyData::readFromCDRMessage(
         bool should_filter_locators,
         fastdds::rtps::VendorId_t source_vendor_id)
 {
-    auto param_process = [this, &network, &is_shm_transport_available, &should_filter_locators, &source_vendor_id](
+    auto param_process = [this, &network, &is_shm_transport_available, &should_filter_locators, source_vendor_id](
         CDRMessage_t* msg, const ParameterId_t& pid, uint16_t plength)
             {
                 switch (pid)

diff --git a/src/cpp/rtps/builtin/data/ReaderProxyData.cpp b/src/cpp/rtps/builtin/data/ReaderProxyData.cpp
@@ -843,9 +843,8 @@ bool ReaderProxyData::readFromCDRMessage(
                     }
                     case fastdds::dds::PID_NETWORK_CONFIGURATION_SET:
                     {
-                        // Ignore custom PID when coming from other vendors except RTI Connext
-                        if ((c_VendorId_eProsima != source_vendor_id) &&
-                                (fastdds::rtps::c_VendorId_rti_connext != source_vendor_id))
+                        // Ignore custom PID when coming from other vendors
+                        if (c_VendorId_eProsima != source_vendor_id)
                         {
                             EPROSIMA_LOG_INFO(RTPS_PROXY_DATA,
                                     "Ignoring custom PID" << pid << " from vendor " << source_vendor_id);
@@ -989,8 +988,9 @@ bool ReaderProxyData::readFromCDRMessage(
 
                     case fastdds::dds::PID_DISABLE_POSITIVE_ACKS:
                     {
-                        // Ignore custom PID when coming from other vendors
-                        if (c_VendorId_eProsima != source_vendor_id)
+                        // Ignore custom PID when coming from other vendors except RTI Connext
+                        if ((c_VendorId_eProsima != source_vendor_id) &&
+                                (fastdds::rtps::c_VendorId_rti_connext != source_vendor_id))
                         {
                             EPROSIMA_LOG_INFO(RTPS_PROXY_DATA,
                                     "Ignoring custom PID" << pid << " from vendor " << source_vendor_id);

diff --git a/src/cpp/rtps/builtin/data/WriterProxyData.cpp b/src/cpp/rtps/builtin/data/WriterProxyData.cpp
@@ -857,9 +857,8 @@ bool WriterProxyData::readFromCDRMessage(
                     }
                     case fastdds::dds::PID_NETWORK_CONFIGURATION_SET:
                     {
-                        // Ignore custom PID when coming from other vendors except RTI Connext
-                        if ((c_VendorId_eProsima != source_vendor_id) &&
-                                (fastdds::rtps::c_VendorId_rti_connext != source_vendor_id))
+                        // Ignore custom PID when coming from other vendors
+                        if (c_VendorId_eProsima != source_vendor_id)
                         {
                             EPROSIMA_LOG_INFO(RTPS_PROXY_DATA,
                                     "Ignoring custom PID" << pid << " from vendor " << source_vendor_id);
@@ -971,8 +970,9 @@ bool WriterProxyData::readFromCDRMessage(
                     }
                     case fastdds::dds::PID_DISABLE_POSITIVE_ACKS:
                     {
-                        // Ignore custom PID when coming from other vendors
-                        if (c_VendorId_eProsima != source_vendor_id)
+                        // Ignore custom PID when coming from other vendors except RTI Connext
+                        if ((c_VendorId_eProsima != source_vendor_id) &&
+                                (fastdds::rtps::c_VendorId_rti_connext != source_vendor_id))
                         {
                             EPROSIMA_LOG_INFO(RTPS_PROXY_DATA,
                                     "Ignoring custom PID" << pid << " from vendor " << source_vendor_id);

diff --git a/src/cpp/rtps/reader/WriterProxy.cpp b/src/cpp/rtps/reader/WriterProxy.cpp
@@ -507,7 +507,8 @@ bool WriterProxy::perform_initial_ack_nack()
             if (writer)
             {
                 bool tmp;
-                writer->process_acknack(guid(), reader_->getGuid(), 1, SequenceNumberSet_t(), false, tmp);
+                writer->process_acknack(guid(), reader_->getGuid(), 1,
+                        SequenceNumberSet_t(), false, tmp, c_VendorId_eProsima);
             }
         }
         else

diff --git a/src/cpp/rtps/writer/StatefulWriter.cpp b/src/cpp/rtps/writer/StatefulWriter.cpp
@@ -485,7 +485,7 @@ bool StatefulWriter::intraprocess_gap(
     RTPSReader* reader = reader_proxy->local_reader();
     if (reader)
     {
-        return reader->processGapMsg(m_guid, first_seq, SequenceNumberSet_t(last_seq));
+        return reader->processGapMsg(m_guid, first_seq, SequenceNumberSet_t(last_seq), c_VendorId_eProsima);
     }
 
     return false;
@@ -519,7 +519,8 @@ bool StatefulWriter::intraprocess_heartbeat(
         {
             incrementHBCount();
             returned_value =
-                    reader->processHeartbeatMsg(m_guid, m_heartbeatCount, first_seq, last_seq, true, liveliness);
+                    reader->processHeartbeatMsg(m_guid, m_heartbeatCount, first_seq, last_seq, true, liveliness,
+                            c_VendorId_eProsima);
         }
     }
 

diff --git a/test/mock/rtps/RTPSWriter/fastdds/rtps/writer/RTPSWriter.h b/test/mock/rtps/RTPSWriter/fastdds/rtps/writer/RTPSWriter.h
@@ -19,17 +19,19 @@
 #ifndef _FASTDDS_RTPS_RTPSWRITER_H_
 #define _FASTDDS_RTPS_RTPSWRITER_H_
 
-#include <fastrtps/rtps/attributes/WriterAttributes.h>
-#include <fastrtps/rtps/writer/WriterListener.h>
-#include <fastrtps/rtps/Endpoint.h>
-#include <fastrtps/rtps/common/CacheChange.h>
+#include <condition_variable>
+
+#include <gmock/gmock.h>
+
+#include <fastdds/rtps/common/VendorId_t.hpp>
 #include <fastdds/rtps/interfaces/IReaderDataFilter.hpp>
 #include <fastdds/rtps/messages/RTPSMessageGroup.h>
 #include <fastdds/rtps/writer/DeliveryRetCode.hpp>
 #include <fastdds/rtps/writer/LocatorSelectorSender.hpp>
-
-#include <condition_variable>
-#include <gmock/gmock.h>
+#include <fastrtps/rtps/attributes/WriterAttributes.h>
+#include <fastrtps/rtps/common/CacheChange.h>
+#include <fastrtps/rtps/Endpoint.h>
+#include <fastrtps/rtps/writer/WriterListener.h>
 
 namespace eprosima {
 namespace fastrtps {
@@ -227,9 +229,15 @@ class RTPSWriter : public Endpoint
             uint32_t ack_count,
             const SequenceNumberSet_t& sn_set,
             bool final_flag,
-            bool& result)
+            bool& result,
+            fastdds::rtps::VendorId_t origin_vendor_id = c_VendorId_Unknown)
     {
-        (void)writer_guid; (void)reader_guid; (void)ack_count; (void)sn_set; (void)final_flag;
+        static_cast<void>(writer_guid);
+        static_cast<void>(reader_guid);
+        static_cast<void>(ack_count);
+        static_cast<void>(sn_set);
+        static_cast<void>(final_flag);
+        static_cast<void>(origin_vendor_id);
 
         result = false;
         return true;
@@ -241,9 +249,14 @@ class RTPSWriter : public Endpoint
             uint32_t ack_count,
             const SequenceNumber_t& seq_num,
             const FragmentNumberSet_t fragments_state,
-            bool& result)
+            bool& result,
+            fastdds::rtps::VendorId_t origin_vendor_id = c_VendorId_Unknown)
     {
-        (void)reader_guid; (void)ack_count; (void)seq_num; (void)fragments_state;
+        static_cast<void>(reader_guid);
+        static_cast<void>(ack_count);
+        static_cast<void>(seq_num);
+        static_cast<void>(fragments_state);
+        static_cast<void>(origin_vendor_id);
 
         result = false;
         return writer_guid == m_guid;