Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nVitius
Copy link

@nVitius nVitius commented Jul 1, 2024

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:

  • The properties for the underglow-indicators DT node are all optional now. This means you can pick and choose which indicators you would like to use
  • A couple of the properties are renamed to be a bit more clear (in my opinion)

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.

@nVitius nVitius requested a review from a team as a code owner July 1, 2024 23:41
@caksoylar caksoylar added enhancement New feature or request lighting labels Jul 2, 2024
@caksoylar
Copy link
Contributor

caksoylar commented Jul 2, 2024

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 nVitius force-pushed the feat/underglow-status-indicators branch from 903d24f to 8ec03d4 Compare July 2, 2024 19:42
@nVitius
Copy link
Author

nVitius commented Jul 2, 2024

@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
Copy link
Contributor

ReFil commented Jul 2, 2024

@nVitius
Copy link
Author

nVitius commented Jul 3, 2024

@ReFil Thanks for the link!

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

@nVitius nVitius force-pushed the feat/underglow-status-indicators branch from 8ec03d4 to 8047bf8 Compare December 26, 2024 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lighting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants