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

gBoards FaunchPad #8925

Closed
wants to merge 5 commits into from
Closed

gBoards FaunchPad #8925

wants to merge 5 commits into from

Conversation

germ
Copy link
Contributor

@germ germ commented Apr 26, 2020

Part two of the gboards-consolidation project. The Mega PR has been separated out into smaller per-keyboard PRs.

This PR contains a single keyboard as per @zvecr , in this case FaunchPad. This code was pulled from #8262

  • Enhancement/optimization
  • Keymap/layout/userspace (addition or update)

Issues Fixed or Closed by This PR

@zvecr zvecr added the awaiting_pr Relies on another PR to be merged first label Apr 26, 2020
Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

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

And a qmk cformat pass would be nice.

keyboards/gboards/k/faunchpad/config.h Outdated Show resolved Hide resolved
keyboards/gboards/k/faunchpad/rules.mk Outdated Show resolved Hide resolved
@zvecr zvecr added keyboard and removed awaiting_pr Relies on another PR to be merged first labels May 4, 2020
@stale
Copy link

stale bot commented Jun 19, 2020

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@germ
Copy link
Contributor Author

germ commented Jun 22, 2020

What's holding this back?

@drashna drashna requested a review from a team July 8, 2020 13:04
@stale stale bot removed the awaiting changes label Jul 8, 2020
@drashna drashna added awaiting changes breaking_change Changes that need to wait for a version increment labels Jul 8, 2020
@stale stale bot removed the awaiting changes label Jul 8, 2020
@Erovia
Copy link
Member

Erovia commented Jul 9, 2020

What's holding this back?

Mostly the lack of resources to review PRs.

But from a quick glance, I'd recommend the same things as I did for #8928.

  • modernize pin initialization

  • check if custom matrix is really needed, if yes, check if custom matrix lite is enough

Btw, this is a new (to QMK) board, right?

@germ
Copy link
Contributor Author

germ commented Jul 9, 2020

Makes sense, I'll see what I can do about the updated interface but the main reason I'm avoid it is simply lack of time due to COVID related backlog and I need to refactor/test 9 boards. :P

This is a new board though, same with Ginny. The Split boards operate just like the ergoDox, so anything that would work for it will work for my splits

@Erovia Erovia removed the breaking_change Changes that need to wait for a version increment label Jul 9, 2020
@Erovia
Copy link
Member

Erovia commented Jul 9, 2020

No worries, somehow life is quite busy for everybody right now, I wonder why.... :D

Since this is a new board, I removed the breaking_change tag, so that's at least -1 concern.

@germ
Copy link
Contributor Author

germ commented Jul 9, 2020

As far as custom matrix goes, by the looks of it I'll need that (same as the EZ guys). I can work on migrating to the new lite code when I have a chance (and that will help clean up the code a bunch).

As far as the other recommendations I'll try and get those in soon

@drashna drashna changed the base branch from master to develop July 28, 2020 05:34
Copy link
Member

@noroadsleft noroadsleft left a comment

Choose a reason for hiding this comment

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

Additionally, references to files in the g/ directory should be fixed. edit: never mind that

/* USB Device descriptor parameter */
#define VENDOR_ID 0x0007
#define PRODUCT_ID 0x0006
#define DEVICE_VER 1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#define DEVICE_VER 1
#define DEVICE_VER 0x0001

#define DEVICE_VER 1
#define MANUFACTURER g Heavy Industries
#define PRODUCT FaunchPad
#define DESCRIPTION Teeny Toiny Macropad
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#define DESCRIPTION Teeny Toiny Macropad

Comment on lines +5 to +6
#define LAYOUT_faunch(k00, k01, k02, k03, k10, k11, k12, k13) \
{ {k00, k01, k02, k03}, {k10, k11, k12, k13}, }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#define LAYOUT_faunch(k00, k01, k02, k03, k10, k11, k12, k13) \
{ {k00, k01, k02, k03}, {k10, k11, k12, k13}, }
#define LAYOUT_ortho_2x4( \
k00, k01, k02, k03, \
k10, k11, k12, k13 \
) { \
{ k00, k01, k02, k03 }, \
{ k10, k11, k12, k13 } \
}

Comment on lines +19 to +20
[0] = LAYOUT_faunch( KC_1, KC_2, KC_3, KC_4,
KC_5, KC_6, KC_7, KC_8)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[0] = LAYOUT_faunch( KC_1, KC_2, KC_3, KC_4,
KC_5, KC_6, KC_7, KC_8)
[0] = LAYOUT_ortho_2x4(
KC_1, KC_2, KC_3, KC_4,
KC_5, KC_6, KC_7, KC_8
)

Comment on lines +3 to +6
[0] = LAYOUT_faunch( KC_1, KC_2, KC_3, KC_4,
KC_5, KC_6, KC_7, KC_8),
[1] = LAYOUT_faunch( KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS,
KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[0] = LAYOUT_faunch( KC_1, KC_2, KC_3, KC_4,
KC_5, KC_6, KC_7, KC_8),
[1] = LAYOUT_faunch( KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS,
KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS)
[0] = LAYOUT_ortho_2x4(
KC_1, KC_2, KC_3, KC_4,
KC_5, KC_6, KC_7, KC_8
),
[1] = LAYOUT_ortho_2x4(
KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS,
KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS
),
[2] = LAYOUT_ortho_2x4(
KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS,
KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS
),
[3] = LAYOUT_ortho_2x4(
KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS,
KC_TRNS, KC_TRNS, KC_TRNS, KC_TRNS
)

@stale
Copy link

stale bot commented Jan 15, 2021

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@Frenzie
Copy link
Contributor

Frenzie commented Jan 15, 2021

Fwiw, works great on my Faunchpad.

@stale
Copy link

stale bot commented Feb 14, 2021

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.

@stale stale bot closed this Feb 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting changes keyboard keymap via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants