Skip to content

Conversation

@Yowkees
Copy link

@Yowkees Yowkees commented Dec 3, 2025

Description

I created new 8-key keyboard "Keybit8" for junior high school students.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout (addition or update)
  • Documentation

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@waffle87 waffle87 added invalid pr_checklist_pending Needs changes as per the PR checklist labels Dec 3, 2025
Copy link
Member

@waffle87 waffle87 left a comment

Choose a reason for hiding this comment

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

Please stop opening and close PRs for the same content.

This submission and its code needs quite significant modification to adhere to the PR Checklist. I suggest you begin there, and if you have further questions, you're welcome to ask in the QMK Discord.

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.

All files should be lower case, due to file system compatibility issues.

Copy link
Member

Choose a reason for hiding this comment

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

via.json needs to be removed. We don't accept these, and these need to be PR'ed to the correct VIA repo.

Copy link
Member

Choose a reason for hiding this comment

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

Default keymap should be clean and pristine.

Note that the default keymap is what is copied when a user creates a new keymap, and many users struggle with more complicated code/config.

A lot of this code could/should be moved to the keyboard level, as well. (eg keyboards/keybit8/keybit8.c, using _kb functions)

Comment on lines +8 to +17
/* USB Device descriptor parameter */
/* USB Device descriptor is defined in info.json */

/* VIA/Remap configuration */
#define DYNAMIC_KEYMAP_LAYER_COUNT 4

/* key matrix size */
#define MATRIX_ROWS 2
#define MATRIX_COLS 4

Copy link
Member

Choose a reason for hiding this comment

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

layer count defaults to 4, no need to set it.

Also, rows+columns are generated automatically, if correctly configured.

Suggested change
/* USB Device descriptor parameter */
/* USB Device descriptor is defined in info.json */
/* VIA/Remap configuration */
#define DYNAMIC_KEYMAP_LAYER_COUNT 4
/* key matrix size */
#define MATRIX_ROWS 2
#define MATRIX_COLS 4

Comment on lines +18 to +29
/*
* Keyboard Matrix Assignments
* Arduino pin numbers:
* rowPins[2] = {21, 20}; -> F4, F5
* colPins[4] = {4, 5, 6, 7}; -> D4, C6, D7, E6
*/
#define MATRIX_ROW_PINS { F4, F5 } // Pins 21, 20 (Arduino Pro Micro)
#define MATRIX_COL_PINS { D4, C6, D7, E6 } // Pins 4, 5, 6, 7 (Arduino Pro Micro)

/* COL2ROW, ROW2COL */
#define DIODE_DIRECTION COL2ROW

Copy link
Member

Choose a reason for hiding this comment

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

These should be moved to keyboard.json. qmk migrate or qmk info -kb (name) -f json can help with that.

Comment on lines +32 to +37
/* Debounce reduces chatter (unintended double-presses) - set 0 if debouncing is not needed */
#define DEBOUNCE 5

/* Tapping term for layer toggle keys (200ms default, 50ms was too short) */
#define TAPPING_TERM 200

Copy link
Member

Choose a reason for hiding this comment

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

These are the default setting and should be removed.

Suggested change
/* Debounce reduces chatter (unintended double-presses) - set 0 if debouncing is not needed */
#define DEBOUNCE 5
/* Tapping term for layer toggle keys (200ms default, 50ms was too short) */
#define TAPPING_TERM 200

Comment on lines +10 to +11
"bootloader": "caterina",
"processor": "atmega32u4",
Copy link
Member

Choose a reason for hiding this comment

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

This sets the processor and bootloader, and allows for easy conversion to pro micro compatible controllers:

Suggested change
"bootloader": "caterina",
"processor": "atmega32u4",
"development_board": "promicro",

Comment on lines +12 to +21
"features": {
"bootmagic": true,
"extrakey": true,
"mousekey": false,
"nkro": false,
"backlight": false,
"rgblight": false,
"audio": false,
"encoder": false
},
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
"features": {
"bootmagic": true,
"extrakey": true,
"mousekey": false,
"nkro": false,
"backlight": false,
"rgblight": false,
"audio": false,
"encoder": false
},
"features": {
"bootmagic": true,
"extrakey": true,
},

Copy link
Member

Choose a reason for hiding this comment

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

This file can be removed, completely. All of the info is already in json file

Copy link
Member

Choose a reason for hiding this comment

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

This should be renamed to keyboard.json, and could use a qmk format-json pass.

Copy link
Member

Choose a reason for hiding this comment

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

This should follow the standard template for keyboard readmes. Eg:
https://github.com/qmk/qmk_firmware/blob/master/data/templates/keyboard/readme.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

invalid keyboard keymap pr_checklist_pending Needs changes as per the PR checklist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants