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

[20307] Ignore custom PIDs defined in Fast DDS when they are received from other vendors #4299

Merged
merged 13 commits into from
Feb 26, 2024
Merged

[20307] Ignore custom PIDs defined in Fast DDS when they are received from other vendors #4299

merged 13 commits into from
Feb 26, 2024

Conversation

EduPonz
Copy link

@EduPonz EduPonz commented Jan 26, 2024 •
edited
Loading

Description

With this PR, Fast DDS ignores all custom PIDs defined in Fast DDS when they are received from other vendors.

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.
  • Changes are ABI compatible: This is OK as this PR is in preparation for v2.14.0
  • Changes are API compatible.
  • N/A: New feature has been added to the versions.md file (if applicable).
  • N/A: 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.

@EduPonz EduPonz added this to the v2.14.0 milestone Jan 26, 2024
@EduPonz EduPonz force-pushed the feature/ignore_other_vendors_custom_pids branch from 872a9a6 to dbc4812 Compare January 27, 2024 07:52
@EduPonz
Copy link
Author

EduPonz commented Jan 27, 2024

@richiprosima please test this

@EduPonz
Copy link
Author

EduPonz commented Jan 28, 2024

@richiprosima please test mac

1 similar comment
@JesusPoderoso
Copy link
Contributor

@richiprosima please test mac

MiguelCompany
MiguelCompany requested changes Jan 29, 2024
View reviewed changes
test/unittest/rtps/common/VendorIdTests.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/builtin/data/ParticipantProxyData.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/builtin/data/ReaderProxyData.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/builtin/data/WriterProxyData.cpp Outdated Show resolved Hide resolved
src/cpp/fastdds/core/policy/ParameterList.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/builtin/data/ReaderProxyData.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/builtin/data/WriterProxyData.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/messages/MessageReceiver.cpp Show resolved Hide resolved
test/unittest/rtps/builtin/BuiltinDataSerializationTests.cpp Show resolved Hide resolved
@EduPonz EduPonz force-pushed the feature/ignore_other_vendors_custom_pids branch from dbc4812 to 5931f7a Compare January 30, 2024 13:57
MiguelCompany
MiguelCompany previously approved these changes Feb 1, 2024
View reviewed changes
@EduPonz
Copy link
Author

EduPonz commented Feb 23, 2024

@Mergifyio rebase

@mergify Mergify
Copy link
Contributor

mergify bot commented Feb 23, 2024

rebase

☑️ Nothing to do

  • -conflict [📌 rebase requirement]
  • -closed [📌 rebase requirement]

@EduPonz
Copy link
Author

EduPonz commented Feb 23, 2024

@Mergifyio rebase master

@mergify Mergify
Copy link
Contributor

mergify bot commented Feb 23, 2024

rebase master

☑️ Nothing to do

  • -conflict [📌 rebase requirement]
  • -closed [📌 rebase requirement]

@EduPonz
Refs #20307: Add connext vendor id const
46b39dd 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20307: Mark fastrtps::rtps vendor id related usings to be removed
a889262 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20307: Add vendor id ostream operator<<
849b57d 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20307: Add vendor id to CacheChange
30d0cf1 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20307: Add vendor id to all submessage processor interfaces
3f1fb6b 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20307: Ignore custom PIDs from other vendors
a0af8e5 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20307: Migrate remaining fastrtps::rtps::VendorId_t references …
080fb0c 
…to fastdds::rtps::VendorId_t

Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20307: Add tests for custom PID processing from other vendors
033e3e4 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20307: Uncrustify
90df1fe 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20307: Fix Windows v141 build
49e568c 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20307: Fix documentation build
f711d4a 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20307: Appy Miguel's suggestions
76bd597 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20307: Appy Miguel's suggestions 2
03df5d0 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz EduPonz mentioned this pull request Feb 24, 2024
Merged
6 tasks
@EduPonz EduPonz force-pushed the feature/ignore_other_vendors_custom_pids branch from aa243dd to 03df5d0 Compare February 24, 2024 06:29
@EduPonz
Copy link
Author

EduPonz commented Feb 24, 2024

@richiprosima please test this

@EduPonz EduPonz added the ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. label Feb 24, 2024
@EduPonz EduPonz merged commit f900e15 into master Feb 26, 2024
12 of 17 checks passed
@EduPonz EduPonz deleted the feature/ignore_other_vendors_custom_pids branch February 26, 2024 06:42
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
ready-to-merge Ready to be merged. CI and changes have been reviewed and approved.
Projects
None yet
Milestone
v2.14.0
Development

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