Skip to content
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

[20729] Allow processing of AckNack submessages with count == 0 #4639

Merged
merged 4 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
9 changes: 5 additions & 4 deletions include/fastdds/rtps/writer/ReaderProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,10 @@ class ReaderProxy
bool check_and_set_acknack_count(
uint32_t acknack_count)
{
if (last_acknack_count_ < acknack_count)
if (acknack_count >= next_expected_acknack_count_)
EduPonz marked this conversation as resolved.
Show resolved Hide resolved
{
last_acknack_count_ = acknack_count;
next_expected_acknack_count_ = acknack_count;
++next_expected_acknack_count_;
return true;
}

Expand Down Expand Up @@ -442,8 +443,8 @@ class ReaderProxy
TimedEvent* initial_heartbeat_event_;
//! Are timed events enabled?
std::atomic_bool timers_enabled_;
//! Last ack/nack count
uint32_t last_acknack_count_;
//! Next expected ack/nack count
uint32_t next_expected_acknack_count_;
//! Last NACKFRAG count.
uint32_t last_nackfrag_count_;

Expand Down
4 changes: 2 additions & 2 deletions src/cpp/rtps/writer/ReaderProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ ReaderProxy::ReaderProxy(
, nack_supression_event_(nullptr)
, initial_heartbeat_event_(nullptr)
, timers_enabled_(false)
, last_acknack_count_(0)
, next_expected_acknack_count_(0)
, last_nackfrag_count_(0)
{
nack_supression_event_ = new TimedEvent(writer_->getRTPSParticipant()->getEventResource(),
Expand Down Expand Up @@ -166,7 +166,7 @@ void ReaderProxy::stop()
disable_timers();

changes_for_reader_.clear();
last_acknack_count_ = 0;
next_expected_acknack_count_ = 0;
last_nackfrag_count_ = 0;
changes_low_mark_ = SequenceNumber_t();
}
Expand Down
32 changes: 32 additions & 0 deletions test/unittest/rtps/writer/ReaderProxyTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,38 @@ TEST(ReaderProxyTests, has_been_delivered_test)
expect_result({0, 3}, false, false);
}

// Test expectations regarding acknack count.
// Serves as a regression test for redmine issue #20729.
TEST(ReaderProxyTests, acknack_count)
{
StatefulWriter writer_mock;
WriterTimes w_times;
RemoteLocatorsAllocationAttributes alloc;
ReaderProxy rproxy(w_times, alloc, &writer_mock);

ReaderProxyData reader_attributes(0, 0);
reader_attributes.m_qos.m_reliability.kind = RELIABLE_RELIABILITY_QOS;
rproxy.start(reader_attributes);

// Check that the initial acknack count is 0.
EXPECT_TRUE(rproxy.check_and_set_acknack_count(0u));
// Check that it is not accepted twice.
EXPECT_FALSE(rproxy.check_and_set_acknack_count(0u));
// Check that it is accepted if it is incremented.
EXPECT_TRUE(rproxy.check_and_set_acknack_count(1u));
// Check that it is not accepted twice.
EXPECT_FALSE(rproxy.check_and_set_acknack_count(1u));
// Check that it is not accepted if it is decremented.
EXPECT_FALSE(rproxy.check_and_set_acknack_count(0u));
// Check that it is accepted if it has a big increment.
EXPECT_TRUE(rproxy.check_and_set_acknack_count(100u));
// Check that previous values are rejected.
for (uint32_t i = 0; i <= 100u; ++i)
{
EXPECT_FALSE(rproxy.check_and_set_acknack_count(i));
}
}

} // namespace rtps
} // namespace fastrtps
} // namespace eprosima
Expand Down
Loading