Allow ResourceLimitsQos serialization method compatibility (#5813)

* Refs #23183: Add vendor_id template methods to QosPoliciesSerializer

Signed-off-by: cferreiragonz <[email protected]>

* Refs #23183: Implement methods for Resource Limits

Signed-off-by: cferreiragonz <[email protected]>

* Refs #23183: Pass vendor_id to Resource Limits methods

Signed-off-by: cferreiragonz <[email protected]>

* Refs #23183: Apply suggestions

Signed-off-by: cferreiragonz <[email protected]>

* Refs #23183: Tests

Signed-off-by: cferreiragonz <[email protected]>

* Refs #23183: Apply Test suggestions

Signed-off-by: cferreiragonz <[email protected]>

* Refs #23183: Apply Test suggestions 2

Signed-off-by: cferreiragonz <[email protected]>

---------

Signed-off-by: cferreiragonz <[email protected]>

Author: cferreiragonz

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

@@ -47,6 +47,30 @@ class QosPoliciesSerializer
         return valid;
     }
 
+    /**
+     * @brief Fill a QosPolicy with the data found in a CDR message.
+     * @param vendor_id VendorId to check if specific Fast DDS fields need to be read.
+     * @param qos_policy QosPolicy to be filled.
+     * @param cdr_message CDRMessage to read from.
+     * @param parameter_length Length of the QosPolicy in the CDRMessage.
+     * @return true if the CDR message reading was successful.
+     */
+    static bool read_from_cdr_message(
+            const fastdds::rtps::VendorId_t&,
+            QosPolicy& qos_policy,
+            rtps::CDRMessage_t* cdr_message,
+            const uint16_t parameter_length)
+    {
+        return read_from_cdr_message(qos_policy, cdr_message, parameter_length);
+    }
+
+    /**
+     * @brief Fill a QosPolicy with the data found in a CDR message.
+     * @param qos_policy QosPolicy to be filled.
+     * @param cdr_message CDRMessage to read from.
+     * @param parameter_length Length of the QosPolicy in the CDRMessage.
+     * @return true if the CDR message reading was successful.
+     */
     static bool read_from_cdr_message(
             QosPolicy& qos_policy,
             rtps::CDRMessage_t* cdr_message,
@@ -64,7 +88,7 @@ class QosPoliciesSerializer
     }
 
     /**
-     * * @brief Check if the QosPolicy should be sent by checking if it is default.
+     * @brief Check if the QosPolicy should be sent by checking if it is default.
      * @param qos_policy QosPolicy to check
      * @return true if the QosPolicy should be sent, false otherwise.
      */
@@ -75,10 +99,10 @@ class QosPoliciesSerializer
     }
 
     /**
-     * * @brief Check if the QosPolicy should be sent. Default implementation checks if the QosPolicy is not default
+     * @brief Check if the QosPolicy should be sent. Default implementation checks if the QosPolicy is not default
      *          by calling the should_be_sent method.
      * @param qos_policy QosPolicy to check
-     * @param is_writer  Flag to indicate if the QosPolicy is for a writer. This flag is only used in overwrite methods
+     * @param is_writer  Flag to indicate if the QosPolicy is for a writer. This flag is only used in overwritten methods
      *                   of QosPolicies that have different default values for readers and writers.
      * @return true if the QosPolicy should be sent, false otherwise.
      */
@@ -91,7 +115,7 @@ class QosPoliciesSerializer
     }
 
     /**
-     * * @brief Check if the QosPolicy should be sent. Default implementation checks if the QosPolicy is not default.
+     * @brief Check if the QosPolicy should be sent. Default implementation checks if the QosPolicy is not default.
      * @param optional_qos_policy Optional QosPolicy to check.
      * @return true if the QosPolicy should be sent, false otherwise.
      */
@@ -115,6 +139,13 @@ class QosPoliciesSerializer
         return false;
     }
 
+    /**
+     * @brief Read the content of a CDR message and write it to a QosPolicy.
+     * @param qos_policy QosPolicy to be filled.
+     * @param cdr_message CDRMessage to read from.
+     * @param parameter_length Length of the QosPolicy in the CDRMessage.
+     * @return true if the CDR message reading was successful.
+     */
     static bool read_content_from_cdr_message(
             QosPolicy&,
             rtps::CDRMessage_t*,
@@ -124,6 +155,23 @@ class QosPoliciesSerializer
         return false;
     }
 
+    /**
+     * @brief Read the content of a CDR message and write it to a QosPolicy.
+     * @param vendor_id VendorId to check if specific Fast DDS fields need to be read.
+     * @param qos_policy QosPolicy to be filled.
+     * @param cdr_message CDRMessage to read from.
+     * @param parameter_length Length of the QosPolicy in the CDRMessage.
+     * @return true if the CDR message reading was successful.
+     */
+    static bool read_content_from_cdr_message(
+            const fastdds::rtps::VendorId_t&,
+            QosPolicy& qos_policy,
+            rtps::CDRMessage_t* cdr_message,
+            const uint16_t parameter_length)
+    {
+        return read_content_from_cdr_message(qos_policy, cdr_message, parameter_length);
+    }
+
 };
 
 template<>
@@ -392,11 +440,12 @@ inline bool QosPoliciesSerializer<ResourceLimitsQosPolicy>::add_content_to_cdr_m
 
 template<>
 inline bool QosPoliciesSerializer<ResourceLimitsQosPolicy>::read_content_from_cdr_message(
+        const fastdds::rtps::VendorId_t& vendor_id,
         ResourceLimitsQosPolicy& qos_policy,
         rtps::CDRMessage_t* cdr_message,
         const uint16_t parameter_length)
 {
-    if (parameter_length < 20)
+    if (parameter_length < 12)
     {
         return false;
     }
@@ -407,11 +456,41 @@ inline bool QosPoliciesSerializer<ResourceLimitsQosPolicy>::read_content_from_cd
                     &qos_policy.max_instances);
     valid &= rtps::CDRMessage::readInt32(cdr_message,
                     &qos_policy.max_samples_per_instance);
-    valid &= rtps::CDRMessage::readInt32(cdr_message, &qos_policy.allocated_samples);
-    valid &= rtps::CDRMessage::readInt32(cdr_message, &qos_policy.extra_samples);
+
+    if (vendor_id == rtps::c_VendorId_eProsima)
+    {
+        // The following two fields are not mandatory according the DDS specification
+        if (parameter_length >= 20)
+        {
+            valid &= rtps::CDRMessage::readInt32(cdr_message, &qos_policy.allocated_samples);
+            valid &= rtps::CDRMessage::readInt32(cdr_message, &qos_policy.extra_samples);
+        }
+    }
     return valid;
 }
 
+template<>
+inline bool QosPoliciesSerializer<ResourceLimitsQosPolicy>::read_from_cdr_message(
+        const fastdds::rtps::VendorId_t& vendor_id,
+        ResourceLimitsQosPolicy& qos_policy,
+        rtps::CDRMessage_t* cdr_message,
+        const uint16_t parameter_length)
+{
+    return read_content_from_cdr_message(vendor_id, qos_policy, cdr_message, parameter_length);
+}
+
+template<>
+inline bool QosPoliciesSerializer<ResourceLimitsQosPolicy>::read_from_cdr_message(
+        ResourceLimitsQosPolicy&,
+        rtps::CDRMessage_t*,
+        const uint16_t)
+{
+    EPROSIMA_LOG_ERROR(QOS_POLICIES_SERIALIZER,
+            "ResourceLimitsQosPolicy requires 'vendor_id' to be read from cdr message");
+    assert(false);
+    return false;
+}
+
 template<>
 inline bool QosPoliciesSerializer<TimeBasedFilterQosPolicy>::add_content_to_cdr_message(
         const TimeBasedFilterQosPolicy& qos_policy,

src/cpp/rtps/builtin/data/ReaderProxyData.cpp

@@ -1183,13 +1183,19 @@ bool ReaderProxyData::read_from_cdr_message(
 
                     case fastdds::dds::PID_RESOURCE_LIMITS:
                     {
+                        VendorId_t local_vendor_id = source_vendor_id;
+                        if (c_VendorId_Unknown == local_vendor_id)
+                        {
+                            local_vendor_id = ((c_VendorId_Unknown == vendor_id) ? c_VendorId_eProsima : vendor_id);
+                        }
+
                         if (!resource_limits)
                         {
                             resource_limits.reset(true);
                         }
 
                         if (!dds::QosPoliciesSerializer<dds::ResourceLimitsQosPolicy>::read_from_cdr_message(
-                                    resource_limits.value(), msg, plength))
+                                    local_vendor_id, resource_limits.value(), msg, plength))
                         {
                             EPROSIMA_LOG_ERROR(RTPS_READER_PROXY_DATA,
                                     "Received with error.");

src/cpp/rtps/builtin/data/WriterProxyData.cpp

@@ -1184,13 +1184,19 @@ bool WriterProxyData::read_from_cdr_message(
 
                     case fastdds::dds::PID_RESOURCE_LIMITS:
                     {
+                        VendorId_t local_vendor_id = source_vendor_id;
+                        if (c_VendorId_Unknown == local_vendor_id)
+                        {
+                            local_vendor_id = ((c_VendorId_Unknown == vendor_id) ? c_VendorId_eProsima : vendor_id);
+                        }
+
                         if (!resource_limits)
                         {
                             resource_limits.reset(true);
                         }
 
                         if (!dds::QosPoliciesSerializer<dds::ResourceLimitsQosPolicy>::read_from_cdr_message(
-                                    resource_limits.value(), msg, plength))
+                                    local_vendor_id, resource_limits.value(), msg, plength))
                         {
                             EPROSIMA_LOG_ERROR(RTPS_WRITER_PROXY_DATA,
                                     "Received with error.");

test/unittest/rtps/builtin/BuiltinDataSerializationTests.cpp

@@ -2186,6 +2186,88 @@ TEST(BuiltinDataSerializationTests, interoperability_with_intercomdds)
     }
 }
 
+/*!
+ * This test checks that Fast DDS can properly serialize ResourceLimitsQos in DATA(w) when it has more fields than
+ * max_samples, max_instances and max_samples_per_instance. It also checks that those fields are not serialized
+ * into the allocated_samples and extra samples fields.
+ */
+TEST(BuiltinDataSerializationTests, interoperability_with_other_vendor_writer_resource_limits)
+{
+    const VendorId_t other_vendor_id = { 0, 1 };
+    // DATA(w)
+    {
+        octet data_w_buffer[] =
+        {
+            // Encapsulation
+            0x00, 0x03, 0x00, 0x00,
+            // Resource limits
+            0x41, 0x00, 0x14, 0x00,
+            0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00,
+            0x05, 0x00, 0x00, 0x00,
+            // Sentinel
+            0x01, 0x00, 0x00, 0x00
+        };
+
+        CDRMessage_t msg(0);
+        msg.init(data_w_buffer, static_cast<uint32_t>(sizeof(data_w_buffer)));
+        msg.length = msg.max_size;
+
+        WriterProxyData wpd(max_unicast_locators, max_multicast_locators);
+        EXPECT_NO_THROW(EXPECT_TRUE(wpd.read_from_cdr_message(&msg, other_vendor_id)));
+
+        ASSERT_TRUE(wpd.resource_limits);
+        ASSERT_EQ(wpd.resource_limits->max_samples, 1);
+        ASSERT_EQ(wpd.resource_limits->max_instances, 2);
+        ASSERT_EQ(wpd.resource_limits->max_samples_per_instance, 3);
+        // Allocated samples and extra samples should have default values as they should not be read because
+        // they come from another vendor
+        dds::ResourceLimitsQosPolicy default_values {};
+        ASSERT_EQ(wpd.resource_limits->allocated_samples, default_values.allocated_samples);
+        ASSERT_EQ(wpd.resource_limits->extra_samples, default_values.extra_samples);
+    }
+}
+
+/*!
+ * This test checks that Fast DDS can properly serialize ResourceLimitsQos in DATA(r) when it has more fields than
+ * max_samples, max_instances and max_samples_per_instance. It also checks that those fields are not serialized
+ * into the allocated_samples and extra samples fields.
+ */
+TEST(BuiltinDataSerializationTests, interoperability_with_other_vendor_reader_resource_limits)
+{
+    const VendorId_t other_vendor_id = { 0, 1 };
+    // DATA(r)
+    {
+        uint8_t data_r_buffer[] =
+        {
+            // Encapsulation
+            0x00, 0x03, 0x00, 0x00,
+            // Resource limits
+            0x41, 0x00, 0x14, 0x00,
+            0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00,
+            0x05, 0x00, 0x00, 0x00,
+            // Sentinel
+            0x01, 0x00, 0x00, 0x00
+        };
+
+        CDRMessage_t msg(0);
+        msg.init(data_r_buffer, static_cast<uint32_t>(sizeof(data_r_buffer)));
+        msg.length = msg.max_size;
+
+        ReaderProxyData rpd(max_unicast_locators, max_multicast_locators);
+        EXPECT_NO_THROW(EXPECT_TRUE(rpd.read_from_cdr_message(&msg, other_vendor_id)));
+
+        ASSERT_TRUE(rpd.resource_limits);
+        ASSERT_EQ(rpd.resource_limits->max_samples, 1);
+        ASSERT_EQ(rpd.resource_limits->max_instances, 2);
+        ASSERT_EQ(rpd.resource_limits->max_samples_per_instance, 3);
+        // Allocated samples and extra samples should have default values as they should not be read because
+        // they come from another vendor
+        dds::ResourceLimitsQosPolicy default_values {};
+        ASSERT_EQ(rpd.resource_limits->allocated_samples, default_values.allocated_samples);
+        ASSERT_EQ(rpd.resource_limits->extra_samples, default_values.extra_samples);
+    }
+}
+
 /*!
  * This is a regression test for redmine issue #21537
  *
@@ -2654,6 +2736,41 @@ TEST(BuiltinDataSerializationTests, optional_qos_extensions_reader)
     ASSERT_EQ(rpd.reader_resource_limits->max_samples_per_read, 16);
 }
 
+/*!
+ * This test checks that a correct ReaderProxyData is obtained when sending only max_samples, max_instances
+ * and max_samples_per_instance in the ResourceLimits QoS policy
+ */
+TEST(BuiltinDataSerializationTests, optional_qos_extensions_reader_resource_limits)
+{
+    // DATA(r)
+    uint8_t data_r_buffer[] =
+    {
+        // Encapsulation
+        0x00, 0x03, 0x00, 0x00,
+        // Resource limits (without allocated samples nor extra samples)
+        0x41, 0x00, 0x0c, 0x00,
+        0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00,
+        // Sentinel
+        0x01, 0x00, 0x00, 0x00
+    };
+
+    CDRMessage_t msg(0);
+    msg.init(data_r_buffer, static_cast<uint32_t>(sizeof(data_r_buffer)));
+    msg.length = msg.max_size;
+
+    ReaderProxyData rpd(max_unicast_locators, max_multicast_locators);
+    EXPECT_NO_THROW(EXPECT_TRUE(rpd.read_from_cdr_message(&msg, c_VendorId_eProsima)));
+
+    ASSERT_TRUE(rpd.resource_limits);
+    ASSERT_EQ(rpd.resource_limits->max_samples, 1);
+    ASSERT_EQ(rpd.resource_limits->max_instances, 2);
+    ASSERT_EQ(rpd.resource_limits->max_samples_per_instance, 3);
+    // Allocated samples and extra samples should have default values as they are not present in the data
+    dds::ResourceLimitsQosPolicy default_values {};
+    ASSERT_EQ(rpd.resource_limits->allocated_samples, default_values.allocated_samples);
+    ASSERT_EQ(rpd.resource_limits->extra_samples, default_values.extra_samples);
+}
+
 /*!
  * This test checks that a correct WriterProxyData is obtained
  * from eProsima's optional qos extensions in PublicationBuiltinTopicData
@@ -2773,6 +2890,41 @@ TEST(BuiltinDataSerializationTests, optional_qos_extensions_writer)
     ASSERT_EQ(wpd.writer_resource_limits->reader_filters_allocation.increment, 6u);
 }
 
+/*!
+ * This test checks that a correct WriterProxyData is obtained when sending only max_samples, max_instances
+ * and max_samples_per_instance in the ResourceLimits QoS policy
+ */
+TEST(BuiltinDataSerializationTests, optional_qos_extensions_writer_resource_limits)
+{
+    // DATA(w)
+    octet data_w_buffer[] =
+    {
+        // Encapsulation
+        0x00, 0x03, 0x00, 0x00,
+        // Resource limits (without allocated samples nor extra samples)
+        0x41, 0x00, 0x0c, 0x00,
+        0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00,
+        // Sentinel
+        0x01, 0x00, 0x00, 0x00
+    };
+
+    CDRMessage_t msg(0);
+    msg.init(data_w_buffer, static_cast<uint32_t>(sizeof(data_w_buffer)));
+    msg.length = msg.max_size;
+
+    WriterProxyData wpd(max_unicast_locators, max_multicast_locators);
+    EXPECT_NO_THROW(EXPECT_TRUE(wpd.read_from_cdr_message(&msg, c_VendorId_eProsima)));
+
+    ASSERT_TRUE(wpd.resource_limits);
+    ASSERT_EQ(wpd.resource_limits->max_samples, 1);
+    ASSERT_EQ(wpd.resource_limits->max_instances, 2);
+    ASSERT_EQ(wpd.resource_limits->max_samples_per_instance, 3);
+    // Allocated samples and extra samples should have default values as they are not present in the data
+    dds::ResourceLimitsQosPolicy default_values {};
+    ASSERT_EQ(wpd.resource_limits->allocated_samples, default_values.allocated_samples);
+    ASSERT_EQ(wpd.resource_limits->extra_samples, default_values.extra_samples);
+}
+
 /**
  * This test checks that a correct serialization is obtained
  * when non-default eProsima's optional qos extensions are used in ReaderProxyData.