feat(spi): Add return values to SPI begin + comments on SPI buses #11477
+8
−7
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
👋 Hello P-R-O-C-H-Y, 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 12m 42s ⏱️ Results for commit ab93cbd. ♻️ This comment has been updated with latest results. |
lucasssvaz
approved these changes
Jun 17, 2025
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 updates the SPI initialization API by changing SPIClass::begin to return a bool value instead of void, improving error handling and compatibility with newer ESP32 variants.
- Changes the SPI.begin method signature in SPI.h and its implementation in SPI.cpp to return a bool.
- Updates error-handling returns in SPI.cpp to reflect initialization success or failure.
- Adjusts SPI bus macro definitions in esp32-hal-spi.h to accommodate board-specific mappings.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| libraries/SPI/src/SPI.h | Updated the begin method signature to return a bool. |
| libraries/SPI/src/SPI.cpp | Modified begin implementation to return true/false based on initialization. |
| cores/esp32/esp32-hal-spi.h | Adjusted macro definitions for FSPI and HSPI to match new board mappings. |
Comments suppressed due to low confidence (2)
libraries/SPI/src/SPI.h:64
- Consider updating the documentation for the begin() method to clearly state what the bool return value indicates (e.g., success if already initialized or newly initialized, and failure on errors).
bool begin(int8_t sck = -1, int8_t miso = -1, int8_t mosi = -1, int8_t ss = -1);
libraries/SPI/src/SPI.cpp:69
- A comment here explaining that returning true when _spi is already initialized is intended behavior could help clarify the API's design and usage.
}
me-no-dev
approved these changes
Jun 17, 2025
me-no-dev
added
Status: Pending Merge
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
This pull request is modifying the
SPIClass::beginmethod to return aboolfor better error handling. These updates enhance compatibility with newer ESP32 variants and provide clearer feedback on initialization success.Tests scenarios
Related links
Closes #8192