zmkfirmware / zmk

ZMK Firmware Repository
https://zmk.dev/
MIT License
2.48k stars 2.55k forks source link

feat(underglow): support command for displaying board statuses #2353

Open nVitius opened 3 days ago

nVitius commented 3 days ago

The MoErgo ZMK fork has some cool functionality for displaying board status indicators on the Glove80 using underglow and an RGB command. Until recently, this had been dependent on two outstanding PRs: #999 and #1243. Both of which have been merged in at this point.

This PR ports the code from MoErgo's fork (reference).

I've refactored a bit of the code to be more legible as well as added comments here and there. The functionality between my version and MoErgo's is almost exactly the same with a couple exceptions:

Here is an example of how this is used in a dts: image And here is a short video of what that looks like on my Cyboard Imprint: https://github.com/zmkfirmware/zmk/assets/3804041/8a06fe6b-9e25-45ae-89d1-7b047fb541fc

I've never written C code before, so if you have any improvements or suggestions, I am happy to hear them.

caksoylar commented 3 days ago

I think this would at least need some way to extend to multiple splits. The way it is hardcoded with lhs and rhs doesn't seem flexible. Maybe make it N child nodes for splits with N parts.

You could also consider making this a module so that people can use and test it without having to switch ZMK branches.

nVitius commented 2 days ago

@caksoylar Thanks for your input!

I've updated the PR to support a dynamic number of peripherals. The new configuration looks like this: image

I'll look into making this a module, but I think it will require changes to rgb_underglow.c regardless, as we use the pixels global in there to actually display the indicators.

ReFil commented 2 days ago

@nVitius you can replace source files inside a module, see this from badjeff https://github.com/badjeff/zmk-split-peripheral-bonding-tweak/blob/0671c2148db4f695b6771bc834bf52b0946799de/CMakeLists.txt#L21-L25

nVitius commented 2 days ago

@ReFil Thanks for the link!

Is that really best practice? Seems really wrong to replace parts of the codebase from a module like that.