Add delay to grid scanning function to prevent row/col "ghosting"

[ChrisChrisLoLo]

First of all, want to give a big thank you to all of the members who have contributed to this project! I've been playing around with it, and I'm super happy with it!

I want to outline here a potential enhancement idea to prevent key misreads on larger keyboards.

What is the problem

On my current keyboard project, I was able to set up all of the firmware required to get my keyboard sending key presses to my PC (firmware source here). One thing I noticed though is that whenever I pressed a key on the first column, I would get qa instead of q . This behavior also occurred for other keys on the column (i.e az instead of a, etc.). It seems like every keystroke on the first column also registers the key below it. This is with a typical qwerty layout on a 4x10 keyboard.

What seems to be the issue is that there isn't enough of a delay for my particular board to prevent the signal from a key from bleeding into the row input below it. More specifically, I think there is not enough time between setting the previous row back to high and listening to the next row after it. I suspect the reason why it's happening for me in particular is because my row and column traces are particularly long due to the pcb shape. I know that my board works fine with QMK, and adding a delay to the matrix scan in my local fork of keyberon seemed to resolve the issue. I think my workaround is generally sane since QMK also seems to have delays in their own matrix scanning function (relevant LOC in QMK here).

Proposed solution

To potentially mitigate this solution for others, a potential solution may be adding an optional embedded_hal::blocking::delay (documentation here) field in the Matrix struct, as well as an optional delay duration field that can be used by the matrix scan. The delay would occur after setting the row to low, and then again after setting the row back to high. This may give more flexibility to those whose hardware encounters these kinds of issues.

If anyone has an alternative or better solution, please let me know: it may be entirely possible that there is a better workaround out there that doesn't require modifying this crate. If this general idea is sound however, I can try to make a PR for this.

Thanks!

[TeXitoi]

I never needed any delay, and that's why there is not actually.

It can be needed if you have some capacitance on your circuit, maybe caused by some coil or bad soldering, or slow GPIO.

What's your MCU?

You only need a delay between the set_low() and the reading of the input gpios.

The problems are:

• having some optional object in the matrix will add checks that are not needed, as you have always the delay object or never
• having ownership of a delay object can be "costy" as you might be limited in the number of delay objects (limited number of timers for example). You should give the object as &mut at scanning time.
• having to give a &mut Delay when this feature is not used is quite ugly.

So, as the matrix.rs code is really simple, maybe better to create a dedicated MatrixWithDelay object, that takes a FnMut() closure in the get method, that is called after row.set_low(). The actual matrix can be implemeted over this MatrixWithDelay struct, giving an empty closure at the get method.

[ChrisChrisLoLo]

Thanks for your thoughts on this!

It might very well be due to some vaguely coil like traces I have on my PCB. I'm using a Raspberry Pi Pico right now, and my board effectively runs traces on the outside perimeter of the footprint (since there's a cutout in the middle of the footprint). I'll have to read up on inductance again, software is generally more my forte haha.

I think what you've suggested makes sense! I think that's a fairly clean way to implement it. I'm fairly new to Rust, so I appreciate the API considerations.

When I get the chance to in the next few weeks, I'll take a crack at opening a PR for this!

[TeXitoi]

I thought of a very simple solution: adding a method to matrix:

pub fn get_with_delay<F: FnMut(), E>(&mut self, mut delay: F) -> Result<[[bool; CS]; RS], E>

And just call delay() before reading the input pins, and impl get with self.get_with_delay(|| ())

[rwalkr]

I'm another RP2040 user seeing the same issue - looks like the GPIO needs some settling time between setting the row and reading the columns - I was seeing some ghosting on some keys

I've implemented the get_with_delay() function described above and with a 10us delay, it all works fine:

matrix.get_with_delay(|| { delay.delay_us(10); })