FunctionalColor 2 16-color edition (development discussion)

This thread was used to discuss future development for a new version of the FunctionalColor plugin.
View the current FunctionalColor v2 thread.

Plugin name: FunctionalColor v2.0
Author: JD Lien
Source URL: https://github.com/jdlien/Kaleidoscope-LEDEffect-FunctionalColor/tree/16color

Description:

This plugin automatically colors groups of keys per the user’s preference
based on the current function of the key on the active layer.

This version allows almost every key to be individually set to a desired color.
To allow for this, FunctionalColor uses a 16 color palette with a rainbow of vivid colors, and each color can be set to any of 16 brightness levels, from 0 (off or black) to 15 (full brightness).

0 - WHITE
1 - WARMWHITE
2 - RED
3 - REDORANGE
4 - ORANGE
5 - YELLOW
6 - GREENYELLOW
7 - GREEN
8 - TURQUOISE
9 - CYAN
10 - ROYALBLUE
11 - BLUE
12 - PURPLE
13 - FUCHSIA
14 - MAGENTA
15 - NEONRED

The colors can be referenced by name by including this enum:
enum{WHITE, WARMWHITE, RED, REDORANGE, ORANGE, YELLOW, GREENYELLOW, GREEN, TURQUOISE, CYAN, ROYALBLUE, BLUE, PURPLE, FUCHSIA, MAGENTA, NEONRED};

Using the extension

To use the plugin, include the header, declare an effect using the
LEDFunctionalColor class, and tell the firmware to use the new effect.
Configure your own colors for groups of keys inside setup():

// Automatically sets key's LED on active layer based on the function of the key
#include <Kaleidoscope-LEDEffect-FunctionalColor.h>

// You can make multiple variations of the theme.
// Warning: having several versions consumes a lot of memory! You may have to remove other features or plugins you don't use.
kaleidoscope::LEDFunctionalColor FunColor;
//kaleidoscope::LEDFunctionalColor FunColorMedium;

void setup() {
  Kaleidoscope.use(&FunColor
    //, &FunColorMedium
    );

  Kaleidoscope.setup(

    // If your FUNCTION layer is not the default, you must set it here
    FunColor.functionLayer = FUNCTION;

    // Give the colors in the FunctionalColor palette human-friendly names
    enum{WHITE, WARMWHITE, RED, REDORANGE, ORANGE, YELLOW, GREENYELLOW, GREEN, TURQUOISE, CYAN, ROYALBLUE, BLUE, PURPLE, FUCHSIA, MAGENTA, NEONRED};

    //If you want to adjust these colors, a new method, changePalette allows you to do this:
    // Change WARMWHITE to a pale orange color
    FunColor.changePalette(WARMWHITE, CRGB(255, 210, 140));

    // Here we can set custom colors for your FunctionalColor instance.
    // You can optionally specify a brightness value, 0-15 to dim your lights.

    // Customize key colors by specifying one of the palette colors optionally along with a brightness

    // Set this first to provide a "default" color for all keys, then override with the other settings.
    FunColor.all(WARMWHITE, 7);

    // Set this second to change all modifiers (non-alphabet/numeric/punctuation keys)
    FunColor.allModifiers(WARMWHITE);

    // Set this before individual mouse settings to change all mouse-related keys
    FunColor.allMouse(CYAN); 

    //Set individual groups of colors. You may delete any lines you don't need.
    FunColor.escape(RED, 13);
    FunColor.numbers(WHITE, 14);
    FunColor.letters(WARMWHITE, 7);
    FunColor.punctuation(WARMWHITE, 14);
    FunColor.brackets(WARMWHITE, 14);
    FunColor.backslash(WARMWHITE, 14);
    FunColor.pipe(WARMWHITE, 14);
    FunColor.tab(WHITE, 8);
    FunColor.backspace(RED, 10);
    FunColor.del(RED, 15);
    FunColor.enter(WHITE, 15);
    FunColor.arrows(WHITE, 15);
    FunColor.nav(YELLOW, 11);
    FunColor.insert(YELLOW, 11);
    FunColor.shift(TURQUOISE, 8);
    FunColor.ctrl(ROYALBLUE, 8);
    FunColor.alt(GREEN, 8);
    FunColor.cmd(FUCHSIA, 8);
    FunColor.app(FUCHSIA, 8);
    FunColor.printscreen(ORANGE, 14);
    FunColor.pause(REDORANGE, 9);
    FunColor.scrolllock(GREENYELLOW, 10);
    FunColor.capslock(BLUE, 11);
    FunColor.fkeys(NEONRED, 13);
    FunColor.fn(WHITE, 7);
    FunColor.media(MAGENTA, 12);
    FunColor.led(BLUE, 14);
    FunColor.mousemove(CYAN, 12);
    FunColor.mousebuttons(ROYALBLUE, 15);
    FunColor.mousewarp(CYAN, 15);
    FunColor.mousescroll(ROYALBLUE, 5);

    //If you want to change individual keys, their methods look like this:
    /*
    FunColor.number_1(BLUE); //through 9 and 0
    FunColor.letter_a(BLUE); //through z
    FunColor.number_minus(BLUE);
    FunColor.number_equals(BLUE);
    FunColor.punctuation_backtick(BLUE);
    FunColor.punctuation_period(BLUE);
    FunColor.punctuation_comma(BLUE);
    FunColor.punctuation_semicolon(BLUE);
    FunColor.punctuation_quote(BLUE);
    FunColor.punctuation_slash(BLUE);
    FunColor.bracket_leftbracket(BLUE);
    FunColor.bracket_rightbracket(BLUE);
    FunColor.bracket_leftcurlybracket(BLUE);
    FunColor.bracket_rightcurlybracket(BLUE);
    FunColor.arrow_left(BLUE);
    FunColor.arrow_up(BLUE);
    FunColor.arrow_down(BLUE);
    FunColor.arrow_right(BLUE);
    FunColor.nav_home(BLUE);
    FunColor.nav_end(BLUE);
    FunColor.nav_pageup(BLUE);
    FunColor.nav_pagedown(BLUE);
    FunColor.fkey_f1(BLUE); //through f19
    FunColor.media_play(BLUE);
    FunColor.media_prev(BLUE);
    FunColor.media_next(BLUE);
    FunColor.media_stop(BLUE);
    FunColor.media_volup(BLUE);
    FunColor.media_voldown(BLUE);
    FunColor.media_mute(BLUE);
    */


    //Copy new settings to the dimmed versions
    //FunColorMedium = FunColor;

    // You could make adjustments to your other versions' groups here, if desired.

    // Adjust the brightness of dimmed versions here from 0-15
    //FunColorMedium.brightness(12);


  );
}

Dependencies

2 Likes

To be honest, I’m a bit disappointed in how this turned out which is why I haven’t merged this into the master branch of FunctionalColors just yet.

You’ve got a great deal of flexibility now in that you can customize just about any key individually, but you now are much more limited in how many colors and brightness levels you have. In the end, for all these limitations, this plugin still takes a huge amount of memory. You can easily run one instance of it with the stock firmware, but even two instances won’t really compile without you removing stuff.

It’s certainly more memory efficient than the experimental version that uses a separate color struct for every key, but not dramatically so. Perhaps there is still a lot of work that can be done to make this more effective… perhaps I should just remove the variables for keys that almost no one uses (ie F13-F19).

There’s certainly more work to be done, but this works pretty well, aside from the memory usage, and I’m publishing this in the hopes that it will be useful to a number of people who want something like FunctionalColors but with the ability to specify very specific keys.

1 Like

I took a look at your plugin’s code and found some points that might be worth optimizing.

  1. Color constants in colors.h can be converted to static constexptr to become truly compile time constant. Currently all those values are stored in PROGMEM if used. A variable declared const is still a variable in C++.

  2. You have large amount of spaghetti-code that can be converted to loops. This would require colors to be stored in a matrix similar to that of the keymap. By this means you could drastically reduce the amount of code that needs to be stored in PROGMEM and loaded to RAM. You would also allow users to address the keys with names that match their own keymap. To achieve both, you could define a key position structure and provide inlined lookup methods like so:

struct Keypos {
   byte row; 
   byte col;
};

// ... somewhere in class LEDFunctionalColor

byte colors[ROWS][COLS];

byte &getColor(const Keypos &kp) { return colors[kp.row][kp.col]; }

All those methods that are now named by keys would eventually operate on Keypos. This would drastically reduce the number of methods and the overall amount of code to maintain. Most functions can be shrank down to few lines of loop code. You could define a set of static constexpr Keypos constants that match the key names that you use right now. Users could define similar compile time constants that match the meaning of keys on their keymap. I myself use blank keycaps and Norman layout so the names of all those functions that are related to the standard keymap would not mean much to me.

  1. The palette array could be turned compile time constant and moved to PROGMEM. This could be an additional feature for all those users that do not need runtime palette changes.

  2. current_row and current_column could be converted to uint8_t or the information could even be packed in a single uint8_t.

  3. Turn all the one liner methods into inline methods. This may reduce your PROGMEM and RAM usage tremendously. How much you safe by this optimization depends on how many methods you actually call in your sketch because the others will be omitted at linktime.

Hope this helps to reduce your memory consumption.

3 Likes

One more general hint. In LEDFunctionalColor::setKeyLed there is a huge if-else clause that compares an object of class Key against constants. This could (if not replaced as suggested above) be made much more efficient by using switch-case statements instead.

The overhead of such if-else statements typically grows linearily with growing number of else if-statements n, while switch-case statements grow with log(n) complexity. This is because most compilers generate assembly code that performs a binary search for switch-case statements.

I am not sure wether the current implementation of the Key-union supports direct comparison. You could use Key's .raw member for the comparisons to be on the safe side.

switch(k.raw) {
   case Key_Escape.raw:
      ...
      break;
   case Key_LEDEffectNext:
      ...
      break;
   ...
}
2 Likes

Wow, this is so great. Thank you for this helpful information, @noseglasses.

I’ve been trying to wrap my brain around your point 2 all morning. I probably don’t fully understand what a “key” is - is it just a 3-dimensional array address?

So if a user wanted to make the Key_A RED (RED = 2), could I just create a method that works like this?

FunColors.setColor(Key_A, RED);

I’m not sure if in your example you missed a dimension for the layer or if that’s just not necessary. I was thinking that the method definition could be something along the lines of

byte colors[LAYER][ROWS][COLS];

setColor(key, colorindex) {
	//Magic that looks up Key based on name?
	colors[key] = colorindex;
}

The (main?) part that I’m missing here is how to get from a key name (like Key_A) to the index of a multidimensional array… if I can solve this, then I should be able to implement your suggestion pretty easily and conduct a massive cleanup of my code.

A Key object is a two-byte structure (union, actually). Key.raw is a 16-bit unsigned integer, perfect for use in a switch statement. Key_A.raw is the integer value for the constant named Key_A.

1 Like
for (uint8_t r = 0; r < ROWS; r++) {
  for (uint8_t c = 0; c < COLS; c++) {
    Key k = Layer.lookupOnActiveLayer(r, c);
    if (k == key)
      colors[r][c] = color;
  }
}

Something along these lines. The idea is to iterate over the active layer, find the key the user specified, and store that in colors. If you want to support layers… that becomes a little bit harder. In that case, perhaps something like the Colormap would be more useful.

1 Like

Sorry, I see that I misread what you are up to. I thought you planned to assigned colors by key positions.

So what you actually need is the mapping of an object of type Key to an integer that can be used to lookup in a one dimensional array that stores your color values. You can use the switch statement I mentioned in my previous post.


// ... somewhere in class LEDFunctionalColor

byte colors[NUM_KEYCODES]; // Define NUM_KEYCODES accordingly

byte getColorIndex(const Key &k) { 
   switch(k.raw):
      case Key_Escape.raw:
         return 0;
         break;
      case Key_LEDEffectNext.raw:
         return 1;
         break;
      ...
      default:
         // Error
   }
}

byte &getColor(const Key &kp) {
   return colors[getColorIndex(k)];
}

If you are only interested in distinguishing keycodes without flags you could also just switch(k.keyCode) and adapt the case blocks to check only the .keyCode part. This will likely reduce the size of the generated code as the compiler can use uint8_t instead of uint16_t in the implementation of the binary search that resembles the switch.

Thanks everyone, that’s super helpful.

@noseglasses, I’m not sure what NUM_KEYCODES is or how I’d define that… that’s just a constant representing the maximum possible number of keycodes? Is this not going to always be the same (given a certain version of Kaleidoscope)?

Yes. But it can but it can be less if you decide not to consider those keys that are not used in the keymap. This of course would require additional changes to your plugin. You could let the user define an external lookup function

// Somewhere in the sketch
//
namespace FunctionalColor {

#define KEY_CASE(KEY, ID) \
   case KEY_##KEY.raw: \
         return ID; \
         break;

byte userGetColorIndex(const Key &k) { 
   switch(k.raw):
      KEY_CASE(Escape, 0)
      KEY_CASE(LEDEffectNext, 1)
      default:
        // ...

byte nUserKeycodes = ...;
byte userColors[...];
}

… and use/call userGetColorIndex, nUserKeycodes and userColors from you plugin (which also defines them as a weak symbols to be on the safe side).

Sorry, I had to re-edit the last post as I accidentally submitted it.

… or even more user-friendly providing macros.

#define FC_USER_COLOR_LIST(ID) \
   userColor##ID

#define FC_START_USER_COLOR_LIST(NAME) \
   byte &FC_USER_COLOR_LIST(ID)(const Key &k) { \
      /* EDIT: That was a leftover. Ignore. constexpr byte idOffset = __COUNTER__; */ \
      switch(k.raw) {

#define FC_END_USER_COLOR_LIST \
      default: \
         {
         static byte dummy; \
         return dummy; \
       } \
       break;
    } \
}

#define FC_KEY_CASE(KEY) \
   case KEY_##KEY.raw: \
       { \
         static byte color; \
         return color; \
       } \
       break;

The user can then write

// In the sketch (namespace optional)
//
namespace FunctionalColorPlugin {

FC_START_USER_COLOR_LIST(0)
   FC_KEY_CASE(Escape)
   ...
FC_END_USER_COLOR_LIST
}

// Where the plugin is initialized
//
FunctionalColor.setColorList(FunctionalColorPlugin::FC_USER_COLOR_LIST(0), 0);

The latter allows the user to define as many color lists as desired and supply function pointers to your plugin instances. It’s thus the users responsibility to list all those keys that are used in the keymap.

This solution is the most memory friendly I can come up with. Note the static color variables.

I was a bit in a hurry when I wrote the last post yesterday. As I am not quite sure that it is at all clear, what the code does, here some explanations.

The approach assumes that the plugin instance has (one or more) function pointer members

// in class definition of FunctionalColor

typedef void (*UserColorLookup)(const Key &);

UserColorLookup userColorLookup[MAX_LAYERS]; // Define MAX_LAYERS accordingly

void setColorList(UserColorLookup ucl, int layer) { userColorLookup[layer] = ucl; }

In all places where the plugin needs to lookup or modify the color that is associated with a Key value, it uses the lookup functions like

void LEDFunctionalColor::onActivate(void) {
	// Loop through every row and column and set each LED based on its key's function
	for (uint8_t r = 0; r < ROWS; r++) {
	  for (uint8_t c = 0; c < COLS; c++) {
	    Key k = Layer.lookupOnActiveLayer(r, c);
            auto layer = ... // lookup the layer
            assert(userColorLookup[layer]);
            auto colorId = (*userColorLookup[layer])(k);
            const auto &color = getCRGB(colorId);
            LEDControl.setCrgbAt(r, c, color); 
	  }
	}
}

I have seen you grouping Key values like in

void LEDFunctionalColor::fkeys(uint8_t color, uint8_t brightness)

This can also be made the users respnsibility. Just provide a FC_SHARE_COLOR.

#define FC_COLOR_LIST(ID) \
   userColor##ID

#define FC_START_COLOR_LIST(NAME) \
   byte &FC_COLOR_LIST(ID)(const Key &k) { \
      switch(k.raw) {

#define FC_END_COLOR_LIST \
      default: \
         {
         static byte defaultColorId = 0 /* or any default index */; \
         return defaultColorId; \
       } \
       break;
    } \
}

#define FC_COLOR(KEY, DEFAULT) \
   case KEY_##KEY.raw: \
       { \
         static byte colorId = DEFAULT; \
         return colorId; \
       } \
       break;

#define FC_SHARE_COLOR(KEY) \
   case KEY_##KEY.raw:

The user can then write in the sketch

// in the sketch at global scope
FC_START_COLOR_LIST(0)
   FC_COLOR(Escape)
   ...
   // All function keys share one color id variable that initialized with color id 3
   FC_SHARE_COLOR(KEY_F1)
   FC_SHARE_COLOR(KEY_F2)
   ...
   FC_COLOR(KEY_F19, 3 /* index in color table */)
   ...
FC_END_COLOR_LIST

Instead of using the color id as a variable you could go right back to rgb colors. The storage needed is not too high using this approach given that the number of keys used in the keymap is limited.

Note: I renamed some of the macros from my last example to make it more suitable.

You could also let the user define a palette array in the sketch and register it with the plugin. Thus, the user can decide how many different colors she/he wants and no storage is wasted.

// in the sketch at global scope
cRGB palette[] = {
    CRGB(255,255,255), //white
    CRGB(255,235,215), //warm white aka antiquewhite
    CRGB(255,0,0)
  };

...

// At initialization of plugin 
FunctionalColor.setPalette(palette, sizeof(palette)/sizeof(cRGB) // the number of entries);
1 Like

Alright, it’s clear that you have a few years of C++ experience on me (this is essentially my first C++ code I’ve written except for one course I did in school). I’m struggling to wrap my head around some of this stuff.

The function pointer stuff is confusing me. What is setColorList() doing in your last example? In your idea for FunctionalColor, are you using this function to create an array of colors for all the keys?

It looks like UserColorLookup is a function that, when passed a key, returns the colorbrightness byte (or perhaps CRGB if I switch back to that), but I don’ t understand how it can do that based on your example.

It registers the color index lookup function defined by the sketch with the plugin object which stores it as a member.

The lookup function maps keys to color index variables. It returns a reference to statically allocatede bytes, one for each key or key group.

EDIT:
Yes, the colorbrightness value or cRGB. I can provide you with a full example tomorrow. Currently I have just a smartphone which is quite inconvenient.

I forked your repo and modified it the way I suggested. Changes compile but are untested (lack of time :frowning: ). If you are willing to test it and happen to like it, I can submit a PR back to your repo.

Here is an example sketch (as included in the examples folder).

This is pretty awesome… thanks so much for this. That said, I don’t understand it and in the very short time I’ve had to attempt using it, I couldn’t get it to work.

The first thing I’m stuck on is how the CBLookup/RGBLookup works. Are those… made with preprocessor macros? I don’t see any function declarations anywhere for those. It’s a complete mystery to me how those manage to return a color associated with a Key constant.

Is this typedef just a shortcut to save you from making a really complex pointer statement every time you want to use it?

typedef byte & (*CBLookup)(const Key &);

A little more explanation so I could start to grasp how this works would be appreciated, if you’d be so kind.

Sorry to hear that.

Sure. What about the following procedure.

  1. I will have a look at my code and see what goes wrong on one of the next days. I do not have the keyboard with me but I can test it with the simulator which also can display the LED colors.
  2. Then I will submit a PR to your repo. By this means you can comment on lines of my code and ask specific questions.
  3. Based on your questions I will add comments to the code until it is maintainer-friendly :wink:

Some of your questions I can answer right away.

The FC_CB_COLOR_LIST(ID) macro just creates unique symbol ids for the color lookup functions, say colorBrightness_1 if ID is 1. That’s necessary because we might want to support multiple instances of the same plugin with different color maps.

Line 71 of macro FC_CB_START_COLOR_LIST(NAME) is actually a function definition in disguise. For the name of the function I use the same macro FC_CB_COLOR_LIST(ID) that is used to register the color list with the plugin. This ensures nothing goes wrong with the function name between definition of the list and the registration.

Once the macros have been invoked during keyprocessing, the function signature becomes actually

byte &colorBrightness_1(const Key &k)

A function that maps a key to a reference to a byte that stores its assigned color brightness value (or that of the key’s group (shared color brightness).

Exactly. Code that deals with function pointers tends to be impossible to read unless the function pointer type is typedefed. Even the signature of a function pointer typedef is confusing on first encounter. The name of the newly defined type is the one in the middle, CBLookup. With data typedefs like

typedef unsigned int Grade;

in contrast, one is used to find the name (Grade) at the end of the line.

Once typedefed, function pointers can be passed through a program just as any other data type.

void setCBLookup(CBLookup newLookup) { ... }

A function pointer instance is defined and initialized as any ordinary data type

CBLookup theLookupFunctionPtr = anotherLookupFunctionPtr;

Only the call is a bit strange again

Key aKey = ...
byte aColorBrightness = (*theLookupFunctionPtr)(aKey);

The strange syntax (*theLookupFunctionPtr)(...) is because the function pointer must be dereferenced to call the function.

The function pointer type definition

typedef byte & (*CBLookup)(const Key &);

reads “A pointer to a function that takes a constant reference to a Key and returns a (non constant) reference to a byte”. The latter, the non-const reference is important as we not only want to read the byte but we also want to write it. Thus, what we intent to do is

byte newColorBrightness = ...
(*theLookupFunctionPtr)(aKey) = newColorBrightness;

which is equivalent to

byte newColorBrightness = ...
byte &keysColorBrightness = (*theLookupFunctionPtr)(aKey);
keysColorBrightness = newColorBrightness;

Remember, the referenced byte that is returned by the lookup function is a statically allocated variable. This ensures that we do not waste memory. The lookup function nicely combines two tasks, fast mapping of Keys to CB-bytes (compiler generated binary search based on switch-case) and the automatic allocation of storage (the static CB-bytes). This removes the burden from the plugin to deal with memory allocation. Memory is allocated (implicitly) in the sketch. Using this approach simplifies the syntax for the user who does not have to deal with array definition but can just assign color ids or rgb values.

My implementation follows two principles.

  1. Provide a simple interface and hide details so you can change them later without breaking user code.
  2. Avoid dynamic allocation and let the user (statically) allocate as much memory as needed for a task and pass it to the code that deals with it (the plugin).

The latter is something that I would recommend in general for code that runs on embedded systems with restricted memory.

If you have more questions feel free to ask right away or once I have submitted the PR.

Just working on my proposed changes to your plugin.
The colors in header colors.h might be useful for others too. It there is not a similar header somewhere in the core firmware, I would suggest to create a PR to Keyboardio/Kaleidoscope that submits the color header. What do you think?