-
Notifications
You must be signed in to change notification settings - Fork 7.7k
feat(wire): std::functional Wire slave callback functions #11582
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
base: master
Are you sure you want to change the base?
Conversation
👋 Hello SuGlider, we appreciate your contribution to this project! 📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more. 🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project. Click to see more instructions ...
Review and merge process you can expect ...
|
Test Results 76 files 76 suites 15m 4s ⏱️ Results for commit f2fce44. ♻️ This comment has been updated with latest results. |
Memory usage test (comparing PR against master branch)The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.
Click to expand the detailed deltas report [usage change in BYTES]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the Wire library to support std::function
–based callbacks for I2C slave mode, enabling the use of lambdas and captured contexts.
- Replaces raw function pointers in
TwoWire
andHardwareI2C
withstd::function
foronRequest
andonReceive
- Updates constructors, method signatures, and default initializations to use
std::function
- Adds new example sketch, CI config, and documentation updates demonstrating the functional callback API
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
libraries/Wire/src/Wire.h | Swapped C-style callback pointers for std::function members |
libraries/Wire/src/Wire.cpp | Updated constructor initializers and method implementations |
libraries/Wire/examples/WireSlaveFunctionalCallback/ci.json | Added CI requirement for I2C slave support |
libraries/Wire/examples/WireSlaveFunctionalCallback/WireSlaveFunctionalCallback.ino | New example illustrating functional callbacks |
docs/en/api/i2c.rst | Revised API docs for onReceive /onRequest to show std::function |
cores/esp32/HardwareI2C.h | Updated base class to use std::function and included <functional> |
Comments suppressed due to low confidence (4)
docs/en/api/i2c.rst:440
- [nitpick] The example uses
Wire.printf
, butTwoWire
may not supportprintf
across all cores; consider usingWire.print
/Wire.write
or ensureprintf
is available.
Wire.printf("Response #%d", counter++);
libraries/Wire/src/Wire.cpp:599
- There are no unit tests verifying that the new
std::function
–based callbacks are invoked correctly; consider adding tests for bothonReceive
andonRequest
paths.
void TwoWire::onReceive(const std::function<void(int)>& function) {
libraries/Wire/src/Wire.h:76
- The header uses
std::function
but<functional>
is not included; add#include <functional>
at the top of this file to ensure compilation on all platforms.
std::function<void()> user_onRequest;
libraries/Wire/src/Wire.h:76
- [nitpick] Using
std::function
for callbacks can introduce dynamic allocations and overhead on constrained embedded targets; consider documenting this cost or offering a lightweight alternative.
std::function<void()> user_onRequest;
Description of Change
Modify Wire.h and Wire.cpp to allow the Wire Slave callback functions to be declared as std::functional.
Adds example and changes the documentation.
Tests scenarios
CI + ESP32/ESP32-C3/ESP32-C6/ESP32-S3 WireSlave.ino example
Related links
Closes #11497