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

Jump to bottom

Dual-role key implementation for hexon38 #4709

Merged
merged 4 commits into from Jan 13, 2019
Merged

Dual-role key implementation for hexon38 #4709

merged 4 commits into from Jan 13, 2019

Conversation

cellularmitosis
Copy link
Contributor

@cellularmitosis cellularmitosis commented Dec 22, 2018 •
edited

This PR includes an initial layout for hexon38, a custom keyboard I've been working on.

Additionally, this PR includes an alternate implementation for handling MT() keys which does not rely on timeouts, etc. It appears to work.

I'd be very interested in feedback with regard to this implementation. I'm very new to the internals of QMK, so I'm not sure how easily it could be integrated for others to use (it currently exists in "user space", i.e. process_record_user()). Also, because I'm inexperienced, there may be ways of leveraging QMK functions to simplify my implementation, etc.

Cheers!

Here are a few gifs of some scenarios:

Setup:

  • k105 (ring finger in these gifs) is a dual-role key which is shift when held, f when tapped.
  • k106 (index finger in these gifs) is a single-role g key.

Scenario 1:

  • event 1: k105 down
    • event 1 is ambiguous
  • event 2: k105 up
    • event 1 is now disambiguated
    • emit f down
    • emit f up

imb_tdwk6s

Scenario 2:

  • event 1: k105 down
    • event 1 is ambiguous
  • event 2: k106 down
    • event 1 is still ambiguous
  • event 3: k105 up
    • event 1 is now disambiguated
    • emit f down
    • emit g down
    • emit f up
  • event 4: k106 up
    • emit g up

imb_fjqkx6

Scenario 3:

  • event 1: k105 down
    • event 1 is ambiguous
  • event 2: k106 down
    • event 1 is still ambiguous
  • event 3: k106 up
    • event 1 is now disambiguated
    • emit shift down
    • emit g down
    • emit g up
  • event 4: k105 up
    • emit shift up

imb_eqg2wt

Types of changes

  • Core
  • Bugfix
  • New Feature
  • Enhancement/Optimization
  • Keyboard (addition or update)
  • Keymap/Layout/Userspace (addition or update)
  • Documentation

@cellularmitosis cellularmitosis changed the title Initial dual-role key implementation for hexon38 Dual-role key implementation for hexon38 Dec 22, 2018
drashna
drashna requested changes Dec 22, 2018
View reviewed changes
Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

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

Since this seems to be a personal/handwired board, could you move this to /keyboards/handwired?

keyboards/hexon38/config.h Outdated Show resolved Hide resolved
keyboards/hexon38/config.h Outdated Show resolved Hide resolved
keyboards/hexon38/config.h Outdated Show resolved Hide resolved
keyboards/hexon38/config.h Outdated Show resolved Hide resolved
keyboards/hexon38/hexon38.h Outdated Show resolved Hide resolved
keyboards/hexon38/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/hexon38/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/hexon38/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/hexon38/rules.mk Outdated Show resolved Hide resolved
keyboards/hexon38/rules.mk Outdated Show resolved Hide resolved
@drashna
Copy link
Member

drashna commented Dec 22, 2018

You've opened this pull request from your fork's master branch. While this isn't a major issue now, later changes may result in merge conflicts if you try to file another PR because the commit history will have diverged between your master and QMK's. It is highly recommended for QMK development - regardless of what is being done or where - to keep your master updated, but NEVER commit to it. Instead, do all your changes in a branch (branches are basically free in Git) and issue PRs from your branches when you're developing.

There are instructions on how to keep your master updated here:

Best Practices: Your fork's master: Update Often, Commit Never

If you need any help with this just ask.

You do not need to close this PR. Further changes should still be pushed to your master branch, until this PR is merged.

@cellularmitosis
Copy link
Contributor Author

Thank you so much for all of the help and feedback! I'll update the PR and submit a new commit.

This is such a great project 👍

@cellularmitosis
Copy link
Contributor Author

Ok, this should be ready for a re-review now @drashna

drashna
drashna requested changes Jan 2, 2019
View reviewed changes
Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

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

Also, could you rename the readme file to be all lowercase?

This is to prevent cross-platform issues since some OS's are not great about handling this.

keyboards/handwired/hexon38/hexon38.c Outdated Show resolved Hide resolved
@mechmerlin
Copy link
Contributor

OMG! I love how you added videos to show how things work!!

@cellularmitosis
Copy link
Contributor Author

Thanks @mechmerlin !

@drashna thanks for the additional feedback -- I've updated the PR.

mechmerlin
mechmerlin approved these changes Jan 10, 2019
View reviewed changes
Copy link
Contributor

@mechmerlin mechmerlin left a comment

Choose a reason for hiding this comment

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

looks good

@drashna
Copy link
Member

drashna commented Jan 13, 2019

Thanks!

@drashna drashna merged commit 9ef4649 into qmk:master Jan 13, 2019
loadedsith added a commit to loadedsith/qmk_firmware that referenced this pull request Jan 15, 2019
@loadedsith
Merge branch 'master' into grahampheath
4d30311 
* master: (759 commits)
  [Keyboard] Add cKeys Handwire 101 Keyboard (qmk#4848)
  [Keyboard] Iris via support, Rev 3 updates (qmk#4849)
  [Keyboard] Add bthlabs/geekpad (qmk#4840)
  [Keymap] Added resfury keymap (qmk#4827)
  Pointed LM Docs at expected keycodes (qmk#4835)
  Add personal userspace, update keymaps (qmk#4845)
  [Keyboard] Add support for THE50 (qmk#4844)
  Change handling of CUSTOM_MATRIX in common_features.mk slightly.
  Modified URLs to point to new locations
  [Keymap] Nyquist layout adapted from eorgodox_ez:skug (qmk#4830)
  [Keymap] Adds keymaps for muzfuz DZ60, Planck, Clueboard66 (qmk#4825)
  Changed rest note (qmk#4837)
  [Keymap] Add tw1t611 german keyboard layout for minidox. (qmk#4679)
  [Keyboard] hexon38 and Dual-role key implementation (qmk#4709)
  [Keymap] my keymap for the crkbd and update my iris keymap readme (qmk#4788)
  [Keymap] Update to personal keymaps and userspace (qmk#4831)
  [Keyboard] Fix layout macro name for Gergo info.json (qmk#4828)
  Remove empty action_function()
  Remove empty fn_actions[]
  [Keyboard] Adding support for Gergo (qmk#4792)
  ...
rseymour pushed a commit to rseymour/qmk_firmware that referenced this pull request Mar 13, 2019
@cellularmitosis @rseymour
[Keyboard] hexon38 and Dual-role key implementation (qmk#4709)
c402e2f 
* initial dual-role key implementation for hexon38

* PR feedback, adding README

* Moving to handwired subdir

* Additional PR feedback
dlhextall pushed a commit to dlhextall/qmk_firmware that referenced this pull request May 24, 2019
@cellularmitosis
[Keyboard] hexon38 and Dual-role key implementation (qmk#4709)
c3c0254 
* initial dual-role key implementation for hexon38

* PR feedback, adding README

* Moving to handwired subdir

* Additional PR feedback
@YHRen
Copy link

YHRen commented Sep 28, 2020

@cellularmitosis very interesting work! One quick question, will two mods pressed at the same time works as well? For example, ctrl+shift (as using dual-role keys 'L' and ';') and tab (as key combo 'Q'+'W'). So holding 'L' and ';', while pressing and releasing 'Q'+'W', then releasing "L" and ";" (might not at the exact same time).

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

@mechmerlin mechmerlin mechmerlin approved these changes

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@cellularmitosis @drashna @mechmerlin @YHRen