Qmk_firmware: Debounce with EZ causes lots of duplicate presses

[kthibodeaux]

Disclaimer: I have no idea what debounce even means.

I have had an old clone of the QMK firmware I have been using that was checked out about 8 months ago. I recently switched to a different OS so I now have the latest QMK.

When I made a change to my EZ layout and flashed my keyboard I noticed I get a lot of duplicate key presses. I change debounce to 10 and then 12, that helped a little, but I am still seeing the issue. I switched back to the old sha for now (cd544e10a) and everything seems to be working perfectly (without me needing to change the debounce).

My layout can be found here: https://github.com/kthibodeaux/ergodox-layout

Is there a quick fix that I'm missing to allow my keyboard to work on newer versions on QMK?

Edit: It seems switching it to 15 has totally resolved the problem. With the new default being 5, is there something else I could do to fix this? I assume 15 is no longer the default for a reason, but it is the only way my keyboard will work.

Edit 2: It is still happening, although much less often. I would say without changing the settings it happens every few words, and with the debounce at 15 it happens every few paragraphs. I'm going to switch to that old sha after all. What a roller coaster!

[drashna]

That's because there are two PRs that need to land to fix these issues.

The main one is: https://github.com/qmk/qmk_firmware/pull/6081
This one fixes a bug in the Eager debouncing algorithms. The Ergodox (OG and EZ) use the Eager Per Row algo, which is part of the problem.

Additionally, I suspect that the i2c_master code has an issue when optimization is enabled for it. This PR should fix it:
https://github.com/qmk/qmk_firmware/pull/5617

If you apply both of these PRs to your branch, it may resolve the issues that you're seeing outright. Or at least, help drastically!

[kthibodeaux]

I did a lot of typing and it seems to have helped a lot.

In this comment I'm making I have had to correct 5+ double types.

Thank you for the fast response!

[danielo515]

I also faced this problem. It happens specially on keys that are harder to reach or that you hit with a weaker finger like the pinkie.
I'm on the exact same situation as you. I tried also tunning debounce and changing the settings of repeat keys on my OS, but none of those are a satisfactory solution.

[danielo515]

I just merged the latest master into my fork and seems that the problems with debounce has been fixed. The only problem is that firmware size has increased (again).
I'll setup my keyboard repeat back to normal and see how good it goes.

[kthibodeaux]

I just tried the latest and I am still see the issue, and quite often (3 times in this comment)) - make that 4 :laughing:

Edit: I am not noticing a pattern to the keys affected. Letters, numbers, keys on layers, tap dancing thumb keys, they are all double typing.

[danielo515]

The patter I notice is when my finger is "tired" or it is weak (like the pinky), then I usually get double taps

[drashna]

@danielo515 Yeah, feature creep. :(
And for the duplicate presses, that's an actual debounce issue. Setting that higher may help.

@kthibodeaux That's very odd. Try increasing your debounce setting? Or try adding DEBOUNCING_TYPE = sym_g to your rules.mk and see if that helps.

[danielo515]

Today I noticed something... let's say weird.
This debounce problem seems to not be just tied to our potential problems we have as humans, but something more deep on the code. Today I got a debounce error with a sendstring macro. I got two accented characters like this: éé. The macro is quite complex because it uses the alt key to generate the international keycode. Not sure how that is even possible

[danielo515]

I don't think increasing debounce could help.
Mine was already 11 and I was still getting issues. Making it higher makes my computer feel slow.
I'll try that other debouncing type to see how good it goes.

[ethanspitz]

I'm having this issue with a handwired dactyl. Tried all the debounce algorithms, bringing the time up, etc. Certain things made it worse than others, and bringing up the debounce time helped slightly.

I did notice a pattern though, it's mostly (if not only) on the top row of the slave (right). I have a pro micro (elite-c actually) on each side communicating using I2C. I'm pretty close to main and I don't see any commits since I last grabbed main that could have helped.

I also noticed something odd. When I plug the cable into the right half, the top row doesn't work on that side anymore. Not sure if it's related or some sort of bug in the code.

My configurations are here: https://github.com/ethanspitz/qmk_firmware/tree/master/keyboards/handwired/dactyl_promicro

[danielo515]

I am on the latest version of QMK and it is still being an issue.
This is getting worse every day and make me less productive with my keyboard rather than more.
Did any of the involved ones on this issue found a solution? In which firmware version worked for you? I'm willing to go to that one and just re-apply my changes there.

[kthibodeaux]

@danielo515 I build my changes on top of sha cd544e10a and it works great for me

[kthibodeaux]

@drashna DEBOUNCING_TYPE = sym_g combined with a debounce of 15 (with latest master) has reduced the errors greatly, but it is still unusable

[drashna]

@alex-ong Sorry for tagging you again, but it looks like there are still some issues with the debounce code.

Unfortunately, I don't really see this on my board.... So my ability to help here is limited.

[alex-ong]

I don't have an ez either haha. There is a trade-off between responsiveness and debounce.

Using sym_g with 15 should be the same as before I ever touched it; which from memory works perfectly fine, though people complained about slow responsiveness. Theoretically sym_g should NEVER give you extra presses, though it should have a weird latency feel to it.

Using eager_pr/pk can give you ghost presses since it has no emi protection; you can get presses without pressing anything. I've never experienced this.

Ergodox is problematic because of the incredibly slow scan rate due to i2c bottleneck. Eager_pk with 500hz scan rate and 5-8ms debounce should always work, but eager_pk kills your scan rate to 133hz. This is why we switched to eager_pr, which uses a timer per physical column, and leaves scan rate at around 300hz.

Changes in other parts of qmk have in general made the codebase slower, reducing scanrate over time. Rgb is another thing that reduces scanrate.

I actually don't understand how eager_pr /pk could give ghost presses. I've never seen switches that need more than 5ms debounce. The code is simple ; when you press a key, send a press then block all changes for DEBOUNCE ms. It is literally impossible to get a double press unless your physical switch sent two presses greater than (debounce*2) Ms apart. If you get a double press with DEBOUNCE = 15… then debounce.c is receiving press - 15ms - release -15ms - press

Sym_g is even more un-double inputtable; it waits for completely stable input from the entire keyboard for DEBOUNCE ms. If a change occurs while it's in the frozen state, it resets it's timer and waits even more. To get a double press in sym_g, debounce.c is receiving a press, 15ms of same input, then release, 15ms of same input, then press, with another 15ms of input.

We should verify what the undebounced input looks like; if someone could log a time-stamped (Ms elapsed) / raw button state that could help.

Another thought (along the "debounce is perfect with no bugs, the input is the problem" theory) is that at some point the 5 cycle / 2 nop delay between selecting/reading pins got removed. This might give raw data as noise? It was removed (the nop thing) as talking to the i2c had an incurred 5-10 cycle wait already.

@kthibodeaux - what do you mean by unusable, (after using sym_g)? Latency or erroneous (extra) presses? Extra keypresses shouldn't be possible with sym_g

[kthibodeaux]

@alex-ong Extra keypresses (with sym_g and debounce of 15). More rare, but still happens

We should verify what the undebounced input looks like; if someone could log a time-stamped (Ms elapsed) / raw button state that could help.

Is there any documentation on how I could do that?

Could it be a combination of settings that I have? I am using permissive hold, tap dancing, and set my tap dance term to 100. All of this works great on sha cd544e10a (it could work in some later commits as well, this just happens to be one that I've been sticking with).

[danielo515]

What about asking ergodox_ez company members to have a look? At the end it is their hardware and their clients are going to face this problem sooner or later.
I would love to help also with the error report, because obviously the theory is far from matching the practice, but I need instructions on how to do this.
Just to be clear, this was not happening before.
I'm also using tap_dance and leader key, nothing else.

[drashna]

@danielo515 Well, that's what makes this complicated. The Ergodox board design is open source, and that's what Ergodox EZ is using. This issue effects any of the boards that use that design.

The simplest fix here, is to pull the matrix for the old commit and ues that. (eg run git checkout cd544e10a keyboards/ergodox_ez/matrix.c, I believe).

However, as this affects more than just Ergodox EZ's boards, I would rather that we pursue a proper solution that works in all cases, rather than just reverting code. Especially as there are ~6 or so other boards that use the same hardware setup that the Ergodox does.

As for ZSA (the Ergodox EZ company) looking into the issue, well, that's why I'm monitoring this issue, and doing what I can to get a good resolution. Absolute worst case is that we revert the code. But I would rather avoid that if at all possible.

[alex-ong]

@danielo515

Just to be clear, this was not happening before.

Do you know which commit does work? i'm more than happy to compare matrix.c's and debouncing

[danielo515]

According to @kthibodeaux cd544e10a works fine

[kthibodeaux]

Yep, cd544e10a works fine.

I am not saying that is the latest working commit; it just happens to be the random commit one of my computers had checked out because I rarely pull on it. I attempted to git bisect to try to find exactly when this started, but I had no luck with that.

[danielo515]

If I try to bring stuff from that commit to my branch I get this kind of errors:

keyboards/ergodox_ez/matrix.c: In function 'select_row':
keyboards/ergodox_ez/matrix.c:358:13: error: too many arguments to function 'i2c_stop'
             i2c_stop(ERGODOX_EZ_I2C_TIMEOUT);

So probably not as simple as just making a checkout of some files

[alex-ong]

Alright; looks like the _working_ commit already removed the 30ms selection delay, so we can rule that out. EDIT: maybe not.

Could someone try adding 30us worth of delay between these two lines?

select_row(left_index);
select_row(right_index);

//30us delay. Just to rule out select_row() being too fast.
wait_us(30);

changed |= store_raw_matrix_row(left_index);
changed |= store_raw_matrix_row(right_index);

I could recreate the debounce method in cd544e10a (it has a debounce timer per key. After an action, it waits DEBOUNCE cycles before allowing any changes again). This should be the same as eager_pk (except eager_pk waits DEBOUNCE milliseconds), so assuming its not a bug in something else, eager_pk must be bugged. We could verify by using eager_pk with DEBOUNCE set to roughly 5 * 1/scanrate milliseconds, which should be 16. I'll have look at the rest of the changes (non debounce related) too.

I'd be interested in the current scanrate.
Could someone #define DEBUG_MATRIX_SCAN_RATE in matrix.c, and turn the logger on and let me know the rate?

i2c code looks identical to me.

[kthibodeaux]

@alex-ong I added the wait_us line to master, and still see the issue

I'm not familiar with how to do debugging with QMK. I was following https://beta.docs.qmk.fm/newbs/newbs_testing_debugging but when I have CONSOLE_ENABLE = yes to my rules my firmware is too big to load on my ergodox.

[alex-ong]

Ahh if you're running out of space thats a bit tough; but using CONSOLE_ENABLE is the right way.

I'll try and implement the debouncing code verbatim; if it works then that would be great. Then we could attempt to figure out what's wrong with eager_pr/pk. Unfortunately I don't have the keyboard so can't test it...

[alex-ong]

@kthibodeaux:

Try pulling this: https://github.com/alex-ong/qmk_firmware/tree/ergoez_debounce
It compiles but i havent tested it. I copy pasted the code almost verbatim. Let me know how it goes.
Here's the diff, it should be pretty easy to paste the new file and change a config 👍

https://github.com/qmk/qmk_firmware/compare/master...alex-ong:ergoez_debounce

If it works then its time to figure out whats wrong with eager_pk/pr...

(changes are to rules.mk which just uses debounce.c instead of eager_pr. Then i put the old implementation into debounce.c).

[kthibodeaux]

@alex-ong I cloned the repo and added my layout to it (I assume I didn't need to change my local rules.mk) and the issue still persists :(

[alex-ong]

Just confirming that the rules.mk you're compiling with has debounce.c (my rendition of the working branches debounce)?
@kthibodeaux

[kthibodeaux]

@alex-ong Thank you for confirming. I double checked and forgot to check out your branch. Thank you so much for your continued attention and patience!

I copied debounce.c to my layout's folder, and changed my layout's rules.mk to add SRC += debounce.c and DEBOUNCE_TYPE = custom.

I'm hesitant to say, but it looks like the issue is resolved!!!

[kthibodeaux]

I've been using it for an hour or so now, and every now and then (rare) I still see a duplicate press. It might be every 100 words or so (compared to every 5 before this).

The current master has DEBOUNCE 5, where as the working commit has DEBOUNCE 15. I assume that will fix the issue 100% with the custom debounce method, but it might be worth changing that default for the EZ?

[alex-ong]

I'm glad that it's improved!

Try testing it on 15 and let me know how you go.

One thing to note is that since this implementation of debounce is scan_rate based, its real debounce is DEBOUNCE * time_per_scan_in_ms. Last time i checked this was roughly 3ms. Setting it to 15 means that it is using an effective debounce time of 45ms~, which is definitely not ideal; cherry keys bounce for 5ms MAXIMUM.

The fact that 5 itself isn't enough (remember 5 scan_cycles is roughly 15ms. waaay larger than the spec'ed rate) is very... disturbing. There is something definitely off about ergodox (ez?) or something else in QMK seeing as sym_g, eager_pk, eager_pr and even this algorithm all are giving ghost presses with 15ms / 5 cycle debounce, but only on this keyboard... I feel the that sym_g / eager_pk / this all failing due to all having independent bugs is probably not that likely.

The only difference between eager_pk and this algorithm afaict is this is cycle based and eager_pk is time in ms based. eager_pk + DEBOUNCE = 15 should be same as debounce.c + DEBOUNCE = 5, assuming scan_cycle time is 3ms. A possibility is maybe your scanrate is abnormally low (say 100hz); most of the debounce algorithms would totally break (in different excitng ways) at this point.

Definitely sit on DEBOUNCE=15 for a while and see how that goes; i'll try and get a branch up that does scanrate reporting.

[kthibodeaux]

I've been on DEBOUNCE=15 since making my last comment and I have not had a single issue

Edit: If it matters I'm using Kalih copper switches

[alex-ong]

Could you try mainline branch (eager_pr) and use DEBOUNCE=45? And let me know how that goes?

Otherwise i'm happy to add this debounce algo to debounce folder and hook up ergodox_ez to it.
@kthibodeaux

[kthibodeaux]

@alex-ong using latest master and setting DEBOUNCE=45 works great for me

[danielo515]

Isn't devounce 45 ridiculously high?

El vie., 6 sept. 2019 12:10, Keith Thibodeaux [e-mail]
escribió:

@alex-ong https://github.com/alex-ong using latest master and setting
DEBOUNCE=45 works great for me

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/qmk/qmk_firmware/issues/6269?email_source=notifications&email_token=AARKJWJ6HA6PKXOYUYVGLE3QIIUAXA5CNFSM4H6TI2X2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6CM3GY#issuecomment-528797083,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AARKJWLVDM2FK2T5Q6CA2NLQIIUAXANCNFSM4H6TI2XQ
.

[drashna]

It is on the high side, IMO. but it's also debouncing the entire row, not the entire matrix or individual keys, so I think this may be okay.

[alex-ong]

Isn't devounce 45 ridiculously high?

Yes. There is still an inherit problem _somewhere_ in the chain, but i was just checking to see that there wasnt anything wrong with debounce itself. I ported the old algorithm over and it was found that it needed 15 cycles to be stable in current QMK. This is equivalent to 45ms~. So the new debounce algorithm (eager_pr/pk) is no different in terms of performance.

The original kthibodeuax working commit cd544e10a is 5 cycles. We have currently no idea what the cycle rate is. The new debounce rate is 15 cycles. Since the rest of the qmk codebase gets faster/slower over time this 15 cycles could be exactly the same, or it could be completely different 🤷‍♀.

I'm guessing that

• qmk has gotten slower over time
• all the _time_ based debounce algos (eager_pr/pk) completely crash at really low scan rates,
• _cycle_ based debounce algos also seem to crash at low scan rates (old impl still isn't good at DEBOUNCE=5)
• 15 cycle scan rate / 45ms on time/cycle algorithms just masks it.

Or

• qmk has gotten significantly faster over time (yeah, nah)
• it was always using 45ms debounce rate, cycles were 9ms before and now are 3ms/cycle
• there was always some weird underlying bug in i2c or hardware related that was masked by having a 45ms debounce rate

From a technical point of view eager_pk and the old algo are the extremely similar and should fail almost identically.

If someone can benchmark scanrate/double-tap for the old commit (the latest with debounce=45 would mean you can't have two key-downs faster than 90ms)

So far at least we have a fix (debounce=45. It's eager so you shouldn't get any latency on key-down, but you will get 45ms latency on key-up)

The problem seems to me like its not debounce algo related.

[drashna]

Well, that was about the same time that we set "Prevent Stuck Modifiers" as the default, too. So it's very possible that that's part of why it got slower.
https://github.com/qmk/qmk_firmware/pull/3107

So .... I wouldn't be surprised if that's part of the problem.

As for testing, I can do that later this week.

[alex-ong]

Nice; do you remember how to print out polling rate drashna? Though from memory you can use master branch of ergodox_ez with no problems; right?

[danielo515]

@alex-ong I'm trying to apply your changes in my branch and I'm getting the following error:

Linking: .build/ergodox_ez_danielo515.elf                                                           [ERRORS]
 | 
 | /tmp/cc8xcohF.ltrans5.ltrans.o: In function `keyboard_init':
 | /qmk_firmware/tmk_core/common/keyboard.c:207: undefined reference to `matrix_init'
 | /tmp/cc8xcohF.ltrans5.ltrans.o: In function `suspend_wakeup_condition':
 | /qmk_firmware/tmk_core/common/avr/suspend.c:172: undefined reference to `matrix_scan'
 | /qmk_firmware/tmk_core/common/avr/suspend.c:175: undefined reference to `matrix_get_row'
 | /tmp/cc8xcohF.ltrans5.ltrans.o: In function `keyboard_task':
 | /qmk_firmware/tmk_core/common/keyboard.c:274: undefined reference to `matrix_scan'
 | /qmk_firmware/tmk_core/common/keyboard.c:279: undefined reference to `matrix_get_row'
 | /qmk_firmware/tmk_core/common/keyboard.c:285: undefined reference to `matrix_print'
 | collect2: error: ld returned 1 exit status
 | 
tmk_core/rules.mk:298: recipe for target '.build/ergodox_ez_danielo515.elf' failed

Am I missing something?

[danielo515]

Ok, your branch is on qmk 0.5.174 while I was on 0.6.414

[danielo515]

I can confirm that building with your branch (adding my changes into it) and with a normal debounce of 11 I didn't experienced a single issue on the entire day, so I'm very glad the old mechanism still works.

[tyxieblub]

I can confirm that eager_pr on master and DEBOUNCE=45 works for me too.

Just wanted to point that the issue first occured without any direct update of the firmware (which was from May). The system (debian sid) has been upgraded several times during this time, though.

Edit: Well, on master before #6994… had this page open for too long.

[danielo515]

Is this solved for everyone? I'm still getting lots of duplicated key presses on certain keys, the n is one of the most affected and it is very annoying because I use it a lot.

[kthibodeaux]

I haven't had any issues since changing my debounce to 45.

[danielo515]

Latest master commit and that debounce? I just flashed with the default ergodox debounce on master which is 30. If the problem persist I will raise it to 45.
Thanks

[igorrafael]

After some experimentation, I fixed mine with DEBOUNCE=15 and DEBOUNCE_TYPE = sym_g

[kthibodeaux]

@danielo515 yes

@igorrafael I will have to try that!

[danielo515]

Do you remember that I said the n key was one of the most repeated ones? Well, today it suddently started to do the opposite: despite I was hitting the key, it was not being registered properly, I had to hit it several times.
I just changed the switch (thanks god for ergodox_ez hot swap switches) and now it does not get repeated as much as it used to be. I would say it now behaves like most other keys. Sometimes I get an extra unwanted keystroke (even with the debounce set to 45) but nothing crazy like before.

I'll have to try what @igorrafael proposed. Did you tried @kthibodeaux ? Does it improve?

[kthibodeaux]

@danielo515 I tried what @igorrafael used to fix his, but it does not totally eliminate the issue for me. It is much rarer than DEBOUNCE 15 without DEBOUNCE_TYPE = sym_g but still is an issue.

I went back to just DEBOUNCE 45 to solve it.