-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Added support for the Keebio Levinson Rev 3 Shield. #566
Conversation
…minus and equal in the Lower layer.
… of the board talked to each other correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work on this shield. Super tiny comment and then just these files to update:
.github/workflows/build.yml
: Add your board to the shield list.
docs/docs/hardware.md
: You should add the Levinson to this list
docs/static/setup.ps1
and docs/static/setup.sh
: Levinson should be added as a split option for both files.
#Copyright (c) 2021 The ZMK Contributors | ||
#SPDX-License-Identifier: MIT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#Copyright (c) 2021 The ZMK Contributors | |
#SPDX-License-Identifier: MIT | |
# Copyright (c) 2021 The ZMK Contributors | |
# SPDX-License-Identifier: MIT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Nicell thanks for the further instructions, I have added the Levinson as an option in the files you mentioned following the formatting of all the other shields. I hope this was correct. I also added in those spaces in the copyright lines. Not sure how that one got missed ;)
…l. Changed spacing in the copyright line of Kconfig.defconfig to match other files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed this on my first go around! We've changed how split roles are defined, and I've added comments on the files that need to change. Thanks!
if SHIELD_LEVINSON_LEFT | ||
|
||
config ZMK_KEYBOARD_NAME | ||
default "Levinson Left" | ||
|
||
endif | ||
|
||
if SHIELD_LEVINSON_RIGHT | ||
|
||
config ZMK_KEYBOARD_NAME | ||
default "Levinson Right" | ||
|
||
endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A recent change was made to how we define the central and peripheral for splits, which can be seen here: https://zmkfirmware.dev/docs/development/new-shield#kconfigdefconfig. Could you update this file to match the docs? Along with this you'll need to clear out the left and right .conf
files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes made and pushed.
CONFIG_ZMK_SPLIT=y | ||
CONFIG_ZMK_SPLIT_BLE_ROLE_CENTRAL=y |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CONFIG_ZMK_SPLIT=y | |
CONFIG_ZMK_SPLIT_BLE_ROLE_CENTRAL=y |
@@ -0,0 +1 @@ | |||
CONFIG_ZMK_SPLIT=y |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CONFIG_ZMK_SPLIT=y |
…ntral and peripheral definitions. Cleared contents of the .conf files for both halves. Fixed layout comment where the pipe character was shown as a backslash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@blistergeist The changes look good! We're getting #546 merged in first, then once that's merged I'll ask you to rebase this PR hopefully one last time. Then it should be ready for a final review and merge 👍
I can notify you here when we're ready for you to rebase.
Great! Looking forward to it. |
@blistergeist Sorry about the huge delay here, for some reason our update message didn't get put on this PR, here's a copy of it: Thank you, contributor, for your patience with how long review and merge of boards/shields has taken! There are three recent refactors/changes to boards and shields that require some attention, and then we can finally get this PR merged!
Hardware MetadataThe ProblemWhen first developing the process around contributing new shields/boards to ZMK, we failed to recognize that several key files (setup scripts, documentation page of supported hardware, and GH Action The FixBy adding discrete metadata files that are located with the boards/shields in question, and using that metadata to generate setup scripts, website hardware list, etc., users can contributing new hardware descriptions without the need to change the same files that other contributors are changing. Next StepsFirst, refer to https://zmk.dev/docs/development/hardware-metadata-files to familiarize yourself with the new metadata file format. Next, you have two options for fixing up your PR:
Pro Micro shield DT naming changesIn #876, we have simplified the DT naming for the "nexus node" we expose for pro-micro compatible boards, deprecating the use of Please see https://zmk.dev/docs/development/new-shield#shield-overlays for the updated docs on this. Split Shield Advertising ChangesIn addition, if this is a split PR, please see #658 where we have changed our conventions to remove the the name from the right sides, to prevent users attempting to pair with them and causing split sync issues. This also includes removing the " Left" suffix from the naming on the left side. See the changes in that PR for examples of what to change with your split shield. Getting HelpIf you have any questions about any of these changes, please comment here and tag @zmkfirmware/boards-shields or ask in the |
@blistergeist I'll be closing this PR as we haven't heard anything in 6 months. Please feel free to reopen this if you ever come back to it. |
Added support for the Keebio Levinson Rev 3 shield. Tested locally on nRFMicro revision 1.3.