Conversation
innovaker
added
enhancement
Nicell
left a comment
Nicell
left a comment
There was a problem hiding this comment.
Thanks for making this shield! I have a few comments, and then there's a couple files that still need to be added/updated:
.github/workflows/build.yml: Add your board to the shield list.
gherkin.conf: This file should be added even if it's blank, so that it's copied into user-configs properly
docs/docs/hardware.md: You should add the Gherkin to this list
docs/static/setup.ps1 and docs/static/setup.sh: Gherkin should be added as a non-split option for both files.
| default_transform: keymap_transform_0 { | ||
| compatible = "zmk,matrix-transform"; | ||
| columns = <6>; | ||
| rows = <5>; | ||
| map = < | ||
| RC(0,0) RC(0,1) RC(0,2) RC(0,3) RC(0,4) RC(0,5) | ||
| RC(1,0) RC(1,1) RC(1,2) RC(1,3) RC(1,4) RC(1,5) | ||
| RC(2,0) RC(2,1) RC(2,2) RC(2,3) RC(2,4) RC(2,5) | ||
| RC(3,0) RC(3,1) RC(3,2) RC(3,3) RC(3,4) RC(3,5) | ||
| RC(4,0) RC(4,1) RC(4,2) RC(4,3) RC(4,4) RC(4,5) | ||
|
|
||
| >; | ||
| }; |
There was a problem hiding this comment.
I don't believe this is necessary as this keyboard is a perfect grid. Similar to it missing in the planck seen here: https://github.com/zmkfirmware/zmk/blob/main/app/boards/arm/planck/planck_rev6.dts#L20
It doesn't hurt to include this I suppose, but it's also superfluous.
There was a problem hiding this comment.
I don't actually own a gherkin to test on, so i'm too afraid to delete this because gherkin does have a git of a funny matrix, but if you're confident i can remove it.
There was a problem hiding this comment.
I've added references to the other files.
| if SHIELD_GHERKIN | ||
|
|
||
| config ZMK_KEYBOARD_NAME | ||
| default "GHERKIN" |
There was a problem hiding this comment.
Nitpick, is "GHERKIN" all caps the actual name? I've always seen it as "Gherkin".
There was a problem hiding this comment.
It's a fair cop, renamed to Gherkin
| #define DEFAULT 0 | ||
|
|
There was a problem hiding this comment.
| #define DEFAULT 0 |
Unused define.
| RC(0,0) RC(0,1) RC(0,2) RC(0,3) RC(0,4) RC(0,5) | ||
| RC(1,0) RC(1,1) RC(1,2) RC(1,3) RC(1,4) RC(1,5) | ||
| RC(2,0) RC(2,1) RC(2,2) RC(2,3) RC(2,4) RC(2,5) | ||
| RC(3,0) RC(3,1) RC(3,2) RC(3,3) RC(3,4) RC(3,5) | ||
| RC(4,0) RC(4,1) RC(4,2) RC(4,3) RC(4,4) RC(4,5) |
There was a problem hiding this comment.
If we do keep this transform, it should be in the shape of the Gherkin (3x10?) not 5x6.
|
Is this PR dead? |
|
Apologies, I haven't been able to test the transform changes requested as I don't have access to a gherkin. I know it works 'as is' but I'd be unable to test further changes. I'll keep it available in my own zmk-config. But I guess consider it closed. |
3 participants
Gherkin shield with default keymap.