Support Irregular Matrices

[MirusTech]

Describe the Bug

I have merged your branches manna-harbour/miryoku and manna-harbour/bilateral-combinations in my copy of qmk_firmware/master and enabled the BILATERAL_COMBINATIONS feature in config.h. I have noticed that keyboard combinations Ctrl-Backspace and Ctrl-Delete (e.g., delete previous and next work in Notepad) do not work anymore. Without the merge of manna-harbour/bilateral-combinations (but with manna-harbour/miryoku), both work as expected.

System Information

• Keyboard: Planck
  • Revision (if applicable): 6
• Operating system: Windows 10
• AVR GCC version: 5.4.0

• ARM GCC version: 9.2.1

• QMK Firmware version: 0.11.56 (with manna-harbour/miryoku and manna-harbour/bilateral-combinations merged in)

• Any keyboard related software installed?
  • [ ] AutoHotKey
  • [ ] Karabiner
  • [ ] Other:

Additional Context

You can find the branch here: https://github.com/Shredder/qmk_firmware/tree/dev_jz_bilateral_combinations.

[manna-harbour]

Thanks for your report. I'll try building from your fork when I have a chance. It's possible something has changed upstream. If you'd like to do more testing, please try with the miryoku branch plus bilateral combinations only (i.e. without merging into qmk master) to see if that's the issue.

[manna-harbour]

Can you confirm that this is with left hand mod-tap control? Also, can you specify you build command line?

[MirusTech]

Yes, I do have Ctrl mod-taps (in fact, also the other modifiers like in default Miryoku) on both sides (i.e., my left and right hands). Let me get back to you on the build command line.

[urob]

I have the same problem, and in my case I could trace it down to the right half of the bottom row being treated as if it were on the left half. To give a bit more details, consider the following layout: image

Here, HOME_S is an alias to LCTL_T(KC_S). I have permissive_hold enabled, so nested taps of the opposite hand should immediately trigger the mod. For the KC_BSPC in the middle column, this works fine. However, for the one in the bottom row, the mod does not activate (when held less than the bilateral_combinations timeout.) If on the other hand, I nest the same bottom row KC_BSPC with the right hand control (HOME_E), everything works fine.

So (for now this just being a guess) I suspect that this means that the problem is caused by how the key matrix for the Planck is defined. Maybe it's more of a problem with the Planck than with bilateral combinations?

Shredder, can you confirm this behavior? That is, does Ctrl-backspace and Ctrl-delete work in your case with the right hand mod as well?

[manna-harbour]

Planck has an irregular matrix on the bottom row, see https://github.com/qmk/qmk_firmware/blob/2538d341d828d04392898a9d3ce691bcd4a79e1f/keyboards/planck/rev6/rev6.h#L99

You would have to use #28 and supply your own custom bilateral_combinations_left() function.

For the future implementation (see #29) it would be nice to either use the keymap's rows / cols rather than those from the matrix (then something like BILATERAL_COMBINATIONS_COLS from #28 would be sufficient), or to specify the hand for each key with an array using the keymap layout.

[urob]

Here's a quick workaround, which disables BL for Planck's bottom row using the custom bilateral combinations commit by theol0403 (605ecc6c3b4141a930544ca4a35488d1497df967)

https://github.com/urob/qmk_firmware/commit/3ef760596669f7323abb2acbd5ec7661794eb357#diff-6b5eecce5cfac61a489d60ddc296d779ee957cb48b677973f0e5ce069fab49fc

obviously this is just a quick fix for fellow planck users and no replacement for the more general solutions outlined by manna-harbour

edit: I got the exception for the bottom row wrong on my first commit. This is how it should look like: 2bc16f8476f392b719c3207cceee1092f85ada52

[manna-harbour]

The general solution for an irregular matrix is probably to add a new layer to your keymap and populate with KC_L and KC_R as appropriate for each key position (and something else for keys that can be used with either hand, if adding that feature too), then use keymap_key_to_keycode() on that layer to lookup handedness for the key in question. That way the keyboard's own LAYOUT macro can be used to handle the matrix irregularities.

[ZenghaoWang]

The general solution for an irregular matrix is probably to add a new layer to your keymap and populate with KC_L and KC_R as appropriate for each key position (and something else for keys that can be used with either hand, if adding that feature too), then use keymap_key_to_keycode() on that layer to lookup handedness for the key in question. That way the keyboard's own LAYOUT macro can be used to handle the matrix irregularities.

For anyone else using a planck, this is how I implemented the fix:

1. Merge #28
2. Add KC_LH and KC_RH to my keycodes enum
3. Add another layer to my keymap:

    [LAYER_HANDS] = LAYOUT_ortho_4x12(
        KC_LH, KC_LH, KC_LH, KC_LH, KC_LH, KC_LH, KC_RH, KC_RH, KC_RH, KC_RH, KC_RH, KC_RH,
        KC_LH, KC_LH, KC_LH, KC_LH, KC_LH, KC_LH, KC_RH, KC_RH, KC_RH, KC_RH, KC_RH, KC_RH,
        KC_LH, KC_LH, KC_LH, KC_LH, KC_LH, KC_LH, KC_RH, KC_RH, KC_RH, KC_RH, KC_RH, KC_RH,
        KC_LH, KC_LH, KC_LH, KC_LH, KC_LH, KC_LH, KC_RH, KC_RH, KC_RH, KC_RH, KC_RH, KC_RH
    )};

4. override bilateral_combinations_left in my keymap.c

bool bilateral_combinations_left(keypos_t key) {
    uint16_t keycode = keymap_key_to_keycode(LAYER_HANDS, key);
    return keycode == KC_LH;
}

[manna-harbour]

@ZenghaoWang Thanks for trying it! This function might as well be built-in, so here it is in a new branch:

• branch: https://github.com/manna-harbour/qmk_firmware/tree/bilateral-combinations-hands-test
• documentation:https://github.com/manna-harbour/qmk_firmware/blob/bilateral-combinations-hands-test/docs/tap_hold.md#bilateral-combinations

Please try this branch. If it's ok I'll merge it into the main BC branch (well, force push, as this branch is rebased on current master).

[ZenghaoWang]

Works fine on my end! I merged your branch, deleted my bilateral_combinations_left function, and defined BILATERAL_COMBINATIONS_HANDS in my config.h.