Conversation
289eaaf to
a00babb
Compare
fujitatomoya
left a comment
There was a problem hiding this comment.
@clintlombard thanks for the contribution.
So after all, this exposes sub_opts.ignore_local_publications for rclpy, right?
Is there any related issue to address this feature? I think it would be nice to to support SubscriptionOptions and PublisherOptions.
|
@fujitatomoya I didn't find any applicable issues this was just based on a need I had for a tangential project. I could also look at adding |
adityapande-1995
left a comment
There was a problem hiding this comment.
Just some minor comments, looks good to me !
Signed-off-by: Clint Lombard <clint.lom@gmail.com>
a00babb to
19ce087
Compare
Thanks for catching those. All sorted |
| class SubscriptionOptions(_rclpy.rmw_subscription_options_t): | ||
| pass |
There was a problem hiding this comment.
I was expecting that abstraction class such as QoSProfile with initialization, setter and getter accessors? so that it would be useful as python client library, and conceal implementation details with C/C++ language.
There was a problem hiding this comment.
I was trying to keep the implementation more in line with the rclcpp interface, but can look at the changes this might require.
| std::shared_ptr<rcl_subscription_t> rcl_subscription_; | ||
| }; | ||
| /// Define a pybind11 wrapper for an rclpy::Service | ||
| /// Define a pybind11 wrapper for an rclpy::Subscription |
| EmptyMsg, | ||
| 'test_topic', | ||
| QoSProfile(depth=10).get_c_qos_profile(), | ||
| None |
There was a problem hiding this comment.
I believe that we need to add more test cases here,
- actually use
SubscriptionOptionsas argument. (NotNonecase) - what is the default value for
SubscriptionOptions? - verify
SubscriptionOptionsmember variables set by user application?
There was a problem hiding this comment.
Good idea. I'll add a test case for this.
Expose
rmw_subscription_optionsasSubscriptionOptions.Example usage:
Output: