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.

Sign up for GitHub

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

Jump to bottom

[21120] RTPS reader APIs refactor #4875

Merged
merged 79 commits into from
Jun 11, 2024
Merged

[21120] RTPS reader APIs refactor #4875

merged 79 commits into from
Jun 11, 2024

Conversation

MiguelCompany
Copy link
Member

@MiguelCompany MiguelCompany commented May 30, 2024 •
edited by EduPonz
Loading

Description

This is a refactor on the RTPS reader APIs, so only the ones intended for the user (i.e. public and exported) are present.
The refactor includes adding an intermediate BaseReader class with the internal APIs.

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • N/A Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
  • Any new/modified methods have been properly documented using Doxygen.
  • N/A Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
  • NO Changes are backport compatible: they do NOT break ABI nor change library core behavior.
  • NO Changes are API compatible.
  • NO 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

  • The PR has a milestone assigned.
  • The title and description correctly express the PR's purpose.
  • Check contributor checklist is correct.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@MiguelCompany MiguelCompany added this to the v3.0.0 milestone May 30, 2024
@elianalf elianalf added the needs-review PR that is ready to be reviewed label May 30, 2024
@MiguelCompany MiguelCompany force-pushed the feature/21082/reader branch 2 times, most recently from 910f9a5 to 4169b0c Compare May 31, 2024 08:11
@EduPonz EduPonz self-requested a review May 31, 2024 10:51
@github-actions github-actions bot added the ci-pending PR which CI is running label May 31, 2024
@MiguelCompany MiguelCompany requested review from EduPonz and removed request for EduPonz May 31, 2024 12:22
@MiguelCompany MiguelCompany requested review from EduPonz and removed request for EduPonz June 3, 2024 06:01
@MiguelCompany MiguelCompany mentioned this pull request Jun 3, 2024
Merged
7 tasks
EduPonz
EduPonz suggested changes Jun 3, 2024
View reviewed changes
src/cpp/rtps/messages/MessageReceiver.h Outdated Show resolved Hide resolved
src/cpp/rtps/messages/MessageReceiver.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/reader/BaseReader.hpp Outdated Show resolved Hide resolved
include/fastdds/rtps/reader/RTPSReader.h Outdated Show resolved Hide resolved
include/fastdds/rtps/reader/RTPSReader.h Show resolved Hide resolved
src/cpp/rtps/reader/BaseReader.hpp Outdated Show resolved Hide resolved
src/cpp/rtps/reader/BaseReader.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/reader/StatefulReader.cpp Show resolved Hide resolved
test/mock/rtps/RTPSReader/fastdds/rtps/reader/RTPSReader.h Outdated Show resolved Hide resolved
include/fastdds/rtps/reader/ReaderListener.h Show resolved Hide resolved
@MiguelCompany MiguelCompany force-pushed the feature/21082/reader branch 2 times, most recently from fa1b5e3 to 0f7f9b0 Compare June 5, 2024 13:35
@MiguelCompany MiguelCompany requested review from EduPonz and removed request for EduPonz June 5, 2024 13:55
@MiguelCompany MiguelCompany requested review from EduPonz and removed request for EduPonz June 6, 2024 10:50
@MiguelCompany
Copy link
Member Author

@richiprosima Please test_3 discovery-server

EduPonz
EduPonz suggested changes Jun 10, 2024
View reviewed changes
Copy link

@EduPonz EduPonz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only doxygen docs things.

src/cpp/rtps/reader/StatefulReader.hpp Outdated Show resolved Hide resolved
src/cpp/rtps/reader/BaseReader.cpp Outdated Show resolved Hide resolved
test/unittest/rtps/reader/StatefulReaderTests.cpp Outdated Show resolved Hide resolved
include/fastdds/rtps/reader/RTPSReader.h Show resolved Hide resolved
src/cpp/rtps/reader/BaseReader.hpp Show resolved Hide resolved
test/mock/rtps/RTPSReader/rtps/reader/BaseReader.hpp Outdated Show resolved Hide resolved
@MiguelCompany
Refs #21120. Rename to on_new_cache_change_added.
0998f21 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Uncrustify.
fb0d898 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Refactor of mocks.
d4a0b63 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Add StatefulReader unit test.
77e692d 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Remove RTPS blackbox test.
30c1d02 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Uncrustify.
adeb3cd 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Fix discovery server tests.
473ab82 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Improve test sample_lost_be_dw_be_dr_fragments.
4b17c6f 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Improve find_cache_in_fragmented_process.
bbd4537 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Doxygen motivated refactor.
33967bc 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Refactor reserve_cache.
8125001 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Refactor allow_unknown_writers.
0a6ad26 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Refactor change_removed_by_history.
9aa0e12 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Improve update_liveliness_changed_status.
72072d2 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. More doxygen improvements.
2728ef9 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Remove accept_messages_to_unknown_readers.
9878d9a 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. More doxygen improvements.
a3aeec0 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Snake case for ReaderTimes.
abad6a1 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany
Refs #21120. Update versions.md.
6e2d89f 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@EduPonz EduPonz requested review from EduPonz and removed request for EduPonz June 11, 2024 08:45
@EduPonz
Copy link

EduPonz commented Jun 11, 2024

@richiprosima please test_3 discovery-server

@MiguelCompany MiguelCompany mentioned this pull request Jun 11, 2024
Merged
11 tasks
@EduPonz EduPonz merged commit 8308073 into master Jun 11, 2024
9 of 12 checks passed
@EduPonz EduPonz deleted the feature/21082/reader branch June 11, 2024 13:31
@EduPonz EduPonz removed the ci-pending PR which CI is running label Jun 11, 2024
@JesusPoderoso JesusPoderoso mentioned this pull request Jun 11, 2024
Merged
7 tasks
@EduPonz EduPonz mentioned this pull request Jun 12, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EduPonz EduPonz EduPonz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@MiguelCompany @EduPonz @elianalf