adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.04k stars 1.19k forks source link

Espressif ADC is not accurate, has a deadzone and differs per cpu #8754

Open bill88t opened 8 months ago

bill88t commented 8 months ago

CircuitPython version

It's exactly the same on 8.x and 9.x so it's not related to idf5

Code/REPL

import analogio, board, time; iadc = analogio.AnalogIn(board.any_adc_pin)
while True:
    print(iadc.value)
    time.sleep(0.1)

Behavior

Connect any potentiometer. First pin to 3v3, middle pin to the adc pin of the board, and the third pin to GND.

As we spin it around it should change it value. The maximum should be 65535 and the minimum 0. With a breadboard and jumper wires the values will of course not be exactly accurate, but they should be close.

However with S2 we can see the value fluctuates between 556 and 51157. With S3 it goes from 0 to 61543.

Meanwhile with rp2040 the value can go from 0 to 65535 just fine.

I tested with 2 esp32-s2 boards, one s3 board, two rp2040 boards and 2 different style linear potentiometers.

Additional information

Also it should be noted that there is an amazingly big deadzone it stays at 51157 on s2 (about 20% of my slider). On s3 it's smaller, but can still be felt.

carson-coder commented 8 months ago

This seems more like a hardware error. Maybe we could add some sort of calibration function

dhalbert commented 8 months ago

The ADC on the ESP32-S2 is not great. There is an offset at zero, and it is not rail-to-rail. They started adding some factory-set calibration data at some point.

bill88t commented 8 months ago

All modern espressif mcus are already calibrated at factory and their settings are stored in efuse. I did check the code, CP does load the calibs properly.

The ADC on the ESP32-S2 is not great.

But then s3?

dhalbert commented 8 months ago

In ESP-IDF, for the ESP-S2, ADC_CALI_SCHEME_LINE_FITTING_SUPPORTED is defined. For ESP32-S3, ADC_CALI_SCHEME_CURVE_FITTING_SUPPORTED is defined. So the S3 has a more sophisticated calibration scheme.

anecdata commented 8 months ago

The older API docs show a curve (ESP32-S2) that is reasonably linear (when calibrated), except that it doesn't go to zero, and it doesn't go to 3.3V (iirc 2.6V when using 11dB attenuation): https://docs.espressif.com/projects/esp-idf/en/release-v4.4/esp32s2/api-reference/peripherals/adc.html The APIs have changed, and the chart no loner appears that I can find, but I suspect the underlying hardware limitation hasn't changed.

Attenuation vs. "linear" range: https://docs.espressif.com/projects/esp-idf/en/release-v4.4/esp32s2/api-reference/peripherals/adc.html#_CPPv425adc1_config_channel_atten14adc1_channel_t11adc_atten_t I think we use 11dB, but linear range varies by chip type, haven't seen data for S2 vs. S3.

bill88t commented 8 months ago

Hmm, alright then. Now before I close the issue, maybe we should document that somewhere and maybe update the reference voltage in analogio to be the suitable one for each chip, instead of 3v3. Is that alright? Can I go and do it?

dhalbert commented 8 months ago

Is that alright? Can I go and do it?

That would be great. There is one comment already in the doc:

Note the ADC value may not scale to the actual voltage linearly at ends of the analog range.

I am not sure about the reference voltage, because the attenuation I think may have chosen to allow 3.3V as the reference voltage.

anecdata commented 8 months ago

https://docs.espressif.com/projects/esp-idf/en/latest/esp32s3/api-reference/peripherals/adc_calibration.html

By design, the ADC reference voltage for ESP32-S3 is 1100 mV

but it can vary, and most chips have the actual value in eFuse. Input voltages from the ranges at the link in my prior comment are attenuated so they can be measured against the reference.

But from a user perspective, due to using a fixed 11dB attenuation value, the analog value returned is relative to 3.3v (despite non-linearities and limited range) https://github.com/adafruit/circuitpython/blame/9ac250aa3ba95c77c35c987b3734cfbabf4fede6/ports/espressif/common-hal/analogio/AnalogIn.c#L169

bill88t commented 8 months ago

I took a good read through the s2 technical manual and the attenuation stuff. (Actually managed to 100% understand the analogio code!)

Also, C6 does 0-3.3V perfectly. I did it both outside (esp-idf) and inside CP, and it works on both.

    return voltage * ((1 << 16) - 1) / 3300;
}

float common_hal_analogio_analogin_get_reference_voltage(analogio_analogin_obj_t *self) {
    return 3.3f;
}

I still do not quite understand why I shouldn't go and just and #define the vref for every variant.. We have locked ADC_ATTEN_DB_11, shouldn't we provide a vref for the effective maximum?

anecdata commented 8 months ago

At the user level, isn't vref essentially 3.3v for all variants, despite different top-end limits (like 2.6v)? CircuitPython code would use the vref for what, other than reconstituting a 16-bit value into a voltage?

dhalbert commented 8 months ago

I think the point of the vref is to say what the 65535 value corresponds to, yes.

reference_voltage: float The maximum voltage measurable (also known as the reference voltage) as a float in Volts. Note the ADC value may not scale to the actual voltage linearly at ends of the analog range.

bill88t commented 8 months ago

Then vref for all esp (except C6) is wrong (as it cannot reach 65535) and should be fixed. Imma go gather all the values and hardcode em.

dhalbert commented 8 months ago

Then vref for all esp (except C6) is wrong (as it cannot reach 65535) and should be fixed. Imma go gather all the values and hardcode em.

I think the documentation is not so great, but I think that IF you could measure 3.3V it would be 65535. In other words, the reference voltage is the scale factor: volts = (reading / 65535) * reference_voltage.

bill88t commented 8 months ago

Well, IF it could, it would show 65535, like how it does on C6. But for the rest it's impossible.

We can either do "max is always 65535" or do "65535 is 3v3". Docs say the first, implementation does the second.

anecdata commented 8 months ago

It might be tough to state an exact maximum achievable voltage for a particular variant, since it depends on the per-chip internal vref (1100mV±100mV). And the Espressif docs seem to have stripped out the voltage range details for more recent releases.

(Also we may want to allow attenuation as a param at some point to give more precision (same bit width over a smaller voltage range), though there may not be much accuracy benefit to this.)

bill88t commented 8 months ago

It might be tough to state an exact maximum achievable voltage for a particular variant

Well, CP uses the calibs. We do not have to bother with the chip variance (theoretically).

The technical reference manuals for esp32, esp32s2 and s3 state that: image

But vref is not defined anywhere in this document. Assuming it's 3v3, it would mean it's 3300÷4095×3300=~2660.

But that clearly isn't the case and is more like the 3100 that the docs stated.

dhalbert commented 8 months ago

I wired up a potentiometer on an ESP32-S2 and tested it, and I read through some documentation, both hardware and software. The clearest explanations are in the ESP-IDF V4.x manuals, such as https://docs.espressif.com/projects/esp-idf/en/v4.3/esp32s2/api-reference/peripherals/adc.html. There, the actual maximum voltage and the attenuations are explained. The ESP-IDF v5 documentation is less clear: https://docs.espressif.com/projects/esp-idf/en/v5.1.2/esp32s2/api-reference/peripherals/adc_oneshot.html?highlight=adc#introduction.

When we originally designed the API, I think we were not considering hardware where the ADC input voltage range, even with built-in attenuation was less than Vcc (e.g., 3.3V). We were also thinking about making it kind of like the Arduino APi, though with more bits of precision. So we chose 16 bits and scaled the readings when the ADC had less resolution.

AnalogIn.value returns a count whose full-scale is meant to be .reference_voltage. Backwards compatibility is probably also helpful: it could even be dangerous if someone upgraded CircuitPython and the range changed.

MicroPython has an API that returns volts (well, microvolts) or a count. We could consider augmenting the API:

All this takes code space. It's also a "nice to have", but I don't see it as a serious deficiency right now. The current implemenation is usable for most purposes. The current limitations can be described with better documentation per chip. I don't want to change the semantics of the current AnalogIn.

bill88t commented 8 months ago

I do think there is a bit more value than convenience to this api, as it could be used to fetch many more readings, as the conversion logic would be in C. It would also be more beginner friendly.

Considering this should add almost no strings, it shouldn't take too much space.

However I do not think .reference_voltage should remain useless. If we were to put the effort to revamp the api, shouldn't we at least make it settable and leave the default to be 3v3 as is?

But considering that no other adc issues have been opened for all this time, it may be safe to assume the current implementation is indeed good enough.

I will make a pr updating the docs, that will close this issue when I get the time.

deshipu commented 8 months ago

One reason for the current API is that not all boards have floats enabled.

dhalbert commented 8 months ago

One reason for the current API is that not all boards have floats enabled.

All CircuitPython boards have floats (e.g. for time.sleep()), but not all have longints. I think there may be some MicroPython boards without floats.

dhalbert commented 8 months ago

If we were to put the effort to revamp the api, shouldn't we at least make it settable and leave the default to be 3v3 as is?

That is an idea. I thought you wanted to set them differently at compile time, which would change the current ranges.

deshipu commented 8 months ago

Hmm, I remember the floats problem coming up during the discussion of the ADC and the PWM API, so maybe it wasn't the case back then? Or maybe my memory is just failing me.

bill88t commented 8 months ago

Also just making a setter for the ref voltage could be a tiny tiny change that would not affect any existing apps. It would take (almost) no flash space. Should I add it in my docs pr?

dhalbert commented 8 months ago

Also just making a setter for the ref voltage could be a tiny tiny change that would not affect any existing apps.

But you want it to affect the returned .value, right? It's not just informational.

dhalbert commented 8 months ago

Hmm, I remember the floats problem coming up during the discussion of the ADC and the PWM API, so maybe it wasn't the case back then? Or maybe my memory is just failing me.

The original analogio discussion was maybe before my time, but I think pretty early float was turned on for "everything" (SAMD21). But #85 is maybe what you're thinking about?

bill88t commented 8 months ago

Also just making a setter for the ref voltage could be a tiny tiny change that would not affect any existing apps.

But you want it to affect the returned .value, right? It's not just informational.

Default would always be 3v3, as it currently is. If the value is changed, the math at the value return statement would change yea. If the value is left unchaged, the math will be identical to how it is now.

bablokb commented 3 months ago

I also stumbled across this issue. IMHO, there is a misunderstanding and the CP-documentation is wrong about what VREF is. It is not the maximum measurable voltage as stated by the docs. For many MCU this is the case, but not for all.

ESP32 is one case, the VREF is in the range 1000mV-1200mV, the measurable voltage range is documented in the Arduino docs, see table in https://docs.espressif.com/projects/arduino-esp32/en/latest/api/adc.html.

I also think there should be consistency across the various ports. The raspberrypi or stm ports don't scale the ADC values at all. In fact, that is not possible because the code does not know anything about the specific hardware-setup. E.g. on a rp2040 the maximum measurable voltage should not exceed IOVDD which could be 1.8V even if the ADC_VREF is 3.3V.

So my suggestion would be to implement a clear API across all ports with a settable maximum voltage (not called VREF). Ideally, the API would also just do the scaling to mV.