adafruit / ArduinoCore-samd

114 stars 116 forks source link

PINS_COUNT is useless and inconsistent. #268

Open WestfW opened 3 years ago

WestfW commented 3 years ago

in the variants.h file, PINS_COUNT says it should be the number of pins in the array:

// Number of pins defined in PinDescription array
#define PINS_COUNT           (26u)

However, this rarely true, with the advent of internal pins, semi-internal pins, aliases for pins, and so on. This makes PINS_COUNT useless for error-checking a passed value in library code (for example.) Worse, the meaning seems to vary from board to board, even when the boards are supposed to be somewhat compatible WRT pin numbering...

Also, nothing in the core actually USES PINS_COUNT

gid PINS_COUNT
variants/circuitplay/variant.h:40:#define PINS_COUNT           (39u)
variants/crickit_m0/variant.h:56:#define PINS_COUNT           (26u)
variants/feather_m0/variant.h:56:#define PINS_COUNT           (26u)
variants/feather_m0_express/variant.h:56:#define PINS_COUNT           (42u)
variants/feather_m4/variant.h:60:#define PINS_COUNT           (40u)
variants/gemma_m0/variant.h:56:#define PINS_COUNT           (16u)
variants/grand_central_m4/variant.h:60:#define PINS_COUNT           (96)
variants/hallowing_m0_express/variant.h:56:#define PINS_COUNT           (42u)
variants/hallowing_m4/variant.h:60:#define PINS_COUNT           (50u)
variants/itsybitsy_m0/variant.h:56:#define PINS_COUNT           (26u)
variants/itsybitsy_m4/variant.h:60:#define PINS_COUNT           (38u)
variants/metro_m0/variant.h:56:#define PINS_COUNT           (26u)
variants/metro_m4/variant.h:60:#define PINS_COUNT           (26u)
variants/metro_m4_airlift/variant.h:60:#define PINS_COUNT           (49u)
variants/monster_m4sk/variant.h:60:#define PINS_COUNT           (37u)
variants/pirkey/variant.h:56:#define PINS_COUNT           (22u)
variants/pybadge_airlift_m4/variant.h:60:#define PINS_COUNT           (55u)
variants/pybadge_m4/variant.h:60:#define PINS_COUNT           (51u)
variants/pygamer_advance_m4/variant.h:60:#define PINS_COUNT           (54u)
variants/pygamer_m4/variant.h:60:#define PINS_COUNT           (54u)
variants/pyportal_m4/variant.h:60:#define PINS_COUNT           (52u)
variants/pyportal_m4_titano/variant.h:60:#define PINS_COUNT           (52u)
variants/trellis_m4/variant.h:60:#define PINS_COUNT           (32u)
variants/trinket_m0/variant.h:56:#define PINS_COUNT           (22u)
variants/zero_radio/variant.h:56:#define PINS_COUNT           (51u)

(hey! I think the GCM4 is "correct"!)

This was fixed recently (well, relatively recently) in the arduino samd core: https://github.com/arduino/ArduinoCore-samd/commit/a536cc80168b703a0f1fc446e5c8e942f90df6c1#diff-d93fff30801193a3f0511f85af4a2c56ad1056e133224838d32a171497f2af47 I don't particularly LIKE the way they did it (who needs a function when this could just be an int in variant.cpp, for example), but the adafruit SAMD cores ought to do something...

ladyada commented 3 years ago

thanks, please submit a PR for fixed pins if you care about it

WestfW commented 3 years ago

Sigh. Do y'all have a recommended procedure for pulling a "core", editing it, and actually testing the new code? Since the core doesn't include the rest of the IDE, manual testing, and there's (maybe?) and api directory to worry about, live testing seems harder than I'd hoped. Patching this involves editing a lot of files that apply to hardware that I don't have, so I can look at it and think its fine, but I worry that I'll be overlooking something.

Do you mind if I don't do it the same way that Arduino did it? Do you ever merge the Arduino samd core edits into the adafruit core? Should I change the "arduino" variants in the adafruit core, or leave them?

BTW, the motivation for this is: https://github.com/WestfW/Duino-hacks/tree/master/SAMD_Explorer - a sketch/function/eventually-library for examining SAMD internal state for debugging and educational purposes...

ladyada commented 3 years ago

you can git fork this repo in to ~/Arduino/libraries/hardware/Adafruit folder and it will appear in the boards dropdown as "In Sketchbook" we do sometimes sync with Arduino, its overdue but we try to do it once a year or so

drewfish commented 3 years ago

@WestfW I've also written a library to dump the device configuration, I just iterated over PINS_COUNT: https://github.com/drewfish/arduino-ZeroRegs/blob/main/src/ZeroRegs.cpp#L1433

I've noticed that Arduino (well, Wiring) provides a clean interface for beginners/hobbyists, but the "deeper" API doesn't seem to be too well thought out for "intermediate" developers, folks writing libraries, etc. (I see a lot of libraries fall back to custom written bitbanging if they don't have access to hardware SPI, for example.) That might just be something we're stuck with for historical reasons. Also on embedded devices there's not a lot of overhead for a bunch of layers of abstraction.