Linux key press debounce setting

[Loosetooth]

Describe your changes. Use imperative present tense.

Faulty or dirty keyboard hardware often results in 'chatter' or 'double taps'. Where a single physical press on the key results in multiple key press events in rapid succession.

This PR adds support for configuring a debounce duration on Linux to mitigate keyboard chatter or double taps.
Only key press events are debounced, other events are passed through.

Based on @jtroo 's comment from a while back, advising to create an event loop like in the Windows specific code:
#408

Checklist

• Add documentation to docs/config.adoc
  • Yes
• Add example and basic docs to cfg_samples/kanata.kbd
  • Yes or N/A
• Update error messages
  • N/A. Let me know if I need to add error messages.
• Added tests, or did manual testing
  • Yes, manual testing on NixOS Linux.

This is my first work in rust ever, so please bear with me. Any feedback is welcome.

[jtroo]

Thanks! The high level approach seems reasonable.

[jtroo]

Reading and passing this here means live reloads will not update correctly. This should be read within the preprocessor loop.

Likely would be good to have the datatype within the Kanata struct be an Arc<Mutex<_>> for fine-grained locking of this value.

[Loosetooth]

@jtroo Thanks for the feedback. I am looking into it.

I am a bit stuck on the implementation of a good debouncing algorithm.
If you want to look, check out my WIP code on the debounce branch.

Before you look though (you don't even have to), a summary of what I have been trying:

1. I took a look at the description of debounce algorithms for QMK
2. I found the asym_eager_defer_pk algorithm most relevant for the issue where after a bounced key, repeat events should still be handled

The "eager" algorithms are not that difficult to implement, just keep track of when certain keys are pressed in a HashMap, and block/process events accordingly. But, as you correctly identified before, this can give issues with fast press/release combos and repeats afterwards.

For the "defer" algorithms: I need to have a cancellable timer of some sort in order to delay the sending of events.
Now, I don't know if we can do something like this ourselves in Rust, or if we want to use a package for that.
If so, what package would you use? Is it okay for you to add a dependency to the project to achieve this?

[jtroo]

The existing processing loop follows a pattern that could be useful here:

https://github.com/jtroo/kanata/blob/2f35bcd98ca569bddc284fdb4fcae361e4b37290/src/kanata/mod.rs#L749C1-L758C1

https://github.com/jtroo/kanata/blob/2f35bcd98ca569bddc284fdb4fcae361e4b37290/src/kanata/mod.rs#L800C1-L803C33

https://github.com/jtroo/kanata/blob/2f35bcd98ca569bddc284fdb4fcae361e4b37290/src/kanata/mod.rs#L1924C1-L1933C38

https://github.com/jtroo/kanata/blob/2f35bcd98ca569bddc284fdb4fcae361e4b37290/src/kanata/mod.rs#L2019C1-L2020C37

https://github.com/jtroo/kanata/blob/2f35bcd98ca569bddc284fdb4fcae361e4b37290/src/kanata/mod.rs#L2110C1-L2110C80

You can process while doing a non-blocking check on the receive channel every 1ms if there is any pending action and tick down any deadlines. If no deadlines need to be processed, block on the receive channel to save on processing cost.

[Loosetooth]

@jtroo Thanks again for the feedback.

I did my best to follow your recommendations.
Two algorithms have been implemented for now:

1. asym_eager_defer_pk
2. sym_eager_pk

These can also be selected via the config file.
Some more can be added at a later date.

Some other remarks:

1. I did not test live reloading yet
2. I added quite many log::info! calls in the code. Is this OK? Should we replace or remove them? At the moment they completely hide the pretty 'active layer' preview in the console...

Any other feedback is welcome of course.

[jtroo]

I added quite many log::info! calls in the code. Is this OK? Should we replace or remove them? At the moment they completely hide the pretty 'active layer' preview in the console...

I haven't yet gotten a chance to review in more depth, but you should change these to debug or delete the calls before we can merge.

[jtroo]

Thanks for the work so far! In addition to the comments, please also add tests for the debounce algorithms. PR is heading in the right direction :)

[Loosetooth]

In this test I noticed that the sym_defer_pk algorithm, as implemented right now, can change the order of the pressed keys if they are pressed at exactly the same time.

This can be verified by removing the Simulate a short delay line.

I am not sure if this is an issue. But feel free to comment @jtroo since you have much more experience in these matters.

[jtroo]

Would this be because the iteration order of HashMap is not guaranteed?

        for (&oscode, &(ref event, deadline)) in &self.pending_events {
            if now >= deadline {
                log::debug!("Emitting deferred event for {:?}", oscode);
                try_send_panic(process_tx, event.clone());
                to_remove.push(oscode);
            }
        }

[Loosetooth]

I tried to take your remaining feedback into account.

I also added some tests, feel free to comment on those as well.

This could definitely still use some extra documentation and more debouncing algorithms, but let's leave those for separate PRs.

[Loosetooth]

I already noticed that some of the tests I wrote are flaky.
So these should be rewritten or disabled. (See 7e43873 for the commit in another branch that disables the two tests in question.)

Edit: or the tick algorithm should be rewritten -- as you recommended in one of your comments -- in order to ensure the same order of emitted events as the actual hardware events.

[jtroo]

I've done the splitting out of the tick counting in this commit:

1c53cc0

[jtroo]

I already noticed that some of the tests I wrote are flaky.

I guess rather than a hashmap we need a map that maintains insertion order for iteration order. BTreeMap may do the trick

[jtroo]

I already noticed that some of the tests I wrote are flaky.

I guess rather than a hashmap we need a map that maintains insertion order for iteration order. BTreeMap may do the trick

Actually no this isn't correct; BTreeMap is sorted by key, not insertion order. I guess really it should just be a Vec. The number of elements simultaneously in the collection should always be very small anyway, so the asymptotic benefit of maps don't come into effect.

[Loosetooth]

The debounce preprocessor loop has been updated to use your extracted count_ms_elapsed function.

The debounce algorithms now use Vec instead of Hashmap (where required), to ensure the correct order of emitted events.

[jtroo]

I haven't yet checked the debounce algorithms in-depth but the looping code is looking very close to mergeable :)

Thanks for the work so far!

[Loosetooth]

Alright, count_ms_elapsed is now used correctly.

[Loosetooth]

@jtroo Let me know if anything else is blocking this.

[jtroo]

Ah thanks for the reminder. The PR checks are failing, so would need to resolve those.