Skip to content
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

Speed up AnalogIn.value significantly. #484

Closed
dhalbert opened this issue Dec 15, 2017 · 6 comments
Closed

Speed up AnalogIn.value significantly. #484

dhalbert opened this issue Dec 15, 2017 · 6 comments
Assignees
Milestone

Comments

@dhalbert
Copy link
Collaborator

@tdicola has measured that AnalogIn.value takes about 10msecs per sample, which is very long. Currently it takes 16-bit samples with the 12-bit ADC using the builtin decimation and oversampling. This means it takes 256x the normal time: decimation and oversampling is 4^n.

In addition, there's a clock divisor which is set to 128 and could be set to 32 now, gaining another factor of 4. And finally, two readings are taken for each reading, in case the reference voltage has changed beforehand. Atmel/MicroChip recommends this and I saw that it was necessary when flipping back and forth between using the internal temperature reference and the 1V reference. We could remember the last reference used to avoid the double reading most of the time.

Right now .value returns an unsigned 16-bit value. I don't want to change the API in an incompatible way for 2.2.0, but we can consider API changes or additions for 3.0. I'll open a separate issue for that.

Currently microcontroller.cpu.temperature gets a 14-bit result using oversampling and decimation.

@tdicola points out at 10msecs, we're doing 100 samples/second, which for, say, audio sampling gives us a Nyquist limit of 50Hz. If I fix the prescaler (x4) and don't double-sample (x2), the limit rises to 400Hz. Dropping a bit of precision gives another x4 each time. So 15-bit is 1.6kHz, 14-bit is 6.4kHz, 13-bit is 25.6kHz, and 12-bit is 102.4kHz. (Not counting overhead.) Tony proposes reducing to 12-bit to allow at least audio sampling. For API compatibility, I will *16 that to normalize it back to 16-bit.

This is OK with me for 2.2.0. It's an easy change. @tannewt @ladyada your comments welcome.

@dhalbert dhalbert added this to the 2.x milestone Dec 15, 2017
@dhalbert dhalbert self-assigned this Dec 15, 2017
@dhalbert
Copy link
Collaborator Author

I changed to prescaler division 32. Using the test program below (a slightly sped up variant of @tdicola's), I get these measurements. The "none" measurement is not using the ADC at all, just returning a fake value immediately from the low-level routine.

bits of resolution sample time samples per sec num times ADC is read per call
none 0.17ms 5800 0 (measuring overhead only)
12 0.29ms 3450 2
12 0.28ms 3500 1
13 0.33ms 3030 2
14 0.44ms 2272 2
15 0.89ms 1123 2
16 2.7ms 370 2

So I did not remove the double read. It's complicated to remember the state, and turns out it doesn't save much at 12 or 13 bits resolution. I could do 13 bits resolution without much extra time, but maybe 12 is good enough for now.

Due to the measured overhead, doing audio sampling in a Python loop doesn't look really practical, at least on the M0. Better to use a higher-level API that reads samples in to a buffer.

Test program:

import time
import board
import analogio

adc = analogio.AnalogIn(board.A1)

SAMPLES = 256
while True:
    start = time.monotonic()
    for _ in range(SAMPLES):
        adc_val = adc.value
    duration_secs = time.monotonic() - start
    sample_secs = duration_secs / SAMPLES
    rate = 1/sample_secs
    print('Time per sample: {}ms rate:{} Total time: {}ms'.format(sample_secs*1000, rate, duration_secs*1000))

@ladyada
Copy link
Member

ladyada commented Dec 15, 2017

yah the intention w/16-bit value is that 16-bits is greater than the resolution of microcontroller's ADC resolution (which is 10-bit or 12-bit...maybe rarely 16-bits but as you can see that is terribly slow). and its an int so cleaner, clearer and easier to compare/do math with.

default, the ADC should read whatever is the native rez (12-bit for M0/M4) then pads 0's for the bottom 4 bits

for reading audio, even arduino cannot sample analogRead() very fast, we would always set up a timer. when many samples are needed fast, its easiest to set up a DMA channel + timer and request multiple values like we do with the mic, and get back an array of 16-bit values. otherwise there is just no guarantee that you will get all the data at fixed intervals :)

@tdicola
Copy link

tdicola commented Dec 15, 2017

Just to make sure we don't lose it, this was one thing discussed in the email thread.

From Dan: "So, in the short run, I can speed it up by x4 immediately without losing accuracy. Do you have some feeling for what to do for 2.2? For 3.0 we can do whatever. In chat Scott mentioned returning a float voltage value. We can add ".voltage" later, say in 3.0. I can also add some optional args that set the precision and/or scale the output if it's integer."

My thoughts for ADC, similar to voltage but expose the raw and the normalized or voltage value as a float (but be careful not everything will give you reference voltage):

"- ADC .value = the value from the hardware with no modification. 12-bit, 16-bit, 24-bit, etc. In addition add some concept of .value_size or range that tells you the max value so a user can know what range to expect. This is a relatively low impact change for users now as they just need to change their code from a 16-bit to 12-bit range assumption (or better to change to read the .value_size or similar property that tells them the range).

  • Add an ADC .normalized or similar attribute that's the value scaled to fall within a 0 to 1.0 floating point range. This would be completely resilient to different size ADCs and give a universal attribute users can code against. The down side is that float math is a little bit slower, but that's a trade-off users can make (if it's too slow fall back to .value and deal with the fixed range).

  • Optionally add an ADC .voltage value too. I like it but I'm not sure every ADC or processor will let you read the reference range voltage (definitely not most of our ADC breakouts), so it's unsafe to assume this is universal I think."

@tdicola
Copy link

tdicola commented Dec 15, 2017

Also to clarify, Dan mentioned "Tony proposes reducing to 12-bit to allow at least audio sampling." Actually I proposed speeding it up for all uses, not just audio. Right now for example we can't reliably catch a fast knock or signal when sampling a piezo. 10-20ms period or 50hz sample rate is just too slow unfortunately and frequently misses any quick impulse, like a sudden spike from a knock. If using timer & DMA is the desired way to go then should we open a new issue to add these for users to manipulate and use in their code for 3.0 perhaps too?

@ladyada
Copy link
Member

ladyada commented Dec 15, 2017

yep we will do both things!

  1. reducing M0 and M4 to 12-bit (no oversampling!) and pad LSBs with 0's - that is the intended interface and it's in 2.2 now, you can pull and try it!

  2. add a blocking DMA interface for ADC reading at high freq rates, in 4.x (i'll create an issue)

for getting float/voltages for ADC, we can add helpers in SimpleIO! :)

@ladyada ladyada closed this as completed Dec 15, 2017
@dhalbert
Copy link
Collaborator Author

12-bit and prescaler divisor fixed by #485 for 2.2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants