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

[Keyboard] Refactor JJ50 keyboard mapping to current standard #9415

Merged
merged 1 commit into from
Jun 28, 2020

Conversation

GreatWizard
Copy link
Contributor

@GreatWizard GreatWizard commented Jun 14, 2020

Description

  • [Feature] Add Dvroak, Colemak as base layers
  • [Feature] Add an Adjust layer to change the base layer
  • [Enhancement] Update the mappings in all the layers to follow the default 5x12 ortholinear preonic
  • [Enhancement] Update Fn mapping to have all rgblight and backlight features

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Checklist

  • My code follows the code style of this project.
  • 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).

keyboards/jj50/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/jj50/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/jj50/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/jj50/keymaps/default/keymap.c Outdated Show resolved Hide resolved
@GreatWizard GreatWizard requested a review from fauxpark June 14, 2020 12:49
@GreatWizard GreatWizard force-pushed the update_default_jj50 branch from 804c4b9 to 83aab09 Compare June 14, 2020 13:07
@fauxpark fauxpark added the breaking_change Changes that need to wait for a version increment label Jun 14, 2020
@fauxpark
Copy link
Member

fauxpark commented Jun 14, 2020

With the changes to the archetype keymap this PR must go through the breaking changes process. If you don't mind this, you can actually outright delete that rules.mk as it's identical to the keyboard-level one.

@GreatWizard
Copy link
Contributor Author

I can't delete the rules.mk for the archetype keymap, there is 2 more rules then the keyboard-level one:

TAP_DANCE_ENABLE = yes
AUTO_SHIFT_ENABLE = yes

@fauxpark
Copy link
Member

Ah, well you can delete everything else then.

@GreatWizard
Copy link
Contributor Author

The CI is having a hard time with xvzver, returning error: NKRO not supported with this protocol
And I have no idea why, NKRO is disabled in JJ50 rules

@fauxpark
Copy link
Member

layouts\community\ortho_5x12\xyverz\rules.mk is meant to check whether PROTOCOL is VUSB, which is set in quantum/mcu_selection.mk however the rules.mk is actually evaluated first. You can safely ignore that for now.

@GreatWizard GreatWizard force-pushed the update_default_jj50 branch 2 times, most recently from 4882d77 to 5ea4c7b Compare June 14, 2020 21:23
@GreatWizard
Copy link
Contributor Author

GreatWizard commented Jun 14, 2020

My latest commit adapt a bit the workflow... I'm not sure if there is any blockers by changing the order?

Can we talk somewhere else in order to chat about the subject? (discord irc slack...)

@fauxpark
Copy link
Member

Interesting that that seems to work, considering mcu_selection.mk first needs to know what MCU is, which is defined in rules.mk. I suppose it must have to do with when make evaluates stuff. In any case, that should really be done in a separate PR.

The QMK Discord is linked in the readme (it's one of the badges) but I'll post it here too: https://discord.gg/Uq7gcHh

@GreatWizard
Copy link
Contributor Author

the CI is going well indeed 😊
I will definitively opening a PR only for that change

@fauxpark
Copy link
Member

Well you've now added a core change to this PR so Travis is going to run make all:default instead of make <changedkeyboards>:all.

@GreatWizard
Copy link
Contributor Author

I made the PR #9419
I will rebase this one when the other is merged
let's continue to talk about the "fix" over there and keep this PR related to JJ50 keymaps ^^

@GreatWizard GreatWizard force-pushed the update_default_jj50 branch from 1a287b1 to 5ea4c7b Compare June 14, 2020 22:35
@GreatWizard GreatWizard force-pushed the update_default_jj50 branch 2 times, most recently from 080e964 to 7f87e56 Compare June 16, 2020 20:46
@GreatWizard GreatWizard force-pushed the update_default_jj50 branch 5 times, most recently from 0e432c5 to 9eecbf2 Compare June 22, 2020 08:46
Copy link
Member

@noroadsleft noroadsleft left a comment

Choose a reason for hiding this comment

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

Just some minor fixes.

keyboards/jj50/README.md Outdated Show resolved Hide resolved
keyboards/jj50/keymaps/default/README.md Outdated Show resolved Hide resolved
keyboards/jj50/keymaps/default/README.md Outdated Show resolved Hide resolved
keyboards/jj50/keymaps/default/keymap.c Outdated Show resolved Hide resolved
@GreatWizard GreatWizard force-pushed the update_default_jj50 branch from 0d19931 to abcb756 Compare June 26, 2020 12:56
@GreatWizard GreatWizard requested a review from noroadsleft June 26, 2020 12:56
@GreatWizard GreatWizard force-pushed the update_default_jj50 branch from abcb756 to bb49b5d Compare June 27, 2020 12:31
keyboards/jj50/config.h Outdated Show resolved Hide resolved
@GreatWizard GreatWizard force-pushed the update_default_jj50 branch 2 times, most recently from 2b4b72f to adbb345 Compare June 27, 2020 13:03
@GreatWizard GreatWizard requested a review from fauxpark June 27, 2020 13:03
@GreatWizard GreatWizard force-pushed the update_default_jj50 branch from adbb345 to 9cebf12 Compare June 28, 2020 16:19
@GreatWizard GreatWizard changed the title [WIP] [Keyboard] Refactor JJ50 keyboard mapping to current standard [Keyboard] Refactor JJ50 keyboard mapping to current standard Jun 28, 2020
@GreatWizard
Copy link
Contributor Author

Tested and it works like a charm :)
🎉

@aaarsene
Copy link

I justed tested it and everything seems to work like it should!

@Erovia Erovia merged commit 0928496 into qmk:master Jun 28, 2020
@GreatWizard GreatWizard deleted the update_default_jj50 branch June 28, 2020 17:51
jakobaa pushed a commit to jakobaa/qmk_firmware that referenced this pull request Jul 7, 2020
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