FunctionalColor 2 16-color edition (development discussion)

Programming Plugins
81

That’s not possible the way you attempt it. An array can either exist at compiletime (constexpr) or at runtime (somewhere in memory PROGMEM/RAM). You define it constexpr and then try lookup at runtime.

Even if it wasn’t constexpr this approach would have two drawbacks. Firstly, your code’s runtime complexity grows linearily with number of different keycodes compared with the switch/case approach’s logarithmic complexity. Secondly, you would need extra PROGMEM lookup statements which the switch/case approach doesn’t require either.

82

Yeah, so basically there’s no way to do this as constexpr and have an array that can be iterated through…

I can’t think of a way to do this that would be anywhere near as efficient as the macro that generates the switch statement. I could make it work, but then it’d take a ton of memory.

Hmm…

The thing that bothers me about the approach from before, using the struct to set group colors, is that I pretty much have to have a predefined set of things that can be changed. For instance, you can change your letters, numbers, punctuation, function keys, and arrow keys, but if you wanted to do something that falls outside of that group, like make your homing keys (F and J) a different color, you can’t. I was hoping to implement something that allowed for that, but I can’t think of how…

Perhaps another macro that created a second function for exceptions to the key groups?

Hmm… the more I think about it, the more that sounds like a good idea. Just a macro that is constructs an “exceptions” switch. Maybe I can even use the original macro that you wrote that already essentially does this. I’m excited to try it. Then I can actually have the best of both worlds here – a bunch of preset templates that you can easily tweak or make exceptions to if desired.

83

Note to self: Based on a request, I’m considering implementing user-customizable groupings. I’ll think about how this can be done.

84

What was wrong with the original approach and a default-clause?

struct groupColors {
    static constexpr cRGB alpha = dim(warmwhite, 100);
    static constexpr cRGB number = dim(white, 200);
    static constexpr cRGB punctuation = dim(orange, 190);
    //...
  };
FC_START_COLOR_LIST(myRGBColorList, white, 127)   
   // Let multiple keys be grouped by color to make it easier to change them all at once
   FC_GROUP(Key_LeftShift)
   FC_COLOR(Key_RightShift, darkseagreen)
   
   FC_GROUP(Key_LeftAlt)
   FC_COLOR(Key_RightAlt, forestgreen)
   
   FC_GROUP(Key_LeftGui)
   FC_COLOR(Key_RightGui, pink)
   //...
   
   default:
      return kaleidoscope::LEDFunctionalColorRGB:: groupColorLookup<groupColors>(k);

FC_END_COLOR_LIST

kaleidoscope::LEDFunctionalColorRGB fcRGB(FC_COLOR_LIST(myRGBColorList));
85

With the colorlist-with-default approach you can easily override single keys’ group colors or even remove any color by using

FC_COLOR(Key_RightShift, none)
86

What about using a function macro for the group coloring default.

#define FC_DEFAULT_GROUP_COLORING(COLOR_MAP) \
   default: \
      return kaleidoscope::LEDFunctionalColorRGB:: groupColorLookup<COLOR_MAP>(k); \
      break;

By this means it is even more user friendly.

87

This is exactly what I’m looking for and pretty much what I figured out after thinking about it for a while last night.

One thing I could not figure out is how to remove a color… because I have to return some kind of color, right? How would you propose doing that?

Is “none“ just a specific color that I choose to ignore? Just skip coloring it if I see that?

88 
constexpr cRGB none = CRGB(0, 0, 0);
89

0,0,0 is black, which perhaps may want to used by someone… I was thinking that maybe I’d use some arbitrary but unlikely to be used color like CRGB(1, 2, 1) (which would render as black anyways) and then instead of being black, I could have the code “skip” it and then go onto the defaults or not color it at all so it becomes “transparent” and also won’t change color when the fn layer is invoked if a key is set this way.

90

I’d rather use a boolean function parameter that can be used to tell the code to skip coloring a specific value. This would require a signature like

cRGB colorLookup(const Key &key, bool &skip);

The reference argument skip (think about it as an additional return value) would only be touched and set true by the lookup function in the rare occasion where a key’s color is not supposed to be changed.

1 Like
91

I will finally have a bit of time to work on this now.

Not super relevant to what we were talking about in the last few posts, but I’ve been trying to see if I can simplify use of the macros such that you can have one macro that accepts either one or two arguments. It appears that such usage is possible, although far from straightforward.

Ideally, in the definition you could specify something like the following:

FC_START_COLOR_LIST(myRGBColorList, white)   
  // Let multiple keys be grouped by color to make it easier to change them all at once
  FC_KEY(Key_LeftShift)
  FC_KEY(Key_RightShift, darkseagreen)
FC_END_COLOR_LIST

I think that the following approach isn’t working for me because keys and colors expand prematurely and kind of mess the whole macro expansion up.

Any ideas on how this might be made to work, or is this just a waste of time?

#define FC_GROUPKEY(KEY) \
   case (KEY).flags << 8 | (KEY).keyCode:

#define FC_KEYCOLOR(KEY, COLOR) \
    case (KEY).flags << 8 | (KEY).keyCode: \
       { \
         static cRGB color = COLOR; \
         return color; \
       } \
       break;



#define _ARG2(_0, _1, _2, ...) _2
#define NARG2(...) _ARG2(__VA_ARGS__, 2, 1, 0)

#define _ONE_OR_TWO_ARGS_1(KEY) FC_GROUPKEY(KEY)
#define _ONE_OR_TWO_ARGS_2(KEY, COLOR) FC_KEYCOLOR(KEY, COLOR)

#define __ONE_OR_TWO_ARGS(N, ...) _ONE_OR_TWO_ARGS_ ## N (__VA_ARGS__)
#define _ONE_OR_TWO_ARGS(N, ...) __ONE_OR_TWO_ARGS(N, __VA_ARGS__)

#define FC_KEY(...) _ONE_OR_TWO_ARGS(NARG2(__VA_ARGS__), __VA_ARGS__)
92

I’m afraid, you are right, this will not work, for the exact reason you stated. Although your attempt to generate a function macro whose behavior depends on the number of arguments seems correct, this wont’t work when other macros whose definitions contain commas are passed as call arguments (e.g. the key definitions).

This is a problem that is impossible to solve without extra work of the user. The solution would require to wrap every problematic call parameter in braces like

FC_KEY((Key_LeftShift))
FC_KEY((Key_RightShift), darkseagreen)

I am not sure that this would improve usability against two well documented macros FC_GROUPKEY and FC_KEYCOLOR.

93

Yeah… this seems rather pointless in light of this.

I thought that perhaps this would work using your old approach of prefixing everything in the macro with “Key_” but then that prevents a lot of kinds of keys from being used, which I think it too limiting.

I guess I’ll abandon this approach. For now.

94

I’ve been trying to implement something like this and wasn’t sure how this works… regardless of whether another input parameter is passed to the colorLookup function, I’ll still have to return some kind of color that setKeyLed would use, right?

I was thinking that I could modify setKeyLed() to first check the color of the key at a given position, then set to that color under most circumstances, except if the color matches some definition of a “transparent” index color, like how transparent GIFs work.

Is your idea to just return such a special color then if a key and a true value are both passed to the colorLookup function?

95

I would change FCRGBPlugin::setKeyLed to

void FCRGBPlugin::setKeyLed(uint8_t r, uint8_t c) { 
  Key k = Layer.lookupOnActiveLayer(r, c);
  bool skip = false;
  auto color = (*rgbLookup_)(k, skip);
  if(skip) return;
  ::LEDControl.setCrgbAt(r, c, dim(color, brightnessSetting));
}

As the parameter skip is passed by non-const reference, it can be modified by the color lookup function, thus acting as an additional return value.

FC_START_COLOR_LIST would be adapted to

#define FC_START_COLOR_LIST(NAME, DEFAULT_COLOR) \
   cRGB FC_COLOR_LIST(NAME)(const Key &k, bool &skip) { \
      constexpr cRGB initialDefaultColor = DEFAULT_COLOR; \
      switch(k.raw) {

and we could have a skip function macro

#define FC_SKIP_KEY(KEY) \
    case (KEY).flags << 8 | (KEY).keyCode: \
       skip = true;
       return black; // could return any color as a dummy

FC_DEFAULT_GROUP_COLORING would become

#define FC_DEFAULT_GROUP_COLORING(COLOR_MAP) \
   default: \
      return kaleidoscope::LEDFunctionalColorRGB:: groupColorLookup<COLOR_MAP>(k, skip);

FWIW, in some of the macros there are break statements after return statements that I introduced in my previous examples. Those breaks, that are never reached, can be removed to improve readability.

96

I have put a fair bit of work and cleanup into FunctionalColor today and I feel that it is getting close to being in a usable state. I’m now using both a colorMap struct with groupColorLookup and an “override” function, optionally generated by the macros, to inform the color lookup function. This should offer the best of both worlds in terms of being easy to use, flexible, and really customizable without a great deal of difficulty and awkward code. I’ve updated my Git repo with all the latest updates. Note that there is a new example .ino included that shows many use cases in action.

I still need to do a little more polishing and testing and complete the documentation and such. Before I finish that, I’d like a little bit of feedback to see if there are any improvements that can be made.

I’ve now got functions that can be used within macros to control brightness. Can they be made a little smarter so they only control the active instance? I could probably do something to update a static variable with a pointer to the last instance updated and then use a static method to update that. Am I on the right track?

Are there any terrible inefficiencies in what I’ve got so far? At about 4k of memory usage I know the plugin isn’t as trim as the first version @noseglasses made, but I think that’s mostly due to my more complex examples and all the functions I’ve added.

In the groupColorLookup function I have cases where there are overlapping groups of keys - for instance shift keys are contained within modifier keys. I thus needed a way to specify in the colorMap struct that a color was to not be used so that it would fall back to its other container (the parent, if you will). I have done this by defining a color called nocolor (it is close to black) and checking if the color is set to that before returning it. Perhaps there is a better way but I couldn’t figure out how to use the bool &skip approach that worked in other scenarios.

97

Yay, I got this working quite well. Now brightnessUp/Down are static methods that affect the last-updated instance only, so they can affect the brightness for whichever FC instance is active. This is a nice feature. Now instead of making several instances of FC just for brightness changes, you can adjust one instance through 16 brightness levels just with keyboard shortcuts. If you’re stingy with keys you can even just use one, because it wraps around once you pass the maximum or minimum.

1 Like
98

I have now completed the documentation and posted a new forum thread about this project here:
https://community.keyboard.io/t/functionalcolor-v2/

This thread seems suitable for continuing technical discussion, however.

99

Congratulations! Really nice work.

There are some minor issues that I found when looking at the code (I did not test the plugin yet). Some of those are critical, others not and all of them are easy to fix without requiring any great changes.

In header Kaleidoscope-LEDEffect-FunctionalColor.h you added

using namespace kaleidoscope::LEDFunctionalColor;

I would recommend to remove this and let the user add it to the sketch or fully qualify the plugin class name.
II am aware that you intent to make things as user friendly as possible. But if you add such a using directive to a header it automatically imports all symbols from the respective namespace to the global namespace (or where the header is included). This makes the namespace whose actual purpose it is to prevent symbol name collisions kind of useless.

One issue that will degrade memory consumption is the void FCPlugin::themeSelect(int themeID) method. It instantiates all groupColorLookup template methods even though just on is used. groupColorLookup is quite a large method so there is plenty of PROGMEM to save here.

mainColorLookup is initialized as nullptr but used by void FCPlugin::setKeyLed(uint8_t r, uint8_t c) without being checked. This can cause a segfault.

If there was no refresh before the first brightness adjustment, then FCPlugin::lastFC is an uninitialized pointer which will likely cause a segfault.

But once again, all these are just minor issues. All in all the plugin is great and I am eager to try it, once I got my M01 running.

100

Forget about what I wrote about segfaults. I am not sure what the atmega does but it will either crash or exhibit strange random behavior.