Skip to content

Some cardputer definition improvements#9273

Merged
dhalbert merged 1 commit intoadafruit:mainfrom
bill88t:cardputer_gripes
May 25, 2024
Merged

Some cardputer definition improvements#9273
dhalbert merged 1 commit intoadafruit:mainfrom
bill88t:cardputer_gripes

Conversation

@bill88t
Copy link

@bill88t bill88t commented May 24, 2024

Some tiny improvements.

The grove port pins are labeled as G1 & G2 on the physical product.
The current definition has literally every other name for the pins, other than these.
Note: They have never been labeled as any of the other names in docs or on the physical product.
Since this board was added in 9.0.x and not 9.1 I think we should leave the rest of the other pin names as is.

The port itself is labeled PORT.A, not the pins.
IMG_2024-05-25-01-00-52-226

While I was at it I fixed the M5Stack branding to match their website and the board default hostname.
Also for compilation time savings, I disabled the MAX3421E module, since this board doesn't have enough exposed pins to do spi (it's literally only got G1 and G2 exposed).

@dhalbert
Copy link
Collaborator

@joshua-beck-0908
Copy link

@jamesjnadeau @CDarius @RetiredWizard @joshua-beck-0908 Take a look.

Just looked through the commit.
I'm in favour of this change.
It improves the consistency with what's actually written on the device.
Especially so for the uniformity of the board name, host name, and USB device name.

@jamesjnadeau
Copy link

Looks good to me, thanks for cleaning this up!

@RetiredWizard
Copy link

I'm phone access only (travelling 😀) so I didn't search the core to understand the MAX3421E parameter impact but the changes look good to me.

@CDarius
Copy link

CDarius commented May 25, 2024

Looks good to me too!

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks everyone for your reviews!

@dhalbert dhalbert merged commit 5cc8d44 into adafruit:main May 25, 2024
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

Successfully merging this pull request may close these issues.

6 participants