ESP32-S3 SPI spiAttach... spiDetach... function use wrong pins

[UlliBien]

Board

ESP32-S3

Device Description

any

Hardware Configuration

doesn't matter

Version

v2.0.11

IDE Name

Arduino-IDE

Operating System

Windows 11

Flash frequency

doesn't matter

PSRAM enabled

yes

Upload speed

doesn't matter

Description

spiAttachMOSI attaches pin 35 instead of pin 11.
#if CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3
is wrong. Later is a separate statement for the ESP32-S3 with the correct pin assignment:
#elif CONFIG_IDF_TARGET_ESP32S3

spiDetachSCK, spiDetachMISO, spiDetachMOSI the same.

Sketch

doesn't matter

Debug Message

doesn't matter

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.

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

@UlliBien I can confirm this, will open a PR with a fix.

[UlliBien]

I again looked deeper to the code. In methods spiDetach... you should not replace default values -1. If there are default values the pin could not have been attached before (spiAttach.. replaces defaults). spiDetach... sets pin mode to INPUT. This may have unexpected effects. Just do nothing when pin has value -1.

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

In my opinion we can leave it as it is. It's been there for a long time without any complains.
In the new 3.0 version of Arduino-esp32 core this issue won't be existing as we implemented the peripheral manager and the pin can't be -1 anymore.
If you want to take a look, you can do it there: https://github.com/espressif/arduino-esp32/blob/esp-idf-v5.1-libs/cores/esp32/esp32-hal-spi.c

[VojtechBartoska]

@P-R-O-C-H-Y Can we mark this as pending "to be covered" by Peripheral Manager?

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

@VojtechBartoska Main of the issue is being fixed by that linked PR. In 5.1-libs branch this is already fixed and even the second part (UlliBien's comment) is already fixed there. So probably we can mark it like that.

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

Closing this as fixed by #8512 and in 5.1 branch this issue no longer exists as commented before.