I’d like to discuss the possibility of adding two small libraries I have written to Kaleidoscope’s core. These were originally for my LED control plugin, FunctionalColor.
What these do is fairly simple:
colors.h Provides a list of human-friendly English color names, using the CSS color names. Additionally this offers a dim() function to change the brightness from 100% (255) to 0.
keygroups.h Provides functions to determine which “group” of keys, a key is in, or if it fits into a given category.
Both of these plugins are very useful/essential for the way FunctionalColors works but I think they would be really useful for anyone dabbling in customizing their keyboard layout, even if just to be able to have access to easy color names and a simple way to change the brightness of keys.
keygroups.h would be useful for more advanced uses but FunctionalColors uses it to easily assign colors to groups of keys, saving a user from assigning each and every key individually.
They have been written (with lots of help from @noseglasses) to be extremely efficient and I believe this functionality uses no memory in firmware.
Would I have support from the maintainers of kaleidoscope for integrating this functionality into the core of kaleidoscope for the use of users and plugin developers? If so, what changes (if any) would be required? (Likely there are some namespace considerations, and perhaps some contention about which keygroups should be included).
I haven’t looked at keygroups yet, but the color names have my support. We’d need to find a good namespace for them (how about kaleidoscope::led::color?), and I’d keep the nocolor “color” specific to FunctionalColor, and not include them in the in-core header.
The last part (use no memory in firmware) is not entirely true as the compiler must generate code for any constexpr functions that are called with a non-constexpr arguments. Thus, it depends on the use-case if code is generated and program memory is used.
Moreover, I experienced recently that the compiler for strange reaons in some cases opts against evaluating constexpr function calls at compile time. Instead of treating the result as a constant, it sometimes instanciates the constexpr functions and actually calls them at runtime. Only if it is clear that such a function is called only a single time, it can be a viable solution to add __attribute__((always_inline)) to the inline function definition.
In general it makes sense to check the assembly code to see what the compiler actually does.
That said, I would love to see both, colors.h and keygroups.h in the firmware core.
It is, but it isn’t always included. If someone doesn’t have mousekeys, but wants the keygroups, then ideally, keygroups shouldn’t pull in mousekeys as a dependency.
I’m not sure what you mean by doesn’t have mousekeys. But by solely including the header, the firmware size is not supposed to change, no? That means it’s a dependency that wouldn’t hurt. Otherwise, the mousekeys related checks in keygroups.h could be moved to one of the headers of plugin MouseKeys.
I was thinking of this… that would require a little more code changes to core.
Could we also do something like an ifdef and only add the mousekey groups if the mousekeys.h is included? This would complicate aspects of FunctionalColors, however, as I’d have to check all the time whether I should add that part for each of the structs for the themes. (Granted, currently FunctionalColors already requires mousekeys so that doesn’t have to change).
I seem to remember that including mousekeys would pull in the mouse HID descriptors and whatnot, even when unused. That may not be the case anymore, I’d need to check. If merely including MouseKeys doesn’t change the firmware size, then the dependency is no problem as far as I’m concerned.
Due to linker garbage collection only symbols referenced directly or indirectly from the sketch can affect firmware size. I can’t see this happen here but did not check either.