How to get the number of defined layers

While looking through layers.cpp, I noticed that updateHighestLayer() has a displeasing hardcoded number (it starts at 31, and looks for the first active layer it finds as it counts down, reading the bits of LayerState). I thought I’d try something out and have it start at the highest layer defined in the keymaps array (defined in Model01-Firmware.ino), making it slightly more efficient (most people won’t define nearly so many layers), and also eliminating that hardcoded number, but my lack of structural familiarity with C++ has me stuck. I tried creating a new variable in Model01-Firmware.ino, immediately following the definition of keymaps:

const uint8_t DefinedLayersCount PROGMEM = sizeof(keymaps) / sizeof(*keymaps);

…but when I tried to access it from layers.cpp, in nearly the same way keymaps is read from the same file, it didn’t work. Here’s one of my attempts:

uint8_t l = pgm_read_byte(&DefinedLayersCount);

That doesn’t work because DefinedLayersCount is out of scope. So I went looking for the header file (presumably) that defined keymaps in that scope, and almost immediately found the following in Kaleidoscope.h:

#define KEYMAP_SIZE (sizeof(keymaps) / ROWS / COLS / sizeof(Key))

…which I assumed must be good, but when I try to actually use that:

error: invalid application of 'sizeof' to incomplete type 'const Key [][4][16] {aka const Key_ [][4][16]}'

…so I figured it makes more sense at this point to ask an expert to tell me the correct way to access this data. How should I expose a byte storing the number of defined layers in the keymaps array?

You can add an extern uint8_t DefinedLayersCount line to layers.h, to make it available to layers.cpp. But herein lies a little problem, the reason why you weren’t able to use KEYMAP_SIZE in layers.cpp either: the keymap is defined in a different compilation unit than layers.cpp. When the Kaleidoscope library is compiled, the keymap variable does not exist yet. The compiler only knows that it will exist at some point, and can record a symbolic reference (so to say), but it has no idea about its size. It compiles the Kaleidoscope library and the sketch in two different steps. You can tell it that some symbol will be made available in a different compilation unit, but you won’t be able to work with it at compile time - only at runtime. Determining the size of the keymap at run-time is not currently possible. We have no way of knowing where it ends, and where some other data starts.

I know of no reasonable way to work around this limitation. It is technically possible to compile everything as one unit, but the Arduino tools do not support that.

…but I can determine it in the sketch, and store it in a variable in PROGMEM, just like the keymaps array itself. Why can’t I access that variable from layers.cpp, just like keymaps? Whether or not it’s worth trying to implement a change to updateHighestLayer(), how is layers.cpp getting access to keymaps?

Nevermind. I found it in key_events.h. Since I’m not trying to determine the keymap size at runtime, do you still see a problem with the idea?

Right, my bad. You should be able to access it via PROGMEM, similar to how keymap is accessed. Shouldn’t be problems there, once you mark it up the same way as keymap.

Mind you, we had something similar in the very beginning, but it was removed, because we didn’t want to force users to have to declare two magic variables in their sketches. (At least, if I remember correctly. I might be wrong.)

Yes, that’s the tradeoff. But this magic variable doesn’t require any editing; it’s always just the one line:

const uint8_t DefinedLayersCount PROGMEM = sizeof(keymaps) / sizeof(*keymaps);

(And I’m not totally sure why (or if) the PROGMEM declaration needs to be there).

As a side note, there’s another way to find the highest layer, by counting up instead of down, which I think is more efficient (assuming the typical case is that the highest active layer is lower than layer 16):

void Layer_::updateHighestLayer(void) {
  // First, shift LayerState by one bit so we get the number of the highest active layer, not the number of active layers
  uint32_t ls = LayerState >> 1;
  // Next, shift by one bit at a time, until all bits are zero to count the number of active layers (minus one, due to the bit shift above
  for (highestLayer = 0; ls != 0; highestLayer++) ls >>= 1;
}

This would not require the declaration of any magic variables, and if most shifts are between layers 0, 1, and 2, it would also require fewer iterations for more complex sketches that contain many layers. The obvious downside is that it’s much more obtuse.

1 Like

At a glance this does look more efficient. I’ll try to see if the code ends up being more efficient too. What makes me a bit sceptical is the use of a 32-bit integer, which are a bit costy on AVR (itself being a 8bit MCU). I hope this is more efficient, because this’d save us a few cycles if it were. =)

Well, my first idea only uses one 8-bit integer (albeit global), and should save some cycles, too. It also does a better job of removing that hard-coded value which needs to match a different hard-coded value defined elsewhere.

We could do something clever (always dangerous) and use just an 8-bit integer, and test the bytes of LayerState individually, since the first three are likely to be all zeros in most cases. I have no idea how that would affect speed, though.