kiibohd / controller

Kiibohd Controller
GNU General Public License v3.0
806 stars 270 forks source link

Not firing key-up when layer shift is released out of order. #10

Closed mythmon closed 9 years ago

mythmon commented 9 years ago

I have a key that triggers layerShift( 1 ). On the associated layer, I have a key to press the left arrow key. If I hold the layerShift key, then hold the left arrow key, then release the layerShift key, and then release the left arrow key, the left arrow keyup event is not sent, and the key repeats until any other key is pressed.

I'm using an Infinity keyboard.

haata commented 9 years ago

Interesting. I believe I've run into the same problem as well (though I wasn't quite sure how it happened).

What would you think the correct behaviour would be?

I'm sort've leaning towards sending all release codes for anything that is being triggered by that layer. I'll have to check how easy it is to do this.

tmk commented 9 years ago

It is exaclty what my firmware does. It unregisters all of hold down keys every time changing layers. But except for modifiers, keeping modifier state is useful for most case. So modifiers can be still stuck in unusual keymap in my firmware. Though, this behaviour rarely bother users.

mythmon commented 9 years ago

To be concrete, the key that has the left arrow is H normally. So if I pressed Fn+H, I'd get left arrow. I'd expect that if I released Fn, the left arrow would still be repeated until I released H. I could also live with sending the key up when Fn was released. As long as the key release gets sent when one of the keys is released, I think the bug would be fixed.

cucook commented 9 years ago

The HHKB works in the same manner that @mythmon describes, where the key-up for the layer-shifted button is only sent when that physical key is released, not when the layer is released.

haata commented 9 years ago

I've had a chance to think about this for a few days, and I think I have a good way to fix this now.

When a key is pressed, there is a "pending keys" buffer. This is used to process all possible key input events (just think of this like a giant regex). Right now, on each processing loop the layer is checked right before evaluating a step in the regex. The planned change is to evaluate and track the layer when each key is "pressed" in the "pending keys" buffer.

This way the layer stays constant, even if it has been unset.

Pending other issues, I'll try to get a fix in for this in the next week or so.

Duckle29 commented 9 years ago

I didn't quite understand that last comment of yours haata :/ Will this mean my delete key wont keep deleting the rest of my document if I let go of the fn key before I let go of the delete key? :P

If you want to keep this behaviour for anyone making a feature out of it, maybe you could have a switch per layer that can be set to true or false, as in "KEY_UP all keys on layer exit?

haata commented 9 years ago

Yeah, I think the current behaviour is a bug.

However, "locking" a key might be useful in some cases. So I'm going to add a feature like this in future revisions of the kll spec.

haata commented 9 years ago

I nearly have a fix written. Should have at least a feature branch to test this evening.

haata commented 9 years ago

Fixed: f07e9342ddebebc1ca78033e86f132dbf1f89cd7

Because it made the most sense, I am doing what the HHKB does and assign a key to layer it was first pressed on. Any layer changes that happen while the key is held will have no effect until the key is released.

Let me know if there are still any issues.