From 7db38f46424233255b36e0430594665df4c0d7e6 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 10 Oct 2023 07:54:39 +0200 Subject: [PATCH] Fix updatability of immutable DataWriterQos (#3915) * Refs #19687. Add regression test. Signed-off-by: Miguel Company * Refs #19687. Rename argument. Signed-off-by: Miguel Company * Refs #19687. Refactor on DataWriterImpl::set_qos. Signed-off-by: Miguel Company --------- Signed-off-by: Miguel Company --- src/cpp/fastdds/publisher/DataWriterImpl.cpp | 159 ++++++++++-------- src/cpp/fastdds/publisher/DataWriterImpl.hpp | 2 +- .../dds/publisher/DataWriterTests.cpp | 53 ++++++ 3 files changed, 145 insertions(+), 69 deletions(-) diff --git a/src/cpp/fastdds/publisher/DataWriterImpl.cpp b/src/cpp/fastdds/publisher/DataWriterImpl.cpp index 1b448dd2bea..c465b46bd8f 100644 --- a/src/cpp/fastdds/publisher/DataWriterImpl.cpp +++ b/src/cpp/fastdds/publisher/DataWriterImpl.cpp @@ -1124,7 +1124,7 @@ ReturnCode_t DataWriterImpl::set_qos( return ReturnCode_t::RETCODE_IMMUTABLE_POLICY; } - set_qos(qos_, qos_to_set, enabled); + set_qos(qos_, qos_to_set, !enabled); if (enabled) { @@ -1614,114 +1614,137 @@ LivelinessLostStatus& DataWriterImpl::update_liveliness_lost_status( void DataWriterImpl::set_qos( DataWriterQos& to, const DataWriterQos& from, - bool is_default) + bool update_immutable) { - if (is_default && !(to.durability() == from.durability())) + // Check immutable policies + if (update_immutable) { - to.durability() = from.durability(); - to.durability().hasChanged = true; - } - if (is_default && !(to.durability_service() == from.durability_service())) - { - to.durability_service() = from.durability_service(); - to.durability_service().hasChanged = true; + if (!(to.durability() == from.durability())) + { + to.durability() = from.durability(); + to.durability().hasChanged = true; + } + + if (!(to.durability_service() == from.durability_service())) + { + to.durability_service() = from.durability_service(); + to.durability_service().hasChanged = true; + } + + if (!(to.liveliness() == from.liveliness())) + { + to.liveliness() = from.liveliness(); + to.liveliness().hasChanged = true; + } + + if (!(to.reliability().kind == from.reliability().kind)) + { + to.reliability().kind = from.reliability().kind; + to.reliability().hasChanged = true; + } + + if (!(to.destination_order() == from.destination_order())) + { + to.destination_order() = from.destination_order(); + to.destination_order().hasChanged = true; + } + + if (!(to.history() == from.history())) + { + to.history() = from.history(); + to.history().hasChanged = true; + } + + if (!(to.resource_limits() == from.resource_limits())) + { + to.resource_limits() = from.resource_limits(); + to.resource_limits().hasChanged = true; + } + + if (!(to.ownership() == from.ownership())) + { + to.ownership() = from.ownership(); + to.ownership().hasChanged = true; + } + + to.publish_mode() = from.publish_mode(); + + if (!(to.representation() == from.representation())) + { + to.representation() = from.representation(); + to.representation().hasChanged = true; + } + + to.properties() = from.properties(); + + if (!(to.reliable_writer_qos() == from.reliable_writer_qos())) + { + RTPSReliableWriterQos& rel_to = to.reliable_writer_qos(); + rel_to.disable_heartbeat_piggyback = from.reliable_writer_qos().disable_heartbeat_piggyback; + rel_to.disable_positive_acks.enabled = from.reliable_writer_qos().disable_positive_acks.enabled; + } + + to.endpoint() = from.endpoint(); + + to.writer_resource_limits() = from.writer_resource_limits(); + + to.data_sharing() = from.data_sharing(); + + to.throughput_controller() = from.throughput_controller(); } + if (!(to.deadline() == from.deadline())) { to.deadline() = from.deadline(); to.deadline().hasChanged = true; } + if (!(to.latency_budget() == from.latency_budget())) { to.latency_budget() = from.latency_budget(); to.latency_budget().hasChanged = true; } - if (is_default && !(to.liveliness() == from.liveliness())) - { - to.liveliness() = from.liveliness(); - to.liveliness().hasChanged = true; - } - if (is_default && !(to.reliability() == from.reliability())) + + if (!(to.reliability().max_blocking_time == from.reliability().max_blocking_time)) { - to.reliability() = from.reliability(); + to.reliability().max_blocking_time = from.reliability().max_blocking_time; to.reliability().hasChanged = true; } - if (is_default && !(to.destination_order() == from.destination_order())) - { - to.destination_order() = from.destination_order(); - to.destination_order().hasChanged = true; - } - if (is_default && !(to.history() == from.history())) - { - to.history() = from.history(); - to.history().hasChanged = true; - } - if (is_default && !(to.resource_limits() == from.resource_limits())) - { - to.resource_limits() = from.resource_limits(); - to.resource_limits().hasChanged = true; - } + if (!(to.transport_priority() == from.transport_priority())) { to.transport_priority() = from.transport_priority(); to.transport_priority().hasChanged = true; } + if (!(to.lifespan() == from.lifespan())) { to.lifespan() = from.lifespan(); to.lifespan().hasChanged = true; } + if (!(to.user_data() == from.user_data())) { to.user_data() = from.user_data(); to.user_data().hasChanged = true; } - if (is_default && !(to.ownership() == from.ownership())) - { - to.ownership() = from.ownership(); - to.ownership().hasChanged = true; - } + if (!(to.ownership_strength() == from.ownership_strength())) { to.ownership_strength() = from.ownership_strength(); to.ownership_strength().hasChanged = true; } + if (!(to.writer_data_lifecycle() == from.writer_data_lifecycle())) { to.writer_data_lifecycle() = from.writer_data_lifecycle(); } - if (is_default && !(to.publish_mode() == from.publish_mode())) - { - to.publish_mode() = from.publish_mode(); - } - if (!(to.representation() == from.representation())) - { - to.representation() = from.representation(); - to.representation().hasChanged = true; - } - if (is_default && !(to.properties() == from.properties())) - { - to.properties() = from.properties(); - } - if (is_default && !(to.reliable_writer_qos() == from.reliable_writer_qos())) - { - to.reliable_writer_qos() = from.reliable_writer_qos(); - } - if (is_default && !(to.endpoint() == from.endpoint())) - { - to.endpoint() = from.endpoint(); - } - if (is_default && !(to.writer_resource_limits() == from.writer_resource_limits())) - { - to.writer_resource_limits() = from.writer_resource_limits(); - } - if (is_default && !(to.throughput_controller() == from.throughput_controller())) - { - to.throughput_controller() = from.throughput_controller(); - } - if (is_default && !(to.data_sharing() == from.data_sharing())) + + if (!(to.reliable_writer_qos() == from.reliable_writer_qos())) { - to.data_sharing() = from.data_sharing(); + RTPSReliableWriterQos& rel_to = to.reliable_writer_qos(); + rel_to.times = from.reliable_writer_qos().times; + rel_to.disable_positive_acks.duration = from.reliable_writer_qos().disable_positive_acks.duration; } } diff --git a/src/cpp/fastdds/publisher/DataWriterImpl.hpp b/src/cpp/fastdds/publisher/DataWriterImpl.hpp index c9ef7699876..3399bfb7e35 100644 --- a/src/cpp/fastdds/publisher/DataWriterImpl.hpp +++ b/src/cpp/fastdds/publisher/DataWriterImpl.hpp @@ -587,7 +587,7 @@ class DataWriterImpl : protected rtps::IReaderDataFilter static void set_qos( DataWriterQos& to, const DataWriterQos& from, - bool is_default); + bool update_immutable); /** * Extends the check_qos() call, including the check for diff --git a/test/unittest/dds/publisher/DataWriterTests.cpp b/test/unittest/dds/publisher/DataWriterTests.cpp index 5b57dd1305d..b19e70dec87 100644 --- a/test/unittest/dds/publisher/DataWriterTests.cpp +++ b/test/unittest/dds/publisher/DataWriterTests.cpp @@ -429,6 +429,59 @@ TEST(DataWriterTests, ChangeDataWriterQos) ASSERT_TRUE(DomainParticipantFactory::get_instance()->delete_participant(participant) == ReturnCode_t::RETCODE_OK); } +TEST(DataWriterTests, ChangeImmutableDataWriterQos) +{ + DomainParticipant* participant = + DomainParticipantFactory::get_instance()->create_participant(0, PARTICIPANT_QOS_DEFAULT); + ASSERT_NE(participant, nullptr); + + PublisherQos pub_qos = PUBLISHER_QOS_DEFAULT; + pub_qos.entity_factory().autoenable_created_entities = false; + Publisher* publisher = participant->create_publisher(pub_qos); + ASSERT_NE(publisher, nullptr); + + TypeSupport type(new TopicDataTypeMock()); + type.register_type(participant); + + Topic* topic = participant->create_topic("footopic", type.get_type_name(), TOPIC_QOS_DEFAULT); + ASSERT_NE(topic, nullptr); + + DataWriter* datawriter = publisher->create_datawriter(topic, DATAWRITER_QOS_DEFAULT); + ASSERT_NE(datawriter, nullptr); + + ASSERT_FALSE(datawriter->is_enabled()); + + DataWriterQos qos; + datawriter->get_qos(qos); + ASSERT_EQ(qos, DATAWRITER_QOS_DEFAULT); + + qos.reliable_writer_qos().disable_positive_acks.enabled = true; + + ASSERT_TRUE(datawriter->set_qos(qos) == ReturnCode_t::RETCODE_OK); + DataWriterQos wqos; + datawriter->get_qos(wqos); + + ASSERT_EQ(qos, wqos); + ASSERT_TRUE(wqos.reliable_writer_qos().disable_positive_acks.enabled); + + ASSERT_TRUE(datawriter->enable() == ReturnCode_t::RETCODE_OK); + ASSERT_TRUE(datawriter->is_enabled()); + + qos.reliable_writer_qos().disable_positive_acks.enabled = false; + ASSERT_FALSE(qos == wqos); + ASSERT_TRUE(datawriter->set_qos(qos) == ReturnCode_t::RETCODE_IMMUTABLE_POLICY); + + DataWriterQos wqos2; + datawriter->get_qos(wqos2); + ASSERT_EQ(wqos, wqos2); + ASSERT_TRUE(wqos2.reliable_writer_qos().disable_positive_acks.enabled); + + ASSERT_TRUE(publisher->delete_datawriter(datawriter) == ReturnCode_t::RETCODE_OK); + ASSERT_TRUE(participant->delete_topic(topic) == ReturnCode_t::RETCODE_OK); + ASSERT_TRUE(participant->delete_publisher(publisher) == ReturnCode_t::RETCODE_OK); + ASSERT_TRUE(DomainParticipantFactory::get_instance()->delete_participant(participant) == ReturnCode_t::RETCODE_OK); +} + TEST(DataWriterTests, ForcedDataSharing) { DomainParticipant* participant =