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

LT too slow #303

Closed
languitar opened this issue May 3, 2016 · 55 comments
Closed

LT too slow #303

languitar opened this issue May 3, 2016 · 55 comments

Comments

@languitar
Copy link

I am using LT to temporarily switch to other layers and often end up in the situation that I pressed the following key too early, which just results in two keypresses, one from the LT usual key and one from the other key but in the base layer.

Is there any way to speed up the reactions of LT? I don't even need the real toggling into the target layer. Instead something like "if this key is pressed and while being hold, another key is pressed, use the keycode from the target layer" would be sufficient for me.

@nrrkeene
Copy link

nrrkeene commented May 4, 2016

I see where LT is defined as a bitmask but where is it handled?

@eltang
Copy link
Contributor

eltang commented May 4, 2016

@nrrkeene It's handled here!

@languitar
Copy link
Author

Ok, I have observed in more detail what is making me accidentally type characters from the base layer instead of the target layer: If you press the LT-key first, then the key in the target layer and then release the LT first and then the target key (and all of this with rapid speed).

@jackhumbert
Copy link
Member

Unfortunately, that's just the design/implementation of the LT shortcut, which is really just ACTION_LAYER_TAP_KEY - the code that handles the action is here, but the code that handles the LT shortcut is here.

@languitar
Copy link
Author

Any easy way to change this so that the order of key release doesn't matter?

@nrrkeene
Copy link

nrrkeene commented May 6, 2016

Ianguitar, you are using a Layer Tap feature which is handled with the code that eltang linked to. The thing missing from that code is the 'interrupted' feature. When you tap a key, the event that gets processed tells you whether another key was pressed during the timeout period: if so, then the 'interrupted' flag is true.

But in the code eltang linked to, there is no check for the interrupted flag. That means, if you press the key and a second key quickly, the code produces the Tap character and then produces the second character for the other button. What you are saying is instead if interrupted is true, you want to switch layers.

You can do that with a custom handler. Try this.

Define an integer X. In the layout, call F(X), then in fn_actions define [X] = ACTION_MACRO_TAP(X), then in action_get_macro write a block to handle it:

case X:
if (record->event.pressed) {
    if (record->tap.count && !record->tap.interrupted) {
        register_code(KC_QUOT);
    } else {
        layer_on(3);
    }
} else {
    if(record->tap.count && !record->tap.interrupted) {
        unregister_code(KC_QUOT);
    } else {
        layer_off(3);
    }
}
break;

The default handling is probably more popular with people. I am currently writing some code to handle layer-switching keys and I experimented with 'interrupted' and finally settled on code semantically identical to the default code. My problem is because the key in question is my apostrophe key, so when I type it's isn't can't I kept getting symbols from my second layer, which is not what I want.

@eltang
Copy link
Contributor

eltang commented May 6, 2016

@nrrkeene Instead of testing record->tap.interrupted on key release, it might be easier just to test record->tap.count. All you have to do is set record->tap.count to zero on keypress if record->tap.interrupted was true then so that the second test of record->tap.count fails.

@nrrkeene
Copy link

nrrkeene commented May 6, 2016

To be honest I don't really understand tap count so I haven't monkeyed around with that value.

@eltang
Copy link
Contributor

eltang commented May 6, 2016

That's just a variable that counts the number of taps! You can see where it gets incremented here.

@nrrkeene
Copy link

nrrkeene commented May 6, 2016

Like number of taps of that one key? Like if I tapped it twice super fast, the count would be two? Would I get two events (a 1 event then a 2 event) or just one event?

@eltang
Copy link
Contributor

eltang commented May 6, 2016

Yup! There would be two events!

@nrrkeene
Copy link

nrrkeene commented May 6, 2016

So if I tap twice super fast I get

Event: mouse down, tap 1
Event: mouse up, tap 1
Event: mouse down, tap 2
Event: mouse up, tap 2

If I set the first event tap count to zero, is it zero for all of the remaining three events?

@eltang
Copy link
Contributor

eltang commented May 6, 2016

Nope! The tapping restarts.

@nrrkeene
Copy link

nrrkeene commented May 6, 2016

I bet in that situation Ianguitar would expect two characters but if I understand correctly he wouldn't get it if tapping restarts.

@languitar
Copy link
Author

Since I haven't had a look into the details of the firmware: is the snippet above still the recommended approach to try this?

@nrrkeene
Copy link

I'm stuck on this problem again. I have my custom tap-macro handler which either shifts layers or outputs a key. The problem is when I want to use shift in conjunction with the output key (to get double quotes instead of single quotes). Similar to what languitar describes, but the opposite: If I press shift, then my key, then release shift, then release my key.

When I do that I get the lower-case symbol but I want the upper-case symbol.

I think what is happening is that

  • the shift-down event is handled
  • then the tap-macro-down event is sent to the TAP_MACRO function which buffers it waiting for the tap timeout
  • then the shift-up event is handled
  • then finally the tap-macro-up event is sent to TAP_MACRO function which emits the event-down and event-up events to my handler

If that is right then the problem is that shift is already released before my code receives the two events.

I thought I could fix this by using the oneshot feature but it didn't work for me. When I handle the shift event I tried calling set_oneshot_mods(KC_LSHFT) but that caused me to get no symbol output at all.

@eltang
Copy link
Contributor

eltang commented May 12, 2016

@nrrkeene I think I know what the problem is. Try using the version of action_tapping.c here.

@nrrkeene
Copy link

I tried that yesterday and it didn't help, but I diffed it against my version and all I saw was something about special keys? What does that do?

@eltang
Copy link
Contributor

eltang commented May 13, 2016

Usually, the tapping code can detect when you try to modify a tap key and retains the modifiers. Any other keys are released before the tapping starts. Your Shift macro is not recognized by the tapping code and is therefore released before the apostrophe is sent. I tried to make the tapping code recognize the keys in those positions as modifiers, but somehow that didn't work. I'll need to look further to see what went wrong.

@nrrkeene
Copy link

Oh! That makes sense. Now I understand that code, I see what you are doing with the keypos_t's.

To tell the whole truth I had trouble compiling it yesterday and the 'fixes' I made, I bet, were catastrophic. Now I understand better but I don't have my keyboard to test. I'll let you know in the coming days if this feature works for me - thank you!

@nrrkeene
Copy link

I've tried it again with this tweaked function

static bool is_special_tap_key(keypos_t key) {
    keypos_t a = (keypos_t){ .row = 0, .col = 3 };
    keypos_t b = (keypos_t){ .row = 0xD, .col = 3 };
    if KEYEQ(key, a) return true;
    if KEYEQ(key, b) return true;
    return false;
}

and, no, I get the same behavior. I get single-quote instead of double-quote if I release (my simulated version of) Shift before releasing the quote key.

@eltang
Copy link
Contributor

eltang commented May 15, 2016

@nrrkeene I didn't realize that your Shift keys are tap keys themselves. That throws some extra wrenches into the works. Can you explain how they work? I'll try to make an implementation that does not have to use the tapping code.

Edit: Your apostrophe key is a tap key too? :O

@nrrkeene
Copy link

I spent some more time reading action.c and action_tapping.c and I haven't yet figured out why my code doesn't have the same behavior as just pressing LSFT. When I change the key to KC_LSFT, though, it functions the way I want my macro to function.

So I'm looking for the right way to emulate the LSFT key. Should I set a weak_mod or macro_mod? Should I instantiate a keyevent_t object and call action_exec()?

@nrrkeene
Copy link

My shift keys are not in fact tap keys. I have the tap-macros defined but I call the code with M() not F() so it doesn't go through the tapping code. (Right?)

@eltang
Copy link
Contributor

eltang commented May 15, 2016

Oops, some stuff in your fn_actions array confused me. You are exactly right that it will not be processed as a tap key if you call it with M(). Why are you checking record->tap.count in the macro code, though? I think it be easier for us to find what's wrong if you avoid putting your macros through the tapping code unless it's absolutely necessary.

@nrrkeene
Copy link

I see you are looking at my code carefully -- so am I! An hour ago I took out that tap.count check form line 241 because it was vestigial.

I'm out of time today, but I've been looking carefully at action.c in the case ACT_LMODS block. I see in there it add_mods() before calling register_code(), so maybe I need to do that. Unless you think otherwise, I'm assuming that my problem is due to incorrectly using the keyboard API, so I'm not expecting a code change on your part. I'm just trying to understand the platform well enough to use it.

@eltang
Copy link
Contributor

eltang commented May 15, 2016

In your case, register_code is no different than add_mods. The former just calls the latter if the keycode is a modifier. It would also be helpful if you could open your keyboard viewer and identify at which moment your system stops recognizing that Shift is pressed. I suspect that this happens right as you press your apostrophe key.

@languitar
Copy link
Author

Since I am still struggling with this: any progress here? ;-)

@nrrkeene
Copy link

nrrkeene commented Jun 2, 2016

I haven't done any work on this for a month. My life has been busy. But I've been using the keyboard every day and the quotation-mark problem is the only beef I have with my layout.

eltang asked me when my shift key stops being engaged but I don't think it does. I have my red LED light up when I press shift and it stays illuminated when I do the key combo with my fake shift key and my tap-quote key.

Eric, I've never used the 'keyboard viewer" how do I do that?

@eltang
Copy link
Contributor

eltang commented Jun 2, 2016

@nrrkeene The LED might only be reflecting the physical position of the key (up or down), and not the actual signals that are being sent to your computer. The on-screen keyboard can be found buried somewhere in your computer's settings, but it might be easier for you just to use this site instead.

@eltang
Copy link
Contributor

eltang commented Jun 2, 2016

@nrrkeene That is extremely strange. Your computer should be printing a quotation mark instead of an apostrophe if it detects that Shift is pressed.

@nrrkeene
Copy link

nrrkeene commented Jun 3, 2016

The shift key is up by the time the keyboard sends the quote character, but
the tap feature should be supplying the necessary shiftiness.

On Thu, Jun 2, 2016 at 1:02 PM, Eric Tang notifications@github.com wrote:

@nrrkeene https://github.com/nrrkeene That is extremely strange. Your
computer should be printing a quotation mark instead of an apostrophe if it
detects that Shift is pressed.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#303 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAO2UfbGqx--dNvTtcWTsJ10OuV3sJ-sks5qHza5gaJpZM4IWKSf
.

@languitar
Copy link
Author

So what is the solution? I still struggle with this issue.

@jackhumbert
Copy link
Member

Sorry, might have closed this too soon. Can you try this in your keymap.c real quick?

#define LT_CUSTOM 0x7100

bool layer_interrupted = false;

bool process_record_user(uint16_t keycode, keyrecord_t *record) {
    switch(keycode) {
      case LT_CUSTOM: {
      if (record->event.pressed) {
        layer_interrupted = false;
        layer_on(3);
      } else {
        if (!layer_interrupted) {
          register_code(KC_V);
          unregister_code(KC_V);
        }
        layer_off(3);
      }
      return false;
      break;
    }
    default: {
      layer_interrupted = true;
      break;
    }
}

You'll need to put the #define at the top of your keymap, and then put LT_CUSTOM on the key you'd like to do this, and on the receiving layer (or have KC_TRNS). Right now, it outputs v on tap, and turns on layer 3 on hold.

@languitar
Copy link
Author

Keyboard is at work. I'll try that on Monday.

@languitar
Copy link
Author

I just can't get it this working. Once I set a key to LT_CUSTOM it just doesn't do anything anymore.

@jackhumbert
Copy link
Member

Is the rest of your code up to date? This might require some of the more recent changes. If you could share your keymap where you have it implemented, that might help too.

One more caveat with this is that it requires you to release the key before typing another, otherwise the second key will get the layer modifier applied to it.

@languitar
Copy link
Author

Right, my fork was too old. Now this snippet works and the behavior seems much better to my mind.

Is there any way to get this in a more general form? I'd also need this for other layers and more keys, and also CTRL_T etc.

@jackhumbert
Copy link
Member

Sweet! You can copy and paste it as much as you'd like, but you'll need unique names for the layer_interrupted variable and LT_CUSTOM of course - you can increment up from 0x7100, or use an enum like this:

enum custom_keycodes {
  LT_CUSTOM = 0x7100,
  LT_CUSTOM2,
  LT_CUSTOM3
};

Eventually the *_T and others will be reimplemented in a similar way to this.

@languitar
Copy link
Author

Ok, I was hoping there was a more automatic solution ;-)

How do I do this for CTL_T?

@jackhumbert
Copy link
Member

Something like this should work (haven't tested this one, unlike the first):

#define CTL_T_CUSTOM 0x7101

bool ctl_t_interrupted = false;

bool process_record_user(uint16_t keycode, keyrecord_t *record) {
    switch(keycode) {
      case CTL_T_CUSTOM: {
      if (record->event.pressed) {
        ctl_t_interrupted = false;
        register_code(KC_LCTL);
      } else {
        unregister_code(KC_LCTL);
        if (!ctl_t_interrupted) {
          register_code(KC_V);
          unregister_code(KC_V);
        }
      }
      return false;
      break;
    }
    default: {
      ctl_t_interrupted = true;
      break;
    }
}

@languitar
Copy link
Author

Ok, one more final thing: with the current LT macro I can double-tap to get back the original key being pressed continuously. Is this supported somehow?

@jackhumbert
Copy link
Member

Not with this implementation - it'd be possible to do something like this though! I believe you'd have to implement a timer of sorts.

@languitar
Copy link
Author

Yes, probably. there is also another minor issue that now the layer led blinks each time i press the key. Maybe these are things to collect for the new implementation of the LT macro.

@jackhumbert
Copy link
Member

That's pretty interesting :) Do you have your full code on github?

@languitar
Copy link
Author

https://github.com/languitar/qmk_firmware/tree/issue-303

The keymap is called languitar ;-)

@jackhumbert
Copy link
Member

Nice! Well this is why your LEDs light up :) it should stay on for as long as your hold the key though. The layer is active from the second you press the key, which is why the LED lights up when you press it. You could add a cycle delay to the LED lighting up if it's super annoying, but then it would always be a little delayed (possibly more annoying).

@languitar
Copy link
Author

If this wasn't implemented as a single keycode, shouldn't it be possible then to postpone the layer activation to the point where the next key is pressed?

@languitar
Copy link
Author

languitar commented Jun 20, 2016

Nice! Well this is why your LEDs light up

With the existing LT the layer also only becomes active if it is not a single key stroke. Therefore this didn't matter so far.

@jackhumbert
Copy link
Member

Yeap! This is just modelled off of the modifier control, which you usually want to activate as soon as you press the key.

@languitar
Copy link
Author

Ok. maybe I find some time to experiment a bit more how to get this more into the direction I was thinking about.

@languitar
Copy link
Author

Any progress on modeling the default LT this way?

@yroeht
Copy link

yroeht commented Nov 4, 2019

I ran into this exact issue, I was trying to make my layer keys (e.g. RAISE) act as another key (e.g. KC_SPC) when tapped, and soon realized I could no longer rapidly insert symbols (e.g. releasing RAISE before KC_J would result in "j" instead of "-").

The solution suggested by jackhumbert seems to work fine (thank you for this). For reference, this is how I adapted it to my use case: yroeht@cc78964

Having to implement custom keycodes for this feels cumbersome though. Is there any interest in changing LT's behaviour, or implementing a new alternative? Or is this issue being closed a sign that developing this would not be welcome?

@yanfali
Copy link
Contributor

yanfali commented Nov 4, 2019

@yroeht this issue is 3 years old. Take a look at https://github.com/qmk/qmk_firmware/blob/master/docs/config_options.md#behaviors-that-can-be-configured TAPPING_TERM or TAPPING_TERM_PER_KEY

p00ya pushed a commit to p00ya/qmk_firmware that referenced this issue Aug 20, 2020
* fix: italian keymap it quot is redefined

* fix: missing deprecated swedish key code

* Adds Moonlander to the list of supported keyboards

* Sorts list and adds EZ suffix to Planck in Readme

* Update Mouse Wheel config

Co-authored-by: Florian Didron <0x6664@hey.com>
Co-authored-by: Erez Zukerman <1092548+ezuk@users.noreply.github.com>
BlueTufa pushed a commit to BlueTufa/qmk_firmware that referenced this issue Aug 6, 2021
* Capture KC_APP properly
* Use a lookup table on ev.key
jiaxin96 pushed a commit to Oh-My-Mechanical-Keyboard/qmk_firmware that referenced this issue Dec 20, 2022
Jpe230 pushed a commit to Jpe230/qmk_firmware that referenced this issue Jul 12, 2023
* [Keyboard] Add FL-Esports FL980

* update license headers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants