Restore playback port after each test

Each test was changing the active playback sink port to perform the test on that port, but then not setting the port back to its original setting after the test.  This could cause another test that expects the port to be set to the system default to fail.  It was also leaving the system in a state where audio would not play out the speakers if the last test done was on a non-speaker port.

BUG=cros partner 724
TEST=System should still play audio fine after completing tests

[davejcool, 2010-09-02 20:49:55]

Hi,

I added a call in the PlaybackRecordSemiAuto test to restore the playback port
to its original setting after the test.  Tests were leaving the port set to the
one being tested after exiting.  It's my first time at Python script, but it is
doing what it is supposed to do.

Thanks,
-David.

[ericli, 2010-09-02 20:55:54]

http://codereview.chromium.org/3293004/diff/2001/3001
File
client/site_tests/audiovideo_PlaybackRecordSemiAuto/audiovideo_PlaybackRecordSemiAuto.py
(right):

http://codereview.chromium.org/3293004/diff/2001/3001#newcode772
client/site_tests/audiovideo_PlaybackRecordSemiAuto/audiovideo_PlaybackRecordSemiAuto.py:772:
if m is not None:
if m:

Please help to clean all the above if statement.

http://codereview.chromium.org/3293004/diff/2001/3001#newcode863
client/site_tests/audiovideo_PlaybackRecordSemiAuto/audiovideo_PlaybackRecordSemiAuto.py:863:
if device['active_port'] is not None:
if device['active_port'] should be good enough

[awong, 2010-09-02 21:00:45]

http://codereview.chromium.org/3293004/diff/2001/3001
File
client/site_tests/audiovideo_PlaybackRecordSemiAuto/audiovideo_PlaybackRecordSemiAuto.py
(right):

http://codereview.chromium.org/3293004/diff/2001/3001#newcode863
client/site_tests/audiovideo_PlaybackRecordSemiAuto/audiovideo_PlaybackRecordSemiAuto.py:863:
if device['active_port'] is not None:
On 2010/09/02 20:55:55, ericli wrote:
> if device['active_port'] should be good enough

I think it's possible for active_port to be the empty string.  In that case, you
want the is check versus None I think.

http://codereview.chromium.org/3293004/diff/2001/3001#newcode961
client/site_tests/audiovideo_PlaybackRecordSemiAuto/audiovideo_PlaybackRecordSemiAuto.py:961:
self.restore_playback_port(playback_device)
Should the save/restore be done in some sort of higher-level initialize() and
cleanup() call?

[scherkus (not reviewing), 2010-09-02 21:31:38]

http://codereview.chromium.org/3293004/diff/2001/3001
File
client/site_tests/audiovideo_PlaybackRecordSemiAuto/audiovideo_PlaybackRecordSemiAuto.py
(right):

http://codereview.chromium.org/3293004/diff/2001/3001#newcode961
client/site_tests/audiovideo_PlaybackRecordSemiAuto/audiovideo_PlaybackRecordSemiAuto.py:961:
self.restore_playback_port(playback_device)
On 2010/09/02 21:00:46, awong wrote:
> Should the save/restore be done in some sort of higher-level initialize() and
> cleanup() call?

yeah this seems like some sort of per-test/suite setup/cleanup routine

[davejcool, 2010-09-04 00:45:43]

This last rev should clear up a lot of the confusion being had over which ports
were being tested, and chooses a more intelligent default port for the recording
test instead of just the first in the list.  On each test page, we now list just
the port being tested instead of listing all the ports when only one was being
specifically tested.  If you see on the page we are testing the "headphone"
port, for example, you would not expect to hear sound out the speaker, etc.

http://codereview.chromium.org/3293004/diff/2001/3001
File
client/site_tests/audiovideo_PlaybackRecordSemiAuto/audiovideo_PlaybackRecordSemiAuto.py
(right):

http://codereview.chromium.org/3293004/diff/2001/3001#newcode772
client/site_tests/audiovideo_PlaybackRecordSemiAuto/audiovideo_PlaybackRecordSemiAuto.py:772:
if m is not None:
On 2010/09/02 20:55:55, ericli wrote:
> if m:
> 
> Please help to clean all the above if statement.

I was trying to be consistent with the style of the rest of this file.  'if m:'
would be fine I think, since m is an object, but if I change all these, then I
should change them everywhere, and I'm afraid that might introduce a bug if I
happen to switch one that shouldn't have been.

http://codereview.chromium.org/3293004/diff/2001/3001#newcode961
client/site_tests/audiovideo_PlaybackRecordSemiAuto/audiovideo_PlaybackRecordSemiAuto.py:961:
self.restore_playback_port(playback_device)
On 2010/09/02 21:31:38, scherkus wrote:
> On 2010/09/02 21:00:46, awong wrote:
> > Should the save/restore be done in some sort of higher-level initialize()
and
> > cleanup() call?
> 
> yeah this seems like some sort of per-test/suite setup/cleanup routine
> 

I'll put it at the end of the set then, since all the individual tests set the
port when they begin.

http://codereview.chromium.org/3293004/diff/8001/9001#newcode624
client/site_tests/audiovideo_PlaybackRecordSemiAuto/audiovideo_PlaybackRecordSemiAuto.py:624:
server.wfile.write('<li>%d' % channel)
On the performing test page, it was listing ALL ports instead of the specific
port being tested, which could be confusing (it may list the 'speaker' port when
testing the 'headphone' port, where no audio would be heard out the speaker).

http://codereview.chromium.org/3293004/diff/8001/9001#newcode925
client/site_tests/audiovideo_PlaybackRecordSemiAuto/audiovideo_PlaybackRecordSemiAuto.py:925:
for channel in xrange(0, device['channels']):
Setting playback port the the system default, most likely to be heard rather
than the first port in the list, which on some devices may be one where you can
not hear the audio.

[scherkus (not reviewing), 2010-09-08 03:00:11]

this LGTM to me but I think it'd be wise for ericli (or someone more familiar
with python/autotest!) to give a final LGTM

[seano, 2010-09-08 08:38:10]

Taking a look.

On Wed, Sep 8, 2010 at 5:00 AM,  <scherkus@chromium.org> wrote:
> this LGTM to me but I think it'd be wise for ericli (or someone more
> familiar
> with python/autotest!) to give a final LGTM
>
> http://codereview.chromium.org/3293004/show
>

[seano, 2010-09-08 09:57:06]

LGTM

On 2010/09/08 08:38:10, seano wrote:
> Taking a look.
> 
> On Wed, Sep 8, 2010 at 5:00 AM,  <mailto:scherkus@chromium.org> wrote:
> > this LGTM to me but I think it'd be wise for ericli (or someone more
> > familiar
> > with python/autotest!) to give a final LGTM
> >
> > http://codereview.chromium.org/3293004/show
> >
>

[awong, 2010-09-08 17:06:45]

LGTM