Merge lp:~roadmr/checkbox/audio-test-fixes into lp:checkbox

[Daniel Manrique]

Added retrying with a pause when invoking pacmd, to guard against uninitialized pulseaudio

Added retrying with a pause when invoking pacmd, to guard against uninitialized pulseaudio.

[Marc Tardif (cr3)]

I'm not sure I understand when check_output might raise an AttributeError exception. First, I'd make sure this is even possible. Second, if it is, I suspect it might have to be handled differently than the CallProcessError exception. For example, you might not want to continue looping when an AttributeError exception is raised because it might not make sense.

review: Needs Fixing

[Daniel Manrique (roadmr)]

Thanks for spotting that, since check_output is the only method called in the block, and that does not return AttributeError under any circumstances, I just removed it.

review: Needs Resubmitting

[Marc Tardif (cr3)]

Sweet! I'll just approve and let tarmac complain about the merge conflict, thanks!

review: Approve

[Zygmunt Krynicki (zyga)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

[Marc Tardif (cr3)]

Oh boy, now I get to review a second time!

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2012-10-22 14:38:54 +0000
+++ debian/changelog	2012-10-22 20:18:18 +0000
@@ -1,5 +1,6 @@
 checkbox (0.14.10) quantal; urgency=low
 
+  [ Jeff Lane ]
   * Increased version number after final Ubuntu Quantal release.
 
   [Brendan Donegan]
@@ -27,6 +28,8 @@
     filling the screen with dots. (LP: #926104)
   * Increased version number after final Ubuntu Quantal release.
   * Added environment resource to whitelists (LP: #1067038)
+  * Added retrying pactl commands in case audio layer is not up yet
+    (LP: #1065908)
 
   [Marc Tardif]
   * plugins/environment_info.py: Enabling environment to take precedence

=== modified file 'scripts/audio_test'
--- scripts/audio_test	2012-08-30 15:34:42 +0000
+++ scripts/audio_test	2012-10-22 20:18:18 +0000
@@ -8,6 +8,7 @@
 import re
 import sys
 import subprocess
+import time
 #Trick to prevent gst from hijacking argv parsing
 argv = sys.argv
 sys.argv = []
@@ -205,10 +206,18 @@
         return valid_elements[0]
 
     def _pactl_output(self, command):
-        try:
-            return subprocess.check_output(command, universal_newlines=True)
-        except (subprocess.CalledProcessError, AttributeError):
-            return False
+        #This method mainly calls pactl (hence the name). Since pactl may
+        #return a failure if the audio layer is not yet initialized, we will
+        #try running a few times in case of failure. All our invocations of 
+        #pactl should be "idempotent" so repeating them should not have 
+        #any bad effects.
+        for attempt in range(0, 3):
+            try:
+                return subprocess.check_output(command, 
+                                               universal_newlines=True)
+            except (subprocess.CalledProcessError):
+                time.sleep(5)
+        return False
 
 
 class FileDumper(object):
@@ -567,8 +576,8 @@
         return_value = 1
     #Is the microphone broken?
     if len(set(analyzer.spectrum)) <= 1:
-        logging.info("WARNING: Microphone seems broken, didn't even record ambient"
-              " noise")
+        logging.info("WARNING: Microphone seems broken, didn't even "
+                     "record ambient noise")
 
     #Write some files to disk for later analysis
     if args.audio:
@@ -580,12 +589,14 @@
 
     if args.spectrum:
         if args.verbose:
-            logging.info("Saving spectrum data for plotting as %s" % args.spectrum)
+            logging.info("Saving spectrum data for plotting as %s" % 
+                         args.spectrum)
         if not FileDumper().write_to_file(args.spectrum,
                                        ["%s,%s" % t for t in
                                         zip(analyzer.frequencies,
                                             analyzer.spectrum)]):
-            logging.error("Couldn't save spectrum data for plotting", file=sys.stderr)
+            logging.error("Couldn't save spectrum data for plotting", 
+                          file=sys.stderr)
 
     return return_value