Breakage caused by PinStatus and PinMode types

[per1234]

This project changes LOW, HIGH, INPUT, INPUT_PULLUP, and OUTPUT from macros to enums:

ArduinoCore-API/api/Common.h

Lines 10 to 23 in e1eb8de

	typedef enum {
	LOW = 0,
	HIGH = 1,
	CHANGE = 2,
	FALLING = 3,
	RISING = 4,
	} PinStatus;
	typedef enum {
	INPUT = 0x0,
	OUTPUT = 0x1,
	INPUT_PULLUP = 0x2,
	INPUT_PULLDOWN = 0x3,
	} PinMode;

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

An example is the popular Keypad library. Compilation of both the original library and the version in Library Manager 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:

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.

Some previous discussion on the topic:

• http://forum.arduino.cc/index.php?topic=455579 (from here 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
• digitalWrite has a wrong signature ArduinoCore-megaavr#68

[PaulStoffregen]

I can confirm more code in the wild which does an ifdef check on INPUT_PULLUP (and falls back to pinMode & digitalWrite), because INPUT_PULLUP was added to Arduino later than the others.

[matthijskooijman]

I can confirm more code in the wild which does an ifdef check on INPUT_PULLUP (and falls back to pinMode & digitalWrite), because INPUT_PULLUP was added to Arduino later than the others.

One way to fix that, without sacrificing the enum, is to add #define INPUT_PULLUP INPUT_PULLUP. I think we've used this trick for other macros converted to constants in the AVR core before (or at least discussed it).

As for the convert-from-boolean point, I guess one could introduce a class type to wrap these constants and add the appropriate conversion operators on them, though that might become a bit more complex than one would like. It does keep the advantage of separate types for each of these. It would also allow splitting PinStatus and SignalFlank (or whatever), and still allow using LOW and HIGH for both by adding implicit conversions.

[Coding-Badly]

The digitalRead can be handled with an operator...

bool operator ! (const PinStatus &v)
{
  return v == HIGH ? LOW : HIGH;
}

[Coding-Badly]

The digitalWrite can handled with an overload...

void digitalWrite( unsigned pin, PinStatus value )
{
...
}

void digitalWrite( unsigned pin, bool value )
{
  digitalWrite( pin, value ? HIGH : LOW );
}

Ideally, the unsigned / bool function is placed in a header file and marked to be inlined.

[Floessie]

Maybe a stupid question: Why does digitalWrite() take a PinStatus at all? CHANGE, FALLING, and RISING make no sense, or do they? As PinStatus is a class-less enum, providing only a void digitalWrite(unsigned pin, bool value) would solve the problem IMHO.

[WestfW]

I agree with Floessie - having digitalRead/digitalWrite use pinStatus() is no better than the code that expects/passes booleans. There is no reason to overload the enum with functionality from two separate sets of functionality...

[per1234]

Today another report came in of a popular library broken by this:
https://bitbucket.org/fmalpartida/new-liquidcrystal

Minimal demonstration sketch:

#include <LiquidCrystal_I2C.h>
void setup() {}
void loop() {}

E:\arduino\libraries\Newliquidcrystal_1.3.5\LiquidCrystal.cpp: In member function 'virtual void LiquidCrystal::send(uint8_t, uint8_t)':

E:\arduino\libraries\Newliquidcrystal_1.3.5\LiquidCrystal.cpp:125:48: error: cannot convert 'bool' to 'PinStatus' for argument '2' to 'void digitalWrite(pin_size_t, PinStatus)'

    digitalWrite( _rs_pin, ( mode == LCD_DATA ) );

                                                ^

E:\arduino\libraries\Newliquidcrystal_1.3.5\LiquidCrystal.cpp: In member function 'void LiquidCrystal::writeNbits(uint8_t, uint8_t)':

E:\arduino\libraries\Newliquidcrystal_1.3.5\LiquidCrystal.cpp:305:48: warning: invalid conversion from 'int' to 'PinStatus' [-fpermissive]

       digitalWrite(_data_pins[i], (value >> i) & 0x01);

                                                ^

In file included from C:\Users\per\AppData\Local\Arduino15\packages\arduino\hardware\megaavr\1.6.210\cores\arduino/api/ArduinoAPI.h:52:0,

                 from C:\Users\per\AppData\Local\Arduino15\packages\arduino\hardware\megaavr\1.6.210\cores\arduino/Arduino.h:23,

                 from E:\arduino\libraries\Newliquidcrystal_1.3.5\LiquidCrystal.cpp:36:

C:\Users\per\AppData\Local\Arduino15\packages\arduino\hardware\megaavr\1.6.210\cores\arduino/api/Common.h:104:6: note:   initializing argument 2 of 'void digitalWrite(pin_size_t, PinStatus)'

 void digitalWrite(pin_size_t pinNumber, PinStatus status);

      ^

Reported at:
http://forum.arduino.cc/index.php?topic=595987

[facchinm]

Could something like this work for everyone? (it is from the namespace_arduino branch, hence the namespace stuff 🙂

ArduinoCore-API/api/Compat.h

Lines 1 to 16 in 55cbc08

	#ifndef __COMPAT_H__
	#define __COMPAT_H__
	namespace arduino {
	inline void pinMode(pin_size_t pinNumber, int mode) {
	pinMode(pinNumber, (PinMode)mode);
	};
	inline void digitalWrite(pin_size_t pinNumber, int status) {
	digitalWrite(pinNumber, (PinStatus)status);
	};
	}
	#endif

[per1234]

@facchinm this is probably a dumb question, but it's not clear to me exactly how this would be implemented. I get the idea of overloading the functions. But pinMode and digitalWrite are C, which doesn't support function overloading in this manner.

I was trying to modify the Arduino megaAVR Boards core code to test this solution against the known issues reported so far in the thread. I tried just replacing the api folder with the one from the namespace_arduino branch of ArduinoCore-API, but then I get the same issues as well as some new unrelated errors.

[facchinm]

@per1234 it's not a dumb question at all!
In the namespace branch all implementations are C++ only, and only one of them is declared extern "C" so it can be used from any C file.
In this case, the (non compact) implementation gets exposed, while the other one is only available in C++ files.
Maybe it's not the most elegant approach but it should solve 99% of the problems (if the nonstandard code lives in the sketch or in a c++ library)

[per1234]

Thanks for the explanation! I agree that the amount of C code in existence affected by this issue is probably very small. I hadn't considered that the solution would be limited to C++/.ino.

[MCUdude]

I'll have to agree with @WestfW and @Floessie on this one.

I understand that @facchinm tries to provide a solution that will keep the new implementation, but what do we need this new implementation in the first place? It only leaves ut with a workaround that's not compatible with C libraries, not any benefits as far as I can see.

[JChristensen]

I'd be interested to know what problem is solved by introducing the PinStatus type. If this is about type safety, then I think there is such a thing as getting carried away. For example, it seems perfectly natural to pass a bool as the second argument to digitalWrite(). In that case, introducing PinStatus is not only confusing but seems like reinventing the wheel, as the language already provides a completely adequate type. If in fact this change is about type safety, then I'd have to wonder how big the problem was in the first place.

Perhaps worse, the current situation is inconsistent between various boards, with megaAVR architecture demanding a PinStatus type (e.g. for digitalWrite), and regular AVR being more permissive. In fact, PinStatus is not defined for the AVR architecture.

[Coding-Badly]

Jack! I hope you've been well!

Excellent question. I can imagine two possibilities.

1. bool is severely limited in the amount of information that it can carry. A pin status type can carry the same information and later, if necessary, be changed to carry additional information. It's a way of "future proofing" the interface.

2. Having a type specific to pin status allows overloading. It is possible for digitalWrite(pin, bool) to have different behaviour than digitalWrite(pin, PinStatus). (I can't think of a valid reason to do that but I also have not put much thought into it.)

[bxparks]

Question 1:

Looking at $ARDUINO_IDE/portable/packages/arduino/hardware/megaavr/1.8.1/cores/arduino/wiring_digital.c, I see that digitalWrite() now accepts 3 values of PinStatus (HIGH, LOW and CHANGE). It seems that CHANGE causes the pin to toggle, and CHANGE is defined to be an enum of 2.

However, on a normal AVR, CHANGE is defined to be 1 (see $ARDUINO_IDE/hardware/arduino/avr/cores/arduino/Arduino.h) so digitalWrite(pin, CHANGE) compiles, but has the same effect as digitalWrite(pin, HIGH).

Is this intended?

Question 2:

Is there a #define macro that identifies the megaAVR boards which use this new PinMode and PinStatus API, so that I can fix my library for the megaAVR?

[facchinm]

Hi @bxparks

1. probably implementing CHANGE as pin toggle was not the best idea ever; anyway, not using it doesn't hurt 🙂 What's your use case?
2. you can target any core implementing PinMode and PinStatus with #if (ARDUINO_API_VERSION >= 10000)

[bxparks]

Hi @facchinm,

I don't use digitalWrite(pin, CHANGE)... I was trying to deduce the valid range of PinStatus for the new digitalWrite() and digitalRead(). Previously both of them were documented to handle only 2 values: HIGH and LOW. Now digitalWrite() accepts 3 values. So I'm wondering, will digitalRead() ever return any other values of PinStatus?

The reason for asking is that my AceButton library stores a value of digitalRead() that represents "UNKNOWN", in other words, the state of the pin before any digitalRead() has been performed. But the documentation for digitalRead() does not define the value of HIGH or LOW, so I have a compile-time check like this:

#if HIGH != 1
  #error HIGH must be defined to be 1
#endif
#if LOW != 0
  #error LOW must be defined to be 0
#endif

Which enforces that my "UNKNOWN" value is different from either HIGH or LOW. Obviously for the megaAVR, these checks no longer work. And obviously, I cannot use PinStatus to store my digitalRead() result, because "UNKNOWN" cannot be represented by the enum.

Thanks for the pointer to ARDUINO_API_VERSION. Is it correct to assume that eventually even the normal AVRs will migrate to the new API, with the PinStatus and PinMode enums?

[facchinm]

The whole idea of HIGH and LOW is to hide the implementations details (HIGH could be 0 and LOW 0x12 in some random funky architecture, but your Arduino code will run seamlessly).
Expecting them to be 0 and 1 (with no other value allowed) breaks this assumption, so maybe there's a better way to enforce the UNKNOWN field 😉

[PaulStoffregen]

Overloading digitalWrite will break much of the Arduino ecosystem. A tremendous amount of code has been built up over the last decade equating digitalWrite & digitalRead values with the C/C++ convention of zero representing logical false to mean LOW and non-zero representing logical true to mean HIGH.

[facchinm]

@PaulStoffregen of course most (all?) the architectures are sane and HIGH is 1 while LOW is zero but since the APIs don't specify the value of those macros/enum/whatever (and never did) the code runs "by chance" 🙂

[bxparks]

I guess I'm having a hard time seeing what advantages the PinStatus API offers that outweighs the cost breaking backwards compatibility. We've already seen that the one new feature that it does provide, the ability to toggle using digitalWrite(pin, CHANGE) cannot be used, since it breaks compatibility with all other platforms.

I also understand that writing:

pinStatus = (pinStatus != LOW)? LOW : HIGH;

is probably more clear to most people than

pinStatus ^= 0x1;

but it would be nice to have the option of writing the second form in places where it makes sense (e.g. deep inside a library where app developers are not expected to visit often). The XOR can save a few bytes of flash memory and CPU cycles, since the compiler cannot optimize the first into the second. (Interestingly, when I wrote the first version using ?: operator, I actually wrote it wrong initially and flipped the LOW and HIGH. That expression is not 100% easy to get correct. Hmm.)

With regards to UNKNOWN, yes it's always possible to use a second byte to store the bool isUnknown flag. But that's using 2 bytes to store 2 bits of information (LOW, HIGH, UNKNOWN), instead of just one byte. Now, if PinStatus contained a PinStatus::UNKNOWN whose value was guaranteed to be never returned by digitalRead(), then PinStatus would actually provide some value to me.

In any case, it looks like the new PinStatus API was introduced almost 2 years ago, so the ship has sailed? I will use

#if (ARDUINO_API_VERSION >= 10000)
...
#endif

to target different platforms appropriately.

[WestfW]

the APIs don't specify the value of those macros/enum/whatever (and never did) the code runs "by chance"
wiring_shift.c in the arduino core ( https://github.com/arduino/ArduinoCore-avr/blob/master/cores/arduino/wiring_shift.c#L32 ) passes a boolean to digitalWrite(), and the Firmata library uses 0/non-zero: https://github.com/firmata/arduino/blob/master/Boards.h#L970

[PaulStoffregen]

That sort of code is pervasive throughout the Arduino ecosystem.

I fear these API changes could bring widespread misery.

[bxparks]

I tried to protect myself against those assumptions with my compile-time checks, e.g.:

#if HIGH != 1
  #error HIGH must be defined to be 1
#endif
#if LOW != 0
  #error LOW must be defined to be 0
#endif

But even that breaks under this new API. :-/

[WestfW]

That's presumably because HIGH and LOW are now part of an enum rather than mere constants.

You can probably do:

void unexpected_HIGH_or_LOW_value(void) __attribute__ (( error("") ));
// ...
void MyFunction (...) {
   if (HIGH != 1 || LOW != 0)
      unexpected_HIGH_or_LOW_value();
//    ...
}

Since HIGH and LOW are still compile-time constants, this won't normally produce any actual code, but they no longer need to be preprocessor-time constants. (There are a bunch of places where it might be useful to use this sort of thing in the Arduino code. But that's for another time/issue!)

[bxparks]

(This is getting slightly off topic, but hopefully it's useful to someone else facing a similar problem with compile-time checking of HIGH and LOW values)

@WestfW: Thanks for the pointer! I didn't know about the error attribute. I discovered 3 problems with your solution unfortunately:

1. The compiler complains about an unused MyFunction() if this check is the only code inside.
2. I have to create a new unexpected_HIGH_or_LOW_value() function for each compile-time check that I want to perform.
3. The error attribute is a g++ only extension, clang++ does not recognize it. (This is a problem for me because I try to write portable code, so that I can compile and run my Arduino unit tests on Linux (g++ or clang++) and MacOS (clang++)).

After reading the GCC guide on __attributte__ on the extern char [(condition) ? 1 : -1]; trick, and this blog on catching compile-time errors, I came up with the following:

#define CONCAT_(x, y) x##y
#define CONCAT(x,y) CONCAT_(x,y)
#define COMPILE_TIME_ASSERT(cond, msg) \
    extern char CONCAT(compile_time_assert, __LINE__)[(cond) ? 1 : -1];
COMPILE_TIME_ASSERT(HIGH == 1, "HIGH must be 1")
COMPILE_TIME_ASSERT(LOW == 0, "LOW must be 0")

Works on both a #define HIGH and an enum value of HIGH.

[bxparks]

For what it's worth, I decided to explicitly blacklist the ArduinoCore-API for most the 15-20 Arduino libraries that I publish publicly, using something like the following at the top of the main header file:

#if defined(ARDUINO_API_VERSION)
#error Platforms using ArduinoCore-API not supported
#endif

One, it gives a short, understandable error message to the end-user, instead of hundreds of lines of inscrutable compiler errors.

Two, it completely stopped the repeated bug tickets against my libraries, saying "Your libraries does not work with [MegaAVR | Nano Every | RaspberryPi Pico | MKR | etc...]".

Three, it saved me dozens (maybe hundreds?) of hours of development and debugging time that I don't have to spend porting my libraries to the new Arduino API, and hundreds of dollars for buying the various microcontrollers that I no longer have to test and validate.

[PaulStoffregen]

FWIW, I'm keeping an eye on this with concern about how this is likely to affect the Arduino ecosystem, and eventually what I'll need to support on Teensy boards.

[drf5n]

I like that the PinStatus enum names LOW = 0 and HIGH = 1 as used in the text of :

https://docs.arduino.cc/built-in-examples/basics/DigitalReadSerial

from

https://github.com/arduino/docs-content/blob/main/content/built-in-examples/01.basics/DigitalReadSerial/DigitalReadSerial.md

Now that your setup has been completed, move into the main loop of your code. When your button is pressed, 5 volts will freely flow through your circuit, and when it is not pressed, the input pin will be connected to ground through the 10k ohm resistor. This is a digital input, meaning that the switch can only be in either an on state (seen by your Arduino as a "1", or HIGH) or an off state (seen by your Arduino as a "0", or LOW), with nothing in between.
...
Now, when you open your Serial Monitor in the Arduino Software (IDE), you will see a stream of "0"s if your switch is open, or "1"s if your switch is closed.

and the

     thirdSensor = map(digitalRead(2), 0, 1, 0, 255);

line from https://docs.arduino.cc/built-in-examples/communication/SerialCallResponse

It doesn't seem fair to call depending on LOW=0/HIGH=1 as bad code if the reference documentation has been pushing it for years

On the other hand, isn't this PinStatus enum inconsistent with the either-or text from https://www.arduino.cc/reference/en/language/functions/digital-io/digitalread/ and
https://www.arduino.cc/reference/en/language/functions/digital-io/digitalwrite/

And doesn't this break expectations for this code:

https://github.com/arduino/ArduinoCore-megaavr/blob/ffab9cb2e4bca7647f11d6e25727788aec597a03/cores/arduino/wiring_digital.c#L148-L161

If you call digitalWrite(13,FALLING); should it set the pin or clear the pin?

[SpenceKonde]

IMO this whole concept of pin enums was something that would be great if we were starting from scratch low end ARMs where we could afford to piss away some flash like that. But we''er on AVRs, there's a long established tradition of the 0/1 being legal for digitalWrite, and it's easy to come up with plausible use cases and common ideoms where it falls over.

That's why I 86'ed it from my cores. (the whole Arduino API was sort of a disaster IMO - at this point it's been pretty much gutted on my cores as people have complained (with good reason) about the bloat it introduced (virtual functions are the devil's creation).

[WestfW]

If we were starting from scratch, "pins" should probably have been objects.

[aentinger]

virtual functions are the devil's creation

Why are virtual functions the devils creation? Please inform me about the performance cost you think they'll incur ;)

Note: I'll grant you that they do incur a cost, but not as great as many people apparently think.

[bperrybap]

One issue with virtual functions is that they will get linked in regardless if they are used.
The compiler & linker options that are used to remove unused functions don't work for virtual functions and while there was a way (hack) to patch the vtable to allow the linker to remove unused fucntions, that no longer works in the more recent gcc toolsets.

[ianfixes]

I'm only just running into this issue in Arduino CI; I found this thread by running a google search for the actual definition of digitalWrite, and so far this discussion is very discouraging.

Arduino CI enables unit testing an Arduino library by mocking the entire set of Arduino built-ins, such that they can be fed scripted inputs and have their outputs analyzed. See the definitions of HIGH and LOW and digitalRead/digitalWrite respectively. (I'm not sure how correct these are, but they have been working for quite some time as various commenters in this thread can personally attest to.)

To make code more testable in a library that I am writing, I prefer to follow the dependency injection pattern. This simplifies the problem of testing, since I can precisely simulate the inputs I want my library to handle by writing a mocked version of digitalRead (rather than manually energizing wires attached to real hardware and looking at the serial port monitor). All I need to do to support Dependency Injection is declare the precise type signature of the digitalRead function in the library, and require the sketch to pass a reference to digitalRead to the library as part of setup().

But what is the correct type signature of this function???

For this to have 2 different signatures for 2 different boards spells disaster for any library that wants to work for multiple boards -- it looks like I have to pick one signature or the other, and using preprocessor directives to sort this out seems like a very unsustainable way to go.

How can I encapsulate the possibly changing signature of this function in (1) an Arduino library that I write and (2) my mocks for testing Arduino libraries as part of Arduino CI?

[ianfixes]

Addendum, even overloading isn't an obvious solution here, because in some cases PinStatus won't even be defined.

[bperrybap]

For this to have 2 different signatures for 2 different boards spells disaster for any library that wants to work for multiple boards -- it looks like I have to pick one signature or the other, and using preprocessor directives to sort this out seems like a very unsustainable way to go.

How can I encapsulate the possibly changing signature of this function in (1) an Arduino library that I write and (2) my mocks for testing Arduino libraries as part of Arduino CI?

Number 1 rule: never abuse the API and use the APIs the way they are documented.
(using a specific type for HIGH and LOW is abusing the API)
You have to treat HIGH and LOW as an opaque unknown type.

• never assume that their type is anything and could change.
• never attempt to assign HIGH or LOW to a variable and never use a variable as the value parameter to digitalWrite()

You do things like when you call functions like digitalWrite() always use HIGH or LOW for the value parameter, not a variable.

For digitalRead() always compare the return value to LOW or HIGH and then do things based on that evaluation.
i.e. if LOW set a variable to some value, if HIGH then set it to some other value.
When using your variable assigned based on return value of digitalRead() to set a pin using digitalWrite(), look at the variable and then call dititalWrite() with LOW or HIGH

My libraries do this and it didn't have any issues with the different types for HIGH and LOW.
It works on all boards within all cores and the types for HIGH and LOW can change and it won't affect my code compiling and working.

If you are trying to do something special or out of the ordinary for testing / emulation purposes, again you cannot ever assume any type for the symbols HIGH and LOW given the way the API is documented and defined.

If you for some reason absolutely need their type you can get to them by using decltype()
i.e decltype(HIGH) or decltype(LOW)

So I would think that between a clever combination of use of macros to remap function names and the use decltype() you could get to anything you want / need for your simulation / emulation purposes.

stuff like

decltype(HIGH) my_digitalRead(int pin);
void my_digitalWrite(int pin, decltype(HIGH));
decltype(HIGH) value; // this variable can be used with digitalRead() and digitalWrite()

You could even create a typedef for the decltype(HIGH) that would hide this away

[Michael-Brodsky]

Untyped object-like macros, IMHO this is half the problem with the API.

…

On Tue, May 2, 2023 at 8:59 AM Ian ***@***.***> wrote: I'm only just running into this issue in Arduino CI <https://github.com/Arduino-CI/arduino_ci>; I found this thread by running a google search for the actual definition of digitalWrite, and so far this discussion is very discouraging. Arduino CI enables unit testing an Arduino library by mocking the entire set of Arduino built-ins, such that they can be fed scripted inputs and have their outputs analyzed. See the definitions of HIGH and LOW <https://github.com/Arduino-CI/arduino_ci/blob/6f326f8070e619802ff6efe07bc3340313806d1f/cpp/arduino/ArduinoDefines.h#L5-L6> and digitalRead/digitalWrite <https://github.com/Arduino-CI/arduino_ci/blob/6f326f8070e619802ff6efe07bc3340313806d1f/cpp/arduino/Godmode.h#L166-L167> respectively. To make code more testable, I prefer to follow the dependency injection <https://allencch.wordpress.com/2017/12/05/c-unit-test-and-dependency-injection/> pattern. This simplifies the problem of testing, since I can simply write a mocked version of digitalRead and use it to precisely simulate the inputs I want my library to handle. All I need to do is declare the precise type of the digitalRead function. But what is the correct type signature of this function??? For this to have 2 different signatures for 2 different boards spells disaster for any library that wants to work for multiple boards -- it looks like I have to pick one signature or the other, and using preprocessor directives to sort this out seems like a very unsustainable way to go. How can I encapsulate the possibly changing signature of this function in (1) an Arduino library that I write and (2) my mocks for testing Arduino libraries as part of Arduino CI? — Reply to this email directly, view it on GitHub <#25 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANSCUI5C3ZFFPIRA74LY3VLXEEOL5ANCNFSM4GM5J24Q> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[SpenceKonde]

Thanks for the tip on decltype()

@Michael-Brodsky hits the nail on the head - #define was horribly overused throughout the API. And where things were typed, int's were overused and uint8_t's underused.... But now it is too late to fix most of this stuff - there's too much code in the wild, and we long ago reached the point where the API was set in stone effectively. We can't go changing it without destroying compatibility short of straight-up magic, where one could shout "abracadabra!"* while brandishing an illuminated, arduino powered wand, and have all the code in existence changed to match a new API

*I've been told that this was originally an invocation of the demon-god of numbers and math; I don't know enough about such mythology to know if that's true or not, but that's the demon upon whose power you'd need to call to retroactively correct an API. Based on the absence of anyone working such magic, I conclude that the demon must demand too high a price for such alterations of reality (I certainly would if I were such a demon - it looks like a lot of work, even with unholy supernatural powers you'd expect of that sort of entity). It could also be that demons like that don't exist, and we'd need a full on time machine, and physics has pretty much constrained those out of the realm of possibility (well, for backward time travel. Forwards time travel is easy. I just used a time machine to travel forwards a half hour writing this crap)

[PaulStoffregen]

If ease of maintenance for volunteer efforts isn't the goal, I'm open to hearing that argument as well.

Arduino's incredible success has been attributed to design for ease of learning and lowering barriers for new users who may have little or no programming experience. Traditionally, removing novice user barriers has been of paramount importance. In the many years Arduino had a developer's mail list, over and over heated discussions involved programmers arguing Arduino should more closely follow certain programming standards. Time and time again, the Arduino developers would explain Arduino's not-so-standard design choices to simplify learning, usually commenting that numerous other far more powerful IDEs exist on the market for professional programmers who want standards compliance, and that people who choose Arduino IDE do so because it is an alternative where very different design choices were made to prioritize ease of learning.

Maybe Arduino's goals have shifted in recent years? I really don't know. They certainly are now selling "Pro" products. I don't speak for Arduino, and since the pandemic I've not been to any conferences where I've had an opportunity to speak with Arduino people in person.

But regarding what is and isn't the goal, your (perhaps rhetorical) question, traditionally Arduino's overriding design goal has been simplicity for novice users, especially removing barriers beginners and non-programmers experience while using it to accomplish their projects.

Seeing all this focus on ISO C++ standard compliance, done in a manner which causes compile errors with so many programs already published in the Arduino ecosystem, really feels like Arduino priorities & values may have shifted from the formula that paved Arduino's popularity and success in a market filled with far more advanced and standards compliant programming tools.

[ianfixes]

Arduino's overriding design goal has been simplicity for novice users, especially removing barriers beginners and non-programmers experience while using it to accomplish their projects

I think we are in perfect agreement here, and would point to the 5833 libraries in the library manager as evidence of how many people are working to make the novice user experience more accessible. Would you agree that the novice user is the one who ultimately benefits from improvements to the quality and breadth of libraries (in that they can avoid writing all of that code from scratch, for any given algorithm, data structure, or attached circuit)?

I'd also point out that there is an active community of developers; the forums suggest 372 forum topics per week are discussed. Would you agree that the ability for advanced users to demonstrate bugs through unit tests -- directly copy/pastable into a novice's project -- is at least as useful as an advanced user trying to explain the (mis)behavior of a code snippet in English?

TL;DR

I agree completely that removing barriers for novice users is the goal, and I don't mean to suggest any features that would detract from that goal. What I am suggesting is that at the individual level we lack continuity between the novice, intermediate, and advanced skill sets, and at the group level we lack some mechanisms that could greatly improve collaboration.

[runger1101001]

Would someone have a hint on how to write a generally compatible declaration like:

PinStatus polarity = RISING;
attachInterrupt(digitalPinToInterrupt(pin), callback, polarity);

but above fails on the frameworks that don't define PinStatus :-(

int polarity = RISING;
attachInterrupt(digitalPinToInterrupt(pin), callback, polarity);

but above fails on the frameworks that do define PinStatus :-(

I'm finding it really troublesome to maintain compatibility based on checking a bunch of #if defined(TARGET_RP2040) etc
as the frameworks change, and in some cases it seems to depend on the board used (e.g. Adafruit vs. Arduino) :-(

[bperrybap]

I would assume something like:
decltype(RISING) polarity = RISING;