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

Add BlueHand Keyboard again #2228

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

DonPavlov
Copy link

Board/Shield Check-list

  • [X ] This board/shield is tested working on real hardware
  • Definitions follow the general style of other shields/boards upstream (Reference)
  • .zmk.yml metadata file added
  • Proper Copyright + License headers added to applicable files (Generally, we stick to "The ZMK Contributors" for copyrights to help avoid churn when files get edited)
  • General consistent formatting of DeviceTree files
  • Keymaps do not use deprecated key defines (Check using the upgrader tool)
  • &pro_micro used in favor of &pro_micro_d/a if applicable
  • If split, no name added for the right/peripheral half
  • Kconfig.defconfig file correctly wraps all configuration in conditional on the shield symbol
  • .conf file has optional extra features commented out
  • Keyboard/PCB is part of a shipped group buy or is generally available in stock to purchase (OSH/personal projects without general availability should create a zmk-config repo instead)

Copy link
Contributor

@lesshonor lesshonor left a comment

Choose a reason for hiding this comment

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

I'm going to leave immediately-actionable suggestions in good faith and humor—but after a year and a half of silence and many attempts before that1...I think the writing is on the wall and you should continue setting up that board module.

Should you keep this pull request open, you definitely need to install and run pre-commit on the files you're submitting to deal with formatting issues (tabs, trailing whitespace, etc). Squashing your commits before rebasing on main and rewriting the commit message so it adheres to Conventional Commit guidelines is also strongly encouraged.

Footnotes

  1. Not sure if this macropad was redesigned or if it should always have been submitted as a board.


/ {
model = "BlueHand";
compatible = "blue,hand";
Copy link
Contributor

Choose a reason for hiding this comment

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

Per Zephyr docs, the format here should be "manufacturer,product".

Yes, some boards appear to have gotten this wrong and some may have tried to get "cute" (though nice is reasonably-arguable as a manufacturer alias for "nice keyboards") ...but blue isn't jpconstantineau's company name and "bluehand" is the full name of the board. I'd either use his handle or (per this suggestion) just leave it unprefixed.

Suggested change
compatible = "blue,hand";
compatible = "bluehand";


kscan0: kscan_0 {
compatible = "zmk,kscan-gpio-direct";
label = "KSCAN";
Copy link
Contributor

Choose a reason for hiding this comment

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

This property has been deprecated. But you'll want to add wakeup-source so keypresses can wake the board from deep sleep (see a8a0d27).

Suggested change
label = "KSCAN";
wakeup-source;


board_runner_args(nrfjprog "--nrf-family=NRF52" "--softreset")
include(${ZEPHYR_BASE}/boards/common/blackmagicprobe.board.cmake)
include(${ZEPHYR_BASE}/boards/common/nrfjprog.board.cmake)
Copy link
Contributor

Choose a reason for hiding this comment

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

Update per 506c3b0.

Suggested change
include(${ZEPHYR_BASE}/boards/common/nrfjprog.board.cmake)
include(${ZEPHYR_BASE}/boards/common/nrfjprog.board.cmake)
include(${ZEPHYR_BASE}/boards/common/uf2.board.cmake)

CONFIG_SETTINGS_NVS=y
CONFIG_FLASH=y
CONFIG_FLASH_PAGE_LAYOUT=y
CONFIG_FLASH_MAP=y
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a better fit here, I think.

Suggested change
CONFIG_FLASH_MAP=y
CONFIG_FLASH_MAP=y
CONFIG_ZMK_USB=y
CONFIG_ZMK_BLE=y

Comment on lines +20 to +25

config ZMK_BLE
default y

config ZMK_USB
default y
Copy link
Contributor

@lesshonor lesshonor Apr 4, 2024

Choose a reason for hiding this comment

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

See previous comment.

Suggested change
config ZMK_BLE
default y
config ZMK_USB
default y

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.

None yet

2 participants