espressif / arduino-esp32

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

Inconsistent checks #if and #ifdef for CONFIG_IDF_TARGET_ESP32 #8957

Open TD-er opened 9 months ago

TD-er commented 9 months ago

Board

Any

Device Description

not related to hardware

Hardware Configuration

not related to hardware

Version

latest master (checkout manually)

IDE Name

PlatformIO

Operating System

Windows 11

Flash frequency

40MHz

PSRAM enabled

yes

Upload speed

115200

Description

This applies both to the Arduino ESP32 code, and also the ESP-IDF code. Sometimes the check for platform starts with #ifdef CONFIG_IDF_TARGET_ESP32 and more often #if CONFIG_IDF_TARGET_ESP32 The following checks for platforms like C2, C3, etc only check the value and not whether it is defined.

Example in the IDF code: https://github.com/espressif/esp-idf/blob/master/components/bootloader_support/src/bootloader_efuse.c

Some examples in Arduino ESP32: https://github.com/espressif/arduino-esp32/blob/b811ea40875bbfbf8bd62b4038879ac7be2c2e2b/cores/esp32/esp32-hal-misc.c#L71 https://github.com/espressif/arduino-esp32/blob/b811ea40875bbfbf8bd62b4038879ac7be2c2e2b/cores/esp32/esp32-hal-i2c-slave.c#L164 https://github.com/espressif/arduino-esp32/blob/b811ea40875bbfbf8bd62b4038879ac7be2c2e2b/libraries/ESP32/examples/Serial/Serial_All_CPU_Freqs/Serial_All_CPU_Freqs.ino#L38 https://github.com/espressif/arduino-esp32/blob/b811ea40875bbfbf8bd62b4038879ac7be2c2e2b/libraries/ESP32/examples/AnalogReadContinuous/AnalogReadContinuous.ino#L6

Can we assume these are never defined as 0 and only defined as 1 when applicable?

Sketch

nope

Debug Message

---

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

me-no-dev commented 9 months ago

Can we assume these are never defined as 0 and only defined as 1 when applicable?

Yes you can. They are only ever defined as 1 is they are applicable, else they are not defined at all.

TD-er commented 9 months ago

Check! So that means it is much less of an issue, leaving only OCD senses tingling with the current inconsistency in the coding :)

me-no-dev commented 9 months ago

Yup, I can totally understand you there. Maybe worth "fixing" for us OCDers