[sugar] [PATCH] Add speaker device and icon by default
Tomeu Vizoso
tomeu at tomeuvizoso.net
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
mailing list