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

handwired/split89 Layout Macro Refactor #15210

Merged
merged 4 commits into from
Nov 19, 2021
Merged

handwired/split89 Layout Macro Refactor #15210

merged 4 commits into from
Nov 19, 2021

Conversation

noroadsleft
Copy link
Member

Description

  • rearranges the layout macro and keycodes to resemble the assembled board.
    KC_Y is now between KC_T and KC_U; KC_H between KC_G and KC_J; KC_N between KC_B and KC_M
  • updates split89.h to use k<row><column> notation
  • refactors default keymap for readability
  • reformats info.json for readability/to match layout
    before
    image
    After
    image

No logic changes in this PR.

cc @jurassic73 (keyboard maintainer)

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.
  • I have read the CONTRIBUTING document.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@noroadsleft
split89.h: use XXX for KC_NO
1a24578
@noroadsleft
physically arrange layout macro
feaa06f 
Rearranges the layout macro and keymaps so the keycodes are arranged like the assembled keyboard. No logic change.
@noroadsleft
info.json: friendly-format
df019c0 
One key object per line; completes/updates key labels; line breaks between keyboard rows.
@noroadsleft
info.json: update key positions
9a6410e 
Visually re-positions the keys when rendered in QMK Configurator (improves readability).
@noroadsleft noroadsleft requested a review from a team November 19, 2021 02:19
@drashna drashna merged commit 26ae432 into qmk:master Nov 19, 2021
@jurassic73
Copy link
Contributor

@noroadsleft thanks for your time on this.

@noroadsleft noroadsleft deleted the cf/handwired_split89 branch November 19, 2021 08:24
beelzebubi pushed a commit to beelzebubi/qmk_firmware that referenced this pull request Nov 21, 2021
@noroadsleft
handwired/split89 Layout Macro Refactor (qmk#15210)
4591d92
@jurassic73
Copy link
Contributor

This pull request broke the keyboard layout.

Another user that built my keyboard downloaded firmware and the changes in this pull request broke the mapping. I just compiled and downloaded the firmware and this mapping is indeed broken. Can this please be rolled back to remove the changes this pull request implemented?

@noroadsleft
Copy link
Member Author

@jurassic73,

Did your user rearrange their keycodes similarly to how the default keymap is edited in this PR? I just rechecked the branch and the default keymap compiles exactly the same before and after these changes – running shasum handwired_split89_default.hex from the base commit (cd50fdf) and this branch gives me a SHA-1 checksum of 83b86f75b747a7def91d9a28edb0c7e24f3ea4c7. (Your checksum here may differ if you do the same check, but should be the same before and after.)

@jurassic73
Copy link
Contributor

No user keymap rearragment took place by users or myself before or now. Before your pull request, simply bringing up the split89 layout and compiling produced working firmware. After your pull request was added, the layout is now broken.

If there is a proper key layout I need to adhere to, I can work towards it but for now could the changes made in your pull request be rolled back as the generated firmware request worked before this, so users can download working firmware by default? Currently it generates an incorrect firmware file for users.

If you could do this, I could work on installing QMK on my end in a container and try to work this out so it's to your standards?

@noroadsleft
Copy link
Member Author

Before your pull request, simply bringing up the split89 layout and compiling produced working firmware.

Are you talking about compiling in QMK Configurator here?

@jurassic73
Copy link
Contributor

Yes, compiling firmware in the web interface to download.

@noroadsleft
Copy link
Member Author

Odd, I can't see any reason why this wouldn't work. I'll go ahead and open a PR to revert these changes.

Perhaps we can connect next week and see about reworking this and/or coordinating on what changes should be made.

noroadsleft added a commit to noroadsleft/qmk_firmware that referenced this pull request Nov 23, 2021
@noroadsleft
Revert "handwired/split89 Layout Macro Refactor (qmk#15210)"
af22c63 
This reverts commit 26ae432.

Requested by jurassic73.
@noroadsleft noroadsleft mentioned this pull request Nov 23, 2021
Merged
7 tasks
@jurassic73
Copy link
Contributor

jurassic73 commented Nov 23, 2021 via email

noroadsleft added a commit that referenced this pull request Nov 24, 2021
@noroadsleft
Revert "handwired/split89 Layout Macro Refactor (#15210)" (#15284)
59e1513 
This reverts commit 26ae432.

Requested by jurassic73.
tacahiroy pushed a commit to tacahiroy/qmk_firmware that referenced this pull request Nov 26, 2021
@noroadsleft @tacahiroy
Revert "handwired/split89 Layout Macro Refactor (qmk#15210)" (qmk#15284)
789c090 
This reverts commit 26ae432.

Requested by jurassic73.
cadusk pushed a commit to cadusk/qmk_firmware that referenced this pull request Dec 2, 2021
Merge remote-tracking branch 'qmk/develop' into develop
e04f8a1 
* qmk/develop: (80 commits)
  Remove use of __flash due to LTO issues (qmk#15268)
  Revert "handwired/split89 Layout Macro Refactor (qmk#15210)" (qmk#15284)
  New Keyboard: TGR Jane CE (qmk#14713)
  Portal 66 Layout Macro Refactor (qmk#15255)
  Pluckey: Fix QMK Configurator Implementation (qmk#15254)
  [Tests] Increase QMK test coverage take 2 (qmk#15269)
  Ignore exit codes for formatters (qmk#15276)
  [Keyboard] Disable features on SplitKB boards to fit under size (qmk#15262)
  Ignore exit codes for formatters (qmk#15275)
  Ignore deleted files when formatting codebase (qmk#15274)
  qmk format-python - filter for Python files (qmk#15271)
  Revert "[Tests] Increase QMK test coverage (qmk#13789)"
  [Tests] Increase QMK test coverage (qmk#13789)
  [Docs] Squeezing space out of AVR (qmk#15243)
  Add uint to char functions (qmk#15244)
  [Keyboard] Disable console on Keebio foldkb and iris rev3 (qmk#15260)
  layer_combo → sd_combo (qmk#15266)
  [Keymap] Disable console on Sofle default keymap (qmk#15261)
  [Keyboard] Enable LTO on viktus/sp_mini via keymap (qmk#15263)
  Macros in JSON keymaps (qmk#14374)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drashna drashna drashna approved these changes

@fauxpark fauxpark fauxpark approved these changes

Assignees
No one assigned
Labels
enhancement keyboard keymap
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@noroadsleft @jurassic73 @drashna @fauxpark