analogRead buggy pin handling

[louisrubet]

This code is found in uint32_t analogRead(uint32_t pin):

uint32_t analogRead(uint32_t pin)
{
  (...)
  if (pin < A0) {
    pin += A0;
  }
  (...)

The pin passed as an argument is deliberately changed if it doesn't fulfill a certain requirement.
This seems inappropriate:

• This forces a particular (and arbitrary) order in the pin map (and the user doesn't know it),
• Variables called pin always refer to an index in g_APinDescription and never something else,
• It's present in analogRead but not in in analogWrite
• This induces issues, one example :
  • I change PA10 to analog on my mkrzero board by changing PIO_DIGITAL to PIO_ANALOG in g_APinDescription[2]
  • when I call analogRead(2) it does analogRead(17)

Imo it's really worth removing these 3 lines.

[matthijskooijman]

The usecase for this is that you can pass analogRead(0) and then read the first ADC/analog input pin A0. This was the original analogRead() API, passing pin numbers instead of ADC channel numbers was later added, and even though passing pin numbers is a better API, there is a lot of code out there that uses channel numbers, so both must be supported.

The reason it is present in analogRead and not analogWrite is because the latter has nothing to do with the ADC (and it is not even actually analog, just PWM).

I agree this situation sucks, but I think pin maps just have to deal with it / work around it, unfortunately...

[louisrubet]

Yes we can particularly well guess the reasons that lead to that.
Alas I was afraid the reason was about lots of existing code doing both.
I remove my PR 😭