[Sugar-devel] [PATCH sugar-toolkit] Handle DBUS tubes in the activity telepathy client, part of OLPC #10738

[Sascha Silbe]

Excerpts from Simon Schampijer's message of Thu Apr 14 19:43:05 +0200 2011:

> This adds the handling of DBUS tube channels to the
> 'HandlerChannelFilter' in activity's telepathy client.

It would be nice to have some rationale for the change. Why didn't we do
this before and in what way is the new design better? Are there any
compatibility issues for activities that don't use sugar-toolkit (e.g.
because they're not written in Python)?


[src/sugar/activity/activity.py]
> @@ -65,9 +65,11 @@ import cjson
>  from telepathy.server import DBusProperties
>  from telepathy.interfaces import CHANNEL, \
>                                   CHANNEL_TYPE_TEXT, \
> +                                 CHANNEL_TYPE_DBUS_TUBE, \
>                                   CLIENT, \
>                                   CLIENT_HANDLER

This can be shortened to:

from telepathy.interfaces import CHANNEL, CHANNEL_TYPE_DBUS_TUBE, \
                                 CHANNEL_TYPE_TEXT, CLIENT, CLIENT_HANDLER


> @@ -927,15 +930,23 @@ class _ClientHandler(dbus.service.Object, DBusProperties):
>            })
>  
>      def __get_filters_cb(self):
> -        logging.debug('__get_filters_cb')
> -        filters = {
> +        logging.debug('__get_filters_cb Activity')
> +        filter_text = {
>              CHANNEL + '.ChannelType': CHANNEL_TYPE_TEXT,
>              CHANNEL + '.TargetHandleType': CONNECTION_HANDLE_TYPE_CONTACT,
>              }
> -        filter_dict = dbus.Dictionary(filters, signature='sv')
> -        logging.debug('__get_filters_cb %r', dbus.Array([filter_dict],
> -                                                        signature='a{sv}'))
> -        return dbus.Array([filter_dict], signature='a{sv}')
> +        filter_dict = dbus.Dictionary(filter_text, signature='sv')
> +        filters = dbus.Array([filter_dict], signature='a{sv}')
> +
> +        filter_tube = {
> +            CHANNEL + '.ChannelType': CHANNEL_TYPE_DBUS_TUBE,
> +            CHANNEL + '.TargetHandleType': CONNECTION_HANDLE_TYPE_ROOM,
> +            }
> +        filter_dict = dbus.Dictionary(filter_tube, signature='sv')
> +        filters.append(filter_dict)
> +
> +        logging.debug('__get_filters_cb Activity filters=%r', filters)
> +        return filters

Like for the sugar patch, we should be able to get rid of the explicit
conversions, making the code terser (and thus easier to grok).


> @@ -946,7 +957,12 @@ class _ClientHandler(dbus.service.Object, DBusProperties):
>                  user_action_time, handler_info)
>          try:
>              for channel in channels:
> -                self._got_channel_cb(connection, channel[0])
> +                channel_properties = channel[1]

How about:

            for object_path, properties in channels:

or:

            for channel_object_path, channel_properties in channels:

> +                channel_type = channel_properties[CHANNEL + '.ChannelType']
> +                handle_type = channel_properties[CHANNEL + '.TargetHandleType']

> +                if channel_type == CHANNEL_TYPE_TEXT and \
> +                        handle_type == CONNECTION_HANDLE_TYPE_ROOM:
> +                    self._got_channel_cb(connection, channel[0])

PEP8:

                if channel_type == CHANNEL_TYPE_TEXT and \
                   handle_type == CONNECTION_HANDLE_TYPE_ROOM:
                    self._got_channel_cb(connection, channel[0])


>          except Exception, e:
>              logging.exception(e)

logging.exception already includes the exception, we should instead use
some more informative message including the actual values (esp.
channels) that were passed in. Since it was part of the old code, we can
address it in a separate patch if you prefer.

Sascha

-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 500 bytes
Desc: not available
URL: <http://lists.sugarlabs.org/archive/sugar-devel/attachments/20110416/43c541c2/attachment.pgp>