-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
Improve the 16 pins interface #23
Conversation
Interesting idea extra parameter for writeReg an readReg is not needed as _hwspi is available in both functions.. Extra peam might be cause of slower swspi. Need to check increase of codebase, esp AVR |
Drawback is that it "breaks" the architectures of keeping the SPI commands only in lowest level. From design point I would create a (private) writeReg16 an a readReg16 function. |
SW SPIIn the proposal the SW SPI got (1) two extra calls and (2) extra parameter which slows things down. bool MCP23S17::pinMode16(uint16_t value)
{
MCP23S17::beginSpiTransaction(); <<<<<<<<< when using SW SPI this call is added.
writeReg(MCP23S17_DDR_A, value >> 8, !_hwSPI); <<< extra parameter
writeReg(MCP23S17_DDR_B, value & 0xFF, !_hwSPI); <<< extra parameter
MCP23S17::endSpiTransaction(); <<<<<<<<< when using SW SPI this call is added.
_error = MCP23S17_OK;
return true;
} |
beginTransaction()The proposed solution spreads The possible gain is still very interesting, and we need to investigate how it can be coded while keeping the architecture clean. |
This is a good idea, and it should keep the code cleaner. I'll do those changes and see how it look :) |
You need to merge the latest master branch of the library first as I have merged the other PR and created a new release |
Note: the registers for the 16 bit calls are always |
Sounds good, thx. I'll let you know when is ready. |
6ea5398
to
409e621
Compare
I'm not exactly sure what loop, sorry. Also, I think that I can see a possible problem with the Software SPI. Now, instead of a single |
Test results at 10 MHz using Software SPI: Using master:
Using the current branch:
Test results at 10 MHz using Hardware SPI:
Using the current branch:
|
7728c15
to
5f34da3
Compare
Sorry, I'm confused, just had a long day. as in:
Is the problem the fact that we have |
I created a develop branch with the readReg16() and writeReg16() side by side with the 8 bit low level functions. The code is more within my architectural ideas, all low level SPI hidden for the user. Tests here shows reasonable gains for my ESP32 for the 16 bit calls (tested without hardware) |
Sounds good, Thanks |
Replaced by PR #24 |
- optimize performance 16 bit interface - add readReg16() + writeReg16() - based upon ideas from Alex Uta (PR #23) - update performance test sketch (multi speeds in one run) - update readme.md - minor edits
After running multiple tests and trying to figure out what else I could improve for the data read I realised that we could avoid multiple SPI channels when reading or writing all the ports.
I'm not exactly sure why the software SPI went up because nothing really changed for that one.
It looks like now 4 MHz can compete with the old 10MHz.