Skip to content
This repository was archived by the owner on Oct 7, 2021. It is now read-only.

Support for ON_REQUESTED_INCOMPATIBLE_QOS and ON_OFFERED_INCOMPATIBLE_QOS events #294

Closed
jaisontj wants to merge 4 commits intoros2:masterfrom
aws-ros-dev:jaisontj/incompatible_qos
Closed

Support for ON_REQUESTED_INCOMPATIBLE_QOS and ON_OFFERED_INCOMPATIBLE_QOS events #294
jaisontj wants to merge 4 commits intoros2:masterfrom
aws-ros-dev:jaisontj/incompatible_qos

Conversation

@jaisontj
Copy link
Copy Markdown
Contributor

@jaisontj jaisontj commented Nov 15, 2019

Must not be merged before ros2/rmw:193 is merged. The build will fail until then since the required enums have been defined in that PR.

Related to this feature request. The design and implementation details can also be found there.

Handling events of type REQUESTED_INCOMPATIBLE_QOS_STATUS and OFFERED_INCOMPATIBLE_QOS_STATUS

Signed-off-by: Jaison Titus jaisontj92@gmail.com

Signed-off-by: Jaison Titus <jaisontj92@gmail.com>
OFFERED_INCOMPATIBLE_QOS_STATUS

Signed-off-by: Jaison Titus <jaisontj92@gmail.com>
@jaisontj jaisontj force-pushed the jaisontj/incompatible_qos branch from f1bfeda to 826fe07 Compare November 19, 2019 01:08
Signed-off-by: Jaison Titus <jaisontj92@gmail.com>
Signed-off-by: Jaison Titus <jaisontj92@gmail.com>
virtual void on_publication_matched(
DDS::DataWriter_ptr writer,
const DDS::PublicationMatchedStatus & status);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed new line to make code style more consistent with the rest of the file.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please, avoid unrelated changes.
In any case, adding the missing spaces in other places would be better.

Copy link
Copy Markdown
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

Just one nit, otherwise LGTM!

virtual void on_publication_matched(
DDS::DataWriter_ptr writer,
const DDS::PublicationMatchedStatus & status);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please, avoid unrelated changes.
In any case, adding the missing spaces in other places would be better.

@wjwwood wjwwood self-assigned this Jan 9, 2020
@mm318
Copy link
Copy Markdown
Member

mm318 commented Feb 24, 2020

Is rmw_opensplice_cpp still being actively developed? Is this pull request still needed?

@dirk-thomas
Copy link
Copy Markdown
Member

Is rmw_opensplice_cpp still being actively developed?

While rmw_opensplice_cpp is being supported in Dashing and Eloquent it won't be supported in Foxy: see https://discourse.ros.org/t/opensplice-move-to-eclipse-cyclone-dds-in-ros-2/12370

Is this pull request still needed?

I would think no since this feature targets Foxy and isn't intended to be backported to older distros anyway.

@mm318
Copy link
Copy Markdown
Member

mm318 commented Feb 24, 2020

Great!

Then anybody with permission may close this pull request (I don't have the permission to).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants