[asterisk-dev] [Code Review]

[Mark Michelson]

> On 2009-02-06 16:44:01, Mark Michelson wrote:
> >

I can't seem to just update the summary on this review request. It should be "Fix the 'd' option to Dial and add the ability to not mark a CDR answered in the Answer application"


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/145/#review367
-----------------------------------------------------------


On 2009-02-06 16:38:49, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/145/
> -----------------------------------------------------------
> 
> (Updated 2009-02-06 16:38:49)
> 
> 
> Review request for Asterisk Developers and Steve Murphy.
> 
> 
> Summary
> -------
> 
> Issue 14164 was opened because the 'd' option to Dial was not working correctly when using SIP phones. I determined that the reason was that rfc2833 or inband DTMF would not be able to flow without an RTP session set up. While it would be nice if this could be solved by setting up early media between the endpoint and Asterisk, my experience in working on this (plus a nudge from file, thanks!) showed that phones do not seem willing to pass DTMF until the call has been answered. Thus the simple fix for this bug was to answer the channel in app_dial if we see that the OPT_DTMF_EXIT flag is set.
> 
> I got to thinking, though, and realized that this method could result in unexpected CDR dispositions and answer times, so I modified __ast_answer to take a third parameter. This third parameter is a boolean which tells if we should mark the CDR as answered or not. Since I was adding this to the __ast_answer function, I realized that this could be a handy option to have in the dialplan for the Answer application. So I modified builtin_answer in pbx.c to take an optional "nocdr" argument which will not mark the CDR answered if set.
> 
> My plan, assuming this is approved, is to place everything except for the modification of builtin_answer into all branches of Asterisk. The modification to builtin_answer will be trunk-only since it is a new feature.
> 
> 
> This addresses bug 14164.
>     http://bugs.digium.com/view.php?id=14164
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_dial.c 174076 
>   /trunk/include/asterisk/channel.h 174076 
>   /trunk/main/channel.c 174076 
>   /trunk/main/pbx.c 174076 
> 
> Diff: http://reviewboard.digium.com/r/145/diff
> 
> 
> Testing
> -------
> 
> My testing included making sure that the CDR was not marked answered until the call was actually answered by the remote party when used after an answer that does not mark the CDR answered. My testing also included making sure the 'd' option to Dial worked. It did.
> 
> 
> Thanks,
> 
> Mark
> 
>