espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
13.61k stars 7.41k forks source link

analogReadMilliVolts() does not handle changes in resolution made by analogReadResolution() #8999

Closed bkari02 closed 10 months ago

bkari02 commented 10 months ago

Board

ESP32-S2

Device Description

Tested with Unexpected Maker FeatherS2 & senseBox MCU-S2 ESP32-S2

Hardware Configuration

Any analog sensor (e.g. Truebner SMT50 or SparkFun SEN-13322) is connected.

Version

v2.0.14

IDE Name

ArduinoIDE

Operating System

macOS 14.1.2

Flash frequency

80MHz

PSRAM enabled

yes

Upload speed

921600

Description

Expected behaviour: When using ESP32-S2 and analogReadMilliVolts(pin) I should always get the measured voltage in milliVolts.

Weird behaviour: When changing the default 13 bit resolution of the ADC1 to e.g. 12bit using analogReadResolution(12); the outputs of analogReadMilliVolts(pin) are incorrect (half in size in case of 12bit).

Tried to track it down and found this to be happening (in esp32-hal-adc.c): The __analogRead() function uses the 12 bit resolution (as expected because of analogReadResolution(12)). But __analogReadMilliVolts() internally also uses __analogRead() which returns a 12bit ADC value. But during transformation in esp_adc_cal_raw_to_voltage((uint32_t)adc_reading, &chars) the adc_reading is always expected to be the raw 13bit value in case of the ESP32-S2. There it assumes the 12 bit value to be 13 bit and the resulting value in milliVolts half of the correct value.

I assume this also happens with other chips than the S2, but haven't tested it so far.

Sketch

// Define the pin that the analog sensor is connected to (in my case A2 of the UM FeatherS2)
#define analogSensorPin A2

void setup() {
  // initialize serial communication at 115200 bits per second:
  Serial.begin(115200);
  delay(1000); 

  // set the resolution to the 13 bits (0-8192)  --> this is default, but just to make sure
  analogReadResolution(13);
  int analogValue = analogRead(analogSensorPin);
  int analogVolts = analogReadMilliVolts(analogSensorPin);
  Serial.printf("13 bit - ADC analog value = %d\n",analogValue);
  Serial.printf("13 bit - ADC millivolts value = %d\n",analogVolts);

  // set the resolution to 12 bits (0-4096)
  analogReadResolution(12);
  analogValue = analogRead(analogSensorPin);
  analogVolts = analogReadMilliVolts(analogSensorPin);
  Serial.printf("12 bit - ADC analog value = %d\n",analogValue);
  Serial.printf("12 bit - ADC millivolts value = %d\n",analogVolts);

    // set the resolution to 10 bits (0-1024)
  analogReadResolution(10);
  analogValue = analogRead(analogSensorPin);
  analogVolts = analogReadMilliVolts(analogSensorPin);
  Serial.printf("10 bit - ADC analog value = %d\n",analogValue);
  Serial.printf("10 bit - ADC millivolts value = %d\n",analogVolts);
}

void loop() {

}

Debug Message

Serial Output:
13 bit - ADC analog value = 2232
13 bit - ADC millivolts value = 711
12 bit - ADC analog value = 1104
12 bit - ADC millivolts value = 349
10 bit - ADC analog value = 278
10 bit - ADC millivolts value = 88

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

bkari02 commented 10 months ago

Might be related to #6436 #4941 and #4386 but these have all been closed a long time ago and not mentioned the relation to the analogWidth/readResolution.

P-R-O-C-H-Y commented 10 months ago

Hello @bkari02, can you please try to run your sketch on latest dev release 3.0.0-alpha3 or on master? The ADC got refactored to new ESP-IDF NG drivers. Thanks!

bkari02 commented 10 months ago

Hey @P-R-O-C-H-Y thanks for the quick response! Indeed, everything does work as expected in the 3.0.0-alpha3 release, so the refactoring solved the issue here.

Since there are some other breaking changes that does not allow us to fully migrate to v3 yet, is there a chance this can get fixed in a new v2.x release? I could also try to contribute a PR if that helps.

VojtechBartoska commented 10 months ago

Hello @bkari02, feel free to open a PR targeting https://github.com/espressif/arduino-esp32/tree/release/v2.x branch. We are now collecting fixes for v2.X and will decide at the beginning of 2024 if it makes sense to release v2.0.15. I am adding this issue to that milestone for tracking.

bkari02 commented 10 months ago

Solved in #9006