diff --git a/src/cpp/rtps/messages/submessages/DataMsg.hpp b/src/cpp/rtps/messages/submessages/DataMsg.hpp index 586d5764003..8a295d5e5cb 100644 --- a/src/cpp/rtps/messages/submessages/DataMsg.hpp +++ b/src/cpp/rtps/messages/submessages/DataMsg.hpp @@ -42,7 +42,8 @@ struct DataMsgUtils { inlineQosFlag = (nullptr != inlineQos) || - ((WITH_KEY == topicKind) && (expectsInlineQos || change->kind != ALIVE)) || + ((WITH_KEY == topicKind) && + (!change->writerGUID.is_builtin() || expectsInlineQos || change->kind != ALIVE)) || (change->write_params.related_sample_identity() != SampleIdentity::unknown()); dataFlag = ALIVE == change->kind && @@ -129,7 +130,7 @@ struct DataMsgUtils change->write_params.related_sample_identity()); } - if (WITH_KEY == topicKind && (expectsInlineQos || ALIVE != change->kind)) + if (WITH_KEY == topicKind && (!change->writerGUID.is_builtin() || expectsInlineQos || ALIVE != change->kind)) { fastdds::dds::ParameterSerializer::add_parameter_key(msg, change->instanceHandle); diff --git a/test/blackbox/common/BlackboxTestsKeys.cpp b/test/blackbox/common/BlackboxTestsKeys.cpp index f2a4f7f4817..5e58a978b7b 100644 --- a/test/blackbox/common/BlackboxTestsKeys.cpp +++ b/test/blackbox/common/BlackboxTestsKeys.cpp @@ -17,6 +17,8 @@ #include "PubSubReader.hpp" #include "PubSubWriter.hpp" +#include + TEST(KeyedTopic, RegistrationNonKeyedFail) { PubSubWriter writer(TEST_TOPIC_NAME); @@ -181,6 +183,105 @@ TEST(KeyedTopic, UnregisterWhenHistoryKeepAll) ASSERT_TRUE(writer.unregister_instance(data.back(), instance_handle_2)); } +// Regression test for redmine issue #20239 +TEST(KeyedTopic, DataWriterAlwaysSendTheSerializedKeyViaInlineQoS) +{ + PubSubWriter writer(TEST_TOPIC_NAME); + PubSubReader reader(TEST_TOPIC_NAME); + + auto testTransport = std::make_shared(); + + bool writer_sends_inline_qos = true; + bool writer_sends_pid_key_hash = true; + + testTransport->drop_data_messages_filter_ = [&writer_sends_inline_qos, + &writer_sends_pid_key_hash](eprosima::fastrtps::rtps::CDRMessage_t& msg) -> bool + { + // Check for inline_qos + uint8_t flags = msg.buffer[msg.pos - 3]; + auto old_pos = msg.pos; + + // Skip extraFlags, read octetsToInlineQos, and calculate inline qos position. + msg.pos += 2; + uint16_t to_inline_qos = 0; + eprosima::fastrtps::rtps::CDRMessage::readUInt16(&msg, &to_inline_qos); + uint32_t inline_qos_pos = msg.pos + to_inline_qos; + + // Filters are only applied to user data + // no need to check if the packets comer from a builtin + + writer_sends_inline_qos &= static_cast((flags & (1 << 1))); + + // Stop seeking if inline qos are not present + // Fail the test afterwards + if (!writer_sends_inline_qos) + { + return false; + } + else + { + // Process inline qos + msg.pos = inline_qos_pos; + bool key_hash_was_found = false; + while (msg.pos < msg.length) + { + uint16_t pid = 0; + uint16_t plen = 0; + + eprosima::fastrtps::rtps::CDRMessage::readUInt16(&msg, &pid); + eprosima::fastrtps::rtps::CDRMessage::readUInt16(&msg, &plen); + uint32_t next_pos = msg.pos + plen; + + if (pid == eprosima::fastdds::dds::PID_KEY_HASH) + { + key_hash_was_found = true; + } + else if (pid == eprosima::fastdds::dds::PID_SENTINEL) + { + break; + } + + msg.pos = next_pos; + } + + writer_sends_pid_key_hash &= key_hash_was_found; + msg.pos = old_pos; + } + + // Do not drop the packet in any case + return false; + }; + + writer. + disable_builtin_transport(). + add_user_transport_to_pparams(testTransport). + init(); + + ASSERT_TRUE(writer.isInitialized()); + + reader. + expect_inline_qos(false). + init(); + + ASSERT_TRUE(reader.isInitialized()); + + // Wait for discovery. + writer.wait_discovery(); + reader.wait_discovery(); + + auto data = default_keyedhelloworld_data_generator(5); + + reader.startReception(data); + writer.send(data); + + // In this test all data should be sent. + EXPECT_TRUE(data.empty()); + reader.block_for_all(); + + EXPECT_TRUE(writer_sends_inline_qos); + EXPECT_TRUE(writer_sends_pid_key_hash); +} + /* Uncomment when DDS API supports NO_WRITERS_ALIVE TEST(KeyedTopic, WriteSamplesBestEffort) { diff --git a/test/blackbox/common/DDSBlackboxTestsListeners.cpp b/test/blackbox/common/DDSBlackboxTestsListeners.cpp index 5a9acdf4f02..90a0d9d4bfb 100644 --- a/test/blackbox/common/DDSBlackboxTestsListeners.cpp +++ b/test/blackbox/common/DDSBlackboxTestsListeners.cpp @@ -1732,7 +1732,7 @@ TEST(DDSStatus, sample_rejected_key_re_dw_re_dr_keep_all_max_samples_2) ASSERT_EQ(5u, test_status.total_count); ASSERT_EQ(5u, test_status.total_count_change); ASSERT_EQ(eprosima::fastdds::dds::REJECTED_BY_SAMPLES_LIMIT, test_status.last_reason); - ASSERT_EQ(c_InstanceHandle_Unknown, test_status.last_instance_handle); + ASSERT_NE(c_InstanceHandle_Unknown, test_status.last_instance_handle); } /*! @@ -1832,7 +1832,7 @@ TEST(DDSStatus, sample_rejected_key_large_re_dw_re_dr_keep_all_max_samples_2) ASSERT_EQ(5u, test_status.total_count); ASSERT_EQ(5u, test_status.total_count_change); ASSERT_EQ(eprosima::fastdds::dds::REJECTED_BY_SAMPLES_LIMIT, test_status.last_reason); - ASSERT_EQ(c_InstanceHandle_Unknown, test_status.last_instance_handle); + ASSERT_NE(c_InstanceHandle_Unknown, test_status.last_instance_handle); } /*! @@ -1926,7 +1926,7 @@ TEST(DDSStatus, sample_rejected_key_re_dw_re_dr_keep_last_max_samples_2) ASSERT_EQ(5u, test_status.total_count); ASSERT_EQ(5u, test_status.total_count_change); ASSERT_EQ(eprosima::fastdds::dds::REJECTED_BY_SAMPLES_LIMIT, test_status.last_reason); - ASSERT_EQ(c_InstanceHandle_Unknown, test_status.last_instance_handle); + ASSERT_NE(c_InstanceHandle_Unknown, test_status.last_instance_handle); } /*! @@ -2026,7 +2026,7 @@ TEST(DDSStatus, sample_rejected_key_large_re_dw_re_dr_keep_last_max_samples_2) ASSERT_EQ(5u, test_status.total_count); ASSERT_EQ(5u, test_status.total_count_change); ASSERT_EQ(eprosima::fastdds::dds::REJECTED_BY_SAMPLES_LIMIT, test_status.last_reason); - ASSERT_EQ(c_InstanceHandle_Unknown, test_status.last_instance_handle); + ASSERT_NE(c_InstanceHandle_Unknown, test_status.last_instance_handle); } /*!