[asterisk-dev] [Code Review] Add command to send DTMF from ExternalIVR

[Tilghman Lesher]

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



/trunk/apps/app_externalivr.c
<https://reviewboard.asterisk.org/r/465/#comment3117>

    Not a direct criticism of your code, but it would be helpful in the future if this list were alphabetized by command letter -- this helps avoid command conflicts.



/trunk/apps/app_externalivr.c
<https://reviewboard.asterisk.org/r/465/#comment3121>

    This isn't actually a timeout, as it's used.  It's a pause between DTMF digits, in millisecond units.



/trunk/apps/app_externalivr.c
<https://reviewboard.asterisk.org/r/465/#comment3118>

    It's better to use sscanf to convert from ASCII to a number, because sscanf can detect errors in the input.  While you're here, add braces around conditionals.
    
    Possibly better, use ast_app_parse_timelen() for these intervals.



/trunk/apps/app_externalivr.c
<https://reviewboard.asterisk.org/r/465/#comment3120>

    %d for integers, please



/trunk/apps/app_externalivr.c
<https://reviewboard.asterisk.org/r/465/#comment3119>

    Fix leading spaces while you're modifying this code.


- Tilghman


On 2010-01-18 12:21:44, thedavidfactor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/465/
> -----------------------------------------------------------
> 
> (Updated 2010-01-18 12:21:44)
> 
> 
> Review request for Asterisk Developers and Chris Tooley.
> 
> 
> Summary
> -------
> 
> This duplicates the functionality of app_senddtmf to allow app_externalivr to send DTMFs. This allows IVRs to communicate with Alarms systems and other DTMF based systems for command and control operations among other possiblities
> 
> 
> This addresses bug 16615.
>     https://issues.asterisk.org/view.php?id=16615
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_externalivr.c 240969 
> 
> Diff: https://reviewboard.asterisk.org/r/465/diff
> 
> 
> Testing
> -------
> 
> This has only had limited testing, pushing out for review as quickly as possible. Will update as more feedback and testing is done.
> 
> 
> Thanks,
> 
> thedavidfactor
> 
>