cub-uanic / tmk_keyboard

keyboard controller firmware for Atmel AVR USB family
158 stars 157 forks source link

Improved debouncing code, scan rate ~950Hz #43

Open fd0 opened 6 years ago

fd0 commented 6 years ago

Hi,

thank you very much for making your firmware for the ErgoDox available! I was only a user until recently, when I thought I had a bad switch in my keyboard. Long story short: The perceived latency until my terminal emulator shows a window on a largish screen makes me hold the keys until the auto repeat kicks in and a second terminal emulator is started. I've detailed it in an issue here: https://github.com/i3/i3/issues/3306#issuecomment-401632787

So, in the course of this odyssey, I've rewritten the debouncing code (which was not so great, it blocks the whole keyboard for ~8ms when a single key is pressed) and managed to increase the keyboard scan rate from ~370Hz to ~950Hz, tested with the scan latency measurement code.

I've published my forked code here: https://github.com/fd0/ergodox-tmk

If you're interested, we can try merging my changes back here. I just thought I give you a heads up and say thanks! :)

squarefrog commented 6 years ago

Haven't had a chance to test this out yet, but it looks very good! Thanks for letting us know.

fd0 commented 6 years ago

So, if you haven't played with it yet, I encourage you to do that. It's a completely different (much better) way of typing. I did not know what I was missing!

squarefrog commented 6 years ago

When you say it’s completely different, how so?

marknsikora commented 6 years ago

The basic idea seems valid, doing a full scan instead of debouncing on each row. I'm a bit more weary about removing some of the delays and increasing the i2c frequency. I had issues with missing key strokes when I tried mucking with that stuff when I took over the project.

As for pulling the changes that will be quite a bit more difficult since you didn't clone our repo. You can either make a clone, reimplement your changes, and submit a pull request. Or one of us can make the changes but you won't be the author of any of the diffs.

fd0 commented 6 years ago

Ah, sorry, I should have been clearer: I'm willing to spend some time merging my changes back into this project, as pull requests. Debouncing is the most important one I think, followed by a few optimizations that don't require increasing the i2c frequency.

I was just testing the waters here if somebody reacted, to find out if you're willing to merge a PR. Otherwise I can save myself some time :)

fd0 commented 6 years ago

When you say it’s completely different, how so?

It feels much better. It's a bit hard to describe, at least in retrospect typing on the ErgoDox always felt different than on other keyboards. It may have very well been the input latency due to the debouncing code blocking the keyboard.

Here's a bit of background on latency and debouncing keyboards: https://michael.stapelberg.de/posts/2018-04-17-kinx-keyboard-controller/

Btw, the ErgoDox EZ people are using the qmk_keyboard firmware, which for the ErgoDox EZ is also derived from this very firmware here, and they have also changed the debouncing algorithm by using a scan counter (as uint8_t) for each key: https://github.com/qmk/qmk_firmware/blob/master/keyboards/ergodox_ez/matrix.c#L217

fd0 commented 6 years ago

Without raising the i2c frequency at all, I'm getting around 770Hz scan rate (vs. 360Hz with the default code) :)

fd0 commented 6 years ago

PR #44 has only the debouncing code so you can give it a try if you like.

squarefrog commented 6 years ago

I'm currently trying this out. Can't say I've noticed a huge difference to be honest - perhaps I simply don't type quick enough to notice?

fd0 commented 6 years ago

That's possible. Michael described in his article that in a small experiment he did that one person was able to reliably detect 15ms of added input latency, while other people did not notice 75ms of additional latency (over the whole stack). So it's very personal, depending on your perception.