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

DecenTKL numpad #24696

Merged
merged 10 commits into from
Dec 24, 2024
Merged

DecenTKL numpad #24696

merged 10 commits into from
Dec 24, 2024

Conversation

bleroy
Copy link
Contributor

@bleroy bleroy commented Dec 9, 2024

This is the firmware customization for the open source DecenTKL numpad, inspired by the Atari XE and ST design.

Description

The customization sets up the matrices for the numpad, adds Atari joystick support.

Types of Changes

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

Issues Fixed or Closed by This PR

None

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

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.

As per the PR checklist:

  • PRs should contain the smallest amount of modifications required for a single change to the codebase
    - multiple keyboards at the same time is not acceptable
    - the smaller the PR, the higher likelihood of a quicker review, higher likelihood of quicker merge, and less chance of conflicts

Please remove the keyboards/decent/tkl code from this pull request and submit it in another.

keyboards/decent/numpad/keyboard.json Outdated Show resolved Hide resolved
keyboards/decent/numpad/keyboard.json Outdated Show resolved Hide resolved
keyboards/decent/numpad/keyboard.json Outdated Show resolved Hide resolved
keyboards/decent/numpad/keymaps/default/config.h Outdated Show resolved Hide resolved
keyboards/decent/numpad/keymaps/default/config.h Outdated Show resolved Hide resolved
keyboards/decent/numpad/keymaps/default/config.h Outdated Show resolved Hide resolved
keyboards/decent/numpad/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/decent/numpad/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/decent/numpad/readme.md Outdated Show resolved Hide resolved
keyboards/decent/numpad/rules.mk Outdated Show resolved Hide resolved
@bleroy
Copy link
Contributor Author

bleroy commented Dec 10, 2024

I addressed what I could but there are still a few unclear comments. Thanks for your review.

@waffle87
Copy link
Member

there are still a few unclear comments

Are there specific things you have questions about?

@zvecr zvecr changed the title DecenTKL keyboard and numpad DecenTKL numpad Dec 10, 2024
@bleroy
Copy link
Contributor Author

bleroy commented Dec 10, 2024

there are still a few unclear comments

Are there specific things you have questions about?

Ah yes, apologies, I forgot to publish my review. That's now done. Mainly, I'm not sure where to move the joystick config stuff since I didn't find relevant documentation. There are also minor questions around RGB constants. Thanks for the quick turnout on PRs, that's quite amazing.

@bleroy
Copy link
Contributor Author

bleroy commented Dec 11, 2024

I addressed more comments. Please let me know if anything more is needed.

Copy link
Contributor

@dunk2k dunk2k left a comment

Choose a reason for hiding this comment

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

layout information additions to match keymap diagram

keyboards/decent/numpad/keyboard.json Outdated Show resolved Hide resolved
keyboards/decent/numpad/keyboard.json Outdated Show resolved Hide resolved
keyboards/decent/numpad/keyboard.json Outdated Show resolved Hide resolved
@bleroy bleroy requested a review from waffle87 December 13, 2024 20:50
keyboards/decent/numpad/numpad.c Show resolved Hide resolved
keyboards/decent/numpad/numpad.c Outdated Show resolved Hide resolved
keyboards/decent/numpad/numpad.c Show resolved Hide resolved
@bleroy bleroy requested a review from waffle87 December 14, 2024 20:28
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.

Otherwise LGTM.

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.

A number of small nit picks

keyboards/decent/numpad/config.h Outdated Show resolved Hide resolved
keyboards/decent/numpad/keyboard.json Outdated Show resolved Hide resolved
keyboards/decent/numpad/numpad.c Outdated Show resolved Hide resolved
keyboards/decent/numpad/numpad.c Outdated Show resolved Hide resolved
@drashna drashna requested a review from a team December 22, 2024 20:17
@drashna drashna merged commit e6fa351 into qmk:master Dec 24, 2024
3 checks passed
dpnetca pushed a commit to dpnetca/qmk_firmware that referenced this pull request Dec 24, 2024
@bleroy bleroy deleted the decenTKL branch December 24, 2024 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants