feat(wire): std::functional Wire slave callback functions #11582
+184
−17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SuGlider
added
Type: Feature request
👋 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. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
SuGlider
commented
Jul 10, 2025
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
SuGlider
changed the title
SuGlider
changed the title
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.
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
TwoWireandHardwareI2Cwithstd::functionforonRequestandonReceive - 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, butTwoWiremay not supportprintfacross all cores; consider usingWire.print/Wire.writeor ensureprintfis 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 bothonReceiveandonRequestpaths.
void TwoWire::onReceive(const std::function<void(int)>& function) {
libraries/Wire/src/Wire.h:76
- The header uses
std::functionbut<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::functionfor 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;
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 using new WireSlaveFunctionalCallback.ino example
Related links
Closes #11497