Re: [sugar] [PATCH] Add speaker device and icon by default
On Wed, May 14, 2008 at 12:14:57PM +0200, Tomeu Vizoso wrote:
> Hi, sorry for the delay.
Same here - was on holiday.
>
> On Tue, May 13, 2008 at 12:43 AM, Martin Dengler
> <[EMAIL PROTECTED]> wrote:
> > +def get_mute(self):
>
> You use 'muted' instead of 'mute' below, which one is more correct?
I prefer 'muted', because 'muted' sounds more like a predicate/boolean
to me and 'mute' sounds more like a verb. (consider 'get_stopped()'
vs. 'get_stop'). Changed get_mute() to get_muted().
> > +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?
I thought we should assume we're not muted if we don't know what the
volume is, but I think I see your point. I don't think the return
value is sensible in this scenario, though, so perhaps we should
raise()? I don't have strong feelings, so I've changed to return
True.
> > 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).
Gotcha - I didn't want to reorder but as I'm changing the battery.py
line anyway I'll reorder it.
> > @@ -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().
It should be in the constructor, I agree. I'm sure I put it there
but...must've slipped a method somehow. Scary. Moved.
> Perhaps we should add try..except blocks to all the add_device calls?
> Not in this patch, though.
Yes, possibly. It's the Device-subclass __init__ and the .py file
import itself that we have to worry about, though, I think (consider
that the logical extension of what you've said would be to just put a
try...except in add_device()). Something to think about in a later
patch, I agree.
> > +def get_type(self):
> > +return 'speaker'
>
> Maybe should be a constant at the module level?
Every Device-subclass implements this method in a similar form. I'm
not a huge fan of this function - __name__ should be enough.
AFAICS, it's just a way of helping deviceview.py with the fact that
model & view implementations are split at a high level
(src/{model,view}/...):
http://dev.laptop.org/git?p=sugar;a=blob;f=src/view/devices/deviceview.py;h=90ebbf55413925f537a9e9e900d3828bb4f28bac;hb=HEAD
.
Assuming you prefer the consistency I'll leave this as-is, but I'm not
at all bothered about changing it.
> > +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.
ibid/fixed.
> > +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.
I don't know much gtk so the Adjustment positional args mean little to
me - I have to look them up each time I care - so far about twice :).
I've looked them up a third time and tried a bit more sensical split.
> Thanks!
Thank you!
> Tomeu
Martin
pgpeYmngRZ2Hb.pgp
Description: PGP signature
___
Sugar mailing list
Sugar@lists.laptop.org
http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
Hi, sorry for the delay.
On Tue, May 13, 2008 at 12:43 AM, Martin Dengler
<[EMAIL PROTECTED]> 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
___
Sugar mailing list
Sugar@lists.laptop.org
http://lists.laptop.org/listinfo/sugar
[sugar] [PATCH] Add speaker device and icon by default
Add speaker device and icon by default, as in
http://wiki.laptop.org/go/Designs/Frame#07
---
src/hardware/hardwaremanager.py | 31 +-
src/model/devices/Makefile.am |4 +-
src/model/devices/devicesmodel.py |8 +++
src/model/devices/speaker.py | 59 ++
src/view/devices/Makefile.am |4 +-
src/view/devices/speaker.py | 119 +
6 files changed, 220 insertions(+), 5 deletions(-)
create mode 100644 src/model/devices/speaker.py
create mode 100644 src/view/devices/speaker.py
diff --git a/src/hardware/hardwaremanager.py b/src/hardware/hardwaremanager.py
index 5b9e330..284a283 100644
--- a/src/hardware/hardwaremanager.py
+++ b/src/hardware/hardwaremanager.py
@@ -50,6 +50,13 @@ class HardwareManager(object):
if track.flags & gst.interfaces.MIXER_TRACK_MASTER:
self._master = track
+def get_mute(self):
+if not self._mixer or not self._master:
+logging.error('Cannot get the mute status')
+return False
+return self._master.flags & gst.interfaces.MIXER_TRACK_MUTE \
+ == gst.interfaces.MIXER_TRACK_MUTE
+
def get_volume(self):
if not self._mixer or not self._master:
logging.error('Cannot get the volume')
@@ -57,9 +64,19 @@ class HardwareManager(object):
max_volume = self._master.max_volume
min_volume = self._master.min_volume
-volume = self._mixer.get_volume(self._master)[0]
-return volume * 100.0 / (max_volume - min_volume) + min_volume
+volumes = self._mixer.get_volume(self._master)
+
+#sometimes we get a spurious zero from one/more channel(s)
+#TODO: consider removing this when trac #6933 is resolved
+nonzero_volumes = [v for v in volumes if v > 0]
+
+if len(nonzero_volumes) > 0:
+#we could just pick the first nonzero volume, but this converges
+volume = sum(nonzero_volumes) / len(nonzero_volumes)
+return volume * 100.0 / (max_volume - min_volume) + min_volume
+else:
+return 0
def set_volume(self, volume):
if not self._mixer or not self._master:
@@ -76,7 +93,15 @@ class HardwareManager(object):
volume = volume * (max_volume - min_volume) / 100.0 + min_volume
volume_list = [ volume ] * self._master.num_channels
-self._mixer.set_volume(self._master, tuple(volume_list))
+#sometimes alsa sets one/more channels' volume to zero instead
+# of what we asked for, so try a few times
+#TODO: consider removing this loop when trac #6934 is resolved
+last_volumes_read = [0]
+read_count = 0
+while (0 in last_volumes_read) and (read_count < 3):
+self._mixer.set_volume(self._master, tuple(volume_list))
+last_volumes_read = self._mixer.get_volume(self._master)
+read_count += 1
def set_mute(self, mute):
if not self._mixer or not self._master:
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
+
diff --git a/src/model/devices/devicesmodel.py
b/src/model/devices/devicesmodel.py
index 691e19e..e72a9d4 100644
--- a/src/model/devices/devicesmodel.py
+++ b/src/model/devices/devicesmodel.py
@@ -23,6 +23,7 @@ from model.devices import device
from model.devices.network import wireless
from model.devices.network import mesh
from model.devices import battery
+from model.devices import speaker
from hardware import hardwaremanager
from hardware import nmclient
@@ -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())
+
def _observe_network_manager(self):
network_manager = hardwaremanager.get_network_manager()
if not network_manager:
diff --git a/src/model/devices/speaker.py b/src/model/devices/speaker.py
new file mode 100644
index 000..d3b9967
--- /dev/null
+++ b/src/model/devices/speaker.py
@@ -0,0 +1,59 @@
+# Copyright (C) 2008 Martin Dengler
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+
Re: [sugar] [PATCH] Add speaker device and icon by default
On Tue, May 06, 2008 at 05:40:30PM +0100, Martin Dengler wrote: > On Tue, May 06, 2008 at 04:39:30PM +0200, Tomeu Vizoso wrote: > > On 5/4/08, Martin Dengler <[EMAIL PROTECTED]> wrote: > > > On Tue, Apr 29, 2008 at 02:12:58PM +0200, Tomeu Vizoso wrote: > > > > I think that Michael has spotted here a wonderful opportunity for > > > > making Sugar more robust, thanks to your patch. > ... > > > I haven't forgotten this thread, just been unable to work on it. After working on it a bit more I think the right thing to do is just the small change Tomeu suggested. I would really like in the future to submit a patch that allows one to place a few files in a devices.d/ subdir - like model.py, view.py, icon.svg, etc. - and then the devicesmodel.py code could try to load each model.py appropriately, and so forth, but right now most of our "devices" really are device brokers (hal-dbus, networkmanager) and such a setup doesn't mesh well with them. Obviously in this patch would be the "do the initialization lazily and safely"-type of code that would make m_stone a lot happier. But in the meantime the two/three lines Tomeu suggested will still make things safer without adding lots of code; I will resubmit the patch and hopefully that will make everyone a bit happier. > > Thanks, > > > > Tomeu > > Martin Martin pgp19PvPR6OJ3.pgp Description: PGP signature ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On Tue, May 06, 2008 at 04:39:30PM +0200, Tomeu Vizoso wrote:
> On 5/4/08, Martin Dengler <[EMAIL PROTECTED]> wrote:
> > On Tue, Apr 29, 2008 at 02:12:58PM +0200, Tomeu Vizoso wrote:
> > > I think that Michael has spotted here a wonderful opportunity for
> > > making Sugar more robust, thanks to your patch.
...
> > I haven't forgotten this thread, just been unable to work on it.
> > After an hour of messing about with some ideas/code today, my current
> > approach is a bit more involved than just a try/except but nothing too
> > dramatic (just a small yak to shave):
> >
> > - refactor sugar/model/devices/device{,smodel}.py to make
> > adding/removing device.Device subclasses safe
> >
> > ... this begs one to do the next step or be left with many many
> > try/excepts and very ugly code (or just one try/except around
> > speaker.py, which kind of doesn't improve matters that much - though
> > it's very simple code and I'm happy to move on to other things if
> > people would accept that :) )
> >
> > - refactor sugar/model/devices/device{,semodel}.py to make discovering
> > device python files safe using metaclasses (see
> > http://blogs.gnome.org/jamesh/2008/02/12/python-metaclasses/ ); I
> > have considered the objections and alternatives including martian,
> > Zope's plugin framework design, and settled on this approach because
> > it's very simple, meets the very simple needs we have, and is very
> > little code
> >
> > ...this requires one to remove the networkmanager- and
> > halmanager-specific logic in devicesmodel.py:
> >
> > - create a new networkmanager model class that will add_device() and
> > remove_device() using the same logic that used to be in
> > devicesmodel.py
> >
> > - create a new halmanager model class in the same way
> >
> > ...and then we only need to:
> >
> > - trivially update battery.py and speaker.py
> >
> > - pretty trivially update network/mesh.py, wired.py, wireless.py;
> > these are only slightly more than trivial because devicesmodel.py
> > used to define some very network-specific variables that (IMO)
> > should be exposed by the networkmanager model (above) instead
> >
> > This explanation is long-winded only because I wanted to allow people
> > to poke holes in the approach, not because it's a lot of work.
>
> If it's not much work, can you provide a patch that gives an idea of
> what you plan to do? No need to be the final patch right now.
>
> Your ideas look interesting but I'm having a bit of difficulty in
> visualizing how you would refactor things.
This is not a patch because I think it's easier to read due to the
volume of refactoring, and it's very very much incomplete and
unpolished (as I said it's an hour's work), but if you look at them in
order you'll get an idea of what I'm trying, hopefully:
http://pastebin.be/11020 - devicesmodel.py - note this is refactored
to be much simpler, and the major *new* feature is simply the
metaclass usage (I still have to make it import everything in the
model.devices subtree, but you see where it's going)
http://pastebin.be/11021 - device.py - again, much simpler now and the
main changes are 1) register with the metaclass; and 2) subclassers
shoudl implement realize() rather than __init__() to do the real work
http://pastebin.be/11022 - battery.py - example of the changes; they
are basically trivial (superclass, move __init__() to realize(), call
super in realize())
http://pastebin.be/11023 - networkmanagermodel.py - this is where a
lot of devicesmodel.py has gone, because this is really a "meta
device" that creates new devices.Device subclasses as NM tells us
about device appearance/disappearance. This is completely in flux and
definitely doesn't work, but at least the comments might show you
where I'm going (this is my bullet point #3 above)
> Thanks,
>
> Tomeu
Martin
pgp6oA04ymjJy.pgp
Description: PGP signature
___
Sugar mailing list
Sugar@lists.laptop.org
http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On 5/4/08, Martin Dengler <[EMAIL PROTECTED]> wrote:
> On Tue, Apr 29, 2008 at 02:12:58PM +0200, Tomeu Vizoso wrote:
> > I think that Michael has spotted here a wonderful opportunity for
> > making Sugar more robust, thanks to your patch.
> >
> > What about just making the shell to catch (and log) any exception
> > resulting from the initialization of a device? That should amount to
> > just add one try..except block.
> >
> > Devices are thought to be easily added, so its an expected source of
> > new bugs. The shell should be able to start when any of them fail.
>
> I haven't forgotten this thread, just been unable to work on it.
> After an hour of messing about with some ideas/code today, my current
> approach is a bit more involved than just a try/except but nothing too
> dramatic (just a small yak to shave):
>
> - refactor sugar/model/devices/device{,smodel}.py to make
> adding/removing device.Device subclasses safe
>
> ... this begs one to do the next step or be left with many many
> try/excepts and very ugly code (or just one try/except around
> speaker.py, which kind of doesn't improve matters that much - though
> it's very simple code and I'm happy to move on to other things if
> people would accept that :) )
>
> - refactor sugar/model/devices/device{,semodel}.py to make discovering
> device python files safe using metaclasses (see
> http://blogs.gnome.org/jamesh/2008/02/12/python-metaclasses/ ); I
> have considered the objections and alternatives including martian,
> Zope's plugin framework design, and settled on this approach because
> it's very simple, meets the very simple needs we have, and is very
> little code
>
> ...this requires one to remove the networkmanager- and
> halmanager-specific logic in devicesmodel.py:
>
> - create a new networkmanager model class that will add_device() and
> remove_device() using the same logic that used to be in
> devicesmodel.py
>
> - create a new halmanager model class in the same way
>
> ...and then we only need to:
>
> - trivially update battery.py and speaker.py
>
> - pretty trivially update network/mesh.py, wired.py, wireless.py;
> these are only slightly more than trivial because devicesmodel.py
> used to define some very network-specific variables that (IMO)
> should be exposed by the networkmanager model (above) instead
>
> This explanation is long-winded only because I wanted to allow people
> to poke holes in the approach, not because it's a lot of work.
If it's not much work, can you provide a patch that gives an idea of
what you plan to do? No need to be the final patch right now.
Your ideas look interesting but I'm having a bit of difficulty in
visualizing how you would refactor things.
Thanks,
Tomeu
___
Sugar mailing list
Sugar@lists.laptop.org
http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On Tue, Apr 29, 2008 at 02:12:58PM +0200, Tomeu Vizoso wrote:
> I think that Michael has spotted here a wonderful opportunity for
> making Sugar more robust, thanks to your patch.
>
> What about just making the shell to catch (and log) any exception
> resulting from the initialization of a device? That should amount to
> just add one try..except block.
>
> Devices are thought to be easily added, so its an expected source of
> new bugs. The shell should be able to start when any of them fail.
I haven't forgotten this thread, just been unable to work on it.
After an hour of messing about with some ideas/code today, my current
approach is a bit more involved than just a try/except but nothing too
dramatic (just a small yak to shave):
- refactor sugar/model/devices/device{,smodel}.py to make
adding/removing device.Device subclasses safe
... this begs one to do the next step or be left with many many
try/excepts and very ugly code (or just one try/except around
speaker.py, which kind of doesn't improve matters that much - though
it's very simple code and I'm happy to move on to other things if
people would accept that :) )
- refactor sugar/model/devices/device{,semodel}.py to make discovering
device python files safe using metaclasses (see
http://blogs.gnome.org/jamesh/2008/02/12/python-metaclasses/ ); I
have considered the objections and alternatives including martian,
Zope's plugin framework design, and settled on this approach because
it's very simple, meets the very simple needs we have, and is very
little code
...this requires one to remove the networkmanager- and
halmanager-specific logic in devicesmodel.py:
- create a new networkmanager model class that will add_device() and
remove_device() using the same logic that used to be in
devicesmodel.py
- create a new halmanager model class in the same way
...and then we only need to:
- trivially update battery.py and speaker.py
- pretty trivially update network/mesh.py, wired.py, wireless.py;
these are only slightly more than trivial because devicesmodel.py
used to define some very network-specific variables that (IMO)
should be exposed by the networkmanager model (above) instead
This explanation is long-winded only because I wanted to allow people
to poke holes in the approach, not because it's a lot of work.
> Thanks,
>
> Tomeu
Martin
pgpo4ewaNMhgI.pgp
Description: PGP signature
___
Sugar mailing list
Sugar@lists.laptop.org
http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On Tue, Apr 29, 2008 at 2:36 PM, Marco Pesenti Gritti <[EMAIL PROTECTED]> wrote: > On Tue, Apr 29, 2008 at 2:28 PM, Tomeu Vizoso <[EMAIL PROTECTED]> wrote: > > The idea is that we have a complex system, and that value can be seen > > in having this system degrading gracefully when an unexpected error > > happens. > > I see that point. > > > > Even more when is an area where kids are supposed to play > > with (adding device icons to the shell). > > I'm not sure we can do this kind of assumption about what kids will play > with. Don't you think there are areas in the shell that are more likely to be customized? Tomeu ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On Tue, Apr 29, 2008 at 2:28 PM, Tomeu Vizoso <[EMAIL PROTECTED]> wrote: > The idea is that we have a complex system, and that value can be seen > in having this system degrading gracefully when an unexpected error > happens. I see that point. > Even more when is an area where kids are supposed to play > with (adding device icons to the shell). I'm not sure we can do this kind of assumption about what kids will play with. Marco ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
The idea is that we have a complex system, and that value can be seen
in having this system degrading gracefully when an unexpected error
happens. Even more when is an area where kids are supposed to play
with (adding device icons to the shell).
Adding just one try..except block before gtk.main won't give us this
graceful degradation.
Tomeu
On Tue, Apr 29, 2008 at 2:24 PM, Marco Pesenti Gritti
<[EMAIL PROTECTED]> wrote:
> I'm not really sure about the value of this kind of try/except blocks.
> If the target is to avoid the shell exiting in any case, we could as
> well try/except the whole code executed before gtk.main().
>
> Marco
>
>
>
> On Tue, Apr 29, 2008 at 2:12 PM, Tomeu Vizoso <[EMAIL PROTECTED]> wrote:
> >
> > On Tue, Apr 29, 2008 at 1:37 AM, Martin Dengler
> > <[EMAIL PROTECTED]> wrote:
> > > On Mon, Apr 28, 2008 at 06:12:31PM -0400, Michael Stone wrote:
> > > > On Mon, Apr 28, 2008 at 10:26:21PM +0100, Martin Dengler wrote:
> > > > > On Mon, Apr 28, 2008 at 01:08:04PM -0400, Michael Stone wrote:
> > > > > > Can calls to self._mixer or self._master fail even when these
> attributes
> > > > > > are not None?
> > > > >
> > > > > It doesn't appear a concern from the existing hardwaremanager.py
> code,
> > > > > and not in practice: I've tested checking/changing the volume in a
> few
> > > > > activities.
> > > >
> > > > I seem to recall having encountered situations where sugar startup
> would
> > > > fail (in early versions of the QEMU image, before sound began
> working)
> > > > as a result of unchecked use of sound hardware. I fixed the problem
> by
> > > > commenting out volume control in the sugar shell. It was a
> particularly
> > > > annoying problem because it was persistent, which meant that X
> entered
> > > > an infinite reboot loop.
> > >
> > > Yes, that problem exists and my patch is no worse in this respect - I
> > > meant to make both points very explicit later in the email to which
> > > you replied. Given the clear implication that we should do better,
> > > I'll change my patch.
> > >
> > > If you, or marco, or anyone has an opinion about where the best place
> > > to introduce the real paranoia, please let me know. OTTOMH, given we
> > > obviously want to make Sugar not crash and (secondarily) minimize
> > > spamming of 'try:...except's, hardwaremanager.py's where I would
> > > introduce the bulk of the changes (rather than make speaker.py,
> > > randomactivity.py,
> > > presence-palette-that-wants-to-make-a-dinging-noise.py, etc. do this).
> > >
> > > If anyone thinks that's controversial let me know.
> > >
> > >
> > > > > The original author of the hardwaremanager.py trusted the gst
> classes
> > > > > just as much as I am... also keep in mind that I actually saw and
> > > > > worked around two failures (#6933, #6934) of exactly these
> > > > > attributes/implementations,
> > > >
> > > > In your opinion, did the original author of hardwaremanager.py (or
> > > > authors of the clients of hardwaremanager.py?) exercise the degree of
> > > > caution necessary to deliver a solid, reliable Sugar experience to
> our
> > > > present audience? (I say "present" audience because I think that our
> > > > audience has changed from one which consisted primarily of
> developers to
> > > > one which consists primarily of semi-literate children.)
> > >
> > > As a rhetorical question I think I understand the point. As a
> > > non-rhetorical question, I'll decline to answer due to lack of
> > > context/familiarity with the conditions.
> > >
> > >
> > > > > > Also, what happens if an exception is thrown by, say,
> Device.__init__()?
> > > > >
> > > > > Given the current state of error checking, sugar (the shell) will
> fail
> > > > > to start and matchbox will exit (I saw this during testing, and
> just
> > > > > now tried 'raise Exception("we love m_stone")' to confirm.)
> > > >
> > > > Is the exception properly recorded?
> > >
> > > I'm sorry, I will have to research the proper way to record such an
> > > exception. I don't know.
> > >
> > >
> > > > Is it possible (easy?) to send the traceback upstream to us?
> > >
> > > Very interesting idea. Is there a plan for aggregating such data?
> > > cscott's FISL presenation had some (http-sourced?) usage graphs...is
> > > there a "Send Microsoft a Report"- (or "We Share Your Pain" :)) like
> > > server to which our code could send such reports? Do you want
> > > automated/staged trac submissions? The design/architecture/maintenance
> > > solution space is well beyond my time to explore. Lacking any leads
> > > therein I'm going to answer to your question as "I know not this
> > > 'upstream', would be happy to use one, but have no resources to build
> > > one".
> > >
> > >
> > > > > Device.__init__() is four lines
Re: [sugar] [PATCH] Add speaker device and icon by default
I'm not really sure about the value of this kind of try/except blocks.
If the target is to avoid the shell exiting in any case, we could as
well try/except the whole code executed before gtk.main().
Marco
On Tue, Apr 29, 2008 at 2:12 PM, Tomeu Vizoso <[EMAIL PROTECTED]> wrote:
>
> On Tue, Apr 29, 2008 at 1:37 AM, Martin Dengler
> <[EMAIL PROTECTED]> wrote:
> > On Mon, Apr 28, 2008 at 06:12:31PM -0400, Michael Stone wrote:
> > > On Mon, Apr 28, 2008 at 10:26:21PM +0100, Martin Dengler wrote:
> > > > On Mon, Apr 28, 2008 at 01:08:04PM -0400, Michael Stone wrote:
> > > > > Can calls to self._mixer or self._master fail even when these
> attributes
> > > > > are not None?
> > > >
> > > > It doesn't appear a concern from the existing hardwaremanager.py code,
> > > > and not in practice: I've tested checking/changing the volume in a few
> > > > activities.
> > >
> > > I seem to recall having encountered situations where sugar startup would
> > > fail (in early versions of the QEMU image, before sound began working)
> > > as a result of unchecked use of sound hardware. I fixed the problem by
> > > commenting out volume control in the sugar shell. It was a particularly
> > > annoying problem because it was persistent, which meant that X entered
> > > an infinite reboot loop.
> >
> > Yes, that problem exists and my patch is no worse in this respect - I
> > meant to make both points very explicit later in the email to which
> > you replied. Given the clear implication that we should do better,
> > I'll change my patch.
> >
> > If you, or marco, or anyone has an opinion about where the best place
> > to introduce the real paranoia, please let me know. OTTOMH, given we
> > obviously want to make Sugar not crash and (secondarily) minimize
> > spamming of 'try:...except's, hardwaremanager.py's where I would
> > introduce the bulk of the changes (rather than make speaker.py,
> > randomactivity.py,
> > presence-palette-that-wants-to-make-a-dinging-noise.py, etc. do this).
> >
> > If anyone thinks that's controversial let me know.
> >
> >
> > > > The original author of the hardwaremanager.py trusted the gst classes
> > > > just as much as I am... also keep in mind that I actually saw and
> > > > worked around two failures (#6933, #6934) of exactly these
> > > > attributes/implementations,
> > >
> > > In your opinion, did the original author of hardwaremanager.py (or
> > > authors of the clients of hardwaremanager.py?) exercise the degree of
> > > caution necessary to deliver a solid, reliable Sugar experience to our
> > > present audience? (I say "present" audience because I think that our
> > > audience has changed from one which consisted primarily of developers to
> > > one which consists primarily of semi-literate children.)
> >
> > As a rhetorical question I think I understand the point. As a
> > non-rhetorical question, I'll decline to answer due to lack of
> > context/familiarity with the conditions.
> >
> >
> > > > > Also, what happens if an exception is thrown by, say,
> Device.__init__()?
> > > >
> > > > Given the current state of error checking, sugar (the shell) will fail
> > > > to start and matchbox will exit (I saw this during testing, and just
> > > > now tried 'raise Exception("we love m_stone")' to confirm.)
> > >
> > > Is the exception properly recorded?
> >
> > I'm sorry, I will have to research the proper way to record such an
> > exception. I don't know.
> >
> >
> > > Is it possible (easy?) to send the traceback upstream to us?
> >
> > Very interesting idea. Is there a plan for aggregating such data?
> > cscott's FISL presenation had some (http-sourced?) usage graphs...is
> > there a "Send Microsoft a Report"- (or "We Share Your Pain" :)) like
> > server to which our code could send such reports? Do you want
> > automated/staged trac submissions? The design/architecture/maintenance
> > solution space is well beyond my time to explore. Lacking any leads
> > therein I'm going to answer to your question as "I know not this
> > 'upstream', would be happy to use one, but have no resources to build
> > one".
> >
> >
> > > > Device.__init__() is four lines of code, and depends on
> > > > util.unique_id() and gobject.GObject.__init__().
> > >
> > > Device.__init__() sounds like the sort of thing that I expect will be
> > > overridden by subclasses in interesting ways and it sounds like the sort
> > > of thing that will break when people try to run Sugar on platforms we
> > > haven't tested pre-qualified.
> >
> > I think you're encouraging me to make Device.__init__() a bit safer,
> > or add some comments, or something similar, rather than changing
> > speaker.py's __init__(). It's going to get a bit hairy if we can't
> > trust our superclass(es)'s constructor(s). Or perhaps you'd have me
> > consider patching devices.py, to survive any of
Re: [sugar] [PATCH] Add speaker device and icon by default
On Tue, Apr 29, 2008 at 1:37 AM, Martin Dengler
<[EMAIL PROTECTED]> wrote:
> On Mon, Apr 28, 2008 at 06:12:31PM -0400, Michael Stone wrote:
> > On Mon, Apr 28, 2008 at 10:26:21PM +0100, Martin Dengler wrote:
> > > On Mon, Apr 28, 2008 at 01:08:04PM -0400, Michael Stone wrote:
> > > > Can calls to self._mixer or self._master fail even when these
> attributes
> > > > are not None?
> > >
> > > It doesn't appear a concern from the existing hardwaremanager.py code,
> > > and not in practice: I've tested checking/changing the volume in a few
> > > activities.
> >
> > I seem to recall having encountered situations where sugar startup would
> > fail (in early versions of the QEMU image, before sound began working)
> > as a result of unchecked use of sound hardware. I fixed the problem by
> > commenting out volume control in the sugar shell. It was a particularly
> > annoying problem because it was persistent, which meant that X entered
> > an infinite reboot loop.
>
> Yes, that problem exists and my patch is no worse in this respect - I
> meant to make both points very explicit later in the email to which
> you replied. Given the clear implication that we should do better,
> I'll change my patch.
>
> If you, or marco, or anyone has an opinion about where the best place
> to introduce the real paranoia, please let me know. OTTOMH, given we
> obviously want to make Sugar not crash and (secondarily) minimize
> spamming of 'try:...except's, hardwaremanager.py's where I would
> introduce the bulk of the changes (rather than make speaker.py,
> randomactivity.py,
> presence-palette-that-wants-to-make-a-dinging-noise.py, etc. do this).
>
> If anyone thinks that's controversial let me know.
>
>
> > > The original author of the hardwaremanager.py trusted the gst classes
> > > just as much as I am... also keep in mind that I actually saw and
> > > worked around two failures (#6933, #6934) of exactly these
> > > attributes/implementations,
> >
> > In your opinion, did the original author of hardwaremanager.py (or
> > authors of the clients of hardwaremanager.py?) exercise the degree of
> > caution necessary to deliver a solid, reliable Sugar experience to our
> > present audience? (I say "present" audience because I think that our
> > audience has changed from one which consisted primarily of developers to
> > one which consists primarily of semi-literate children.)
>
> As a rhetorical question I think I understand the point. As a
> non-rhetorical question, I'll decline to answer due to lack of
> context/familiarity with the conditions.
>
>
> > > > Also, what happens if an exception is thrown by, say,
> Device.__init__()?
> > >
> > > Given the current state of error checking, sugar (the shell) will fail
> > > to start and matchbox will exit (I saw this during testing, and just
> > > now tried 'raise Exception("we love m_stone")' to confirm.)
> >
> > Is the exception properly recorded?
>
> I'm sorry, I will have to research the proper way to record such an
> exception. I don't know.
>
>
> > Is it possible (easy?) to send the traceback upstream to us?
>
> Very interesting idea. Is there a plan for aggregating such data?
> cscott's FISL presenation had some (http-sourced?) usage graphs...is
> there a "Send Microsoft a Report"- (or "We Share Your Pain" :)) like
> server to which our code could send such reports? Do you want
> automated/staged trac submissions? The design/architecture/maintenance
> solution space is well beyond my time to explore. Lacking any leads
> therein I'm going to answer to your question as "I know not this
> 'upstream', would be happy to use one, but have no resources to build
> one".
>
>
> > > Device.__init__() is four lines of code, and depends on
> > > util.unique_id() and gobject.GObject.__init__().
> >
> > Device.__init__() sounds like the sort of thing that I expect will be
> > overridden by subclasses in interesting ways and it sounds like the sort
> > of thing that will break when people try to run Sugar on platforms we
> > haven't tested pre-qualified.
>
> I think you're encouraging me to make Device.__init__() a bit safer,
> or add some comments, or something similar, rather than changing
> speaker.py's __init__(). It's going to get a bit hairy if we can't
> trust our superclass(es)'s constructor(s). Or perhaps you'd have me
> consider patching devices.py, to survive any of its devices' not
> initializing.
>
> If you / anyone has a preference for either approach, or how I
> structure the changes into one or more patches (this is part of the
> culture that I haven't picked up yet), please let me know.
>
> Otherwise I will go forward with what you've said on the principle
> that you/others would rather slap my code back/down than micromanage
> me forward.
>
>
> > > and no other similar bit of code... does any more checking than this.
> >
> > Is this good? In particular, is it good that an exception that bu
Re: [sugar] [PATCH] Add speaker device and icon by default
On Mon, Apr 28, 2008 at 06:12:31PM -0400, Michael Stone wrote:
> On Mon, Apr 28, 2008 at 10:26:21PM +0100, Martin Dengler wrote:
> > On Mon, Apr 28, 2008 at 01:08:04PM -0400, Michael Stone wrote:
> > > Can calls to self._mixer or self._master fail even when these attributes
> > > are not None?
> >
> > It doesn't appear a concern from the existing hardwaremanager.py code,
> > and not in practice: I've tested checking/changing the volume in a few
> > activities.
>
> I seem to recall having encountered situations where sugar startup would
> fail (in early versions of the QEMU image, before sound began working)
> as a result of unchecked use of sound hardware. I fixed the problem by
> commenting out volume control in the sugar shell. It was a particularly
> annoying problem because it was persistent, which meant that X entered
> an infinite reboot loop.
Yes, that problem exists and my patch is no worse in this respect - I
meant to make both points very explicit later in the email to which
you replied. Given the clear implication that we should do better,
I'll change my patch.
If you, or marco, or anyone has an opinion about where the best place
to introduce the real paranoia, please let me know. OTTOMH, given we
obviously want to make Sugar not crash and (secondarily) minimize
spamming of 'try:...except's, hardwaremanager.py's where I would
introduce the bulk of the changes (rather than make speaker.py,
randomactivity.py,
presence-palette-that-wants-to-make-a-dinging-noise.py, etc. do this).
If anyone thinks that's controversial let me know.
> > The original author of the hardwaremanager.py trusted the gst classes
> > just as much as I am... also keep in mind that I actually saw and
> > worked around two failures (#6933, #6934) of exactly these
> > attributes/implementations,
>
> In your opinion, did the original author of hardwaremanager.py (or
> authors of the clients of hardwaremanager.py?) exercise the degree of
> caution necessary to deliver a solid, reliable Sugar experience to our
> present audience? (I say "present" audience because I think that our
> audience has changed from one which consisted primarily of developers to
> one which consists primarily of semi-literate children.)
As a rhetorical question I think I understand the point. As a
non-rhetorical question, I'll decline to answer due to lack of
context/familiarity with the conditions.
> > > Also, what happens if an exception is thrown by, say, Device.__init__()?
> >
> > Given the current state of error checking, sugar (the shell) will fail
> > to start and matchbox will exit (I saw this during testing, and just
> > now tried 'raise Exception("we love m_stone")' to confirm.)
>
> Is the exception properly recorded?
I'm sorry, I will have to research the proper way to record such an
exception. I don't know.
> Is it possible (easy?) to send the traceback upstream to us?
Very interesting idea. Is there a plan for aggregating such data?
cscott's FISL presenation had some (http-sourced?) usage graphs...is
there a "Send Microsoft a Report"- (or "We Share Your Pain" :)) like
server to which our code could send such reports? Do you want
automated/staged trac submissions? The design/architecture/maintenance
solution space is well beyond my time to explore. Lacking any leads
therein I'm going to answer to your question as "I know not this
'upstream', would be happy to use one, but have no resources to build
one".
> > Device.__init__() is four lines of code, and depends on
> > util.unique_id() and gobject.GObject.__init__().
>
> Device.__init__() sounds like the sort of thing that I expect will be
> overridden by subclasses in interesting ways and it sounds like the sort
> of thing that will break when people try to run Sugar on platforms we
> haven't tested pre-qualified.
I think you're encouraging me to make Device.__init__() a bit safer,
or add some comments, or something similar, rather than changing
speaker.py's __init__(). It's going to get a bit hairy if we can't
trust our superclass(es)'s constructor(s). Or perhaps you'd have me
consider patching devices.py, to survive any of its devices' not
initializing.
If you / anyone has a preference for either approach, or how I
structure the changes into one or more patches (this is part of the
culture that I haven't picked up yet), please let me know.
Otherwise I will go forward with what you've said on the principle
that you/others would rather slap my code back/down than micromanage
me forward.
> > and no other similar bit of code... does any more checking than this.
>
> Is this good? In particular, is it good that an exception that bubbles
> up through Device.__init__() causes X to enter an infinite restart loop?
ibid.
> > And please excuse me if I've missed a howler of a bug that you're
> > socraticly/patiently trying to make me realize - just feel free to say
> > outright what sucks and I'll fix it!
>
> You seem to be telling me that Sugar will crash if any of its d
Re: [sugar] [PATCH] Add speaker device and icon by default
On Mon, Apr 28, 2008 at 10:26:21PM +0100, Martin Dengler wrote:
> On Mon, Apr 28, 2008 at 01:08:04PM -0400, Michael Stone wrote:
> > Can calls to self._mixer or self._master fail even when these attributes
> > are not None?
>
> It doesn't appear a concern from the existing hardwaremanager.py code,
> and not in practice: I've tested checking/changing the volume in a few
> activities.
I seem to recall having encountered situations where sugar startup would
fail (in early versions of the QEMU image, before sound began working)
as a result of unchecked use of sound hardware. I fixed the problem by
commenting out volume control in the sugar shell. It was a particularly
annoying problem because it was persistent, which meant that X entered
an infinite reboot loop.
> The original author of the hardwaremanager.py trusted the gst classes
> just as much as I am... also keep in mind that I actually saw and
> worked around two failures (#6933, #6934) of exactly these
> attributes/implementations,
In your opinion, did the original author of hardwaremanager.py (or
authors of the clients of hardwaremanager.py?) exercise the degree of
caution necessary to deliver a solid, reliable Sugar experience to our
present audience? (I say "present" audience because I think that our
audience has changed from one which consisted primarily of developers to
one which consists primarily of semi-literate children.)
> > Also, what happens if an exception is thrown by, say, Device.__init__()?
>
> Given the current state of error checking, sugar (the shell) will fail
> to start and matchbox will exit (I saw this during testing, and just
> now tried 'raise Exception("we love m_stone")' to confirm.)
Is the exception properly recorded? Is it possible (easy?) to send the
traceback upstream to us?
> Device.__init__() is four lines of code, and depends on
> util.unique_id() and gobject.GObject.__init__().
Device.__init__() sounds like the sort of thing that I expect will be
overridden by subclasses in interesting ways and it sounds like the sort
of thing that will break when people try to run Sugar on platforms we
haven't tested pre-qualified.
> and no other similar bit of code... does any more checking than this.
Is this good? In particular, is it good that an exception that bubbles
up through Device.__init__() causes X to enter an infinite restart loop?
> And please excuse me if I've missed a howler of a bug that you're
> socraticly/patiently trying to make me realize - just feel free to say
> outright what sucks and I'll fix it!
You seem to be telling me that Sugar will crash if any of its device
abstractions fails to initialize. That seems like a howler of a bug to
me (though perhaps not one in your code). Is this desirable behavior? Is
it intentional?
Thanks for putting up me,
Michael
___
Sugar mailing list
Sugar@lists.laptop.org
http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On Mon, Apr 28, 2008 at 01:08:04PM -0400, Michael Stone wrote:
> Can calls to self._mixer or self._master fail even when these attributes
> are not None?
It doesn't appear a concern from the existing hardwaremanager.py code,
and not in practice: I've tested checking/changing the volume in a few
activities.
The original author of the hardwaremanager.py trusted the gst classes
just as much as I am, though I wouldn't want to hide behind that; if
you're concerned I haven't done enough research about these APIs (I
love abusing abstraction and limited documentation in a language
without checked exceptions to write code fast:)), well, that's
understandable (though I have done some research on GstElement[1,2],
GstState[3], and the anemic state of python-gst documentation (could
be a sign of great code :))[4,5] and I think it's OK enough for me).
Also keep in mind that I actually saw and worked around two failures
(#6933, #6934) of exactly these attributes/implementations, so if your
concern is that they'll DoS/make Sugar more fragile, I'm happy to add
a bit more paranoia to hardwaremanager.py - so far I just have the
existing code to guide me as to what level of paranoia is justified,
but as you're the one paid to be paranoid :) I of course would not
object to being corrected / directed differently.
Can you explain a bit more about your concern(s) so I can address them
better?
> Also, what happens if an exception is thrown by, say, Device.__init__()?
Given the current state of error checking, sugar (the shell) will fail
to start and matchbox will exit (I saw this during testing, and just
now tried 'raise Exception("we love m_stone")' to confirm.)
Device.__init__() is four lines of code, and depends on
util.unique_id() and gobject.GObject.__init__(). I'm no expert on
those two, but that seems trivial/robust enough to not wrap in a
try/except, and no other similar bit of code (battery.py,
network/*.py) that depends on Device.__init__() and has the same
exposure to its behavior does any more checking than this.
Again, can you explain a bit more about your concern(s) - e.g., would
you prefer a try/except in speaker.py, or device.py, or something
else? Or were you just curious as to the likelihood of
failure/complexity of Device.__init__()?
And please excuse me if I've missed a howler of a bug that you're
socraticly/patiently trying to make me realize - just feel free to say
outright what sucks and I'll fix it!
> Thanks,
>
> Michael
Martin
1.
http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/GstElementFactory.html#gst-element-factory-make
2.
http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/GstElement.html
3.
http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/GstElement.html#GstState
4. The example for interacting with the mixer:
http://webcvs.freedesktop.org/gstreamer/gst-python/examples/mixer.py?revision=1.2&view=markup
. Of course it's an example and it's got about 4 PEP 008 style
violations that smacked me on the head, but the last commit is an
attempt to make it not grossly fail under some normal error condition
so it's at least a minimum bar of how to interact with the mixer,
which it seems hardwaremanager.py vaults easily, before and after my
patch :).
5. http://www.jonobacon.org/?p=750 <-- this is the author of
python-gst, it seems, complaining about the lack of documentation but
doing a really good job of starting to rectify the situation :)
pgpUOuaQUKeKt.pgp
Description: PGP signature
___
Sugar mailing list
Sugar@lists.laptop.org
http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
Can calls to self._mixer or self._master fail even when these attributes are not None? Also, what happens if an exception is thrown by, say, Device.__init__()? Thanks, Michael ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
[sugar] [PATCH] Add speaker device and icon by default
Add speaker device and icon by default, as in
http://wiki.laptop.org/go/Designs/Frame#07
---
src/hardware/hardwaremanager.py | 31 +-
src/model/devices/Makefile.am |4 +-
src/model/devices/devicesmodel.py |3 +
src/model/devices/speaker.py | 59 ++
src/view/devices/Makefile.am |4 +-
src/view/devices/speaker.py | 119 +
6 files changed, 215 insertions(+), 5 deletions(-)
create mode 100644 src/model/devices/speaker.py
create mode 100644 src/view/devices/speaker.py
diff --git a/src/hardware/hardwaremanager.py b/src/hardware/hardwaremanager.py
index 29be450..42676e4 100644
--- a/src/hardware/hardwaremanager.py
+++ b/src/hardware/hardwaremanager.py
@@ -50,6 +50,13 @@ class HardwareManager(object):
if track.flags & gst.interfaces.MIXER_TRACK_MASTER:
self._master = track
+def get_mute(self):
+if not self._mixer or not self._master:
+logging.error('Cannot get the mute status')
+return False
+return self._master.flags & gst.interfaces.MIXER_TRACK_MUTE \
+ == gst.interfaces.MIXER_TRACK_MUTE
+
def get_volume(self):
if not self._mixer or not self._master:
logging.error('Cannot get the volume')
@@ -57,9 +64,19 @@ class HardwareManager(object):
max_volume = self._master.max_volume
min_volume = self._master.min_volume
-volume = self._mixer.get_volume(self._master)[0]
-return volume * 100.0 / (max_volume - min_volume) + min_volume
+volumes = self._mixer.get_volume(self._master)
+
+#sometimes we get a spurious zero from one/more channel(s)
+#TODO: consider removing this when trac #6933 is resolved
+nonzero_volumes = [v for v in volumes if v > 0]
+
+if len(nonzero_volumes) > 0:
+#we could just pick the first nonzero volume, but this converges
+volume = sum(nonzero_volumes) / len(nonzero_volumes)
+return volume * 100.0 / (max_volume - min_volume) + min_volume
+else:
+return 0
def set_volume(self, volume):
if not self._mixer or not self._master:
@@ -76,7 +93,15 @@ class HardwareManager(object):
volume = volume * (max_volume - min_volume) / 100.0 + min_volume
volume_list = [ volume ] * self._master.num_channels
-self._mixer.set_volume(self._master, tuple(volume_list))
+#sometimes alsa sets one/more channels' volume to zero instead
+# of what we asked for, so try a few times
+#TODO: consider removing this loop when trac #6934 is resolved
+last_volumes_read = [0]
+read_count = 0
+while (0 in last_volumes_read) and (read_count < 3):
+self._mixer.set_volume(self._master, tuple(volume_list))
+last_volumes_read = self._mixer.get_volume(self._master)
+read_count += 1
def set_mute(self, mute):
if not self._mixer or not self._master:
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
+
diff --git a/src/model/devices/devicesmodel.py
b/src/model/devices/devicesmodel.py
index 691e19e..32c77da 100644
--- a/src/model/devices/devicesmodel.py
+++ b/src/model/devices/devicesmodel.py
@@ -23,6 +23,7 @@ from model.devices import device
from model.devices.network import wireless
from model.devices.network import mesh
from model.devices import battery
+from model.devices import speaker
from hardware import hardwaremanager
from hardware import nmclient
@@ -54,6 +55,8 @@ class DevicesModel(gobject.GObject):
for udi in hal_manager.FindDeviceByCapability('battery'):
self.add_device(battery.Device(udi))
+self.add_device(speaker.Device())
+
def _observe_network_manager(self):
network_manager = hardwaremanager.get_network_manager()
if not network_manager:
diff --git a/src/model/devices/speaker.py b/src/model/devices/speaker.py
new file mode 100644
index 000..d3b9967
--- /dev/null
+++ b/src/model/devices/speaker.py
@@ -0,0 +1,59 @@
+# Copyright (C) 2008 Martin Dengler
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for
Re: [sugar] [PATCH] Add speaker device and icon by default
On Mon, Apr 28, 2008 at 03:49:12PM +0200, Tomeu Vizoso wrote:
> On Fri, Apr 25, 2008 at 11:50 PM, Martin Dengler
> <[EMAIL PROTECTED]> wrote:
>
> > +self.palette = SpeakerPalette(_('My Speakers'), model=model)
> > +self.set_palette(self.palette)
>
> 'set_palette' is the setter for the 'palette' property, so the second
> line shouldn't be needed.
Thanks, I'll remove. That code was cut & pasted from battery.py, so
I'm guilty of cargo-culting here.
>
> > +model.connect('notify::level', self._speaker_status_changed_cb)
> > +model.connect('notify::muted', self._speaker_status_changed_cb)
> > +self.connect('expose-event', self._expose_event_cb)
>
> Callbacks should start with two underscores in order to avoid name
> clashes.
Thanks -- others have pointed this out as well in other code sections
and I guess I missed those.
> Tomeu
Martin
pgpVzj1mJl35E.pgp
Description: PGP signature
___
Sugar mailing list
Sugar@lists.laptop.org
http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On Fri, Apr 25, 2008 at 11:50 PM, Martin Dengler
<[EMAIL PROTECTED]> wrote:
> +self.palette = SpeakerPalette(_('My Speakers'), model=model)
> +self.set_palette(self.palette)
'set_palette' is the setter for the 'palette' property, so the second
line shouldn't be needed.
> +model.connect('notify::level', self._speaker_status_changed_cb)
> +model.connect('notify::muted', self._speaker_status_changed_cb)
> +self.connect('expose-event', self._expose_event_cb)
Callbacks should start with two underscores in order to avoid name clashes.
The rest looks good to me.
Thanks,
Tomeu
___
Sugar mailing list
Sugar@lists.laptop.org
http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On Sat, Apr 26, 2008 at 06:07:45AM -0400, Wade Brainerd wrote: > On Fri, Apr 25, 2008 at 1:56 PM, Eben Eliason <[EMAIL PROTECTED]> wrote: > > On Fri, Apr 25, 2008 at 1:46 PM, Martin Dengler > > > self._mute_item.get_child().set_text(_(mute_item_text)) > > > > Oh, I missed that. I would wrap the localization around the string > > literal, so the connection is clear. (Yes, you have to use it twice > > instead of oncebut it was imported as _ for a reason. ;) ) > > Won't gettext fail to pick these up when generating the .pot file, the > way it's written now? > > I was under the impression it just looks blindly through the source > for _() when building the translation files... Yes -- I didn't know this at the time. Thanks for pointing it out. Others pointed it out to me on IRC, too, and my most recent patch addresses the point. Thanks again for the time/feedback. > Wade Martin pgpAhI0Q7Jed4.pgp Description: PGP signature ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On Fri, Apr 25, 2008 at 1:56 PM, Eben Eliason <[EMAIL PROTECTED]> wrote: > On Fri, Apr 25, 2008 at 1:46 PM, Martin Dengler > > self._mute_item.get_child().set_text(_(mute_item_text)) > > Oh, I missed that. I would wrap the localization around the string > literal, so the connection is clear. (Yes, you have to use it twice > instead of oncebut it was imported as _ for a reason. ;) ) Won't gettext fail to pick these up when generating the .pot file, the way it's written now? I was under the impression it just looks blindly through the source for _() when building the translation files... Wade ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
[sugar] [PATCH] Add speaker device and icon by default
Add speaker device and icon by default, as in
http://wiki.laptop.org/go/Designs/Frame#07
---
src/hardware/hardwaremanager.py | 31 +-
src/model/devices/Makefile.am |4 +-
src/model/devices/devicesmodel.py |3 +
src/model/devices/speaker.py | 59 ++
src/view/devices/Makefile.am |4 +-
src/view/devices/speaker.py | 118 +
6 files changed, 214 insertions(+), 5 deletions(-)
create mode 100644 src/model/devices/speaker.py
create mode 100644 src/view/devices/speaker.py
diff --git a/src/hardware/hardwaremanager.py b/src/hardware/hardwaremanager.py
index 29be450..42676e4 100644
--- a/src/hardware/hardwaremanager.py
+++ b/src/hardware/hardwaremanager.py
@@ -50,6 +50,13 @@ class HardwareManager(object):
if track.flags & gst.interfaces.MIXER_TRACK_MASTER:
self._master = track
+def get_mute(self):
+if not self._mixer or not self._master:
+logging.error('Cannot get the mute status')
+return False
+return self._master.flags & gst.interfaces.MIXER_TRACK_MUTE \
+ == gst.interfaces.MIXER_TRACK_MUTE
+
def get_volume(self):
if not self._mixer or not self._master:
logging.error('Cannot get the volume')
@@ -57,9 +64,19 @@ class HardwareManager(object):
max_volume = self._master.max_volume
min_volume = self._master.min_volume
-volume = self._mixer.get_volume(self._master)[0]
-return volume * 100.0 / (max_volume - min_volume) + min_volume
+volumes = self._mixer.get_volume(self._master)
+
+#sometimes we get a spurious zero from one/more channel(s)
+#TODO: consider removing this when trac #6933 is resolved
+nonzero_volumes = [v for v in volumes if v > 0]
+
+if len(nonzero_volumes) > 0:
+#we could just pick the first nonzero volume, but this converges
+volume = sum(nonzero_volumes) / len(nonzero_volumes)
+return volume * 100.0 / (max_volume - min_volume) + min_volume
+else:
+return 0
def set_volume(self, volume):
if not self._mixer or not self._master:
@@ -76,7 +93,15 @@ class HardwareManager(object):
volume = volume * (max_volume - min_volume) / 100.0 + min_volume
volume_list = [ volume ] * self._master.num_channels
-self._mixer.set_volume(self._master, tuple(volume_list))
+#sometimes alsa sets one/more channels' volume to zero instead
+# of what we asked for, so try a few times
+#TODO: consider removing this loop when trac #6934 is resolved
+last_volumes_read = [0]
+read_count = 0
+while (0 in last_volumes_read) and (read_count < 3):
+self._mixer.set_volume(self._master, tuple(volume_list))
+last_volumes_read = self._mixer.get_volume(self._master)
+read_count += 1
def set_mute(self, mute):
if not self._mixer or not self._master:
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
+
diff --git a/src/model/devices/devicesmodel.py
b/src/model/devices/devicesmodel.py
index 691e19e..32c77da 100644
--- a/src/model/devices/devicesmodel.py
+++ b/src/model/devices/devicesmodel.py
@@ -23,6 +23,7 @@ from model.devices import device
from model.devices.network import wireless
from model.devices.network import mesh
from model.devices import battery
+from model.devices import speaker
from hardware import hardwaremanager
from hardware import nmclient
@@ -54,6 +55,8 @@ class DevicesModel(gobject.GObject):
for udi in hal_manager.FindDeviceByCapability('battery'):
self.add_device(battery.Device(udi))
+self.add_device(speaker.Device())
+
def _observe_network_manager(self):
network_manager = hardwaremanager.get_network_manager()
if not network_manager:
diff --git a/src/model/devices/speaker.py b/src/model/devices/speaker.py
new file mode 100644
index 000..d3b9967
--- /dev/null
+++ b/src/model/devices/speaker.py
@@ -0,0 +1,59 @@
+# Copyright (C) 2008 Martin Dengler
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for
[sugar] [PATCH] Add speaker device and icon by default
Add speaker device and icon by default, as in
http://wiki.laptop.org/go/Designs/Frame#07
---
src/hardware/hardwaremanager.py | 26 -
src/model/devices/Makefile.am |4 +-
src/model/devices/devicesmodel.py |3 +
src/model/devices/speaker.py | 59 ++
src/view/devices/Makefile.am |4 +-
src/view/devices/speaker.py | 118 +
6 files changed, 210 insertions(+), 4 deletions(-)
create mode 100644 src/model/devices/speaker.py
create mode 100644 src/view/devices/speaker.py
diff --git a/src/hardware/hardwaremanager.py b/src/hardware/hardwaremanager.py
index 29be450..5a8f0ad 100644
--- a/src/hardware/hardwaremanager.py
+++ b/src/hardware/hardwaremanager.py
@@ -50,6 +50,13 @@ class HardwareManager(object):
if track.flags & gst.interfaces.MIXER_TRACK_MASTER:
self._master = track
+def get_mute(self):
+if not self._mixer or not self._master:
+logging.error('Cannot get the mute status')
+return False
+return self._master.flags & gst.interfaces.MIXER_TRACK_MUTE \
+ == gst.interfaces.MIXER_TRACK_MUTE
+
def get_volume(self):
if not self._mixer or not self._master:
logging.error('Cannot get the volume')
@@ -57,7 +64,14 @@ class HardwareManager(object):
max_volume = self._master.max_volume
min_volume = self._master.min_volume
-volume = self._mixer.get_volume(self._master)[0]
+
+#sometimes we get a spurious zero from one/more channel(s)
+#TODO: consider removing this when trac # is resolved
+nonzero_volumes = [v for v in self._mixer.get_volume(self._master)
+ if v > 0]
+
+#we could just pick the first channel's volume, but this converges
+volume = sum(nonzero_volumes) / len(nonzero_volumes)
return volume * 100.0 / (max_volume - min_volume) + min_volume
@@ -76,7 +90,15 @@ class HardwareManager(object):
volume = volume * (max_volume - min_volume) / 100.0 + min_volume
volume_list = [ volume ] * self._master.num_channels
-self._mixer.set_volume(self._master, tuple(volume_list))
+#sometimes alsa sets one/more channels' volume to zero instead
+# of what we asked for, so try a few times
+#TODO: consider removing this when trac # is resolved
+last_volumes_read = [0]
+read_count = 0
+while (0 in last_volumes_read) and (read_count < 3):
+self._mixer.set_volume(self._master, tuple(volume_list))
+last_volumes_read = self._mixer.get_volume(self._master)
+read_count += 1
def set_mute(self, mute):
if not self._mixer or not self._master:
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
+
diff --git a/src/model/devices/devicesmodel.py
b/src/model/devices/devicesmodel.py
index 691e19e..32c77da 100644
--- a/src/model/devices/devicesmodel.py
+++ b/src/model/devices/devicesmodel.py
@@ -23,6 +23,7 @@ from model.devices import device
from model.devices.network import wireless
from model.devices.network import mesh
from model.devices import battery
+from model.devices import speaker
from hardware import hardwaremanager
from hardware import nmclient
@@ -54,6 +55,8 @@ class DevicesModel(gobject.GObject):
for udi in hal_manager.FindDeviceByCapability('battery'):
self.add_device(battery.Device(udi))
+self.add_device(speaker.Device())
+
def _observe_network_manager(self):
network_manager = hardwaremanager.get_network_manager()
if not network_manager:
diff --git a/src/model/devices/speaker.py b/src/model/devices/speaker.py
new file mode 100644
index 000..d3b9967
--- /dev/null
+++ b/src/model/devices/speaker.py
@@ -0,0 +1,59 @@
+# Copyright (C) 2008 Martin Dengler
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+
+import gobject
+
+from
[sugar] [PATCH] Add speaker device and icon by default
Add speaker device and icon by default, as in
http://wiki.laptop.org/go/Designs/Frame#07
---
src/hardware/hardwaremanager.py | 24 +++-
src/model/devices/Makefile.am |4 +-
src/model/devices/devicesmodel.py |3 +
src/model/devices/speaker.py | 59 ++
src/view/devices/Makefile.am |4 +-
src/view/devices/speaker.py | 118 +
6 files changed, 208 insertions(+), 4 deletions(-)
create mode 100644 src/model/devices/speaker.py
create mode 100644 src/view/devices/speaker.py
diff --git a/src/hardware/hardwaremanager.py b/src/hardware/hardwaremanager.py
index 29be450..b52a30a 100644
--- a/src/hardware/hardwaremanager.py
+++ b/src/hardware/hardwaremanager.py
@@ -50,6 +50,13 @@ class HardwareManager(object):
if track.flags & gst.interfaces.MIXER_TRACK_MASTER:
self._master = track
+def get_mute(self):
+if not self._mixer or not self._master:
+logging.error('Cannot get the mute status')
+return False
+return self._master.flags & gst.interfaces.MIXER_TRACK_MUTE \
+ == gst.interfaces.MIXER_TRACK_MUTE
+
def get_volume(self):
if not self._mixer or not self._master:
logging.error('Cannot get the volume')
@@ -57,7 +64,13 @@ class HardwareManager(object):
max_volume = self._master.max_volume
min_volume = self._master.min_volume
-volume = self._mixer.get_volume(self._master)[0]
+
+#sometimes we get a spurious zero from one/more channel(s)
+nonzero_volumes = [v for v in self._mixer.get_volume(self._master)
+ if v > 0]
+
+#we could just pick the first channel's volume, but this converges
+volume = sum(nonzero_volumes) / len(nonzero_volumes)
return volume * 100.0 / (max_volume - min_volume) + min_volume
@@ -76,7 +89,14 @@ class HardwareManager(object):
volume = volume * (max_volume - min_volume) / 100.0 + min_volume
volume_list = [ volume ] * self._master.num_channels
-self._mixer.set_volume(self._master, tuple(volume_list))
+#sometimes alsa sets one/more channels' volume to zero instead
+# of what we asked for, so try a few times
+last_volumes_read = [0]
+read_count = 0
+while (0 in last_volumes_read) and (read_count < 3):
+self._mixer.set_volume(self._master, tuple(volume_list))
+last_volumes_read = self._mixer.get_volume(self._master)
+read_count += 1
def set_mute(self, mute):
if not self._mixer or not self._master:
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
+
diff --git a/src/model/devices/devicesmodel.py
b/src/model/devices/devicesmodel.py
index 691e19e..32c77da 100644
--- a/src/model/devices/devicesmodel.py
+++ b/src/model/devices/devicesmodel.py
@@ -23,6 +23,7 @@ from model.devices import device
from model.devices.network import wireless
from model.devices.network import mesh
from model.devices import battery
+from model.devices import speaker
from hardware import hardwaremanager
from hardware import nmclient
@@ -54,6 +55,8 @@ class DevicesModel(gobject.GObject):
for udi in hal_manager.FindDeviceByCapability('battery'):
self.add_device(battery.Device(udi))
+self.add_device(speaker.Device())
+
def _observe_network_manager(self):
network_manager = hardwaremanager.get_network_manager()
if not network_manager:
diff --git a/src/model/devices/speaker.py b/src/model/devices/speaker.py
new file mode 100644
index 000..d3b9967
--- /dev/null
+++ b/src/model/devices/speaker.py
@@ -0,0 +1,59 @@
+# Copyright (C) 2008 Martin Dengler
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+
+import gobject
+
+from hardware import hardwaremanager
+from model.devices import device
+
+class Device(device.Device):
+__gproperties__ = {
+'le
Re: [sugar] [PATCH] Add speaker device and icon by default
On Fri, Apr 25, 2008 at 1:46 PM, Martin Dengler <[EMAIL PROTECTED]> wrote: > On Fri, Apr 25, 2008 at 01:21:50PM -0400, Eben Eliason wrote: > > Alright...I'll go at it again, sits it's been sitting around for a day... > =) > > > > > +nonzero_vols = [v for v in volumes if v > 0.0] > > > > Is it actually necessary to compare against a forced float (0.0) here? > > Probably not. The sequence elements are floats, so I figured why not. It's not wrong. I've just never seen it done. =) Obviously, 0 is losslessly representable as both int and float. > > > +if len(nonzero_vols) != 0 and len(nonzero_vols) != > len(volumes): > > > +volumes = nonzero_vols > > > > The second condition seems unnecessary. > > Possibly. We want to eliminate the nonzero volumes, but what if there > are no nonzero volumes? It's only unnecessary because sum([]) - used If there are none at all, then the first condition short circuits anyway, and we're in good shape. > later on - turns out to be zero (which I thought of now because it > wasn't 5am). So I could just replace the whole volumes = ... return > volumne ... with: > > ... > # sometimes we get a spurious zero from one/more channel(s) > nonzero_volumes = self._mixer.get_volume(self._master) > volume = sum(nonzero_volumes) / len(nonzero_volumes) > ... > > ...and then it'd work. I'll do that. Sounds much nicer. > > > +#sometimes alsa fails to set all channels' volume, so try a > few times > > > > What's the rate of failure here? > > About 25-50% if I hold down one of the arrow kesy while the slider has > focus and watch alsamixer's indication of the L+R channels' volume > from an ssh session into the laptop. > > > If we're always ignoring the zeros when reading the volume, does it > > matter if we do extra work to be sure to set them all? > > Yes, because reading of zeros and the failure to set the volumes have > different solutions. The comment should better read > > # sometimes alsa sets a channel's volume to zero rather than what we > tell it to > > ...and the problem is now sufficiently clear to justify the retry code. Yuck. And OK. > > > +mute_item_text = 'Unmute' > > ... > > > +mute_item_text = 'Mute' > > > > These should be localized! > > They are: > > > self._mute_item.get_child().set_text(_(mute_item_text)) Oh, I missed that. I would wrap the localization around the string literal, so the connection is clear. (Yes, you have to use it twice instead of oncebut it was imported as _ for a reason. ;) ) - Eben ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On Fri, Apr 25, 2008 at 01:21:50PM -0400, Eben Eliason wrote:
> Alright...I'll go at it again, sits it's been sitting around for a day... =)
>
> > +nonzero_vols = [v for v in volumes if v > 0.0]
>
> Is it actually necessary to compare against a forced float (0.0) here?
Probably not. The sequence elements are floats, so I figured why not.
>
> > +if len(nonzero_vols) != 0 and len(nonzero_vols) != len(volumes):
> > +volumes = nonzero_vols
>
> The second condition seems unnecessary.
Possibly. We want to eliminate the nonzero volumes, but what if there
are no nonzero volumes? It's only unnecessary because sum([]) - used
later on - turns out to be zero (which I thought of now because it
wasn't 5am). So I could just replace the whole volumes = ... return
volumne ... with:
...
# sometimes we get a spurious zero from one/more channel(s)
nonzero_volumes = self._mixer.get_volume(self._master)
volume = sum(nonzero_volumes) / len(nonzero_volumes)
...
...and then it'd work. I'll do that.
> > +#sometimes alsa fails to set all channels' volume, so try a few
> > times
>
> What's the rate of failure here?
About 25-50% if I hold down one of the arrow kesy while the slider has
focus and watch alsamixer's indication of the L+R channels' volume
from an ssh session into the laptop.
> If we're always ignoring the zeros when reading the volume, does it
> matter if we do extra work to be sure to set them all?
Yes, because reading of zeros and the failure to set the volumes have
different solutions. The comment should better read
# sometimes alsa sets a channel's volume to zero rather than what we
tell it to
...and the problem is now sufficiently clear to justify the retry code.
> > +TrayIcon.__init__(self, icon_name=_ICON_NAME,
> > + xo_color=profile.get_color())
>
> You can align this with "self, ..."
Will do...code was cut & pasted...
> +class SpeakerPalette(Palette):
> > +
> > +def __init__(self, primary_text, model):
> > +Palette.__init__(self, primary_text)
>
> It would be better to pass primary text as a keyword arg, using the
> property API. Passing the standard arg directly is deprecated.
Again, cut & pasted...will do.
> > +self._mute_item = MenuItem(_('Unmute'))
>
> Tomeu will whine here because you use this string in two places.
I didn't like this either...I wasn't sure about initializing it to
None, and now you've clarified that, so I'll set it to ''.
> > +self.set_content(vbox)
>
> Any need to put this down below the MenuItem creation? It seems oddly
> detatched from the other VBox stuff.
Nope.
> > +mute_item_text = 'Unmute'
> ...
> > +mute_item_text = 'Mute'
>
> These should be localized!
They are:
self._mute_item.get_child().set_text(_(mute_item_text))
> - Eben
Martin
pgpKbLYPCrhhu.pgp
Description: PGP signature
___
Sugar mailing list
Sugar@lists.laptop.org
http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
Alright...I'll go at it again, sits it's been sitting around for a day... =)
> +nonzero_vols = [v for v in volumes if v > 0.0]
Is it actually necessary to compare against a forced float (0.0) here?
> +if len(nonzero_vols) != 0 and len(nonzero_vols) != len(volumes):
> +volumes = nonzero_vols
The second condition seems unnecessary. If they are the same length,
they are the same, and performing the reassignment is not an issue.
Also, the first comparison would read more clearly if it used a
greater than comparison, instead of inequality, since the length
cannot be negative.
> +#sometimes alsa fails to set all channels' volume, so try a few
> times
What's the rate of failure here? If we're always ignoring the zeros
when reading the volume, does it matter if we do extra work to be sure
to set them all? For that matter, why do we need to mess with a bunch
of channels at all? (Pardon my ignorance...feel free to ignore these
questions entirely.)
> +TrayIcon.__init__(self, icon_name=_ICON_NAME,
> + xo_color=profile.get_color())
You can align this with "self, ..."
+class SpeakerPalette(Palette):
> +
> +def __init__(self, primary_text, model):
> +Palette.__init__(self, primary_text)
It would be better to pass primary text as a keyword arg, using the
property API. Passing the standard arg directly is deprecated.
> +self._mute_item = MenuItem(_('Unmute'))
Tomeu will whine here because you use this string in two places. You
could instead initialize it with a null string and call _update_info
to set the label appropriately. For some reason, it seem that you
cannot simply pass None (or leave out the argument completely), but
must instead pass '' (null string) instead.
> +self.set_content(vbox)
Any need to put this down below the MenuItem creation? It seems oddly
detatched from the other VBox stuff.
> +mute_item_text = 'Unmute'
...
> +mute_item_text = 'Mute'
These should be localized!
- Eben
___
Sugar mailing list
Sugar@lists.laptop.org
http://lists.laptop.org/listinfo/sugar
[sugar] [PATCH] Add speaker device and icon by default
Add speaker device and icon by default, as in
http://wiki.laptop.org/go/Designs/Frame#07
---
src/hardware/hardwaremanager.py | 26 -
src/model/devices/Makefile.am |4 +-
src/model/devices/devicesmodel.py |3 +
src/model/devices/speaker.py | 59 ++
src/view/devices/Makefile.am |4 +-
src/view/devices/speaker.py | 119 +
6 files changed, 211 insertions(+), 4 deletions(-)
create mode 100644 src/model/devices/speaker.py
create mode 100644 src/view/devices/speaker.py
diff --git a/src/hardware/hardwaremanager.py b/src/hardware/hardwaremanager.py
index 29be450..434f481 100644
--- a/src/hardware/hardwaremanager.py
+++ b/src/hardware/hardwaremanager.py
@@ -50,6 +50,13 @@ class HardwareManager(object):
if track.flags & gst.interfaces.MIXER_TRACK_MASTER:
self._master = track
+def get_mute(self):
+if not self._mixer or not self._master:
+logging.error('Cannot get the mute status')
+return False
+return self._master.flags & gst.interfaces.MIXER_TRACK_MUTE \
+ == gst.interfaces.MIXER_TRACK_MUTE
+
def get_volume(self):
if not self._mixer or not self._master:
logging.error('Cannot get the volume')
@@ -57,7 +64,16 @@ class HardwareManager(object):
max_volume = self._master.max_volume
min_volume = self._master.min_volume
-volume = self._mixer.get_volume(self._master)[0]
+
+volumes = self._mixer.get_volume(self._master)
+
+#sometimes we get a spurious zero from one/more channel(s)
+nonzero_vols = [v for v in volumes if v > 0.0]
+if len(nonzero_vols) != 0 and len(nonzero_vols) != len(volumes):
+volumes = nonzero_vols
+
+#we could just pick the first channel's volume, but this converges
+volume = sum(volumes) / len(volumes)
return volume * 100.0 / (max_volume - min_volume) + min_volume
@@ -76,7 +92,13 @@ class HardwareManager(object):
volume = volume * (max_volume - min_volume) / 100.0 + min_volume
volume_list = [ volume ] * self._master.num_channels
-self._mixer.set_volume(self._master, tuple(volume_list))
+#sometimes alsa fails to set all channels' volume, so try a few times
+last_volumes_read = [0]
+read_count = 0
+while (0 in last_volumes_read) and (read_count < 3):
+self._mixer.set_volume(self._master, tuple(volume_list))
+last_volumes_read = self._mixer.get_volume(self._master)
+read_count += 1
def set_mute(self, mute):
if not self._mixer or not self._master:
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
+
diff --git a/src/model/devices/devicesmodel.py
b/src/model/devices/devicesmodel.py
index 691e19e..32c77da 100644
--- a/src/model/devices/devicesmodel.py
+++ b/src/model/devices/devicesmodel.py
@@ -23,6 +23,7 @@ from model.devices import device
from model.devices.network import wireless
from model.devices.network import mesh
from model.devices import battery
+from model.devices import speaker
from hardware import hardwaremanager
from hardware import nmclient
@@ -54,6 +55,8 @@ class DevicesModel(gobject.GObject):
for udi in hal_manager.FindDeviceByCapability('battery'):
self.add_device(battery.Device(udi))
+self.add_device(speaker.Device())
+
def _observe_network_manager(self):
network_manager = hardwaremanager.get_network_manager()
if not network_manager:
diff --git a/src/model/devices/speaker.py b/src/model/devices/speaker.py
new file mode 100644
index 000..d3b9967
--- /dev/null
+++ b/src/model/devices/speaker.py
@@ -0,0 +1,59 @@
+# Copyright (C) 2008 Martin Dengler
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+
+import gobject
+
+from hardware import hardwaremanager
+from model.devices import device
+
+class Device(dev
Re: [sugar] [PATCH] Add speaker device and icon by default
On Thu, Apr 24, 2008 at 12:47:11PM -0400, Eben Eliason wrote: > On Thu, Apr 24, 2008 at 12:40 PM, Martin Dengler wrote: > > On Thu, Apr 24, 2008 at 12:01:52PM -0400, Eben Eliason wrote: > > > On Thu, Apr 24, 2008 at 7:33 AM, Martin Dengler wrote: > > > > +return self._master.flags & gst.interfaces.MIXER_TRACK_MUTE \ > > > > + == gst.interfaces.MIXER_TRACK_MUTE > > > > > > If MIXER_TRACK_MUTE is a bit mask, isn't the equality check redundant? > > > > It's not a bitmask, IIUC. So no, it's not redundant AFAICS. But yes, > > it'd read much more nicely without the equality check if it wasn't. > > Hmmm, but you appear to be using it as such anyway. How else could > the boolean & be interpreted logically otherwise? (I may very well be > missing something here.) the GFlags class overrides the __and__ method...nice, eh? >>> master.flags >>> gst.interfaces.MIXER_TRACK_MUTE.__and__ >>> type(gst.interfaces.MIXER_TRACK_MUTE) >>> type(gst.interfaces.MIXER_TRACK_MUTE & gst.interfaces.MIXER_TRACK_MUTE) > - Eben Martin pgplSrcTMG9Dw.pgp Description: PGP signature ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On Thu, Apr 24, 2008 at 12:40 PM, Martin Dengler <[EMAIL PROTECTED]> wrote: > On Thu, Apr 24, 2008 at 12:01:52PM -0400, Eben Eliason wrote: > > I'll /try/ to keep my comments mostly to the UI and leave the code > > review for others. I'll also fail at that to some extent, since I > > have curiosities about bits and pieces. > > > > On Thu, Apr 24, 2008 at 7:33 AM, Martin Dengler > > <[EMAIL PROTECTED]> wrote: > > > > > +return self._master.flags & gst.interfaces.MIXER_TRACK_MUTE \ > > > + == gst.interfaces.MIXER_TRACK_MUTE > > > > If MIXER_TRACK_MUTE is a bit mask, isn't the equality check redundant? > > It's not a bitmask, IIUC. So no, it's not redundant AFAICS. But yes, > it'd read much more nicely without the equality check if it wasn't. Hmmm, but you appear to be using it as such anyway. How else could the boolean & be interpreted logically otherwise? (I may very well be missing something here.) It was pointed out to me that one reason for it would be to return a boolean type, instead of an int, which is a valid assessment (I'm too familiar with C). If that's the core reason, however, I might find simply casting to bool clearer. > > Let's use the dialog-ok and dialog-cancel icons for now, which will > > match the current mockups for the mic and camera. We can change them > > easily later if we need to. > > dialog-ok to go with the "Unmute" text, and -cancel to go with "Mute"? Yup. > > > +mute_item_text += '...' > > > > This is a bad habit that everyone seems to get into. Cut this line. > > Sorry! I know the rule(s) you cite, and must blame lack of sleep for > leaving this in. I was cargo-culting "Disconnect..." from > network/wireless.py, I think. Good! I expect not to have to slap you again for this. :-P - Eben ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On Thu, Apr 24, 2008 at 12:01:52PM -0400, Eben Eliason wrote: > I'll /try/ to keep my comments mostly to the UI and leave the code > review for others. I'll also fail at that to some extent, since I > have curiosities about bits and pieces. > > On Thu, Apr 24, 2008 at 7:33 AM, Martin Dengler > <[EMAIL PROTECTED]> wrote: > > > +return self._master.flags & gst.interfaces.MIXER_TRACK_MUTE \ > > + == gst.interfaces.MIXER_TRACK_MUTE > > If MIXER_TRACK_MUTE is a bit mask, isn't the equality check redundant? It's not a bitmask, IIUC. So no, it's not redundant AFAICS. But yes, it'd read much more nicely without the equality check if it wasn't. > > +#sometimes we get a spurious zero from one/more channel(s) > > +#we could just pick the first channel's volume, but this converges > > +#sometimes alsa fails to set all channels' volume, so try a few > > times > > That's all fairly ugly, huh? Oh well. I know! > > +badge_name = None > ... > > +self.icon.props.badge_name = badge_name > > What's the logic for leaving this in? Hmmm...I must've missed that pylint warning. The variable goes. > > +def _mute_item_text(self): > ... > > +return _(mute_item_text) > > Defining this in a function seems to work OK here, but I think I also > want icons on the menu item, which also depends on the current state. > As such, it may actually be cleaner to have an _update function of > some kind which handles both text and image together, setting them > directly instead of returning a value. Agree -- I just didn't have any icons, so no use for such a function yet. > Let's use the dialog-ok and dialog-cancel icons for now, which will > match the current mockups for the mic and camera. We can change them > easily later if we need to. dialog-ok to go with the "Unmute" text, and -cancel to go with "Mute"? > > +mute_item_text = self._model.props.muted and 'Unmute' or 'Mute' > > This is a tricky ternary stand-in. Very clever. Is it clear enough for > others? > I'll change to the proper ternary clause as recommended by bemasc. > > +mute_item_text += '...' > > This is a bad habit that everyone seems to get into. Cut this line. Sorry! I know the rule(s) you cite, and must blame lack of sleep for leaving this in. I was cargo-culting "Disconnect..." from network/wireless.py, I think. > - Eben Martin pgpTWPw2nOiy6.pgp Description: PGP signature ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On Thu, Apr 24, 2008 at 12:13:19PM -0400, Benjamin M. Schwartz wrote: > I would prefer a true ternary expression (new in 2.5): > mute_item_text = 'Unmute' if self._model.props.muted else 'Mute' Doh -- I forgot we're on 2.5. Of course this is the right thing to do. Thanks. > - --Ben Martin pgp7SSjrlo6ER.pgp Description: PGP signature ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On Thu, Apr 24, 2008 at 12:14 PM, Bert Freudenberg <[EMAIL PROTECTED]> wrote: > > On 24.04.2008, at 18:01, Eben Eliason wrote: > > > >> +mute_item_text = self._model.props.muted and 'Unmute' or > >> 'Mute' > > > > This is a tricky ternary stand-in. Very clever. Is it clear enough > > for others? > > > It's an abuse of Python, IMHO. Tricky and abusive often go hand in hand, and abuses of languages are often the most interesting, though clearly not the most transparent. =) C, for instance, seems to taunt with all of the glorious hacks it is capable of executing as syntactically correct code. Duff's Device [1], anyone? Of course, usually susceptibility to such temptations result in a clean shot through one's own foot, with a sharp segfault bullet or worse. - Eben [1] http://en.wikipedia.org/wiki/Duff's_device ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
On 24.04.2008, at 18:01, Eben Eliason wrote: > >> +mute_item_text = self._model.props.muted and 'Unmute' or >> 'Mute' > > This is a tricky ternary stand-in. Very clever. Is it clear enough > for others? It's an abuse of Python, IMHO. - Bert - ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Eben Eliason wrote: |> +mute_item_text = self._model.props.muted and 'Unmute' or 'Mute' | | This is a tricky ternary stand-in. Very clever. Is it clear enough for others? If you want it on one line, I would prefer a true ternary expression (new in 2.5): mute_item_text = 'Unmute' if self._model.props.muted else 'Mute' - --Ben -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.7 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFIELGfUJT6e6HFtqQRAsmlAJ0UFPQhOLuWvvW7zFwR38ZjaB161ACbBhuc 9JJaPTMpBtNfOOwyzTaFv3g= =Ot34 -END PGP SIGNATURE- ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
I'll /try/ to keep my comments mostly to the UI and leave the code review for others. I'll also fail at that to some extent, since I have curiosities about bits and pieces. On Thu, Apr 24, 2008 at 7:33 AM, Martin Dengler <[EMAIL PROTECTED]> wrote: > +return self._master.flags & gst.interfaces.MIXER_TRACK_MUTE \ > + == gst.interfaces.MIXER_TRACK_MUTE If MIXER_TRACK_MUTE is a bit mask, isn't the equality check redundant? > +#sometimes we get a spurious zero from one/more channel(s) > +#we could just pick the first channel's volume, but this converges > +#sometimes alsa fails to set all channels' volume, so try a few > times That's all fairly ugly, huh? Oh well. > +badge_name = None ... > +self.icon.props.badge_name = badge_name What's the logic for leaving this in? Is there a circumstance in which you think we may later add a badge, because at the moment this variable is never used. > +def _mute_item_text(self): ... > +return _(mute_item_text) Defining this in a function seems to work OK here, but I think I also want icons on the menu item, which also depends on the current state. As such, it may actually be cleaner to have an _update function of some kind which handles both text and image together, setting them directly instead of returning a value. I suppose you could also have separate functions for _mute_item_text and _mute_item_icon, as wellI'll let the others decide. Let's use the dialog-ok and dialog-cancel icons for now, which will match the current mockups for the mic and camera. We can change them easily later if we need to. > +mute_item_text = self._model.props.muted and 'Unmute' or 'Mute' This is a tricky ternary stand-in. Very clever. Is it clear enough for others? > +mute_item_text += '...' This is a bad habit that everyone seems to get into. Cut this line. Every menu item takes an action when clicked, and that's implicit, making the ellipsis effectively redundant. The ellipsis should only be used when (a) the text represents a "state of flux", eg. in the AP palettes /while/ "Connecting..." (not on the "Connect" menu item itself, which initiates it) or (b) The action of the menu item itself reveals a dialog which requires further feedback before actually initiating a "real" action. - Eben ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
[sugar] [PATCH] Add speaker device and icon by default
Add speaker device and icon by default, as in
http://wiki.laptop.org/go/Designs/Frame#07
---
src/hardware/hardwaremanager.py | 28 +-
src/model/devices/Makefile.am |4 +-
src/model/devices/devicesmodel.py |3 +
src/model/devices/speaker.py | 59 +++
src/view/devices/Makefile.am |4 +-
src/view/devices/speaker.py | 113 +
6 files changed, 207 insertions(+), 4 deletions(-)
create mode 100644 src/model/devices/speaker.py
create mode 100644 src/view/devices/speaker.py
diff --git a/src/hardware/hardwaremanager.py b/src/hardware/hardwaremanager.py
index 29be450..f5bb0e3 100644
--- a/src/hardware/hardwaremanager.py
+++ b/src/hardware/hardwaremanager.py
@@ -50,6 +50,13 @@ class HardwareManager(object):
if track.flags & gst.interfaces.MIXER_TRACK_MASTER:
self._master = track
+def get_mute(self):
+if not self._mixer or not self._master:
+logging.error('Cannot get the mute status')
+return False
+return self._master.flags & gst.interfaces.MIXER_TRACK_MUTE \
+ == gst.interfaces.MIXER_TRACK_MUTE
+
def get_volume(self):
if not self._mixer or not self._master:
logging.error('Cannot get the volume')
@@ -57,7 +64,16 @@ class HardwareManager(object):
max_volume = self._master.max_volume
min_volume = self._master.min_volume
-volume = self._mixer.get_volume(self._master)[0]
+
+volumes = self._mixer.get_volume(self._master)
+
+#sometimes we get a spurious zero from one/more channel(s)
+nonzero_vols = [v for v in volumes if v > 0.0]
+if len(nonzero_vols) != 0 and len(nonzero_vols) != len(volumes):
+volumes = nonzero_vols
+
+#we could just pick the first channel's volume, but this converges
+volume = sum(volumes) / len(volumes)
return volume * 100.0 / (max_volume - min_volume) + min_volume
@@ -73,10 +89,18 @@ class HardwareManager(object):
max_volume = self._master.max_volume
min_volume = self._master.min_volume
+logging.debug("setting volume: %s (%s/%s/%s)" % (volume, min_volume,
max_volume, self._master.num_channels))
volume = volume * (max_volume - min_volume) / 100.0 + min_volume
volume_list = [ volume ] * self._master.num_channels
-self._mixer.set_volume(self._master, tuple(volume_list))
+#sometimes alsa fails to set all channels' volume, so try a few times
+last_volumes_read = [0]
+read_count = 0
+while (0 in last_volumes_read) and (read_count < 3):
+logging.debug("setting volume_list %d: %s" % (read_count,
volume_list))
+self._mixer.set_volume(self._master, tuple(volume_list))
+last_volumes_read = self._mixer.get_volume(self._master)
+read_count += 1
def set_mute(self, mute):
if not self._mixer or not self._master:
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
+
diff --git a/src/model/devices/devicesmodel.py
b/src/model/devices/devicesmodel.py
index 691e19e..32c77da 100644
--- a/src/model/devices/devicesmodel.py
+++ b/src/model/devices/devicesmodel.py
@@ -23,6 +23,7 @@ from model.devices import device
from model.devices.network import wireless
from model.devices.network import mesh
from model.devices import battery
+from model.devices import speaker
from hardware import hardwaremanager
from hardware import nmclient
@@ -54,6 +55,8 @@ class DevicesModel(gobject.GObject):
for udi in hal_manager.FindDeviceByCapability('battery'):
self.add_device(battery.Device(udi))
+self.add_device(speaker.Device())
+
def _observe_network_manager(self):
network_manager = hardwaremanager.get_network_manager()
if not network_manager:
diff --git a/src/model/devices/speaker.py b/src/model/devices/speaker.py
new file mode 100644
index 000..d3b9967
--- /dev/null
+++ b/src/model/devices/speaker.py
@@ -0,0 +1,59 @@
+# Copyright (C) 2008 Martin Dengler
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have
Re: [sugar] [PATCH] Add speaker device and icon by default
On Thu, Apr 24, 2008 at 4:24 AM, Martin Dengler <[EMAIL PROTECTED]> wrote: > Should I be submitting patches that include the Makefile.am changes? > Or does automake magically figure that out for me somehow? > Nope, you have to do it manually. Marco ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
Re: [sugar] [PATCH] Add speaker device and icon by default
I forgot - as in, wasn't aware - that/whether I should be adding the new files to Makefile.am(s) in the new files' directories. I've a patch that does this. It seemed to do the job for my sugar-jhbuild emulation; I've been manually scp'ing the files to my XO to test "for real". Should I be submitting patches that include the Makefile.am changes? Or does automake magically figure that out for me somehow? Martin On Thu, Apr 24, 2008 at 03:13:19AM +0100, Martin Dengler wrote: > Add speaker device and icon by default, as in > http://wiki.laptop.org/go/Designs/Frame#07 > --- > src/model/devices/devicesmodel.py |3 + > src/model/devices/speaker.py | 59 ++ > src/view/devices/speaker.py | 99 > + > 3 files changed, 161 insertions(+), 0 deletions(-) > create mode 100644 src/model/devices/speaker.py > create mode 100644 src/view/devices/speaker.py > ... pgpVJBHQhqKWJ.pgp Description: PGP signature ___ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
[sugar] [PATCH] Add speaker device and icon by default
Add speaker device and icon by default, as in
http://wiki.laptop.org/go/Designs/Frame#07
---
src/model/devices/devicesmodel.py |3 +
src/model/devices/speaker.py | 59 ++
src/view/devices/speaker.py | 99 +
3 files changed, 161 insertions(+), 0 deletions(-)
create mode 100644 src/model/devices/speaker.py
create mode 100644 src/view/devices/speaker.py
diff --git a/src/model/devices/devicesmodel.py
b/src/model/devices/devicesmodel.py
index 691e19e..32c77da 100644
--- a/src/model/devices/devicesmodel.py
+++ b/src/model/devices/devicesmodel.py
@@ -23,6 +23,7 @@ from model.devices import device
from model.devices.network import wireless
from model.devices.network import mesh
from model.devices import battery
+from model.devices import speaker
from hardware import hardwaremanager
from hardware import nmclient
@@ -54,6 +55,8 @@ class DevicesModel(gobject.GObject):
for udi in hal_manager.FindDeviceByCapability('battery'):
self.add_device(battery.Device(udi))
+self.add_device(speaker.Device())
+
def _observe_network_manager(self):
network_manager = hardwaremanager.get_network_manager()
if not network_manager:
diff --git a/src/model/devices/speaker.py b/src/model/devices/speaker.py
new file mode 100644
index 000..9f84b81
--- /dev/null
+++ b/src/model/devices/speaker.py
@@ -0,0 +1,59 @@
+# Copyright (C) 2008 Martin Dengler
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+
+import gobject
+
+from hardware import hardwaremanager
+from model.devices import device
+
+class Device(device.Device):
+__gproperties__ = {
+'level' : (int, None, None, 0, 100, 0, gobject.PARAM_READWRITE),
+'muted' : (bool, None, None, False, gobject.PARAM_READWRITE),
+}
+
+def __init__(self):
+device.Device.__init__(self)
+self._manager = hardwaremanager.get_manager()
+
+def _get_level(self):
+return self._manager.get_volume()
+
+def _set_level(self, new_volume):
+self._manager.set_volume(new_volume)
+self.notify('level')
+
+def _get_muted(self):
+return self.props.level == 0.0
+
+def _set_muted(self, mute):
+self._manager.set_mute(mute)
+self.notify('muted')
+
+def get_type(self):
+return 'speaker'
+
+def do_get_property(self, pspec):
+if pspec.name == "level":
+return self._get_level()
+elif pspec.name == "muted":
+return self._get_muted()
+
+def do_set_property(self, pspec, value):
+if pspec.name == "level":
+self._set_level(value)
+elif pspec.name == "muted":
+self._set_muted(value)
diff --git a/src/view/devices/speaker.py b/src/view/devices/speaker.py
new file mode 100644
index 000..3fe36eb
--- /dev/null
+++ b/src/view/devices/speaker.py
@@ -0,0 +1,99 @@
+# Copyright (C) 2008 Martin Dengler
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+
+from gettext import gettext as _
+
+import gtk
+
+from sugar import profile
+from sugar.graphics import style
+from sugar.graphics.icon import get_icon_state
+from sugar.graphics.tray import TrayIcon
+from sugar.graphics.palette import Palette
+from sugar.graphics.xocolor import XoColor
+
+from view.frame.frameinvoker import FrameWidgetInvoker
+
+_ICON_NAME = 'speaker'
+
+class DeviceView(TrayIcon):
+def __init__(self, model):
+TrayIcon.__init__(self, icon_name=_ICON_NAME,
+ xo_color=profile.get_color())
+
+self._model = model
+self.palette = SpeakerPalette(_('My Speakers'), mo
