Skip to content

Conversation

IDisposable
Copy link
Contributor

This is the beginnings of the rework of the keyboard support.

Goals:

  1. Remove client/browser-side tracking of meta key state (Shift, Control, Alt, Super)
  2. Add device-side (go code) to track the meta key state and communicate it back to the client (for Virtual Keyboard layout tracking)
  3. Remove option to select LED tracking for latching key state (Caps Lock, Num Lock, Scroll Lock, Compose, Kana) since we have to trust the device
  4. Ensure the device tracks the LED state at all times
  5. Change the handling of keys in WebRTCVideo to use reportKeypress to pass all the keydown / keyup events so it can track the meta state and LED state
  6. Allow for multiple-key-down (up to the standard 6 key-rollover of HID keyboards)

@IDisposable
Copy link
Contributor Author

@adamshiervani looks like this is mostly complete to get the state managed device-side. There was a lot of work done to make the type-checking better because I had trouble working out some issues, so I just hardened up the code to get it to the point where the error was "obvious". Sadly in the end it was some odd interaction between Go's default serialization of uint8/byte values to JSON, which if given an array slice of uint8/byte will emit the slice as a base-64 encoded blob... on the typescript side, that doesn't get recognized as an array, so passing the keys-down wasn't working. In the end, I added a ByteSlice type on the Go side, and forced it to serialize as an array... sigh...

Now that I'm mostly done, might want to chat through if I did this the way we want. One thing I've realized just today is that if the client UI tries to make the new rpc calls to the device and it's not current.. well. Might have to restore the code I elided in commit 0c76634 and do some device-version detection?

@IDisposable IDisposable changed the title WIP: DO NOT REVIEW/MERGE WIP: DO NOT MERGE Rework keyboard management to allow device-side tracking of modifier states Aug 14, 2025
@IDisposable
Copy link
Contributor Author

Rebased and conflicts resolved

Remove LED sync source option and add keypress reporting while still working with devices that haven't been upgraded
We return the modifiers as the valid bitmask so that the VirtualKeyboard and InfoBar can represent the correct keys as down. This is important when we have strokes like Left-Control + Right-Control + Keypad-1 (used in switching KVMs and such).
Fix handling of modifier keys in client and also removed the extraneous resetKeyboardState.
Manage state to eliminate rerenders by judicious use of useMemo.
Centralized keyboard layout and localized display maps
Move keyboardOptions to useKeyboardLayouts
Added translations for display maps.
Add documentation on the legacy support.

Return the KeysDownState from keyboardReport
Clear out the hidErrorRollOver once sent to reset the keyboard to nothing down.
Handles the returned KeysDownState from keyboardReport
Now passes all logic through handleKeyPress.
If we get a state back from a keyboardReport, use it and also enable keypressReport because we now know it's an upgraded device.
Added exposition on isoCode management

Fix de-DE chars to reflect German E2 keyboard.
https://kbdlayout.info/kbdgre2/overview+virtualkeys

Ran go modernize
Morphs Interface{} to any
Ranges over SplitSeq and FieldSeq for iterating splits
Used min for end calculation remote_mount.Read
Used range 16 in wol.createMagicPacket
DID NOT apply the Omitempty cleanup.

Strong typed in the typescript realm.
Cleanup react state management to enable upgrading Zustand
@IDisposable IDisposable force-pushed the feature/keyboard-refactor branch from 942de9d to f903a6d Compare August 25, 2025 23:39
Copy link
Contributor

@adamshiervani adamshiervani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hell yeah, amazing work!!! Let's get this merged now.

@adamshiervani adamshiervani merged commit 3ec2432 into jetkvm:dev Aug 26, 2025
4 checks passed
@IDisposable IDisposable deleted the feature/keyboard-refactor branch August 26, 2025 17:59
@IDisposable
Copy link
Contributor Author

Whew...

@Caedis
Copy link
Contributor

Caedis commented Aug 28, 2025

Trying this out locally and seems like LMeta is not able to be held down. When I press and hold it down, the status bar only shows it pressed for a split second

@IDisposable
Copy link
Contributor Author

Trying this out locally and seems like LMeta is not able to be held down. When I press and hold it down, the status bar only shows it pressed for a split second

LMeta is pretty hard to manage because it pops up the OS menus when the browser/remote is on Windows and MacOS... mostly the only way I can get things to work with LMeta+something is to use Macros or Virtual Keyboard, but I would like to know more about how it acts on your system. Can you record a video or screen-share?

@Caedis
Copy link
Contributor

Caedis commented Aug 28, 2025

Before this pr, i was able to easily use Windows+Shift+S, but now I have to basically press all 3 at once and release quickly

Jetkvm connected to Windows
Browser is on Linux

@IDisposable
Copy link
Contributor Author

Before this pr, i was able to easily use Windows+Shift+S, but now I have to basically press all 3 at once and release quickly

Jetkvm connected to Windows Browser is on Linux

Trying to get to the bottom of this! Please bear with me...

Do you mean pressing keys on the physical keyboard?

What OS is the browser/client running on? I think you mean you've got

flowchart LR
    PC[Linux] -->|press physical key | LMeta
    LMeta --> Browser[fa:fa-browser Chrome]
    Browser -->|event| JetKVM[client]
    JetKVM -->|RPC| Device
    Device -->|HID| Host[Windows]
Loading

Also, are you keyboard trapped, fullscreen + running the JetKVM on an HTTPS (not HTTP)?

@IDisposable
Copy link
Contributor Author

Before this pr, i was able to easily use Windows+Shift+S, but now I have to basically press all 3 at once and release quickly

Oh, do you mean that you before could press-and-release LMeta, press-and-release Shift, press S and it would be seen as a Windows+Shift+S on the host?

If that's the case I suspect that's because the old keyboard stack wasn't processing things in order or synchronously (which broke all kinds of input things and basically had to be fixed to be in-order processing.

@Caedis
Copy link
Contributor

Caedis commented Aug 29, 2025

No, i could press shift, then meta, then s, Windows would register it, then I would release all 3.

Yes, pressing and holding the physical key would only show as a quick press and release of the LMeta key.

I have 0 issues holding down LMeta in dwm or hyprland to move stuff around or executing other shortcuts that use LMeta.

@Caedis
Copy link
Contributor

Caedis commented Aug 29, 2025

Non-fullscreen and absolute mouse so not keyboard trapped. Wish that worked but it doesnt even with an https reverse proxy with an LE cert.

@Caedis
Copy link
Contributor

Caedis commented Aug 29, 2025

Client = Arch in Firefox
Host/Jetkvm is attached to Windows.

I have 0 shortcuts set to LMeta+Shift+S

@Caedis
Copy link
Contributor

Caedis commented Aug 29, 2025

Preview:

20250828_235857.mp4

@IDisposable
Copy link
Contributor Author

Great repro, thanks...will look into that ASAP

@alexballas
Copy link
Contributor

One thing I noticed:
Over https://app.jetkvm.com/ Holding LeftShift down and then pressing MetaLeft doesn't even trigger this momentary "MetaLeft" press. Nothing appears.

Over IP (without HTTPS) I do see this momentary press

@alexballas
Copy link
Contributor

      if (!isKeyboardLockActive && hidKey === keys.MetaLeft) {
        // If the left meta key was just pressed and we're not keyboard locked
        // we'll never see the keyup event because the browser is going to lose
        // focus so set a deferred keyup after a short delay
        setTimeout(() => {
          console.debug(`Forcing the left meta key release`);
          handleKeyPress(hidKey, false);
        }, 100);
      }

this part here is the one causing the momentary MetaLeft press

@Caedis
Copy link
Contributor

Caedis commented Aug 29, 2025

Nice find

@alexballas
Copy link
Contributor

alexballas commented Aug 29, 2025

On Linux when I press Alt key and change the focus of the window without releasing it, the browser will force release it. So I dont know if the following statement is correct.

        // If the left meta key was just pressed and we're not keyboard locked
        // we'll never see the keyup event because the browser is going to lose
        // focus so set a deferred keyup after a short delay

One for @IDisposable to confirm

@Caedis
Copy link
Contributor

Caedis commented Aug 29, 2025

Commented out that 2 blocks and deployed and everything works for me
(meta and shift being on at the start was due to my shortcut to record a selection being Meta + Shift + PrintSc)

20250829_083600.mp4


u.log.Trace().Interface("old", u.keysDownState).Interface("new", state).Msg("keysDownState updated")
u.keysDownState = state

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On review post folks beginning to test, I think I should have released the Lock here to ensure we're not holding through the entire callback for onKeysDownChange. Thoughts @adamshiervani ?

@IDisposable
Copy link
Contributor Author

Over https://app.jetkvm.com/ Holding LeftShift down and then pressing MetaLeft doesn't even trigger this momentary "MetaLeft" press. Nothing appears.

As far as I know, the app.jetkvm.com has not been updated yet since this isn't fully released. Thus it is still using the ReportKeys (old stack/Macros) instead of the ReportKeyPress (new stack)

      if (!isKeyboardLockActive && hidKey === keys.MetaLeft) {
        // If the left meta key was just pressed and we're not keyboard locked
        // we'll never see the keyup event because the browser is going to lose
        // focus so set a deferred keyup after a short delay
        setTimeout(() => {
          console.debug(`Forcing the left meta key release`);
          handleKeyPress(hidKey, false);
        }, 100);
      }

this part here is the one causing the momentary MetaLeft press

Correct, not sure how to handle it now, but I certainly can remove that code in favor of a lazy timer. The comment remains true, though, because if someone is not in fullscreen on an HTTPS connection, we can't grab a keyboard lock, and thus the LMeta kicks in for the browser's OS (and thus we would never see the key up event. This code (in a different form existed before) see https://github.com/jetkvm/kvm/pull/725/files#diff-503edceb49a3a14452eb4663ab40040a5ff04a2a89f13e48be02539e2c939f08L441-L450

I will try to find a better way that doesn't force the key-up event unless we've lost focus and some interval passes... but that's going to be fun because it's possible for the key state to get out of sync with the device and host... so have to protect that.

@IDisposable
Copy link
Contributor Author

On Linux when I press Alt key and change the focus of the window without releasing it, the browser will force release it. So I dont know if the following statement is correct.

        // If the left meta key was just pressed and we're not keyboard locked
        // we'll never see the keyup event because the browser is going to lose
        // focus so set a deferred keyup after a short delay

One for @IDisposable to confirm

Interesting... maybe I should just not special-case the LMeta at all...

@IDisposable
Copy link
Contributor Author

Over https://app.jetkvm.com/ Holding LeftShift down and then pressing MetaLeft doesn't even trigger this momentary "MetaLeft" press. Nothing appears.

As far as I know, the code on app.jetkvm.com is the old stack still (since 0.5 hasn't actually released).

To get the new stack for now, you will have to be running against the device directly (via IP or your local DNS name)

In fact I had to rework some stuff in this PR to support the opposite case (when app has been upgraded but the device hasn't)... wheeee

@IDisposable
Copy link
Contributor Author

Commented out that 2 blocks and deployed and everything works for me (meta and shift being on at the start was due to my shortcut to record a selection being Meta + Shift + PrintSc)

Awesome, thanks for confirming... I'm all in favor of just not special casing things :)

What's the "other" block you deleted?

@Caedis
Copy link
Contributor

Caedis commented Aug 30, 2025

image

ym pushed a commit to ym/jetkvm-kvm that referenced this pull request Sep 26, 2025
Remove LED sync source option and add keypress reporting while still working with devices that haven't been upgraded
We return the modifiers as the valid bitmask so that the VirtualKeyboard and InfoBar can represent the correct keys as down. This is important when we have strokes like Left-Control + Right-Control + Keypad-1 (used in switching KVMs and such).
Fix handling of modifier keys in client and also removed the extraneous resetKeyboardState.
Manage state to eliminate rerenders by judicious use of useMemo.
Centralized keyboard layout and localized display maps
Move keyboardOptions to useKeyboardLayouts
Added translations for display maps.
Add documentation on the legacy support.

Return the KeysDownState from keyboardReport
Clear out the hidErrorRollOver once sent to reset the keyboard to nothing down.
Handles the returned KeysDownState from keyboardReport
Now passes all logic through handleKeyPress.
If we get a state back from a keyboardReport, use it and also enable keypressReport because we now know it's an upgraded device.
Added exposition on isoCode management

Fix de-DE chars to reflect German E2 keyboard.
https://kbdlayout.info/kbdgre2/overview+virtualkeys

Ran go modernize
Morphs Interface{} to any
Ranges over SplitSeq and FieldSeq for iterating splits
Used min for end calculation remote_mount.Read
Used range 16 in wol.createMagicPacket
DID NOT apply the Omitempty cleanup.

Strong typed in the typescript realm.
Cleanup react state management to enable upgrading Zustand
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants