Skip to content

Commit 0d50b6c

Browse files
author
iRobot ROS
authored
Merge pull request #50 from mauropasse/mauro/group-callback-data
Rework executor callback data
2 parents 00666c1 + df37d31 commit 0d50b6c

23 files changed

+283
-129
lines changed

rclcpp/include/rclcpp/client.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,9 @@ class ClientBase
158158

159159
RCLCPP_PUBLIC
160160
void
161-
set_events_executor_callback(
162-
rclcpp::executors::EventsExecutor * executor,
163-
rmw_listener_callback_t executor_callback) const;
161+
set_listener_callback(
162+
rmw_listener_callback_t callback,
163+
const void * user_data) const;
164164

165165
protected:
166166
RCLCPP_DISABLE_COPY(ClientBase)

rclcpp/include/rclcpp/executors/events_executor.hpp

+10-9
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,14 @@
2222

2323
#include "rclcpp/executor.hpp"
2424
#include "rclcpp/executors/events_executor_entities_collector.hpp"
25+
#include "rclcpp/executors/events_executor_event_types.hpp"
2526
#include "rclcpp/executors/events_executor_notify_waitable.hpp"
2627
#include "rclcpp/executors/timers_manager.hpp"
2728
#include "rclcpp/experimental/buffers/events_queue.hpp"
2829
#include "rclcpp/experimental/buffers/simple_events_queue.hpp"
2930
#include "rclcpp/node.hpp"
3031

31-
#include "rmw/listener_event_types.h"
32+
#include "rmw/listener_callback_type.h"
3233

3334
namespace rclcpp
3435
{
@@ -214,20 +215,20 @@ class EventsExecutor : public rclcpp::Executor
214215
// This function is called by the DDS entities when an event happened,
215216
// like a subscription receiving a message.
216217
static void
217-
push_event(void * executor_ptr, rmw_listener_event_t event)
218+
push_event(const void * event_data)
218219
{
219-
// Check if the executor pointer is not valid
220-
if (!executor_ptr) {
221-
throw std::runtime_error("The executor pointer is not valid.");
220+
if (!event_data) {
221+
throw std::runtime_error("Executor event data not valid.");
222222
}
223223

224-
auto this_executor = static_cast<executors::EventsExecutor *>(executor_ptr);
224+
auto data = static_cast<const executors::EventsExecutorCallbackData *>(event_data);
225+
226+
executors::EventsExecutor * this_executor = data->executor;
225227

226228
// Event queue mutex scope
227229
{
228230
std::unique_lock<std::mutex> lock(this_executor->push_mutex_);
229-
230-
this_executor->events_queue_->push(event);
231+
this_executor->events_queue_->push(data->event);
231232
}
232233
// Notify that the event queue has some events in it.
233234
this_executor->events_queue_cv_.notify_one();
@@ -236,7 +237,7 @@ class EventsExecutor : public rclcpp::Executor
236237
// Execute a single event
237238
RCLCPP_PUBLIC
238239
void
239-
execute_event(const rmw_listener_event_t & event);
240+
execute_event(const ExecutorEvent & event);
240241

241242
// Queue where entities can push events
242243
rclcpp::experimental::buffers::EventsQueue::SharedPtr events_queue_;

rclcpp/include/rclcpp/executors/events_executor_entities_collector.hpp

+18
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <vector>
2323

2424
#include "rclcpp/executors/event_waitable.hpp"
25+
#include "rclcpp/executors/events_executor_event_types.hpp"
2526
#include "rclcpp/executors/timers_manager.hpp"
2627
#include "rclcpp/node_interfaces/node_base_interface.hpp"
2728

@@ -213,6 +214,12 @@ class EventsExecutorEntitiesCollector final
213214
void
214215
unset_guard_condition_callback(const rcl_guard_condition_t * guard_condition);
215216

217+
void
218+
remove_callback_data(void * entity_id, ExecutorEventType type);
219+
220+
const EventsExecutorCallbackData *
221+
get_callback_data(void * entity_id, ExecutorEventType type);
222+
216223
/// Return true if the node belongs to the collector
217224
/**
218225
* \param[in] group_ptr a node base interface shared pointer
@@ -262,6 +269,17 @@ class EventsExecutorEntitiesCollector final
262269
EventsExecutor * associated_executor_ = nullptr;
263270
/// Instance of the timers manager used by the associated executor
264271
TimersManager::SharedPtr timers_manager_;
272+
273+
/// Callback data objects mapped to the number of listeners sharing the same object.
274+
/// When no more listeners use the object, it can be removed from the map.
275+
/// For example, the entities collector holds every node's guard condition, which
276+
/// share the same EventsExecutorCallbackData object ptr to use as their callback arg:
277+
/// cb_data_object = {executor_ptr, entities_collector_ptr, WAITABLE_EVENT};
278+
/// Node1->gc(&cb_data_object)
279+
/// Node2->gc(&cb_data_object)
280+
/// So the maps has: (cb_data_object, 2)
281+
/// When both nodes are removed (counter = 0), the cb_data_object can be destroyed.
282+
std::unordered_map<EventsExecutorCallbackData, size_t, KeyHasher> callback_data_map_;
265283
};
266284

267285
} // namespace executors
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// Copyright 2021 Open Source Robotics Foundation, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#ifndef RCLCPP__EXECUTORS__EVENTS_EXECUTOR_EVENT_TYPES_HPP_
16+
#define RCLCPP__EXECUTORS__EVENTS_EXECUTOR_EVENT_TYPES_HPP_
17+
18+
namespace rclcpp
19+
{
20+
namespace executors
21+
{
22+
23+
// forward declaration of EventsExecutor to avoid circular dependency
24+
class EventsExecutor;
25+
26+
enum ExecutorEventType
27+
{
28+
SUBSCRIPTION_EVENT,
29+
SERVICE_EVENT,
30+
CLIENT_EVENT,
31+
WAITABLE_EVENT
32+
};
33+
34+
struct ExecutorEvent
35+
{
36+
const void * entity_id;
37+
ExecutorEventType type;
38+
};
39+
40+
// The EventsExecutorCallbackData struct is what the listeners
41+
// will use as argument when calling their callbacks from the
42+
// RMW implementation. The listeners get a (void *) of this struct,
43+
// and the executor is in charge to cast it back and use the data.
44+
struct EventsExecutorCallbackData
45+
{
46+
EventsExecutorCallbackData(
47+
EventsExecutor * _executor,
48+
ExecutorEvent _event)
49+
{
50+
executor = _executor;
51+
event = _event;
52+
}
53+
54+
// Equal operator
55+
bool operator==(const EventsExecutorCallbackData & other) const
56+
{
57+
return (event.entity_id == other.event.entity_id);
58+
}
59+
60+
// Struct members
61+
EventsExecutor * executor;
62+
ExecutorEvent event;
63+
};
64+
65+
// To be able to use std::unordered_map with an EventsExecutorCallbackData
66+
// as key, we need a hasher. We use the entity ID as hash, since it is
67+
// unique for each EventsExecutorCallbackData object.
68+
struct KeyHasher
69+
{
70+
size_t operator()(const EventsExecutorCallbackData & key) const
71+
{
72+
return std::hash<const void *>()(key.event.entity_id);
73+
}
74+
};
75+
76+
} // namespace executors
77+
} // namespace rclcpp
78+
79+
#endif // RCLCPP__EXECUTORS__EVENTS_EXECUTOR_EVENT_TYPES_HPP_

rclcpp/include/rclcpp/executors/events_executor_notify_waitable.hpp

+5-6
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,15 @@ class EventsExecutorNotifyWaitable final : public EventWaitable
6060

6161
RCLCPP_PUBLIC
6262
void
63-
set_events_executor_callback(
64-
rclcpp::executors::EventsExecutor * executor,
65-
rmw_listener_callback_t executor_callback) const override
63+
set_listener_callback(
64+
rmw_listener_callback_t callback,
65+
const void * user_data) const override
6666
{
6767
for (auto gc : notify_guard_conditions_) {
6868
rcl_ret_t ret = rcl_guard_condition_set_listener_callback(
6969
gc,
70-
executor_callback,
71-
executor,
72-
this,
70+
callback,
71+
user_data,
7372
false);
7473

7574
if (RCL_RET_OK != ret) {

rclcpp/include/rclcpp/experimental/buffers/events_queue.hpp

+6-6
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
#include "rclcpp/macros.hpp"
2121
#include "rclcpp/visibility_control.hpp"
2222

23-
#include "rmw/listener_event_types.h"
23+
#include "rclcpp/executors/events_executor_event_types.hpp"
2424

2525
namespace rclcpp
2626
{
@@ -31,7 +31,7 @@ namespace buffers
3131

3232
/**
3333
* @brief This abstract class can be used to implement different types of queues
34-
* where `rmw_listener_event_t` can be stored.
34+
* where `ExecutorEvent` can be stored.
3535
* The derived classes should choose which underlying container to use and
3636
* the strategy for pushing and popping events.
3737
* For example a queue implementation may be bounded or unbounded and have
@@ -42,7 +42,7 @@ namespace buffers
4242
class EventsQueue
4343
{
4444
public:
45-
RCLCPP_SMART_PTR_DEFINITIONS(EventsQueue)
45+
RCLCPP_SMART_PTR_ALIASES_ONLY(EventsQueue)
4646

4747
/**
4848
* @brief Destruct the object.
@@ -57,7 +57,7 @@ class EventsQueue
5757
RCLCPP_PUBLIC
5858
virtual
5959
void
60-
push(const rmw_listener_event_t & event) = 0;
60+
push(const rclcpp::executors::ExecutorEvent & event) = 0;
6161

6262
/**
6363
* @brief removes front element from the queue.
@@ -73,7 +73,7 @@ class EventsQueue
7373
*/
7474
RCLCPP_PUBLIC
7575
virtual
76-
rmw_listener_event_t
76+
rclcpp::executors::ExecutorEvent
7777
front() const = 0;
7878

7979
/**
@@ -108,7 +108,7 @@ class EventsQueue
108108
*/
109109
RCLCPP_PUBLIC
110110
virtual
111-
std::queue<rmw_listener_event_t>
111+
std::queue<rclcpp::executors::ExecutorEvent>
112112
pop_all_events() = 0;
113113
};
114114

rclcpp/include/rclcpp/experimental/buffers/simple_events_queue.hpp

+6-6
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ class SimpleEventsQueue : public EventsQueue
4646
RCLCPP_PUBLIC
4747
virtual
4848
void
49-
push(const rmw_listener_event_t & event)
49+
push(const rclcpp::executors::ExecutorEvent & event)
5050
{
5151
event_queue_.push(event);
5252
}
@@ -68,7 +68,7 @@ class SimpleEventsQueue : public EventsQueue
6868
*/
6969
RCLCPP_PUBLIC
7070
virtual
71-
rmw_listener_event_t
71+
rclcpp::executors::ExecutorEvent
7272
front() const
7373
{
7474
return event_queue_.front();
@@ -107,7 +107,7 @@ class SimpleEventsQueue : public EventsQueue
107107
init()
108108
{
109109
// Make sure the queue is empty when we start
110-
std::queue<rmw_listener_event_t> local_queue;
110+
std::queue<rclcpp::executors::ExecutorEvent> local_queue;
111111
std::swap(event_queue_, local_queue);
112112
}
113113

@@ -118,16 +118,16 @@ class SimpleEventsQueue : public EventsQueue
118118
*/
119119
RCLCPP_PUBLIC
120120
virtual
121-
std::queue<rmw_listener_event_t>
121+
std::queue<rclcpp::executors::ExecutorEvent>
122122
pop_all_events()
123123
{
124-
std::queue<rmw_listener_event_t> local_queue;
124+
std::queue<rclcpp::executors::ExecutorEvent> local_queue;
125125
std::swap(event_queue_, local_queue);
126126
return local_queue;
127127
}
128128

129129
private:
130-
std::queue<rmw_listener_event_t> event_queue_;
130+
std::queue<rclcpp::executors::ExecutorEvent> event_queue_;
131131
};
132132

133133
} // namespace buffers

rclcpp/include/rclcpp/experimental/subscription_intra_process_base.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ class SubscriptionIntraProcessBase : public rclcpp::Waitable
7676

7777
RCLCPP_PUBLIC
7878
void
79-
set_events_executor_callback(
80-
rclcpp::executors::EventsExecutor * executor,
81-
rmw_listener_callback_t executor_callback) const override;
79+
set_listener_callback(
80+
rmw_listener_callback_t callback,
81+
const void * user_data) const override;
8282

8383
protected:
8484
std::recursive_mutex reentrant_mutex_;

rclcpp/include/rclcpp/qos_event.hpp

+3-4
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,11 @@ class QOSEventHandlerBase : public Waitable
107107
bool
108108
is_ready(rcl_wait_set_t * wait_set) override;
109109

110-
/// Set EventsExecutor's callback
111110
RCLCPP_PUBLIC
112111
void
113-
set_events_executor_callback(
114-
rclcpp::executors::EventsExecutor * executor,
115-
rmw_listener_callback_t executor_callback) const override;
112+
set_listener_callback(
113+
rmw_listener_callback_t callback,
114+
const void * user_data) const override;
116115

117116
protected:
118117
rcl_event_t event_handle_;

rclcpp/include/rclcpp/service.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,9 @@ class ServiceBase
128128

129129
RCLCPP_PUBLIC
130130
void
131-
set_events_executor_callback(
132-
rclcpp::executors::EventsExecutor * executor,
133-
rmw_listener_callback_t executor_callback) const;
131+
set_listener_callback(
132+
rmw_listener_callback_t callback,
133+
const void * user_data) const;
134134

135135
protected:
136136
RCLCPP_DISABLE_COPY(ServiceBase)

rclcpp/include/rclcpp/subscription_base.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -271,9 +271,9 @@ class SubscriptionBase : public std::enable_shared_from_this<SubscriptionBase>
271271

272272
RCLCPP_PUBLIC
273273
void
274-
set_events_executor_callback(
275-
rclcpp::executors::EventsExecutor * executor,
276-
rmw_listener_callback_t executor_callback) const;
274+
set_listener_callback(
275+
rmw_listener_callback_t callback,
276+
const void * user_data) const;
277277

278278
protected:
279279
template<typename EventCallbackT>

rclcpp/include/rclcpp/waitable.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -211,9 +211,9 @@ class Waitable
211211
RCLCPP_PUBLIC
212212
virtual
213213
void
214-
set_events_executor_callback(
215-
rclcpp::executors::EventsExecutor * executor,
216-
rmw_listener_callback_t executor_callback) const;
214+
set_listener_callback(
215+
rmw_listener_callback_t callback,
216+
const void * user_data) const;
217217

218218
private:
219219
std::atomic<bool> in_use_by_wait_set_{false};

rclcpp/src/rclcpp/client.cpp

+6-7
Original file line numberDiff line numberDiff line change
@@ -200,17 +200,16 @@ ClientBase::exchange_in_use_by_wait_set_state(bool in_use_state)
200200
}
201201

202202
void
203-
ClientBase::set_events_executor_callback(
204-
rclcpp::executors::EventsExecutor * executor,
205-
rmw_listener_callback_t executor_callback) const
203+
ClientBase::set_listener_callback(
204+
rmw_listener_callback_t callback,
205+
const void * user_data) const
206206
{
207207
rcl_ret_t ret = rcl_client_set_listener_callback(
208208
client_handle_.get(),
209-
executor_callback,
210-
executor,
211-
this);
209+
callback,
210+
user_data);
212211

213212
if (RCL_RET_OK != ret) {
214-
throw std::runtime_error("Couldn't set the EventsExecutor's callback to client");
213+
throw std::runtime_error("Couldn't set listener callback to client");
215214
}
216215
}

0 commit comments

Comments
 (0)