kmk_firmware
kmk_firmware copied to clipboard
[status_indicator] add hardware agnostic extension for layer indication
After a few hacky try I got a versatile led layer/capslock indicator for myself. As I've seen a few time asked on the discord how to do such layer indicator, I cleaned it up so that it is hardware independent.
I am myself using it now (the example with a twist for the each side of my split keyboard).
Wouldn't it make sense to show users how to connect the indicator with one of the existing led extensions, instead of introducing a fourth one?
So. I looked at it for a while and think something similar happend when I tried to get my indicator into the wild.
The actual Indicator
extension has barely any usefull code in it, arguably not enough to warrant a seperate module. All the interesting stuff is happening in the doc: More or less a "how to build your own indicator". This personalized code doesn't make sense in a one-size-fits-all module, resulting in a very sparse Indicator
extension, coupled with a somewhat complicated child class in the doc.
What do you think about making this a pure HowTo with two examples, maybe one inherited from Status_indicator
hooking into the PWM LED extension, and one a custom animation for addressable RGB. (I could contribute the second example.)
@xs5871 To answer your first comment:
No one of the current module can do what I want, I don't know much about the other people on discord but they ask about custom implementation right away, letting me think that they maybe already considered the existing module before. Some very common hardware/feature should definitely have their own module but they cannot realistically take into account all the hardware/custom features people will want, hence the need for something basic. The fact that this extension is something for everyone to build upon makes it rather different for the others that makes assumption and hard-code choices. Calling it just 'a fourth one' is misleading.
My personal example that wont realistically fit the existing extension (afaik): I have a split with neopixels in each side. I want them to be one color for capslock, one for every layer !=0. If both condition are met at once, one side will have the capslock color, the other the layer color. Also, in the end, I don't want to use the NeoPixel class, but a custom subclass HSVPixel that can take an int (hue) as assignment in addition to the rgb tuple (and using the corrected rainbow from fastled instead of the plain classic spectrum like what is used in the hsv_to_rgb()
of the rgb extension...)
second comment:
After thinking about it, I think that what drives me to do this is actually a missing common base for the different indicator feature. I don't see the problem in having not much code if that small part is meant to be reused/copied/reinvented (sometimes more poorly) numerous times...
Imho:
- it is useful to provide a clean base for people wanting to do very custom thing.
- the existing modules could even inherit from it, so they will be more standardized.
So, I agree that this code is a bit TOO generic.
The real problem here is due to a few things...
- The statusLED extension is WAY too generically named
- The statusLED extension is WAY too opinionated
- We can't really do much about either without potentially breaking a lot of people's code.
Can we deprecate the current statusLED extension and create a v2? If so, what would v2 look like in terms of instantiating? Do we just create an extension called "statusLEDv2"? Do we some how make an optional parameter as part of the constructor that triggers v2?
Another alternative would be to leave statusLED alone and add the ability to assign callback functions to lock_status for each status type... The more that I think about it, the more that makes sense honestly. That and some better documentation as pointed out above.
This discussion is addressed and resolved in #553.