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

[Bug] Durgod K320 layout macro is wrong #11883

Closed
baruch opened this issue Feb 13, 2021 · 5 comments · Fixed by #11885
Closed

[Bug] Durgod K320 layout macro is wrong #11883

baruch opened this issue Feb 13, 2021 · 5 comments · Fixed by #11885
Assignees

Comments

@baruch
Copy link

baruch commented Feb 13, 2021

Describe the Bug

I installed QMK on my Durgod K320 and tried to configure it with the QMK configurator. The result was completely garbled and unusable. It seems that the LAYOUT_tkl_ansi macro does not match the actual layout as shown in the info.json which results in a mess. The layout macro also doesn't match the physical layout which makes it very hard to write a manual keyboard layout since the keys in the layout macro are not in an order that one can make sense of.

Possible resolution

I came out with the following layout macro and it works for me and works with the QMK Configurator output as well. It probably means adapting the existing layouts to this new macro and I'm feeling a bit lazy to do it right now.

#define LAYOUT_tkl_ansi_visual( \
		K00, K01, K02, K03, K04, K05, K06, K07, K08, K09, K0A, K0B, K0C, K0D, K0E, K0F, \
		K10, K11, K12, K13, K14, K15, K16, K17, K18, K19, K1A, K1B, K1C, K1E, K2E, K2F, K1F, \
		K20, K21, K22, K23, K24, K25, K26, K27, K28, K29, K2A, K2B, K2C, K2D, K3D, K3E, K3F, \
		K30, K31, K32, K33, K34, K35, K36, K37, K38, K39, K3A, K3B, K4E, \
		K40, K42, K43, K44, K45, K46, K47, K48, K49, K4A, K4B, K4D, K4F, \
		K50, K51, K52, K56, K5A, K5B, K5C, K5D, K5E, K5F, K6F \
) { \
    { K00,  K01,  K02,  K03,  K04,  K05,  K06,  K07,  K08,  K09,  K0A,  K0B,  K0C,  K0D,  K0E,  K0F },  \
    { K10,  K11,  K12,  K13,  K14,  K15,  K16,  K17,  K18,  K19,  K1A,  K1B,  K1C,  XXX,  K1E,  K1F },  \
    { K20,  K21,  K22,  K23,  K24,  K25,  K26,  K27,  K28,  K29,  K2A,  K2B,  K2C,  K2D,  K2E,  K2F },  \
    { K30,  K31,  K32,  K33,  K34,  K35,  K36,  K37,  K38,  K39,  K3A,  K3B,  XXX,  K3D,  K3E,  K3F },  \
    { K40,  XXX,  K42,  K43,  K44,  K45,  K46,  K47,  K48,  K49,  K4A,  K4B,  XXX,  K4D,  K4E,  K4F },  \
    { K50,  K51,  K52,  XXX,  XXX,  XXX,  K56,  XXX,  XXX,  XXX,  K5A,  K5B,  K5C,  K5D,  K5E,  K5F },  \
    { XXX,  XXX,  XXX,  XXX,  XXX,  XXX,  XXX,  XXX,  XXX,  XXX,  XXX,  XXX,  XXX,  XXX,  XXX,  K6F }   \
}
@spidey3
Copy link
Contributor

spidey3 commented Feb 13, 2021

I just noticed this yesterday, while reviewing PR #11778.
Fixing it will probably need to go into the develop branch as a breaking change, because it will definitely break any keymaps that have been created using the existing [broken] layout macro.

@spidey3 spidey3 self-assigned this Feb 13, 2021
@baruch
Copy link
Author

baruch commented Feb 13, 2021

I have a patch ready, it fixes some of the layouts, mostly the default one. I don't really have time to spend to fix the others so I just kept an old layout macro for the ones I didn't fix and made a new one that makes sense (as I did above).

baruch added a commit to baruch/qmk_firmware_durgod_k320 that referenced this issue Feb 13, 2021
@baruch
Copy link
Author

baruch commented Feb 13, 2021

I pushed my patch into the repo, feel free to use it or ignore @spidey3.

@spidey3
Copy link
Contributor

spidey3 commented Feb 13, 2021

@baruch - can you please test PR #11883? It should fix this issue.
Note, it's against the develop branch, since this will be a breaking change for anyone out there who has keymaps only in their fork.

@baruch
Copy link
Author

baruch commented Feb 13, 2021

Tested. PR is #11885 and the default keymap works. I don't have a Mac or ISO layout to test these.

spidey3 added a commit to spidey3/qmk_firmware that referenced this issue Feb 14, 2021
spidey3 added a commit that referenced this issue Feb 14, 2021
* unscramble Durgod k320 keymap / fix Issue #11883

* fix a few keymaps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants