# FunctionalColor 2 16-color edition (development discussion)

**URL:** <https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261>\
**Category:** Plugins\
**Created:** [March 16, 2018, 5:47pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261 "2018-03-16T17:47:01Z")\
**Posts on this page:** 20\
**Page:** 5

<div class="post-metadata">

**Author:** ![noseglasses](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/noseglasses/32/429_2.png) [@noseglasses](https://community.keyboard.io/u/noseglasses)\
**Post date:** [March 28, 2018, 4:32pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/81 "2018-03-28T16:32:40Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![jdlien](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/jdlien/32/372_2.png) [@jdlien](https://community.keyboard.io/u/jdlien)\
**Post date:** [March 28, 2018, 4:48pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/82 "2018-03-28T16:48:11Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![jdlien](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/jdlien/32/372_2.png) [@jdlien](https://community.keyboard.io/u/jdlien)\
**Post date:** [March 28, 2018, 9:28pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/83 "2018-03-28T21:28:08Z")

</div>

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

---

<div class="post-metadata">

**Author:** ![noseglasses](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/noseglasses/32/429_2.png) [@noseglasses](https://community.keyboard.io/u/noseglasses)\
**Post date:** [March 29, 2018, 12:09pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/84 "2018-03-29T12:09:28Z")

</div>

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

```cpp
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));

```

---

<div class="post-metadata">

**Author:** ![noseglasses](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/noseglasses/32/429_2.png) [@noseglasses](https://community.keyboard.io/u/noseglasses)\
**Post date:** [March 29, 2018, 12:18pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/85 "2018-03-29T12:18:15Z")

</div>

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

```cpp
FC_COLOR(Key_RightShift, none)

```

---

<div class="post-metadata">

**Author:** ![noseglasses](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/noseglasses/32/429_2.png) [@noseglasses](https://community.keyboard.io/u/noseglasses)\
**Post date:** [March 29, 2018, 1:14pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/86 "2018-03-29T13:14:58Z")

</div>

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

```cpp
#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.

---

<div class="post-metadata">

**Author:** ![jdlien](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/jdlien/32/372_2.png) [@jdlien](https://community.keyboard.io/u/jdlien)\
**Post date:** [March 29, 2018, 1:24pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/87 "2018-03-29T13:24:01Z")

</div>

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?

---

<div class="post-metadata">

**Author:** ![noseglasses](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/noseglasses/32/429_2.png) [@noseglasses](https://community.keyboard.io/u/noseglasses)\
**Post date:** [March 29, 2018, 1:35pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/88 "2018-03-29T13:35:01Z")

</div>

> [@jdlien](#):
>
> Is “none“ just a specific color that I choose to ignore? Just skip coloring it if I see that?

```cpp
constexpr cRGB none = CRGB(0, 0, 0);

```

---

<div class="post-metadata">

**Author:** ![jdlien](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/jdlien/32/372_2.png) [@jdlien](https://community.keyboard.io/u/jdlien)\
**Post date:** [March 29, 2018, 1:44pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/89 "2018-03-29T13:44:43Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![noseglasses](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/noseglasses/32/429_2.png) [@noseglasses](https://community.keyboard.io/u/noseglasses)\
**Post date:** [March 29, 2018, 1:57pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/90 "2018-03-29T13:57:58Z")

</div>

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

```cpp
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.

---

<div class="post-metadata">

**Author:** ![jdlien](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/jdlien/32/372_2.png) [@jdlien](https://community.keyboard.io/u/jdlien)\
**Post date:** [April 5, 2018, 5:34pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/91 "2018-04-05T17:34:09Z")

</div>

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:

```auto
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?

```auto
#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__ )

```

---

<div class="post-metadata">

**Author:** ![noseglasses](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/noseglasses/32/429_2.png) [@noseglasses](https://community.keyboard.io/u/noseglasses)\
**Post date:** [April 6, 2018, 10:22am UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/92 "2018-04-06T10:22:30Z")

</div>

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

```cpp
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`.

---

<div class="post-metadata">

**Author:** ![jdlien](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/jdlien/32/372_2.png) [@jdlien](https://community.keyboard.io/u/jdlien)\
**Post date:** [April 6, 2018, 5:59pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/93 "2018-04-06T17:59:14Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![jdlien](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/jdlien/32/372_2.png) [@jdlien](https://community.keyboard.io/u/jdlien)\
**Post date:** [April 6, 2018, 6:53pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/94 "2018-04-06T18:53:24Z")

</div>

> [@noseglasses](#):
>
> 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.

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?

---

<div class="post-metadata">

**Author:** ![noseglasses](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/noseglasses/32/429_2.png) [@noseglasses](https://community.keyboard.io/u/noseglasses)\
**Post date:** [April 7, 2018, 8:34am UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/95 "2018-04-07T08:34:58Z")

</div>

I would change `FCRGBPlugin::setKeyLed` to

```cpp
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

```cpp
#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

```cpp
#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

```cpp
#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.

---

<div class="post-metadata">

**Author:** ![jdlien](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/jdlien/32/372_2.png) [@jdlien](https://community.keyboard.io/u/jdlien)\
**Post date:** [April 8, 2018, 7:34am UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/96 "2018-04-08T07:34:21Z")

</div>

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.

> **[jdlien/Kaleidoscope-LEDEffect-FunctionalColor](https://github.com/jdlien/Kaleidoscope-LEDEffect-FunctionalColor)**
>
> Lighting effect for Keyboardio Model 01 that colors keys by function - jdlien/Kaleidoscope-LEDEffect-FunctionalColor

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.

---

<div class="post-metadata">

**Author:** ![jdlien](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/jdlien/32/372_2.png) [@jdlien](https://community.keyboard.io/u/jdlien)\
**Post date:** [April 8, 2018, 1:49pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/97 "2018-04-08T13:49:03Z")

</div>

> [@jdlien](#):
>
> 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?

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.

---

<div class="post-metadata">

**Author:** ![jdlien](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/jdlien/32/372_2.png) [@jdlien](https://community.keyboard.io/u/jdlien)\
**Post date:** [April 8, 2018, 5:19pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/98 "2018-04-08T17:19:25Z")

</div>

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

This thread seems suitable for continuing technical discussion, however.

---

<div class="post-metadata">

**Author:** ![noseglasses](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/noseglasses/32/429_2.png) [@noseglasses](https://community.keyboard.io/u/noseglasses)\
**Post date:** [April 8, 2018, 5:40pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/99 "2018-04-08T17:40:08Z")

</div>

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

```cpp
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.

---

<div class="post-metadata">

**Author:** ![noseglasses](https://yyz1.discourse-cdn.com/flex031/user_avatar/community.keyboard.io/noseglasses/32/429_2.png) [@noseglasses](https://community.keyboard.io/u/noseglasses)\
**Post date:** [April 8, 2018, 6:05pm UTC](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261/100 "2018-04-08T18:05:21Z")

</div>

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.

[Previous page](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261.md?page=4)

[Next page](https://community.keyboard.io/t/functionalcolor-2-16-color-edition-development-discussion/1261.md?page=6)