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

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

Closed
1 task done
bkari02 opened this issue Dec 14, 2023 · 5 comments
Closed
1 task done
Assignees
Labels
Area: Peripherals API Relates to peripheral's APIs. Resolution: Awaiting response Waiting for response of author
Milestone

Comments

@bkari02
Copy link
Contributor

bkari02 commented Dec 14, 2023

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

  • I confirm I have checked existing issues, online documentation and Troubleshooting guide.
@bkari02 bkari02 added the Status: Awaiting triage Issue is waiting for triage label Dec 14, 2023
@bkari02
Copy link
Contributor Author

bkari02 commented Dec 14, 2023

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 P-R-O-C-H-Y self-assigned this Dec 14, 2023
@P-R-O-C-H-Y P-R-O-C-H-Y added the Area: Peripherals API Relates to peripheral's APIs. label Dec 14, 2023
@P-R-O-C-H-Y
Copy link
Member

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!

@P-R-O-C-H-Y P-R-O-C-H-Y added Resolution: Awaiting response Waiting for response of author and removed Status: Awaiting triage Issue is waiting for triage labels Dec 14, 2023
@bkari02
Copy link
Contributor Author

bkari02 commented Dec 14, 2023

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
Copy link
Collaborator

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
Copy link
Contributor Author

bkari02 commented Dec 15, 2023

Solved in #9006

@bkari02 bkari02 closed this as completed Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Peripherals API Relates to peripheral's APIs. Resolution: Awaiting response Waiting for response of author
Projects
None yet
Development

No branches or pull requests

3 participants