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

[Mark Michelson]

> On 2009-02-06 18:20:26, Steve Murphy wrote:
> > 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.
> >

In regards to your concern about Background and similar apps, I don't think that would actually be a problem. While apps like Background are set to auto-answer, they will not do so if the channel's state is AST_STATE_UP. So picture a dialplan like this one:

exten => blah,1,Answer(0,nocdr)
exten => blah,2,Background(hello-world)
exten => blah,3,Dial(SIP/400)
exten => blah,4,Hangup

For the sake of this discussion, let's say that the caller is at SIP/200. When priority 1 is reached, Asterisk will send a 200 OK to SIP/200, will mark SIP/200's channel as up, and will not do any operation on the CDR. At priority 2, the Background application does not attempt to do any answer operation on the SIP/200's channel since it has already been marked as up. When the Dial application executes, ast_answer will be called on SIP/200 once we receive a 200 OK from SIP/400. It is at this point that the CDR actually is marked answered and an answer time is logged.

As long as applications which can automatically answer a channel only do so in the case where the channel is not already up, there should be no unexpected behavior.


- Mark


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


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
> 
>