[sugar] [PATCH] Add speaker device and icon by default

Tomeu Vizoso tomeu
Wed May 14 06:14:57 EDT 2008


Hi, sorry for the delay.

On Tue, May 13, 2008 at 12:43 AM, Martin Dengler
<martin at martindengler.com> wrote:
>  +    def get_mute(self):

You use 'muted' instead of 'mute' below, which one is more correct?

>  +        if not self._mixer or not self._master:
>  +            logging.error('Cannot get the mute status')
>  +            return False

Shouldn't we return True if there's no way to get the volume?

>  diff --git a/src/model/devices/Makefile.am b/src/model/devices/Makefile.am
>  index 5440eeb..3124144 100644
>  --- a/src/model/devices/Makefile.am
>  +++ b/src/model/devices/Makefile.am
>  @@ -5,4 +5,6 @@ sugar_PYTHON =                  \
>         __init__.py             \
>         device.py               \
>         devicesmodel.py         \
>  -       battery.py
>  +       battery.py              \
>  +       speaker.py

Just as a comment, the convention is to sort files alphabetically (it
was already wrong).

>  @@ -54,6 +55,13 @@ class DevicesModel(gobject.GObject):
>
>          for udi in hal_manager.FindDeviceByCapability('battery'):
>              self.add_device(battery.Device(udi))
>
>  +        try:
>
> +            self.add_device(speaker.Device())
>  +        except Exception, speaker_fail_msg:
>  +            logging.error("could not initialize speaker device: %s" %
>  +                          speaker_fail_msg)
>  +            logging.debug(traceback.format_exc())

I would add the speaker device in the constructor, instead of in
_observe_hal_manager().

Perhaps we should add try..except blocks to all the add_device calls?
Not in this patch, though.

>  +    def get_type(self):
>  +        return 'speaker'

Maybe should be a constant at the module level?

>  +    def do_get_property(self, pspec):
>  +        if pspec.name == "level":
>  +            return self._get_level()
>  +        elif pspec.name == "muted":
>  +            return self._get_muted()

See above comment about muted vs mute.

>  +        self._adjustment = gtk.Adjustment(
>  +            self._model.props.level, 0.0, 101.0, 1, 1, 1)

Not the most common of breaking an arg list, but I don't particularly care.

Thanks!

Tomeu



More information about the Sugar-devel mailing list