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

Combining Shift with certain characters #50

Closed
majutsushi opened this issue Dec 21, 2015 · 14 comments
Closed

Combining Shift with certain characters #50

majutsushi opened this issue Dec 21, 2015 · 14 comments

Comments

@majutsushi
Copy link

I am trying to replicate the Shift-key functionality that Steve Losh described here: http://stevelosh.com/blog/2012/10/a-modern-space-cadet/#shift-parentheses . The idea is that tapping the Shift key produces a left or right parenthesis, respectively, and holding it results in the normal shifted functionality. Unfortunately this doesn't work for me with this firmware; if I try to do it the obvious way with SFT_T(KC_LPRN) then tapping the key produces a 9, not a (. This has probably to do with the fact that the parenthesis is a shifted 9, but it would be great if this could work regardless.

@aspickard
Copy link

+1 for this issue

@jackhumbert
Copy link
Member

Just a quick explanation of why this is - the SFT_T(<kc>) and similar functions are really just shortcuts for ACTION_MODS_TAP_KEY. The S(<kc>) and KC_LPRN are shortcuts for ACTION_MODS_KEY, so when you try to combine these two, the QMK implementations of each don't connect.

It's possible to do this, but my guess is that it'll need to be a custom function/framework.

@eltang
Copy link
Contributor

eltang commented Jan 18, 2016

@majutsushi Does this work?

static uint16_t start;
if (record->event.pressed) {
    start = timer_read();
    return MACRO(D(LSFT), END);
} else {
    if (timer_elapsed(start) > 150) {
        return MACRO(U(LSFT), END);
    } else {
        return MACRO(T(9), U(LSFT), END);
    }
}

@aspickard
Copy link

@Eric-L-T

Just gave that shot and it did the trick! Thanks a lot for the suggestion :)

More detail for anyone else that needs to do this:

// Have to include timer for timer_read and timer_elapsed
#include "timer.h"

// Give our macro a human readable name
#define HOLD_SHIFT_TAP_TILDA 1

// Put everything inside the action macros function in the base template
const macro_t *action_get_macro(keyrecord_t *record, uint8_t id, uint8_t opt)
{
      // This will hold the timestamp of when we start holding the key
      static uint16_t start;
      switch(id) {
          // When we hit our new macro
          case HOLD_SHIFT_TAP_TILDA:
              // On keydown, start the timer
              if (record->event.pressed) {
                  start = timer_read();
                  // Start holding shift
                  return MACRO(D(LSFT), END);
              } else {
                  // On key up
                  // If time was greater than 150ms, it was a hold
                  if (timer_elapsed(start) > 150) {
                      // Only release the shift key
                      return MACRO(U(LSFT), END);
                  } else {
                      // Otherwise it was a tap, put in a tilda (since shift is still held)
                      // and release the shift key
                      return MACRO(T(GRV), U(LSFT), END);
                  }
            }
            break;
      }
}

Now all you have to do is use M(HOLD_SHIFT_TAP_TILDA) or M(1) in your keymap to have tap/hold functionality with higher value characters.

@noahfrederick
Copy link
Contributor

@Eric-L-T @jackhumbert

I'm not sure that this is sufficient to implement the original shift-parentheses feature. There may be an issue with executing multiple macros at once, or there is some additional complexity that is not being taken into account.

If both shift keys are pressed at overlapping intervals, such as can happen when typing (), the keyboard permanently gets into a state where key presses produce seemingly arbitrary results. The firmware must be re-flashed at that point to be used.

This is what I have:

// Max duration that counts as a tap (ms)
#ifndef TAPPING_TERM
#define TAPPING_TERM 150
#endif

// Special macros
#define LSFT_PAREN 0
#define RSFT_PAREN 1

const macro_t *action_get_macro(keyrecord_t *record, uint8_t id, uint8_t opt)
{
  static uint16_t start[2];

  switch(id) {
    case LSFT_PAREN:
      if (record->event.pressed) {
        start[LSFT_PAREN] = timer_read();
        return MACRO(D(LSFT), END);
      } else {
        if (timer_elapsed(start[LSFT_PAREN]) > TAPPING_TERM) {
          return MACRO(U(LSFT), END);
        } else {
          return MACRO(T(9), U(LSFT), END);
        }
      }
      break;
    case RSFT_PAREN:
      if (record->event.pressed) {
        start[RSFT_PAREN] = timer_read();
        return MACRO(D(RSFT), END);
      } else {
        if (timer_elapsed(start[RSFT_PAREN]) > TAPPING_TERM) {
          return MACRO(U(RSFT), END);
        } else {
          return MACRO(T(0), U(RSFT), END);
        }
      }
      break;
  }
}

@jackhumbert
Copy link
Member

Ah, that's likely to do the one timer variable being used - you're not having that problem when using the two, right? I'd throw the static uint16_t start[2]; declaration outside of the function to be safe.

@noahfrederick
Copy link
Contributor

you're not having that problem when using the two, right?

No, I'm afraid I'm experiencing the issue even with multiple timers. I'm actually not so sure when exactly the issue is triggered. I was just now able to press both shift keys together a couple times with reasonable results before it locked up into this strange state again.

Here's the full keymap file for reference: https://gist.github.com/noahfrederick/353457890c8bcae4671a

@DidierLoiseau
Copy link
Contributor

I don't know whether that's your issue but remember that pressing both shift keys at the same time is the Magic key combination, which allows you to turn on debug mode, or change the default layer by pressing Magic+number. Magic is LShift + RShift, but I have checked and SFT_T()+RShift also works.

If you switch to an unused layer, its sometimes impossible to come back…

As far as I can tell this would be reset when unplugging the keyboard, but I think are read somewhere that it is possible to make some parameters persistent.

@jackhumbert
Copy link
Member

Ah, yeah - that sounds right. @noahfrederick Check your config.h to change the IS_COMMAND() definition, and see if the weirdness still happens.

@noahfrederick
Copy link
Contributor

@DidierLoiseau I think you hit on the problem.

As far as I can tell this would be reset when unplugging the keyboard

My mistake. That is indeed the case here. I can unplug the keyboard to restore it.

@jackhumbert

Check your config.h to change the IS_COMMAND() definition, and see if the weirdness still happens.

Yep, that seems to have done the trick. Thanks.

Note: I made a modification to avoid typing (0 when rolling the shift keys to get (), because a U(LSFT) can be registered after the D(RSFT) and before the T(0). I send D(*SFT) again in the key-up branch of the condition to avoid that scenario. Hopefully that doesn't hurt anything.

static uint16_t start[6];

const macro_t *action_get_macro(keyrecord_t *record, uint8_t id, uint8_t opt)
{
  switch(id) {
    case LSFT_PAREN:
      if (record->event.pressed) {
        start[LSFT_PAREN] = timer_read();
        return MACRO(D(LSFT), END);
      } else {
        if (timer_elapsed(start[LSFT_PAREN]) > TAPPING_TERM) {
          return MACRO(U(LSFT), END);
        } else {
          return MACRO(D(LSFT), T(9), U(LSFT), END);
        }
      }
      break;
    case RSFT_PAREN:
      if (record->event.pressed) {
        start[RSFT_PAREN] = timer_read();
        return MACRO(D(RSFT), END);
      } else {
        if (timer_elapsed(start[RSFT_PAREN]) > TAPPING_TERM) {
          return MACRO(U(RSFT), END);
        } else {
          return MACRO(D(RSFT), T(0), U(RSFT), END);
        }
      }
      break;
  }
}

@jackhumbert
Copy link
Member

Awesome!

@ezuk
Copy link
Contributor

ezuk commented Mar 17, 2016

@noahfrederick - revisiting this because I'm interested in the feature; is your keymap anywhere on github?

@noahfrederick
Copy link
Contributor

@ezuk Sure. It's here: https://github.com/noahfrederick/dots/blob/51bd77988eb6a1b368281196df0280b51b0ca0e9/planck/keymap.c

It turns out this implementation needs some refinement, though. As I've gotten faster at typing on the Planck, I'm more and more often typing an unwanted parenthesis after a capital, because the time I'm holding shift doesn't exceed TAPPING_TERM. A timer is only part of the picture. There also needs to be a way of tracking that another (non-modifier) key was pressed before shift-up is registered.

@ezuk
Copy link
Contributor

ezuk commented Mar 20, 2016

A fellow Fish user! That's awesome :)

And yes -- I agree re the need to detect another key. This is a good starting point though, thank you!

jackhumbert pushed a commit that referenced this issue Jul 7, 2017
Add compiled HID bootloader CLI binary to .gitignore.
drashna referenced this issue in drashna/qmk_firmware Jul 9, 2019
Fix issue with Split Common backlight disabling
rizalfr pushed a commit to rizalfr/qmk_firmware that referenced this issue Sep 4, 2021
add vial support for Sofle and its encoders
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

7 participants