Skip to content

[23183] Allow ResourceLimitsQos serialization method compatibility #5813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 86 additions & 7 deletions src/cpp/fastdds/core/policy/QosPoliciesSerializer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
*/
Expand All @@ -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.
*/
Expand All @@ -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.
*/
Expand All @@ -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*,
Expand All @@ -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<>
Expand Down Expand Up @@ -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;
}
Expand All @@ -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,
Expand Down
8 changes: 7 additions & 1 deletion src/cpp/rtps/builtin/data/ReaderProxyData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down
8 changes: 7 additions & 1 deletion src/cpp/rtps/builtin/data/WriterProxyData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down
152 changes: 152 additions & 0 deletions test/unittest/rtps/builtin/BuiltinDataSerializationTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
Loading