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 once....but it was imported as _ for a reason. ;) ) - Eben _______________________________________________ Sugar mailing list Sugar@lists.laptop.org http://lists.laptop.org/listinfo/sugar
