Murdock results

✔️ PASSED

85d65aa boards/common/esp32x: cleanup Arduino leftover

Artifacts

• Documentation preview

Or do these boards not have Arduino headers and the GPIOs are just chosen arbitrary?

None of the boards have Arduino headers so that the choice of the GPIOs is arbitrary. The only ESP32 board with Arduino headers is that one in PR #21479.

None of the boards have Arduino headers so that the choice of the GPIOs is arbitrarily.

That is in-line with what the I/O mapping documentation claims: https://doc.riot-os.org/iomaps.html#iomaps-shields

Only when a feature such as arduino_shield_nano/arduino_shield_uno/xiao_shield is provided, the board has electrical and mechanical compatibility with the Arduino Nano/Arduino UNO/XIAO form factor and must use the same numbering scheme. If no such feature is provided by the board, the I/O mapping can just be arbitrary.

(I wonder why we are not having a feature declaring consistent mapping for feather compatible boards yet. I guess someone(tm) should do that :D)

Only when a feature such as arduino_shield_nano/arduino_shield_uno/xiao_shield is provided

That's why the only ESP32 board with Arduino headers in PR #21479 provides the feature arduino_shield_uno while all other ESP32x boards doesn't.

What I don't understand, but my understanding of ESP32 is limited, is the change of pins. Were there so many errors in the pin definitions? Wrong GPIOs selected for the Arduino pins?

Many of the changes are either to document the pins more clearly or to complete missing mappings, e.g. for esp32s3-pros3 or esp32s3-devkit.

But when I went through the Arduino configuration board by board again, I noticed some misconfigurations on some boards that either hadn't been noticed before or didn't matter, e.g. for the esp32-olimex-evb board.

I also realized that some of the configurations don't make sense, even in the peripheral configuration, e.g. using a pull-up button input as a PWM channel on esp32-heltec-lora-v2.

I'm sorry that all these cleanups lead to 897 lines + and 300 lines - 🙈 If it helps, I could split this PR into seperate PRs, one for each board which would result in 12 separate PRs.

I'm sorry that all these cleanups lead to 897 lines + and 300 lines - 🙈 If it helps, I could split this PR into seperate PRs, one for each board which would result in 12 separate PRs.

I went through everything yesterday reasonably thoroughly and the changes all look reasonable. Of course one could do more cleaning and whatever, but that is beyond the scope of this PR.

@maribu One thing is not really clear to me from the documentation. Have the following macros to be declared only for boards with Arduino headers or for any board with some mapping, even if it doesn't have Arduino headers and is not mechanically compatible?

#define ARDUINO_UART_D0D1       UART_DEV(0) /**< Arduino UART interface */
#define ARDUINO_SPI_D11D12D13   SPI_DEV(0)  /**< Arduino SPI bus */
#define ARDUINO_I2C_UNO         I2C_DEV(0)  /**< Arduino I2C bus */

For the macros

#define ARDUINO_UART_D0D1       UART_DEV(0) /**< Arduino UART interface */
#define ARDUINO_SPI_D11D12D13   SPI_DEV(0)  /**< Arduino SPI bus */

I think it would be fair to use them also for non-Arduino compatible I/O mappings, as long as the named pins (or rather their I/O mapping names) are actually used. But we should update the doc to indicate that.

#define ARDUINO_I2C_UNO         I2C_DEV(0)  /**< Arduino I2C bus */

For this one, I think it is confusing to use that for non-Arudino-UNO compatible boards.

#define ARDUINO_I2C_UNO         I2C_DEV(0)  /**< Arduino I2C bus */

For this one, I think it is confusing to use that for non-Arudino-UNO compatible boards.

Ok, that means you would define ARDUINO_UART_D0D1 and ARDUINO_SPI_D11D12D13 in arduino_iomap.h also for such boards that don't have Arduino headers, but not ARDUINO_I2C_UNO. Would you then define ARDUINO_I2C_DEV instead or nothing? ARDUINO_I2C_DEV is defined by the Arduino driver anyway, if it is not already defined.

#define ARDUINO_I2C_UNO         I2C_DEV(0)  /**< Arduino I2C bus */

For this one, I think it is confusing to use that for non-Arudino-UNO compatible boards.

Ok, that means you would define ARDUINO_UART_D0D1 and ARDUINO_SPI_D11D12D13 in arduino_iomap.h also for such boards that don't have Arduino headers, but not ARDUINO_I2C_UNO. Would you then define ARDUINO_I2C_DEV instead or nothing? ARDUINO_I2C_DEV is defined by the Arduino driver anyway, if it is not already defined.

@maribu ?

I think either is fine

I think either is fine

Ok, then I prefer the removal and leave the default definition.

@crasbe Is your approval still valid with the last fixups?

@crasbe Is your approval still valid with the last fixups?

Sure :)

@crasbe @maribu Thanks for reviewing and your support.