Skip to content

AnalogReadResolution and AnalogRead not handling low and high resolutions correctly #5163

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

Closed
mMerlin opened this issue May 8, 2021 · 4 comments · Fixed by #5776
Closed
Assignees

Comments

@mMerlin
Copy link

mMerlin commented May 8, 2021

Hardware:

Board: Adafruit ESP32 Feather
Core Installation version: 1.0.6
IDE name: Arduino IDE
Flash Frequency: 80Mhz
PSRAM enabled: no
Upload Speed: 921600
Computer OS: Fedora 33 5.11.17-200.fc33.x86_64

Description:

AnalogReadResolution was implemented, but testing shows that the full range was not. The code comments say the range is 1 to 16.

/*
* Set the resolution of analogRead return values. Default is 12 bits (range from 0 to 4096).
* If between 9 and 12, it will equal the set hardware resolution, else value will be shifted.
* Range is 1 - 16
*
* Note: compatibility with Arduino SAM
*/
void analogReadResolution(uint8_t bits);

However, only bit counts in the range of 9 to 12 give the expected results. Values less than 9 act as if 9 was set, and values greater than 12 act as if 12 was set. What is supposed to happen, is that values between 1 and 32 should work. Values outside of the 9 to 12 directly supported by the hardware should cause analogRead() results to be "shifted" left or right to match the range for the specified number of bits. So a full range value would return 1 for a resolution of 1 bit, and a little less than 2 ** 32 for 32 bits. The "little less" because the shifting zero fills. With the 12 bits available from the hardware adc, with a real value of 0xfff, being turned into 0xfff00000. Implementing that is also going to need analogRead() to return a uint32_t, instead a uint16_t. Without that, the resolution could still be implement for up to 16 bits.

It is not sufficent to "force" the resolution bits into the 9 to 12 range. The actual specified resolution needs to be use by analogRead() (all return paths) to shift the hardware result into the correct range.

void __analogReadResolution(uint8_t bits)
{
if(!bits || bits > 16){
return;
}
#if CONFIG_IDF_TARGET_ESP32
__analogSetWidth(bits); // hadware from 9 to 12
#endif
}

#if CONFIG_IDF_TARGET_ESP32
void __analogSetWidth(uint8_t bits){
if(bits < 9){
bits = 9;
} else if(bits > 12){
bits = 12;
}
__analogWidth = bits - 9;
adc1_config_width(__analogWidth);
}
#endif

The logic needed is similar to how the debug sketch generates the expected values.

  • when bits < 9 return value >> (9 - bits);
  • when bits > 12 return value << (bits - 12);
  • otherwise return value';
#define MIN_HARDWARE_BITS 9
#define MAX_HARDWARE_BITS 12
uinit16_t __scaleToResolution(uinit16_t raw, uint8_t bits)
{
  uint16_t value = raw;
  if(bits < MIN_HARDWARE_BITS) {
    return value >> (MIN_HARDWARE_BITS - bits);
  }
  if(bits > MAX_HARDWARE_BITS) {
    return value << (bits - MAX_HARDWARE_BITS);
  }
  return value;
}

where the bits value needs to be carried over from when analogReadResolution() was called, or defaulted (I assume) to 12. Not the adjusted value calculated in __analogSetWidth()

Sketch: esp32_analog_resolution

/* Analog read «adc» testing for ESP32
 *
 * Reference:
 * https://www.arduino.cc/reference/en/language/functions/zero-due-mkr-family/analogreadresolution/
 *   "If you set the analogReadResolution() value to a value higher than your
 *    board’s capabilities, the Arduino will only report back at its highest
 *    resolution, padding the extra bits with zeros."
 *   "If you set the analogReadResolution() value to a value lower than your
 *    board’s capabilities, the extra least significant bits read from the ADC
 *    will be discarded."
 */

const unsigned long SERIAL_BAUD = 115200;
const unsigned long READING_INTERVAL = 10;
const unsigned long REPEAT_INTERVAL = 1000;

const size_t INPUT_PIN = A2; // gpio 34 for analogRead()

void setup() {
  Serial.begin(SERIAL_BAUD);
  delay(200);
  Serial.println("starting test sketch");
}

void loop() {
  uint16_t rawADC[16];
  uint16_t max12 = 0xfff;  

  // With input set to VDD to get range maximum value
  for (size_t i = 0; i <16; i++) {
    analogReadResolution(i + 1);
    rawADC[i] = analogRead(INPUT_PIN);
    delay(READING_INTERVAL);
  }
  Serial.print("bits:   ");
  for (size_t i = 1; i <= 16; i++) {
    Serial.printf("%7d", i);
  }
  Serial.print("\nreading: ");
  for (size_t i = 0; i < 16; i++) {
    Serial.printf(" %#6x", rawADC[i]);
  }
  Serial.print("\nexpected:");
  for (size_t i = 1; i <= 12; i++) {
    Serial.printf(" %#6x", max12 >> (12 - i));
  }
  for (size_t i = 13; i <= 16; i++) {
    Serial.printf(" %#6x", max12 << (i - 12));
  }
  Serial.println("\n"); // add blank line
  delay(REPEAT_INTERVAL);
}

Debug Messages:

output from test sketch

bits:         1      2      3      4      5      6      7      8      9     10     11     12     13     14     15     16
reading:   0x1ff  0x1ff  0x1ff  0x1ff  0x1ff  0x1ff  0x1ff  0x1ff  0x1ff  0x3ff  0x7ff  0xfff  0xfff  0xfff  0xfff  0xfff
expected:    0x1    0x3    0x7    0xf   0x1f   0x3f   0x7f   0xff  0x1ff  0x3ff  0x7ff  0xfff 0x1ffe 0x3ffc 0x7ff8 0xfff0
@stale
Copy link

stale bot commented Jul 8, 2021

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Status: Stale Issue is stale stage (outdated/stuck) label Jul 8, 2021
@mMerlin
Copy link
Author

mMerlin commented Jul 8, 2021

nudge so stale bot does not close this

@stale
Copy link

stale bot commented Jul 8, 2021

[STALE_CLR] This issue has been removed from the stale queue. Please ensure activity to keep it openin the future.

@stale stale bot removed the Status: Stale Issue is stale stage (outdated/stuck) label Jul 8, 2021
@VojtechBartoska VojtechBartoska added the Status: Test needed Issue needs testing label Jul 14, 2021
@brentru
Copy link

brentru commented Aug 24, 2021

bump, not stale

@P-R-O-C-H-Y P-R-O-C-H-Y self-assigned this Oct 18, 2021
@P-R-O-C-H-Y P-R-O-C-H-Y removed the Status: Test needed Issue needs testing label Oct 19, 2021
me-no-dev pushed a commit that referenced this issue Oct 24, 2021
…ulotion() (#5776)

Function analogReadResolution set how many bits will analogRead return.

Find out that this functionality was added back 2017 by @me-no-dev in #161.

Related issues:
#5163
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

Successfully merging a pull request may close this issue.

4 participants