Refs #22658: Included reviewer suggestions

Signed-off-by: Juanjo Garcia <[email protected]>
eProsima · Jan 29, 2025 · 5a968ab · 5a968ab
1 parent 92c93c3
commit 5a968ab
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 4 deletions.
diff --git a/src/cpp/rtps/writer/StatefulWriter.cpp b/src/cpp/rtps/writer/StatefulWriter.cpp
@@ -1521,6 +1521,8 @@ void StatefulWriter::check_acked_status()
 
         if (min_low_mark >= get_seq_num_min())
         {
+            // get_seq_num_min() returns SequenceNumber_t::unknown() when the history is empty.
+            // Thus, it is set to 2 to indicate that all samples have been removed.
             may_remove_change_ = (get_seq_num_min() == SequenceNumber_t::unknown()) ? 2 : 1;
         }
 

diff --git a/test/blackbox/api/dds-pim/PubSubWriter.hpp b/test/blackbox/api/dds-pim/PubSubWriter.hpp
@@ -894,10 +894,10 @@ class PubSubWriter
 
     PubSubWriter& reliability(
             const eprosima::fastdds::dds::ReliabilityQosPolicyKind kind,
-            int max_blocking_time)
+            eprosima::fastdds::dds::Duration_t max_blocking_time)
     {
         datawriter_qos_.reliability().kind = kind;
-        datawriter_qos_.reliability().max_blocking_time.seconds = max_blocking_time;
+        datawriter_qos_.reliability().max_blocking_time = max_blocking_time;
         return *this;
     }
 

diff --git a/test/blackbox/common/DDSBlackboxTestsListeners.cpp b/test/blackbox/common/DDSBlackboxTestsListeners.cpp
@@ -3437,7 +3437,7 @@ TEST(DDSStatus, keyed_reliable_positive_acks_disabled_on_unack_sample_removed)
 }
 
 /*!
- * Regression Test for 22658: when he entire history is acked in volatile, given that the entries are deleted from the
+ * Regression Test for 22658: when the entire history is acked in volatile, given that the entries are deleted from the
  * history, check_acked_status satisfies min_low_mark >= get_seq_num_min() because seq_num_min is unknown. This makes
  * try_remove to fail, because it tries to remove changes but there were none. This causes prepare_change to not
  * perform the changes, since the history was full and could not delete any changes.
@@ -3448,7 +3448,7 @@ TEST(DDSStatus, entire_history_acked_volatile_unknown_pointer)
     PubSubWriter<HelloWorldPubSubType> writer(TEST_TOPIC_NAME);
     PubSubReader<HelloWorldPubSubType> reader(TEST_TOPIC_NAME);
 
-    writer.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS, 200)
+    writer.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS, eprosima::fastdds::dds::Duration_t (200,0))
             .durability_kind(eprosima::fastdds::dds::VOLATILE_DURABILITY_QOS)
             .history_kind(eprosima::fastdds::dds::KEEP_ALL_HISTORY_QOS)
             .resource_limits_max_instances(1)
@@ -3469,6 +3469,9 @@ TEST(DDSStatus, entire_history_acked_volatile_unknown_pointer)
     auto data = default_helloworld_data_generator(2);
     for (auto sample : data)
     {
+        // A value of true means that the sample was sent successfully.
+        // This aligns with the expected behaviour of having the history
+        // acknowledged and emptied before the next message.
         EXPECT_TRUE(writer.send_sample(sample));
     }
 }