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

[Bug]: Wireless Paper (ESP32s3) does not show battery percentage #3131

Closed
ixt opened this issue Jan 25, 2024 · 6 comments
Closed

[Bug]: Wireless Paper (ESP32s3) does not show battery percentage #3131

ixt opened this issue Jan 25, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@ixt
Copy link
Contributor

ixt commented Jan 25, 2024

Category

Hardware Compatibility

Hardware

Heltec Wireless Paper

Firmware Version

2.2.17+

Description

Currently Wireless Paper (Assuming both versions, although only tested v1.1) does not display battery power, this is due to the battery sensing pin being on ADC2 and incorrectly stated.

ADC2 doesn't by default like being read from ESP32 or ESP32s3, and has to be read via the RTC due to Wifi-related issues in IDF (If i'm understanding correctly).

The schematic doesn't make obvious which pin we should be reading from, but its either 19 or 20. In the Heltech v2 boards we use a workaround that sets up the registers (SENS_SAR_READ_CTRL2_REG). But on ESP32s3 they are renamed to SENS_SAR_READER2_CTRL_REG. Making those updates however doesn't get me a working solution. I dont quite understand the esp_adc_cal code yet, so not sure which knobs to turn to help it along, but I think I've found the right tree to bark up...

I imagine this would be an issue on any other boards that happen to place the battery sensing on the ADC2 on ESP32/s3/c3. So any Heltec folks listening (if they are here), for v1.2 move the battery sensing to a pin with ADC1, I know thats a ridiculous ask but it does seem like even if we get this working it'll probably be unreliable.

I tried finding other examples of people using ADC2 on a ESP32s3, but I don't feel convinced anyone has managed to do it from scratch as most of the instances I could find where just ESP32 examples with commented out parts that would have given compile issues.

It's possible I missed some so it's worth searching anyway. Attached is a log of the assert fail I get from the loop.

Again, not my area at all so might have just missed something obvious that someone in the area might know. Don't mistake my depth for subject knowledge lol

I also changed the battery pin stuff to this

#define ADC_MULTIPLIER 0.02
#define BATTERY_PIN 20 // A battery voltage measurement pin, voltage divider connected here to measure battery voltage
#define ADC_ATTENUATION ADC_ATTEN_DB_11 // lower dB for high resistance voltage divider
#define ADC_CTRL 19
#define ADC_CHANNEL ADC2_GPIO20_CHANNEL
#define BAT_MEASURE_ADC_UNIT 2

The ADC_MULTIPLIER I adjusted through a range of amounts and same with the Battery and ADC pins.

ESP32s3 technical reference

Relevant log output

assert failed: esp_adc_cal_raw_to_voltage esp_adc_cal.c:162 (adc_reading < 4096)


Backtrace: 0x40377d62:0x3fceba90 0x403805e9:0x3fcebab0 0x40387285:0x3fcebad0 0x420924b1:0x3fcebc00 0x4200490d:0x3fcebc40 0x4200425d:0x3fcebc80 0x4213c2f3:0x3fcebca0 0x42004582:0x3fcebcc0 0x42004789:0x3fcebd70 0x4200605e:0x3fcebd90 0x420359fd:0x3fcebdb0 0x42010cf6:0x3fcebde0 0x420641a5:0x3fcebe00

  #0  0x40377d62:0x3fceba90 in panic_abort at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/esp_system/panic.c:408
  #1  0x403805e9:0x3fcebab0 in esp_system_abort at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/esp_system/esp_system.c:137
  #2  0x40387285:0x3fcebad0 in __assert_func at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/newlib/assert.c:85
  #3  0x420924b1:0x3fcebc00 in esp_adc_cal_raw_to_voltage at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/esp_adc_cal/esp32s3/esp_adc_cal.c:162 (discriminator 1)
  #4  0x4200490d:0x3fcebc40 in AnalogBatteryLevel::getBattVoltage() at src/Power.cpp:224
  #5  0x4200425d:0x3fcebc80 in AnalogBatteryLevel::getBatteryPercent() at src/Power.cpp:135
  #6  0x4213c2f3:0x3fcebca0 in AnalogBatteryLevel::isBatteryConnect() at src/Power.cpp:248
  #7  0x42004582:0x3fcebcc0 in Power::readPowerStatus() at src/Power.cpp:457
  #8  0x42004789:0x3fcebd70 in Power::runOnce() at src/Power.cpp:564
  #9  0x4200605e:0x3fcebd90 in concurrency::OSThread::run() at src/concurrency/OSThread.cpp:85
  #10 0x420359fd:0x3fcebdb0 in ThreadController::runOrDelay() at .pio/libdeps/heltec-wireless-paper/Thread/ThreadController.cpp:59
  #11 0x42010cf6:0x3fcebde0 in loop() at src/main.cpp:973
  #12 0x420641a5:0x3fcebe00 in loopTask(void*) at /Users/nfnorange/.platformio/packages/framework-arduinoespressif32/cores/esp32/main.cpp:50
@ixt ixt added the bug Something isn't working label Jan 25, 2024
@code8buster
Copy link
Contributor

The esp adc calibration struct/interface I added was, at the time, already slated to be deprecated in the next version of the IDF and I'm certain that has come to pass already.

It should probably be redone with the new interfaces that IDF provides rather than try to shoehorn the workaround into using the deprecated stuff.

@mverch67
Copy link
Collaborator

Espressif provides an example how to read the ADC, maybe this is a good starting point for adopting the new API:

https://github.com/espressif/esp-idf/tree/master/examples/peripherals/adc/oneshot_read

@todd-herbert
Copy link
Contributor

Espressif provides an example how to read the ADC, maybe this is a good starting point for adopting the new API:
https://github.com/espressif/esp-idf/tree/master/examples/peripherals/adc/oneshot_read

To my untrained eye, it doesn't look to me like the new API is available in the environment provided by Heltec for these boards just yet. Can anyone double check this?

has to be read via the RTC due to Wifi-related issues in IDF (If i'm understanding correctly).

As far as I'm aware, this means that WiFi must be powered off to read from ADC2. I don't know how frequently the battery level needs to be checked; is it an option to briefly power WiFi off to make the reading?

The schematic doesn't make obvious which pin we should be reading from, but its either 19 or 20.

VBAT is connected to ADC_IN (GPIO20) through a voltage divider, and a P-Channel MOSFET. The gate is ADC_CTRL, which is connected to ADC_CTRL (GPIO19). When GPIO19 is set LOW, ADC_IN is at roughly 50% of VBAT.

Wireless Paper schematic

Admittedly, I've never used the ADC on an ESP32 before now, but I had no trouble reading ADC_IN using Arduino's analogReadMilliVolts() just now.

#include <Arduino.h>

#define PIN_ADC_CTRL 19
#define PIN_ADC_IN 20

void setup() {
    Serial.begin(9600);

    pinMode(PIN_ADC_CTRL, OUTPUT);
    digitalWrite(PIN_ADC_CTRL, LOW);    // Unneeded, pins default to LOW (?)    
}

void loop() {
    const uint8_t repeats = 10;
    uint32_t millivolts = 0;

    // Multiple readings
    for (uint8_t i = 0; i < repeats; i++) {
        millivolts += analogReadMilliVolts(PIN_ADC_IN);
        delay(100);
    }

    // Average the readings
    millivolts /= repeats;

    // Adjust for voltage divider: roughly 50% drop
    millivolts *= 2;
    
    Serial.print(millivolts);
    Serial.println("mV");
    delay(2000);
}
analogReadMilliVolts.mp4

@todd-herbert
Copy link
Contributor

A bit more sniffing around and it sounds like it might not be necessary to power-off the WiFi

ADC can be used together with Wi-Fi, although usage by Wi-Fi has priority over the usage by software

Source


An older example for reading ADC2 suggests that reading with adc2_get_raw() will return a status which can be used to check if WiFi interfered with the ADC sample.

@todd-herbert
Copy link
Contributor

Apologies, looking closer at the power.cpp code I can see that I definitely went on a bit of a tangent there, explaining things that have clearly been thought of and dealt with before.

@todd-herbert
Copy link
Contributor

todd-herbert commented Feb 1, 2024

Okay, some success:

adc2.mp4
  • A few parameters need adjusting in variant.h
#define ADC_CTRL 19
#define BATTERY_PIN 20
#define ADC_CHANNEL ADC2_GPIO20_CHANNEL
#define ADC_MULTIPLIER 2                    // Voltage divider is roughly 1:1
#define BAT_MEASURE_ADC_UNIT 2              // Use ADC2
#define ADC_ATTENUATION ADC_ATTEN_DB_11     // Voltage divider output too high

ESP32S3 doesn't seem to suffer from the same issue as ESP32: supposedly, WiFi and ADC2 can both be used, however the ADC reading can theoretically fail if WiFi is currently transmitting / receiving? By checking the esp_err_t returned by adc2_get_raw() this can be handled one way or another:

for (int i = 0; i < BATTERY_SENSE_SAMPLES;) {
    esp_err_t read_result = adc2_get_raw(adc_channel, ADC_WIDTH_BIT_12, &adc_buf);

    if (read_result == ESP_OK) {
        raw += adc_buf;
        i++;
    } 
    else {
        LOG_DEBUG("ADC read failed, retrying\n");
    }
}

In the few minutes I spent testing, I didn't see any failed readings.

Are there any other ESP32S boards using ADC2 right now, which need to be considered / avoided? I can probably look into writing a fix in the next few days, unless anyone else wants to work on it, or thinks we should hold off for now.


Although VBAT will read correctly, it seems that there are still some issues with
power (detecting USB,
& charging):

DEBUG | 15:34:21 91 [Power] Battery: usbPower=0, isCharging=0, batMv=3870, batPct=64

I also observed that usbPower went true after waking from sleep, even when powered from battery. If nobody gets to it first, I'll open this as a new issue in the next day or so; It is late here now.

todd-herbert added a commit to todd-herbert/meshtastic-firmware that referenced this issue Feb 11, 2024
todd-herbert added a commit to todd-herbert/meshtastic-firmware that referenced this issue Feb 11, 2024
todd-herbert added a commit to todd-herbert/meshtastic-firmware that referenced this issue Feb 11, 2024
thebentern added a commit that referenced this issue Feb 13, 2024
* fix: Wireless Paper (v1.0 & v1.1) not showing battery percentage
Addresses #3131

* refactor: count only valid samples
Responds to #3208 (comment)

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants