Skip to content

Convert last of pub/sub getters to pybind11#733

Merged
gbalke merged 9 commits intoros2:masterfrom
gbalke:last_of_pubsub
Mar 25, 2021
Merged

Convert last of pub/sub getters to pybind11#733
gbalke merged 9 commits intoros2:masterfrom
gbalke:last_of_pubsub

Conversation

@gbalke
Copy link
Copy Markdown
Contributor

@gbalke gbalke commented Mar 23, 2021

Part of #665.

This handles:

rclpy_get_publisher_logger_name
rclpy_get_publishers_info_by_topic
rclpy_get_subscriptions_info_by_topic

Greg Balke added 3 commits March 23, 2021 15:01
Move graph funcs
f3ddb8b 
Signed-off-by: Greg Balke <greg@openrobotics.org>
Move publisher getter
bf86bd6 
Signed-off-by: Greg Balke <greg@openrobotics.org>
Publisher conversion
8a23cea 
Signed-off-by: Greg Balke <greg@openrobotics.org>
@gbalke gbalke requested review from hidmic and sloretz March 23, 2021 23:48
@gbalke gbalke self-assigned this Mar 23, 2021
Touching up
5a6bd6c 
Signed-off-by: Greg Balke <greg@openrobotics.org>
hidmic
hidmic reviewed Mar 24, 2021
View reviewed changes
@sloretz sloretz mentioned this pull request Mar 24, 2021
Closed
34 tasks
Throw instead of printing errors and some docstring nits
cef9f2c 
Signed-off-by: Greg Balke <greg@openrobotics.org>
gbalke
gbalke commented Mar 24, 2021
View reviewed changes
@gbalke
Copy link
Copy Markdown
Contributor Author

gbalke commented Mar 24, 2021

@ros-pull-request-builder retest this please

hidmic
hidmic reviewed Mar 24, 2021
View reviewed changes
rclpy/src/rclpy/graph.cpp Outdated
if (RCL_RET_UNSUPPORTED == ret) {
throw RCLError(
"Failed to get information by topic. "
"Function not supported by RMW_IMPLEMENTATION");
Copy link
Copy Markdown
Contributor

@hidmic hidmic Mar 24, 2021

Choose a reason for hiding this comment

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

@gbalke I think it'd be best not to change the exception type (used to be NotImplementedError). You may add a new exception and register a translator to PyExc_NotImplementedError, see https://pybind11.readthedocs.io/en/stable/advanced/exceptions.html#registering-custom-translators.

CC @sloretz for feedback.

@hidmic
Copy link
Copy Markdown
Contributor

hidmic commented Mar 24, 2021

Also, this boy needs a rebase.

Merge branch 'master' into last_of_pubsub
5521ad9 
Signed-off-by: Greg Balke <greg@openrobotics.org>
@gbalke
Copy link
Copy Markdown
Contributor Author

gbalke commented Mar 24, 2021

Also, this boy needs a rebase.

Doesn't really matter as I'm going to squash and merge in the end?

Fix error handling
3b4057f 
Signed-off-by: Greg Balke <greg@openrobotics.org>
@gbalke gbalke requested a review from hidmic March 24, 2021 22:09
hidmic
hidmic approved these changes Mar 24, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@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 but for a few minor comments

rclpy/src/rclpy_common/include/rclpy_common/exceptions.hpp
using RCLError::RCLError;
};

class NotImplementedError : public RCLError
Copy link
Copy Markdown
Contributor

@hidmic hidmic Mar 24, 2021

Choose a reason for hiding this comment

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

@gbalke hmm, does RCL_RET_UNSUPPORTED set an rcl error?

Copy link
Copy Markdown
Contributor Author

@gbalke gbalke Mar 24, 2021

Choose a reason for hiding this comment

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

I assumed it was in the same boat as NodeNameNonExistant https://github.com/ros2/rcl/blob/0ca8c6162136594e490ba69fbf288de1b5f7073d/rcl/src/rcl/common.c

Greg Balke added 2 commits March 24, 2021 15:46
Error message fixes
80954ef 
Signed-off-by: Greg Balke <greg@openrobotics.org>
Merge branch 'master' into last_of_pubsub
a82efab 
Signed-off-by: Greg Balke <greg@openrobotics.org>
@gbalke
Copy link
Copy Markdown
Contributor Author

gbalke commented Mar 24, 2021

CI up to rclpy

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@gbalke gbalke merged commit 6765802 into ros2:master Mar 25, 2021
@gbalke gbalke deleted the last_of_pubsub branch March 25, 2021 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hidmic hidmic hidmic approved these changes

@sloretz sloretz Awaiting requested review from sloretz sloretz is a code owner

Assignees

@gbalke gbalke

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@gbalke @hidmic