espressif / arduino-esp32

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

ESP32 pin checking macro contraints error #9505

Open S5NC opened 5 months ago

S5NC commented 5 months ago

ESP32 pins_arduino.h definition, tested on ESP32-S3 Dev Module on Arduino IDE

The macro here checks if the pin are less than a number.

#define analogInputToDigitalPin(p)  (((p)<NUM_ANALOG_INPUTS)?(analogChannelToDigitalPin(p)):-1)
#define digitalPinToInterrupt(p)    ((((uint8_t)digitalPinToGPIONumber(p))<NUM_DIGITAL_PINS)?digitalPinToGPIONumber(p):NOT_AN_INTERRUPT)
#define digitalPinHasPWM(p)         (((uint8_t)digitalPinToGPIONumber(p))<NUM_DIGITAL_PINS)

If I pass 20, which is an ADC pin, into analogInputToDigitalPin(p) it would return -1 due to the first check as 20 is not less than NUM_ANALOG_INPUTS which is 20. If I pass 19, which is an ADC pin, into analogInputToDigitalPin(p) it would return 20.

This is unlike digitalPinToInterrupt, which returns the same pin number.

There is also no check that the value is greater or equal to 0

These functions assume that all pins with the same type of functions are all sequentially located.

I don't see GPIO 20 referred to as pin 19 in this table image

S5NC commented 5 months ago

The check for a valid GPIO pin for the ESP32-S3, which means any GPIO pin, should be

#define digitalPinCheck(p) ((0 <= p <= 21 || 26 <= p <= 48) ? (p) : -1)
softhack007 commented 4 months ago

If I pass 20, which is an ADC pin, into analogInputToDigitalPin(p) it would return -1 due to the first check as 20 is not less than NUM_ANALOG_INPUTS which is 20. If I pass 19, which is an ADC pin, into analogInputToDigitalPin(p) it would return 20.

Maybe you are using the function wrongly - on esp32, analogInputToDigitalPin(p) is meant to convert "ADC channels" to GPIO pin numbers.

ESP32-S3 has 2 ADC units with 10 channels, i.e.

I think the macro works correctly with your examples.

S5NC commented 2 months ago

If I pass 20, which is an ADC pin, into analogInputToDigitalPin(p) it would return -1 due to the first check as 20 is not less than NUM_ANALOG_INPUTS which is 20. If I pass 19, which is an ADC pin, into analogInputToDigitalPin(p) it would return 20.

Maybe you are using the function wrongly - on esp32, analogInputToDigitalPin(p) is meant to convert "ADC channels" to GPIO pin numbers.

ESP32-S3 has 2 ADC units with 10 channels, i.e.

* ADC1: channel 0...9

* ADC2: channel 10..19

I think the macro works correctly with your examples.

* channel 20 => not existing => -1

* channel 19 => ADC2_9 => gpio 20

You are right, I thought an analog input meant an analog-capable pin but it refers to the analog channel.

What do you think about digitalPinCheck?