[asterisk-dev] [Code Review] Fix the 'd' option to Dial and add the ability to not mark a CDR answered in the Answer application

[Steve Murphy]

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


In bug 13892, I've got a patch in which I found it necessary to reset the CDR disposition and answer time at the beginning of Dial, so that any answer time that Dial might report might not be preempted by an Answer()
call that might occur before a Dial() is performed. I'd
think that if an incoming call came in, and a Dial were performed, the CDR would probably be more valuable reporting the disposition of the Dial rather than the disposition of an Answer(). (If the dial() got executed, you know the channel got answered anyway...)

Providing an option for Answer() that would allow it not to set the Answer() time adds to the raw power of the Dialplan in manipulating CDR contents, so I'm all for it, but I wonder if other applications like Background might not answer the channel afterwards and set the CDR answer time anyway.

The whole CDR system is incredibly baroque and complex and hard to describe/define/understand, so adding this little feature seems very apropo! I hope I'm not coming across as a little sarcastic here, I don't mean to be, I've looked over the code itself and it looks acceptably formatted and coded. Perhaps in 1.8 or sometime we can rip all the CDR code out of Asterisk and go to a simpler way of life.


- Steve


On 2009-02-06 17:13:57, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/145/
> -----------------------------------------------------------
> 
> (Updated 2009-02-06 17:13:57)
> 
> 
> 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
> 
>