arduino / arduino/ArduinoCore-API

Breakage caused by PinStatus and PinMode types

Offen
#25 69 Kommentare 2 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
bug
Vorherrschende Sprache
C++
Sterne
306
Forks
150
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Beschreibung

This project changes `LOW`, `HIGH`, `INPUT`, `INPUT_PULLUP`, and `OUTPUT` from macros to enums:
https://github.com/arduino/ArduinoCore-API/blob/e1eb8de126786b7701b211332dda3f09aa400f35/api/Common.h#L10-L23

I'm concerned that this will cause breakage of a significant amount of existing code.

An example is the popular [Keypad library](https://playground.arduino.cc/code/keypad). Compilation of both the [original](https://playground.arduino.cc/uploads/Code/keypad.zip) library and the [version in Library Manager](https://github.com/Chris--A/Keypad) fails once this change is made.
```
In file included from E:\electronics\arduino\libraries\Keypad-master\examples\HelloKeypad\HelloKeypad.ino:10:0:

E:\electronics\arduino\libraries\Keypad-master\src/Keypad.h: In member function 'virtual void Keypad::pin_write(byte, boolean)':

E:\electronics\arduino\libraries\Keypad-master\src/Keypad.h:81:81: error: cannot convert 'boolean {aka bool}' to 'PinStatus' for argument '2' to 'void digitalWrite(pin_size_t, PinStatus)'

virtual void pin_write(byte pinNum, boolean level) { digitalWrite(pinNum, level); }
```
This commonly used code will also now fail:
```c++
digitalWrite(13, !digitalRead(13)); // toggle pin 13
```
```
toggle:2:36: error: cannot convert 'bool' to 'PinStatus' for argument '2' to 'void digitalWrite(pin_size_t, PinStatus)'

digitalWrite(13, !digitalRead(13)); // toggle pin 13
```
I understand that the root cause of these errors is bad code and that any code which followed best practices will have no problems with this change. However, I fear there is a lot of bad code in widespread use that currently works fine. In the case of the Keypad library, it is unlikely it can even be fixed since Chris--A has gone AWOL. I'm sure there are other such abandoned projects.

I do like the spirit of this change (though lumping `CHANGE`, `FALLING`, and `RISING` into `PinStatus` is questionable). I'm open to being convinced that it's worth the breakage and, if so, I'm willing to help ease the transition by providing user support and submitting PRs to fix broken code. I just think this warrants some consideration before ArduinoCore-API goes into more widespread use.

### Additional context

Some previous discussion on the topic:

- http://forum.arduino.cc/index.php?topic=455579 (from [here](http://forum.arduino.cc/index.php?topic=455579.msg4001965#msg4001965) onward)
- https://forum.arduino.cc/index.php?topic=584322
- http://forum.arduino.cc/index.php?topic=602250
- https://forum.arduino.cc/index.php?topic=621429
- https://forum.arduino.cc/index.php?topic=627883.msg4319254#msg4319254
- https://forum.arduino.cc/index.php?topic=659624
- https://github.com/arduino/ArduinoCore-megaavr/issues/68

#### Related

- https://github.com/arduino/ArduinoCore-mbed/issues/1107

Beitragsleitfaden

Für dieses Repository ist kein Beitragsleitfaden indexiert

Rechercherichtung

Beginne mit api/Common.h und reproduziere die gemeldeten Kompilierungsfehler anhand der Keypad-Beispiele und des Falls digitalWrite(13, !digitalRead(13)). Sieh dir die verknüpften Diskussionen und das zugehörige Issue an, um die Kompatibilitätsentscheidung zu verstehen; dieses Issue ist erst abgeschlossen, wenn das Projekt eine Entscheidung erreicht und dokumentiert hat und das betroffene Kompatibilitätsverhalten verifiziert wurde.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
cpp
Bereich
api, embedded-iot
Issue-Typ
Bug
Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Aktivitätsstatus
Veraltet
Klarheit
Muss geklärt werden
Anfängerfreundlichkeit
25/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.