890186 - [Bluetooth][HID] Keycode-to-charcode mapping for BT keyboard key events

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Bluetooth prototype

Comment 1

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

I have prototype for HID. It might need to clean up, but I tested with Logitech K760, use getevent to monitor key event, key event can be delievered.

See:
https://github.com/shawnjohnjr/mozilla-central/commit/b5ef9aa4cb3ac13af4e51473a1851ea41ff86f48

Comment 2

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

It seems Gaia IME does not monitor keyboard KeyEvent? So far we can only make sure key event was been delivered. Since bug 842927 has implemented patches for key event, maybe Gaia can use non-printable event.

Comment 3

[Ben Tian (inactive)]

Hi Masayuki,

We're working on typing with bluetooth keyboard on FxOS phone and currently we've connected the phone to the keyboard. However the tabs have no reaction to the key events due to lack of charCode, as struct UserInputData in event queue contains no charCode for nsAppShell to propagate out.

Do you have any suggestion to extract charCode from struct UserInputData? or we may need to implement a keyCode-to-charCode map in nsAppShell?

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I don't know the low layer of Android OS.

However, typically, IME should have mapping table from physical keycode to virtual keycode and input text.

So, I guess that nsAppShell should know which IME is selected and queries the input text to IME.

Or, we may design as that keyboard layout doesn't depend on IME. This is a little bit complicated for users, but physical keyboard layout doesn't depend on IME's implementation.

Comment 5

[Ben Tian (inactive)]

Attachment: Patch 1: v1: keycode-to-charcode mapping for BT keyboard

The attached patch implements keycode-to-charcode mapping for bluetooth keyboard key events. 

Changes:
- Add EventHub::mapChar() function to query char code from keyMap.
- Query char code based on key code and meta state in KeyboardInputMapper.
- Propagate char code to nsAppShell for dispatch of input key events
- Correct some typos in in widget/nsGUIEvent.h.

Masayuki, can you help review my patch?

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 782926 [details] [diff] [review]
Patch 1: v1: keycode-to-charcode mapping for BT keyboard

Sorry, I don't understand this patch because I don't familiar with Gonk/B2G design.

If the key-char map is included by IME and IME can have two or more mapping table, I believe that it's correct approach.

> @@ -118,16 +118,17 @@ struct UserInputData {
> +            char16_t charCode;

I'm not sure if this is "right" approach. nsKeyEvent.charCode is uint32_t which means the charCode can be stored UTF-32 character.

However, currently, at least on Windows and Mac, the charCode is always UTF-16 character. So, this might not cause any problems actually, though.

Additionally, could you separate the patch for nsGUIEvent.h because you don't change any logic related this bug. So, the commit summary for the changes of nsGUIEvent.h should indicate just modifying the comment.

Comment 7

[Ben Tian (inactive)]

> If the key-char map is included by IME and IME can have two or more mapping
> table, I believe that it's correct approach.
The key-char map is included in keyMap [1] and can be loaded from different .kcm files under system/usr/keychars.

> > @@ -118,16 +118,17 @@ struct UserInputData {
> > +            char16_t charCode;
> 
> I'm not sure if this is "right" approach. nsKeyEvent.charCode is uint32_t
> which means the charCode can be stored UTF-32 character.
> 
> However, currently, at least on Windows and Mac, the charCode is always
> UTF-16 character. So, this might not cause any problems actually, though.
I use char16_t as the keyMap returns char16_t char code [2].

> Additionally, could you separate the patch for nsGUIEvent.h because you
> don't change any logic related this bug. So, the commit summary for the
> changes of nsGUIEvent.h should indicate just modifying the comment.
Sure. I'll separate the patch.

[1] http://dxr.mozilla.org/mozilla-central/source/widget/gonk/libui/Keyboard.h#l48
[2] http://dxr.mozilla.org/mozilla-central/source/widget/gonk/libui/KeyCharacterMap.h#l73

Comment 8

[Ben Tian (inactive)]

Attachment: Patch 1: v2: keycode-to-charcode mapping for BT keyboard

The attached patch implements keycode-to-charcode mapping for bluetooth keyboard key events. 

Changes:
- Add EventHub::mapChar() function to query char code from keyMap.
- Query char code based on key code and meta state in KeyboardInputMapper.
- Propagate char code to nsAppShell for dispatch of input key events

:mwu, can you help review my patch?

Comment 9

[Ben Tian (inactive)]

Attachment: Patch 2: v1: Correct typos in widget/nsGUIEvent.h

Comment 10

[Kan-Ru Chen [:kanru] (UTC+9)]

Comment on attachment 783094 [details] [diff] [review]
Patch 2: v1: Correct typos in widget/nsGUIEvent.h

let's try a Widget peer.

Comment 11

[Michael Wu [:mwu]]

Comment on attachment 783087 [details] [diff] [review]
Patch 1: v2: keycode-to-charcode mapping for BT keyboard

Review of attachment 783087 [details] [diff] [review]:
-----------------------------------------------------------------

We avoid modifying the code in libui if at all possible. It looks like the new version of EventHub does what you need though - http://androidxref.com/4.3_r2.1/xref/frameworks/base/services/input/EventHub.h#300 . Can you update our copy of input code in libui to the 4.3 version and use the new API? I would normally do this but I'm leaving soon and won't be back until August 17. If you can't, I'll do it when I return.

Comment 12

[:gerard-majax]

I've tried those patches on master for HTC Desire Z, it works but the charCode send are wrong: we get small gray squares instead of characters.

Comment 13

[Michael Wu [:mwu]]

Attachment: Do keycode to charcode mapping

This is relatively straightforward, but I cleaned up a little bit of the key processing which makes the patch bigger. I also fixed the mapping of the delete and backspace keys.

Comment 14

[Michael Wu [:mwu]]

Comment on attachment 829406 [details] [diff] [review]
Do keycode to charcode mapping

Need to fix some things, will put up a new patch soon.

Comment 15

[Michael Wu [:mwu]]

Attachment: Do keycode to charcode mapping, v2

Comment 16

[Michael Wu [:mwu]]

https://hg.mozilla.org/integration/b2g-inbound/rev/e6ca4088799c

I didn't check in the typo fix since that patch doesn't apply anymore.

Comment 17

[:gerard-majax]

FYI I bought a bluetooth keyboard while waiting in SFO, it paired nice with my Desire Z but I could not get any input, while the physical keyboard of the device was working okay.

Comment 18

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e6ca4088799c

Comment 19

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Alexandre LISSY :gerard-majax from comment #17)
> FYI I bought a bluetooth keyboard while waiting in SFO, it paired nice with
> my Desire Z but I could not get any input, while the physical keyboard of
> the device was working okay.
Which BT keyboard do you use?

Comment 20

[:gerard-majax]

(In reply to Shawn Huang [:shuang] [:shawnjohnjr] from comment #19)
> (In reply to Alexandre LISSY :gerard-majax from comment #17)
> > FYI I bought a bluetooth keyboard while waiting in SFO, it paired nice with
> > my Desire Z but I could not get any input, while the physical keyboard of
> > the device was working okay.
> Which BT keyboard do you use?

I don't even see the events and the keyboard listed in the |getevent| output.

The device is http://www.verbatim.com/prod/accessories/keyboards/wireless-mobile-keyboard/, and on the box it claims to be working with Android >= 3.0.

Comment 21

[nguyenthanh.trung]

@Michael Wu [:mwu]: So by your comment in bug 907081. I checked and found the reason why key repeating does not work.
Please check https://hg.mozilla.org/mozilla-central/file/7b014f0f3b03/widget/gonk/libui/EventHub.cpp#l1214. The change seem comes from Android, but I don't know if Mozilla had already make change to handle key repeat manually somewhere.

Comment 22

[:gerard-majax]

(In reply to Shawn Huang [:shuang] [:shawnjohnjr] from comment #19)
> (In reply to Alexandre LISSY :gerard-majax from comment #17)
> > FYI I bought a bluetooth keyboard while waiting in SFO, it paired nice with
> > my Desire Z but I could not get any input, while the physical keyboard of
> > the device was working okay.
> Which BT keyboard do you use?

I checked and after pairing the bluetooth keyboard, there is no /dev/input/ entry created nor anything in /sys/class/bluetooth/hci0:*/, so I suspect there are some other missing bits somewhere.