Skip to content

Retrieve HistoryQoS in discovery when available#829

Merged
ahcorde merged 3 commits intoros2:rollingfrom
eProsima:qos/history
Jun 4, 2025
Merged

Retrieve HistoryQoS in discovery when available#829
ahcorde merged 3 commits intoros2:rollingfrom
eProsima:qos/history

Conversation

@Mario-DL
Copy link
Contributor

@Mario-DL Mario-DL commented May 30, 2025 •
edited
Loading

Description

This PR adds the HistoryQoS to the entities discovery callback when available.
Related tests PRs

Fixes #684

Is this user-facing behavior change?

No

Did you use Generative AI?

No

Additional Information

Should be backported only to Kilted

@Mario-DL
Retrieve HistoryQoS in discovery when available
d688eb1 
Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>
@Mario-DL Mario-DL mentioned this pull request May 30, 2025
Merged
@Mario-DL Mario-DL mentioned this pull request May 30, 2025
Merged
@Mario-DL
Linter
5c6f53c 
Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>
@ahcorde
Copy link
Contributor

ahcorde commented May 30, 2025

Pulls: #829, ros2/rcl#1242, ros2/ros2cli#1040
Gist: https://gist.githubusercontent.com/ahcorde/e1d6b10259d33a4a01ea1e33a4dc7da6/raw/ddeaa93863d812677523326fe68db8b191509d9d/ros2.repos
BUILD args: --packages-above-and-dependencies rcl rmw_fastrtps_shared_cpp ros2cli
TEST args: --packages-above rcl rmw_fastrtps_shared_cpp ros2cli
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16136

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

fujitatomoya
fujitatomoya reviewed May 30, 2025
View reviewed changes
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

@Mario-DL are you trying to fix particular issue with this PR?

these are RTPS QoS, so i thought there is no history available via discovery process here. the new Fast-DDS supports this history QoS during this discovery process?

@Mario-DL
Change if-else to switch
5346e28 
Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>
@Mario-DL
Copy link
Contributor Author

Mario-DL commented Jun 2, 2025

@Mario-DL are you trying to fix particular issue with this PR?

these are RTPS QoS, so i thought there is no history available via discovery process here. the new Fast-DDS supports this history QoS during this discovery process?

Yes (and commonly accepted across vendors). The standard defines the PID for the HistoryQoS but it is optional to exchange it during discovery since it does not intervene in the matching. In short, we simply made HistoryQoS available in the Data(r/w).

fujitatomoya
fujitatomoya approved these changes Jun 2, 2025
View reviewed changes
Copy link
Collaborator

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

@ahcorde
Copy link
Contributor

ahcorde commented Jun 2, 2025 •
edited
Loading

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@ahcorde ahcorde merged commit e78f3f4 into ros2:rolling Jun 4, 2025
3 checks passed
@MiguelCompany MiguelCompany deleted the qos/history branch June 4, 2025 10:02
@MiguelCompany
Copy link
Collaborator

MiguelCompany commented Jun 4, 2025

Thanks for merging @ahcorde. Would be nice to backport this into kilted

@ahcorde
Copy link
Contributor

ahcorde commented Jun 4, 2025

https://github.com/Mergifyio backport kilted

@mergify
Copy link

mergify bot commented Jun 4, 2025 •
edited
Loading

backport kilted

✅ Backports have been created

Details

mergify bot pushed a commit that referenced this pull request Jun 4, 2025
@Mario-DL @mergify
Retrieve HistoryQoS in discovery when available (#829)
167e703 
Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>
(cherry picked from commit e78f3f4)
This was referenced Jun 4, 2025
Merged
Merged
Merged
ahcorde pushed a commit that referenced this pull request Jun 4, 2025
@mergify @Mario-DL
Retrieve HistoryQoS in discovery when available (#829) (#830)
69c97ee 
(cherry picked from commit e78f3f4)

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>
Co-authored-by: Mario Domínguez López <116071334+Mario-DL@users.noreply.github.com>
@emersonknapp
Copy link
Contributor

emersonknapp commented Oct 15, 2025 •
edited
Loading

@Mergifyio backport jazzy humble

@Mario-DL is it possible to backport this feature to Jazzy/Humble? Or is the version of Fast-DDS in use on those platforms too diverged to backport the new discovery information?

@mergify
Copy link

mergify bot commented Oct 15, 2025 •
edited
Loading

backport jazzy humble

✅ Backports have been created

Details

mergify bot pushed a commit that referenced this pull request Oct 15, 2025
@Mario-DL @mergify
Retrieve HistoryQoS in discovery when available (#829)
b64408f 
Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>
(cherry picked from commit e78f3f4)

# Conflicts:
#	rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/qos.hpp
@mergify mergify bot mentioned this pull request Oct 15, 2025
Open
mergify bot pushed a commit that referenced this pull request Oct 15, 2025
@Mario-DL @mergify
Retrieve HistoryQoS in discovery when available (#829)
39a93dc 
Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>
(cherry picked from commit e78f3f4)

# Conflicts:
#	rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/qos.hpp
@mergify mergify bot mentioned this pull request Oct 15, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ahcorde ahcorde ahcorde approved these changes

@fujitatomoya fujitatomoya fujitatomoya approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

get_publishers_info_by_topic does not return qos depth for publishers with fastDDS

5 participants

@Mario-DL @ahcorde @MiguelCompany @emersonknapp @fujitatomoya