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

Stubs for rmw_get_publishers_info_by_topic and rmw_get_subscriptions_info_by_topic #289

Merged
merged 6 commits into from
Jan 14, 2020

Conversation

jaisontj
Copy link
Contributor

@jaisontj jaisontj commented Oct 18, 2019

NOTE: DO NOT MERGE until rmw #186 and rmw_implementation #72 are merged.

Leaving stubs for rmw_get_publishers_info_by_topic and rmw_get_subscriptions_info_by_topic as per discussion here

Related to - aws-roadmap#94

@prajakta-gokhale
Copy link

prajakta-gokhale commented Nov 13, 2019

Could you look at the test failures and see if it's related to this change?

rmw_opensplice_cpp failures 1 and 2.

@jaisontj
Copy link
Contributor Author

It is, but the error is because this PR has not been merged and that PR is the one that defines the required structs needed for this one. I've updated the PR description to say the same.

rmw_get_subscriptions_info_by_topic

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

@prajakta-gokhale prajakta-gokhale left a comment

Choose a reason for hiding this comment

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

Looks good.

Signed-off-by: Jaison Titus <jaisontj92@gmail.com>
@jaisontj
Copy link
Contributor Author

@ivanpauno

Signed-off-by: Jaison Titus <jaisontj92@gmail.com>
Copy link
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.

I'm ok with adding the stubs temporally.
But please create an issue. The task should be completed before next release.

rmw_opensplice_cpp/src/rmw_get_topic_info.cpp Outdated Show resolved Hide resolved
rmw_opensplice_cpp/src/rmw_get_topic_info.cpp Outdated Show resolved Hide resolved
@ivanpauno
Copy link
Member

A similar PR is missing for https://github.com/ros2/rmw_cyclonedds/.

@jaisontj
Copy link
Contributor Author

jaisontj commented Dec 11, 2019

I'm ok with adding the stubs temporally.
But please create an issue. The task should be completed before next release.

Issue created here.

Copy link

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM too

Signed-off-by: Miaofei <miaofei@amazon.com>
Signed-off-by: Miaofei <miaofei@amazon.com>
// limitations under the License.

#include "rmw/error_handling.h"
#include "rmw/get_topic_info.h"
Copy link
Member

Choose a reason for hiding this comment

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

@mm318 CI I've ran failed because of the unupdated name.

Copy link
Member

@mm318 mm318 Jan 13, 2020

Choose a reason for hiding this comment

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

Oops, my editor somehow missed that file.

@ivanpauno, can you please run the CI again? Thanks!

Signed-off-by: Miaofei <miaofei@amazon.com>
@ivanpauno ivanpauno merged commit d2dbf27 into ros2:master Jan 14, 2020
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.

None yet

6 participants