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

[21295] Participant discovery structures refactor #5042

Merged
merged 23 commits into from
Jul 24, 2024
Merged

[21295] Participant discovery structures refactor #5042

merged 23 commits into from
Jul 24, 2024

Conversation

elianalf
Copy link
Contributor

@elianalf elianalf commented Jul 9, 2024 •
edited by MiguelCompany
Loading

Description

This PR is a refactor of Participant discovery structures. In particular:

  • Change the name of the enum DISCOVERY_STATUS → ParticipantDiscoveryStatus.
  • Change the name of the callback: onParticipantDiscovery -> on_participant_discovery.
  • Change the signature for both RTPSParticipantListener and DomainParticipantListener. The methods on_participant_discover() now take ParticipantBuiltinTopicData and a ParticipantDiscoveryStatus, instead of ParticipantDiscoveryInfo.
  • Remove ParticipantDiscoveryInfo
  • Extend ParticipantBuiltinTopicData with additional fields: locators, participant name, properties, lease duration
  • Create an internal method to create ParticipantBuiltinTopicData from a ParticipantProxyData.
  • Make ParticipantProxyData header private.
  • Change signature of fill_discovery_data_from_cdr_message() to take ParticipantBuiltinTopicData

Related PR:

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; 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)
  • ❌ Changes are backport compatible: they do NOT break ABI nor change library core behavior.
  • ❌ Changes are API compatible.
  • 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.
  • N/A If this is a critical bug fix, backports to the critical-only supported branches have been requested.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@elianalf elianalf added this to the v3.0.0 milestone Jul 9, 2024
@elianalf elianalf force-pushed the participant_discovery_data_refactor branch from bbc8c80 to 70d0d45 Compare July 9, 2024 12:29
@MiguelCompany MiguelCompany force-pushed the feature/21239 branch 2 times, most recently from c46e765 to 1d202cb Compare July 10, 2024 07:43
@elianalf elianalf added the needs-review PR that is ready to be reviewed label Jul 11, 2024
@elianalf elianalf force-pushed the participant_discovery_data_refactor branch from 70d0d45 to 02dc073 Compare July 12, 2024 08:12
@MiguelCompany MiguelCompany self-requested a review July 12, 2024 08:42
@github-actions github-actions bot added ci-pending PR which CI is running labels Jul 12, 2024
@elianalf elianalf force-pushed the participant_discovery_data_refactor branch 2 times, most recently from 5aa7fa2 to a5a6ec0 Compare July 16, 2024 10:48
This was referenced Jul 16, 2024
Merged
Merged
Merged
@elianalf elianalf requested review from MiguelCompany and removed request for MiguelCompany July 16, 2024 14:37
@elianalf elianalf changed the base branch from feature/21239 to feature/21301 July 16, 2024 14:48
Base automatically changed from feature/21301 to master July 17, 2024 05:58
@elianalf elianalf force-pushed the participant_discovery_data_refactor branch from 94baedc to b6e5229 Compare July 17, 2024 12:41
MiguelCompany
MiguelCompany requested changes Jul 17, 2024
View reviewed changes
include/fastdds/dds/builtin/topic/ParticipantBuiltinTopicData.hpp Outdated Show resolved Hide resolved
include/fastdds/dds/domain/DomainParticipantListener.hpp Outdated Show resolved Hide resolved
include/fastdds/rtps/builtin/data/ParticipantBuiltinTopicData.hpp Outdated Show resolved Hide resolved
include/fastdds/rtps/builtin/data/ParticipantBuiltinTopicData.hpp Outdated Show resolved Hide resolved
include/fastdds/rtps/builtin/data/ParticipantBuiltinTopicData.hpp Outdated Show resolved Hide resolved
test/mock/rtps/ParticipantProxyData/rtps/builtin/data/ParticipantProxyData.hpp Outdated Show resolved Hide resolved
test/mock/rtps/RTPSParticipant/fastdds/rtps/participant/RTPSParticipant.hpp Outdated Show resolved Hide resolved
test/mock/rtps/SecurityPluginFactory/rtps/security/MockAccessControlPlugin.h Outdated Show resolved Hide resolved
test/mock/rtps/external_locators/rtps/network/utils/external_locators.hpp Outdated Show resolved Hide resolved
versions.md Outdated Show resolved Hide resolved
@elianalf elianalf force-pushed the participant_discovery_data_refactor branch from d27effc to 5e999f7 Compare July 18, 2024 18:11
@MiguelCompany MiguelCompany force-pushed the participant_discovery_data_refactor branch from 5e999f7 to 9e7c525 Compare July 19, 2024 09:09
@MiguelCompany MiguelCompany requested review from MiguelCompany and removed request for MiguelCompany July 19, 2024 09:11
@MiguelCompany MiguelCompany removed the needs-review PR that is ready to be reviewed label Jul 19, 2024
MiguelCompany
MiguelCompany previously approved these changes Jul 19, 2024
View reviewed changes
Copy link
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

LGTM with green CI

elianalf and others added 23 commits July 23, 2024 15:39
@elianalf
Refs #21295: changing the name of the callback from onParticipantDisc…
2845bb5 
…overy to on_participant_discovery

Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Change listener signature and DISCOVERY_STATUS to PARTIC…
2abf514 
…IPANT_DISCOVERY_STATUS

Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Move PARTICIPANT_DISCOVERY_STATUS out of ParticipantDisc…
250e4de 
…overyInfo

Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Substitute ParticipantDiscoveryInfo with ParticipantProx…
ecd6d07 
…yData

Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Remove ParticiapntDiscoveryInfo
92af3ae 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Extend ParticipantBuiltinTopicData
835e9fe 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Create ParticipantBuiltinTopicData in rtps namespace
8c94e64 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Create from_proxy_to_builtin method for participant
250fd1f 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Substitute ParticipantProxyData with ParticipantBuiltinT…
3809fcc 
…opicData

Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Make ParticipantProxyData private
2e58bd7 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Update version.md
cb24187 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Uncrustify
2f68fe4 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Fix compilation error after rebase
3d0918e 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Change name in camel case ParticipantDiscoveryStatus
4047e7f 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Change signature of fill_discovery_data_from_cdr_message
a771a46 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Apply changes to new example
29a9a64 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Apply suggestions
270a979 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Apply suggestions
ca29ad0 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@MiguelCompany @elianalf
Refs #21295. Fix communication test build.
4355e4e 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany @elianalf
Refs #21295. Fix include order.
b96554b 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@MiguelCompany @elianalf
Refs #21295. Additional change on versions.md.
890eca0 
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@elianalf
Refs #21295: Add missing guid conversion in the converter
2da7756 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
Refs #21295: Adjust after rebase
2c4d7f5 
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf elianalf force-pushed the participant_discovery_data_refactor branch from e1958d6 to 2c4d7f5 Compare July 23, 2024 14:01
MiguelCompany
MiguelCompany approved these changes Jul 23, 2024
View reviewed changes
Copy link
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

LGTM with green CI

@MiguelCompany MiguelCompany merged commit aa80b72 into master Jul 24, 2024
16 of 17 checks passed
@MiguelCompany MiguelCompany deleted the participant_discovery_data_refactor branch July 24, 2024 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MiguelCompany MiguelCompany MiguelCompany approved these changes

Assignees
No one assigned
Labels
ci-pending PR which CI is running first-to-merge
Projects
None yet
Milestone
v3.0.0
Development

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