Change rmw_count_publishers API, to rcl equivalent rcl_count_publishe… #425
Conversation
…rs and remove the TODO line. Signed-off-by: Sriram Raghunathan <rsriram7@visteon.com>
| @@ -174,8 +174,7 @@ NodeGraph::count_publishers(const std::string & topic_name) const | |||
| false); // false = not a service | |||
|
|
|||
| size_t count; | |||
| // TODO(wjwwood): use the rcl equivalent methods | |||
| auto ret = rmw_count_publishers(rmw_node_handle, fqdn.c_str(), &count); | |||
| auto ret = rcl_count_publishers(rmw_node_handle, fqdn.c_str(), &count); | |||
There was a problem hiding this comment.
rcl_count_publishers takes an rcl node handle instead of an rmw one
dhood
added
in progress
…ure to topic_names. Signed-off-by: Sriram Raghunathan <rsriram7@visteon.com>
|
Hi @dhood made changes after reading a few things inside rcl. Sorry for the initial patch which was incomplete. Could you review the changes? |
| @@ -166,16 +166,9 @@ NodeGraph::get_node_names() const | |||
| size_t | |||
| NodeGraph::count_publishers(const std::string & topic_name) const | |||
| { | |||
| auto rmw_node_handle = rcl_node_get_rmw_handle(node_base_->get_rcl_node_handle()); | |||
| auto fqdn = rclcpp::expand_topic_or_service_name( | |||
There was a problem hiding this comment.
This step will combine the topic name and namespace into a "fully qualified" topic name (example) and should not be removed
Signed-off-by: Sriram Raghunathan <rsriram7@visteon.com>
|
@dhood made changes to use rcl_* specific API's have a look. |
| @@ -167,8 +167,17 @@ size_t | |||
| NodeGraph::count_publishers(const std::string & topic_name) const | |||
| { | |||
| auto rcl_node_handle = node_base_->get_rcl_node_handle(); | |||
| auto name = rcl_node_get_name(rcl_node_handle); | |||
| auto namespace_ = rcl_node_get_namespace(rcl_node_handle); | |||
There was a problem hiding this comment.
please use trailing underscores only for member variables. same below.
There was a problem hiding this comment.
@Karsten1987 I have'nt read variables with '_' prefix, is that a valid declaration within "ros2" coding styles? May I use it here?
There was a problem hiding this comment.
We normally adhere to the C++ coding guidelines: https://google.github.io/styleguide/cppguide.html#Variable_Names
Now, I think you can actually use it here, because namespace is a reserved keyword in C++. I've seen we used it in other places as well, so that I believe in this special case, it's fine.
Please ignore my comment then.
|
@dhood could you review the latest commits? |
dhood
added
in review
and linters are happy after my fixup: |
…ging_log4cxx (ros2#425) Signed-off-by: burekn <burekn@amazon.com>
ros2#436) * Revert "Changes the default 3rd party logger from rcl_logging_noop to rcl_logging_log4cxx (ros2#425)" This reverts commit ac8ee90. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org> * Revert "add explicit dependency on rcl_logging_log4cxx (ros2#435)" This reverts commit 816ce67. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org> * Add dependency on rcl_logging_noop. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Change rmw_count_publishers API to rcl equivalent rcl_count_publishers and remove the TODO comment from the source.
Signed-off-by: Sriram Raghunathan rsriram7@visteon.com