Make a few changes to the Sphinx engine

- Raise an error if a grammar has no active public rules.
- Disallow grammars with the same name and add a test for that.
- Improve grammar rule activation/deactivation and list update
  performance.

dragonfly/engines/backend_sphinx/engine.py

@@ -352,8 +352,7 @@ def _validate_words(self, words, search_type):
 
     def _build_grammar_wrapper(self, grammar):
         return GrammarWrapper(grammar, self,
-                              self._recognition_observer_manager,
-                              len(self._grammar_wrappers))
+                              self._recognition_observer_manager)
 
     def _set_grammar(self, wrapper, activate, partial=False):
         if not wrapper:
@@ -367,23 +366,26 @@ def activate_search_if_necessary():
                 self._decoder.end_utterance()
                 self._decoder.active_search = wrapper.search_name
 
-        # Check if the wrapper's search_name is valid
-        if wrapper.search_name in self._valid_searches:
+        # Check if the wrapper's search name is valid.
+        # Set the search (again) if necessary.
+        valid_search = wrapper.search_name in self._valid_searches
+        if valid_search and not wrapper.set_search:
             # wrapper.search_name is a valid search, so return.
             activate_search_if_necessary()
             return
 
         # Return early if 'partial' is True as an optimisation to avoid
         # recompiling grammars for every rule activation/deactivation.
-        if partial:
+        # Also return if the search doesn't need to be set.
+        if partial or not wrapper.set_search:
             return
 
         # Compile and set the jsgf search.
         compiled = wrapper.compile_jsgf()
 
-        # Only set the grammar's search if there are still active rules.
+        # Raise an error if there are no active public rules.
         if "public <root> = " not in compiled:
-            return
+            raise EngineError("no public rules found in the grammar")
 
         # Check that each word in the grammar is in the pronunciation
         # dictionary. This will raise an UnknownWordError if one or more
@@ -396,8 +398,8 @@ def activate_search_if_necessary():
         self._decoder.set_jsgf_string(wrapper.search_name, compiled)
         activate_search_if_necessary()
 
-        # Grammar search has been loaded, add the search name to the set.
-        self._valid_searches.add(wrapper.search_name)
+        # Grammar search has been loaded, so set the wrapper's flag.
+        wrapper.set_search = False
 
     def _unset_search(self, name):
         # Unset a Pocket Sphinx search with the given name.
@@ -497,6 +499,17 @@ def _load_grammar(self, grammar):
                 grammar.add_dependency(d)
 
         wrapper = self._build_grammar_wrapper(grammar)
+
+        # Check that the engine doesn't already have a grammar with the same
+        # search name. This will include grammars with the same reference
+        # name, e.g. "some grammar" and "some_grammar".
+        if wrapper.search_name in self._valid_searches:
+            message = "Failed to load grammar %s: multiple grammars with " \
+                "the same name are not allowed" % grammar
+            self._log.error(message)
+            raise EngineError(message)
+
+        # Attempt to set the grammar search.
         try:
             self._set_grammar(wrapper, False)
         except UnknownWordError as e:
@@ -510,6 +523,10 @@ def _load_grammar(self, grammar):
                                 % (grammar, e))
             raise EngineError("Failed to load grammar %s: %s."
                               % (grammar, e))
+
+        # Set the grammar wrapper's search name as valid and return the
+        # wrapper.
+        self._valid_searches.add(wrapper.search_name)
         return wrapper
 
     def _unload_grammar(self, grammar, wrapper):
@@ -534,7 +551,6 @@ def activate_rule(self, rule, grammar):
             return
         try:
             wrapper.enable_rule(rule.name)
-            self._unset_search(wrapper.search_name)
             self._set_grammar(wrapper, False, True)
         except UnknownWordError as e:
             self._log.error(e)
@@ -550,7 +566,6 @@ def deactivate_rule(self, rule, grammar):
             return
         try:
             wrapper.disable_rule(rule.name)
-            self._unset_search(wrapper.search_name)
             self._set_grammar(wrapper, False, True)
         except UnknownWordError as e:
             self._log.error(e)
@@ -569,7 +584,6 @@ def update_list(self, lst, grammar):
         wrapper.update_list(lst)
 
         # Reload the grammar.
-        self._unset_search(wrapper.search_name)
         try:
             self._set_grammar(wrapper, False)
         except Exception as e:

dragonfly/engines/backend_sphinx/grammar_wrapper.py

@@ -14,15 +14,15 @@ class GrammarWrapper(object):
 
     _log = logging.getLogger("engine")
 
-    def __init__(self, grammar, engine, observer_manager, id_):
+    def __init__(self, grammar, engine, observer_manager):
         """
         :type grammar: Grammar
         :type engine: SphinxEngine
         """
         self.grammar = grammar
         self.engine = engine
         self._observer_manager = observer_manager
-        self._id = id_
+        self.set_search = True
 
         # Compile the grammar into a JSGF grammar and set the language.
         self._jsgf_grammar = engine.compiler.compile_grammar(grammar)
@@ -32,18 +32,34 @@ def _get_reference_name(self, name):
         return self.engine.compiler.get_reference_name(name)
 
     def enable_rule(self, name):
-        self._jsgf_grammar.enable_rule(self._get_reference_name(name))
+        ref_name = self._get_reference_name(name)
+        jsgf_rule = self._jsgf_grammar.get_rule_from_name(ref_name)
+
+        # Only enable the rule and set the flag if the rule is disabled.
+        if not jsgf_rule.active:
+            jsgf_rule.enable()
+            self.set_search = True
 
     def disable_rule(self, name):
-        self._jsgf_grammar.disable_rule(self._get_reference_name(name))
+        ref_name = self._get_reference_name(name)
+        jsgf_rule = self._jsgf_grammar.get_rule_from_name(ref_name)
+
+        # Only disable the rule and set the flag if the rule is enabled.
+        if jsgf_rule.active:
+            jsgf_rule.disable()
+            self.set_search = True
 
     def update_list(self, lst):
-        # Remove the old list, recompile the list again and add it to the
-        # grammar.
+        # Recompile the list again.
         name = self._get_reference_name(lst.name)
-        self._jsgf_grammar.remove_rule(name, ignore_dependent=True)
+        old_rule = self._jsgf_grammar.get_rule_from_name(name)
         new_rule = self.engine.compiler.compile_list(lst)
-        self._jsgf_grammar.add_rule(new_rule)
+
+        # Only replace the old rule if the list has changed.
+        if old_rule != new_rule:
+            self._jsgf_grammar.remove_rule(old_rule, ignore_dependent=True)
+            self._jsgf_grammar.add_rule(new_rule)
+            self.set_search = True
 
     def compile_jsgf(self):
         return self._jsgf_grammar.compile_as_root_grammar()
@@ -75,7 +91,7 @@ def search_name(self):
 
         :return: str
         """
-        return "g_%d" % self._id
+        return "g_%s" % self._jsgf_grammar.name
 
     @property
     def grammar_active(self):

dragonfly/test/test_engine_sphinx.py

@@ -390,6 +390,19 @@ def test_reference_names_with_spaces(self):
         finally:
             grammar.unload()
 
+    def test_grammar_name_conflicts(self):
+        """ Verify that grammars with the same name are not allowed. """
+        grammar1 = Grammar("test_grammar")
+        grammar1.add_rule(CompoundRule(name="rule", spec="test"))
+        grammar2 = Grammar("test grammar")
+        grammar2.add_rule(CompoundRule(name="rule", spec="test"))
+        try:
+            grammar1.load()
+            self.assertTrue(grammar1.loaded)
+            self.assertRaises(EngineError, grammar2.load)
+        finally:
+            grammar1.unload()
+
     def test_training_session(self):
         """ Verify that no recognition processing occurs when training. """
         # Set up a rule to "train".