qmk_firmware icon indicating copy to clipboard operation
qmk_firmware copied to clipboard

Add multilayer VIA support to Hasu usb_usb

Open jshuf opened this issue 2 years ago • 4 comments

Description

Adds VIA support to the Hasu usb_usb converter. 'multilayer' comes from some additional complexity above and beyond most VIA ports - in order to fit more than one dynamic keymap layer into the atmega32u4 EEPROM, the converter's default scheme of using USB HID codes to directly index a 16x16 scan matrix is replaced with an intermediate mapping. This mapping translates from the smaller (142-code) set of USB HID codes actually used by the existing usb_usb layout macros, to cells in a smaller 9x16 scan matrix. This reduced matrix size allows three dynamic keymap layers to be stored in the atmega32u4 EEPROM. A more detailed explanation can be found in the comments in keymaps/via/keymap.c.

All code changes to support this are ifdef'd under VIA_ENABLE, and hence should have no impact on existing usage of the QMK usb_usb converter code. Because the LAYOUT macro for this VIA implementation has scan matrix dimensions that differ from the others, it lives in keymaps/via/keymap.c to avoid confusion with existing LAYOUT macros and info.json.

Types of Changes

  • [ ] Core
  • [ ] Bugfix
  • [x] New feature
  • [ ] Enhancement/optimization
  • [x] Keyboard (addition or update)
  • [x] Keymap/layout/userspace (addition or update)
  • [ ] Documentation

Issues Fixed or Closed by This PR

  • None

Checklist

  • [x] My code follows the code style of this project: C, Python
  • [x] I have read the PR Checklist document and have made the appropriate changes.
  • [ ] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [x] I have read the CONTRIBUTING document.
  • [ ] I have added tests to cover my changes.
  • [x] I have tested the changes and verified that they work and don't break anything (as well as I can manage).

jshuf avatar Feb 27 '22 00:02 jshuf

I'd like to convert this board to Custom Matrix Lite first - this change should still be doable after that, but hopefully with a little less boilerplate.

fauxpark avatar Mar 26 '22 08:03 fauxpark

This PR is very helpful to me for Reduce program size . http://www.neko.ne.jp/~freewing/hardware/qmk_usb_converter_max3421e_host_shield/

FREEWING-JP avatar Jun 10 '22 04:06 FREEWING-JP

Thank you for your contribution! This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready. For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

github-actions[bot] avatar Jul 26 '22 02:07 github-actions[bot]

Still active (at least as far as I'm concerned).

jshuf avatar Jul 29 '22 12:07 jshuf

Thank you for your contribution! This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready. For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

github-actions[bot] avatar Nov 09 '22 02:11 github-actions[bot]

Still active.

jshuf avatar Nov 09 '22 02:11 jshuf

Updated PR to reflect changes to master, and replaced VIA_ENABLE with COMPRESS_MATRIX, to better reflect that the changes to the keymap are not VIA-specific.

jshuf avatar Dec 09 '22 01:12 jshuf

The remaining CI failures are outside the scope of this PR - they're in userspace code, and are failing to compile in master.

jshuf avatar Dec 09 '22 11:12 jshuf

@jshuf In an attempt to free up as much EEPROM as possible, In my local code base, I added a keymaps/via_ansi folder which removed 34 keys in total ( F13-F24, International keys, legacy key codes ). So that makes 108 HID keycodes for a US ANSI layout, which could fit into 16x7 with 4 spare.
How would I go about removing 2 rows from hidindex? I suppose I don't fully understand the meaning of val = ((row) << 4) + (col)) and how that fits in with the various codes ie. 0x5D. Thanks for your PR. I'd like to see this get included in the code base.

bbecker-inlogik avatar Sep 20 '23 22:09 bbecker-inlogik

Merge conflicts fixed in attached custom_matrix.cpp file. custom_matrix.zip

bbecker-inlogik avatar Sep 20 '23 22:09 bbecker-inlogik

@bbecker-inlogik thanks for the code changes - I'll try to integrate and resubmit when I have time. There have been other QMK changes since the last review that I need to factor back into this code. In the meantime...

If foo is the HID code emitted by the keyboard and received by the converter, then hidindex[matrixindex[foo]] is the HID code that the converter will send to the host. That extra level of indirection is what allows us to reduce the size of each VIA layer in EEPROM. So, to make 16x7 work in your local code base, you'd want to do at least these things (there may be more, I haven't looked at this in a while, but this should get you started):

  • in via_ansi/config.h, set MATRIX_ROWS to 7
  • make hidindex an array of length 112, and make sure it includes all 108 of the HID codes you want, filling the remaining entries of the array with 0x00. The order in which you enumerate the 108 HID codes doesn't really matter here.
  • matrixindex remains an array of length 256, representing all 256 HID codes that could theoretically be received by the converter. Each value of this matrix is an index into the hidindex array. So, for each of those 256 HID codes, matrixindex will contain either XBAD (meaning that the HID code won't be supported) or it will contain the index 0x{R}{C}, where (R<<4) + C is the index into hidindex where you've stored the HID code you want the converter to send to the host.
  • in via_ansi/keymap.c, the LAYOUT_via macro and keymaps[][][] would need to be modified to correspond to your 16x7 array - this is what would initialize the layers in VIA.

jshuf avatar Sep 21 '23 01:09 jshuf

The matrix compression feature could be useful even without VIA (the current implementation uses 400 bytes for the data tables, but reduces each layer by 224 bytes, so it may be a win even starting from 2 layers, depending on the code size increase). So the implementation probably should not be coupled with VIA_ENABLE.

One major issue is that adding an option which changes the matrix layout makes all layouts which are currently defined in info.json invalid — all matrix locations in there must be adjusted to match the new virtual matrix. The solutions that I could think of are:

  1. Use the compressed matrix unconditionally; change all layout definitions in info.json accordingly. This makes the converter work mostly like “normal” keyboards (no weird extra options), but might break things for users of strange keyboards which could send keycodes outside of the normal set (however, I'm not sure whether such USB keyboards actually exist in the wild, so this might not be a problem in practice).
  2. Split the board into multiple revisions — one with the existing uncompressed matrix and layout definitions (and no VIA support), another with the compressed matrix and updated layout definitions. Unfortunately, this board already has 3 subrevisions, which would then need to be duplicated for the uncompressed/compressed revisions.

sigprof avatar Sep 21 '23 06:09 sigprof

In any case, changes like this are rather major, and need to go to the develop branch, not directly to master.

sigprof avatar Sep 21 '23 06:09 sigprof

Also you should think about converting the matrix to the “lite” custom matrix — even though it would have some overhead due to the debounce logic (the debounce itself could be disabled, but the matrix and raw_matrix arrays would still remain separate), the compressed matrix would take just 18 bytes of RAM, which would probably be an acceptable price for using more generic code (and might actually end up being faster than the custom matrix_get_row() implementation). And changing the code to convert the HID reports into bit arrays all at once would allow dropping the hidindex table (only matrixindex would be needed to determine the bit position for each pressed key).

sigprof avatar Sep 21 '23 07:09 sigprof