Bug 34738

Summary: Merge OLPC "mechanical" keyboard maps (us and es)
Product: xkeyboard-config Reporter: Martin Langhoff <martin>
Component: GeneralAssignee: xkb
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: dan, walter
Version: unspecifiedKeywords: NEEDINFO
Hardware: Other   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: Revised patch that address comment 23
rebased patch against current master
fix AE00 alias

Comment 1 Sergey V. Udaltsov 2011-02-25 15:33:14 UTC
1st patch: why do you need separate line for us, it is already in $olpclayouts
Comment 2 Sergey V. Udaltsov 2011-02-25 15:34:45 UTC
2nd patch: the part for base.ml_s.part looks strange. Isn't it already there?
Comment 3 Martin Langhoff 2011-02-27 09:19:53 UTC
Patch 1 - if you tell me that it's equivalent, we drop it. I'm not familiar with the syntax and the (%) variable expansion rules.

Patch 2 - you're right, we're duplicating a block in base.ml_s.part. Oops.

Are the other bits ok to merge? Do you want me to rebase it myself or will you do a commit --amend or commit with the good paths?

(Thanks for the review and sanity-check!)
Comment 4 Sergey V. Udaltsov 2011-02-27 12:15:01 UTC
Ok, lets drop #1.

About #2. I think it would make sense to merge keycodes into keycodes/evdev (default section evdev). I do not see any conflict there - it is pure expansion. That way, you do not have to put new keycodes into the rules. Would you try that?
Comment 5 Martin Langhoff 2011-03-02 08:42:48 UTC
Sergey -- can you look at my notes in #34732 about olpcm as a model? Does your recommendation on moving the keycodes to evdev still stand?
Comment 6 Martin Langhoff 2011-03-02 10:07:41 UTC
Sergey - going back to your doubts about base.ml_s.part -- the new entries are for olpcm.

They are similar to the "olpc" lines but they are for a different XKV_MODEL.

(I don't know about parsing/matching rules, perhaps they do get matched as being the same.)
Comment 7 Martin Langhoff 2011-03-02 10:08:29 UTC
BTW, I've misrepresented things before. "olpcm" is a model, not a variant.
Comment 8 Sergey V. Udaltsov 2011-03-02 12:17:44 UTC
I am happy to create olpcm model. But I still think that keycodes could be merged.
Comment 9 Martin Langhoff 2011-03-03 08:21:00 UTC
So there's a bit of discussion in #34732 about the reasons behind XKB_MODEL=olpcm

I summarize... OLPC ships 2 physical keyboards: 

 - membrane keyboards, XKV_MODEL=olpc -- pictures and links at
http://wiki.laptop.org/go/Keyboards_and_key_bindings

 - mechanical keyboard, XKV_MODEL=olpcm -- pictures at
http://blog.laptop.org/2010/07/24/xo-1-5-hackably-sweet/

About how users select the model & variant -- they don't. We have an init script that sets all that based on data read from the BIOS.

See code at 
http://dev.laptop.org/git/projects/olpc-utils/tree/etc/rc.d/init.d/olpc-configure#n160

Some more info about the data recorded in the BIOS - and how it drives keyboard
settings http://wiki.laptop.org/go/Manufacturing_data#Keyboards

Advanced users can perhaps set their XKB variables, but those are not the users
we work hard for... :-)

Note: some people play with "vanilla" distros on our hardware. Those might benefit from having the model defined.
Comment 10 Martin Langhoff 2011-03-03 08:23:24 UTC
Here I am open to adding a model (as our patch does) or moving codes to evdev. Don't know which one is more appropriate. 

OLPC may add more keyboard models to our lineup -- we have some prototypes that are different HW, and one or more may enter production. The reason behind them is improved resistence to use/abuse. We want them to survive ~5 years in the hands of real (and rough!) children.
Comment 11 Sergey V. Udaltsov 2011-03-03 12:45:27 UTC
Creating the model is right, it is fine! But that model would use the same keycodes as other models.

The only purpose of that model is to choose automatically proper symbols for 'es' default variant - and use olpc compat section.
Comment 12 Martin Langhoff 2011-03-03 15:35:37 UTC
I am confused -- re-reading the patch to understand your comment.

Again, I am not an expert in these keyboard maps. Apologies if I'm stating nonsense.

My patch does have some "olpc" changes, that maybe don't belong there. I think they are not required for "olpcm" -- if that is the case, I can separate them, see what they are meant to do, and file a new bug to discuss merging them.

More background: With this 'olpcm' model keyboard (en and es) the behaviour is radically different from any 'olpc' model keyboard. 

For example, the whole "function key" row changes behaviour; it becomes Mac-like, with the F-keys bound to functionality (f11 key:volume up, f12 key: volume down...); to get a real F11 scancode you have to hit the 'Fn' key _and_ the F1 key.
Comment 13 Sergey V. Udaltsov 2011-03-04 11:52:08 UTC
> My patch does have some "olpc" changes, that maybe don't belong there. I think
> they are not required for "olpcm" -- if that is the case, I can separate them,
> see what they are meant to do, and file a new bug to discuss merging them.
Well, it is not worth a separate bug, let's do it all together in one patch.

> More background: With this 'olpcm' model keyboard (en and es) the behaviour is
> radically different from any 'olpc' model keyboard. 
I do not know about reality - I am looking at the patch:)

So, here is what I propose for your patch:

1. Entire keycodes/olpc can be merged into keycodes/evdev
2. evdev.m_k.part - no changes needed at all!
3. base.xml.in needs extra entry for olpcm.
Comment 14 Martin Langhoff 2011-03-07 08:32:47 UTC
Thanks for the detailed hints. Something like this then?
http://dev.laptop.org/git/users/martin/xkeyboard-config/commit/?h=post-2.1-rough-rebase-3&id=0ad0fa46d082af2bf61f22ff2219884423dc6395
Comment 15 Martin Langhoff 2011-03-07 10:21:51 UTC
Hm - reviewing this patch (the original is not mine), I see some keycodes that we are aliasing in keycodes/evdev listed again in symbols/es and symbols/en . 

I don't think that's right. For example

In keycodes/evdev we said
+	alias <AE00> = <TLDE>;	// many OLPC layouts don't put tilde there

so symbols/es should _not_ have this change:

-    key <TLDE> { [    masculine,    ordfeminine      ] };
+    key <AE00> { [    masculine,    ordfeminine      ] };
Comment 16 Martin Langhoff 2011-03-07 10:28:24 UTC
Dropped the suspicious symbols/es and symbols/en changes. 
http://dev.laptop.org/git/users/martin/xkeyboard-config/commit/?h=post-2.1-rough-rebase-4&id=08e3062a6026d6f5f3dba4d1712937640b9f3785
Comment 17 Sergey V. Udaltsov 2011-03-07 12:52:03 UTC
Are you sure your keycodes are right? I suspect you switched the left side and the right side. If that's the case - we have to return back olpc-specific keycodes.
Comment 18 Martin Langhoff 2011-03-07 12:59:04 UTC
Ha ha ha. Knowing the semantics sure helps ;-)

If the bits I've shown above read right to you, then we stay with the original patch.
Comment 19 Sergey V. Udaltsov 2011-03-07 13:03:52 UTC
Yes. Just swap the sides in the original patch
Comment 20 Martin Langhoff 2011-03-09 08:38:20 UTC
Sergey -

I'm not sure about the Alias line being swapped. 

I can't find readable documentation on the semantics (XKBproto.pdf doesn't help here, except for something that hints that first param is 'real' and second is 'alias').

But we can see what's being done in the codebase -  

$ git grep alias.*TLDE
keycodes/digital_vndr/lk:    alias <TLDE> = <AE00>;
keycodes/digital_vndr/pc:    alias <TLDE> = <AE00>;
keycodes/digital_vndr/pc:    alias <TLDE> = <AE00>;
keycodes/evdev: alias <HZTG> = <TLDE>;
keycodes/evdev: alias <AE00> = <TLDE>;  // many OLPC layouts don't put tilde there
keycodes/sgi_vndr/indigo:    alias <AE00> = <TLDE>;
keycodes/sgi_vndr/indy:    alias <AE00> = <TLDE>;
keycodes/sgi_vndr/indy:    alias <TLDE> = <HZTG>;
keycodes/sun:    alias <TLDE> = <AE00>;
keycodes/sun:    alias <TLDE> = <AF13>;
keycodes/xfree86:    alias <AE00> = <TLDE>;     // Some geometries use AE00
keycodes/xfree86:    alias <HZTG> = <TLDE>;     // Hankaku_Zenkaku toggle
Comment 21 Sergey V. Udaltsov 2011-03-13 06:02:06 UTC
> I'm not sure about the Alias line being swapped. 
I guess you want
alias <TLDE> = <AE00>;
instead of
alias <AE00> = <TLDE>;
?

Am I right?
Comment 22 Sergey V. Udaltsov 2011-03-13 06:03:51 UTC
BTW, if I am right - olpc keycodes are redundant (covered by evdev), olpcm keycodes can be reduced to 2 lines
Comment 23 Sergey V. Udaltsov 2011-03-13 06:06:39 UTC
Sorry, I meant olpcm can be reduced to 3 lines:
 alias <AA02> = <BKSL>; // on model olpcm, new physical position of BKSL
 alias <AA06> = <AE12>; // on model olpcm, new physical position of =+
 alias <AA07> = <AC11>; // on model olpcm, new physical position of '"

But why do you need AA02/06/07 aliases at all? Why cannot you just use BKSL/AE12/AC11?
Comment 24 Walter Bender 2013-04-18 13:04:20 UTC
Sergey,

I am hoping to dust off this old thread and get this ticket closed (OLPC has just released new hardware that is blocking on keyboard defs.)

Just to bring you back up to speed, Martin Langhoff had proposed a series of patches to add support for a new OLPC keyboard model, olpcm, to support a mechanical keyboard used on some OLPC models. There was a back and forth about the best way to achieve this, both from the POV of adhering to convention and minimizing the complexity of the patch. It was never quite resolved due to some misunderstanding (on the OLPC side) about when to use keycode aliases. You had posed a question that was left unanswered (undoubtedly Martin got distracted by other pressing issues).

So, to answer the question you posed in comment 23, I think you are correct. Are there any other issues with the patch series I should know about before generating a new version?

regards.

-walter
Comment 25 Sergey V. Udaltsov 2013-04-18 23:44:35 UTC
Not that I remember of any other issues. You are welcome to submit the updated patch!
Comment 26 Walter Bender 2013-04-19 01:44:01 UTC
OK. Will get it to you ASAP. Thanks.
Comment 27 Walter Bender 2013-04-19 03:40:26 UTC
Created attachment 78205 [details] [review]
Revised patch that address comment 23
Comment 28 Sergey V. Udaltsov 2013-04-19 21:46:04 UTC
The patch does not apply to git:

$ git am /home/svu/Downloads/0001-add-support-for-the-OLPC-mechanical-non-membrane-key.patch 
Applying: add support for the OLPC mechanical non-membrane keyboard
error: patch failed: rules/base.ml_s.part:37
error: rules/base.ml_s.part: patch does not apply
error: patch failed: symbols/es:158
error: symbols/es: patch does not apply
Patch failed at 0001 add support for the OLPC mechanical non-membrane keyboard
The copy of the patch that failed is found in:
   /home/svu/git/xkeyboard-config/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Comment 29 Walter Bender 2013-04-20 16:07:34 UTC
Created attachment 78275 [details]
rebased patch against current master

Oops. I generated the patch from Martin's repository, which was stale. The newly attached patch generated against fresh pull from current master. Should apply without errors.
Comment 30 Sergey V. Udaltsov 2013-04-20 20:22:14 UTC
That is better! Thank you!
Comment 31 Daniel Drake 2013-05-03 20:10:06 UTC
Sorry to be irritating and reopen this multi-year affair... ;)

I think we are almost there but after more testing we found a problem in the patch that has been committed: on the olpcm es layout, the key that is physically below the escape key does not work correctly.

A reminder of what the keyboard looks like:
http://wiki.laptop.org/go/OLPC_Spanish_Non-membrane_Keyboard

With the latest xkeyboard-config from git, this key produces left apostrophe for unshifted, tilde for shifted, and nothing for AltGr.

Whereas we expected: "upside down question mark", "upside down exclamation" for shifted, and backslash for AltGr.

At a glance the patch does look right, since in symbols/es "olpcm" section it adds:
+    key <AE00> { [    questiondown,    exclamdown, backslash      ] };

but that's not working. And digging further, I can't see where AE00 gets defined in this configuration, with the final version of the patch.

The original patch did include one but we removed it here:

(In reply to comment #22)
> BTW, if I am right - olpc keycodes are redundant (covered by evdev), olpcm
> keycodes can be reduced to 2 lines

(In reply to comment #23)
> Sorry, I meant olpcm can be reduced to 3 lines:
>  alias <AA02> = <BKSL>; // on model olpcm, new physical position of BKSL
>  alias <AA06> = <AE12>; // on model olpcm, new physical position of =+
>  alias <AA07> = <AC11>; // on model olpcm, new physical position of '"

With the AE00 alias removed from the xkb_keycodes "olpcm" section, where is AE00 supposed to be defined? It is not included in evdev as comment #22 might be suggesting.
Comment 32 Daniel Drake 2013-05-08 19:54:06 UTC
Created attachment 79032 [details] [review]
fix AE00 alias

Here is a patch that adds the AE00 alias to make things work.

I'd be happy to fix this another way if there is some preference.
Comment 33 Sergey V. Udaltsov 2013-05-14 21:38:04 UTC
Thanks for being so annoying!:) Commited

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.