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

Revert "Fixed the ortho60 and ortho48 matrix layout after testing" #13456

Merged

Conversation

cyril279
Copy link

@cyril279 cyril279 commented Jul 5, 2021

Reverts #12106

Summary:
The ortho-fix commit breaks the 2uC layout of the ortho48 (& possibly the ortho60) by changing the matrix to associate the center/2u key to switch-position 36 (was previously 35). When the center switch is installed, position 36 is empty (no switch installed).

The commit was apparently because one user had an issue with a VIA map.
There is nothing in the PR communicating the user's actual issue, or how it translates to a correction to the matrix.

Troubleshooting/testing:
I probed my ortho48 pcb and found that the center/2u position is traced to the same pin as position 35.
Without probing the pcb, the routing of the center-switch can be determined practically by performing the column-numbers-in-the-keymap steps (outlined in the comments).

Other thoughts:
I bought this kit in June of 2020 and the matrix/Layouts for both grid and 2uC worked as expected (for me and many others). Considering the direct relationship of the matrix to the routing of the pcb, the maintainer and/or vendor should be consulted prior to such a change.

-Cyril

@cyril279 cyril279 marked this pull request as draft July 5, 2021 11:56
@cyril279 cyril279 force-pushed the revert-12106-nblyumberg_cannonkeys_orthofix branch from 8bd06df to b9bee79 Compare July 5, 2021 12:38
@github-actions github-actions bot removed the keymap label Jul 5, 2021
@cyril279 cyril279 marked this pull request as ready for review July 5, 2021 12:40
@drashna
Copy link
Member

drashna commented Jul 14, 2021

@awkannan Any input?

@awkannan
Copy link
Contributor

Yep! This is a good fix.

@tzarc tzarc added the bug label Jul 25, 2021
@tzarc tzarc requested a review from a team July 25, 2021 01:27
@drashna
Copy link
Member

drashna commented Jul 26, 2021

Thanks!

@drashna drashna merged commit a73e0a7 into qmk:master Jul 26, 2021
nhongooi pushed a commit to nhongooi/qmk_firmware that referenced this pull request Dec 5, 2021
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