From 63d6260df47284b57d8c1957aa148625666507fe Mon Sep 17 00:00:00 2001 From: Miguel Barro Date: Tue, 27 Apr 2021 08:15:35 +0200 Subject: [PATCH 01/11] [10796] RESENT_DATAS callback implementation --- include/fastdds/rtps/writer/ReaderProxy.h | 8 +++---- .../statistics/rtps/StatisticsCommon.hpp | 7 ++++++ .../statistics/rtps/StatisticsCommonEmpty.hpp | 9 ++++++++ src/cpp/rtps/writer/ReaderProxy.cpp | 10 ++++----- src/cpp/rtps/writer/StatefulWriter.cpp | 15 +++++++++++-- src/cpp/statistics/rtps/StatisticsBase.hpp | 8 +++++-- .../rtps/writer/StatisticsWriterImpl.cpp | 22 +++++++++++++++++++ 7 files changed, 66 insertions(+), 13 deletions(-) diff --git a/include/fastdds/rtps/writer/ReaderProxy.h b/include/fastdds/rtps/writer/ReaderProxy.h index c1ecc1089d6..13644eb7e80 100644 --- a/include/fastdds/rtps/writer/ReaderProxy.h +++ b/include/fastdds/rtps/writer/ReaderProxy.h @@ -235,9 +235,9 @@ class ReaderProxy /** * Turns all REQUESTED changes into UNSENT. - * @return true if at least one change changed its status, false otherwise. + * @return the number of changes that changed its status. */ - bool perform_acknack_response(); + uint32_t perform_acknack_response(); /** * Call this to inform a change was removed from history. @@ -466,9 +466,9 @@ class ReaderProxy * Converts all changes with a given status to a different status. * @param previous Status to change. * @param next Status to adopt. - * @return true when at least one change has been modified, false otherwise. + * @return the number of changes that have been modified. */ - bool convert_status_on_all_changes( + uint32_t convert_status_on_all_changes( ChangeForReaderStatus_t previous, ChangeForReaderStatus_t next); diff --git a/include/fastdds/statistics/rtps/StatisticsCommon.hpp b/include/fastdds/statistics/rtps/StatisticsCommon.hpp index ae40dc92ecf..fbcf1bee85f 100644 --- a/include/fastdds/statistics/rtps/StatisticsCommon.hpp +++ b/include/fastdds/statistics/rtps/StatisticsCommon.hpp @@ -159,6 +159,13 @@ class StatisticsWriterImpl //! Report that a GAP message is sent void on_gap(); + + /* + * @brief Report that several changes are marked for redelivery + * @param number of changes to redeliver + */ + void on_resent_data( + uint32_t to_send); }; // Members are private details diff --git a/include/fastdds/statistics/rtps/StatisticsCommonEmpty.hpp b/include/fastdds/statistics/rtps/StatisticsCommonEmpty.hpp index 0ff091972bb..f1ed1afb7dc 100644 --- a/include/fastdds/statistics/rtps/StatisticsCommonEmpty.hpp +++ b/include/fastdds/statistics/rtps/StatisticsCommonEmpty.hpp @@ -65,6 +65,15 @@ class StatisticsWriterImpl { } + /* + * @brief Report that several changes are marked for redelivery + * @param number of changes to redeliver + */ + inline void on_resent_data( + uint32_t) + { + } + }; class StatisticsReaderImpl diff --git a/src/cpp/rtps/writer/ReaderProxy.cpp b/src/cpp/rtps/writer/ReaderProxy.cpp index c7b5b5470f1..f1a9d854af6 100644 --- a/src/cpp/rtps/writer/ReaderProxy.cpp +++ b/src/cpp/rtps/writer/ReaderProxy.cpp @@ -501,12 +501,12 @@ bool ReaderProxy::perform_nack_supression() return convert_status_on_all_changes(UNDERWAY, UNACKNOWLEDGED); } -bool ReaderProxy::perform_acknack_response() +uint32_t ReaderProxy::perform_acknack_response() { return convert_status_on_all_changes(REQUESTED, UNSENT); } -bool ReaderProxy::convert_status_on_all_changes( +uint32_t ReaderProxy::convert_status_on_all_changes( ChangeForReaderStatus_t previous, ChangeForReaderStatus_t next) { @@ -515,17 +515,17 @@ bool ReaderProxy::convert_status_on_all_changes( // NOTE: This is only called for REQUESTED=>UNSENT (acknack response) or // UNDERWAY=>UNACKNOWLEDGED (nack supression) - bool at_least_one_modified = false; + uint32_t changed = 0; for (ChangeForReader_t& change : changes_for_reader_) { if (change.getStatus() == previous) { - at_least_one_modified = true; + ++changed; change.setStatus(next); } } - return at_least_one_modified; + return changed; } void ReaderProxy::change_has_been_removed( diff --git a/src/cpp/rtps/writer/StatefulWriter.cpp b/src/cpp/rtps/writer/StatefulWriter.cpp index 99bfe1fcfc3..189b33cad6a 100644 --- a/src/cpp/rtps/writer/StatefulWriter.cpp +++ b/src/cpp/rtps/writer/StatefulWriter.cpp @@ -2379,14 +2379,20 @@ void StatefulWriter::perform_nack_response() { std::unique_lock lock(mp_mutex); bool must_wake_up_async_thread = false; + uint32_t changes_to_resend = 0; + for_matched_readers(matched_local_readers_, matched_datasharing_readers_, matched_remote_readers_, - [&must_wake_up_async_thread](ReaderProxy* reader) + [&must_wake_up_async_thread, &changes_to_resend](ReaderProxy* reader) { - if (reader->perform_acknack_response() || reader->are_there_gaps()) + uint32_t pending = reader->perform_acknack_response(); + changes_to_resend += pending; + + if ( pending > 0 || reader->are_there_gaps()) { must_wake_up_async_thread = true; // Do not exit the loop, perform_acknack_response must be executed for all readers } + return false; } ); @@ -2395,6 +2401,11 @@ void StatefulWriter::perform_nack_response() { mp_RTPSParticipant->async_thread().wake_up(this); } + + lock.unlock(); + + // Notify the statistics module + on_resent_data(changes_to_resend); } void StatefulWriter::perform_nack_supression( diff --git a/src/cpp/statistics/rtps/StatisticsBase.hpp b/src/cpp/statistics/rtps/StatisticsBase.hpp index 058dc08e6b4..9c75aa5e008 100644 --- a/src/cpp/statistics/rtps/StatisticsBase.hpp +++ b/src/cpp/statistics/rtps/StatisticsBase.hpp @@ -57,6 +57,7 @@ struct StatisticsWriterAncillary { unsigned long long data_counter = {}; unsigned long long gap_counter = {}; + unsigned long long resent_counter = {}; }; struct StatisticsReaderAncillary @@ -69,11 +70,14 @@ template Function StatisticsListenersImpl::for_each_listener( Function f) { - std::lock_guard lock(get_statistics_mutex()); + // Use a collection copy to prevent locking on traversal + std::unique_lock lock(get_statistics_mutex()); + auto listeners = members_->listeners; + lock.unlock(); if (members_) { - for (auto& listener : members_->listeners) + for (auto& listener : listeners) { f(listener); } diff --git a/src/cpp/statistics/rtps/writer/StatisticsWriterImpl.cpp b/src/cpp/statistics/rtps/writer/StatisticsWriterImpl.cpp index 6bef809256e..2c7c81837db 100644 --- a/src/cpp/statistics/rtps/writer/StatisticsWriterImpl.cpp +++ b/src/cpp/statistics/rtps/writer/StatisticsWriterImpl.cpp @@ -128,3 +128,25 @@ void StatisticsWriterImpl::on_gap() listener->on_statistics_data(data); }); } + +void StatisticsWriterImpl::on_resent_data( + uint32_t to_send) +{ + EntityCount notification; + notification.guid(to_statistics_type(get_guid())); + + { + std::lock_guard lock(get_statistics_mutex()); + notification.count(get_members()->resent_counter += to_send); + } + + // Perform the callbacks + Data data; + // note that the setter sets RESENT_DATAS by default + data.entity_count(notification); + + for_each_listener([&data](const std::shared_ptr& listener) + { + listener->on_statistics_data(data); + }); +} From c2cb127b4acd2b8158e6c2e8bc7674f962563e51 Mon Sep 17 00:00:00 2001 From: Miguel Barro Date: Tue, 27 Apr 2021 09:37:38 +0200 Subject: [PATCH 02/11] [10796] RESENT_DATAS test implementation --- .../statistics/rtps/RTPSStatisticsTests.cpp | 52 +++++++++++++++++-- 1 file changed, 47 insertions(+), 5 deletions(-) diff --git a/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp b/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp index cd084c54820..e31887244b1 100644 --- a/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp +++ b/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp @@ -69,6 +69,9 @@ struct MockListener : IListener case DATA_COUNT: on_data_count(data.entity_count()); break; + case RESENT_DATAS: + on_resent_count(data.entity_count()); + break; case GAP_COUNT: on_gap_count(data.entity_count()); break; @@ -472,6 +475,7 @@ TEST_F(RTPSStatisticsTests, statistics_rpts_listener_management) * This test checks RTPSParticipant, RTPSWriter and RTPSReader statistics module related APIs. * - RTPS_SENT callbacks are performed * - DATA_COUNT callbacks are performed for DATA submessages + * - RESENT_DATAS callbacks are performed for DATA submessages demanded by the readers * - ACKNACK_COUNT callbacks are performed * - HEARBEAT_COUNT callbacks are performed */ @@ -482,6 +486,39 @@ TEST_F(RTPSStatisticsTests, statistics_rpts_listener_callbacks) using namespace fastrtps::rtps; using namespace std; + // make sure some messages are lost to assure the RESENT_DATAS callback + set_transport_filter( + DATA, + [](fastrtps::rtps::CDRMessage_t& msg)-> bool + { + static unsigned int samples_filtered = 0; + uint32_t old_pos = msg.pos; + + // see RTPS DDS 9.4.5.3 Data Submessage + EntityId_t readerID, writerID; + SequenceNumber_t sn; + + msg.pos += 2; // flags + msg.pos += 2; // octets to inline quos + CDRMessage::readEntityId(&msg, &readerID); + CDRMessage::readEntityId(&msg, &writerID); + CDRMessage::readSequenceNumber(&msg, &sn); + + // restore buffer pos + msg.pos = old_pos; + + // generate losses + if ( samples_filtered < 10 // only a few times (mind the interfaces) + && (writerID.value[3] & 0xC0) == 0 // only user endpoints + && (sn == SequenceNumber_t{0, 1})) // only first sample + { + ++samples_filtered; + return true; + } + + return false; + }); + // create the testing endpoints uint16_t length = 255; create_endpoints(length, RELIABLE); @@ -492,7 +529,8 @@ TEST_F(RTPSStatisticsTests, statistics_rpts_listener_callbacks) // writer callbacks through participant listener auto participant_writer_listener = make_shared(); - ASSERT_TRUE(participant_->add_statistics_listener(participant_writer_listener, EventKind::DATA_COUNT)); + ASSERT_TRUE(participant_->add_statistics_listener(participant_writer_listener, + EventKind::DATA_COUNT | EventKind::RESENT_DATAS)); // writer specific callbacks auto writer_listener = make_shared(); @@ -518,9 +556,13 @@ TEST_F(RTPSStatisticsTests, statistics_rpts_listener_callbacks) .Times(AtLeast(1)); EXPECT_CALL(*writer_listener, on_data_count) .Times(AtLeast(1)); + EXPECT_CALL(*writer_listener, on_resent_count) + .Times(AtLeast(1)); EXPECT_CALL(*participant_writer_listener, on_data_count) .Times(AtLeast(1)); + EXPECT_CALL(*participant_writer_listener, on_resent_count) + .Times(AtLeast(1)); // + RTPSReader: SUBSCRIPTION_THROUGHPUT, // SAMPLE_DATAS & PHYSICAL_DATA @@ -552,7 +594,8 @@ TEST_F(RTPSStatisticsTests, statistics_rpts_listener_callbacks) EXPECT_TRUE(reader_->remove_statistics_listener(reader_listener)); EXPECT_TRUE(participant_->remove_statistics_listener(participant_listener, EventKind::RTPS_SENT)); - EXPECT_TRUE(participant_->remove_statistics_listener(participant_writer_listener, EventKind::DATA_COUNT)); + EXPECT_TRUE(participant_->remove_statistics_listener(participant_writer_listener, + EventKind::DATA_COUNT | EventKind::RESENT_DATAS)); EXPECT_TRUE(participant_->remove_statistics_listener(participant_reader_listener, EventKind::ACKNACK_COUNT)); } @@ -658,6 +701,8 @@ TEST_F(RTPSStatisticsTests, statistics_rpts_listener_gap_callback) .Times(AtLeast(1)); EXPECT_CALL(*writer_listener, on_data_count) .Times(AtLeast(1)); + EXPECT_CALL(*writer_listener, on_resent_count) + .Times(AtLeast(1)); EXPECT_CALL(*participant_writer_listener, on_gap_count) .Times(AtLeast(1)); @@ -684,9 +729,6 @@ TEST_F(RTPSStatisticsTests, statistics_rpts_listener_gap_callback) // match writer and reader on a dummy topic match_endpoints(false, "string", "statisticsSmallTopic"); - // std::this_thread::sleep_for(std::chrono::seconds(10)); - - // wait for reception EXPECT_TRUE(reader_->wait_for_unread_cache(Duration_t(5, 0))); From 7e22bd31650b2d998ddd2b8f67e70f0500d05e74 Mon Sep 17 00:00:00 2001 From: Miguel Barro Date: Tue, 27 Apr 2021 09:54:54 +0200 Subject: [PATCH 03/11] [10796] speed up statistics testing by reducing wait in acknowledgement milestones --- test/unittest/statistics/rtps/RTPSStatisticsTests.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp b/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp index e31887244b1..cd1d9bc3544 100644 --- a/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp +++ b/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp @@ -237,6 +237,8 @@ class RTPSStatisticsTestsImpl writer_history_ = new WriterHistory(history_attributes); WriterAttributes w_att; + w_att.times.heartbeatPeriod.seconds = 0; + w_att.times.heartbeatPeriod.nanosec = 250 * 1000 * 1000; // reduce acknowledgement wait w_att.endpoint.reliabilityKind = reliability_qos; w_att.endpoint.durabilityKind = durability_qos; From 04ce29aa82450381515902c5856ce82c55b4c2a7 Mon Sep 17 00:00:00 2001 From: Miguel Barro Date: Tue, 27 Apr 2021 10:36:07 +0200 Subject: [PATCH 04/11] Update Github action --- .github/workflows/statistics_coverage.yml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/statistics_coverage.yml b/.github/workflows/statistics_coverage.yml index 3f3966f3c0a..1a422e6865b 100644 --- a/.github/workflows/statistics_coverage.yml +++ b/.github/workflows/statistics_coverage.yml @@ -30,16 +30,27 @@ jobs: run: | cat src/Fast-DDS/.github/workflows/statistics_module.meta colcon build \ + --packages-up-to fastrtps --packages-skip fastrtps \ --event-handlers=console_direct+ \ --metas src/Fast-DDS/.github/workflows/statistics_module.meta \ --mixin coverage-gcc + for target in RTPSStatisticsTests StatisticsDomainParticipantTests StatisticsQosTests StatisticsDomainParticipantMockTests + do + colcon build \ + --packages-select fastrtps \ + --cmake-target $target \ + --cmake-target-skip-unavailable \ + --event-handlers=console_direct+ \ + --metas src/Fast-DDS/.github/workflows/colcon.meta \ + --mixin coverage-gcc + done - name: Run tests run: | colcon test \ --packages-select fastrtps \ --event-handlers=console_direct+ \ - --ctest-args -R Statistics + --ctest-args -R RTPSStatisticsTests -R Statistics -E CreateParticipant - name: Generate coverage report run: | From d812e77d88a19069a5734a0969c0a78619754970 Mon Sep 17 00:00:00 2001 From: Miguel Barro Date: Wed, 28 Apr 2021 09:59:06 +0200 Subject: [PATCH 05/11] [10796] addressing reviwer's comments --- src/cpp/rtps/writer/ReaderProxy.cpp | 4 ++-- src/cpp/statistics/rtps/writer/StatisticsWriterImpl.cpp | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/cpp/rtps/writer/ReaderProxy.cpp b/src/cpp/rtps/writer/ReaderProxy.cpp index f1a9d854af6..8a55ac56409 100644 --- a/src/cpp/rtps/writer/ReaderProxy.cpp +++ b/src/cpp/rtps/writer/ReaderProxy.cpp @@ -401,7 +401,7 @@ bool ReaderProxy::process_initial_acknack() { if (is_local_reader()) { - return convert_status_on_all_changes(UNACKNOWLEDGED, UNSENT); + return 0 != convert_status_on_all_changes(UNACKNOWLEDGED, UNSENT); } return true; @@ -498,7 +498,7 @@ bool ReaderProxy::mark_fragment_as_sent_for_change( bool ReaderProxy::perform_nack_supression() { - return convert_status_on_all_changes(UNDERWAY, UNACKNOWLEDGED); + return 0 != convert_status_on_all_changes(UNDERWAY, UNACKNOWLEDGED); } uint32_t ReaderProxy::perform_acknack_response() diff --git a/src/cpp/statistics/rtps/writer/StatisticsWriterImpl.cpp b/src/cpp/statistics/rtps/writer/StatisticsWriterImpl.cpp index 2c7c81837db..080760573dc 100644 --- a/src/cpp/statistics/rtps/writer/StatisticsWriterImpl.cpp +++ b/src/cpp/statistics/rtps/writer/StatisticsWriterImpl.cpp @@ -132,6 +132,11 @@ void StatisticsWriterImpl::on_gap() void StatisticsWriterImpl::on_resent_data( uint32_t to_send) { + if ( 0 == to_send ) + { + return; + } + EntityCount notification; notification.guid(to_statistics_type(get_guid())); From f17809fe297f898e74692b9299028a42e7d69143 Mon Sep 17 00:00:00 2001 From: Miguel Barro Date: Wed, 28 Apr 2021 08:10:43 +0200 Subject: [PATCH 06/11] [10796] rebase flaws --- .github/workflows/statistics_coverage.yml | 2 +- test/unittest/statistics/rtps/RTPSStatisticsTests.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/statistics_coverage.yml b/.github/workflows/statistics_coverage.yml index 1a422e6865b..3ce8e962fb4 100644 --- a/.github/workflows/statistics_coverage.yml +++ b/.github/workflows/statistics_coverage.yml @@ -50,7 +50,7 @@ jobs: colcon test \ --packages-select fastrtps \ --event-handlers=console_direct+ \ - --ctest-args -R RTPSStatisticsTests -R Statistics -E CreateParticipant + --ctest-args -R RTPSStatisticsTests -R Statistics - name: Generate coverage report run: | diff --git a/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp b/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp index cd1d9bc3544..e5ffb8a3127 100644 --- a/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp +++ b/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp @@ -90,6 +90,7 @@ struct MockListener : IListener MOCK_METHOD1(on_heartbeat_count, void(const eprosima::fastdds::statistics::EntityCount&)); MOCK_METHOD1(on_acknack_count, void(const eprosima::fastdds::statistics::EntityCount&)); MOCK_METHOD1(on_data_count, void(const eprosima::fastdds::statistics::EntityCount&)); + MOCK_METHOD1(on_resent_count, void(const eprosima::fastdds::statistics::EntityCount&)); MOCK_METHOD1(on_gap_count, void(const eprosima::fastdds::statistics::EntityCount&)); MOCK_METHOD1(on_nackfrag_count, void(const eprosima::fastdds::statistics::EntityCount&)); MOCK_METHOD1(on_entity_discovery, void(const eprosima::fastdds::statistics::DiscoveryTime&)); From f70c8c99c34cc27df291ba962ae94fd5e4ebdbd1 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 28 Apr 2021 15:27:14 +0200 Subject: [PATCH 07/11] Fix workflow Signed-off-by: Miguel Company --- .github/workflows/statistics_coverage.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/statistics_coverage.yml b/.github/workflows/statistics_coverage.yml index 3ce8e962fb4..2adba6c65a1 100644 --- a/.github/workflows/statistics_coverage.yml +++ b/.github/workflows/statistics_coverage.yml @@ -41,7 +41,7 @@ jobs: --cmake-target $target \ --cmake-target-skip-unavailable \ --event-handlers=console_direct+ \ - --metas src/Fast-DDS/.github/workflows/colcon.meta \ + --metas src/Fast-DDS/.github/workflows/statistics_module.meta \ --mixin coverage-gcc done From c69826b5906cc004888a1e9c29f0ebc10601f643 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 28 Apr 2021 17:27:20 +0200 Subject: [PATCH 08/11] Add BlackboxTests_DDS_PIM to workflow Signed-off-by: Miguel Company --- .github/workflows/statistics_coverage.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/statistics_coverage.yml b/.github/workflows/statistics_coverage.yml index 2adba6c65a1..cdfb745a7db 100644 --- a/.github/workflows/statistics_coverage.yml +++ b/.github/workflows/statistics_coverage.yml @@ -34,7 +34,7 @@ jobs: --event-handlers=console_direct+ \ --metas src/Fast-DDS/.github/workflows/statistics_module.meta \ --mixin coverage-gcc - for target in RTPSStatisticsTests StatisticsDomainParticipantTests StatisticsQosTests StatisticsDomainParticipantMockTests + for target in RTPSStatisticsTests StatisticsDomainParticipantTests StatisticsQosTests StatisticsDomainParticipantMockTests BlackboxTests_DDS_PIM do colcon build \ --packages-select fastrtps \ From 8b912822cb3670b37f07f8fae224fc1b2b266e68 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 28 Apr 2021 19:06:35 +0200 Subject: [PATCH 09/11] Add DomainParticipantStatisticsListenerTests to workflow Signed-off-by: Miguel Company --- .github/workflows/statistics_coverage.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/statistics_coverage.yml b/.github/workflows/statistics_coverage.yml index cdfb745a7db..f916bd664b4 100644 --- a/.github/workflows/statistics_coverage.yml +++ b/.github/workflows/statistics_coverage.yml @@ -34,7 +34,7 @@ jobs: --event-handlers=console_direct+ \ --metas src/Fast-DDS/.github/workflows/statistics_module.meta \ --mixin coverage-gcc - for target in RTPSStatisticsTests StatisticsDomainParticipantTests StatisticsQosTests StatisticsDomainParticipantMockTests BlackboxTests_DDS_PIM + for target in RTPSStatisticsTests StatisticsDomainParticipantTests StatisticsQosTests DomainParticipantStatisticsListenerTests StatisticsDomainParticipantMockTests BlackboxTests_DDS_PIM do colcon build \ --packages-select fastrtps \ From 5f554e258ff99bba93d0c26726b445fa9b0703a6 Mon Sep 17 00:00:00 2001 From: Miguel Barro Date: Thu, 29 Apr 2021 16:31:02 +0200 Subject: [PATCH 10/11] [10796] solving a single line coverage fault in bombastic fashion --- .../statistics/rtps/RTPSStatisticsTests.cpp | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp b/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp index e5ffb8a3127..9079c622bfb 100644 --- a/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp +++ b/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp @@ -246,6 +246,28 @@ class RTPSStatisticsTestsImpl writer_ = RTPSDomain::createRTPSWriter(participant_, w_att, writer_history_); } + void create_lazy_writer( + uint32_t payloadMaxSize, + fastrtps::rtps::ReliabilityKind_t reliability_qos = fastrtps::rtps::ReliabilityKind_t::RELIABLE, + fastrtps::rtps::DurabilityKind_t durability_qos = fastrtps::rtps::DurabilityKind_t::TRANSIENT_LOCAL) + { + using namespace fastrtps::rtps; + + HistoryAttributes history_attributes; + history_attributes.payloadMaxSize = payloadMaxSize; + writer_history_ = new WriterHistory(history_attributes); + + WriterAttributes w_att; + w_att.times.heartbeatPeriod.seconds = 3; + w_att.times.heartbeatPeriod.nanosec = 0; + w_att.times.nackResponseDelay.seconds = 0; + w_att.times.nackResponseDelay.nanosec = 300 * 1000 * 1000; // increase ACKNACK response delay + w_att.endpoint.reliabilityKind = reliability_qos; + w_att.endpoint.durabilityKind = durability_qos; + + writer_ = RTPSDomain::createRTPSWriter(participant_, w_att, writer_history_); + } + void create_endpoints( uint32_t payloadMaxSize, fastrtps::rtps::ReliabilityKind_t reliability_qos = fastrtps::rtps::ReliabilityKind_t::RELIABLE) @@ -803,6 +825,112 @@ TEST_F(RTPSStatisticsTests, statistics_rpts_listener_discovery_callbacks) EXPECT_TRUE(participant_->remove_statistics_listener(participant_listener, EventKind::DISCOVERED_ENTITY)); } +/* + * This test checks improves coverity by asserting that every RESENT_DATAS callback is associated with a message + */ +TEST_F(RTPSStatisticsTests, statistics_rpts_avoid_empty_resent_callbacks) +{ + using namespace ::testing; + using namespace fastrtps; + using namespace fastrtps::rtps; + using namespace std; + + // The history must be cleared after the acknack is received + // but before the response is processed + atomic_bool acknack_sent(false); + + // filter out all user DATAs + set_transport_filter( + DATA, + [](fastrtps::rtps::CDRMessage_t& msg)-> bool + { + uint32_t old_pos = msg.pos; + + // see RTPS DDS 9.4.5.3 Data Submessage + EntityId_t readerID, writerID; + SequenceNumber_t sn; + + msg.pos += 2; // flags + msg.pos += 2; // octets to inline quos + CDRMessage::readEntityId(&msg, &readerID); + CDRMessage::readEntityId(&msg, &writerID); + CDRMessage::readSequenceNumber(&msg, &sn); + + // restore buffer pos + msg.pos = old_pos; + + // filter user traffic + return ((writerID.value[3] & 0xC0) == 0); + }); + + set_transport_filter( + ACKNACK, + [&acknack_sent](fastrtps::rtps::CDRMessage_t& msg)-> bool + { + uint32_t old_pos = msg.pos; + + // see RTPS DDS 9.4.5.2 Acknack Submessage + EntityId_t readerID, writerID; + SequenceNumberSet_t snSet; + + CDRMessage::readEntityId(&msg, &readerID); + CDRMessage::readEntityId(&msg, &writerID); + snSet = CDRMessage::readSequenceNumberSet(&msg); + + // restore buffer pos + msg.pos = old_pos; + + // The acknack has been sent + acknack_sent = !snSet.empty(); + + // we are not filtering + return false; + }); + + // create the listeners and set expectations + auto writer_listener = make_shared(); + + // check callbacks on data exchange + EXPECT_CALL(*writer_listener, on_heartbeat_count) + .Times(AtLeast(1)); + EXPECT_CALL(*writer_listener, on_data_count) + .Times(AtLeast(1)); + EXPECT_CALL(*writer_listener, on_resent_count) + .Times(0); // never called + + // create the writer, reader is a late joiner + uint16_t length = 255; + create_lazy_writer(length, RELIABLE, TRANSIENT_LOCAL); + + // writer specific callbacks + ASSERT_TRUE(writer_->add_statistics_listener(writer_listener)); + + // create the late joiner as VOLATILE + create_reader(length, RELIABLE, VOLATILE); + + // match writer and reader on a dummy topic + match_endpoints(false, "string", "statisticsSmallTopic"); + + // add a sample to the writer history that cannot be delivered + // due to the filter + write_small_sample(length); + + // wait for the acknack to be sent before clearing the history + while(!acknack_sent) + { + this_thread::sleep_for(chrono::milliseconds(100)); + } + + // remove the sample from the writer + ASSERT_TRUE(writer_history_->remove_change(SequenceNumber_t{ 0, 1 })); + + // give room to process the ACKNACK + this_thread::sleep_for(chrono::milliseconds(400)); + + // release the listeners + EXPECT_TRUE(writer_->remove_statistics_listener(writer_listener)); +} + } // namespace rtps } // namespace statistics } // namespace fastdds From 5a6cc7c4cdb9686c73563cfb27cc22dd0e4cd83e Mon Sep 17 00:00:00 2001 From: Miguel Barro Date: Fri, 30 Apr 2021 10:47:18 +0200 Subject: [PATCH 11/11] [10796] linter --- test/unittest/statistics/rtps/RTPSStatisticsTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp b/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp index 9079c622bfb..5fd64c90872 100644 --- a/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp +++ b/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp @@ -916,7 +916,7 @@ TEST_F(RTPSStatisticsTests, statistics_rpts_avoid_empty_resent_callbacks) write_small_sample(length); // wait for the acknack to be sent before clearing the history - while(!acknack_sent) + while (!acknack_sent) { this_thread::sleep_for(chrono::milliseconds(100)); }