Single-byte key addressing (for boards with fewer than 256 keys)

(Michael Richters) #1

I’ve been doing an experiment to see how much difference it would make to switch from row,col key addressing to using just a single integer to index each key. So, I made changes in a whole lot of modules, but mostly Kaleidoscope-Hardware-Model01 and Kaleidoscope itself. I built a slightly-customized version of the default firmware without AlphaSquare and TestMode, but with Qukeys. Here’s what the builder reports for the program without my “keyaddr” changes:

BOARD_HARDWARE_PATH="/Users/merlin/Documents/Arduino/hardware" /Users/merlin/Documents/Arduino/hardware/keyboardio/avr/libraries/Kaleidoscope/bin//kaleidoscope-builder build-all
Building output/Model01-Firmware/Model01-Firmware (0.0.0-gv1.22-2-gedd0) ...
- Size: firmware/Model01-Firmware/Model01-Firmware-0.0.0-gv1.22-2-gedd0.elf
  - Program:   18330 bytes (63.9% Full)
  - Data:       1716 bytes (67.0% Full)

…and here’s the same sketch with single-byte addresses:

BOARD_HARDWARE_PATH="/Users/merlin/Documents/Arduino/hardware" /Users/merlin/Documents/Arduino/hardware/keyboardio/avr/libraries/Kaleidoscope/bin//kaleidoscope-builder build-all
Building output/Model01-Firmware/Model01-Firmware (0.0.0-gv1.22-2-gedd0) ...
- Size: firmware/Model01-Firmware/Model01-Firmware-0.0.0-gv1.22-2-gedd0.elf
  - Program:   17796 bytes (62.1% Full)
  - Data:       1649 bytes (64.4% Full)

That’s 534 bytes smaller in PROGMEM, and 67 fewer bytes in RAM (at least, to start with). It’s only about 3% smaller, but that might make a significant difference in some cases.

Now, it’s not all working quite properly (I didn’t quite get the conversion of the LED matrix correct, so the wrong keys light up in some modes, and I introduced at least one bug in NumPad), but it is mostly working (typing, MouseKeys). For most plugins, the conversion was surprisingly easy to do.

The real downside here is that it’s very difficult to make this change backwards-compatible, because it changes the signature of the eventHandlerHook() functions for plugins. I think it can be done, but it may require some trickery to get a version of Kaleidoscope that works with both KeyAddr and (row,col) plugins – probably a new transitional hook point.

Kaleidoscope's implementation - possible improvements
(Michael Richters) #2

I’ll submit a PR (or two, or three…) tomorrow morning so people can see what I’ve done.

(Noseglasses) #3

I am interested in your PR(s). Would you please link them here once you have submitted them?

(Michael Richters) #4

Will do. I’ll probably go ahead and make an Arduino-Boards fork (if I haven’t already) to collect all the changes for people to review.

(Noseglasses) #5

Hope this doesn’t come too late.
If you haven’t done so already. What about using a typedef type for the key position id? That way it could have any size and conditionally (through a prepro macro) also support keyboards with more than 256 keys.

(Michael Richters) #6

That’s exactly what I did:

typedef uint8_t KeyAddr;

So if there’s a board with more than 255 keys, it can be changed in just one place.

(Michael Richters) #7

By the way, 255 is the limit for a single byte because we need one reserved for “not a real keyswitch”.

(Noseglasses) #8

… of course a POD class like the key class would be even more convenient.

(Michael Richters) #9

I thought about it, but I wasn’t sure about how to do it properly. I find the C++ compiler confounding enough, and I could pattern-match a working typedef more easily. I also didn’t know what overhead would come with a class or struct.

It would be pretty easy to change, though.

Do accessors increase PROGMEM consumption?
(Michael Richters) #10

I would be very happy to get help in doing this better, by the way – before last month, I think the previous C++ program I wrote was in 1995.

(Noseglasses) #11

There is no extra overhead. The class could have (inline) getters and setters for row and column values, which would encapsule the bit manipulations. You probably couldn’t distinguish both implementations in the assembly code.

(Michael Richters) #12

What I was wondering about was use with operators, mostly.

(Noseglasses) #13

You seem to progress fast, though. :+1:t5:

(Noseglasses) #14

Could you state this more precisely?

(Michael Richters) #15

I want to be able to do this:

for (KeyAddr k = 0; k < TOTAL_KEYS; k++) {
    . . .

and this:

if (key_addr < TOTAL_KEYS) {...

…et cetera, without setters and getters. Also casting other integer types.

(Michael Richters) #16

I’m up way too late already, but at @algernon’s request, I pushed the branches to the two most important parts of this change for people to check out while I’m sleeping, if they want:

I touched a whole bunch of other ones, too, but these have the really substantive changes. I’ll post again when I’ve got the whole set together tomorrow morning.

(Noseglasses) #17

Sounds like a perfect application scenario of an overloaded < operator. Check out the Key_ class to see how it can be done.

(Noseglasses) #18

And don’t be confused that Key_ is a union. Unions are just restricted classes in C++.

(Michael Richters) #19

I did think of operator overloading, but I wasn’t sure if that stayed in the compiler or got added to the program, and I wasn’t sure if it was worth it. It was also quicker for me to do this way.

You could easily convince me to change it, though. Thanks for all the help. (Seriously, I’m moving on to testing out your interface changes next, and planning to build on that).

(Michael Richters) #20

Oh, don’t get me started on the confusion caused by union, class, struct, and typedef

And especially how all of those things are different in C vs C++.