Skip to content

Commit 0c87d5e

Browse files
committed
Backport of the EventsExecutor to humble
- Generics have been added to some parts of rclpy in a later version. Therefore, we can not use them, and they were removed from the affected type hints. - The TimerInfo argument is not present in humble. Anything related to it is removed. - The execution order of the SingleThreadedExecutor is not fifo in humble. This has been fixed in rolling, but the PR will not be backported, as this would be a breaking change. Therefore, a special case is added to one test that flips the order so we expect a fifo behavior for the events executor. - Subscription CallbackType enum is not present in humble, references to it have been removed. - The rcl function rcl_timer_set_on_reset_callback is not present in humble, removed timer reset callback from events executor. - The rclpy event_handler submodule is not implemented in humble, removed event callback checks from events executor test. - Replaced references to submodule rclpy clock_type with clock in events executor test. - Manually wake executor in future done callback for spin_once_until_future_complete tests (method differs from Jazzy/Rolling). Signed-off-by: Nathan Hui <ncynhui@gmail.com>
1 parent ae80fc1 commit 0c87d5e

9 files changed

Lines changed: 139 additions & 259 deletions

File tree

rclpy/rclpy/task.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import inspect
1717
import sys
1818
import threading
19-
from typing import Callable
2019
import warnings
2120
import weakref
2221

@@ -198,7 +197,7 @@ def add_done_callback(self, callback):
198197
if invoke:
199198
callback(self)
200199

201-
def remove_done_callback(self, callback: Callable[['Future[T]'], None]) -> bool:
200+
def remove_done_callback(self, callback) -> bool:
202201
"""
203202
Remove a previously-added done callback.
204203

rclpy/src/rclpy/events_executor/events_executor.cpp

Lines changed: 65 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,10 @@ EventsExecutor::EventsExecutor(py::object context)
5252
inspect_iscoroutine_(py::module_::import("inspect").attr("iscoroutine")),
5353
inspect_signature_(py::module_::import("inspect").attr("signature")),
5454
rclpy_task_(py::module_::import("rclpy.task").attr("Task")),
55-
rclpy_timer_timer_info_(py::module_::import("rclpy.timer").attr("TimerInfo")),
5655
signal_callback_([this]() {events_queue_.Stop();}),
5756
rcl_callback_manager_(&events_queue_),
5857
timers_manager_(
59-
&events_queue_, std::bind(&EventsExecutor::HandleTimerReady, this, pl::_1, pl::_2))
58+
&events_queue_, std::bind(&EventsExecutor::HandleTimerReady, this, pl::_1))
6059
{
6160
}
6261

@@ -135,10 +134,11 @@ void EventsExecutor::wake()
135134
{
136135
if (!wake_pending_.exchange(true)) {
137136
// Update tracked entities.
138-
events_queue_.Enqueue([this]() {
137+
events_queue_.Enqueue(
138+
[this]() {
139139
py::gil_scoped_acquire gil_acquire;
140140
UpdateEntitiesFromNodes(!py::cast<bool>(rclpy_context_.attr("ok")()));
141-
});
141+
});
142142
}
143143
}
144144

@@ -175,7 +175,7 @@ void EventsExecutor::spin(std::optional<double> timeout_sec, bool stop_after_use
175175

176176
const bool ok = py::cast<bool>(rclpy_context_.attr("ok")());
177177
if (!ok) {
178-
Raise(py::module_::import("rclpy.executors").attr("ExternalShutdownException")());
178+
Raise(py::module_::import("rclpy.executors").attr("ExternalShutdownException")());
179179
}
180180
}
181181

@@ -277,12 +277,12 @@ void EventsExecutor::HandleAddedSubscription(py::handle subscription)
277277
const auto cb = std::bind(&EventsExecutor::HandleSubscriptionReady, this, subscription, pl::_1);
278278
if (
279279
RCL_RET_OK != rcl_subscription_set_on_new_message_callback(
280-
rcl_ptr, RclEventCallbackTrampoline,
281-
rcl_callback_manager_.MakeCallback(rcl_ptr, cb, with)))
280+
rcl_ptr, RclEventCallbackTrampoline,
281+
rcl_callback_manager_.MakeCallback(rcl_ptr, cb, with)))
282282
{
283283
throw std::runtime_error(
284-
std::string("Failed to set the on new message callback for subscription: ") +
285-
rcl_get_error_string().str);
284+
std::string("Failed to set the on new message callback for subscription: ") +
285+
rcl_get_error_string().str);
286286
}
287287
}
288288

@@ -292,8 +292,8 @@ void EventsExecutor::HandleRemovedSubscription(py::handle subscription)
292292
const rcl_subscription_t * rcl_ptr = py::cast<const Subscription &>(handle).rcl_ptr();
293293
if (RCL_RET_OK != rcl_subscription_set_on_new_message_callback(rcl_ptr, nullptr, nullptr)) {
294294
throw std::runtime_error(
295-
std::string("Failed to clear the on new message callback for subscription: ") +
296-
rcl_get_error_string().str);
295+
std::string("Failed to clear the on new message callback for subscription: ") +
296+
rcl_get_error_string().str);
297297
}
298298
rcl_callback_manager_.RemoveCallback(rcl_ptr);
299299
}
@@ -313,9 +313,6 @@ void EventsExecutor::HandleSubscriptionReady(py::handle subscription, size_t num
313313
Subscription & _rclpy_sub = py::cast<Subscription &>(subscription.attr("handle"));
314314
const py::object msg_type = subscription.attr("msg_type");
315315
const bool raw = py::cast<bool>(subscription.attr("raw"));
316-
const int callback_type = py::cast<int>(subscription.attr("_callback_type").attr("value"));
317-
const int message_only =
318-
py::cast<int>(subscription.attr("CallbackType").attr("MessageOnly").attr("value"));
319316
const py::handle callback = subscription.attr("callback");
320317

321318
// rmw_cyclonedds has a bug which causes number_of_events to be zero in the case where messages
@@ -327,11 +324,7 @@ void EventsExecutor::HandleSubscriptionReady(py::handle subscription, size_t num
327324
py::object msg_info = _rclpy_sub.take_message(msg_type, raw);
328325
if (!msg_info.is_none()) {
329326
try {
330-
if (callback_type == message_only) {
331-
callback(py::cast<py::tuple>(msg_info)[0]);
332-
} else {
333-
callback(msg_info);
334-
}
327+
callback(py::cast<py::tuple>(msg_info)[0]);
335328
} catch (const py::error_already_set & e) {
336329
HandleCallbackExceptionInNodeEntity(e, subscription, "subscriptions");
337330
throw;
@@ -348,28 +341,13 @@ void EventsExecutor::HandleAddedTimer(py::handle timer) {timers_manager_.AddTime
348341

349342
void EventsExecutor::HandleRemovedTimer(py::handle timer) {timers_manager_.RemoveTimer(timer);}
350343

351-
void EventsExecutor::HandleTimerReady(py::handle timer, const rcl_timer_call_info_t & info)
344+
void EventsExecutor::HandleTimerReady(py::handle timer)
352345
{
353346
py::gil_scoped_acquire gil_acquire;
354347
py::object callback = timer.attr("callback");
355-
// We need to distinguish callbacks that want a TimerInfo object from those that don't.
356-
// Executor._take_timer() actually checks if an argument has type markup expecting a TypeInfo
357-
// object. This seems like overkill, vs just checking if it wants an argument at all?
358-
py::object py_info;
359-
if (py::len(inspect_signature_(callback).attr("parameters").attr("values")()) > 0) {
360-
using py::literals::operator""_a;
361-
py_info = rclpy_timer_timer_info_(
362-
"expected_call_time"_a = info.expected_call_time,
363-
"actual_call_time"_a = info.actual_call_time,
364-
"clock_type"_a = timer.attr("clock").attr("clock_type"));
365-
}
366348
py::object result;
367349
try {
368-
if (py_info) {
369-
result = callback(py_info);
370-
} else {
371-
result = callback();
372-
}
350+
result = callback();
373351
} catch (const py::error_already_set & e) {
374352
HandleCallbackExceptionInNodeEntity(e, timer, "timers");
375353
throw;
@@ -394,12 +372,12 @@ void EventsExecutor::HandleAddedClient(py::handle client)
394372
const auto cb = std::bind(&EventsExecutor::HandleClientReady, this, client, pl::_1);
395373
if (
396374
RCL_RET_OK != rcl_client_set_on_new_response_callback(
397-
rcl_ptr, RclEventCallbackTrampoline,
398-
rcl_callback_manager_.MakeCallback(rcl_ptr, cb, with)))
375+
rcl_ptr, RclEventCallbackTrampoline,
376+
rcl_callback_manager_.MakeCallback(rcl_ptr, cb, with)))
399377
{
400378
throw std::runtime_error(
401-
std::string("Failed to set the on new response callback for client: ") +
402-
rcl_get_error_string().str);
379+
std::string("Failed to set the on new response callback for client: ") +
380+
rcl_get_error_string().str);
403381
}
404382
}
405383

@@ -409,8 +387,8 @@ void EventsExecutor::HandleRemovedClient(py::handle client)
409387
const rcl_client_t * rcl_ptr = py::cast<const Client &>(handle).rcl_ptr();
410388
if (RCL_RET_OK != rcl_client_set_on_new_response_callback(rcl_ptr, nullptr, nullptr)) {
411389
throw std::runtime_error(
412-
std::string("Failed to clear the on new response callback for client: ") +
413-
rcl_get_error_string().str);
390+
std::string("Failed to clear the on new response callback for client: ") +
391+
rcl_get_error_string().str);
414392
}
415393
rcl_callback_manager_.RemoveCallback(rcl_ptr);
416394
}
@@ -466,12 +444,12 @@ void EventsExecutor::HandleAddedService(py::handle service)
466444
const auto cb = std::bind(&EventsExecutor::HandleServiceReady, this, service, pl::_1);
467445
if (
468446
RCL_RET_OK != rcl_service_set_on_new_request_callback(
469-
rcl_ptr, RclEventCallbackTrampoline,
470-
rcl_callback_manager_.MakeCallback(rcl_ptr, cb, with)))
447+
rcl_ptr, RclEventCallbackTrampoline,
448+
rcl_callback_manager_.MakeCallback(rcl_ptr, cb, with)))
471449
{
472450
throw std::runtime_error(
473-
std::string("Failed to set the on new request callback for service: ") +
474-
rcl_get_error_string().str);
451+
std::string("Failed to set the on new request callback for service: ") +
452+
rcl_get_error_string().str);
475453
}
476454
}
477455

@@ -481,8 +459,8 @@ void EventsExecutor::HandleRemovedService(py::handle service)
481459
const rcl_service_t * rcl_ptr = py::cast<const Service &>(handle).rcl_ptr();
482460
if (RCL_RET_OK != rcl_service_set_on_new_request_callback(rcl_ptr, nullptr, nullptr)) {
483461
throw std::runtime_error(
484-
std::string("Failed to clear the on new request callback for service: ") +
485-
rcl_get_error_string().str);
462+
std::string("Failed to clear the on new request callback for service: ") +
463+
rcl_get_error_string().str);
486464
}
487465
rcl_callback_manager_.RemoveCallback(rcl_ptr);
488466
}
@@ -559,12 +537,12 @@ void EventsExecutor::HandleAddedWaitable(py::handle waitable)
559537
pl::_1);
560538
if (
561539
RCL_RET_OK != rcl_subscription_set_on_new_message_callback(
562-
rcl_sub, RclEventCallbackTrampoline,
563-
rcl_callback_manager_.MakeCallback(rcl_sub, cb, with_waitable)))
540+
rcl_sub, RclEventCallbackTrampoline,
541+
rcl_callback_manager_.MakeCallback(rcl_sub, cb, with_waitable)))
564542
{
565543
throw std::runtime_error(
566-
std::string("Failed to set the on new message callback for Waitable subscription: ") +
567-
rcl_get_error_string().str);
544+
std::string("Failed to set the on new message callback for Waitable subscription: ") +
545+
rcl_get_error_string().str);
568546
}
569547
}
570548
for (size_t i = 0; i < rcl_waitset->size_of_timers; ++i) {
@@ -590,12 +568,12 @@ void EventsExecutor::HandleAddedWaitable(py::handle waitable)
590568
with_waitset, pl::_1);
591569
if (
592570
RCL_RET_OK != rcl_client_set_on_new_response_callback(
593-
rcl_client, RclEventCallbackTrampoline,
594-
rcl_callback_manager_.MakeCallback(rcl_client, cb, with_waitable)))
571+
rcl_client, RclEventCallbackTrampoline,
572+
rcl_callback_manager_.MakeCallback(rcl_client, cb, with_waitable)))
595573
{
596574
throw std::runtime_error(
597-
std::string("Failed to set the on new response callback for Waitable client: ") +
598-
rcl_get_error_string().str);
575+
std::string("Failed to set the on new response callback for Waitable client: ") +
576+
rcl_get_error_string().str);
599577
}
600578
}
601579
for (size_t i = 0; i < rcl_waitset->size_of_services; ++i) {
@@ -607,12 +585,12 @@ void EventsExecutor::HandleAddedWaitable(py::handle waitable)
607585
with_waitset, pl::_1);
608586
if (
609587
RCL_RET_OK != rcl_service_set_on_new_request_callback(
610-
rcl_service, RclEventCallbackTrampoline,
611-
rcl_callback_manager_.MakeCallback(rcl_service, cb, with_waitable)))
588+
rcl_service, RclEventCallbackTrampoline,
589+
rcl_callback_manager_.MakeCallback(rcl_service, cb, with_waitable)))
612590
{
613591
throw std::runtime_error(
614-
std::string("Failed to set the on new request callback for Waitable service: ") +
615-
rcl_get_error_string().str);
592+
std::string("Failed to set the on new request callback for Waitable service: ") +
593+
rcl_get_error_string().str);
616594
}
617595
}
618596
for (size_t i = 0; i < rcl_waitset->size_of_events; ++i) {
@@ -624,12 +602,12 @@ void EventsExecutor::HandleAddedWaitable(py::handle waitable)
624602
with_waitset, pl::_1);
625603
if (
626604
RCL_RET_OK != rcl_event_set_callback(
627-
rcl_event, RclEventCallbackTrampoline,
628-
rcl_callback_manager_.MakeCallback(rcl_event, cb, with_waitable)))
605+
rcl_event, RclEventCallbackTrampoline,
606+
rcl_callback_manager_.MakeCallback(rcl_event, cb, with_waitable)))
629607
{
630608
throw std::runtime_error(
631-
std::string("Failed to set the callback for Waitable event: ") +
632-
rcl_get_error_string().str);
609+
std::string("Failed to set the callback for Waitable event: ") +
610+
rcl_get_error_string().str);
633611
}
634612
}
635613

@@ -648,9 +626,10 @@ void EventsExecutor::HandleRemovedWaitable(py::handle waitable)
648626
for (const rcl_subscription_t * const rcl_sub : sub_entities.subscriptions) {
649627
if (RCL_RET_OK != rcl_subscription_set_on_new_message_callback(rcl_sub, nullptr, nullptr)) {
650628
throw std::runtime_error(
651-
std::string("Failed to clear the on new message "
652-
"callback for Waitable subscription: ") +
653-
rcl_get_error_string().str);
629+
std::string(
630+
"Failed to clear the on new message "
631+
"callback for Waitable subscription: ") +
632+
rcl_get_error_string().str);
654633
}
655634
rcl_callback_manager_.RemoveCallback(rcl_sub);
656635
}
@@ -660,26 +639,28 @@ void EventsExecutor::HandleRemovedWaitable(py::handle waitable)
660639
for (const rcl_client_t * const rcl_client : sub_entities.clients) {
661640
if (RCL_RET_OK != rcl_client_set_on_new_response_callback(rcl_client, nullptr, nullptr)) {
662641
throw std::runtime_error(
663-
std::string("Failed to clear the on new response "
664-
"callback for Waitable client: ") +
665-
rcl_get_error_string().str);
642+
std::string(
643+
"Failed to clear the on new response "
644+
"callback for Waitable client: ") +
645+
rcl_get_error_string().str);
666646
}
667647
rcl_callback_manager_.RemoveCallback(rcl_client);
668648
}
669649
for (const rcl_service_t * const rcl_service : sub_entities.services) {
670650
if (RCL_RET_OK != rcl_service_set_on_new_request_callback(rcl_service, nullptr, nullptr)) {
671651
throw std::runtime_error(
672-
std::string("Failed to clear the on new request "
673-
"callback for Waitable service: ") +
674-
rcl_get_error_string().str);
652+
std::string(
653+
"Failed to clear the on new request "
654+
"callback for Waitable service: ") +
655+
rcl_get_error_string().str);
675656
}
676657
rcl_callback_manager_.RemoveCallback(rcl_service);
677658
}
678659
for (const rcl_event_t * const rcl_event : sub_entities.events) {
679660
if (RCL_RET_OK != rcl_event_set_callback(rcl_event, nullptr, nullptr)) {
680661
throw std::runtime_error(
681-
std::string("Failed to clear the callback for Waitable event: ") +
682-
rcl_get_error_string().str);
662+
std::string("Failed to clear the callback for Waitable event: ") +
663+
rcl_get_error_string().str);
683664
}
684665
rcl_callback_manager_.RemoveCallback(rcl_event);
685666
}
@@ -904,23 +885,23 @@ void define_events_executor(py::object module)
904885
.def("get_nodes", &EventsExecutor::get_nodes)
905886
.def("spin", [](EventsExecutor & exec) {exec.spin();})
906887
.def(
907-
"spin_once",
888+
"spin_once",
908889
[](EventsExecutor & exec, std::optional<double> timeout_sec) {
909890
exec.spin(timeout_sec, true);
910-
},
911-
py::arg("timeout_sec") = py::none())
891+
},
892+
py::arg("timeout_sec") = py::none())
912893
.def(
913-
"spin_until_future_complete",
894+
"spin_until_future_complete",
914895
[](EventsExecutor & exec, py::handle future, std::optional<double> timeout_sec) {
915896
exec.spin_until_future_complete(future, timeout_sec);
916-
},
917-
py::arg("future"), py::arg("timeout_sec") = py::none())
897+
},
898+
py::arg("future"), py::arg("timeout_sec") = py::none())
918899
.def(
919-
"spin_once_until_future_complete",
900+
"spin_once_until_future_complete",
920901
[](EventsExecutor & exec, py::handle future, std::optional<double> timeout_sec) {
921902
exec.spin_until_future_complete(future, timeout_sec, true);
922-
},
923-
py::arg("future"), py::arg("timeout_sec") = py::none())
903+
},
904+
py::arg("future"), py::arg("timeout_sec") = py::none())
924905
.def("__enter__", &EventsExecutor::enter)
925906
.def("__exit__", &EventsExecutor::exit);
926907
}

rclpy/src/rclpy/events_executor/events_executor.hpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ class EventsExecutor
108108

109109
void HandleAddedTimer(pybind11::handle);
110110
void HandleRemovedTimer(pybind11::handle);
111-
void HandleTimerReady(pybind11::handle, const rcl_timer_call_info_t &);
111+
void HandleTimerReady(pybind11::handle);
112112

113113
void HandleAddedClient(pybind11::handle);
114114
void HandleRemovedClient(pybind11::handle);
@@ -167,7 +167,6 @@ class EventsExecutor
167167
const pybind11::object inspect_iscoroutine_;
168168
const pybind11::object inspect_signature_;
169169
const pybind11::object rclpy_task_;
170-
const pybind11::object rclpy_timer_timer_info_;
171170

172171
EventsQueue events_queue_;
173172
ScopedSignalCallback signal_callback_;

rclpy/src/rclpy/events_executor/rcl_support.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,17 @@ const void * RclCallbackManager::MakeCallback(
5353
}
5454
CbEntry new_entry;
5555
new_entry.cb =
56-
std::make_unique<std::function<void(size_t)>>([this, callback, key](size_t number_of_events) {
57-
events_queue_->Enqueue([this, callback, key, number_of_events]() {
56+
std::make_unique<std::function<void(size_t)>>(
57+
[this, callback, key](size_t number_of_events) {
58+
events_queue_->Enqueue(
59+
[this, callback, key, number_of_events]() {
5860
if (!owned_cbs_.count(key)) {
5961
// This callback has been removed, just drop it as the objects it may want to touch may
6062
// no longer exist.
6163
return;
6264
}
6365
callback(number_of_events);
64-
});
66+
});
6567
});
6668
new_entry.with = with;
6769
const void * ret = new_entry.cb.get();

0 commit comments

Comments
 (0)