getreuer / qmk-keymap

My keymap & reusable QMK gems
Apache License 2.0
301 stars 45 forks source link

Achordion support for keyboards with irregular bottom row matrix (i.e. Planck) #10

Closed jweickm closed 2 years ago

jweickm commented 2 years ago

Hi @getreuer,

I found your Achordion feature and am been eager to try it out (have been using a rather hacky custom solution thus far). I saw that you determine the side of the keyboard by using:

static bool on_left_hand(keypos_t pos) {
#ifdef SPLIT_KEYBOARD
  return pos.row < MATRIX_ROWS / 2;
#else
  return (MATRIX_COLS > MATRIX_ROWS)
      ? pos.col < MATRIX_COLS / 2 : pos.row < MATRIX_ROWS / 2;
#endif
}

It has been reported that the Planck has an irregular matrix on the bottom row, and therefore I expect some issues when using your library as is. I have very little experience with C, so I wanted to ask, whether there was an easy fix to just ignore the bottom row from the achordion processing completely? Maybe akin to what was done here? Best, jweickm

rschenk commented 2 years ago

I have a Planck, and indeed it does have an irregular matrix layout on the bottom row of keys (and only the bottom row) that will confuse on_left_hand. The main utility of Achordion is on the alpha keys however... in fact it's kind of annoying on the bottom row. And the on_left_hand function works just fine out of the box for home row mods on the alpha keys.

I personally disabled Achordion entirely on the bottom row of keys. While it's true that the Planck has an irregular key matrix on that bottom row, those keys are all on row 3 and 7, but not in the exact column order you might expect. If you want to disable Achordion on the bottom row, you can use the code in @getreuer's blog post almost verbatim and it will work, just change the 4 to 3.

// Also allow same-hand holds when the other key is in the rows below the
// alphas. I need the `% (MATRIX_ROWS / 2)` because my keyboard is split.
if (other_record->event.key.row % (MATRIX_ROWS / 2) >= 4) { return true; }
                    // change from 4 to 3 for Planck --^

You can see it in my keymap

jweickm commented 2 years ago

Hi Ryan,

thank you for your kind and comprehensive reply. That sounds amazingly simple to implement. I will give it a shot. Also, thanks for sharing your keymap as well. It's always easier when copying from an actually working example.

rschenk commented 2 years ago

You're welcome. I guess I should have mentioned, I have a Planck rev6. No idea if the other revisions have a similar key matrix, but at least it's a staring point!

jweickm commented 2 years ago

Thanks, I have a Planck/rev6 as well, so your keymap should be a perfect example to try it out.

getreuer commented 2 years ago

Jakob, thank you for the good question. And, Ryan, thank you for the quick solution!

jweickm commented 2 years ago

@rschenk Hi Ryan, May I ask one more thing? When you are using a Planck, why did you use the (MATRIX_ROWS / 2) instead of just (MATRIX_ROWS)? From reading @getreuer's doc, I was under the impression that this only applies to split boards. Or is the Planck handled like a split board in the matrix processing?

getreuer commented 2 years ago

Jakob, I think you are right about Planck handling its matrix like a split board. When defining your layers, do you use the LAYOUT_planck_grid macro? I found that for the Planck rev 6, this macro is defined here

https://github.com/qmk/qmk_firmware/blob/master/keyboards/planck/rev6/rev6.h#L86-L91

The arg names suggest that physical button locations correspond to "(row, col)" coordinates like this:

(0,0)  (0,1)  (0,2)  (0,3)  (0,4)  (0,5)  (4,0)  (4,1)  (4,2)  (4,3)  (4,4)  (4,5) 
(1,0)  (1,1)  (1,2)  (1,3)  (1,4)  (1,5)  (5,0)  (5,1)  (5,2)  (5,3)  (5,4)  (5,5) 
(2,0)  (2,1)  (2,2)  (2,3)  (2,4)  (2,5)  (6,0)  (6,1)  (6,2)  (6,3)  (6,4)  (6,5) 
(3,0)  (3,1)  (3,2)  (7,3)  (7,4)  (7,5)  (7,0)  (7,1)  (7,2)  (3,3)  (3,4)  (3,5) 

Besides some anomalies in the bottom row, this is mapped like a split board.

jweickm commented 2 years ago

Pascal,

Thanks for your reply! I just checked, and yes! I am using LAYOUT_planck_grid when defining the layout. I wouldn't have known how to interpret the table that you sent, but I assume you are referring to the fact, that the matrix is defined as 8 "rows" and 6 "columns" so to speak. So if I just use the code that Ryan suggested, I should be fine. I actually have a checkerboards QUARK PCB incoming, so it might pay off to check how that matrix is define under "keyboards/..."!

Thanks again!

rschenk commented 2 years ago

Hey @jweickm, yes I did use % (MATRIX_ROWS / 2) because the Plank's bottom rows are both row 3 and 7. MATRIX_ROWS is defined as 8 on the Plank, and 3 % 4 == 7 % 4 == 3

Heres' the exact line in my keymap