fix: pin value can be higher than 127 so type should be uint8_t

[fpistm]

Hi,

Issue was found by several STM32 users.
Several STM32 have more than 127 digital I/0's.
Moreover, STM32 uses some pin alias PYn (PA0, PC13,...) to ease usage, alias with analog capabilities start from 0xC0 so
alias was also unusable to define TFT_CS, TFT_RST, TFT_DC.

To go further, ArdunioCore-API defined it as uint8_t by default:
https://github.com/arduino/ArduinoCore-API/blob/173e8eadced2ad32eeb93bcbd5c49f8d6a055ea6/api/Common.h#L91

So I guess it is more compliant to use uint8_t.
-1 default values are kept and check is done on 0xFF

Tested with SPI ST7789 and ST7735 devices.

[ag88]

i'd like to suggest Adafruit can use int16_t for pin numbers, this would prevent pin number > 127 from being treated as negative numbers. of course it'd not avoid all cases of issues but it'd avoid a majority of them.
i think it is quite common to use port number + pin number as a 'fast io' means to access ports (this maps hardware registers), so a common way is upper 4 bits is the port number and lower 4 bits is the pin numbers, this will go on a collision course with int8 if that combination so happens to be > 127

[fpistm]

I've fixed the Clang issue.

[PaintYourDragon]

Sometimes a negative pin index is used to indicate that certain items (usu. reset or CS) aren’t connected to the MCU. Although the proposed changes will catch -1’s, I’m not certain that this is the only case, there may have been other negative values/meanings in certain code or libraries. Also this might generate lots of warnings or errors in any SPITFT subclasses that are still using int8_t, unless every single one of them is updated to cast pins to unsigned.

Possibly a more robust solution…and something I’d been considering a while back and just didn’t want to break everything…would be to introduce something like a GFX_pin_t, and typedef it to an int16_t only on whatever device(s) have pin ID’s > 127, and an int8_t everywhere else by default. Existing subclasses, since int8_t and GFX_pin_t are the same thing nearly everywhere, would then compile without complaint (I think)…and then one by one any SPITFT subclasses could be updated to use GFX_pin_t as the need arises.

Either approach requires eventually updating every SPITFT subclass, I don’t have a painless solution here. A third also-non-optimal solution would be only pins <= 127 are supported (but IIRC there was some chip or family, maybe it was certain STM32s, where pin numbers also incorporated the PORT # and thus are nearly always big 12-bit values).

[fpistm]

Hi @PaintYourDragon
Thanks for your feedback. I know a change like this would impact several libraries as several relies on this one.
Anyway, I think this issue should be managed as it is not inline with Arduino pin type.

GFX_pin_t is a good idea but in this case why not simply use the same name than Arduino: pin_size_t ?

Either approach requires eventually updating every SPITFT subclass, I don’t have a painless solution here. A third also-non-optimal solution would be only pins <= 127 are supported (but IIRC there was some chip or family, maybe it was certain STM32s, where pin numbers also incorporated the PORT # and thus are nearly always big 12-bit values).

As stated, the STM32 core use the full range of uint8_t to defined alias pin number (PYn) as analog (0xC0 based) so using those alias will not work.
For example, the Adafruit Feather F405, those pins will not be usable using PYn naming:
https://github.com/stm32duino/Arduino_Core_STM32/blob/6d5668f95b95bfb1070a7c63f90a6ad33d9a25c4/variants/STM32F4xx/F405RGT_F415RGT/variant_FEATHER_F405.h#L41-L47

but OK this is not the standard Arduino naming, just a convenient way to use a pin. 😉
Anyway using Dx or x will work until 127.

About 12 bits, in STM32 core it does not exists. I take care about this to be Arduino compatible.

[erlef10]

When will this changed be merged?

I have just had a longer debug session due to this issue.

My finding btw is that the problem is solved by using the digital definitions of the pin ea Dx and not the analog definition PAx.

I do not appose the change, but for future reference and other googlers, using the digital pin definition may solve your issue. At least for the STM32F4 generic definitions.