ONESHOT_TIMEOUT not timing out - firing off character repeatedly

chrischambers commented 8 years ago

The behaviour is only working as expected in the 'townk_osx' keymap, for reasons I can't identify. I've tested in my own keymap as well as in minimalist versions based on some of the existing keymaps.

To easily reproduce, take any keymap with >= 2 layers, set a key to OSL(1) in the base layer, and set ONESHOT_TIMEOUT = 3000, say, in your config_user.h

Expected behaviour: Given the above, I should be able to press the OSL(1) key, wait 3 seconds, and have the layer revert back to the base layer. Alternatively, I should be able to hit the OSL(1) key and then hit another key within 3 seconds to have its equivalent layer 1 mapping fire.

Actual behaviour: If you press OSL(1) and wait more than the 3 seconds, the layer doesn't revert back to base: when you then hit a key, it will repeatedly fire off the layer 1 mapping for that key until you press a keystroke. The alternate behaviour does work as expected.

chrischambers commented 8 years ago

@Townk - any idea why ONESHOT_TIMEOUT is working properly in your keymap but isn't elsewhere?

Townk commented 8 years ago

Sorry for the late response. I'll check it out tonight to see if I can find something.

chrischambers commented 8 years ago

That'd be awesome, thanks. :)

Townk commented 8 years ago

Chris, do you have your key map in some place so I can check it out?

chrischambers commented 8 years ago

Sure, it's available here. Having said that, I don't think it's particular to my mapping - I believe you can take any of the ergodox_ez mappings, make the alterations I've listed above and encounter the same issues. I think I tested with the default mapping and the kines-ish one.

Townk commented 8 years ago

Try to use the fn3 key for the one shot layer with time out. That is the difference between your key map and mine. Also it explains why I was able to change the default and have it working :).

Townk commented 8 years ago

Btw, if that works we know the problem is on the OSL macro.

chrischambers commented 8 years ago

Pretty sure I've already tried that, but I'll test again now...

chrischambers commented 8 years ago

Yup, it's not a problem with the OSL macro, same issue occurs when I use ACTION_LAYER_ONESHOT.

Townk commented 8 years ago

Well, you're right! I changed my layout to use the macros instead and it's still working. Also I realized that I didn't test the default keyboard yesterday and indeed the OSL does not behaves as expected. I'm trying to get my layout to be like someone else's to see what is different and I'll get back to you once I have something.

Townk commented 8 years ago

ok, I could even reproduce the problem on my layout. If I try to use OSL on my KEYPAD layer I have the same behavior as you. I tried to change the layer values of default keyboard (from 1 which is my KEYPAD to 2 which is my FN) but the problem persist.

At this point I'm pretty sure is something on the action.c file. I'll try to setup a console here to try to debug it but I'm not finding any documentation on how to do so. If anyone have any ideas on the original problem or how to get a console here, let me know.

ezuk commented 8 years ago

@jackhumbert thoughts?

chrischambers commented 8 years ago

Any progress @Townk ?

Townk commented 8 years ago

Didn't have time to dig into this yet, but is in my todo list.

ahtn commented 8 years ago

Could you please provide a keymap which has this bug? I tried to reproduce it on my branch (not QMK) and I couldn't reproduce it.

chrischambers commented 8 years ago

@ahtn : I've linked to mine in the 6th post in this thread, but as said, it applies to any of the ergodox_ez keymaps (steps to reproduce in the first post).

ahtn commented 8 years ago

Ah thanks, how did I miss that? Tried again on my branch and I still couldn't reproduce this, so it's probably specific to the ergodox ez or QMK. The code I used was based off this branch.

@Townk To get console output for debugging, you use the HID listen program. Also, add these flags to your makefile

EXTRAFLAGS += -DDEBUG -DDEBUG_ACTION

See debug.h and print.h for more info.

ezuk commented 8 years ago

So @jackhumbert and I took quite a bit of time tonight to dig into this. Long story short, definitely able to replicate on my end -- this is a confirmed bug.

What's interesting is that @Townk's keymap relies on code that he added to action.c, but it only works with his keymap. The problem here is that this is nestled deep within action.c, which is very hard to decipher. We're going to take much of the stuff implemented in this file and essentially rewrite it into quantum.c in a way that we feel is easier to understand, and then we'll also be able to fix this issue.

I know this is a small comfort, but let's keep this issue open so this stays on the radar. And @Townk if you feel like digging in, that would be amazing :)

Townk commented 8 years ago

I want to give it a try but no matter what I do I can't get the HID listen program to show anything. When I add -DDEBUG to EXTRAFLAGS I get this error:

`Compiling: matrix.c [ERRORS]		:0:12: error: expected ',' or '}' before '-' token	../../quantum/keymap.h:88:5: note: in expansion of macro 'DEBUG'	DEBUG,	^

make[1]: [.build/obj_ergodox_ez_townk_osx/matrix.o] Error 1 make: [all] Error 2`

ahtn commented 8 years ago

Ops, looks like you also need this in the makefile

CONSOLE_ENABLE = yes # Console for debug(+400)

Even if the -DDebug flag is still casing errors, you should still be able to use the functions from print.c with CONSOLE_ENABLE.

I haven't used QMK, so it's possible something might have changed from TMK.

Townk commented 8 years ago

I found some flaws on the one shot layer implementation and now I'm working to find a fix for it. I still didn't find the reason why it's working on my keymap but I found that even on my keymap there are bugs (the one shot layer does not time out as expected). I'll update you more after I spend more time with this code over the weekend.

jackhumbert commented 8 years ago

@Townk are you interested at all in moving the implementation into the new quantum format? I'm working on making that more standardised here, where each feature's function will get called from the process_record_quantum function in quantum.c. I'm hoping eventually all of the shortcuts we have setup (MT, MO, LT, OSL, etc) will be implemented here in their own files instead of in action.c.

chrischambers commented 8 years ago

Status update? :)

Townk commented 8 years ago

I probably will look into the new way once is available but right now I don't have time to investigate (family is getting bigger and we started to do classes after my work hours :P). One thing though, I try to check the link @jackhumbert pasted here (https://github.com/jackhumbert/qmk_firmware/tree/quantum-keypress-process/quantum/process_keycode) but I get a 404 there. Is there a branch that I should checkout for it?