Qmk_firmware: Layer tap keycode has to be less than 256

[skewwhiffy]

Hi,

See here for an example of this issue.

What I want is for layer tap buttons on the comma and the C keys which activate a symbols layer when held down. In addition, on the symbols layer, the comma key should produce a right bracket, and the C key should produce an asterix.

When I try to implement this in the obvious way (using LT(_SY, UK_ASTR)) on the symbols layer on the C key), I get an 8 when I try and get an asterix, and then the keyboard behaves in weird and wonderful ways.

Following through the code, I see that LT combines with bitwise OR QK_LAYER_TAP = 0x4000, the keycode for UK_ASTR and the layer number left shifted by 8. This issue arises because UK_ASTR is an alias for KC_ASTR, which is in turn an alias for LSFT(KC_8), which is 0x200 ORed with KC_8. This interferes with the layer number. In fact, there is a comment to this effect here: https://github.com/qmk/qmk_firmware/blob/master/quantum/quantum_keycodes.h#L592

I've worked around this by splitting the symbols layer into two, and thereby avoiding having a layer tap on the * button. It works, but it's messy.

Ideally, I'd like to remove the 256 limit, but I'm not sure that's going to be trivial or desirable.

Alternatively, I could probably use a custom keycode to implement this in process_record_user.

Any guidance on how to fix this properly would be good, and I can try a PR. Alternatively, any guidance on how to workaround this would be good: it seems a waste to use an extra layer to do this.

[drashna]

Yes, the keycode has to be a basic keycode. As in, in this list:
https://docs.qmk.fm/#/keycodes_basic

And yeah, fixing this issue is non-trivial, from what I can tell.

[skewwhiffy]

@drashna thanks.

What I was thinking was moving the layer numbers over a little bit, but I'm not sure what bits of code that I need to run to test that I haven't borked anything. I'm not overly familiar with C, but to my untrained eye, there doesn't seem to be any automated testing in this: is this correct?

If I find myself with time to look into that, I might give it a go. Otherwise, I'll try and provide a workaround in my keymap, and link to it in the comment in quantum_keycodes: that might be the more pragmatic approach to this.

[drashna]

I've tried tinkering with this myself, and it screws things up badly. I'm not an expert with C either, so I have no idea why. Somebody with more know-how would need to overhaul these functions.

[fauxpark]

This is another example of the limitations of the keycode structure in QMK. You can also see it in the mod-tap keycodes: https://github.com/qmk/qmk_firmware/issues/2440

Every keycode in QMK is 16 bits wide. In the case of LT(), it looks like:

| 0100 | llll | kkkk | kkkk |

In quantum_keycodes.c, QK_LAYER_TAP_MAX is defined as 0x4FFF, and the next entry, QK_TO is 0x5000, which indicates that the top four bits of a layer-tap must always be 0x4, otherwise it is not a layer-tap anymore. This limits the number of layers to 16 and the number of keycodes to 256 as the keycode portion must be 8 bits wide in order to represent at least the basic keycode set.

Unfortunately the workaround for using more advanced keycodes in a mod-tap or layer-tap etc. appears to be: don't. Solving this problem looks to be very non-trivial. A macro might be able to provide this functionality, I'm not sure.

[skewwhiffy]

I have to admit to being a little bit out of my depth here, not being able to encapsulate the whole keycode in an object of some sort, not even touching upon efficiency/code-size footprint issues that might be present in programming for a limited-resource system.

Is there a possibility of solving this in a similar way to something like the ASCII to Unicode transition, i.e. by setting llll|kkkk|kkkk to 1111 1111 1111 as a kind of place-holder for "wait for another keycode because the one I'm going to send you is too big"? Of course this results in significant extra code, and the possibility of breaking stuff without meaning to (I take it there's no unit test suite to help here, is there?).

Alternatively, is there a way of making this issue more obvious when it breaks, by, say, making LT() break when you try and send in a code that's too large? A large proportion of the user base for QMK aren't devs, and an even larger proportion aren't C devs, and it seems that a precompile hook or similar might be a sensible way to tell the user base about this limitation when they hit it.

In terms of an actual workaround, what's available in process_record_user method? Could you trigger a change of layer in there, or the sending of an asterix in there, and store state in the same file, for instance? If this seems possible to someone who knows this codebase better than I, then some pointers would be good for me to try and get a PR for a workaround in as a PoC.

[skewwhiffy]

@drashna Having looked deeper into the issue, perhaps the 'Good first issue' label isn't that appropriate? Just a suggestion :).

[drashna]

Sorry, yeah, it's definitely a more complicated issue. And actually, there was some discussion on the discord yesterday about this exact issue. And IIRC, there isn't a clear solution.

[fauxpark]

Unfortunately 0xFF is already in use as KC_MS_ACCEL2, and 0xF is a (potentially) valid layer. A more UTF8-y approach to keycodes would be cool and much more extensible, but again, it's not a simple task.

Keycodes can't be "chained" in the way I think you mean. C arrays (the keymap) do not have the ability to take variable-length elements. (at least, I don't think so)

The easiest workaround I can see is to use the numpad asterisk KC_PAST in place of KC_ASTR. A solution in process_record_user would probably involve timer_read() and timer_elapsed() to reproduce the layer tap behaviour. I don't know much about how to use those, though.

Also, the keycode portions of LT(), MT(), etc. should probably be & 0xFF so that the worst that can happen is you just get 8 instead of * and some wackiness.

[skewwhiffy]

@fauxpark Just for your information, I tried using KC_PAST, but seemingly that seems to suffer from the same reason. A little surprising, it doesn't look like it should.

Anyway: my current workaround with an extra layer works, so I'll stick with that.

Thanks for your help.

[fauxpark]

Hm, that doesn't make sense to me either. KC_PAST is an entirely different key, and it resolves to 0x55 so there shouldn't be any overflowing happening. It should be showing up in a keyboard tester as the numpad *. What's the full keycode you're using?

[skewwhiffy]

@fauxpark Yeah, because I'm an idiot...

I need to have a layer tap code on the asterix on the alternate layer (in fact, the whole point about this discussion!) so that I could escape from the alternate layer.

Anyway. quick, merge https://github.com/qmk/qmk_firmware/pull/3520 while it's working :)

Is this issue worth keeping open, or should I close it down? No solution, of course, but at least I have a better workaround than I had before this discussion.

[fauxpark]

Now that I see your keymap I'm even more confused. You have an LT to your symbols layer on your typing layers, but at that spot on the symbols layer is another LT... to the symbols layer. Whenever you have a layer switching keycode like LT or MO, the same key on the destination layer must be KC_TRANS so that QMK knows when the key is released. I don't think you'll be able to put an * there at all, numpad or no.

[skewwhiffy]

@fauxpark Admittedly, I haven't dug too deep into the code, but it looks like action_layer doesn't really care what the keycode is, just what the layer value is, so as long as that matches the layer value in the typing layer, all is fine.

Kind of backed up, seeing as the keymap works. Although I might well be leaking memory all over the place by doing this in a non-standard way :).

Anyway: it works, as is, so I'm happy.

[jackhumbert]

It looks like this was resolved - if you have any other issues with that implementation, feel free to comment here!