Skip to content

Commit 98287ac

Browse files
vortigontvortigont
committed
[Network] deprecate NetworkEvents::removeEvent() for std::function callbacks
removing event callback via std::function pointer does not work as expected for lambdas (issue espressif#10365) here mark NetworkEvents::removeEvent(NetworkEventFuncCb cbEvent, arduino_event_id_t event = ARDUINO_EVENT_MAX) as deprecated in favor of removing by callback's id for NetworkEvents::onEvent remove checking for dublicate event handler, this does not work for lambdas too remove NetworkEvents::find methods as unnecessary move cbEventList container inside the class declare NetworkEventCbList_t as a cpp struct with constructor, allows using std::vector.emplace() when adding new items to container optimize NetworkEvents::remove() calls to use erase-remove idiom for std::vector
1 parent 5a163ac commit 98287ac

File tree

2 files changed

+42
-181
lines changed

2 files changed

+42
-181
lines changed

‎libraries/Network/src/NetworkEvents.cpp

+16-177
Original file line numberDiff line numberDiff line change
@@ -8,28 +8,6 @@
88
#include "esp_task.h"
99
#include "esp32-hal.h"
1010

11-
/**
12-
* @brief an object holds callback's definitions:
13-
* - callback id
14-
* - callback function pointers
15-
* - binded event id
16-
*
17-
*/
18-
struct NetworkEventCbList_t {
19-
static network_event_handle_t current_id;
20-
network_event_handle_t id;
21-
NetworkEventCb cb;
22-
NetworkEventFuncCb fcb;
23-
NetworkEventSysCb scb;
24-
arduino_event_id_t event;
25-
26-
NetworkEventCbList_t() : id(current_id++), cb(NULL), fcb(NULL), scb(NULL), event(ARDUINO_EVENT_NONE) {}
27-
};
28-
// define initial id's value
29-
network_event_handle_t NetworkEventCbList_t::current_id = 1;
30-
31-
// arduino dont like std::vectors move static here
32-
static std::vector<NetworkEventCbList_t> cbEventList;
3311

3412
static void _network_event_task(void *arg) {
3513
for (;;) {
@@ -142,22 +120,6 @@ void NetworkEvents::checkForEvent() {
142120
free(event);
143121
}
144122

145-
uint32_t NetworkEvents::findEvent(NetworkEventCb cbEvent, arduino_event_id_t event) {
146-
uint32_t i;
147-
148-
if (!cbEvent) {
149-
return cbEventList.size();
150-
}
151-
152-
for (i = 0; i < cbEventList.size(); i++) {
153-
NetworkEventCbList_t entry = cbEventList[i];
154-
if (entry.cb == cbEvent && entry.event == event) {
155-
break;
156-
}
157-
}
158-
return i;
159-
}
160-
161123
template<typename T, typename... U> static size_t getStdFunctionAddress(std::function<T(U...)> f) {
162124
typedef T(fnType)(U...);
163125
fnType **fnPointer = f.template target<fnType *>();
@@ -167,209 +129,86 @@ template<typename T, typename... U> static size_t getStdFunctionAddress(std::fun
167129
return (size_t)fnPointer;
168130
}
169131

170-
uint32_t NetworkEvents::findEvent(NetworkEventFuncCb cbEvent, arduino_event_id_t event) {
171-
uint32_t i;
172-
173-
if (!cbEvent) {
174-
return cbEventList.size();
175-
}
176-
177-
for (i = 0; i < cbEventList.size(); i++) {
178-
NetworkEventCbList_t entry = cbEventList[i];
179-
if (getStdFunctionAddress(entry.fcb) == getStdFunctionAddress(cbEvent) && entry.event == event) {
180-
break;
181-
}
182-
}
183-
return i;
184-
}
185-
186-
uint32_t NetworkEvents::findEvent(NetworkEventSysCb cbEvent, arduino_event_id_t event) {
187-
uint32_t i;
188-
189-
if (!cbEvent) {
190-
return cbEventList.size();
191-
}
192-
193-
for (i = 0; i < cbEventList.size(); i++) {
194-
NetworkEventCbList_t entry = cbEventList[i];
195-
if (entry.scb == cbEvent && entry.event == event) {
196-
break;
197-
}
198-
}
199-
return i;
200-
}
201-
202132
network_event_handle_t NetworkEvents::onEvent(NetworkEventCb cbEvent, arduino_event_id_t event) {
203133
if (!cbEvent) {
204134
return 0;
205135
}
206136

207-
if (findEvent(cbEvent, event) < cbEventList.size()) {
208-
log_w("Attempt to add duplicate event handler!");
209-
return 0;
210-
}
211-
212-
NetworkEventCbList_t newEventHandler;
213-
newEventHandler.cb = cbEvent;
214-
newEventHandler.fcb = NULL;
215-
newEventHandler.scb = NULL;
216-
newEventHandler.event = event;
217-
cbEventList.push_back(newEventHandler);
218-
return newEventHandler.id;
137+
cbEventList.emplace_back(++_current_id, cbEvent, nullptr, nullptr, event);
138+
return cbEventList.back().id;
219139
}
220140

221141
network_event_handle_t NetworkEvents::onEvent(NetworkEventFuncCb cbEvent, arduino_event_id_t event) {
222142
if (!cbEvent) {
223143
return 0;
224144
}
225145

226-
if (findEvent(cbEvent, event) < cbEventList.size()) {
227-
log_w("Attempt to add duplicate event handler!");
228-
return 0;
229-
}
230-
231-
NetworkEventCbList_t newEventHandler;
232-
newEventHandler.cb = NULL;
233-
newEventHandler.fcb = cbEvent;
234-
newEventHandler.scb = NULL;
235-
newEventHandler.event = event;
236-
cbEventList.push_back(newEventHandler);
237-
return newEventHandler.id;
146+
cbEventList.emplace_back(++_current_id, nullptr, cbEvent, nullptr, event);
147+
return cbEventList.back().id;
238148
}
239149

240150
network_event_handle_t NetworkEvents::onEvent(NetworkEventSysCb cbEvent, arduino_event_id_t event) {
241151
if (!cbEvent) {
242152
return 0;
243153
}
244154

245-
if (findEvent(cbEvent, event) < cbEventList.size()) {
246-
log_w("Attempt to add duplicate event handler!");
247-
return 0;
248-
}
249-
250-
NetworkEventCbList_t newEventHandler;
251-
newEventHandler.cb = NULL;
252-
newEventHandler.fcb = NULL;
253-
newEventHandler.scb = cbEvent;
254-
newEventHandler.event = event;
255-
cbEventList.push_back(newEventHandler);
256-
return newEventHandler.id;
155+
cbEventList.emplace_back(++_current_id, nullptr, nullptr, cbEvent, event);
156+
return cbEventList.back().id;
257157
}
258158

259159
network_event_handle_t NetworkEvents::onSysEvent(NetworkEventCb cbEvent, arduino_event_id_t event) {
260160
if (!cbEvent) {
261161
return 0;
262162
}
263163

264-
if (findEvent(cbEvent, event) < cbEventList.size()) {
265-
log_w("Attempt to add duplicate event handler!");
266-
return 0;
267-
}
268-
269-
NetworkEventCbList_t newEventHandler;
270-
newEventHandler.cb = cbEvent;
271-
newEventHandler.fcb = NULL;
272-
newEventHandler.scb = NULL;
273-
newEventHandler.event = event;
274-
cbEventList.insert(cbEventList.begin(), newEventHandler);
275-
return newEventHandler.id;
164+
cbEventList.emplace(cbEventList.begin(), ++_current_id, cbEvent, nullptr, nullptr, event);
165+
return cbEventList.front().id;
276166
}
277167

278168
network_event_handle_t NetworkEvents::onSysEvent(NetworkEventFuncCb cbEvent, arduino_event_id_t event) {
279169
if (!cbEvent) {
280170
return 0;
281171
}
282172

283-
if (findEvent(cbEvent, event) < cbEventList.size()) {
284-
log_w("Attempt to add duplicate event handler!");
285-
return 0;
286-
}
287-
288-
NetworkEventCbList_t newEventHandler;
289-
newEventHandler.cb = NULL;
290-
newEventHandler.fcb = cbEvent;
291-
newEventHandler.scb = NULL;
292-
newEventHandler.event = event;
293-
cbEventList.insert(cbEventList.begin(), newEventHandler);
294-
return newEventHandler.id;
173+
cbEventList.emplace(cbEventList.begin(), ++_current_id, nullptr, cbEvent, nullptr, event);
174+
return cbEventList.front().id;
295175
}
296176

297177
network_event_handle_t NetworkEvents::onSysEvent(NetworkEventSysCb cbEvent, arduino_event_id_t event) {
298178
if (!cbEvent) {
299179
return 0;
300180
}
301181

302-
if (findEvent(cbEvent, event) < cbEventList.size()) {
303-
log_w("Attempt to add duplicate event handler!");
304-
return 0;
305-
}
306-
307-
NetworkEventCbList_t newEventHandler;
308-
newEventHandler.cb = NULL;
309-
newEventHandler.fcb = NULL;
310-
newEventHandler.scb = cbEvent;
311-
newEventHandler.event = event;
312-
cbEventList.insert(cbEventList.begin(), newEventHandler);
313-
return newEventHandler.id;
182+
cbEventList.emplace(cbEventList.begin(), ++_current_id, nullptr, nullptr, cbEvent, event);
183+
return cbEventList.front().id;
314184
}
315185

316186
void NetworkEvents::removeEvent(NetworkEventCb cbEvent, arduino_event_id_t event) {
317-
uint32_t i;
318-
319187
if (!cbEvent) {
320188
return;
321189
}
322190

323-
i = findEvent(cbEvent, event);
324-
if (i >= cbEventList.size()) {
325-
log_w("Event handler not found!");
326-
return;
327-
}
328-
329-
cbEventList.erase(cbEventList.begin() + i);
191+
cbEventList.erase(std::remove_if(cbEventList.begin(), cbEventList.end(), [cbEvent, event](const NetworkEventCbList_t& e) { return e.cb == cbEvent && e.event == event; }), cbEventList.end());
330192
}
331193

332194
void NetworkEvents::removeEvent(NetworkEventFuncCb cbEvent, arduino_event_id_t event) {
333-
uint32_t i;
334-
335195
if (!cbEvent) {
336196
return;
337197
}
338198

339-
i = findEvent(cbEvent, event);
340-
if (i >= cbEventList.size()) {
341-
log_w("Event handler not found!");
342-
return;
343-
}
344-
345-
cbEventList.erase(cbEventList.begin() + i);
199+
cbEventList.erase(std::remove_if(cbEventList.begin(), cbEventList.end(), [cbEvent, event](const NetworkEventCbList_t& e) { return getStdFunctionAddress(e.fcb) == getStdFunctionAddress(cbEvent) && e.event == event; }), cbEventList.end());
346200
}
347201

348202
void NetworkEvents::removeEvent(NetworkEventSysCb cbEvent, arduino_event_id_t event) {
349-
uint32_t i;
350-
351203
if (!cbEvent) {
352204
return;
353205
}
354206

355-
i = findEvent(cbEvent, event);
356-
if (i >= cbEventList.size()) {
357-
log_w("Event handler not found!");
358-
return;
359-
}
360-
361-
cbEventList.erase(cbEventList.begin() + i);
207+
cbEventList.erase(std::remove_if(cbEventList.begin(), cbEventList.end(), [cbEvent, event](const NetworkEventCbList_t& e) { return e.scb == cbEvent && e.event == event; }), cbEventList.end());
362208
}
363209

364210
void NetworkEvents::removeEvent(network_event_handle_t id) {
365-
for (uint32_t i = 0; i < cbEventList.size(); i++) {
366-
NetworkEventCbList_t entry = cbEventList[i];
367-
if (entry.id == id) {
368-
cbEventList.erase(cbEventList.begin() + i);
369-
return;
370-
}
371-
}
372-
log_w("Event handler not found!");
211+
cbEventList.erase(std::remove_if(cbEventList.begin(), cbEventList.end(), [id](const NetworkEventCbList_t& e) { return e.id == id; }), cbEventList.end());
373212
}
374213

375214
int NetworkEvents::setStatusBits(int bits) {

‎libraries/Network/src/NetworkEvents.h

+26-4
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ class NetworkEvents {
134134
network_event_handle_t onEvent(NetworkEventFuncCb cbEvent, arduino_event_id_t event = ARDUINO_EVENT_MAX);
135135
network_event_handle_t onEvent(NetworkEventSysCb cbEvent, arduino_event_id_t event = ARDUINO_EVENT_MAX);
136136
void removeEvent(NetworkEventCb cbEvent, arduino_event_id_t event = ARDUINO_EVENT_MAX);
137-
void removeEvent(NetworkEventFuncCb cbEvent, arduino_event_id_t event = ARDUINO_EVENT_MAX);
137+
void removeEvent(NetworkEventFuncCb cbEvent, arduino_event_id_t event = ARDUINO_EVENT_MAX) __attribute__((deprecated("removing functional callbacks via pointer is deprecated, use removeEvent(network_event_handle_t event_handle) instead")));
138138
void removeEvent(NetworkEventSysCb cbEvent, arduino_event_id_t event = ARDUINO_EVENT_MAX);
139139
void removeEvent(network_event_handle_t event_handle);
140140

@@ -159,15 +159,37 @@ class NetworkEvents {
159159

160160
protected:
161161
bool initNetworkEvents();
162-
uint32_t findEvent(NetworkEventCb cbEvent, arduino_event_id_t event);
163-
uint32_t findEvent(NetworkEventFuncCb cbEvent, arduino_event_id_t event);
164-
uint32_t findEvent(NetworkEventSysCb cbEvent, arduino_event_id_t event);
165162
network_event_handle_t onSysEvent(NetworkEventCb cbEvent, arduino_event_id_t event = ARDUINO_EVENT_MAX);
166163
network_event_handle_t onSysEvent(NetworkEventFuncCb cbEvent, arduino_event_id_t event = ARDUINO_EVENT_MAX);
167164
network_event_handle_t onSysEvent(NetworkEventSysCb cbEvent, arduino_event_id_t event = ARDUINO_EVENT_MAX);
168165

169166
private:
167+
/**
168+
* @brief an object holds callback's definitions:
169+
* - callback id
170+
* - callback function pointers
171+
* - binded event id
172+
*
173+
*/
174+
struct NetworkEventCbList_t {
175+
network_event_handle_t id;
176+
NetworkEventCb cb;
177+
NetworkEventFuncCb fcb;
178+
NetworkEventSysCb scb;
179+
arduino_event_id_t event;
180+
181+
explicit NetworkEventCbList_t(network_event_handle_t id, NetworkEventCb cb = nullptr, NetworkEventFuncCb fcb = nullptr, NetworkEventSysCb scb = nullptr, arduino_event_id_t event = ARDUINO_EVENT_MAX) :
182+
id(id), cb(cb), fcb(fcb), scb(scb), event(event) {}
183+
};
184+
185+
// define initial id's value
186+
network_event_handle_t _current_id{0};
187+
170188
EventGroupHandle_t _arduino_event_group;
171189
QueueHandle_t _arduino_event_queue;
172190
TaskHandle_t _arduino_event_task_handle;
191+
192+
// registred events callbacks containter
193+
std::vector<NetworkEventCbList_t> cbEventList;
194+
173195
};

0 commit comments

Comments
 (0)