CapsUnlocked CU80 v2

[rys]

Support for the CapsUnlocked CU80 v2, a fixed layout hotswap TKL with two PCB variants.

Description

• Adds CU80 v2 support
• Two board variants (ISO and ANSI) each with a default and via keymap
• Tested on both variants of the production CU80 v2 PCB including the VIA maps

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Issues Fixed or Closed by This PR

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[sigprof]

Thanks for the effort to add yet another keyboard to QMK!

There are some minor inconsistencies with the PR checklist, which hopefully could be fixed easily.

[sigprof]

In addition to some minor inconsistencies with the PR checklist noted above, there are some deeper issues here.

1. Having the CU80 v1 board supported under the cu80 name, and the CU80 v2 board supported under the capsunlocked/cu80 name looks very confusing, especially when neither readme.md mentions that there are multiple incompatible versions of the PCB with apparently the same name. Without an obvious clue like v1 or v2 in the name, someone would inevitably flash a wrong firmware version into their PCB and would probably need to open the case to reach the reset button.

   Ideally these boards should have names like capsunlocked/cu80/v1 and capsunlocked/cu80/v2, and each readme.md should mention the group buy round when the corresponding PCB has been sold, and a way to distinguish one version from another (e.g., does the CU80 v2 PCB have an explicit “v2” marking somewhere?).

   However, just moving the v1 files to a new location would be a “breaking change”, which would go through the develop branch; I suppose that you want to avoid the delay associated with going through this process. I'm not sure whether adding a keyboards/cu80/rules.mk file with DEFAULT_FOLDER = capsunlocked/cu80/v1 would be sufficient to avoid that “breaking change” process for such movement — I don't see any examples of moving a keyboard into a “vendor” folder that way (but lots of such files specifying a rev1 or v1 subfolder). Maybe some other reviewer would explain the exact policy about such keyboard renames. If moving to a vendor folder without going through the breaking change process is not acceptable, you may need to add this keyboard as cu80/v2 (moving the existing cu80 code to cu80/v1, using DEFAULT_FOLDER = cu80/v1 for compatibility), and then queue another PR moving all versions of cu80 under capsunlocked during the next round of breaking changes.

2. AFAIK, physically different PCBs should be handled as different keyboard models, not different layouts for the same model. So you may need to split this keyboard into capsunlocked/cu80/v2/ansi and capsunlocked/cu80/v2/iso (or, if compatibility concerns from point 1 would prevent using the vendor folder just now, cu80/v2/ansi and cu80/v2/iso). These keyboard models may have mostly shared config.h, except for PRODUCT_ID and PRODUCT (they would also require separate VIA JSON files, but those files would no longer require the ISO/ANSI layout option).

3. RGBLIGHT_ENABLE = yes is not the proper way to add support for per-key RGB LEDs — the QMK way to support such LEDs is the RGB Matrix feature. Some examples of keyboards which use the WS2812 driver for RGB matrix: bm40hsrgb, bm60poker, bm60rgb, bm60rgb_iso.

   Adding RGB matrix support would require filling the led_config_t g_led_config structure with the appropriate data; this step would also require having separate keyboard models for ANSI and ISO PCBs (as noted in point 2), because these keyboards would have different number of per-key LEDs, and the coordinates of some LEDs also would be slightly different. (I suppose that the hs60/v1 board, which requires adding #define HS60_ANSI to config.h in the keymap to make RGB Matrix work properly on an ANSI PCB, is an example of how not to do it.)

[rys]

In terms of the deeper issues with where the CU80 v1 lives in the tree, and implementing support for the two PCB variants as completely separate boards, I'd like maintainer confirmation I need to before reworking everything.

If so, I think the following is a good idea:

• Split ISO and ANSI variants out into separate boards (capsunlocked/cu80/v2/iso and capsunlocked/cu80/v2/ansi)
• Move cu80 to capsunlocked/cu80/v1 and update its documentation

I'm just not sure whether to do the v1 move as a separate PR or whether I can include it in this one. Is an in-tree relocation with no functional changes classed as a breaking change?

At the very least I've added text into the readme to make it clear it's just a firmware for the v2 PCBs, to remove any initial confusion.

I don't think implementing RGB with RGB Matrix is necessary here since the more basic support works just fine.

[sigprof]

Is an in-tree relocation with no functional changes classed as a breaking change?

It might be, because people have personal keymaps under keyboards/cu80/keymaps/, and those keymaps must still compile and work after the keyboard change; setting DEFAULT_FOLDER should take care of this, but I'm not sure about a rename across the tree, as opposed to just moving to a subfolder like v1 (maybe there are some subtle problems with such renames, or just an unwritten policy that major renames are performed as breaking changes).

At the very least I've added text into the readme to make it clear it's just a firmware for the v2 PCBs, to remove any initial confusion.

Arent't PCBs from the round 2 of the group buy also v2 (so v1 was used only for round 1)?

I don't think implementing RGB with RGB Matrix is necessary here since the more basic support works just fine.

Then you need to convince some real maintainers that the current implementation is enough at least for now (so that the people who got boards from R2 can get a usable firmware version). A proper RGB Matrix support is required for effects which honor the physical location of LEDs, or even are based on the actually pressed keys. Although at the moment RGB Matrix has some problems with VIA compatibility.

[rys]

It might be, because people have personal keymaps under keyboards/cu80/keymaps/, and those keymaps must still compile and work after the keyboard change; setting DEFAULT_FOLDER should take care of this, but I'm not sure about a rename across the tree, as opposed to just moving to a subfolder like v1 (maybe there are some subtle problems with such renames, or just an unwritten policy that major renames are performed as breaking changes).

Let's see what the other maintainers recommend, happy to put in the work here to get it right.

Arent't PCBs from the round 2 of the group buy also v2 (so v1 was used only for round 1)?

Yep, I've fixed the URL for the PCB to be the round 2 buy URL, not r3 in 6175fa4. My mistake there.

Then you need to convince some real maintainers that the current implementation is enough at least for now (so that the people who got boards from R2 can get a usable firmware version). A proper RGB Matrix support is required for effects which honor the physical location of LEDs, or even are based on the actually pressed keys. Although at the moment RGB Matrix has some problems with VIA compatibility.

What do you mean usable firmware version? It currently works fine on the v2 PCBs I developed the firmware for and we don't need the more flexible features of the RGB matrix system, but maybe I'm missing what you're getting at.

[rys]

I asked on Discord if the different PCB variants should be reworked into two different board defs and the answer is yes (thanks @zvecr!). So I'll rework, and then move the cu80 v1 into the same bit of the tree as a separate PR.

[rys]

I've split the board into two discrete variants, v2_iso and v2_ansi, with default and via maps for each, and tested all four firmware variants across both PCBs.

Couple of minor changes versus the original board support: reverted to stock poll rate, and removed the LED brightness cap.

Should be good to review again! I've updated the VIA PR as well to split that into two boards.

[sigprof]

I noticed another small thing — because this board has a standard TKL ANSI or ISO layout, it should have LAYOUTS set in rules.mk, so that the community keymaps from layouts/community/tkl_ansi/ or layouts/community/tkl_iso/ could be used with this board.

[rys]

Committed both of those suggestions, thanks!