Wow, that sounds awesome - I’m jealous.
No apology necessary, you’ve been more than helpful. Have fun and don’t spend too much time on your phone! 
I take this to mean that the RGBLookup function now returns a cRGB value instead of merely a reference to one. Correct?
With your proposed changes I now get the following error on compile:
Model01-Firmware_noseglasses:380: error: invalid conversion from 'cRGB& (*)(const Key&) {aka cRGB& (*)(const Key_&)}' to 'kaleidoscope::LEDFunctionalColorRGB::RGBLookup {aka cRGB (*)(const Key_&)}' [-fpermissive]
kaleidoscope::LEDFunctionalColorRGB fcRGB(&FC_RGB_COLOR_LIST(myRGBColorList), FUNCTION);
If my earlier assumption is correct, I thought that must mean that the macros also have to be adjusted so that they are returning cRGB instead of a reference, so I tried removing the & from
cRGB &FC_RGB_COLOR_LIST(NAME)(const Key &k) { \
and
kaleidoscope::LEDFunctionalColorRGB fcRGB(&FC_RGB_COLOR_LIST(myRGBColorList), FUNCTION);
but then I go back to getting the
Model01-Firmware_noseglasses:401: error: no matching function for call to 'kaleidoscope::LEDFunctionalColorRGB::setColorLookup(<unresolved overloaded function type>)'
fcRGB.setColorLookup(kaleidoscope::LEDFunctionalColorRGB::template groupColorLookup<FColorMap>);
Error when compiling, so I don’t seem to quite be on the right track.
Yes, please remove all the references and have the template stuff in the header. Then commit to your github repo and post the full compiler output or put it in a gist.
Done, I’ve pushed those changes to master in my repo.
Here’s the compiler output.
/Users/jdlien/Dropbox/Keyboards/Keyboardio/Model01-Firmware_noseglasses/Model01-Firmware_noseglasses.ino: In function 'void setup()':
Model01-Firmware_noseglasses:401: error: no matching function for call to 'kaleidoscope::LEDFunctionalColorRGB::setColorLookup(<unresolved overloaded function type>)'
fcRGB.setColorLookup(kaleidoscope::LEDFunctionalColorRGB::template groupColorLookup<FColorMap>);
^
/Users/jdlien/Dropbox/Keyboards/Keyboardio/Model01-Firmware_noseglasses/Model01-Firmware_noseglasses.ino:401:96: note: candidate is:
In file included from /Users/jdlien/Dropbox/Keyboards/Keyboardio/Model01-Firmware_noseglasses/Model01-Firmware_noseglasses.ino:62:0:
/Users/jdlien/Library/Arduino15/packages/keyboardio/hardware/avr/1.22.0/libraries/Kaleidoscope-LEDEffect-FunctionalColor/src/Kaleidoscope-LEDEffect-FunctionalColor.h:156:8: note: void kaleidoscope::LEDFunctionalColorRGB::setColorLookup(kaleidoscope::LEDFunctionalColorRGB::RGBLookup)
void setColorLookup(RGBLookup rgbLookup) {
^
/Users/jdlien/Library/Arduino15/packages/keyboardio/hardware/avr/1.22.0/libraries/Kaleidoscope-LEDEffect-FunctionalColor/src/Kaleidoscope-LEDEffect-FunctionalColor.h:156:8: note: no known conversion for argument 1 from '<unresolved overloaded function type>' to 'kaleidoscope::LEDFunctionalColorRGB::RGBLookup {aka cRGB (*)(const Key_&)}'
Multiple libraries were found for "HID.h"
Used: /Users/jdlien/Library/Arduino15/packages/keyboardio/hardware/avr/1.22.0/libraries/HID
Not used: /Applications/Arduino.app/Contents/Java/hardware/arduino/avr/libraries/HID
exit status 1
no matching function for call to 'kaleidoscope::LEDFunctionalColorRGB::setColorLookup(<unresolved overloaded function type>)'
Edit:
Oops, forgot to update the example ino since the one I’m actually compiling is a different file. That’s in the repo now.
Try replacing
fcRGB.setColorLookup(kaleidoscope::LEDFunctionalColorRGB::template groupColorLookup<FColorMap>);
with
fcRGB.setColorLookup(&kaleidoscope::LEDFunctionalColorRGB:: groupColorLookup<FColorMap>);
You need to take a pointer of a function, therefore the &. Maybe I forgot that in my previous examples. Compilers are pretty tolerant with this matter for ordinary functions. But obviously not for template functions.
So this
kaleidoscope::LEDFunctionalColorRGB fcRGB(FC_RGB_COLOR_LIST(myRGBColorList), FUNCTION);
must read
kaleidoscope::LEDFunctionalColorRGB fcRGB(&FC_RGB_COLOR_LIST(myRGBColorList), FUNCTION);
as well.
No luck with that either. I tried a few variations of this.
I put quite a bit of time in tonight implementing a header file called keygroups that I’m using to set color groups as “defaults” and using a user defined struct as the source of the colors for those groups. This is all working really well along with the macros you helped me with earlier.
I could probably make this a bit more efficient with your template function idea but I can’t seem to get that to work no matter what I do… if I comment out the setColorLookup call, it works pretty good otherwise.
It’s almost 3 AM here and I’m exhausted (but pretty satisfied with the progress I made on FunctionalColor tonight) so I’m going to get some sleep. If you have the ability to check out my latest commit on GitHub, maybe you could have a better idea of why this template function won’t run… it’s probably something to do with pointers and references mismatching or the like.
Here’s the last error I got with the version currently on GitHub
Model01-Firmware_noseglasses:425: error: no matching function for call to 'kaleidoscope::LEDFunctionalColorRGB::setColorLookup(<unresolved overloaded function type>)'
fcRGB.setColorLookup(kaleidoscope::LEDFunctionalColorRGB::template groupColorLookup<groupColors>);
^
/Users/jdlien/Dropbox/Keyboards/Keyboardio/Model01-Firmware_noseglasses/Model01-Firmware_noseglasses.ino:425:98: note: candidate is:
In file included from /Users/jdlien/Dropbox/Keyboards/Keyboardio/Model01-Firmware_noseglasses/Model01-Firmware_noseglasses.ino:62:0:
/Users/jdlien/Library/Arduino15/packages/keyboardio/hardware/avr/1.22.0/libraries/Kaleidoscope-LEDEffect-FunctionalColor/src/Kaleidoscope-LEDEffect-FunctionalColor.h:157:8: note: void kaleidoscope::LEDFunctionalColorRGB::setColorLookup(kaleidoscope::LEDFunctionalColorRGB::RGBLookup)
void setColorLookup(RGBLookup rgbLookup) {
^
/Users/jdlien/Library/Arduino15/packages/keyboardio/hardware/avr/1.22.0/libraries/Kaleidoscope-LEDEffect-FunctionalColor/src/Kaleidoscope-LEDEffect-FunctionalColor.h:157:8: note: no known conversion for argument 1 from '<unresolved overloaded function type>' to 'kaleidoscope::LEDFunctionalColorRGB::RGBLookup {aka cRGB (*)(const Key_&)}'
Multiple libraries were found for "HID.h"
Used: /Users/jdlien/Library/Arduino15/packages/keyboardio/hardware/avr/1.22.0/libraries/HID
Not used: /Applications/Arduino.app/Contents/Java/hardware/arduino/avr/libraries/HID
exit status 1
no matching function for call to 'kaleidoscope::LEDFunctionalColorRGB::setColorLookup(<unresolved overloaded function type>)'
Sorry for all that. I shouldn’t have talked you into using templates and then go on vacation without a compiler 
We should stop that template thing here. Could you please push your current state to a separate branch of your repo. Upon my return beginning of April I will tell the templates to behave. 
I have pushed my latest to master. It is working pretty well now. Refer to the new rgb_example.info for an example that compiles and has reasonable defaults for the RGB version.
I’ve made quite a few changes today… instead of your template approach, I’ve essentially abandoned it in favor of adding another macro that lets you specify group colors. I think this approach works pretty well, and is flexible for the user if they don’t want to specify all the groups, but let me know if you still think the template would work better in some way.
At this point I’m pretty satisfied with how functionalColor works - I haven’t been maintaining the ColorBrightness version, and I’m not sure it’s even necessary anymore, but I can come back to that after I polish up the RGB version some more and document how this works so that non-programmers can hopefully understand it.
There are still a couple of shortcomings that I would like to address.
-
I don’t know how to explicitly set colors for keys like Prog, Num, fn, and Any (or other macro keys). Can this be done or must the user be satisfied to just have these be the default color?
-
In the new approach I’ve got a macro for adding group colors, however before the user can invoke the macro, they must add a close curly bracket } to end the switch statement, (which I’ve created a macro FC_ENDKEYS for, for consistency). This is kind of gross… is there a more elegant approach that can somehow automatically close the switch statement upon the invocation of the first FC_GROUPCOLOR macro?
I now have a brightness setting for the entire instance that is applied in setKeyLed so it is easy to create multiple instances and set them to different brightnesses. Another detail you might notice… in colors.h I’ve enhanced my dim() function a bit to create a sort of floor that prevents small adjustments to colors from falling below 28, which, at least on my hardware effectively shuts the LED off. This should prevent minor dimming adjustments to already pretty dim colors from shutting them off completely until they get below a certain threshold. My approach to this wasn’t all that scientific but it seems to prevent already-dim colors from turning off when dimming the entire instance.
Your feedback or any further ideas for improvement are much appreciated.
Doesn’t it work if you just compare against what is specified in the keymap? Like M(MACRO_ANY)? Essentialy, everything that is stored in a keymap evaluates to a constexpr Key.
Here’s a little working demo program with the template stuff: Compiler Explorer
Maybe it helps to find out what’s wrong with the template based approach.
OMG, I just noticed that your groupColorLookup is still not declared static, but it must be for it to work.
When I remove the static in front of groupColorLookup in my little example, I can reproduce your error. Hope that’s it.
It’s really hard to grasp code files on a smartphone display. And compiler explorer is almost unusable on Android’s Chrome 
… in a hammoc.
Huh. Indeed it does work. I thought that there was something special about those keys and that I’d need a different approach. I guess it pays to try the obvious sometimes. I’m still not sure about “prog”, though… I guess that’s just assigned nothing, right? No big deal… the default color can always be used if that’s the only key with no assignment.
PROG is special, because that’s hardwired to be in that position, regardless of your keymap. It is always going to be the R0C0 key.
1 Like
Also - wow, I actually got it to compile using the template function passed to setColorLookup()! Considering how many days I was stuck on that, that’s something of an achievement. Thanks for the example - it made it pretty clear.
I’m not exactly sure what to do with it now, though. Do I just execute groupColorLookup(k) after the end of the switch statement?
(I’ll add that I’m asking because this is what I’m trying but it’s not working at the moment… I’m getting a “no matching function” error, so I’m obviously doing something wrong.)
Congratulations!
I thought one idea was to provide a set of predefined lookup functions to make it easy for the user like e.g. colorStandard, colorAlphaAndMod, color… These functions define, which keys or keygroups are colored, the user supplies the colors through the colormap.
This would be the most basic usage scenario where users wouldn’t be required to supply color lists. As an alternative, users can also supply key-color lists or write their own lookup functions. They could also use the predefined templated lookup functions as default, e.g. in a switch-case or if-else clause.
BTW I would suggest to make these template functions free functions in the plugins own namespace. This makes sense as they do not oprate on plugin class inventory. Also this would enable users to write using namespace ... and then use the lookup function name without full qualification.
Hmm, some interesting ideas here for providing a set of defaults that users could possibly just call using setColorLookup, even without defining their own.
I added a simple test struct into the class definition
struct sampleColorMap {
static constexpr cRGB alpha = dim(white, 170);
static constexpr cRGB number = dim(blue, 190);
};
Then I was able to use it for a new color lookup function with setColorLookup. (I can’t figure out how to make the function call not require specifying the class name, but I read that it can’t be done. Is this true here?)
using namespace kaleidoscope;
fcMax.setColorLookup(&LEDFunctionalColorRGB::groupColorLookup<LEDFunctionalColorRGB::sampleColorMap>);
Would adding a bunch of “preset” structs like my example above incur any overhead if they are not used?
One potential disadvantage with this approach is that I’ll have to always have all the same members in all versions of this struct because I can’t seem to find a way to check for the existence of a member in a struct, the way I can in, say, JavaScript (which is one language I use in my day job). With the macro approach At least the user can just specify the groups that they care about and make it as simple or as complicated as necessary. On the other hand, if they need to define a struct to set color groups, they will have to ensure that they define every one of the 15 group colors or else some of the members that groupColorLookup references won’t be defined.
As for ways around this, I wonder if I can use setColorLookup to fill out an internal “defaults” struct with the user’s structure that overrides those defaults and then use that one. This is probably a much less efficient approach.
I’m kind of thinking out loud…
By the way, I’ve pushed another update to GitHub if you want to see the full code listing.
Not if it’s a static class method. But as suggested if you move it out of the class into a namespace kaleidoscope::LEDFunctionalColorRGB and possibly rename the plugin class kaleidoscope::LEDFunctionalColorRGB::Plugin then the user can write using namespace kaleidoscope::LEDFunctionalColorRGB.
No.
That’s not possible. There’s no reflection/introspection in C++.
They can simply derive a ColorMap struct from a predefined one that is provided by your plugin (I mean not the plugin class here - better put it in a namespace) and just specify those colors that they want to replace.
struct UserColorMap : public DefaultColorMap
{
// color1 also defined in DefaultColorMap
static constexpr cRGB color1 = ...
};
1 Like
Your keygroups header is awesome!
1 Like
Thanks! Perhaps after I test it a bit more thoroughly I’ll submit a pull request to see if there’s any interest in having it added to Kaleidoscope.
I’m attempting a different approach and was hoping you might have some advice for me. I think I’m close to getting this to work, maybe, but I’m don’t think I’m using the optimal approach from a memory usage perspective. (Perhaps this approach is actually a wholly bad idea aside from that).
But I had the idea of allowing a user to create an array of structs that lets a user specify a map of keys and colors.
Here’s my (probably broken) code in FunctionalColor.h
//This sets colors on a per-key basis from the array of structs for key colors
template<typename ColorMap> static cRGB keyColorLookup(const Key &k) {
//In this case ColorMap should be an array, right?
for(int i = 0; i < sizeof(ColorMap)/sizeof(ColorMap[0]); i++ ) {
//if the key member matches k, return color
if (k == ColorMap::k) return ColorMap::color;
}
}
//Is a typedef any value here?
struct keyColor {
Key k;
cRGB color;
};
keyColor constexpr sampleKeyMap[] = {
{Key_A,red},
{Key_B,blue},
{Key_C,green}
};
//In the .ino file under setup()
fcMax.setColorLookup(&keyColorLookup<sampleKeyMap>);
Is this a viable approach? It doesn’t compile currently because I don’t think I’ve got the right format for the template function, but before I put too much effort into it, I wanted to get an opinion on whether this approach is even a good idea to begin with. I think if a user could just specify an array of structs that might be a pretty simple way to configure a handful of keys that are exceptions to the groupings.
On the other hand, this can already be done pretty effectively using the Macros, but I thought it’d be fun to experiment with another approach.
I’ve updated the master branch of the FunctionalColors repo with this sample code. (And boy do I ever feel bad for anyone attempting to use that branch at this point).