[16737] On unacknowledged sample removed implementation by JLBuenoLopez · Pull Request #3240 · eProsima/Fast-DDS

JLBuenoLopez · 2023-01-23T16:06:44Z

Description

This PR implements on_unacknowledged_sample_removed callback.
This PR depends on:

Documentation PR: eProsima/Fast-DDS-docs#439

Contributor Checklist

Commit messages follow the project guidelines.
The code follows the style guidelines of this project.
Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added.
Any new/modified methods have been properly documented using Doxygen.
Fast DDS test suite has been run locally.
NO Changes are ABI compatible.
Changes are API compatible.
N/A Documentation builds and tests pass locally.
New feature has been added to the versions.md file (if applicable).
New feature has been documented/Current behavior is correctly described in the documentation.

N/A Applicable backports have been included in the description.

Reviewer Checklist

Check contributor checklist is correct.
Check CI results: changes do not issue any warning.
Check CI results: failing tests are unrelated with the changes.

JLBuenoLopez · 2023-01-24T13:33:10Z

@richiprosima please test this

JLBuenoLopez · 2023-01-30T15:22:35Z

@richiprosima please test this

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

…r as parameter Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

…orrect one Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

JLBuenoLopez · 2023-02-01T06:09:15Z

@richiprosima please test this

MiguelCompany · 2023-02-01T08:29:45Z

@richiprosima Please test mac

MiguelCompany · 2023-02-01T09:09:48Z

@JLBuenoLopez-eProsima Test BlackboxTests_RTPS.RTPS.reliable_has_been_fully_delivered is failing on Windows. Could you take a look?

include/fastdds/rtps/writer/ReaderProxy.h

include/fastdds/rtps/writer/StatefulWriter.h

include/fastdds/rtps/writer/StatelessWriter.h

src/cpp/fastdds/publisher/DataWriterHistory.cpp

src/cpp/fastdds/publisher/DataWriterHistory.hpp

src/cpp/fastdds/publisher/DataWriterHistory.cpp

src/cpp/fastdds/publisher/DataWriterImpl.cpp

src/cpp/rtps/writer/ReaderProxy.cpp

src/cpp/rtps/writer/StatefulWriter.cpp

test/mock/rtps/PublisherHistory/fastdds/publisher/DataWriterHistory.hpp

…for ack Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

…RTPSWriter Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

…_acks ABI break Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

…n_delivered Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

JLBuenoLopez · 2023-02-02T15:39:24Z

@richiprosima please test this

JLBuenoLopez · 2023-02-02T15:40:15Z

@MiguelCompany I have applied the suggestions from the review with the exception to the ReaderProxy::has_been_delivered tests.

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

JLBuenoLopez · 2023-02-02T15:44:22Z

@richiprosima please test this

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

MiguelCompany

Two minor things aside from what we have talked offline.

src/cpp/fastdds/publisher/DataWriterImpl.cpp

src/cpp/fastdds/publisher/DataWriterHistory.cpp

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

JLBuenoLopez · 2023-02-03T09:35:09Z

@richiprosima please test this

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

MiguelCompany · 2023-02-03T09:40:13Z

@richiprosima Please test this again

test/unittest/rtps/writer/ReaderProxyTests.cpp

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

JLBuenoLopez · 2023-02-03T10:31:26Z

@richiprosima please test this

MiguelCompany · 2023-02-03T10:33:59Z

@richiprosima Please test mac

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

JLBuenoLopez · 2023-02-03T12:32:40Z

@richiprosima please test this

…istener (#3275) * Add DataWriterListener on_unacknowledged_sample_removed callback (#3205) * Refs #16736: public API Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: add feature to versions.md Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: apply review suggestion Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Public RTPS API: RTPSWriter::has_been_delivered() (#3211) * Refs #16737: add new virtual method RTPSWriter::has_been_delivered Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: update versions.md Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: fix style Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Update RTPS API method name (#3230) Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * RTPSWriter::has_been_fully_delivered tests (#3233) * Refs #16736: extend RTPSWithRegistrationWriter to register flow controller Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: reorder headers, use fastdds headers instead of fastrtps ones Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: add reader data filter API to RTPSWithRegistrationWriter Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: send_sample method Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: expose has_been_fully_delivered API Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: add RTPSWriter::has_been_fully_delivered tests Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: linters Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: modify RTPS API Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: fix RTPSWriter::has_been_fully_delivered tests Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: fix RTPSWithRegistrationWriter has_been_delivered API Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: fix test Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: fix waitForAllAcked timeout conversion Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: fix has_been_delivered tests Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> --------- Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * On unacknowledged sample removed tests (#3210) * Refs #16736: update namespaces (when possible) Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: add API to PubSubWriter Listener Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: save notified instance collection Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: add feature test suite Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: fix windows warnings Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: add listener tests according to changes introduced in the test plan during review process Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: fix tests with suggestions from test plan review Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: fix listener tests: ack race condition Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: avoid waiting and instead wait for ACK in listener tests Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16736: apply review suggestion Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> --------- Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * On unacknowledged sample removed implementation (#3240) * Refs #16737: ReaderProxy::has_been_delivered Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: add StatelessWriter::is_acked_by_all with sequence number as parameter Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: StatelessWriter::has_been_fully_delivered implementation Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: StatefulWriter::has_been_fully_delivered implementation Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: improve internal documentation Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: linters Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: add unacknowledged sample removed functor Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: fix DataWriterHistory mock Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: add auxiliary method change_is_acked_or_fully_delivered Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: fix StatefulWriter mock Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: notify if a change is removed unacknowledged Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: linters Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: safety check, ensure listener_ is not nullptr Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: add test to ensure that the notified DataWriter is the correct one Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: improve RTPSWithRegistrationWriter API to wait infinite for ack Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: improve reliable_has_been_fully_delivered test Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: apply review suggestion: fix doxygen documentation Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: inject functor in DataWriterHistory constructor Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: refactor get_disable_positive_acks as virtual method of RTPSWriter Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: update versions.md with RTPSWriter::get_disable_positive_acks ABI break Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: apply review suggestions. Fix RTPSWriter mock Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: apply review suggestions concerning ReaderProxy::has_been_delivered Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: fix build Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: linters Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: fix Clang warning Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: increase timeout to ensure ACK reception Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: add ReaderProxy::has_been_delivered test Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: apply review suggestions Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: apply review suggestion to ReaderProxyTests Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #16737: fix Clang warning Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> --------- Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #15565: add regression test: Best Effort Writer with no matched Readers and change in the future Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #15565: apply review suggestion, fix to regression Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Refs #15565: apply review suggestions Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Fix parameter in doxygen documentation. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> --------- Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> Co-authored-by: Miguel Company <miguelcompany@eprosima.com>

JLBuenoLopez changed the base branch from master to feature/on-unacknowledged-sample-removed January 23, 2023 16:07

JLBuenoLopez changed the title ~~[16737Feature/on unack sample removed dds listener impl~~ [16737] On unacknowledged sample removed RTPS/DDS linkage Jan 23, 2023

JLBuenoLopez force-pushed the feature/on-unacknowledged-sample-removed branch from 4dde7d3 to 9bdc93c Compare January 24, 2023 13:17

JLBuenoLopez force-pushed the feature/on-unack-sample-removed-DDSListener-impl branch from c41c1b8 to a3eedc0 Compare January 24, 2023 13:32

JLBuenoLopez force-pushed the feature/on-unack-sample-removed-DDSListener-impl branch from c434742 to 1d76d82 Compare January 30, 2023 15:20

JLBuenoLopez added 14 commits February 1, 2023 07:01

Refs #16737: ReaderProxy::has_been_delivered

5b0461c

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: add StatelessWriter::is_acked_by_all with sequence numbe…

79b639c

…r as parameter Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: StatelessWriter::has_been_fully_delivered implementation

df48838

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: StatefulWriter::has_been_fully_delivered implementation

be4346a

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: improve internal documentation

6ad37d5

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: linters

d4015c0

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: add unacknowledged sample removed functor

221eb7e

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: fix DataWriterHistory mock

557ce06

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: add auxiliary method change_is_acked_or_fully_delivered

d93b0a8

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: fix StatefulWriter mock

5157818

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: notify if a change is removed unacknowledged

b0c3ef1

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: linters

b13b64b

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: safety check, ensure listener_ is not nullptr

b6c5079

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: add test to ensure that the notified DataWriter is the c…

c9b32cb

…orrect one Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

JLBuenoLopez force-pushed the feature/on-unack-sample-removed-DDSListener-impl branch from 1d76d82 to c9b32cb Compare February 1, 2023 06:08

JLBuenoLopez mentioned this pull request Feb 1, 2023

[16737] RTPSWriter::has_been_fully_delivered implementation #3237

Closed

9 tasks

MiguelCompany requested changes Feb 1, 2023

View reviewed changes

JLBuenoLopez added 3 commits February 2, 2023 14:53

Refs #16737: improve RTPSWithRegistrationWriter API to wait infinite …

23eba0b

…for ack Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: improve reliable_has_been_fully_delivered test

25ab7c5

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: apply review suggestion: fix doxygen documentation

b71753b

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

JLBuenoLopez added 6 commits February 2, 2023 15:25

Refs #16737: inject functor in DataWriterHistory constructor

c8cde8c

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: refactor get_disable_positive_acks as virtual method of …

95f8b55

…RTPSWriter Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: update versions.md with RTPSWriter::get_disable_positive…

36b50a2

…_acks ABI break Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: apply review suggestions. Fix RTPSWriter mock

e89d49f

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: apply review suggestions concerning ReaderProxy::has_bee…

b60cc43

…n_delivered Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: fix build

e9cc00b

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: linters

4889fe1

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

JLBuenoLopez added 2 commits February 3, 2023 08:37

Refs #16737: fix Clang warning

0fe9a4a

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Refs #16737: increase timeout to ensure ACK reception

251c575

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

MiguelCompany requested changes Feb 3, 2023

View reviewed changes

src/cpp/fastdds/publisher/DataWriterImpl.cpp Outdated Show resolved Hide resolved

src/cpp/fastdds/publisher/DataWriterHistory.cpp Outdated Show resolved Hide resolved

Refs #16737: add ReaderProxy::has_been_delivered test

b546076

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

MiguelCompany changed the title ~~[16737] On unacknowledged sample removed RTPS/DDS linkage~~ [16737] On unacknowledged sample removed implementation Feb 3, 2023

Refs #16737: apply review suggestions

66f1066

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

MiguelCompany requested changes Feb 3, 2023

View reviewed changes

test/unittest/rtps/writer/ReaderProxyTests.cpp Outdated Show resolved Hide resolved

Refs #16737: apply review suggestion to ReaderProxyTests

4efe46e

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

MiguelCompany approved these changes Feb 3, 2023

View reviewed changes

MiguelCompany added the ci-pending PR which CI is running label Feb 3, 2023

Refs #16737: fix Clang warning

b42674c

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

MiguelCompany approved these changes Feb 6, 2023

View reviewed changes

MiguelCompany merged commit 281f3f3 into feature/on-unacknowledged-sample-removed Feb 6, 2023

MiguelCompany deleted the feature/on-unack-sample-removed-DDSListener-impl branch February 6, 2023 06:21

Conversation

JLBuenoLopez commented Jan 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Contributor Checklist

Reviewer Checklist

Uh oh!

JLBuenoLopez commented Jan 24, 2023

Uh oh!

JLBuenoLopez commented Jan 30, 2023

Uh oh!

JLBuenoLopez commented Feb 1, 2023

Uh oh!

MiguelCompany commented Feb 1, 2023

Uh oh!

MiguelCompany commented Feb 1, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JLBuenoLopez commented Feb 2, 2023

Uh oh!

JLBuenoLopez commented Feb 2, 2023

Uh oh!

JLBuenoLopez commented Feb 2, 2023

Uh oh!

MiguelCompany left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JLBuenoLopez commented Feb 3, 2023

Uh oh!

MiguelCompany commented Feb 3, 2023

Uh oh!

Uh oh!

JLBuenoLopez commented Feb 3, 2023

Uh oh!

MiguelCompany commented Feb 3, 2023

Uh oh!

JLBuenoLopez commented Feb 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JLBuenoLopez commented Jan 23, 2023 •

edited

Loading