FunctionalColor 2 16-color edition (development discussion)


(JD Lien) #21

Great idea. I was hoping to add things like this to the stock firmware that might make it easier to program but am concerned about memory usage and such. Seems you know how to minimize the impact of that, however.


(Noseglasses) #22

I just converted all of your CSS color definitions to constexp. Such compile time constants do not have any memory impact at all. There might be other plugins that deal with colors or users would like to use such descriptive color names in their sketches.


(JD Lien) #23

Cool.
Would something like my dim() function also make sense in the core?


(Noseglasses) #24

Certainly, if not already present somewhere else. I would suggest an (inlined constexpr) version of this function that also accepts the number of brightness levels as an additional parameter.


(JD Lien) #25

I sure couldn’t find anything like this when I looked…

If we’re creating generic utility functions like this there might be other useful ones. How about something to set an LED by the key name? I’m have to think about whether this makes sense in a generic context and can be done efficiently, but obviously that is half the point of FunctionalColor itself.

It seems you’ve significantly improved the bigass if/else block I had that iterates through all the known keys so maybe that could be generalized.


(Noseglasses) #26

That’s exactly what FunctionalColor does. But the idea could be generalized. A Kaleidoscope plugin does not necesarily need to be hooking into the core firmware but can also provide services. Many parts of the core firmware work this way.

I just submitted a PR to your repo. Would you be so kind and have a go with it on the real hardware? I updated the example firmware. There are now two LED modes. One with CB-based lookup that turns the whole keyboard green except for two letter keys and some numbers. Then there is another RGB-based mode that uses white as default color and apart from that colors also two letters and the numbers.


(JD Lien) #27

Ironically, I brought my keyboard home last night and ended up not having time to work on it, now I’m at work and can take a look at the code but left my keyboardio at home.

I’ll examine the code today and attempt installing it on the Model 01 later tonight. (I’m in MST, so it’s just after 9 AM right now - it’ll be a while.)


(Noseglasses) #28

That’s ok :wink:

As an european, dare I ask what MST means? The results that google brought up are quite ambiguous.

As a little teaser the following image shows what my simulator predicts.


(JD Lien) #29

MST = Mountain Standard Time, technically we’re in MDT (Mountain Daylight Time) means, which is the time zone used in Alberta, Canada this time of year. One of these days we’ll surely manage to abolish Daylight Savings Time…

The simulator screenshot is interesting. I’m just having a peek at your code now and simultaneously reviewing pointers/references/typedefs in C++.


(Noseglasses) #30

Yeah, it’s the same in Germany.

:+1:


(JD Lien) #31

I think I’m beginning to understand how this works now… I’m currently unraveling how the preprocessor macros work to construct a switch statement that retrieves the desired colors for each key.

Is it possible somehow to see the resulting code that is created after the preprocessor does its business with the macros?


(Noseglasses) #32

What you mean is to just pre-process the sources. That’s unfortunately not supported by Kaleidoscope-Builder.


(Noseglasses) #33

With Leidokos-CMake as build system you can easily pre-process individual files of the firmware build.

Here’s the pre-processed sketch of the example that I added to my PR.


(JD Lien) #34

Interesting. I’ll have a look in a bit. Vielen dank for all your effort on this and your patience in helping me understand all these concepts I’ve not really used before. I truly appreciate it and I’m sure this is all helping me become a better programmer.

I have a pretty solid understanding of how your code works now, and I spent some time last night making a few tweaks to allow brightness adjustments and (I think) I (mostly) fixed a bug that prevented the colors from switching when fn is pressed.

I want to see if I can simplify this a bit for the end users, as I find it a bit more complicated to use than I’d like… this might mean, for example, removing the ability to customize the palette on the CB version, but I’m not too sure yet. Ideally, I’d like it if a user could just create an instance and use it “out of the box” with no configuration by incorporating useful defaults.

This is all pretty great, though. The memory usage is far, far lower than before but I haven’t yet constructed a complete and more elaborate example that sets lots of colors. I will do that today and examine how the memory usage ramps.

In your opinion, would there be a big difference in memory use between the CB and RGB versions of this plugin? In your comments it was implied that the RGB version uses less memory, but is that just because you set fewer keys?

I’d rather not have both versions if either one is clearly more resource intensive or if there isn’t a big difference but one is far more flexible.


(Noseglasses) #35

I wish I had had such a nice project to play with like a Keyboard firmware when I started programming. There are lots of interesting technical aspects about this project, good code quality, interesting discussions about changes and improvements and most of all nice people around. Definitely a good place to learn and improve your C++ skills.

The RGB version has less code, which means that it is likely to be more memory efficient when not too many colors are used. I would expect a break-even point where the CB version becomes more efficient when increasing the number of colors because of the lower memory requirements to store CB bytes instead of cRGB.

Good idea. The nice thing about the firmware build is that all parts of code that are not called are stripped away during linkage. This means that you can provide an infinite number of predefined color lookup functions. Only those that the user actually selects get linked into the firmware.

Even though I understand your wish to simplify the user interface, registering color lookup functions from the sketch is still a good idea. This is because users can e.g. change the lookup at runtime by just resetting plugin instances’ current lookup functions.

I would recommend against a set of color lookup functions that are compiled with the plugin and set via a if/then or switch statements of a plugin-method, as all those functions would be linked into the firmware.

To understand this, it is important to think about how the stripping of functions at linktime works. AFIK the linker generates a static function call graph, starting from the main function (which is synthesized by Arduino). All functions that are not part of the graph, are stripped because they are not called at runtime. If you have a if/then or switch/case statement that selects lookup functions, then the linker cannot strip anything at all as all those functions can potentially be called at runtime.

Even though it makes the sketch slightly more complicated, it is more efficient to e.g. offer something like this

class Kaleidoscope_FunctionalColorRGB
{
   public:
       
      static cRGB colorLookup1(const Key &k) { ... }
      static cRGB colorLookup2(const Key &k) { ... }
      static cRGB colorLookup3(const Key &k) { ... }
      ...

      typedef void (*ColorLookupFnc)(const Key &);
      void setColorLookup(ColorLookupFnc clf) { ... }

      ...
};

// In the sketch at global scope
//
Kaleidoscope_FunctionalColorRGB fcRGB;

...
void setup() {
   fcRGB.setColorLookup(Kaleidoscope_FunctionalColorRGB::colorLookup1);
   ...
   Kaleidoscope.setup();
   Kaleidoscope.use(
      ...
      &fcRGB,
      ...
   );
}

Users could then use predefined color lookup functions and toggle them, e.g. from macros. Only those lookup functions that are called are actually linked.

Except for maintainance overhead, there is nothing wrong with having both. If the user only uses one of both versions, only the selected one will be linked.

I also thought about a simplification/generalization. In most cases I would expect the key to color mapping to be fix. Only few users might want to change colors at runtime. This would simplify the signature color lookup function slightly as it would not need to return references to cRGB values. It would become something like

cRGB colorLookup(const Key& k) { ... }

The methods of the plugin class that control coloring could then be omitted.

With the interface I submitted yesterday but also with the new one, generalized key classing would be possible.

cRGB colorLookup(const Key& k) { 

   if(modShift(k) {
      return green;
   }

   if(modControl(k)) {
      return blue;
   }

  if(isNumber(k)) {
     return red;
  }

  if(isWordCharacter(k)) {
     return cyan;
  }

  // Switch case as already implemented
  ///
  switch(k.raw) {
     case ...
  }
}

(JD Lien) #36

What are the modShift(), modControl(), isNumber(), isWordCharacter() functions? Those aren’t something already in the firmware, are they?

I’m guessing they are examples of functions I could create that would just have a switch that returns true if a key is one of a given group…

ie something like this?

bool LEDFunctionalColorCB::isNumber(const Key& k) {
  switch(k.raw) {
    case (Key_1).flags << 8 | (Key_1).keyCode:
    case (Key_2).flags << 8 | (Key_2).keyCode:
    case (Key_3).flags << 8 | (Key_3).keyCode:
    case (Key_4).flags << 8 | (Key_4).keyCode:
    case (Key_5).flags << 8 | (Key_5).keyCode:
    case (Key_6).flags << 8 | (Key_6).keyCode:
    case (Key_7).flags << 8 | (Key_7).keyCode:
    case (Key_8).flags << 8 | (Key_8).keyCode:
    case (Key_9).flags << 8 | (Key_9).keyCode:
    case (Key_0).flags << 8 | (Key_0).keyCode:
    {
      return true;
    }
    break;
    default: return false;
    break;
  }
}//end isNumber()

In these case statements, is “(Key_1).flags << 8 | (Key_1).keyCode” just a way of converting a Key struct into its raw value?


(Noseglasses) #37

Yes, sorry, I forgot to mention that this is pseudo-code.

As the number keys are in consecutive order, there must be a better way to do this, something like

if((k.keyCode >= ...) && (k.keyCode <= ...)) { /* its a color */ }

but for other key classes/groups that switch statement would be reasonable.

We had a bit of a discussion about this topic on GitHub.

Currently there is no way to use a Key as a case label. That’s why we have to do the extraction in this akward way. (Key_1).flags << 8 | (Key_1).keyCode evaluates to a compiletime constant expression which is necessary for use as a case label.


(JD Lien) #38

if((k.keyCode >= …) && (k.keyCode <= …)) { /* its a color */ }

Sorry if this should be obvious, but where can I look up these keycodes? I presume I could do something similar with, say, alpha keys as well.


(Noseglasses) #39

No, its far from obvious because there quite a number of files. Keycodes are defined in repo Keyboardio/Kaleidoscope in src/key_....h


(JD Lien) #40

Alright, so most of them are here:

Is it safe to assume that all keys defined consecutively (and that should reasonably stay that way through future revisions of Kaleidoscope) would have consecutive keycodes? I presume these are all given by the compiler and not under the direct control of a developer…

That being the case, I could use >= && <= for letters, numbers, punctuation (Key_LeftBracket through Key_Slash), F1-F12, arrows, keypad keys, and maybe even modifiers (LeftControl through RightGui).

In addition, I’d want to allow customizing the consumerctl keys and have noted that some of your macros seem to presume that keys all start with Key_ which isn’t the case here… but I presume this can be rectified by just making the user enter Key_A instead of simply A, and leave that out of the macro. (Unless there’s a better way).

It looks like all the Consumer keys could be grouped very easily in this way as well… (which is handy as there is a nutty number of them).