Adds nullbitsco/nibble mapping with multiplex keymatrix support #991
Conversation
boards/nullbitsco/nibble/README.md
Outdated
| Edit the key mapping in `main.py` to match your keyboard layout. | ||
|
|
||
| The Keyboard constructor supports an optional `encoder` argument (see `kb.py`). | ||
| See the sample `main.py` for an example of how to configure encoder |
There was a problem hiding this comment.
| See the sample `main.py` for an example of how to configure encoder | |
| See the sample `main.py` for an example of how to configure encoder. |
| multiplexed=True, | ||
| ) | ||
|
|
||
| if encoder: |
There was a problem hiding this comment.
The encoder import belongs here, otherwise the encoder code is always loaded and wastes memory. Not an issue for this board, but a bad example to set.
| if encoder: | |
| if encoder: | |
| from kmk.modules.encoder import EncoderHandler |
There was a problem hiding this comment.
Actually, this entire code block should go in main.py, including the conditional import.
|
|
||
| XXXXX = KC.NO | ||
|
|
||
| keyboard.keymap = [ |
There was a problem hiding this comment.
Would you mind formatting this in the usual "looks like keymatrix" way? Prefix the keymap definition with # fmt: off and postfix with # fmt:on to selectively disable the linter.
boards/nullbitsco/nibble/main.py
Outdated
| def set_backlight(hsv): | ||
| rgb.hue, rgb.sat, rgb.val = hsv | ||
|
|
||
|
|
||
| set_backlight((180, 255, 50)) |
There was a problem hiding this comment.
Why is this here? You can set the default hsv in the RGB constructor.
There was a problem hiding this comment.
good point, hangover from some previous code
kmk/scanners/digitalio.py
Outdated
| @@ -35,21 +36,23 @@ def __init__( | |||
| # | |||
| # repr() hackery is because CircuitPython Pin objects are not hashable | |||
| unique_pins = {repr(c) for c in cols} | {repr(r) for r in rows} | |||
| print(unique_pins) | |||
There was a problem hiding this comment.
| print(unique_pins) |
| @@ -27,7 +27,7 @@ from kmk.extensions.rgb import RGB, AnimationModes | |||
| keyboard = KMKKeyboard(active_encoders=[0], landscape_layout=True) | |||
There was a problem hiding this comment.
This is not correct. There is no landscape_layout parameter.
There was a problem hiding this comment.
There is for the tidbit, but not for the nibble. See here: https://github.com/KMKfw/kmk_firmware/blob/main/boards/nullbitsco/tidbit/kb.py#L48
| offset=0, | ||
| multiplexed=False, # 2^k outputs are multiplexed on k output pins |
There was a problem hiding this comment.
Not a big fan of packing even more conditional functionality into the already slow digitalio scanner.
I also don't quite understand what's happening here. Is that something that could be replaced by
CircuitPythons native keypad_demux.DemuxKeyMatrix (currently beta)?
There was a problem hiding this comment.
I looked at keypad_demux docs, but it didn't seem to be available on my board yet. It also didn't seem quite as flexible in terms of configuring which direction is multiplexed, and pullup v pulldown etc.
Honestly I was a bit confused why digitialio.MatrixScanner exists at all when we have keypad.KeyMatrix natively but I am pretty new to circuitpython/kmk. I thought the modification was relatively small: in the regular case we scan column n by activating the n-th column pin, but in the mutiplexed case we scan column n by writing n as a bit pattern b0 b1 .. bk and set each multiplexed column bit.
If you prefer I could clone or subclass MatrixScanner to DemuxMatrixScanner instead?
There was a problem hiding this comment.
Getting keypad_demux build for your board shouldn't be too difficult. It is still in beta in any case.
The digitalio scanner predates the CP native matrix scanner, and there're hardware implementations that don't work with KeyMatrix -- at least for now -- that's the reason it's still around. If in doubt: always use the native scanner.
Thank you for the explanation, I understand what kind of mux we're talking about now.
Can you subclass without duplicating all of the code? I'm a bit short on time atm. and can't estimate that right now.
|
Thanks for the thorough review. Let me know what you think about digitalio.MatrixScanner and I can refactor as needed. |
This extends digitalio.MatrixScanner to support multiplexed output scanning which is used by nibble.