Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PINS_COUNT is useless and inconsistent. #268

Open
WestfW opened this issue Oct 22, 2020 · 4 comments
Open

PINS_COUNT is useless and inconsistent. #268

WestfW opened this issue Oct 22, 2020 · 4 comments

Comments

@WestfW
Copy link

WestfW commented Oct 22, 2020

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:
arduino@a536cc8#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
Copy link
Member

ladyada commented Oct 22, 2020

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

@WestfW
Copy link
Author

WestfW commented Oct 23, 2020

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
Copy link
Member

ladyada commented Oct 25, 2020

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
Copy link

@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.

WestfW added a commit to WestfW-patches/AdadfruitCore-samd that referenced this issue Oct 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants