Bug 31340

Did you forget to attach the patch?

Apperantly. I will upload it now. Please review the patch before applying. Thanks.

Created attachment 40143 [details] [review]
Patch for review, see also line with TODO

Review of attachment 40143 [details] [review]:

The patch changes the final files in rules subdirectory. Instead, the original files should be changed (base.xml.in, base.m_g.part)

The name convention for the model is awkward. Instead of sanwa_skb-kg3 I would use sanwaskbkg3 (see http://www.freedesktop.org/wiki/Software/XKeyboardConfig/Rules )

About your TODO, KP_Separator looks like a logical choice. But I cannot tell for sure. Could you try xev utility? What keysym does it produce when you press that key?

(In reply to comment #4)
> Review of attachment 40143 [details] [review]:
> 
> The patch changes the final files in rules subdirectory. Instead, the original
> files should be changed (base.xml.in, base.m_g.part)

Ah, where do I find these? In the source files? Which URL to get for a proper patch?

> The name convention for the model is awkward. Instead of sanwa_skb-kg3 I would
> use sanwaskbkg3 (see
> http://www.freedesktop.org/wiki/Software/XKeyboardConfig/Rules )

OK, I will change this.

> Ah, where do I find these? In the source files? Which URL to get for a proper
> patch?
Just pick the xkeyboard-config source code. For example, here: http://www.x.org/releases/individual/data/xkeyboard-config/xkeyboard-config-2.0.tar.bz2

Or in git. See http://www.freedesktop.org/wiki/Software/XKeyboardConfig/Development

(In reply to comment #5)
> About your TODO, KP_Separator looks like a logical choice. But I cannot tell
> for sure. Could you try xev utility? What keysym does it produce when you press
> that key?

KeyPress event, serial 33, synthetic NO, window 0x3e00001,
    root 0x112, subw 0x0, time 89217272, (855,482), root:(856,540),
    state 0x0, keycode 91 (keysym 0xff9f, KP_Delete), same_screen YES,
    XLookupString gives 0 bytes: 
    XmbLookupString gives 0 bytes: 
    XFilterEvent returns: False

KeyRelease event, serial 33, synthetic NO, window 0x3e00001,
    root 0x112, subw 0x0, time 89217336, (855,482), root:(856,540),
    state 0x0, keycode 91 (keysym 0xff9f, KP_Delete), same_screen YES,
    XLookupString gives 0 bytes: 
    XFilterEvent returns: False

But it should not be KP_Delete. When you search the source files, there are
some comments on JPCM, that it should be used for evdev. But evdev is
deprecated now? So that is not the one to use.

See the URL in this bug report to view the keyboard photograph for the key.
Probably KP_Separator is correct for this Japanese centered dot, but I'm not an
expert. Can some expert on Japanese keyboards in this community advise on this?

Created attachment 40147 [details] [review]
Patch 0.4 against source

I have improved the patch a lot, since now it is against the source files. Please review it again.

The last patch is REALLY much better!

Since you provided symbols/jp(sanwaskbkg3), why don't you just map keycode 91 to whatever you think is right? If someone is unhappy, we can always fix it.

Evdev is far from being deprecated, to the best of my knowledge.

Created attachment 40151 [details] [review]
newer version, all descriptions are now "Sanwa Supply Inc. Small Keyboard KG3"

Thanks. Please review also the latest patch.

Do you have experience with launchpad PPA to build version 1.8-1ubuntu8 with this patch in order to do a proper test?

Good. One more question. Your mapping, other than a series of "include" lines, is mostly about keypad mapping. What's so special about keypad of that keyboard?

May be, one day I will put xk-c into my PPA.

(In reply to comment #14)
> Good. One more question. Your mapping, other than a series of "include" lines,
> is mostly about keypad mapping. What's so special about keypad of that
> keyboard?

keypad(overlay1) and keypad(overlay2) map [ KP_Left, KP_6 ] on KP6, I only wnat
KP_6 to be mapped on KP6. I could not find any other existing keypad that
matches. You perhaps?

(In reply to comment #15)
> May be, one day I will put xk-c into my PPA.

I'm going to try patching on Ubuntu source and upload it to my PPA.

> keypad(overlay1) and keypad(overlay2) map [ KP_Left, KP_6 ] on KP6, I only wnat
> KP_6 to be mapped on KP6. I could not find any other existing keypad that
> matches. You perhaps?
You're right. For modularity purposes, would it make sense to add new section 'nonav' to symbols/keypad and including it in symbols/jp? There is nothing Japan-specific in your keypad mapping IMHO - may be, other ppl could find it useful as well.

(In reply to comment #18)
> > keypad(overlay1) and keypad(overlay2) map [ KP_Left, KP_6 ] on KP6, I only wnat
> > KP_6 to be mapped on KP6. I could not find any other existing keypad that
> > matches. You perhaps?
> You're right. For modularity purposes, would it make sense to add new section
> 'nonav' to symbols/keypad and including it in symbols/jp? There is nothing
> Japan-specific in your keypad mapping IMHO - may be, other ppl could find it
> useful as well.

See patch 0.6 for extra keypad. At this moment I'm stuck. One way or the other gnome-keyboard-settings is unable to offer this new keyboard. Before it was possible. Can you find any problems with it?

Created attachment 40156 [details] [review]
separate keypad definition, please review

(In reply to comment #20)
> Created an attachment (id=40156) [details]
> separate keypad definition, please review

Now I'm using only "sanwaskbkg3" and "Sanwa Supply SKB-KG3" as geometry, model, variant and symbol definitions. Perhaps in one of these settings something is wrong since gnome-keyboard-properties is unable to offer the keyboard defined in this new version :S

Created attachment 40159 [details] [review]
fixed small typos, see also comments on previous patches

Created attachment 40162 [details] [review]
Patch on Ubuntu installed package: please help with fix

Created attachment 40163 [details] [review]
Patch on source: please review

Sorry for the many patches but this is until where I can get. One patch on the source (please review) and one patch for installed Ubuntu Maverick package (please help with fix). This worked before but not any more :(

I have split geometry (sanwaskbkg3) and symbols (kana86) but gnome-keyboard-properties has problems with this:

This happens when I add the kana86 layout or the sanwaskbkg3 keyboard:


Error activating XKB configuration.
It can happen under various circumstances:
 • a bug in libxklavier library
 • a bug in X server (xkbcomp, xmodmap utilities)
 • X server with incompatible libxkbfile implementation

X server version data:
The X.Org Foundation
10900000

If you report this situation as a bug, please include:
 • The result of xprop -root | grep XKB
 • The result of gconftool-2 -R /desktop/gnome/peripherals/keyboard/kbd


In /var/log/Xorg.0.log I find these lines:

[  2436.163] (EE) Error loading keymap /var/lib/xkb/server-F58B9650039D7539235025B20D25D21DB4B4A5AB.xkm

Your last patch had some typos: incluce instead of include. Fixed. Committed. Please check in git.