CuriousScientist0 / ADS1256

An Arduino-compatible library for the ADS1256 24-bit analog-to-digital converter.
MIT License
31 stars 11 forks source link

Remove DRDY ISR #14

Closed BenjaminPelletier closed 2 months ago

BenjaminPelletier commented 2 months ago

This PR fixes #12 by simply observing DRDY directly rather than relying on an intermediate variable set in an ISR.

I have an alternate branch fix-drdy on my fork that keeps the ISR and tries to prevent DRDY_value from being set true before the falling DRDY signal could possibly indicate actual readiness following the previous operation. But, it seems unnecessary to worry about the complexity of an ISR since we're blocking on the falling edge any way.

This PR would not be a good idea if DRDY fell and then rose again in less time than the while loop took to check again, but this seems unlikely to me (and empirically seems not to be the case). This PR would also not be a good idea if we wanted to recognize a fall in DRDY prior to starting waitForDRDY() even if DRDY had since returned high, but I don't think we want to do this either.

The demo sketch in #12 is fixed by this PR. However, I am still seeing sporadic bad measurements even after this PR (added #15), though this behavior appears to be identical with that of my fix-drdy branch, so I don't think this fix is the cause of that problem.

CuriousScientist0 commented 2 months ago

Actively waiting for the DRDY seems to work better and it results in more stable behaviour.

However, one thing still kind of disturbs me. If you look at table 14, the MUX throughput rates, each sampling frequency has a corresponding throughput rate. This is 1/t19 frequency, or in other words, the period between a MUX-MUX instruction.

Now the thing is that I am not really able to get these numbers exactly, whatever I do. My DRDY signal looks great and the readings are also great, but the speed is not 100% there yet.

Screenshot 2024-09-09 232314_c

At 30 ksps, I was able to achieve 4149 Hz. But it should be 4374 Hz. I stripped down everything in the Arduino code. I just run the cycleDifferential() function in a for loop of 10000 iterations. No printing, no serial reading, nothing.

Have you checked your MUX-MUX timings?

BenjaminPelletier commented 2 months ago

I'm finding that I'm only able to achieve a little over 1kHz for cycles (expecting close 4.374kHz as you mention) / 4 kHz for samples. You know the datasheet better than I do, so are you saying we only expect 4374 calls to cycleDifferential() to complete in one second? I thought (perhaps without basis) that ~4kHz number was referring to the max rate of sampling all channels, so I was expecting roughly 17496 calls to cycleDifferential() to complete in one second. If we're only expecting 4374 calls per second, then I think I'm close to that, but probably a little shy like you.

4149 versus 4374 doesn't bother me that much as we're probably not being perfectly efficient with the call. I would expect to only achieve 4374 if our code did not block the ADC critical path at all, and there are lots of unobvious opportunities to accidentally block for a short time. For my application, I probably wouldn't hunt down a 5% reduction in sample rate from maximum, but I would hunt down a 75% reduction in sample rate :)

CuriousScientist0 commented 2 months ago

If you print on the serial port while reading and cycling the 4 differential channels, then the sampling rate will be much worse. Especially, because you print in a human-readable format. If you buffer the data, for example, read the 4 channels, then print, then read the 4 channels, you get almost 4374 Hz.

With your code, where we directly read the DRDY pin I get a very stable DRDY pattern: Screenshot 2024-09-10 212017_c

The four groups with the longer high DRDY signals are the readings of the four differential channels with MUXing. That's where the throughput should be 1/t19 = 4374 Hz. Then, between two of these groups, you can see a gap with a denser and shorter DRDY periods. The MCU sends the buffered data to the serial during this time. The time between the last MISO bit and the first MOSI is about 1.23 ms. I also measured the DRDY period and it is 33.33 us or 30 kHz, as it should be.

Screenshot 2024-09-10 212036_c

In the picture above, you can see the two distinct actions a bit better. Sampling and data transfer via serial. I think it is not too bad with buffering.

Screenshot 2024-09-10 212122_c And then above, I measured the time between two DRDY falling edges when the ADS1256 is running the acquisition and doing the multiplexing. 4240 Hz. Not exactly the datasheet value, but at this point I am not pushing it more.

One last question: Can I put your name in the files where I acknowledge your help?

I am planning to merge your modification and after carefully checking the rest of the functions, I will create a "v1.3" of the library and push it to the official Arduino library so other people who use my library through the library manager will get the bugfix.