feat(p4): Add method to set the pins for SDIO to WiFi chip #11513
Conversation
👋 Hello me-no-dev, 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 ...
|
me-no-dev
added
Status: Review needed
There was a problem hiding this comment.
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
setPinsmethod to allow custom SDIO pin assignment before WiFi initialization. - Introduce
sdio_pin_config_tstruct with default initialization and refactorwifiHostedInitto 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 | Declared setPins method under CONFIG_ESP_WIFI_REMOTE_ENABLED. |
| libraries/WiFi/src/WiFiGeneric.cpp | Added sdio_pin_config_t, default pin setup, setPins logic, refactored wifiHostedInit, and updated comments. |
Comments suppressed due to low confidence (2)
libraries/WiFi/src/WiFiGeneric.h:87
- The public method
setPinslacks 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
setPinsmethod 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) {
Test Results 76 files 76 suites 13m 8s ⏱️ Results for commit 0b3e248. ♻️ 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]
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Can't wait to try this! |
This pull request introduces enhancements to the SDIO pin configuration mechanism in the
WiFiGenericlibrary, allowing dynamic configuration of pins for external ESP MCU connections. The changes primarily involve adding a newsetPinsmethod, refactoring pin initialization logic, and updating associated comments for clarity.SDIO Pin Configuration Enhancements:
Addition of
setPinsMethod: Introduced a static methodsetPinsinWiFiGenericClassto 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.cppandlibraries/WiFi/src/WiFiGeneric.h, [1] [2]Refactoring Pin Initialization Logic: Replaced hardcoded pin configurations with a new
sdio_pin_config_tstructure, 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 thewifiHostedInitfunction to reflect the use of the newsdio_pin_configstructure instead of hardcoded pin configurations. (libraries/WiFi/src/WiFiGeneric.cpp, libraries/WiFi/src/WiFiGeneric.cppL282-R326)Fixes: #11404