qmk_firmware icon indicating copy to clipboard operation
qmk_firmware copied to clipboard

[Feature Request] Allow forcing individual community layouts in `qmk.json` for `userspace-compile` and GitHub Actions

Open bcat opened this issue 1 year ago • 6 comments

Feature Request Type

  • [ ] Core functionality
  • [ ] Add-on hardware support (eg. audio, RGB, OLED screen, etc.)
  • [x] Alteration (enhancement/optimization) of existing feature(s)
  • [ ] New behavior

Description

See QMK Discord discussion for additional context.

The userspace build infra used by qmk userspace-compile and the default GitHub Actions setup seems to treat each keyboard to compile as a pair of keyboard name and keymap name. This leads to problems when one has multiple keyboards built with the same PCB but different community layouts.

For example, I have a dz60 PCB with the 60_ansi_split_bs_rshift layout and one with the 60_tsangan_hhkb. My current handwritten build wrapper script compiles both by passing the FORCE_LAYOUT environment variable to make. I'd like to start using qmk userspace-compile instead, but qmk userspace-add only lets me set up a qmk.json file like this:

{
    "userspace_version": "1.0",
    "build_targets": [
        ["dz60", "bcat"]
    ]
}

There doesn't seem to be a way to build the dz60 with a particular community layout, nor to build it twice with different community layouts. I would suggest an amended qmk.json format like this instead that allowed specifying a particular FORCE_LAYOUT value for each firmware file to build:

{
    "userspace_version": "1.0",
    "build_targets": [
        {"kb": "dz60", "km": "bcat", "layout": "60_ansi_split_bs_rshift"},
        {"kb": "dz60", "km": "bcat", "layout": "60_tsangan_hhkb"}
    ]
}

bcat avatar Jan 07 '24 04:01 bcat

Note that I think this is distinct from https://github.com/qmk/qmk_firmware/issues/22815. That issue seems to be about the existing ability to pass force layout to qmk compile not working, but even if that bug was fixed, it still wouldn't address this FR. (Passing -e FORCE_LAYOUT=foo to qmk userspace-compile would try to force the same community layout for every keyboard, which is probably not what anybody wants.)

bcat avatar Jan 07 '24 04:01 bcat

I think it'd be more likely we'd end up with per-keyboard environment variables added when invoking qmk userspace-add, with the usual -e FORCE_LAYOUT=xxx at that point -- resulting in something like this for the qmk.json file:

{
    "userspace_version": "1.1",
    "build_targets": [
        [
            "dz60",
            "bcat",
            [
                ["FORCE_LAYOUT", "60_ansi_split_bs_rshift"]  /* override layout */
            ]
        ],
        [
            "dz60",
            "bcat",
            [
                ["FORCE_LAYOUT", "60_tsangan_hhkb"]  /* override layout */
            ]
        ],
        ["40percentclub/luddite", "default"], /* no env vars, as per v1.0 */
        [
            "40percentclub/luddite",
            "default",
            [   /* multiple env vars added during build */
                ["FORCE_LAYOUT", "60_ansi"],
                ["CONVERT_TO", "proton_c"],
                ["BOOTLOADER", "tinyuf2"]
            ]
        ]
    ]
}

tzarc avatar Jan 08 '24 22:01 tzarc

Ah, yeah, that would address my use case, and it's more flexible too.

bcat avatar Jan 09 '24 03:01 bcat

It'd be great if you could give #22887 a go to see if it addresses your issue.

tzarc avatar Jan 11 '24 10:01 tzarc

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs. For maintainers: Please label with bug, in progress, on hold, discussion or to do to prevent the issue from being re-flagged.

github-actions[bot] avatar Apr 11 '24 01:04 github-actions[bot]

Not stale, and has a fix in flight. Thanks @tzarc! :)

bcat avatar Apr 27 '24 17:04 bcat

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs. For maintainers: Please label with bug, in progress, on hold, discussion or to do to prevent the issue from being re-flagged.

github-actions[bot] avatar Jul 27 '24 01:07 github-actions[bot]

Still not stale, and I can confirm PR #22887 seems to work (though I haven't pulled in changes from HEAD for a little while).

bcat avatar Jul 27 '24 05:07 bcat