Add charliplex keyscan

[HookyKB]

Adds support for https://kbd.news/Square-or-round-robin-matrix-1400.html with addition of an extra interrupt pin for low power support.

Implementation of this matrix in hardware is left as an exercise for the user. Ensure all lines to the interrupt pin are not able to re-feed the matrix or it will surely fail.

Example:

• 6 pin charliplex matrix with interrupt support - for a 6x4 + 3 board*

Shields implementing this scan setup may not be suitable for all boards (see the referenced document above).

It is expected that, with proper design, (compared to a matrix kscan) an additional 2*n diodes will be required, where n is the number of input pins in use. One for the line to the input pin (this may not be required for some controllers), and one for each 'column' to the interrupt pin. Note that not all of the charliplex matrix pins need to be used as input to the controller.

eg. An 8 pin charliplex kscan matrix may use all 8 pins for output, but only use 5 for input.

See HookyKB@40221a4 for example usage.

[petejohanson]

Thanks for this! A few thoughts on the details.

[oha-Ohashi]

Also, I'd love to have a feature that allows me to reverse the columns and rows. Currently, I'm working with a circuit that looks like this:

(source: https://jpskenn.github.io/blog/2020/12/05/first-year-of-my-self-made-keyboard-activity)
I find this circuit quite beautiful and it would be amazing if it could be supported by default.
I'm not certain how it can be achieved, though.

This is merely a request. If the implementation is difficult or if this proposal does not align with your thoughts, please feel free to disregard it.

[HookyKB]

@oha-Ohashi

The column/row don't actually matter with the round robin. If you consider RC as send, receive or out, in you can build you settings the same way. Having the diodes before or after the switch doesn't matter either.

In your transform, you would just have:

RC(2,1) RC(3,1)
RC(1,2) RC(3,2)
RC(0,3) RC(2,3)

(note that this is how the switches are labeled anyway, so that helps keep your hardware and software aligned)

It's not really rows and columns, just out/in pairings.

Also note that flipping all the diode directions (but no position) gives you the same capability, but with the out/in reversed, and you keep the same picture.

[oha-Ohashi]

@HookyKB Thank you for your response. it seems that my understanding was not enough.

[HookyKB]

@petejohanson auto polling config added. See #1694 (comment) regarding the wait config.

[nguyendown]

I find that "charlieplex" would be a proper name for this type of key scan.

[HookyKB]

I find that "charlieplex" would be a proper name for this type of key scan.

It is, but Round Robin is equally proper, and less ambiguous IMO.

[nguyendown]

It is, but Round Robin is equally proper, and less ambiguous IMO.

While some people may be familiar with the term "round-robin tournament," it still does not accurately reflect the underlying technique used in the keyscan, which relies on diodes being forward biased.

What's more important is that the keyscan is entirely based on charlieplexing, so there's no reason to come up with a new term for it. Let's just use the already established term. It's less confusing and way more straightforward.

[HookyKB]

@nguyendown I don't see a problem. No-one is 'confused', you would just prefer a different name. I didn't come up with the name, I used the title of the reference document, and in there you'll see that the author was also unaware of the name you're suggesting.

Also, with the additional interrupt handler, it is now its own entity, so, I'm pretty happy calling it not charliplexing. You'll may also like to note that charlieplexing was a term assigned to a long pre-existing circuit structure, so 🤷. (And thus named for someone who didn't come up with it, and I like that even less.)

[petejohanson]

Code looks good. Can you please rebase one more time to pick up the pre-commit check work, to be sure the MD files all pass?

Thanks!

[nguyendown]

Can we put this pull request on hold? Because of the naming convention? I am currently messaging the author of the article about the proper name.

[petejohanson]

I'm happy to discuss more. Regardless of technicalities, the term "round robin matrix" has become pervasive. At a minimum, if we do want to merge this with a different name, then the docs should clearly identify this as being the same design known as "round robin matrix" by many.

[nguyendown]

I'll bring it here so that we can have an open discussion.

There are three names in the article: Charlieplexing, Round Robin Matrix, and Square Matrix. I immediately knew that "Square Matrix" would not work well. The term is not unique, and people may ask, "square matrix? Is it linear algebra?" You'd have to explain "square means N-square keys" every time you mention it. You'd also need to tell them "it's not exactly N-square, but like N-square minus N." Not to mention that "square matrix" could also refer to a normal matrix with four cols, and four rows.

"Round Robin" is interesting, but even if someone is familiar with the term "round robin tournament," they may only have a vague idea of how it works. That's not enough to make a keyboard because the technique relies on diodes being forward biased, which is not covered by "round robin tournament".

Also, the term "round robin" is not unique.
https://en.wikipedia.org/wiki/Round-robin

Charlieplexing does not describe itself at all. Why is this a more appropriate name? Let me explain. Multiplexing is hard to describe in just a few words, which is why a typical name for multiplexing usually is something + "plexing". Just like learning a language, people will have to look it up to understand a word. Because the term is unique, a web search for "Charlieplexing" leads to a handful of material that describes it well. And because the term gives us zero presumptions, it gives zero confusion. Plus, it is a well established term. If someone already knows about Charlieplexing, they'll understand how the keyscan works.

I get that using a simpler term makes it more accessible to a wider range of people. However, I would argue that it's not necessary because only enthusiasts, who are technical audiences, would learn about the keyscan anyway, regardless how complex the term is. End users typically do not care about it. If they want to understand it, "Charlieplexing" is still a better term for web searching.

One more thing I'd like to point out is that adaptations (or discussions) of Charlieplexing in keyboards do exist. It is rare, but that doesn't mean we should replace the term.
https://github.com/thanks4opensource/markoplexing
https://github.com/crides/zmk/blob/wren/app/drivers/kscan/kscan_gpio_charlieplex.c
You can find discussions about "charlieplex" in the Discord servers of ZMK and QMK as well. By keeping the term, we can maintain consistency in communication.

Zephyr

Also, the term "round robin" is not unique.

Zephyr also refers to "round robin" as "round robin scheduling" grep -rl "round.robin" zmk/zephyr

Keyscan

I'll explain why I insist on using the term "Charlieplex" for the keyscan.

Here's the original Charlieplex (1). SW0-1 indicates that the key press will be detected when pin0 outputs and pin1 inputs.

(2) and (3) are the same as the original, but the switches are arranged in a grid.

(4) and (5) have an additional diode per pin to prevent ghosting.

(6) and (7) use an interrupt pin to detect key press from sleep.

All the variants use the same scanning technique as the original, which is why the keyscan should be named "Charlieplex."

Variants

Trying to name every possible variation or arrangement is not practical. Not only is it not feasible, but it also creates even more confusion. My suggestion is to simply describe any additional features. We can refer to (1), (2), and (3) as the Charlieplex. (4) and (5) can be called the Anti-Ghosting Charlieplex, while (6) and (7) can be referred to as the Low-Power Anti-Ghosting Charlieplex.

[michaelrommel]

Hmmmm, is it really now the case, that because of differences in the name of a feature, it is not being merged and users are not able to make use of the feature?

I honestly couldn't care less if the feature is called "Low-Power Anti-Ghosting Charlieplex in Round-Robin, Square-Matrix Fashion with superpowers" or just "Matrix Connection Variant #7"...

Please: who can make a decision to merge a feature here?

Thank you for considering your users and preventing further forks just to implement already submitted feature branches...

[petejohanson]

To be honest... I was fairly "round robin matrix" camp, until doing a bit of googling for the terms "keyboard round robin matrix" and "charliplex keyboard matrix".

The later brings up several authoritative articles, including the KBD News one, such as:

• this from 2015: https://www.embedded.com/keyboard-and-display-multiplexing-charlieplexing/
• The GitHub link mentioned by @nguyendown from 2019: https://github.com/thanks4opensource/markoplexing
• Deskauthority from 202: https://deskthority.net/viewtopic.php?t=23990

To name a few.

The first search term mostly returns non-keyboard related articles, with only a smattering, including the same KBD News article and an Atreus GitHub link actually being what we're talking about.

Given that... I actually am convinced, and suggest we use "charliplex" as the authoritative term, with alternative terms included in the documentation.

@HookyKB I know you've worked very hard on this, for which I'm grateful. Any major objections to adjusting the naming on this before we merge?

[HookyKB]

@petejohanson Yep, no problems. I'll do the rename & rebase this weekend. Two devs in disagreement meant nothing IMO, but once the repo owner rolls in, I'm generally happy with what they decide. 👍

[HookyKB]

@petejohanson all yours. Tested with my intro board, all working as expected.

[ReSummit]

@ReSummit, which version of the Xiao are you using? It has been tested with the Xiao BLE. You can see the config here: 7a9a74e

Can you share your repo so I can check out the settings. Have you tried just setting the pins without the | GPIO_PULL_DOWN?

Also, I doubt you'll get this board to work properly without ghosting as the board is a duplex matrix, not charlieplex.

@HookyKB
I am currently using the Seeeduino Xiao SAMD21. I tried it separately in a Arduino sketch to see if the GPIO input / output switching was a problem but it did fine.

Here is a fork of your repo with the changes I made. Checkout kscan/round_robin and refer to board/shield/011 contents to see the config I have for the board. It does work, though I will note what you mentioned about ghosting:
https://github.com/ReSummit/zmk-charlieplex/tree/kscan/round_robin

I did try doing it without | GPIO_PULL_DOWN but this resulted in the microcontroller believing that I pressed all the keys at once (or at the very least, spamming random keyboard inputs to the computer). This is with all the pins without | GPIO_PULL_DOWN; if >1 of the pins has | GPIO_PULL_DOWN, then it will fail to initialize.

(edit, forgot mention)

[HookyKB]

@ReSummit Nice to know it works. Hopefully this can be a solution for duplex as well.

You might need to disable the console for it to work. I had a problem with console enabled as it was trying to set a pin high when something else was using it.

app/boards/shields/intro/intro.dtsi L14-15:

        /delete-property/ zephyr,console;
        /delete-property/ zephyr,shell-uart;

And set serial to disable at the bottom.

[petejohanson]

@ReSummit, which version of the Xiao are you using? It has been tested with the Xiao BLE. You can see the config here: 7a9a74e
Can you share your repo so I can check out the settings. Have you tried just setting the pins without the | GPIO_PULL_DOWN?
Also, I doubt you'll get this board to work properly without ghosting as the board is a duplex matrix, not charlieplex.

@HookyKB I am currently using the Seeeduino Xiao SAMD21. I tried it separately in a Arduino sketch to see if the GPIO input / output switching was a problem but it did fine.

Here is a fork of your repo with the changes I made. Checkout kscan/round_robin and refer to board/shield/011 contents to see the config I have for the board. It does work, though I will note what you mentioned about ghosting: https://github.com/ReSummit/zmk-charlieplex/tree/kscan/round_robin

I did try doing it without | GPIO_PULL_DOWN but this resulted in the microcontroller believing that I pressed all the keys at once (or at the very least, spamming random keyboard inputs to the computer). This is with all the pins without | GPIO_PULL_DOWN; if >1 of the pins has | GPIO_PULL_DOWN, then it will fail to initialize.

(edit, forgot mention)

This is probably because the SAM0 GPIO driver in Zephyr enforces some checks that the nrfx one doesn't:

• SAM0: https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/gpio/gpio_sam0.c#L72
• nrfx: https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/gpio/gpio_nrfx.c#L85

Your fix seems so-so, I wonder if perhaps we should only be specifying the active high/low in the DTS, and then inferring the pull flag to add whenever setting a pin as in input.

[ReSummit]

@ReSummit Nice to know it works. Hopefully this can be a solution for duplex as well.

You might need to disable the console for it to work. I had a problem with console enabled as it was trying to set a pin high when something else was using it.

app/boards/shields/intro/intro.dtsi L14-15:

        /delete-property/ zephyr,console;
        /delete-property/ zephyr,shell-uart;

And set serial to disable at the bottom.

@HookyKB Unfortunately this doesn't change anything. Just recompiled and no keys are detected being pressed. I actually needed those properties enabled to even get a USB serial debug and get the error messages.

This is probably because the SAM0 GPIO driver in Zephyr enforces some checks that the nrfx one doesn't:

• SAM0: https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/gpio/gpio_sam0.c#L72
• nrfx: https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/gpio/gpio_nrfx.c#L85

Your fix seems so-so, I wonder if perhaps we should only be specifying the active high/low in the DTS, and then inferring the pull flag to add whenever setting a pin as in input.

@petejohanson This was actually what I was looking for when I saw the error code that was returned from gpio_pin_configure_dt as I got an error code -ENOTSUP which happened to be -134. Especially in the gpio_sam0_config function, where I think this error value comes from, at least when doing a LOG_DBG:

Inferring the pull flag sounds like a good idea, but part of me wonders if there should be another parameter for that. Probably a later problem though...?

[petejohanson]

@ReSummit I just push an additional commit:

commit a0cc94e70bfb6662920b7f8730bb8f5048c40926 (HEAD -> pr/HookyKB/1694, HookyKB/kscan/round_robin)
Author: Peter Johanson <[email protected]>
Date:   Fri Nov 24 05:45:42 2023 +0000

    refactor(kscan): Infer pull flags, use LISTIFY.
    
    * To avoid issues with platforms that enforce no pull flags when
      pins are used as outputs, infer the pull flags in code instead of
      setting them in the DTS for charlieplex kscan driver.
    * Use `LISTIFY` macro instead of deprecated `UTIL_LISTIFY`.

Can you please test?

[ReSummit]

@ReSummit I just push an additional commit:

commit a0cc94e70bfb6662920b7f8730bb8f5048c40926 (HEAD -> pr/HookyKB/1694, HookyKB/kscan/round_robin)
Author: Peter Johanson <[email protected]>
Date:   Fri Nov 24 05:45:42 2023 +0000

    refactor(kscan): Infer pull flags, use LISTIFY.
    
    * To avoid issues with platforms that enforce no pull flags when
      pins are used as outputs, infer the pull flags in code instead of
      setting them in the DTS for charlieplex kscan driver.
    * Use `LISTIFY` macro instead of deprecated `UTIL_LISTIFY`.

Can you please test?

Testing at the moment but uh, it doesn't compile... This error shows up:

<zmk-path>/app/module/drivers/kscan/kscan_gpio_charlieplex.c:398:52: error: expected expression before ',' token
  398 |         LISTIFY(INST_LEN(n), KSCAN_GPIO_CFG_INIT, (, ), n)};                                       \
      |

I changed the GPIO flags to now include | GPIO_PULL_DOWN but the same error appears.

[petejohanson]

@ReSummit Seems to compile here fine, but I've identified a small fix that likely was the issue. Please try again?

[ReSummit]

That fixed the compile error. Uploaded the code but the error from before persists

[ReSummit]

Looking back through the code that was added and I think it has to do with the set_as_output function as I think the flags are still there (because in my code modifications, I filtered out all PULLUP / PULLDOWN flags that were causing problems).

[petejohanson]

Looking back through the code that was added and I think it has to do with the set_as_output function as I think the flags are still there (because in my code modifications, I filtered out all PULLUP / PULLDOWN flags that were causing problems).

It doesn't work after removing the pull flag from your DTS/overlay?

[ReSummit]

Looking back through the code that was added and I think it has to do with the set_as_output function as I think the flags are still there (because in my code modifications, I filtered out all PULLUP / PULLDOWN flags that were causing problems).

It doesn't work after removing the pull flag from your DTS/overlay?

Whoops, forgot to take out the flags, uploaded again and am now seeing a different logged output, but none of the keys work. The logs seem to show a debug log about sending events:

.
..
...
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,4 state on
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,5 state on
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,6 state on
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,7 state on
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,8 state on
[00:00:00.014,000] <wrn> zmk: Not found in transform: row: 0, col: 1, pressed: true
[00:00:00.014,000] <wrn> zmk: Not found in transform: row: 0, col: 2, pressed: true
[00:00:00.014,000] <wrn> zmk: Not found in transform: row: 0, col: 3, pressed: true
[00:00:00.014,000] <wrn> zmk: Not found in transform: row: 0, col: 4, pressed: true

[petejohanson]

Looking back through the code that was added and I think it has to do with the set_as_output function as I think the flags are still there (because in my code modifications, I filtered out all PULLUP / PULLDOWN flags that were causing problems).

It doesn't work after removing the pull flag from your DTS/overlay?

Whoops, forgot to take out the flags, uploaded again and am now seeing a different logged output, but none of the keys work. The logs seem to show a debug log about sending events:

.
..
...
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,4 state on
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,5 state on
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,6 state on
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,7 state on
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,8 state on
[00:00:00.014,000] <wrn> zmk: Not found in transform: row: 0, col: 1, pressed: true
[00:00:00.014,000] <wrn> zmk: Not found in transform: row: 0, col: 2, pressed: true
[00:00:00.014,000] <wrn> zmk: Not found in transform: row: 0, col: 3, pressed: true
[00:00:00.014,000] <wrn> zmk: Not found in transform: row: 0, col: 4, pressed: true

Looks like my bit checking logic was errant. Just pushed another fix for testing. Thanks!

[ReSummit]

Looking back through the code that was added and I think it has to do with the set_as_output function as I think the flags are still there (because in my code modifications, I filtered out all PULLUP / PULLDOWN flags that were causing problems).

It doesn't work after removing the pull flag from your DTS/overlay?

Whoops, forgot to take out the flags, uploaded again and am now seeing a different logged output, but none of the keys work. The logs seem to show a debug log about sending events:

.
..
...
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,4 state on
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,5 state on
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,6 state on
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,7 state on
[00:00:00.013,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 9,8 state on
[00:00:00.014,000] <wrn> zmk: Not found in transform: row: 0, col: 1, pressed: true
[00:00:00.014,000] <wrn> zmk: Not found in transform: row: 0, col: 2, pressed: true
[00:00:00.014,000] <wrn> zmk: Not found in transform: row: 0, col: 3, pressed: true
[00:00:00.014,000] <wrn> zmk: Not found in transform: row: 0, col: 4, pressed: true

Looks like my bit checking logic was errant. Just pushed another fix for testing. Thanks!

Just compiled and uploaded. The keys are working now! Just in case anything looks weird, I attached a keypress debug but it looks good!

[00:00:00.000,000] <dbg> zmk: zmk_usb_get_conn_state: state: 11
[00:00:00.000,000] <dbg> zmk: get_selected_transport: No endpoint transports are ready.
[00:00:00.000,000] <inf> zmk: Welcome to ZMK!

[00:00:00.197,000] <inf> usb_hid: Device reset detected
[00:00:00.198,000] <dbg> zmk: zmk_usb_get_conn_state: state: 1
[00:00:00.198,000] <dbg> zmk: zmk_usb_get_conn_state: state: 1
[00:00:00.198,000] <dbg> zmk: get_selected_transport: No endpoint transports are ready.                                  
[00:00:00.325,000] <inf> usb_hid: Device reset detected                                                                  
[00:00:00.325,000] <dbg> zmk: zmk_usb_get_conn_state: state: 1                                                           
[00:00:00.325,000] <dbg> zmk: zmk_usb_get_conn_state: state: 1                                                           
[00:00:00.325,000] <dbg> zmk: get_selected_transport: No endpoint transports are ready.                                  
[00:00:00.431,000] <inf> usb_hid: Device configured
[00:00:00.431,000] <dbg> zmk: zmk_usb_get_conn_state: state: 3
[00:00:00.431,000] <dbg> zmk: zmk_usb_get_conn_state: state: 3
[00:00:00.431,000] <dbg> zmk: get_selected_transport: Only USB is ready.
[00:00:06.213,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 6,0 state on
[00:00:06.214,000] <dbg> zmk: zmk_kscan_process_msgq: Row: 6, col: 0, position: 6, pressed: true
[00:00:06.214,000] <dbg> zmk: position_state_changed_listener: 6 bubble (no undecided hold_tap active)
[00:00:06.214,000] <dbg> zmk: zmk_keymap_apply_position_state: layer: 0 position: 6, binding name: KEY_PRESS
[00:00:06.214,000] <dbg> zmk: on_keymap_binding_pressed: position 6 keycode 0x7001C
[00:00:06.214,000] <dbg> zmk: hid_listener_keycode_pressed: usage_page 0x07 keycode 0x1C implicit_mods 0x00 explicit_mods 0x00
[00:00:06.214,000] <dbg> zmk: zmk_hid_implicit_modifiers_press: Modifiers set to 0x00
[00:00:06.214,000] <dbg> zmk: zmk_endpoints_send_report: usage page 0x07
[00:00:06.264,000] <dbg> zmk: kscan_charlieplex_read: Sending event at 6,0 state off
[00:00:06.264,000] <dbg> zmk: zmk_kscan_process_msgq: Row: 6, col: 0, position: 6, pressed: false
[00:00:06.264,000] <dbg> zmk: position_state_changed_listener: 6 bubble (no undecided hold_tap active)
[00:00:06.264,000] <dbg> zmk: zmk_keymap_apply_position_state: layer: 0 position: 6, binding name: KEY_PRESS
[00:00:06.264,000] <dbg> zmk: on_keymap_binding_released: position 6 keycode 0x7001C
[00:00:06.264,000] <dbg> zmk: hid_listener_keycode_released: usage_page 0x07 keycode 0x1C implicit_mods 0x00 explicit_mods 0x00
[00:00:06.264,000] <dbg> zmk: zmk_hid_implicit_modifiers_release: Modifiers set to 0x00
[00:00:06.264,000] <dbg> zmk: zmk_endpoints_send_report: usage page 0x07

[petejohanson]

Rebased this and removed the deprecated label property. LGTM, will merge once the tests pass. Thanks @HookyKB !