feat(boards): Generic 40%, 50%, 60%, 65%, 75% and tkl physical layouts

[ReFil]

This PR adds physical layout definitions for all the common physical layouts for 40% ortho through tkl unibody keyboards as well as implementing them on the relevant boards.

All boards have been confirmed to build, but I don't have many of these boards to test on personally. I can test on all the boards I do have access to

[lesshonor]

If you have strong sentiments about the naming of the matrix-transform attributes, I think it would be a good idea to add docs with naming guidelines.

[lesshonor]

Suggested change

	matrix_transform_60_ansi: matrix_transform_60_ansi {
	matrix_transform_60_ansi: ansi_transform: matrix_transform_60_ansi {

...Defaulting to physical-layout will already break all builds with keymaps using matrix-transform, which is rough. Renaming all the transforms on top of that seems...excessive, since it isn't even necessary for physical-layout support. At least retain the existing label for some period of time, IMO.

This comment applies everywhere.

[ReFil]

The boards in this pr that changed the matrix transform in the upstream keymap were the polarity works boards, which used funky preprocessor stuff to support multiple layouts, and the preonic. The Planck also provided multiple transforms for user selection.

Polarity works have primarily been directing customers to their own editor and their hosted config repo redefines the whole board. Furthermore, I don't think the Preonic and Planck are especially popular boards for zmk. So the number of affected users would be limited, especially given only Planck users who change the default transform would be affected.

The existing label could be retained using DT aliases I believe, I'll add that. I agree that this is a bit of a blunt measure, but it only has to be done once.

[lesshonor]

The boards in this pr that changed the matrix transform in the upstream keymap were the polarity works boards, which used funky preprocessor stuff to support multiple layouts

Fair enough for the people who are pointed at the PW repo, but the funky preprocessor stuff exacerbates the problem as it means pretty much all BTXX keymaps have a chosen node with a matrix-transform instead of just the nerds who got curious enough to examine the board definition.

No hard feelings if you still disagree and would rather just make the breaking change with no backwards compatibility, though. My comments aren't law, and there's already precedent; all 5-column Corne keymap builds were likely broken by the recent switch to physical-layout, and that has a much greater impact.

[ReFil]

Im not keen on a breaking change, but I don't see any possible way to implement physical-layout without it being breaking currently on the btxx boards.

That being said I have an idea that could resolve the issues. I can tweak the physical layout code to pick up the chosen transform if one is present, it's not an ideal solution as it won't handle multiple layouts but it will still work. Add in DT aliases to preserve old labels and then this pr won't be breaking

[joelspadin]

I think in some of the boards Pete updated already, he did something more like

#include <layouts/whatever.dtsi>

&layout_whatever {
    transform = &my_keyboard_transform;
};

which would not require renaming the transforms. I also like it being more explicit rather than the layout defining a name for a transform which it then expects every keyboard using it to use.

[petejohanson]

The boards in this pr that changed the matrix transform in the upstream keymap were the polarity works boards, which used funky preprocessor stuff to support multiple layouts

Fair enough for the people who are pointed at the PW repo, but the funky preprocessor stuff exacerbates the problem as it means pretty much all BTXX keymaps have a chosen node with a matrix-transform instead of just the nerds who got curious enough to examine the board definition.

No hard feelings if you still disagree and would rather just make the breaking change with no backwards compatibility, though. My comments aren't law, and there's already precedent; all 5-column Corne keymap builds were likely broken by the recent switch to physical-layout, and that has a much greater impact.

Yeah... I'm considering if there's any way to avoid this breakage, perhaps just warning when both are defined and falling back to the "use the assigned transform and ignore the layout" as a reasonable fix for this.

[ReFil]

the use of &kscan0 has been ended up being a de facto standard across a lot of boards. standardising on the matrix transform naming scheme as matrix_transform_<size>_<quirks/specifics> in documentation is a good idea, I'll add that

[lesshonor]

the use of &kscan0 has been ended up being a de facto standard across a lot of boards. standardising on the matrix transform naming scheme as matrix_transform_<size>_<quirks/specifics> in documentation is a good idea, I'll add that

I think it's a great idea to establish naming guidelines. My main concern is that board/shield creators will see the layouts here (with predefined &kscans and &matrix-transforms), say "That isn't close enough to what I need", and roll their own...which kind of defeats the purpose.

Manipulating the devicetree via labels/phandles/etc is an obscure tactic this side of Zephyr; even before app/dts/layouts/foostan/corne.dtsi was merged, parts of it were being extracted "because I wasn't going to need the 5 column transform". (if you're reading this: no shade intended.)

I'm sure "I didn't need X/Y doesn't match my build so I just cut-pasted Z in" will be a constant refrain regardless of what approach is taken here. (Especially if the first ZMK Studio video tutorial does so.) My five cents is just a wager that if the common layouts are defined loosely enough, it will happen less often.

[ReFil]

Unless I'm mistaken the kscan and matrix transform can be overridden in downstream boards by calling the layout node and overriding it, that was the impression under which I did this, if that's not the case then I agree it should be dropped from the layout and configured on a per board basis upstream. Perhaps this is also something that can be addressed through documentation. I'll do some testing quickly and get back on this point

[joelspadin]

I think in some of the boards Pete updated already, he did something more like

#include <layouts/whatever.dtsi>

&layout_whatever {
    transform = &my_keyboard_transform;
};

which would not require renaming the transforms. I also like it being more explicit rather than the layout defining a name for a transform which it then expects every keyboard using it to use.

[joelspadin]

I think defining multiple layouts like this would cause all of the layouts in the included file to be compiled, even if only one of them is being used? IIRC Studio allows the user to select from all compiled layouts, in which case this would allow selecting layouts a keyboard might not support.

/omit-if-no-ref/ could be used to avoid that, but then something would need to reference each layout that is supported somehow.

Would separating each layout into its own file be better? Alternatively, I think you could set status = "disabled" on every layout and make boards/shields explicitly set status = "okay" to enable the ones that are supported, but that's a bit awkward.

[ReFil]

I deal with this by using /delete-node/ for unsupported layouts. I'm not so sure about it though, maybe one file per layout is better

[petejohanson]

Have pondered this for the day. There's a few things that complicate this:

Pros of "one file with related layouts":

• Position maps are really useful, especially if you have something like an ANSI with a split backspace, where if you lake a position map tweak, changing layouts will cause your "backspace" to disappear.
• For many designs, this would likely mean just one file to include.

Cons to "related layouts in one file":

• Risk of C&P because folks don't have good docs/references on how to only use the layouts they want.
• More "busy work" to enable/disable the layouts you want to use.
• Potentially huge shared layout files for some formats, e.g. 60%.
• Accidentally having variants enabled you don't need.

I think the benefits of users not having to figure out the position map bits would outweigh the other issues, if we can improve on the cons.

Perhaps:

• All layouts in the .dtsi files are status = "disabled" by default.
• Users will need to supply their own transform property for each layout the want to use anyways so the can also set status = "okay" at the same time. This matches the Zephyr convention for devices in SoCs, etc too.
• The physical position map code already handles ignoring mapping entries for layouts that are disabled. This means we can have a "complete" map for all possible variants that will only end up built with the pieces needed.
• We don't set a kscan or transform on any layouts in the layout .dtsi, and allow consumers to set them as appropriate.

Thoughts?

[ReFil]

Having rethought it and fiddled with testing things I'm tempted to suggest individual files per layout right now, we can always add a top level file per size (such as how behaviors.dtsi does it) down the line that incorporates the position mapping. Unless I'm mistaken if layouts are included but not provided a transform there's a compilation error, does that go away if they're disabled? I'm keen to make it as easy as possible for people downstream who just want an ansi 60 or hand wired 4x12.

[joelspadin]

Maybe not worth including until something needs them, but there are several TKL variants. Specifically, every combination of

• ANSI or ISO
• winkey (most common) or winkeyless
• 87 (most common) or 88 key (adds F13)

We may at least want to decide on how all of those variants should be named up front.

[ReFil]

For all the other layouts i've named them as layout_<size>_<quirks/specifics>, I think that can be extended to this too e.g. layout_tkl_f13ansi

[ReFil]

Just given this a big shake up, removing transform names from the layouts, moving each layout to it's own file so no need for /delete-node/ schenanigans and going back to chosen kscan nodes

[petejohanson]

Just given this a big shake up, removing transform names from the layouts, moving each layout to it's own file so no need for /delete-node/ schenanigans and going back to chosen kscan nodes

Given the fix merged in #2490 is in, I think we should be moving to:

• Have split files like you have here.
• Also offer a "one stop shop" .dtsi that pulls in the unique files, and adds a position map. I'm on the fence on the status we should set here, but I suspect we should disable them all, and allow importers to enable just the layouts they use. That will allow us to add more variants to the aggregate .dtsi later and not break things.

Thoughts?

[ReFil]

I was thinking about the position map earlier. I'm not sure if it should be done in a generic basis because the layouts each board supports are going to be different, and if a layout is omitted but the position map is tried to be used the build will fail

[petejohanson]

I was thinking about the position map earlier. I'm not sure if it should be done in a generic basis because the layouts each board supports are going to be different, and if a layout is omitted but the position map is tried to be used the build will fail

Can you describe a bit more the build failure you're talking about?

[ReFil]

Would the build not fail if the position map references a disabled or absent layout? It's on my list to add position mapping to the ckp and 40&50% boards anyway so I'll be able to do testing but my understanding of device tree was that it would fail in processing that

[petejohanson]

Would the build not fail if the position map references a disabled or absent layout? It's on my list to add position mapping to the ckp and 40&50% boards anyway so I'll be able to do testing but my understanding of device tree was that it would fail in processing that

The code is set up so the position map can reference a disabled layout, and that layout will be skipped. It should compile and work fine. I've tested with posix minivan setup recently.

[ReFil]

Ah ok awesome, I'll get to work on adding that later then