-
Notifications
You must be signed in to change notification settings - Fork 7.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Regression: NetworkEvents::onEvent()
can't register function calbacks via lambda expressions
#10365
Closed
1 task done
Labels
Status: Awaiting triage
Issue is waiting for triage
Comments
NetworkEvents::onEvent()
can't register function calbacks via labda expressionsNetworkEvents::onEvent()
can't register function calbacks via lambda expressions
vortigont
added a commit
to vortigont/arduino-esp32
that referenced
this issue
Sep 26, 2024
…llbacks 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
me-no-dev
pushed a commit
that referenced
this issue
Oct 24, 2024
…10376) * lib Network: add cpp syntax to structs * [Network] deprecate NetworkEvents::removeEvent() for std::function callbacks removing event callback via std::function pointer does not work as expected for lambdas (issue #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 * [Network] hide event task under private member of NetworkEvents class prevent checkForEvent loop to be callable from outside the task's thread * refactor(NetworkEvents) code polishing and comments - rename NetworkEvents::cbEventList as private member NetworkEvents_cbEventList - NetworkEvents::getStatusBits() add const qualifier - turn statics into constexpr - add indexes to enum::arduino_event_id_t to make events indexing consistent for SOCs with/without WiFi also leave some index gaps to minimize renumbering on adding new events - add doxygen help to NetworkEvents:: methods - declare NetworkEvents::eventName() as static, that could be used without creating class scope - potential mem leak in postEvent * refactor(NetworkEvents) add (optional) mutex lock for container operations provide thread safety for dual core SoCs since std::mutex brings additional componetns of libstdc++ lib it impacts resulting image size significantly (around 50k) Might be enabled on-demand if thread-safety is required * ci(pre-commit): Apply automatic fixes * fix(spelling): Fix spelling mistakes --------- Co-authored-by: Rodrigo Garcia <[email protected]> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Lucas Saavedra Vaz <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Board
ESP32 Dev module
Device Description
plain module
Hardware Configuration
plain module
Version
latest master (checkout manually)
IDE Name
PlatformIO
Operating System
Ubuntu Linux
Flash frequency
defajult
PSRAM enabled
no
Upload speed
115200
Description
Network lib has a regression when updated from 3.0.4 to 3.0.5
The method NetworkEvents::onEvent
can no longer register two or more functional callbacks for Network event handling when those are lambda expressions.
In 2f89026 additional code was added that tries to check if an attempt is made to register a duplicate callback function based on determined address of a callback
arduino-esp32/libraries/Network/src/NetworkEvents.cpp
Line 218 in 7018cd1
This template code does not work properly for lambdas and returns 0 (nullptr) always.
arduino-esp32/libraries/Network/src/NetworkEvents.cpp
Lines 153 to 160 in 7018cd1
Do we really need that duplicate callbacks check? It's up to user to register callback, even if those are called twice there is no big deal in that. While that address-pointer comparison magic is tend to errors.
Sketch
Debug Message
Other Steps to Reproduce
No response
I have checked existing issues, online documentation and the Troubleshooting Guide
The text was updated successfully, but these errors were encountered: