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

[Eben Eliason]

On Fri, Apr 25, 2008 at 1:46 PM, Martin Dengler
<martin at martindengler.com> 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 once....but it was imported as _ for a reason. ;) )

- Eben