Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions rclpy/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,11 @@ pybind11_add_module(_rclpy_pybind11 SHARED
src/rclpy/client.cpp
src/rclpy/clock.cpp
src/rclpy/context.cpp
src/rclpy/destroyable.cpp
src/rclpy/duration.cpp
src/rclpy/graph.cpp
src/rclpy/guard_condition.cpp
src/rclpy/handle.cpp
src/rclpy/init.cpp
src/rclpy/logging.cpp
src/rclpy/names.cpp
Expand Down
18 changes: 9 additions & 9 deletions rclpy/rclpy/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class Client:
def __init__(
self,
context: Context,
client_handle,
client_impl: _rclpy.Client,
srv_type: SrvType,
srv_name: str,
qos_profile: QoSProfile,
Expand All @@ -46,15 +46,15 @@ def __init__(
should call :meth:`.Node.create_client`.

:param context: The context associated with the service client.
:param client_handle: :class:`Handle` wrapping the underlying ``rcl_client_t`` object.
:param client_impl: :class:`_rclpy.Client` wrapping the underlying ``rcl_client_t`` object.
:param srv_type: The service type.
:param srv_name: The name of the service.
:param qos_profile: The quality of service profile to apply the service client.
:param callback_group: The callback group for the service client. If ``None``, then the
nodes default callback group is used.
"""
self.context = context
self.__handle = client_handle
self.__client = client_impl
self.srv_type = srv_type
self.srv_name = srv_name
self.qos_profile = qos_profile
Expand Down Expand Up @@ -122,8 +122,8 @@ def call_async(self, request: SrvTypeRequest) -> Future:
if not isinstance(request, self.srv_type.Request):
raise TypeError()

with self.handle as capsule:
sequence_number = _rclpy.rclpy_send_request(capsule, request)
with self.handle:
sequence_number = self.__client.send_request(request)
Comment on lines +125 to +126
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sloretz nit^N: perhaps using the outcome of self.handle.__enter__ is easier to read (though equivalent)?

Suggested change
with self.handle:
sequence_number = self.__client.send_request(request)
with self.handle as client:
sequence_number = client.send_request(request)

Same elsewhere.

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.

I went the opposite direction in a6149d1 and made the context manager return None when it's entered. I'm a little cautions because I don't know if pybind11 has a way to figure out the Destroyable type returned at __enter__ is an instance of rclpy::Client. If __enter__ returns Destroyable d, then is client.send_request effectively reinterpret_cast<Client *>(d)->send_request()? I think I recall @mxgrey showing me a case (IIRC involving multiple inheritance) where that's unsafe to do.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aha! Interesting. Good to know.

if sequence_number in self._pending_requests:
raise RuntimeError('Sequence (%r) conflicts with pending request' % sequence_number)

Expand All @@ -140,8 +140,8 @@ def service_is_ready(self) -> bool:

:return: ``True`` if a server is ready, ``False`` otherwise.
"""
with self.handle as capsule:
return _rclpy.rclpy_service_server_is_available(capsule)
with self.handle:
return self.__client.service_server_is_available()

def wait_for_service(self, timeout_sec: float = None) -> bool:
"""
Expand All @@ -165,7 +165,7 @@ def wait_for_service(self, timeout_sec: float = None) -> bool:

@property
def handle(self):
return self.__handle
return self.__client

def destroy(self):
self.handle.destroy()
self.__client.destroy_when_not_in_use()
13 changes: 7 additions & 6 deletions rclpy/rclpy/executors.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,8 +340,8 @@ async def _execute_subscription(self, sub, msg):
await await_or_execute(sub.callback, msg)

def _take_client(self, client):
with client.handle as capsule:
return _rclpy.rclpy_take_response(capsule, client.srv_type.Response)
with client.handle:
return client.handle.take_response(client.srv_type.Response)

async def _execute_client(self, client, seq_and_response):
header, response = seq_and_response
Expand Down Expand Up @@ -519,10 +519,11 @@ def _wait_for_ready_callbacks(
except InvalidHandle:
entity_count.num_subscriptions -= 1

client_capsules = []
client_handles = []
for cli in clients:
try:
client_capsules.append(context_stack.enter_context(cli.handle))
context_stack.enter_context(cli.handle)
client_handles.append(cli.handle)
except InvalidHandle:
entity_count.num_clients -= 1

Expand Down Expand Up @@ -561,8 +562,8 @@ def _wait_for_ready_callbacks(
_rclpy.rclpy_wait_set_clear_entities(wait_set)
for sub_capsule in sub_capsules:
_rclpy.rclpy_wait_set_add_entity('subscription', wait_set, sub_capsule)
for cli_capsule in client_capsules:
_rclpy.rclpy_wait_set_add_entity('client', wait_set, cli_capsule)
for cli_handle in client_handles:
_rclpy.rclpy_wait_set_add_client(wait_set, cli_handle)
for srv_capsule in service_capsules:
_rclpy.rclpy_wait_set_add_entity('service', wait_set, srv_capsule)
for tmr_capsule in timer_capsules:
Expand Down
4 changes: 2 additions & 2 deletions rclpy/rclpy/handle.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
from threading import Lock

from rclpy.impl.implementation_singleton import rclpy_handle_implementation as _rclpy_handle
from rclpy.impl.implementation_singleton import rclpy_implementation as _rclpy
from rclpy.impl.implementation_singleton import rclpy_pycapsule_implementation as _rclpy_capsule


class InvalidHandle(Exception):
pass
InvalidHandle = _rclpy.InvalidHandle


class Handle:
Expand Down
6 changes: 2 additions & 4 deletions rclpy/rclpy/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -1383,7 +1383,7 @@ def create_client(
failed = False
try:
with self.handle as node_capsule:
client_capsule = _rclpy.rclpy_create_client(
client_impl = _rclpy.Client(
node_capsule,
srv_type,
srv_name,
Expand All @@ -1393,11 +1393,9 @@ def create_client(
if failed:
self._validate_topic_or_service_name(srv_name, is_service=True)

client_handle = Handle(client_capsule)

client = Client(
self.context,
client_handle, srv_type, srv_name, qos_profile,
client_impl, srv_type, srv_name, qos_profile,
callback_group)
self.__clients.append(client)
callback_group.add_entity(client)
Expand Down
21 changes: 9 additions & 12 deletions rclpy/src/rclpy/_rclpy_pybind11.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "client.hpp"
#include "clock.hpp"
#include "context.hpp"
#include "destroyable.hpp"
#include "duration.hpp"
#include "graph.hpp"
#include "guard_condition.hpp"
Expand All @@ -43,6 +44,8 @@ namespace py = pybind11;
PYBIND11_MODULE(_rclpy_pybind11, m) {
m.doc() = "ROS 2 Python client library.";

rclpy::define_destroyable(m);

py::enum_<rcl_clock_type_t>(m, "ClockType")
.value("UNINITIALIZED", RCL_CLOCK_UNINITIALIZED)
.value("ROS_TIME", RCL_ROS_TIME)
Expand Down Expand Up @@ -100,6 +103,8 @@ PYBIND11_MODULE(_rclpy_pybind11, m) {
m, "UnsupportedEventTypeError", rclerror.ptr());
py::register_exception<rclpy::NotImplementedError>(
m, "NotImplementedError", PyExc_NotImplementedError);
py::register_exception<rclpy::InvalidHandle>(
m, "InvalidHandle", PyExc_RuntimeError);

m.def(
"rclpy_init", &rclpy::init,
Expand All @@ -108,18 +113,7 @@ PYBIND11_MODULE(_rclpy_pybind11, m) {
"rclpy_shutdown", &rclpy::shutdown,
"rclpy_shutdown.");

m.def(
"rclpy_create_client", &rclpy::client_create,
"Create a Client");
m.def(
"rclpy_send_request", &rclpy::client_send_request,
"Send a request");
m.def(
"rclpy_service_server_is_available", &rclpy::client_service_server_is_available,
"Return true if the service server is available");
m.def(
"rclpy_take_response", &rclpy::client_take_response,
"rclpy_take_response");
rclpy::define_client(m);

m.def(
"rclpy_context_get_domain_id", &rclpy::context_get_domain_id,
Expand Down Expand Up @@ -268,6 +262,9 @@ PYBIND11_MODULE(_rclpy_pybind11, m) {
m.def(
"rclpy_wait_set_add_entity", &rclpy::wait_set_add_entity,
"rclpy_wait_set_add_entity.");
m.def(
"rclpy_wait_set_add_client", &rclpy::wait_set_add_client,
"Add a client to the wait set.");
m.def(
"rclpy_wait_set_is_ready", &rclpy::wait_set_is_ready,
"rclpy_wait_set_is_ready.");
Expand Down
Loading