Added DTMF limit support in place of GRXML grammars. #27
Conversation
|
Nice contribution, and thank you for the test as well! |
There was a problem hiding this comment.
This behavior is a little confusing to me. I think what you are intending here is for limit to be at the same logical level as a grammar, meaning that you can provide only limit of the number of DTMF digits that you want and skip providing either grammar or grammar_url. But the name limit doesn't really say that to me.
Since this is a convenience method, what if it was named something like dtmf_digits? The other question I have then is whether it should return a simple number, or instead a hash of DTMF-related options.
Can you share some more use-cases here so we can discuss the right approach?
|
Use Case: I have a project which was initially developed to support ASR, so we wrote it on top of adhearsion-ivr. The logic separation that this enforces is great. As we neared the project completion, our ASR requirement was scrapped so that the IVR will only use DTMF input. With the way that adhearsion-ivr was written, DTMF input requires the UniMRCP recognizer. Our two options appeared to be that we continue using the recognizer or that we completely rewrite all of the menus. This opens up a third option where we can switch between ASR and Freeswitch's internal DTMF processing with relative ease. Renaming this from "limit" to "dtmf_digits" may be a good idea. It may also be a good idea to add support for the :terminator option. Does that make more sense for what this change does? Is there a better way to implement this support (limit + grammar required the recognizer)? |
|
I did try change request #26 also. input_mode => nil comes close to doing what we need, but it required additional changes to the grammar in order for DTMF to behave as our original SRE engine's parser. If we have to modify that code anyway, this is the simpler approach. And... my test changes got pulled into this change request. Sorry about that - we don't need these last two changes. |
| @@ -21,7 +21,8 @@ Gem::Specification.new do |s| | |||
| s.executables = `git ls-files -- bin/*`.split("\n").map{ |f| File.basename(f) } | |||
| s.require_paths = ["lib"] | |||
|
|
|||
| s.add_runtime_dependency 'adhearsion', ["~> 3.0.0.beta"] | |||
| s.add_runtime_dependency 'adhearsion', ["~> 2.0"] | |||
There was a problem hiding this comment.
You should submit this against a support branch for Adhearsion 2.x, not revert master to pre-ahn3
|
Because of my goof-up with code changes, I may have to create another pull request. After thinking about DTMF-relevant names, would these work?: max_dtmf_limit, dtmf_terminator Is there anything else that I should change? |
This should allow us to continue using GRXML with adhearsion-ivr, or to have mod_rayo process DTMF.