espressif/arduino-esp32

Battery pin definition non standard and untestable

Closed this issue · 4 comments

Board

Multiple - e.g. Lilygo T3S3

Device Description

None (attached to laptop, but not relevant)

Hardware Configuration

No

Version

latest stable Release (if not listed below)

Type

Task

IDE Name

PlatformIO

Operating System

N/A

Flash frequency

N/A

PSRAM enabled

yes

Upload speed

N/A

Description

The variant files contain at least 15 different ways of telling you on which pin the battery voltage can be measured.

Try grep BAT /Users/mitra/.platformio/packages/framework-arduinoespressif32/variants/*/pins_arduino.h
VBAT_VOLTAGE BAT_VOLTS BAT_VOLT BAT_MEASURE VBAT_SENSE BATT_ADC_PIN BAT_ADC_PIN BAT_ADC BATT_MONITOR BAT_MEASURE BAT_LV WS_BAT_ADC PIN_VBAT PIN_BATTERY BATTERY_PIN

What is worse is that these are mostly (though not all) static const uint8_t so it is next to impossible to write board independent (library) code without testing by board name

I am sure there is code that depends on most of these different definitions, so I would suggest we pick one and progressively add #defines to all variant files that define one common value.

I don't particularly care which is used but suggest it indicates its an ADC as for example VBAT_VOLTAGE could imply its the voltage level, so if we chose VBAT_BUILTIN

then e.g. on lilygo_t3_s3_sx1262/pins_arduino.h we could add #define VBAT_BUILTIN BAT_VOLT

I'm sure someone could write a quick python script to make all the changes, but I don't want to submit this as a PR if there is likely to be (reasonable) pushback.

Sketch

#ifdef ARDUINO_LILYGO_T3_S3_V1_X
  analogRead(BAT_VOLT); // Specific to a few boards
#elif defined(xxxxx)
  analogRead(VBAT_SENSE)
#elif ... repeat for every board that has built in battery measurement
#endif

Debug Message

n/a

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.

If the mfr has the pin labeled with specific text, the variant file should match.

If the mfr has the pin labeled with specific text, the variant file should match.

I dont think I've seen a battery pin labeled. On the T3S3 for example it doesn't even come to a connector, its purely internal - from chip to V-bat. Code can find and read it, but there is no external pin.

As I understand it, a big part of the variant file is about allowing for libraries to work transparantly
across different boards. For example there are about 1000 variants that define either LED_BUILTIN or BUILTIN_LED, so my library can work with the LED on any of those boards. There are 400 variants that define MISO MOSI SCK so I can use SPI with them; similarly for SDA/SCL for I2C. But there is no commanlity for battery measurement despite 66 variants defining some version of it.

And ... as I pointed out in the first comment, there is really no way to handle this since you can't even put a massive #if #elif without naming every single board, since the definitions are in almost all cases static const rather than #define

The battery pin is not something "standard" to Arduino, so variant submitters define it the way they want. You can make a define based code like

#ifdef ARDUINO_LILYGO_T3_S3_V1_X
  #define BAT_VOLT_PIN BAT_VOLT
#elif defined(xxxxx)
  #define BAT_VOLT_PIN VBAT_SENSE
#elif ... repeat for every board that has built in battery measurement
#endif

Or add the define in the variants files and submit a PR

OK - done - and PR created, the changes were made programatically and then checked by hand, I can redo them if there is some difference (e.g. something other than BAT_VOLT_PIN) prefered.