Add navigation support

[GoogleCodeExporter]

*What feature or enhancement would you like to see in Remuco?*
Be able to issue "navigate" controls to the player adapter.

*Please describe a use case that motivates this feature:*
Use it in any menu-driven application, such as DVD's, interactive rating, etc.

I'm starting a branch in github.com/igoralmeida/remuco with the code I
intend to submit to upstream. I have some questions regarding the code flow.

Original issue reported on code.google.com by igor.con...@gmail.com on 24 Feb 2010 at 2:58

[GoogleCodeExporter]

Maybe this should be blocked until we have more screens implemented?

Original comment by igor.con...@gmail.com on 27 Feb 2010 at 1:31

[GoogleCodeExporter]

Yes, without client side implementation of a navigation screen, tit's quite 
hard to
work on this on the server and player adapter side. Maybe this should be 
blocked on a
Python client? I think of something like a client command line shell, which can 
be
used as a reference implementation for other clients and for testing the server 
or
player adapter without the need for device emulators. I'm not sure about the 
relation
of work and benefit here, what do you think?

Original comment by obensonne@googlemail.com on 2 Mar 2010 at 3:06

• Changed state: Accepted

[GoogleCodeExporter]

A client shell seems nice, but I'd just go for some kind of fork which is able 
to
call Manager.__pa.{ctrl,action,request}_*, perhaps after running the gobject's
mainloop in Manager.run()? 

Tapping into ctrl_* and the like (see the shell pattern above) allows us to 
bypass
__handle_*, which involves binary data and sucks. The shell could be as simple 
as a
"which method should i call?" blocking menu.

We'd have to figure out how to do this, though. Maybe handling SIGHUP is the 
way to go?

Original comment by igor.con...@gmail.com on 2 Mar 2010 at 4:59

[GoogleCodeExporter]

I wish I had decided for text based protocol when I started with Remuco. 
Motivation
was low network communication but this isn't a big advantage anymore. A text 
based
protocol would allow telnet client, sweet. Anyway, this gets off-topic.

SIGHUP does not allow arguments, does it? How would you use it then? Maybe you 
could
open Python console and run a player adapter in that console. Before calling the
manager's run() method, you could add a timeout callback function
(gobject.timeout_add(...)) to the main loop which reads ctrl_navigate arguments 
from
a file and then calls ctrl_navigate(). ... I think that's the fastest way to 
test the
navigate feature of a player adapter ... hm, again, I wish we had a text based 
protocol.

Original comment by obensonne@googlemail.com on 3 Mar 2010 at 9:53

[GoogleCodeExporter]

In my head it would work like this:
Inside _init_main_loop(), I assign a function to handle SIGHUP. This function 
prints
a menu, then expects something from raw_input(), calls e.g.
manager_instance.__pa.ctrl_* (somehow), and then the handler function returns. 
So we
don't need to pass arguments with SIGHUP, our menu thingy will fetch this from 
the
user directly.

I have some code ready, but I paused at raw_input() (the "somehow" part seems 
tricky)
to handle more pressing concerns here. I'll upload it to yet another branch so 
you
can take a look.

To keep testing you'd "kill -s 1 <remuco's pid>", get a menu, choose the 
function and
see what happens.

Original comment by igor.con...@gmail.com on 3 Mar 2010 at 10:21

[GoogleCodeExporter]

I see, yes, that should work.

Original comment by obensonne@googlemail.com on 4 Mar 2010 at 7:48

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I have decided against adding code to manager.py directly, since all I need is 
the
adapter's reference.
So, commit d4d6a4 (test_shell branch) introduces testshell.py, a really ugly 
hack so
I can keep testing the navigation thingy.

About the "I have some questions regarding the code flow" part of this issue,
here's what I mean:

When tracking the code to add the navigate* messages, I couldn't find the code 
which
adds the parameters to the control functions. I thought I'd need only one
ACTION_NAVIGATE and could fetch the direction from somewhere else. Is this not
possible? Do I have to add ACTION_NAVIGATE_UP, _DOWN, _LEFT, ...?

Check http://github.com/igoralmeida/remuco/commit/ae84f5 and .../commit/f771e 
(its
parent) to see the code.

Original comment by igor.con...@gmail.com on 7 Mar 2010 at 9:24

[GoogleCodeExporter]

This issue was updated by revision 49c705f850.

It will print a menu with some functions you can call via gobject.idle_add

To use the test shell, just call testshell.setup(playeradapter_instance) before
calling Manager.run().
Then, whenever you want, issue a SIGHUP signal:
$ kill -s 1 <remuco's pid>

It will use the adapter's stdout, so this won't work if you're running in the
background or as a service.

Original comment by igor.con...@gmail.com on 7 Mar 2010 at 10:41

[GoogleCodeExporter]

The message flow could be realized in 2 ways:

1) Define an extra message code for each control (e.g. CTRL_NAVUP, 
CTRL_NAVDOWN, ...)
2) Define one message with a paramter.

The second one requires to define the parameter as a data object in data.py. It 
only
comes down to an integer but for the message handling and serialization 
back-end it
needs to be a class which implements the Serializable interface (see the other 
data
classes in data.py). The second solution is a bit more work but does not blow 
up the
message code space. See the __handle_message... methods in adpater.py for how 
those
parameters finally get passed to the ctrl_... methods.

Concerning the interface switch, I would do it like that: Whenever the "menu" 
gets
active on a player, its adapter sends a SYNC_IFACE message (with the interface 
name
as parameter) to the client which then switches to the navigation interface. 
Same
thing when the menu disappears again. If the switch to the menu is initiated by 
a
client, it sends a CTRL_IFACE or similar named message to the player. Once the 
player
has switched to the menu view, it sends the above mentioned SYNC_IFACE message. 
That
means the interface switch on the client only happens when the player actually 
shows
that interface.

Original comment by obensonne@googlemail.com on 10 Mar 2010 at 3:38

• Changed state: Started

[GoogleCodeExporter]

I added a few improvements to the test shell, see test_shell branch in my fork.

Passing arguments to a function is not fully functional because the types may 
not
match. For example, MplayerAdapter.ctrl_volume() expects a 'direction' argument 
as an
integer, but it will receive a string, and a TypeError is raised (because the 
code
blindly trusts the manager.
The question is: how far should the test shell go? Should it, somehow, figure 
out the
type of the argument and coerce as necessary; or should it pass along the 
type-check
responsibility to the ctrl_ method?

Original comment by igor.con...@gmail.com on 15 Mar 2010 at 2:00

[GoogleCodeExporter]

I wouldn't introduce type checks in the ctrl-methods just because of the 
test-shell.
As an alternative to type-guessing magic in the test-shell you could use 
Python's
*exec* or *eval* functions:

input = raw_input('Call: ') # example user input: 'ctrl_volume(1)'
stmt = "_paref.%s" % input
exec stmt

Original comment by obensonne@googlemail.com on 16 Mar 2010 at 7:09

[GoogleCodeExporter]

I updated the navigation branch with argument unpacking. I think we're only 
missing
the screen implementation and handling in a capable adapter.
Oben, can you take a look (mainly at the java code)? I may have overlooked the 
part
where ACTION_NAV* gets its assignments.

About the test-shell, how about casting to int if the argument "seems to be a
number", and leaving it as string otherwise? Using eval seems ugly and 
error-prone in
this situation.

Original comment by igor.con...@gmail.com on 4 Apr 2010 at 2:23

[GoogleCodeExporter]

Test-shell:
I think eval() is not too ugly as the whole thing is used internally for testing
only. On the other side, as long as we only have strings and ints as arguments,
casting numbers to ints is fine too.

Navigation:
Message.java and Player.java looks good. Keybindings.java and PlayerScreen.java 
does
not work that way (as you've already guessed by your TODOs there). Currently 
you have
defined *one* navigation action, bound *to* one key. Actually we need one
action/key-binding for each UP, DOWN, ... In PlayerScreen.java you would catch
KeyBindings.ACTION_NAVIGATE_UP and friends separately and call
player.ctrlNavigate(action) accordingly in each case. The feature check is ok.

Currently I have some problem with the hg-git tool so I cannot merge your 
changes
right now, but I'll do that once I've fixed the hg-git issue.

Original comment by obensonne@googlemail.com on 12 Apr 2010 at 9:08

[GoogleCodeExporter]

You seem to be looking at an earlier version.
As of
http://github.com/igoralmeida/remuco/commit/c8089940063bc3d1e88f69ad893103c99def
836d
there are multiple ACTION_NAV*, I think it's what you mean.

I think I'm only missing the older ACTION_NAVIGATE in handleKeyReleased, will 
fix it
soon enough.

Original comment by igor.con...@gmail.com on 12 Apr 2010 at 10:09

[GoogleCodeExporter]

My navigation branch has been updated again and dvd navigation is working on 
mplayer :)

Original comment by igor.con...@gmail.com on 7 Jun 2010 at 2:29

[GoogleCodeExporter]

This issue was updated by revision 4919ea36eb.

While this actually worked, it is a PITA to manually change your 4/5/6 bindings
to navigation and back to playback controls, so we definitely need the new
screens implemented :P

Original comment by igor.con...@gmail.com on 9 Jun 2010 at 9:09

[GoogleCodeExporter]

On my phone I use the number keys for volume, next, previous, ... and the 
joystick-like key for navigation .. not that painful.

A screen would be better though. So I suggest we close this issue and use new 
issues for features building upon the now existing basic navigation support.

If you agree, just close this ticket.

Original comment by obensonne@googlemail.com on 10 Jun 2010 at 6:18

[GoogleCodeExporter]

Sounds good.

Original comment by igor.con...@gmail.com on 10 Jun 2010 at 10:01

• Changed state: Fixed