Adds S3 GPIO48 in digitalPinToInterrupt() + GPIO INTR Review

[SuGlider]

Description of Change

A user has open an issue about not being able to add an Interrupt to ESP32-S3 GPIO 48 because the GPIO num shall be lower than 48 in digitalPinToInterrupt() macro.

Based on it, this PR has a initial review of the macros and the values used in pins_arduio.h for the main SoCs (ESP32, S2, S3 and C3).

Tests scenarios

Simples test done in ESP32/S2/S3/C3 with attachInterrupt().

Related links

Closes #8560

[SuGlider]

@me-no-dev - Please let me know if this change makes sense.
In case it makes sense, should I change all variants, before merging?

[P-R-O-C-H-Y]

Looks good :)

[lucasssvaz]

LGTM

[VojtechBartoska]

Resolution:

• we can add to all variants

[SuGlider]

@lucasssvaz @P-R-O-C-H-Y @me-no-dev -- ready for reviewing....
This was a lot of work and time... :-(

[SuGlider]

I also fixed a wrong already merged file name (pins_arduino.h.txt) :

12 changes: 6 additions & 6 deletions12
variants/lilygo_t_display/pins_arduino.h.txt → variants/lilygo_t_display/pins_arduino.h

[SuGlider]

The changes in this PR also include all recently added new boards (last 4 days).
So this is the complete review of all different SoC boards, RGB LEDs etc.

I've researched a lot of boards to make sure the all RGB NeoPixel LEDs are correctly mapped into the pins_arduino.h for each board.

[SuGlider]

But... there may be errors... so please take some time to review it carefully. Thanks!

[VojtechBartoska]

Thanks @SuGlider for handling this! I can imagine it was time consuming.

[me-no-dev]

I am not sure how to properly review this, but if you are worried that there might be issues, I would suggest we leave it for 3.0.0 and not risk another bugfix 2.0.x

[VojtechBartoska]

What about modifying slightly https://github.com/espressif/arduino-esp32/actions/workflows/boards.yml and check this by that?

[P-R-O-C-H-Y]

I am not sure how to properly review this, but if you are worried that there might be issues, I would suggest we leave it for 3.0.0 and not risk another bugfix 2.0.x

As @VojtechBartoska mentioned, we can at least run the allboards workflow to see if there are any issues. I will add to the workflow new trigger on some label, let's say "Boards test" and then we can run it. I am not sure if we are able to run it now on PR when the workflow is set to run only on remote dispatch.

What do you think @me-no-dev ? That test can at least cover some typos check as I already found one.

[me-no-dev]

I am not sure if we are able to run it now on PR when the workflow is set to run only on remote dispatch.

You would need to specify the target pr when executing the workflow. Pass it as argument and then act in the workflow itself.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

[P-R-O-C-H-Y]

@SuGlider @me-no-dev I see a lot of boards failing to compile the CI test sketch.
CI action on this PR => 33 boards failed
CI action on master for comparison => 4 boards failed (missing SS pin + some other issues)

[SuGlider]

@P-R-O-C-H-Y @me-no-dev - This PR seems now ready and fine. It passes the Boards CI.
https://github.com/SuGlider/arduino-esp32/actions/runs/6067330536

@P-R-O-C-H-Y had some concerns about changing all the boards... we can discuss what would be best to do.
Anyway, if necessary, I can quickly create a new PR that addresses only the critical issue with the S3 GPIO48 and some other issues found in the CI process.

[VojtechBartoska]

This PR with changes for all variants is left to 3.0.0 milestone as it can have more consequences.

@SuGlider will open new PR with GPIO pin change from 48 to 49 for 2.0.12 milestone.