force new enums for pinMode and pinStatus to 8 bits type instead of default int

[JM-FRANCE]

I've seen

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;

This will let the compiler decide how big the backend storage needs to be for those, and it will default to an int (16 or 32 bit depending on architecture). There is no need for this waste of memory and it prevents also some compiler optimization

I suggest forcing the uint8_t type, 256 values should be plenty to represent a pinMode or a pinStatus..

proposed changes:

typedef enum : uint8_t  {
  LOW     = 0,
  HIGH    = 1,
  CHANGE  = 2,
  FALLING = 3,
  RISING  = 4,
} PinStatus;

typedef enum : uint8_t {
  INPUT           = 0x0,
  OUTPUT          = 0x1,
  INPUT_PULLUP    = 0x2,
  INPUT_PULLDOWN  = 0x3,
} PinMode;

but in practice I would vote against getting these changes in production. There is no value added and tons of unwanted consequences for existing code out there to make this worthwhile.

[facchinm]

Hi @JAY-M-ERICSSON ,
you are indeed very right. Would you mind submitting a pull request so we can give the right attribution? Thank you!

[Floessie]

Isn't that a C file rather than a C++ header?

[matthijskooijman]

These values should indeed be usable from C as well as C++, so that won't help. However, I think there is limited gain in having e.g. PinMode be an alias of the actual enum, rather than just uint8_T, with a separate (unnamed) enum for its values (unlike an enum class in C++, which have actual advantages for scoping and warnings on missed cases in switches, etc.).

So maybe an alternative fix would be to just do e.g.

enum {
  INPUT           = 0x0,
  OUTPUT          = 0x1,
  INPUT_PULLUP    = 0x2,
  INPUT_PULLDOWN  = 0x3,
};
typedef uint8_t PinMode;

This would also address #25 (now also for C code), but again lose the additional type safety that these types were intended to provide... However, I'm afraid that between extra type safety, backward compatibility and single-byte types, we'd have to choose 2...

Or, maybe not, it seems you might be able to do this with the (non-standard) __attribute__((__packed__)) on the enum or -fshort-enums on the commandline (where the latter is more invasive, so the former is probably better). https://stackoverflow.com/questions/4879286/specifying-size-of-enum-type-in-c

[JM-FRANCE]

So maybe an alternative fix would be to just do e.g.

enum {
  INPUT           = 0x0,
  OUTPUT          = 0x1,
  INPUT_PULLUP    = 0x2,
  INPUT_PULLDOWN  = 0x3,
};
typedef uint8_t PinMode;

probably still better to force the type of the enum (as mentioned above)?

enum : uint8_t {
  INPUT           = 0x0,
  OUTPUT          = 0x1,
  INPUT_PULLUP    = 0x2,
  INPUT_PULLDOWN  = 0x3,
};

it will default to an int otherwise

[matthijskooijman]

Wait, it seems I misread your proposal. I thought you wanted to switch to C++ enum class, which supports defining an underlying size. I had not realized that normal enum apparently allows this too. For C++ this is a C++11 addition, though and some googling does not turn up any support in C for this. Did you check this still works in C?

If not, maybe an #if defined(__cplusplus__ >= 201103L) could be used, falling back to untyped, or packed enum otherwise? Or maybe just always use packed?

[JM-FRANCE]

Hi

I missed your point of C compatibility. You are 100% right. Won’t do.

My view is that this is for arduino and the compiler is C++ (and so many underlying libraries are Classes) that we should not worry about support in C (I've not double checked but would assume this is getting compiled with the C++ compiler)

That being said, I’m not very supportive of moving this to a type, I don’t think it’s worth all the trouble with so much legacy code around .

Even if I’m a big fan of strongly typed variables, The value added is limited