- Notifications
You must be signed in to change notification settings - Fork7.7k
feat(p4): Add method to set the pins for SDIO to WiFi chip#11513
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
github-actionsbot commentedJun 24, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
👋Hello me-no-dev, we appreciate your contribution to this project! 📘 Please review the project'sContributions Guide for key guidelines on code, documentation, testing, and more. 🖊️ Please also make sure you haveread and signed theContributor License Agreement for this project. Click to see more instructions ...
Review and merge process you can expect ...
|
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
Enhance SDIO pin flexibility by introducing a dynamic configuration method and centralizing pin setup using a new data structure, replacing hardcoded values.
- Add
setPins
method to allow custom SDIO pin assignment before WiFi initialization. - Introduce
sdio_pin_config_t
struct with default initialization and refactorwifiHostedInit
to use it. - Update inline comments to reflect the new pin configuration approach.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
libraries/WiFi/src/WiFiGeneric.h | DeclaredsetPins method underCONFIG_ESP_WIFI_REMOTE_ENABLED . |
libraries/WiFi/src/WiFiGeneric.cpp | Addedsdio_pin_config_t , default pin setup,setPins logic, refactoredwifiHostedInit , and updated comments. |
Comments suppressed due to low confidence (2)
libraries/WiFi/src/WiFiGeneric.h:87
- The public method
setPins
lacks a documentation comment. Consider adding a brief Doxygen-style comment describing each parameter, the return value, and when it should be called to improve clarity for users.
static bool setPins(int8_t clk, int8_t cmd, int8_t d0, int8_t d1, int8_t d2, int8_t d3, int8_t rst);
libraries/WiFi/src/WiFiGeneric.cpp:280
- The new
setPins
method implements validation and configuration logic but isn't covered by any tests. Adding unit or integration tests would help ensure correct behavior and prevent regressions.
bool WiFiGenericClass::setPins(int8_t clk, int8_t cmd, int8_t d0, int8_t d1, int8_t d2, int8_t d3, int8_t rst) {
Uh oh!
There was an error while loading.Please reload this page.
github-actionsbot commentedJun 24, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Test Results 76 files 76 suites 13m 7s ⏱️ Results for commit462efa9. ♻️ 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]
|
Westcott1 commentedJun 24, 2025
Can't wait to try this! |
Uh oh!
There was an error while loading.Please reload this page.
2592a7b
intomasterUh oh!
There was an error while loading.Please reload this page.
This pull request introduces enhancements to the SDIO pin configuration mechanism in the
WiFiGeneric
library, allowing dynamic configuration of pins for external ESP MCU connections. The changes primarily involve adding a newsetPins
method, refactoring pin initialization logic, and updating associated comments for clarity.SDIO Pin Configuration Enhancements:
Addition of
setPins
Method: Introduced a static methodsetPins
inWiFiGenericClass
to allow dynamic configuration of SDIO pins. This method validates the provided pin values and ensures they are set before WiFi initialization. (libraries/WiFi/src/WiFiGeneric.cpp
andlibraries/WiFi/src/WiFiGeneric.h
,[1][2]Refactoring Pin Initialization Logic: Replaced hardcoded pin configurations with a new
sdio_pin_config_t
structure, enabling centralized and flexible management of SDIO pin settings. (libraries/WiFi/src/WiFiGeneric.cpp
,libraries/WiFi/src/WiFiGeneric.cppR249-R309)Code Cleanup and Comments:
wifiHostedInit
: Refined comments in thewifiHostedInit
function to reflect the use of the newsdio_pin_config
structure instead of hardcoded pin configurations. (libraries/WiFi/src/WiFiGeneric.cpp
,libraries/WiFi/src/WiFiGeneric.cppL282-R326)Fixes:#11404