Created attachment 151425 [details] [review]
Patch that attempts to do what is described in the opener for this bug

Halim - could you try this patch out please?  It seems to work OK for me, but I only have one speech server on my machine right now.  :-(

Bulk reassigning Will's bugs to the default assignee. (Sorry for the spam!)

Comment on attachment 151425 [details] [review]
Patch that attempts to do what is described in the opener for this bug

Obsoleting this patch because it looks like much of this already went into Orca as part of the fix for bug 606962.

Jon, can I get you to take this one?

As a quick test, I changed my preferred factory in user-settings.py to 'orca.speechdispatcherfactory' on a system where Speech Dispatcher is not installed. (I think that qualifies for failure to initialize the preferred factory.) Having done this, Orca launched without speech. So this issue seems to still be a valid issue.

If you're not able or interested in taking this one, please assign it to me. Thanks!

Thanks Joanie, I will have a look.

Created attachment 164232 [details] [review]
patch to provide debugging info.

Hi Joanie, Could you please apply and attach debug.out? I think that we 
were failing to pass an exception on in some cases. Tested with no speech 
systems available on the system, and it seems to work well. Tested when i 
wanted gnomespeech, but speechdispatcher was available, and it managed to 
get there.

Thanks.

Here's the output from your debugging patch:

Trying to use speech server factory: orca.speechdispatcherfactory
good evening, timmy.
timmy, everything went to plan.
Speech Dispatcher interface not installed.
The prefered speech module was found.
Speech not available.

Traceback (most recent call last):

+ Trace 222546

Could not initialize connection to speech.

Created attachment 164490 [details] [review]
Fix for bgo#606978 - Detect working speech factory upon failure to initialize the preferred factory.

Thanks for the info Joanie.
Does the attached patch look reasonable?

it tries the prefered speech factory, if it fails, then runs through them 
all, seeing if it can get any of them to work.

This patch doesn't work for me. Here's the output:

~~~~~~~~~~~
Attempting to use orca.speechdispatcherfactory
Trying to use speech server factory: orca.speechdispatcherfactory
Speech Dispatcher interface not installed.
Attempting to use espeechfactory
Trying to use speech server factory: espeechfactory

Traceback (most recent call last):

+ Trace 222572

Attempting to use gnomespeechfactory
Trying to use speech server factory: gnomespeechfactory
Attempting to use openttsfactory
Trying to use speech server factory: openttsfactory
Open TTS interface not installed.
Attempting to use speechdispatcherfactory
Trying to use speech server factory: speechdispatcherfactory
Speech Dispatcher interface not installed.
None of the speech modules seem to be working.
~~~~~~~~~~~

Review to follow.

Review of attachment 164490 [details] [review]:

+    # Pylint is confused, and thinks that moduleName is undefined in the loop.
+    # We know what we are doing, so disable the message.
+    # pylint: disable-msg=W0631

Actually, pylint isn't confused. You shouldn't be referring to moduleName outside of the loop. Outside of the loop, there is this block:

     if _speechserver:
         debug.println(debug.LEVEL_CONFIGURATION,
                       "Using speech server factory: %s" % moduleName)

Look at around line 140.

-    try:
-        moduleName = settings.speechServerFactory
-        _initSpeechServer(moduleName,
-                          settings.speechServerInfo)
-    except:
-        moduleNames = settings.speechFactoryModules
-        for moduleName in moduleNames:

So before, a user could manually set settings.speechServerFactory to nothing if he/she so chose. If a user did so, the above block of code would cause _initSpeechServer() to be called. _initSpeechServer() returns immediately if moduleName is None, which it would be in this hypothetical case. No exception would be raised, so we wouldn't go looking for other speech modules to try and load. In other words, the user apparently doesn't want speech, so we respect that and don't speak.

+    moduleNames  = [settings.speechServerFactory]
+    moduleNames.extend(settings.speechFactoryModules)
+    for moduleName in moduleNames:
+        debug.println(debug.LEVEL_CONFIGURATION, \
+          "Attempting to use %s" % moduleName)

What you're doing seems to insist that a user will have speech if we can manage to get it started. I'm not so sure that's what we want to do -- both in terms of actual users, and also in terms of our regression tests.

+        try:
             if moduleName != settings.speechServerFactory:
-                try:
-                    _initSpeechServer(moduleName, None)
-                    if _speechserver:
-                        break
-                except:
-                    debug.printException(debug.LEVEL_SEVERE)
+                _initSpeechServer(moduleName, None)
+            else:
+                _initSpeechServer(moduleName, settings.speechServerInfo)
+            if _speechserver:
+                debug.println(debug.LEVEL_CONFIGURATION, \
+                  "The %s speech module was successful." % moduleName)
+                break
+        except:
+            debug.printException(debug.LEVEL_ALL)

Quite a lot in the try. I think we should only put stuff in the try that needs to be in there.
 
     if _speechserver:
         debug.println(debug.LEVEL_CONFIGURATION,
                       "Using speech server factory: %s" % moduleName)
     else:
-        debug.println(debug.LEVEL_CONFIGURATION, "Speech not available.")
+        raise Exception("None of the speech modules seem to be working.")

Why do we want to raise an exception here? Especially given the user (or test harness) intentionally chose no speech?

         except:
-            debug.printException(debug.LEVEL_SEVERE)
             debug.println(debug.LEVEL_SEVERE,
-                          "Could not initialize connection to speech.")
+                          "None of the speech modules seem to be working.")
+            settings.enableSpeech = False

I think the original statement is more accurate, again, especially given that the "problem" may be the decision to not have a speechfactory, rather than things actually being broken.

As for changing settings.enableSpeech, I'd have to test more (including with the regression tests) to see if that's a change we indeed want to be making.

Created attachment 164557 [details] [review]
Simpler solution

Jon this solves the problem for me. Pylints to a 10. Doesn't seem to change Orca's current behavior (other than to fix the bug). Please test.

Note that espeechfactory.py prints exceptions like they're going out of style, I'm thinking about changing that to make it consistent it with the other factories -- and less disturbing when we have to fall back on another factory.

Comment on attachment 164557 [details] [review]
Simpler solution

Now that speech dispatcher is the default, we need this fix in place. Given that, the simple nature of the fix, and that this fix tests well, I've gone ahead and committed it to master.

http://git.gnome.org/browse/orca/commit/?id=d7b89cdd4dd7c0f4bf6482ad373bb78f89e48602