Re: [sugar] [patch][connect-activity] Update to high-level API in	latest dbus-python Ar 04/05/2007 am 19:16, ysgrifennodd Simon McVittie:
> -def guess(x):
> -    if x.lower() in ('true', 'false'):
> -        return
> -
> -    if x == 'true':
> -        return True
> -
> -    if x == 'false':
> -        return False
> -
> -    try:
> -        return int(x)
> -    except ValueError:
> -        pass
> -
> -    return x
> +def decode(x, sig):
> +    # XXX: dbus-python ought to do most of the work for this
> +    if sig == 'b':
> +        x = x.lower()
> +        if x in ('true', '1'):
> +            return True
> +        if x in ('false', '0'):
> +            return False
> +        raise ValueError('Not true or false: %r' % x)
> +    elif sig == 'n':
> +        return dbus.Int16(x)
> +    elif sig == 'q':
> +        return dbus.UInt16(x)
> +    elif sig == 'i':
> +        return dbus.Int32(x)
> +    elif sig == 'u':
> +        return dbus.UInt32(x)
> +    elif sig == 'x':
> +        return dbus.Int64(x)
> +    elif sig == 't':
> +        return dbus.UInt64(x)
> +    elif sig == 'd':
> +        return dbus.Double(x)
> +    elif sig == 's':
> +        return dbus.UTF8String(x)
> +    else:
> +        raise TypeError('Unhandled D-Bus signature %r' % sig)

telepathy-python's examples/account.py currently does something like guess()
rather than something like decode(). That should probably be changed.

> --- a/client.py
> +++ b/client.py
> @@ -1,25 +1,28 @@
> -
>  import pprint
>  
> -import dbus.glib
>  import gtk
>  import telepathy
>  
> -# Needed for now, as dbus-python's high-level API doesn't support connecting
> -# to arbitrary addresses, or daemonless connections.
>  import _dbus_bindings
>  import dbus.lowlevel

Are these still necessary?

> +from dbus.connection import Connection
> +from dbus.mainloop.glib import DBusGMainLoop
> +from dbus.service import Object
> +
> +from telepathy.interfaces import CHANNEL_TYPE_TUBES, \\
> +        CHANNEL_TYPE_TEXT, CONN_INTERFACE, CHANNEL_INTERFACE_GROUP
> +from telepathy.constants import TUBE_TYPE_DBUS, \\
> +        TUBE_STATE_LOCAL_PENDING, TUBE_STATE_REMOTE_PENDING, TUBE_STATE_OPEN, \\
> +        CONNECTION_STATUS_CONNECTED, HANDLE_TYPE_ROOM

I have a slight preference towards just using "import telepathy" rather than
"from telepathy import (long list of constants)", but I'm not hugely partial,
so this is ok.

> +dbus_main_loop = DBusGMainLoop(set_as_default=True)

Statements in libraries considered dubious. In other words, "import x"
shouldn't cause side-effects.

What does this actually do? Does it create a new GLib mainloop? Will it cause
odd things to happen if another DBusGMainLoop loop has already been installed?

>  def print_dbus_message(msg):
>      print 'got %s' % (msg.__class__.__name__)

Is this still used? If not, remove it.

> From bdc429db799fb8f18a48ba75fd81e673bb50da01 Mon Sep 17 00:00:00 2001
> From: Simon McVittie 
> Date: Fri, 4 May 2007 17:40:20 +0100
> Subject: [PATCH] client.py: split out game code into a service.Object

...

> +logging.basicConfig()
> +_logger = logging.getLogger('connect-activity.client')

Is this idempotent? The documentation is ambiguous:

  Does basic configuration for the logging system by creating a StreamHandler
  with a default Formatter and adding it to the root logger. The functions
  debug(), info(), warning(), error() and critical() will call basicConfig()
  automatically if no handlers are defined for the root logger.

Does this cause side effects if a handler is defined for the root logger
already? (Whatever that means. :P)

> +class ConnectGame(Object):

Nicely done. Perhaps this should even be split out into a separate file, e.g.
game.py.

-- 
Dafydd
_______________________________________________
Sugar mailing list
Sugar@laptop.org
http://mailman.laptop.org/mailman/listinfo/sugar

