UltimateHackingKeyboard / firmware

Ultimate Hacking Keyboard firmware
Other
420 stars 66 forks source link

Configuration parser #3

Closed mondalaci closed 7 years ago

mondalaci commented 8 years ago

The configuration parser is akin to the configuration serializer but it:

  1. is implemented in the firmware
  2. only reads the configuration
  3. interacts with the various parts of the firmware to actualize relevant data structures

It'd be practical to proceed in a bottom-top manner when implementing the parser. What I mean by that is that first the parser could only parse a single layer, then a keymap, then multiple keymaps, then the root of the configuration. This would allow to gradually extend it while having something working all the time.

mondalaci commented 8 years ago

Related to #2.

algernon commented 7 years ago

I have an alternate proposal: with the keyboard layout done as in #12, if the configuration would have the layout part serialized into the same format, we'd be able to just copy it to RAM. Macros would be a bit harder, because their size is not known in advance, but I have a couple of ideas for that, too.

If we could supply an EEPROM image, and store it as-is, then the parser would not even need to exist. Some initialization functions would need to know where in EEPROM to find some things, but that's about it. Of course, the image would have a header that describes what version the image is for, so the firmware can opt to discard it, if it is found incompatible. Or apply some transformations, if possible, to upgrade an old EEPROM image, etc.

So, in short, my proposal looks like this:

typedef struct {
  uint32_t magic; // "UHK\0"
  uint16_t imageVersion;
  uhk_key_t layout[LAYOUT_KEY_COUNT][LAYOUT_MOD_COUNT];
  macro_t macros[0];
} eeprom_layout_t;

...or something along these lines. This is easy to serialize into, and even easier to use in the firmware.

mondalaci commented 7 years ago

I wish this could be done as simply as you're suggesting. The reason it can't be done this way is that the length of different key action types differ. This is to optimize the storage space of the key actions in the EEPROM.

But still, the final implementation of the layer parser shouldn't be overly complicated. It'd make sense to create 2 structs for every key action type: one for retrieving the members of the key actions from the serialized configuration, and the other struct (that actually resides within a key action union as you properly implemented in the other pull request) to store them in the RAM. The layer parser would iterate the key actions of the layer one by one and copy the related members from the serialized configuration to the RAM on a key action basis.

algernon commented 7 years ago

Do action lengths have to differ? What I had in mind, was to have a keymap where all keys would be of the same 16-bit size. The flags or type (or whatever we end up calling it) part would have a, say UHK_MACRO flag, in which case the value member would be an index. We'd then store the macros elsewhere, and use this index to look them up. Something similar can be done for layer keys, mouse keys, dual-use/long-press keys, and so on. With this setup, everything can be serialized into a format that is directly usable by the firmware.

While not the most efficient format, the keymap, assuming 70 keys, on 16 bits, and 4 layers is merely 560 bytes. If you add macros, that would be about 3-4 bytes / action in addition to the above. Having an entire keyboard, with all layers, made up entirely of macros of, say, 24 actions in length (and assuming a 4 byte / action size), we are looking at 560 + (70 4 24 * 4) = 27440 bytes and some more for header and whatnot, still well within EEPROM limits (iirc there's 32k of that?).

Thinking further:

typedef struct {
  uint32_t magic;
  uint16_t imageVersion;
  uhk_key_t layout[LAYOUT_KEY_COUNT][LAYOUT_MOD_COUNT];
  uint8_t macroCount;
  uint32_t *macroIndexes; // an array of macroCount size
} eeprom_header_t;

And then we'd have the macros, just laid out in an array, and macroIndexes would be offsets from the end of the header to the start of the macro. The same structure can be used for representing macros in RAM too.

Mouse keys, dual-use/long-press mod/layer keys, layer & keymap switching fit entirely within the 16-bit key thing, so as far as I see, only macros need special treatment.

algernon commented 7 years ago

TBH, I'll just whip up a PoC, once #12 is finalised. But for the record, a similar solution works fine on my ErgoDox EZ (without the macro bits, because it only has 1k EEPROM), and supports mouse-keys, dual-use/long-press stuff, layer toggles and momentary switching, leader keys, tap-dance, one-shots, and so on. All representable within a keymap made up of 16-bit keycodes.

mondalaci commented 7 years ago

Yes, unfortunately key action lengths do have to differ because only this way we can store key actions the most efficiently in the EEPROM.

For example, NoneAction occupies merely 1 byte, but a KeystrokeAction can occupy as much as 4 bytes if it has a scancode, modifiers, and long press action. Admittedly, the current scheme does introduce additional complexity, but it's the price of optimizing for storage space.

When putting into the RAM, the flag of the keystroke action should be denormalized to the keystroke action type, then whatever fields the action has, they should be put into the relevant fields of the action. This would maximize lookup speed because of the explicit type value. Given that the keystroke action is the longest action, and if we use unions, I think a key action should occupy 4 bytes of RAM. This isn't a problem because RAM is plentiful and we should optimize for lookup speed.

I can see how your 16 bit storage scheme would work for macros, but not for keystroke actions because of their maximum 4 byte size. Indirection makes sense for macros but not so much for key actions.

If a macro has 24 actions that doesn't mean that every key action occupies 24 bytes because macros are stored elsewhere. Play macro actions point to macros so their length is not affected by macros.

How about having a Hangouts call? I really think that this is a deep topic and it'd be more productive to talk this through. :)

algernon commented 7 years ago

For example, NoneAction occupies merely 1 byte, but a KeystrokeAction can occupy as much as 4 bytes if it has a scancode, modifiers, and long press action.

Mhm, I see wherein lies the difference in our thinking. Long press action + modifiers (by which I think we both mean a combo like Ctrl/Alt+Shit+Z) is something I have not seen in any firmware yet, hence my scheme not supporting it either. But with a little tweak, it can be turned into a long press action that triggers a macro when not doing the modifier thing, and still fit in 16-bits, and the macro part can go to macro storage, and voila!, we can still use the keymap as-is.

Mind you, if you're convinced about the compact format, no biggie, the parser for that won't be a biggie, either.

This would maximize lookup speed because of the explicit type value.

I'm not 100% sure I fully understand what you mean in this paragraph, but as far as I remember, flags & MASK is pretty darn fast too, if the width of the flag variable is smaller or equal to the native int width of the CPU. Mind you, it's been a while I looked at the asm generated for flags & MASK vs a switch or similar.

Admittedly, the current scheme does introduce additional complexity, but it's the price of optimizing for storage space.

One of my points is, that there's plenty of space, and it may not be worth optimizing for compactness, and optimizing for ease of use may be more beneficial.

If a macro has 24 actions that doesn't mean that every key action occupies 24 bytes because macros are stored elsewhere. Play macro actions point to macros so their length is not affected by macros.

Yeah, I know. I was just trying for an example that's easy to calculate with. Could have said average length of 24, though, would have been clearer.

How about having a Hangouts call? I really think that this is a deep topic and it'd be more productive to talk this through. :)

Good idea!

algernon commented 7 years ago

Hrm. Had a look at what the agent supports - yeah, 16 bits won't be enough for that, not without too many hacks. Plan B!

mondalaci commented 7 years ago

Almost the whole configuration parser has been implemented in the meantime, so I'm closing this issue.