[Home]

Summary:ASTERISK-12274: [patch] ResetCDR does not work on non-answered channel
Reporter:Merkulov Alexander (meral)Labels:
Date Opened:2008-06-28 20:50:26Date Closed:2009-11-05 08:52:04.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:CDR/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) null-cdr1.diff
( 1) null-cdr2.diff
Description:i am using resetCDR(w) for re-setup cdr and post prevous one
when channel is answered, resetCDr(w) works as documented
so:
exten =>s,1,Answer
exten =>s,n,ResetCDR(w)
exten =>s,n,ResetCDR(w)
produce 3 records
but if channel unanswered,then
exten =>s,1,ResetCDR(w)
exten =>s,n,ResetCDR(w)
produce only 1 record.
Comments:By: Steve Murphy (murf) 2008-06-28 21:11:07

meral--

Could you please do an "svn checkout' of

http://svn.digium.com/svn/asterisk/team/murf/CDRfix4

which is based on 1.4, and see if the problem is solved there.

If not, I'll look into this problem. I hope to merge these changes into
1.4 very soon, and would highly appreciate your feedback on this.

By: Merkulov Alexander (meral) 2008-06-28 21:33:18

same behavour. nothing changes
with answer:
  -- Executing Answer("SIP/67.213.69.43-08f8f920", "")
   -- Executing Set("SIP/67.213.69.43-08f8f920", "CDR(accountcode)=122")
   -- Executing Set("SIP/67.213.69.43-08f8f920", "incoming_did=123456789")
   -- Executing NOOP("SIP/67.213.69.43-08f8f920", "incoming_macros=4")
   -- Executing Set("SIP/67.213.69.43-08f8f920", "CDR(userfield)=123456789:INCOMING CALL to ivr")
   -- Executing Macro("SIP/67.213.69.43-08f8f920", "inc_4|122|86")
   -- Executing [s@macro-inc_4:1] ResetCDR("SIP/67.213.69.43-08f8f920", "w") in new stack
      > cdr_odbc: Query Successful!

WITHOUT answer
   -- Executing Noop("SIP/67.213.69.43-08f8f920", "")
   -- Executing Set("SIP/67.213.69.43-08f8f920", "CDR(accountcode)=122")
   -- Executing Set("SIP/67.213.69.43-08f8f920", "incoming_did=123456789")
   -- Executing NOOP("SIP/67.213.69.43-08f8f920", "incoming_macros=4")
   -- Executing Set("SIP/67.213.69.43-08f8f920", "CDR(userfield)=123456789:INCOMING CALL to ivr")
   -- Executing Macro("SIP/67.213.69.43-08f8f920", "inc_4|122|86")
   -- Executing [s@macro-inc_4:1] ResetCDR("SIP/67.213.69.43-08f8f920", "w") in new stack
   -- Executing [s@macro-inc_4:2] Set("SIP/67.213.69.43-08f8f920", "CDR(accountcode)=122") in new stack
By: Steve Murphy (murf) 2008-08-06 12:13:01

Just one question: on the surface, whatever unexpected deeply hidden requirement that ResetCDR() might have for Answer(), are you harmed in any way by calling it from your dialplan? This is an easy workaround. Somewhere, way down deep, the CDR system may require a channel to be up for some operations...
By: Merkulov Alexander (meral) 2008-08-06 13:06:08

sorry, it MAY require, but in anycase it is WRONG
becuase if no answer, on softswitch must be zero time, but still must be cdr.
now it works with answer, but if i answer lien will be incoorect answer supervision, it is very bad. cdr work ok. cdr to res_cdr_odbc not work if no answer
By: Merkulov Alexander (meral) 2008-08-06 13:10:48

and only in case of reset_cdr. after hangup it works. but not create 2 record as expected
By: Steve Murphy (murf) 2008-09-10 22:53:36

OK, I've labbed this one up, and ran it thru tests.

These are single-channel CDR's, and are affected by the
unanswered = no
declaration in cdr.conf

if you remove that, or change it to unanswered = yes in cdr.conf,
you'll see all the Reset() cdrs, whether answer() was called or
not.

To be totally truthful, it wouldn't hurt us to sit down and
come up with better criteria as to which CDR's are useful and
which are not. Right now, my personal feeling is that the criteria
for filtering single-channel CDR's is rather shaky. Stuff like
only IVR-answered incoming calls are useful and should end up in the CDR
record, but the unanswered=no was meant to filter out the unanswered
channels that happen when Dial calls multiple lines at once, where one
wins, but the others are dropped.

Perhaps it is time to re-evaluate this entire situation and come up with
a spec that everyone can live with.
By: Steve Murphy (murf) 2009-02-06 12:30:12.000-0600

Earlier, I said (and I quote)

"To be totally truthful, it wouldn't hurt us to sit down and
come up with better criteria as to which CDR's are useful and
which are not. "

Well, I sat down and thought about the current criteria, and
made mods to chan_dahdi and app_dial for 13892; I've been busy testing them under
different conditions.

Probably similar changes will have to be done for other channel drivers,
we'll see. I eliminated disposition entirely as a condition for posting a CDR, in favor of the dstchannel being set.
I cleaned up the publishing of peer dial attempts when unanswered=yes. And when unanswered=no, you get every dial attempt. Dial will now reset any disposition and answer times as it is starting, just in case a previous Answer() was performed. The result is that you see your 3 CDR's no matter the call disposition in every case. I'll be checking that chan_sip is doing the right thing later today.

If you can't test this with dahdi (analog) phones or t1/pri type phones, then wait I check the sip stuff, probably later today.

By: Steve Murphy (murf) 2009-02-06 13:54:23.000-0600

OK, just checked a bunch of sip/ scenarios; sip phone->sip phone, existing/non-existant, FAIL level, everything works fine. No changes nec.
Please test, then and see if the patch on 13892 solves these problems.
By: Leif Madsen (lmadsen) 2009-02-27 16:32:13.000-0600

Pinging meral. Any chance of you testing this patch out for you?
By: Merkulov Alexander (meral) 2009-02-27 16:50:58.000-0600

sorry, i not working with that project now. and have no access.
but test case very simple.
u dialling in, and then dialplan like folowing
exten =>_X.,1,wait(1)
exten =>_X.,2,Busy
it must write record to db(user mut know that was missed call)
By: Merkulov Alexander (meral) 2009-02-27 16:52:56.000-0600

before it must be reset()..
so system must produce 1 record for every dialling attempt.
By: Matthew Nicholson (mnicholson) 2009-05-07 15:20:48

I am taking a look at this issue.  Currently for the following test cases (taken from the original report) in asterisk 1.4 SVN the results are as follows:

exten =>s,1,Answer
exten =>s,n,ResetCDR(w)
exten =>s,n,ResetCDR(w)

Only 1 cdr record

exten =>s,1,ResetCDR(w)
exten =>s,n,ResetCDR(w)

NO cdr records (unanswered = yes)

In both cases, two CDR records should be produced.  I am looking into this.
By: Matthew Nicholson (mnicholson) 2009-05-07 15:41:45

Correction.  This works exactly as it should in asteirsk 1.4 SVN.  I was testing with my oldcdr branch which seems to work differently.
By: Matthew Nicholson (mnicholson) 2009-05-07 16:11:02

Actually this does not work as it should in 1.4 SVN.  It only produces two CDR records in both cases, not 3 as it should.
By: Matthew Nicholson (mnicholson) 2009-05-07 16:16:25

I know why it does not post the last CDR record (disposition is AST_CDR_NULL, and ast_hangup() does not like this).
By: Matthew Nicholson (mnicholson) 2009-05-14 15:31:57

Please test the patch I just uploaded (null-cdr1.diff).  That should fix the problem.  Please look for and note any side effects in your testing.
By: Matthew Nicholson (mnicholson) 2009-05-20 16:41:15

null-cdr2.diff uploaded.  Minor fixes.  Please test.
By: Leif Madsen (lmadsen) 2009-05-21 10:12:08

Any chance we can get a test of this code soon?
By: Leif Madsen (lmadsen) 2009-05-21 10:55:22

Reassigned to dbrooks so he can setup a scenario to verify the issue exists, then apply the patch to run the same test, and find that it solves the issue. Thanks!
By: Leif Madsen (lmadsen) 2009-05-21 15:22:36

meral: sorry, I forgot to scroll up and see you're not on that project anymore.
By: David Brooks (dbrooks) 2009-05-21 15:57:30

Tested issue with 1.4 SVN. Answer -> ResetCDR -> ResetCDR and ResetCDR -> ResetCDR both reported only 2 CDRs (unanswered=yes). After applying the latest patch (null-cdr2.diff), both situations reported 3 CDRs.

It appears the patch correctly fixes the issue reported.
By: Digium Subversion (svnbot) 2009-05-29 13:53:02

Repository: asterisk
Revision: 198068

U   branches/1.4/include/asterisk/cdr.h
U   branches/1.4/main/cdr.c
U   branches/1.4/main/channel.c
U   branches/1.4/res/res_features.c

------------------------------------------------------------------------
r198068 | mnicholson | 2009-05-29 13:53:01 -0500 (Fri, 29 May 2009) | 15 lines

Use AST_CDR_NOANSWER instead of AST_CDR_NULL as the default CDR disposition.

This change also involves the addition of an AST_CDR_FLAG_ORIGINATED flag that is used on originated channels to distinguish: them from dialed channels.

(closes issue ASTERISK-12274)
Reported by: meral
Patches:
     null-cdr2.diff uploaded by mnicholson (license 96)
Tested by: mnicholson, dbrooks

(closes issue ASTERISK-14140)
Reported by: sum
Tested by: sum


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=198068
By: Digium Subversion (svnbot) 2009-05-29 14:04:24

Repository: asterisk
Revision: 198072

_U  trunk/
U   trunk/include/asterisk/cdr.h
U   trunk/main/cdr.c
U   trunk/main/channel.c

------------------------------------------------------------------------
r198072 | mnicholson | 2009-05-29 14:04:24 -0500 (Fri, 29 May 2009) | 21 lines

Merged revisions 198068 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r198068 | mnicholson | 2009-05-29 13:53:01 -0500 (Fri, 29 May 2009) | 15 lines
 
 Use AST_CDR_NOANSWER instead of AST_CDR_NULL as the default CDR disposition.
 
 This change also involves the addition of an AST_CDR_FLAG_ORIGINATED flag that is used on originated channels to distinguish: them from dialed channels.
 
 (closes issue ASTERISK-12274)
 Reported by: meral
 Patches:
       null-cdr2.diff uploaded by mnicholson (license 96)
 Tested by: mnicholson, dbrooks
 
 (closes issue ASTERISK-14140)
 Reported by: sum
 Tested by: sum
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=198072
By: Digium Subversion (svnbot) 2009-05-29 14:13:03

Repository: asterisk
Revision: 198073

_U  branches/1.6.0/
U   branches/1.6.0/include/asterisk/cdr.h
U   branches/1.6.0/main/cdr.c
U   branches/1.6.0/main/channel.c

------------------------------------------------------------------------
r198073 | mnicholson | 2009-05-29 14:13:03 -0500 (Fri, 29 May 2009) | 28 lines

Merged revisions 198072 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r198072 | mnicholson | 2009-05-29 14:04:24 -0500 (Fri, 29 May 2009) | 21 lines
 
 Merged revisions 198068 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r198068 | mnicholson | 2009-05-29 13:53:01 -0500 (Fri, 29 May 2009) | 15 lines
   
   Use AST_CDR_NOANSWER instead of AST_CDR_NULL as the default CDR disposition.
   
   This change also involves the addition of an AST_CDR_FLAG_ORIGINATED flag that is used on originated channels to distinguish: them from dialed channels.
   
   (closes issue ASTERISK-12274)
   Reported by: meral
   Patches:
         null-cdr2.diff uploaded by mnicholson (license 96)
   Tested by: mnicholson, dbrooks
   
   (closes issue ASTERISK-14140)
   Reported by: sum
   Tested by: sum
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=198073
By: Digium Subversion (svnbot) 2009-05-29 14:13:45

Repository: asterisk
Revision: 198074

_U  branches/1.6.1/
U   branches/1.6.1/include/asterisk/cdr.h
U   branches/1.6.1/main/cdr.c
U   branches/1.6.1/main/channel.c

------------------------------------------------------------------------
r198074 | mnicholson | 2009-05-29 14:13:45 -0500 (Fri, 29 May 2009) | 28 lines

Merged revisions 198072 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r198072 | mnicholson | 2009-05-29 14:04:24 -0500 (Fri, 29 May 2009) | 21 lines
 
 Merged revisions 198068 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r198068 | mnicholson | 2009-05-29 13:53:01 -0500 (Fri, 29 May 2009) | 15 lines
   
   Use AST_CDR_NOANSWER instead of AST_CDR_NULL as the default CDR disposition.
   
   This change also involves the addition of an AST_CDR_FLAG_ORIGINATED flag that is used on originated channels to distinguish: them from dialed channels.
   
   (closes issue ASTERISK-12274)
   Reported by: meral
   Patches:
         null-cdr2.diff uploaded by mnicholson (license 96)
   Tested by: mnicholson, dbrooks
   
   (closes issue ASTERISK-14140)
   Reported by: sum
   Tested by: sum
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=198074
By: Digium Subversion (svnbot) 2009-05-29 14:14:30

Repository: asterisk
Revision: 198075

_U  branches/1.6.2/
U   branches/1.6.2/include/asterisk/cdr.h
U   branches/1.6.2/main/cdr.c
U   branches/1.6.2/main/channel.c

------------------------------------------------------------------------
r198075 | mnicholson | 2009-05-29 14:14:30 -0500 (Fri, 29 May 2009) | 28 lines

Merged revisions 198072 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r198072 | mnicholson | 2009-05-29 14:04:24 -0500 (Fri, 29 May 2009) | 21 lines
 
 Merged revisions 198068 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r198068 | mnicholson | 2009-05-29 13:53:01 -0500 (Fri, 29 May 2009) | 15 lines
   
   Use AST_CDR_NOANSWER instead of AST_CDR_NULL as the default CDR disposition.
   
   This change also involves the addition of an AST_CDR_FLAG_ORIGINATED flag that is used on originated channels to distinguish: them from dialed channels.
   
   (closes issue ASTERISK-12274)
   Reported by: meral
   Patches:
         null-cdr2.diff uploaded by mnicholson (license 96)
   Tested by: mnicholson, dbrooks
   
   (closes issue ASTERISK-14140)
   Reported by: sum
   Tested by: sum
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=198075