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

[21371] Change monitor service writer entity id #5070

Merged
merged 7 commits into from
Jul 19, 2024
Merged

[21371] Change monitor service writer entity id #5070

merged 7 commits into from
Jul 19, 2024

Conversation

rsanchez15
Copy link
Contributor

@rsanchez15 rsanchez15 commented Jul 15, 2024 •
edited by cferreiragonz
Loading

Description

The EntityID of the MonitorService DataWriter is 0x004000D2, which is not included in the statistics data writer range (GuidUtils.hpp::37).

Hence, the discovery database fails to recognize the DataWriter and issues an error (DiscoveryDataBase.cpp::400) when the writer is discovered.

@Mergifyio 2.14.x

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
  • N/A 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.
  • 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.
  • 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.
  • 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.

@rsanchez15 rsanchez15 requested a review from cferreiragonz July 15, 2024 13:14
@github-actions github-actions bot added the ci-pending PR which CI is running label Jul 15, 2024
@cferreiragonz cferreiragonz added this to the v3.0.0 milestone Jul 15, 2024
cferreiragonz
cferreiragonz previously approved these changes Jul 15, 2024
View reviewed changes
@rsanchez15 rsanchez15 changed the title Change monitor service writer entity id [21371] Change monitor service writer entity id Jul 16, 2024
@rsanchez15
Add monitor service writer check
853ce20 
Signed-off-by: Raul Sanchez-Mateos <[email protected]>
@rsanchez15
Revert monitor service entity_id
2971971 
Signed-off-by: Raul Sanchez-Mateos <[email protected]>
@rsanchez15 rsanchez15 requested review from cferreiragonz and removed request for cferreiragonz July 17, 2024 12:34
@rsanchez15
Uncrustify
b429233 
Signed-off-by: Raul Sanchez-Mateos <[email protected]>
@rsanchez15 rsanchez15 requested review from cferreiragonz and removed request for cferreiragonz July 17, 2024 12:36
@rsanchez15
Remove extra header file
e9b65a2 
Signed-off-by: Raul Sanchez-Mateos <[email protected]>
@rsanchez15 rsanchez15 requested review from cferreiragonz and removed request for cferreiragonz July 17, 2024 13:00
cferreiragonz
cferreiragonz previously approved these changes Jul 17, 2024
View reviewed changes
Copy link
Contributor

@cferreiragonz cferreiragonz 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

@cferreiragonz cferreiragonz dismissed their stale review July 17, 2024 13:59

Stats build failing

@rsanchez15 rsanchez15 requested a review from cferreiragonz July 17, 2024 14:10
cferreiragonz
cferreiragonz previously approved these changes Jul 17, 2024
View reviewed changes
Copy link
Contributor

@cferreiragonz cferreiragonz 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

@rsanchez15
Fix windows unreachable code warning
f627f64 
Signed-off-by: Raul Sanchez-Mateos <[email protected]>
@rsanchez15 rsanchez15 requested a review from cferreiragonz July 18, 2024 05:28
@rsanchez15 rsanchez15 added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed ci-pending PR which CI is running labels Jul 18, 2024
@cferreiragonz
Copy link
Contributor

@Mergifyio backport 2.14.x

@eProsima eProsima deleted a comment from mergify bot Jul 18, 2024
@mergify Mergify
Copy link
Contributor

mergify bot commented Jul 18, 2024 •
edited
Loading

backport 2.14.x

✅ Backports have been created

@rsanchez15 rsanchez15 merged commit 0bdc1cc into master Jul 19, 2024
17 checks passed
@rsanchez15 rsanchez15 deleted the feature/monitor-service-guid branch July 19, 2024 05:54
mergify bot pushed a commit that referenced this pull request Jul 19, 2024
@rsanchez15 @mergify
Change monitor service writer entity id (#5070)
6f0a4ff 
* Refs #21371: Change monitor service writer entity id

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

* Add monitor service writer check

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

* Revert monitor service entity_id

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

* Uncrustify

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

* Remove extra header file

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

* Change return when statistics is not defined

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

* Fix windows unreachable code warning

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

---------

Signed-off-by: Raul Sanchez-Mateos <[email protected]>
(cherry picked from commit 0bdc1cc)
@mergify mergify bot mentioned this pull request Jul 19, 2024
Merged
11 tasks
MiguelCompany pushed a commit that referenced this pull request Sep 6, 2024
@mergify @rsanchez15
Change monitor service writer entity id (#5070) (#5089)
c73df11 
* Change monitor service writer entity id (#5070)

* Refs #21371: Change monitor service writer entity id

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

* Add monitor service writer check

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

* Revert monitor service entity_id

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

* Uncrustify

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

* Remove extra header file

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

* Change return when statistics is not defined

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

* Fix windows unreachable code warning

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

---------

Signed-off-by: Raul Sanchez-Mateos <[email protected]>
(cherry picked from commit 0bdc1cc)

* Fix entity_id namespace

Signed-off-by: Raul Sanchez-Mateos <[email protected]>

---------

Signed-off-by: Raul Sanchez-Mateos <[email protected]>
Co-authored-by: Raul Sanchez-Mateos Lizano <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cferreiragonz cferreiragonz cferreiragonz 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
v3.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@rsanchez15 @cferreiragonz