[asterisk-dev] [Code Review] 3011: Say suite of applications: Add the ability to listen for DTMF and jump to a new extension (like Background)

[Mark Michelson]

> On Nov. 13, 2013, 7:13 p.m., Mark Michelson wrote:
> > /trunk/CHANGES, lines 26-27
> > <https://reviewboard.asterisk.org/r/3011/diff/2/?file=48180#file48180line26>
> >
> >     Since you switched to use ast_true() instead of a direct string comparison, the value doesn't have to be "true" but any of the strings accepted by ast_true (yes, true, y, t, 1, or on). However, it would suck to have to list all this out everywhere, so probably think of some shorthand way of saying this. "If SAY_DTMF_INTERRUPT is enabled" or "If SAY_DTMF_INTERRUPT is switched on" or something?
> >     
> >     I won't comment on the other places where such a change is necessary.
> 
> Jonathan Rose wrote:
>     Yeah, I agree that it's less than comprehensive, but what I worry about is that if we just say 'if it is enabled' that people won't really know what to set it to just from looking at the documentation. Saying 'true' at least makes it clear what you can set the variable to in order to make the feature work... even if there are other values that will make it work.

Okay, I can see where you're coming from. A channel variable value doesn't have the same semantics as a typical configuration option, so using the same language and expecting the intent to be clear could be a problem.

Ship it!


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3011/#review10174
-----------------------------------------------------------


On Nov. 13, 2013, 5 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3011/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2013, 5 p.m.)
> 
> 
> Review request for Asterisk Developers, Mark Michelson and rmudgett.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Say applications didn't currently have a way to wait for DTMF like the background application does. One exception to this rule existed, that being the SayUnixTime application with 'jump' option enabled. Since some of these applications could not be changed to accept new arguments without breaking existing dialplan, a new channel variable 'SAY_DTMF_INTERRUPT' will now be checked for by these applications, and if it is true on the channel that is running the application, it will listen for DTMF similar to Background.
> 
> 
> Diffs
> -----
> 
>   /trunk/main/pbx.c 402698 
>   /trunk/apps/app_sayunixtime.c 402698 
>   /trunk/CHANGES 402698 
> 
> Diff: https://reviewboard.asterisk.org/r/3011/diff/
> 
> 
> Testing
> -------
> 
> Tested entering extensions in each type of Say application modified with and without setting the SAY_DTMF_INTERRUPT variable to 'true'.  Also tested SayCounted applications (which all already act like Background unconditionally and won't be changed).
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131113/ece18e2f/attachment-0001.html>