In rotated mode, game keys retain original orientation

[blahedo]

When I put the screen into a rotated mode with the rotate button, the scroll buttons (both the directional ones that move the webpage incrementally and the playstation keys that mimic PgUp etc) retain their original orientation. That is, "Circle" is "Page Up" even if, relative to the screen orientation, it is on the left or right or bottom. Even more confusingly, the arrow key pointing toward the screen is "Right" regardless of the screen orientation. Really confusing. The discussion in #1310 looks relevant, but this seems to be a separate issue.

[jg]

The intent is the circular game pad rotate with the screen, but not the buttons.

The reasoning is the buttons have labels, referred to in documents; where the game pad is sensible to talk about up/down/left/right relative to the current orientation.

[blahedo]

I wondered about that, which is why I was careful to point out that even so, the directional pad isn't working.

I've just done some additional triage work, and it's even weirder. Apparently there's two levels of click on that button, like the shutter of a point-n-shoot camera: push down a little bit and there's one click, push a little harder and there's a second click. I have no idea what the intended semantics for this distinction would be, but they are distinguished by the software: the first, shallow click works as I described in the initial bug report, i.e. irrespective of screen rotation, the arrow that scrolls the page down in unrotated mode will continue to scroll it down. However, if you do the "deep click", *then* the rotated direction starts to register, somehow in addition to the unrotated direction, so that in a 90° rotation mode the page will actually scroll diagonally. In 180° rotated mode (upside down), where the deep/shallow behaviour would be in opposition, the shallow one wins: if I press the unrotated down arrow (which is currently pointing up), the page scrolls down.

Incidentally, I've been doing most of the above triage work with the screen just rotated and not in full e-book mode, but I did verify that it does act the same even after I flip the screen around and fold it down to hide the keyboard.

[jg]

yes, we know the directional pad is not working. Too late for us to get this fixed for update.1

This may be not X directly, but OHM or other hardware manager.

But for now, I'll make sure this is on Bernie's plate, as he'll have to help whomever does this (not clear we should wire this in at the driver level...)

[dupuy]

I'm able to reproduce the "weird" "deep click" behavior, but I wonder if what me and blahedo are experiencing as a "deep click" is just pressing the adjacent directions (as they are all under a single rocker button).

[Erik Garrison]

Replying to dupuy:

I'm able to reproduce the "weird" "deep click" behavior, but I wonder if what me and blahedo are experiencing as a "deep click" is just pressing the adjacent directions (as they are all under a single rocker button).

I believe that you are correct. The second "deep click" is an adjacent direction. Thus "deep clicking" has a 50/50 chance of moving the screen in the expected direction when the screen is rotated.

[erikg]

I have patched handle_rotation() in keyhandler.py so that the d-pad keys are rotated to match the display orientation set by xrandr. Per jg's comments I have left the gamepad keys alone. Attached is a patch which I have tested on update.1-703.

[erikg]

patch to rotate d-pad keys with display orientation

[erikg]

Additionally, I believe I may have fixed a sugar-crashing bug whereby the subprocess.Popen() call running xrandr threw an OSError in response to EINTR on an internal call to read(). The bug earlier manifested itself by crashing sugar after rapidly pressing the screen rotate button. Per mstone's suggestions I used subprocess.check_call() to order the xrandr and xmodmap calls, and catch and ignore EINTR to avoid this problem. If the keys become desyncronized (the call to xrandr fails) the attached code will resync them on the next press of the rotation key.

[mstone]

See also #1310.

[tomeu]

Only some style nitpicks to mention, from http://wiki.laptop.org/go/Python_Style_Guide:

+        actual_keycodes = keycodes[self._screen_rotation:self._screen_rotation+4]

Please use spaces around operators.

+        argv = ['xmodmap']

What about s/argv/xmodmap_command?

+        for arg in [('-e', 'keycode %i = %s' % p) for p in code_pairs]:
+            argv.extend(arg)

Cannot think of anything clearer myself, but would be great if you could make it easier to understand for the casual reader.

+            import errno

imports should go at the top of the file

If the keys become desyncronized (the call to xrandr fails) the attached code will resync them on the next press of the rotation key.

Can't we just rotate if the call to xrandr succeeds?

Tried but didn't managed to crash anymore by rotating, so this looks to fix that indeed.

Please get an account in the sugar group in dev.l.o and push it yourself. Thanks!

[tomeu]

Oh, just in case, have you agreed with Eben on this change?

[Eben]

I'm in agreement on this.

[erikg]

Replying to tomeu:

Only some style nitpicks to mention, from http://wiki.laptop.org/go/Python_Style_Guide: {{{ + actual_keycodes = keycodes[self._screen_rotation:self._screen_rotation+4] }}} Please use spaces around operators. {{{ + argv = xmodmap? }}} What about s/argv/xmodmap_command? {{{ + for arg in [('-e', 'keycode %i = %s' % p) for p in code_pairs]: + argv.extend(arg) }}} Cannot think of anything clearer myself, but would be great if you could make it easier to understand for the casual reader.

I'll look into clarifying this. I'll add comments at least; this was just a first-pass implementation to test and share the fix.

{{{ + import errno }}} imports should go at the top of the file

I understand that there is a style guide to be followed, but why should we categorically import this module if it's only used in an unlikely error condition? My understanding is that Python isn't smart enough to just-in-time load library dependencies, so we waste memory by categorically importing.

Obviously this is a discussion for another forum and I'll push the import to the top to meet the guidelines.

If the keys become desyncronized (the call to xrandr fails) the attached code will resync them on the next press of the rotation key.

Can't we just rotate if the call to xrandr succeeds?

Not a bad idea. I'll look back at it when I get ready to push.

Tried but didn't managed to crash anymore by rotating, so this looks to fix that indeed.

Hope so.

Please get an account in the sugar group in dev.l.o and push it yourself. Thanks!

I tried to push earlier but realized that I need to be added to the sugar group. Will do and push with suggested changes.

[erikg]

I have made the requested changes to the patch and pushed it into the main sugar git repo. Please test and verify that this is working as expected.

[erikg]

Replying to erikg:

Replying to tomeu:

If the keys become desyncronized (the call to xrandr fails) the attached code will resync them on the next press of the rotation key.

Can't we just rotate if the call to xrandr succeeds?

Not a bad idea. I'll look back at it when I get ready to push.

Tried but didn't managed to crash anymore by rotating, so this looks to fix that indeed.

Hope so.

Note that the subprocess.check_call() function will throw a CalledProcessError if the called process returns a nonzero exit status. We should expect to see that error in the logs if there is ever xrandr or xmodmop fails.

[tomeu]

Replying to erikg:

Replying to tomeu:

{{{ + import errno }}} imports should go at the top of the file

I understand that there is a style guide to be followed, but why should we categorically import this module if it's only used in an unlikely error condition? My understanding is that Python isn't smart enough to just-in-time load library dependencies, so we waste memory by categorically importing.

Would be good to have all the rationale behind the style guide, but I don't know where we could get that info. I understand that our guidelines are based on some document from the python community? Perhaps Guido himself?

In my opinion, imports at the top helps with understanding which deps a module has, and to easily detect "bad smells" related to that. Regarding memory saving, I bet that same module has been already imported some dozen of times before your code is executed.

[mstone]

This seems like a terrible argument to me because of the categorical imperative: if everyone blindly followed style guidelines without making a conscious assessment of the costs imposed by those guidelines then we'd wind up... where we are today, with Python activities that import 10 seconds worth of code before becoming interactive.

Thus, while you're correct that we should place a high value on following conventions, I happen to think that the 'imports at the top' guideline has been so badly discredited by its effect activity startup time as to be untenable in the presence of python modules which perform arbitrary computation at import time. (We could also decide to refuse to use python modules which perform noticeable computation on import, but that seems more painful to me. What do you think?)

[cjb]

I agree strongly with Michael; well said.

[mtd]

Aren't we a little OT for this bug?

And m_stone, re-reading "if everyone blindly followed style guidelines without making a conscious assessment of the costs imposed by those guidelines then we'd wind up... where we are today, with Python activities that import 10 seconds worth of code before becoming interactive", I don't understand what you really intend to argue with (I suspect many people have slavishly followed PEP 008 and indeed not ended up in that situation[1]). I don't think anyone's arguing we should slavishly follow conventions nor is anyone arguing that we should not try to use good style[2] as defined by the language BDFL[2].

I think the solution to your concern (if I understand it correctly) is something that includes: a) reduce modules doing work upon import (as in #5470; we should use #5228 or similar to identify offending modules); b) lazy importing as in #5452 to squeeze the last ounce out (I volunteer and hope to have something shortly; unless there are mind-bending security issues this is a solved problem - see first four google hits for python lazy import)

Martin

1. I am happy to do research on this given an identifiable corpus of python code to judge, but don't seriously believe anyone's going to question it. 2. http://www.python.org/dev/peps/pep-0008/

[tomeu]

Discussion about imports and performance moved to here:

http://lists.laptop.org/pipermail/sugar/2008-June/006392.html

[mstone]

Paul points out that it might be handy if the touchpad axes rotated with the screen as well. That way, you could actually use the TP in ebook mode (with the screen partially lifted). Is there another simple X command we can run to accomplish this?

[pgf]

Replying to mstone:

Paul points out that it might be handy if the touchpad axes rotated with the screen as well.

thanks michael. i suspect this should be a separate trac item, if only because it's not nearly as clear when the touchpad should be rotated and when not. unlike the directional keys, which are "locked" physically to the screen by virtue of their location, the touchpad and screen can move relative to one another. if you rotate the screen while in laptop orientation you would want a different touchpad rotation than when in ebook mode. (and because you have to lift the screen slightly to use the touchpad while in ebook mode, it's not clear that the ebook sensor can reliably help here.)

[erikg]

This is resolved by git commit 605fe4561f7ea in sugar http://dev.laptop.org/git?p=sugar;a=commit;h=605fe4561f7ea6b1a9ef8b770cfd8c7a2414e20b. I am thus closing this bug.