Read multiple channels using the same SPI transaction.#11
Read multiple channels using the same SPI transaction.#11RobTillaart merged 4 commits intoRobTillaart:masterfrom
Conversation
|
is this toggle needed? as you are still in the same SPI transaction, I assume it is not. // Toggle CS only if there are more channels to read
if (i < numChannels - 1) {
digitalWrite(_select, LOW);
digitalWrite(_select, HIGH);
}(update) |
|
What is the performance loss, if |
Thanks for this enhancement. Good opportunity to improve your skills :) First thing is please add in the demo sketch the ratio of improvement. ==> time ( Have to review the code. (pending post, too much windows open error :) |
|
Something is pretty interesting in your log. Some faster SPI speeds are slower in |
We need to toggle the CS line, if not it won't read the next channel :(. |
I'll have a look and I'll run a few tests. |
but as it is required for all channels you want to read, so it could be done unconditionally. |
Thanks. I'm mainly a Python/PHP dev, and I only used C/C++ with Arduino and ESP32. I'll try to update it later today, if not it will be during the week. Yes, I'm using ESP32 WROVER. |
|
variant for the toggle, at begin and at end of loop. void MCP_ADC::readADCMultiple(uint8_t channels[], uint8_t numChannels, int16_t readings[]) {
if (_hwSPI) {
mySPI->beginTransaction(_spi_settings);
}
for (uint8_t i = 0; i < numChannels; i++) {
digitalWrite(_select, LOW);
uint8_t data[3] = {0, 0, 0};
uint8_t bytes = buildRequest(channels[i], true, data);
if (_hwSPI) {
for (uint8_t b = 0; b < bytes; b++) {
data[b] = mySPI->transfer(data[b]);
}
} else {
for (uint8_t b = 0; b < bytes; b++) {
data[b] = swSPI_transfer(data[b]);
}
}
if (bytes == 2) {
readings[i] = ((256 * data[0] + data[1]) & _maxValue);
} else {
readings[i] = ((256 * data[1] + data[2]) & _maxValue);
}
digitalWrite(_select, HIGH);
}
}
if (_hwSPI) {
mySPI->endTransaction();
}
} |
This is required for all except for the last one, mainly because after the last channel read there's nothing else there to read. But, if we'll find out that there's no improvement in the read speed even if we have the extra toggle, we could just remove it. |
This should work. I don't have that much experience with SPI. Do you think it will be safe to end the spi transaction while the CS line is off? |
That thought underlies the code so yes. the communication is during the transfer function. |
|
Thanks for looking at this :) Just did some tests and I added the new results. Personally, I think that anyone who needs to read more than one channel should use .analogReadMultiple(). It doesn't make sense to read a channel one by one and add extra delay due to the SPI transition start/end. |
Makes sense, e.g when the timestamps should be as simultaneous as possible. The user might have arguments to do individual calls, design/architecture readability erc, so keeping that option is important. The |
|
The performance numbers are impressive, I expect it will show gain for all cases. |
That's actually a really good point.
I'll have a look and I'll try to update it |
|
This is quite interesting. Based on this we can see that's not actually the reading time that's slow, but the SPI start/end communication. |
examples/MCP_ADC_performance/MCP3208_performance/MCP3208_performance.ino
Outdated
Show resolved
Hide resolved
(note: the figures above are platform dependent, still they are very informative) |
examples/MCP_ADC_performance/MCP3208_performance/MCP3208_performance.ino
Show resolved
Hide resolved
examples/MCP_ADC_performance/MCP3208_performance/MCP3208_performance.ino
Show resolved
Hide resolved
|
Created a table of the comparison |
|
Think the code looks good to merge into master. The remaining remarks are minor and I'll need to check if there is other "low hanging fruit" I can include in the release. |
Thanks. I'll try to go thru the comments and do any changes. |
|
Raw output performance test on AVR / UNO |
|
@alx-uta |
|
Apparently I did not think and test well enough, the single flag for differentialRead got lost. int16_t MCP_ADC::readADC(uint8_t channel, bool single)
{
if (channel >= _channels) return 0;
int16_t reading[1];
MCP_ADC::readADCMultiple(&channel, 1, reading);
return reading[0];
} |
The results are close, compared to the esp32 where we can see the performance gain.
That sounds good, I guess you'll also need to update the examples.
Ah, sorry about that, I missed it when I moved the code. |
|
0.2.0 released |
Thanks again for this great library. |
I had a bit of free time, and I decided to see if I could read the data faster.
This was mainly because I have 2x MCP23S17 and 2x MCP3208 on my PCB.
It looks like if we'll read all the channels inside the same SPI transaction the data read is faster in most of the cases.
If possible, can you please have a look and see if this is something you'd add to the library?
I'll be more than happy to do any adjustments, mainly because my programming skills in C/C++ are a bit limited.
Test results: