plover icon indicating copy to clipboard operation
plover copied to clipboard

Update keymap on keymap change

Open user202729 opened this issue 3 years ago • 4 comments

Summary of changes

Listens for XMappingEvents and call _update_keymap whenever the keymap changes.

Closes #967. Might also fix #650.

Relies on the other pull request for self._display_lock.

Problem: for some reason, it refreshes the key map every time I use a different keyboard (in case I have more than one keyboard plugged in the machine -- which is usually the case with the built-in keyboard and a steno keyboard)

Pull Request Checklist

  • [ ] Changes have tests
  • [ ] News fragment added in news.d. See documentation for details

user202729 avatar Oct 27 '20 16:10 user202729

Sorry for the delay, will review later (maybe after 4.0.0 is released): when I investigated updating the layout on keymap change, I notice some issues due to the fact that the code does not handle XKB, which meant in my case I would get a lot of keymap changes because of the switch between the events sent by my physical keyboard, and the XTest keyboard (which use different layouts). I realize that my use case might be special (multiple physical keyboards with different layouts), so maybe that's not a valid reason for not updating on change. The alternative might be proper XKB support, but python-xlib does not support that (and adding support would be a lot of work).

benoit-pierre avatar Apr 01 '21 16:04 benoit-pierre

About xkb... I think that it is also the cause of https://github.com/openstenoproject/plover/issues/1189 so to fix that without xkb support this one is necessary.

Xref: Switching keyboards causes issues in other programs not supporting xkb too (https://github.com/openstenoproject/plover/issues/1030 )

user202729 avatar Apr 01 '21 23:04 user202729

Conflict, okay...

In retrospect, it isn't a very good idea to use inheritance all the time. In fact it's forbidden to override/define anything other than run and __init__ in a subclass of Thread, and in the future when/if output plugin is implemented, it's going to be a mess (with multiple inheritance, KeyboardEmulationBase and XEventLoop).

Edit: okay, thread inheritance is not my fault.

The @with_display_lock decorators are still very hard to place correctly.

user202729 avatar Apr 05 '21 16:04 user202729

Rebased to latest master.

Remarks:

  • Now (after the rebase) the KeyboardCapture object contains 2 display objects, one inside the event loop and one for (sending events|update keymap). The latter one needs a separate lock (just in case), which results in the code being duplicated from XEventLoop to KeyboardEmulation.

    (it may be a better idea to make 3 display objects instead so one lock is avoided... but this crucially relies on XEventLoop's implementation to all on_event functions sequentially...)

  • The event loop is started, but never canceled. (as before the rebase)

  • Now I see Benoit's pointed out problem that the keymap is refreshed on every stroke. Not good.

    Note that this only happen with the normal keyboard machine, not if the machine use a serial protocol. Personally, I use a hack that makes the keyboard float so this is not a problem either.

  • Crucially, it may happen that the keyboard mapping is different for different keyboard, and Plover sets the mapping for the real keyboard but send events using the xtest keyboard which leads to the problem being "unfixably broken" even after a Plover restart.

    In this case it would be necessary to exit Plover, setxkbmap us, reopen Plover.


in any case, some investigation is needed. pull request is not ready.


Actually, I think a possibility is to suppress the whole keyboard instead of just the subset of keys that are used for steno using xinput float; that way when a keyboard simulation is done then it does not switch back-and-forth the layout between the normal keyboard and the xtest keyboard. I use a little hack on my side to do that, and it works well.

user202729 avatar Jun 27 '23 07:06 user202729