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.

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

Use rmw_qos_profile_unknown when adding entity to graph #28

Merged
merged 1 commit into from
Apr 14, 2021

Conversation

ivanpauno
Copy link
Member

Some of the qos policies aren't shared during discovery (history).
Use rmw_qos_profile_unknown as the base profile, so users of rmw_get_publishers_info_by_topic(), rmw_get_subscriptions_info_by_topic() can tell which policy was correctly filled by the middleware and which not.

This was already the case in fastrtps.
Cyclonedds is also sharing history during discovery, so this is not an issue.

We're also documenting this in ros2/rmw#308, because docs weren't clear in the past.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Copy link
Collaborator

@asorbini asorbini left a comment

Choose a reason for hiding this comment

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

LGTM

@ivanpauno ivanpauno changed the title Use rmw_qos_profile_unknown when entity to graph Use rmw_qos_profile_unknown when adding entity to graph Apr 13, 2021
@ivanpauno
Copy link
Member Author

ivanpauno commented Apr 13, 2021

CI:

  • Linux Build Status
  • macOS Build Status
  • Windows Build Status

@ivanpauno ivanpauno added this to In progress in Galactic via automation Apr 13, 2021
@asorbini asorbini added the galactic PR pertaining the Galactic release label Apr 14, 2021
@asorbini
Copy link
Collaborator

asorbini commented Apr 14, 2021

Test results are in line with expected test failures when rmw_connextdds is the default implementation (see e.g. #22).

I'm going to merge the PR.

FYI, @ivanpauno I think you can also run CI with just Connext selected if you want. The required fastrtps packages should be included even if rmw_fastrtps_cpp is disabled.

EDIT: added link to old PR.

@asorbini asorbini merged commit b866136 into master Apr 14, 2021
@delete-merged-branch delete-merged-branch bot deleted the ivanpauno/use-rmw-qos-profile-unknown branch April 14, 2021 01:00
@asorbini asorbini added the foxy-backports PR should be backported to Foxy label Apr 14, 2021
asorbini pushed a commit that referenced this pull request Apr 14, 2021
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
asorbini added a commit that referenced this pull request Apr 14, 2021
* Use rmw_qos_profile_unknown when adding entity to graph (#28)

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Co-authored-by: Andrea Sorbini <asorbini@rti.com>
@asorbini asorbini mentioned this pull request Apr 14, 2021
@clalancette clalancette moved this from In progress to Needs Release in Galactic Apr 14, 2021
@clalancette clalancette moved this from Needs Release to Done in Galactic May 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working foxy-backports PR should be backported to Foxy galactic PR pertaining the Galactic release
Projects
No open projects
Galactic
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants