Skip to content

Extend DISCOVERY_TOPIC to include physical information [14006] #2549

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
Merged
merged 19 commits into from
Mar 9, 2022
Merged

Extend DISCOVERY_TOPIC to include physical information [14006] #2549

merged 19 commits into from
Mar 9, 2022

Conversation

EduPonz
Copy link

@EduPonz EduPonz commented Mar 2, 2022 •
edited
Loading

Extension of Statistics Module DISCOVERY_TOPIC to include physical information about the discovered participant

Description

This PR adds the following features:

  • Extension of DiscoveryTime data type
  • Extension of DATA[p] to include physical information
  • Reporting of physical information in DISCOVERY_TOPIC when a new participant is discovered (link between previous points)

This way, applications using the Statistics Module can get to know "physical" information about the discovered participant without needing to first discover it themselves.

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.
  • Changes are ABI compatible.
  • Changes are API compatible.
  • 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.
    Related documentation PR: Extend DISCOVERY_TOPIC to include physical information [14006] Fast-DDS-docs#352 (PR)

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.

@EduPonz EduPonz added no-test Skip CI tests if PR marked with this label doc-pending Issue or PR which is pending to be documented labels Mar 2, 2022
@EduPonz EduPonz force-pushed the feature/statistics/extend_discovered_entity branch 2 times, most recently from dbab413 to 16fc3e0 Compare March 4, 2022 13:57
@EduPonz EduPonz removed the no-test Skip CI tests if PR marked with this label label Mar 8, 2022
@EduPonz EduPonz force-pushed the feature/statistics/extend_discovered_entity branch from 5a16964 to babc681 Compare March 8, 2022 10:07
@EduPonz EduPonz marked this pull request as ready for review March 8, 2022 10:07
@JLBuenoLopez
Copy link
Contributor

JLBuenoLopez commented Mar 8, 2022 •
edited
Loading

Some initial comments about this PR:

@EduPonz EduPonz mentioned this pull request Mar 8, 2022
Merged
@EduPonz
Copy link
Author

EduPonz commented Mar 8, 2022

  • Ubuntu CI fails to build! It must be fixed!

Done in 5393d88

Done in eProsima/Fast-DDS-docs#352

@JLBuenoLopez
Copy link
Contributor

@richiprosima please test Windows

@EduPonz EduPonz removed the doc-pending Issue or PR which is pending to be documented label Mar 9, 2022
JLBuenoLopez
JLBuenoLopez suggested changes Mar 9, 2022
View reviewed changes
JLBuenoLopez
JLBuenoLopez reviewed Mar 9, 2022
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment •
edited
Loading

Choose a reason for hiding this comment

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

A small typo. LGTM

Linters are failing in some non auto-generated files

@EduPonz EduPonz added no-test Skip CI tests if PR marked with this label skip-ci Automatically pass CI and removed no-test Skip CI tests if PR marked with this label labels Mar 9, 2022
@EduPonz EduPonz force-pushed the feature/statistics/extend_discovered_entity branch from 9853afb to 08ce960 Compare March 9, 2022 14:34
JLBuenoLopez
JLBuenoLopez previously approved these changes Mar 9, 2022
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

LGTM

EduPonz added 10 commits March 9, 2022 16:28
@EduPonz
Refs 14006: Extend DiscoveryTime
0d27eca 
Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Populate physical properties if empty
beb386b 
Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Add empty physical properties if FASTDDS_STATISTICS is ac…
836cf10 
…tivated

Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Add test for default constructed physical properties
bfdba84 
Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Add physical properties to DATA[p]
16c3873 
Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Test that physical data is present in received Participan…
19a3759 
…tProxyData

Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Add negative case to test
945756c 
Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Address MSVC warning
d1454a0 
Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Add physical information to DiscoveryTime samples
c113d9d 
Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Add end-to-end discovery time with physical data positive…
b8bd9d8 
… test

Signed-off-by: Eduardo Ponz <[email protected]>
EduPonz added 9 commits March 9, 2022 16:28
@EduPonz
Refs 14006: Correctly notify physical data to listener
cd94801 
Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Improve discovery_topic_physical_data test
9f33461 
Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Extend discovery_topic_physical_data to cover all specifi…
8d718f4 
…ed behaviour

Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Update versions.md
be6af62 
Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: DomainParticipant properties in XML override values of th…
84ec9a5 
…ose present in QoS

Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Fix compilation when FASTDDS_STATISTICS is not defined
eaf3c4c 
Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Add test for setting physical properties using XML
e2929d2 
Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Apply suggestions
d916655 
Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Refs 14006: Apply suggestions
d9b2259 
Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz EduPonz force-pushed the feature/statistics/extend_discovered_entity branch from 08ce960 to d9b2259 Compare March 9, 2022 15:28
@MiguelCompany MiguelCompany added this to the v2.6.0 milestone Mar 9, 2022
JLBuenoLopez
JLBuenoLopez approved these changes Mar 9, 2022
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

👍

@EduPonz EduPonz merged commit bc1a32a into master Mar 9, 2022
@EduPonz EduPonz deleted the feature/statistics/extend_discovered_entity branch March 9, 2022 15:41
@EduPonz EduPonz mentioned this pull request Mar 16, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JLBuenoLopez JLBuenoLopez JLBuenoLopez approved these changes

Assignees
No one assigned
Labels
skip-ci Automatically pass CI versions-pending
Projects
None yet
Milestone
v2.6.0
Development

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