qmk / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
18.03k stars 38.75k forks source link

Tap dance switching layer instead of tap key (hhkb) #865

Closed cigam closed 7 years ago

cigam commented 7 years ago

I've been playing with getting the tap dance feature on an hhkb (see my first commit)

But when I tap one of the tap keys (1, 2...), it simply switches to the second layer...

algernon commented 7 years ago

I have seen this reported before, usually on keyboards other than the ErgoDox. So far, I have been unable to reproduce the issue on my keyboard, and I'm kinda out of ideas :/

cigam commented 7 years ago

Too bad... I was looking forward to trying it out...

I couldn't find any debugging tips document, is there any documentation on how to debug such an issue?

fredizzimo commented 7 years ago

As @algernon mentioned this was previously reported in #756, but I add my answer here as the title of this issue is more descriptive.

I have checked the tap dance code, and I don't see anything that has to do with layers and that makes me worried that this could be a result of a memory corruption or by some strange interaction with some other feature.

I therefore added a pull request for a variable tracing debug feature, which hopefully could help in tracking this down.

If the layer changes then either the layer_state or default_layer_state variables should also change. So I recommend that you (@cigam) start by tracking the layer_state, by adding it to your matrix_init_user function

void matrix_init_user(void) {
  ADD_TRACED_VARIABLE("layer", &layer_state, sizeof(layer_state));
}

Then you should be able to start tracking down what actually changes is by adding VERIFY_TRACED_VARIABLES to the process_record_quantum function. At first I recommend adding it at the start, and end, and around the matrix_scan_tap_dance function. Then depending on what it reports, continue narrowing the actual place that changes it down.

You will also need to add VARIABLE_TRACE=1 to the end of you make command as described in the readme of the pull request. You probably also want to wait until it's merged, but it is possible to check out the pull request locally before that.

Hope this helps.

shelaf commented 7 years ago

@cigam You need create Makefile for your own keymap and define TAP_DANCE_ENABLE=yes. (See Tap Dance: A single key can do 3, 5, or 100 different things)

Without this difinition, TD() macro defines keymap as KC_NO. (See process_tap_dance.h#L68). But, I have no idea why switches to the second layer...

ofples commented 7 years ago

Hey I think I figured it out. When sending shifted keys, tap dance calls the register_code16 function. This function then calls the do_code16 function which checks if modifiers are held by and'ing the code with the different QK_X modifier values. For left shift this would be 0x200 and for right shift it is 0x1200. What this means is that a left shifted key, for example 0x21F, will be recognized as both left shifted and right shifted, making it a magic key, and switching in this case to the second layer. If you want a quick fix, change your magic key combination to something other than RSHIFT + LSHIFT. Will try to think of something better.

Thanks @fredizzimo for your variable tracer!

Edit: Add if (code < 0x1000) return; to your do_code16 function between right modifiers and left modifiers. This will prevent false positives, and your tap dance should work!

skullydazed commented 7 years ago

I'm closing this to clean up our open issues, but if you still have questions @cigam please reopen this or open a new issue.