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.

Sign up for GitHub

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

Jump to bottom

[Keyboard] Fix up SplitKB keyboards #13511

Merged
merged 15 commits into from
Jul 26, 2021
Merged

[Keyboard] Fix up SplitKB keyboards #13511

merged 15 commits into from
Jul 26, 2021

Conversation

drashna
Copy link
Member

@drashna drashna commented Jul 10, 2021

Description

This does a number of things:

  • Moves the Kyria keyboard folder to /keyboards/splitkb/
  • Updates the Kyria and modernizes the code (OLED/Encoder code in keyboard files for configurator compatibility)
  • Adds VIA support to both the Kyria and the Zima
  • Replaces the snprintf/sprintf function in user keymaps with char manipulation hack that saves a significant amount of firmware

Types of Changes

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

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

@drashna drashna requested a review from a team July 10, 2021 23:02
@github-actions github-actions bot added keyboard keymap via Adds via keymap and/or updates keyboard for via support labels Jul 10, 2021
@drashna drashna mentioned this pull request Jul 10, 2021
Merged
6 tasks
cwebster2
cwebster2 approved these changes Jul 11, 2021
View reviewed changes
Copy link
Contributor

@cwebster2 cwebster2 left a comment

Choose a reason for hiding this comment

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

Changes lgtm.

@drashna
Copy link
Member Author

drashna commented Jul 11, 2021 •
edited
Loading

@j-inc @winterNebs

A heads up, since this does alter your keymaps.

@drashna drashna merged commit 4bb595f into qmk:develop Jul 26, 2021
@drashna drashna deleted the keyboard/fix_up_kyria branch July 26, 2021 01:18
@precondition precondition mentioned this pull request Sep 3, 2021
Merged
marceloalcocer
marceloalcocer reviewed Oct 19, 2021
View reviewed changes
keyboards/splitkb/kyria/rev1/rev1.h
{ KC_NO, KC_NO, R34, R35, R36, R37, R38, R39 }, \
{ R45, R46, R47, R48, R49, KC_NO, KC_NO, KC_NO }, \
}
#define LAYOUT_split_3x6_5 LAYOUT
Copy link

Choose a reason for hiding this comment

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

What was the reasoning behind this?

Defining LAYOUT_split_3x6_5 as LAYOUT breaks existing keymaps defined using LAYOUT_split_3x6_5 as four additional KC_NOs are required in the keymap.

If streamlining was the intention, would it not be better to remove LAYOUT_split_3x6_5 altogether?

nhongooi pushed a commit to nhongooi/qmk_firmware that referenced this pull request Dec 5, 2021
@drashna @nhongooi
[Keyboard] Fix up SplitKB keyboards (qmk#13511)
dad7db3
@leah-splitkb leah-splitkb mentioned this pull request Jan 19, 2023
Merged
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marceloalcocer marceloalcocer marceloalcocer left review comments

@fauxpark fauxpark fauxpark approved these changes

@cwebster2 cwebster2 cwebster2 approved these changes

Assignees
No one assigned
Labels
keyboard keymap via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@drashna @fauxpark @cwebster2 @marceloalcocer