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

Add the ability to send shifted keycodes with a mod-tap #2055

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@dpapavas
Copy link

dpapavas commented Nov 25, 2017

This PR introduces a configurable way to use the mod-tap feature to send shifted keystrokes. When enabled, by defining the proper C macro, it treats mod-tap keycodes, which specify shift as part of the modifier list, but where shift is not the only modifier, as if the shift were a part of the keycode. Specifically, shift is not sent during a held keystroke, but is sent during the tap. This essentially allows one to use the mod-tap feature to combine shifted keycodes, such as parentheses, with any modifier, except a shift. It gives up the ability to have a mod-tap key send combinations of shift and other modifiers when held, but, since it is configurable, it seems to me like a useful trade-off to have.

I don't have much experience with using QMK, or with the QMK source, so my proposed changes may introduce problems in certain parts of the mod-tap, or tap-dance functionality, which have escaped me. Please let me know if that is the case.

@drashna

This comment has been minimized.

Copy link
Collaborator

drashna commented Mar 25, 2018

This should be expanded to encompass all mods (like the OSM key)

@dpapavas

This comment has been minimized.

Copy link
Author

dpapavas commented Mar 27, 2018

This should be expanded to encompass all mods (like the OSM key)

On a technical level, this comment here explains in some detail, why this isn't really possible. To be exact, I don't think it's impossible, but it would take up a lot of the keycode space, as I'll explain below.

On a practical level, I'm not sure of the usefulness of expanding this functionality to other modifiers. The shift modifier is special, in that it is necessary, in order to get certain symbols, such as parentheses, braces, etc., so if you want a mod-tap where holding holds the control down, but tapping sends a parenthesis, that is not possible.

On the other hand, if this patch, is extended to other modifiers, say control, it would allow someone to get a mod-tap, where holding holds down, say alt, but tapping sends ctrl-t. I don't think this is what mod-tap was supposed to accomplish and I'm not sure if that would be very useful, or useful enough at any rate, to justify all those keycodes (see below).

Back to the technical side, as far as I can see, the situation is this: keycodes are encoded in 16 bits. For the case of the mod-tap keycodes, 8 bits are taken up for the tapped keycode and 5 bits are taken up for the modifiers. The fifth modifier bit collectively sets the modifiers to be either left-side, or right-side, therefore you can't mix sides, i.e. you can't have the mod-tap key hold down the right control and left alt. For that, the modifiers would require all 8 remaining bits.

Now, another, perhaps better way to implement this PR, would be the following: one could have a separate, "shifted mod-tap" keycode, or rather keycode range, which would be the same as the mod-tap, but imply sending a shifted keycode. The MT() macro, could then examine whether a shift is present in the keycode argument and return a "shifted mod-tap" keycode, instead of the normal one.

The pros of this approach, would be that you wouldn't have to #define ALLOW_SHIFTED_MOD_TAP and you wouldn't have to forgo having shift on the modifier side, as explained in my initial post. This would practically mean, that you'd be able to have the space cadet functionality with mod-tap. On the other hand, the downside to all this, would be that it will take up 2^13, i.e. 8192 extra keycodes.

I haven't investigated this approach in much detail, so there could be problems I've missed with it and it would also mean much more extensive changes than the current PR. If it is possible though, it could also be used to allow extension to other modifiers, but it would require 8192 keycodes for each one, which would basically exhaust the keycode space, or most of it anyway.

@drashna

This comment has been minimized.

Copy link
Collaborator

drashna commented Mar 27, 2018

Okay, thanks for the good writeup/response on this!

@drashna

This comment has been minimized.

Copy link
Collaborator

drashna commented Mar 27, 2018

@jackhumbert, this looks good to go

@dpapavas

This comment has been minimized.

Copy link
Author

dpapavas commented Mar 27, 2018

Thanks, for reviewing this!

Please note, in case my alternate approach above came across as something to avoid, that it might be preferable, if implemented only for shifted mot-taps. There seem to be plenty of available keycodes, so that reserving an extra 8192 for this might be acceptable, as you would get the ability to have mod-taps that can have shift as the held modifier and also send shifted keycodes when tapped. You allso wouldn't need the special #define.

None of this is a problem in my particular use-case, so I'd be perfectly happy with this PR, but I can see how it might be useful to others and more flexible. On the other hand, the alternate approach would be more global than the one in this PR, in the sense that it would need to affect more of the existing code, so perhaps it'd be better if someone with more experience (than me) could comment on it.

@oscar-b

This comment has been minimized.

Copy link

oscar-b commented Mar 28, 2018

On a practical level, I'm not sure of the usefulness of expanding this functionality to other modifiers. The shift modifier is special, in that it is necessary, in order to get certain symbols, such as parentheses, braces, etc.

The usefulness lies in using these features with other keymaps such as nordics, where you need a finger breaking Alt+Shift+7 to get curly braces for instance.

It’s quite remarkable that handling of keyboard input in the OS-level is still the same as it was 30 years ago.

@dpapavas

This comment has been minimized.

Copy link
Author

dpapavas commented Mar 28, 2018

The usefulness lies in using these features with other keymaps such as nordics, where you need a finger breaking Alt+Shift+7 to get curly braces for instance.

I see. Well, if arbitrary combinations of modifiers are needed on both the mod and tap side of a mod-tap key, as in the example above, we'd need more than 16 bits to encode the keycode, so more extensive changes are certainly going to be needed and I'm afraid I don't have a nearly good enough overview of the overall architecture of the software, or the range of hardware it needs to run on, to be able to judge what such a change would entail.

On the other hand, one could also think along this line: all these symbols, which require modifiers to be generated, such as braces and parentheses, are generally only a handful, at least in the keymaps I've seen. There are 255 "normal" keycodes, so it is likely that one could spare some and it might be useful to be able to override them and be able to say: "instead of sending KC_CLEAR_AGAIN, send a MOD_LSFT and a KC_9".

This would allow arbitrarily complex stuff, expending only as few keycodes, as absolutely necessary, and these codes should furthermore play nicely with all existing features. Sorry for bringing all these alternatives up during the final stages of the PR review, but they occur to me as a result of the discussion.

PS: I've just tried a proof of concept implementation of this and it seems to work pretty well. I overrode KC_CLEAR_AGAIN with a function that sent a parenthesis and then assigned MT(MOD_LSFT, KC_CLEAR_AGAIN) to a key to get the space cadet functionality. It seemed to work fine, without any side-effects, as far as I could see and I don't see why it couldn't be set to send Alt+Shift+7, or any other combination.

So the basic interface could be something like:

void register_my_lparen()
{
    add_mods(MOD_LSFT);
    add_key(KC_9);

    return;
}

void unregister_my_lparen()
{
    del_key(KC_9);
    del_mods(MOD_LSFT);

    return;
}

override_keycode(KC_CLEAR_AGAIN, register_my_lparen, unregister_my_lparen);

Of course more simple interfaces for basic overrides, can probably be layered over that.

The implementation would be equally simple: override_keycode simply maintains a linked list of overridden keycodes, each element being a (keycode, function) pair, which is ckecked inside register_code and unregister_keycode.

I would like to have some feedback from someone with more knowledge of the internals on this, but if there are no serious issues I'm missing, I think this would be a preferable approach to the current PR, as it looks like it could serve the same purpose and much more.

@fredizzimo

This comment has been minimized.

Copy link
Collaborator

fredizzimo commented Apr 7, 2018

I hope I soon will have time to add support for making custom mod_tap keys, and then all modifiers and much more will be supported. So maybe it's worth to wait a little bit more with this still?

@dpapavas

This comment has been minimized.

Copy link
Author

dpapavas commented Apr 10, 2018

I can wait as long as necessary; it's up to project maintainers to merge this PR if and when they see fit. In fact, as I've said, I think that the approach I described in my last post would be prefferable, as it would provide a superset of the functionality, in a simpler way. I wouldn't mind making a new PR with an implementation, if someone with more knowledge of QMK's internals could review the proposed approach and comment on whether there might be any issues with it, that I'm missing, or other reasons that would make it inappropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment