[Home]

Summary:ASTERISK-10252: Incomplete CDR lock
Reporter:Dmytro Mishchenko (arkadia)Labels:
Date Opened:2007-09-07 08:26:19Date Closed:2008-06-13 11:39:06
Priority:MinorRegression?No
Status:Closed/CompleteComponents:CDR/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 10668.proposed.patch
( 1) 10668.proposed.patch2
( 2) 10668.proposed.patch3
( 3) 10668.proposed.patch4
( 4) cdr_missedlock.patch
Description:This is all about the meaning of AST_CDR_FLAG_LOCKED flag for CDR.
I assume if CDR has this flag set then it wont be updated anymore by any ast_cdr_ functions except cases when we force such update. ast_cdr_reset() is example of function which allows to update locked CDRs if special flag is given.

Based on this assumption I prepared the patch with such changes:

 ast_cdr_setvar() - not allowed to touch locked CDRs. I don't see any cases when we need to change locked CDRs too. Not touching locked CDRs is helpfull when ForkCDR being used and every new CDR may have different custom variables.

 ast_cdr_answer() - don't change status of locked CDRs. That might be helpfull when we have a chain of CDRs for one incoming call for which we perform several tries to make outgoing call. Every outgoing attempt may finish with its own completion code. And if the final attempt is 'answered' then we have only one CDR with 'answered' status. All other will keep connection error codes which is very helpful for error monitoring.

ast_cdr_end() - don't change locked CDRs by the same reason as ast_cdr_answer()

As there will be no changes in CDR after lock, app_forkcdr.c should be updated to end original locked CDR.
Also it'll be great to make optional cdr reset in this application, but thats a theme for another patch.

****** ADDITIONAL INFORMATION ******

After this patch it'll be possible to use ForkCDR for generating additional CDRs and keeping information about Dial attempts. Here is dialplan example of such scenario:

exten => _Z.,n, While(routes...)
exten => _Z.,n, Dial(the_route)
exten => _Z.,n, ForkCDR(...)
exten => _Z.,n, EndWhile

I would like to ask murf to check this update too. He was doing cdr related changed in res/res_features.c.
Comments:By: Steve Murphy (murf) 2007-09-11 10:14:42

While I agree with most of your statements, I am afraid (yes, I feel FEAR), that the one change to end the current CDR before locking it, is a non-compatible fix. It's the first glob in the patch.

Why am I afraid to make this change? Because I have no idea how many forkCDR users are counting on the fact that CDR's are ended on hangup, not on forking.

While there is absolutely no documentation associated with forkCDR, and nothing states its purpose, design goals, or philosophy, right now it implicily creates a stack of CDRs, of which the top CDR on the stack is the one that is supposed to get all the mods/updates. They are all closed out on hangup.

Changing this would probably break all current usage, and cause the collapse of the economies of several small countries. Civil war would most likely break out in several others. Undoubtedly, the balance of world power would be shifted, and ... everybody would point at me, and yell "He did it!!!!".

Instead, it might be safer to come up with a new app, or better yet, an option to forkCDR to end the preceding CDR. This way, existing code won't break, and we won't have the death of thousands on our consciences! What do you think?
By: Steve Murphy (murf) 2007-09-11 12:11:49

Another note: I see 10666;

Arkadia... FYI.... my current plan is to eliminate forkCDR entirely in trunk.
I'm doing work (not complete yet by any means) in the branch
team/group/CDRfix5... see funcs/func_cdr.c for CDR_CONTROL(). The function
of your dreams! (or is that nightmare?)
By: Dmytro Mishchenko (arkadia) 2007-09-12 03:50:31

I understand your concerns about ForkCDR. But we may save its original behavior adding extra LOCK flag (e.g. AST_CDR_FLAG_FORKLOCK) in ast_cdr_setvar(), ast_cdr_answer(), ast_cdr_end(). This flag will be set by ForkCDR and will keep CDR unlocked in mentioned functions.

Standard AST_CDR_FLAG_LOCKED will be treated in every ast_cdr_* function as its done in this patch and according our initial assumption what lock should do. I assume these changes compatible with your cdr work in res/res_features.c.


This open possibilities for another 'fork' application. Here is possible description (I'm working on it right now for my own purpose):

[Synopsis]
Forks the Call Data Record

[Description]
 ForkCDRext([options]):  Causes the Call Data Record to fork an additional
cdr record starting from the time of the fork call.
 Options:
   d - clear dstchannel on new CDR
   e - end original CDR. Without this option original CDR not ended.
   R - don't reset new CDR. By default reset new CDR.
   s(name=val) - set CDR variable in original CDR
   v - all cdr variables will be passed along also. Keep all CDR vars.

 Example: we need status of previous Dial attempt,
we don't want duplicate CDRs with the same dstchan and its error,
previous CDRs will be marked with additional variable 'cdridx'.
 ForkCDRext(deRvs(cdridx=${ROUTENUM}))


It may be implemented compatible with current ForkCDR() or ForkCDR(v) calls if required.
By: Dmytro Mishchenko (arkadia) 2007-09-12 05:41:49

I briefly check CDRfix5.

Imagine you have dialplan with Dial(). How are you going to set  Set(CDR_CONTROL(${mycdr})=answer) at the same moment when cannel was answered in Dial? How can it be closed if call is finished in Dial().


Set(mycdr=${CDR_CONTROL(start)}). 'mycdr' by default should behave like its parent CDR. Othervise it'll be too much work in dialplan to take care about every CDR field. E.g. I don't want to spend time setting 'lastapp' after each application call.


In your example:
"                Set(mycdr=${CDR_CONTROL(start)});\n"
"                Set(CDR_CONTROL(${mycdr})=answer);\n"
"                Set(CDR(lastapp,,${mycdr})=Record+BackGround);\n"
"                Set(CDR(lastdata,,${mycdr})=whoknows);\n"
"                Record(recording:wav);\n"
"                Background(recording);\n"
"                Set(CDR_CONTROL(${mycdr})=close);\n"
CDR fields in dialplan updated one by one. We should remember that call may be interrupted at any time and next Set() may never be executed. It opens possibilities for all kind of race conditions. At least all assignments should be done by one Set(CDR()=....,CDR()=...,)


"Every CDR created with CDR_CONTROL() should be matched at some point with a 'close' or 'abort' action to avoid memory leaks." Call may be interrupted at any moment, so I expect "Set(CDR_CONTROL(${mycdr})=close)" won't be reached all the time.


I don't see how 'mycdr' data going to be saved. Are you going to post it together with regular CDR through every supported CDR engine?


As I understand CDR_CONTROL actually controls not a CDR but a set of variables under 'handle'. This kind of behavior can be implemented using current custom CDR variables. Like:
Set(CDR(mycdr1_disposition)=answer)
Set(CDR(mycdr1_lastapp)=Record+BackGround)
Set(CDR(mycdr1_lastdata)=whoknows)
Or I miss something?
By: Steve Murphy (murf) 2007-09-14 14:09:21

There's a lot of things to answer... Let's see if I can cover things.

1. CDR_CONTROL() and a hangup. I suggest making sure that any CDR's created
      get closed in the hangup extension (h), at least, if they are cut short.

2. when you end a CDR_CONTROL() cdr, it gets posted to the backend(s).

3. There will no longer be a string of CDR structures on a channel. (ForkCDR was
  the only mechanism that made the list. So, no LOCK flag, no list. Just one
  CDR. It horribly complicated things, and as your bug points out, never was
  fully implemented.

4. CDR field races. You have a good point. We could easily initialize several
  CDR fields from the channel/cdr on initialization. That would save dialplan
  statements and reduce races.

5. Adding arg options to ForkCDR in 1.4; I gave this some thought, and I don't
  think we can get away with it.
  Because it's released, we really have no excuse for modifying an app
  interface. It's an enhancement, no matter how useful or badly needed.
  And I don't see ForkCDR in trunk; So, let's see if CDR_CONTROL() or dialplan%0
By: Steve Murphy (murf) 2007-09-14 14:10:55

There's a lot of things to answer... Let's see if I can cover things.

1. CDR_CONTROL() and a hangup. I suggest making sure that any CDR's created
      get closed in the hangup extension (h), at least, if they are cut short.

2. when you end a CDR_CONTROL() cdr, it gets posted to the backend(s).

3. There will no longer be a string of CDR structures on a channel. (ForkCDR was
  the only mechanism that made the list. So, no LOCK flag, no list. Just one
  CDR. It horribly complicated things, and as your bug points out, never was
  fully implemented.

4. CDR field races. You have a good point. We could easily initialize several
  CDR fields from the channel/cdr on initialization. That would save dialplan
  statements and reduce races.

5. Adding arg options to ForkCDR in 1.4; I gave this some thought, and I don't
  think we can get away with it.
  Because it's released, we really have no excuse for modifying an app
  interface. It's an enhancement, no matter how useful or badly needed.
  And I don't see ForkCDR in trunk; So, let's see if CDR_CONTROL() or dialplan
  logic can get the needed effect.

6. Set of variables == CDR; no, not really. It's a real live CDR, just not
  attached to a channel; No automatic handling at all. Purely a function
  of the dialplan; end== posted to backends.
By: Digium Subversion (svnbot) 2007-09-14 16:00:58

Repository: asterisk
Revision: 82444

------------------------------------------------------------------------
r82444 | murf | 2007-09-14 16:00:58 -0500 (Fri, 14 Sep 2007) | 1 line

closes issue ASTERISK-10252; thanks to arkadia for his patch; had to leave out the bit about ending the previous cdr in the fork; it would destroy current implementations.
------------------------------------------------------------------------
By: Digium Subversion (svnbot) 2007-09-14 16:12:03

Repository: asterisk
Revision: 82457

------------------------------------------------------------------------
r82457 | murf | 2007-09-14 16:12:03 -0500 (Fri, 14 Sep 2007) | 9 lines

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

........
r82444 | murf | 2007-09-14 15:19:27 -0600 (Fri, 14 Sep 2007) | 1 line

closes issue ASTERISK-10252; thanks to arkadia for his patch; had to leave out the bit about ending the previous cdr in the fork; it would destroy current implementations.
........

------------------------------------------------------------------------
By: Steve Murphy (murf) 2008-05-28 18:29:05

Because of 12726, I have re-evaluated the status of this bug.

I intend to revert the changes made.

Why? because the folks who submitted 12726 note the difference in behavior,
which is traced to these changes.

Apparently, we have now found the reason why at least cdr_end did its thing
on everything in the chain, locked or unlocked... app_forkCDR() is relying on this behavior to close out all the CDR's in the chain at the same time.

Since folks are depending on this behavior, and have been for a long time, apparently, it's foolish to change it now. Sorry, arkadia, but if you wrote CDR related code since last sept, you may have to keep applying your own patch to keep your code going.

If you really need this behavior, then let's figure out a way so you can get what you need, without inconveniencing the other folks who are depending on this behavior.


By: Digium Subversion (svnbot) 2008-05-28 19:19:03

Repository: asterisk
Revision: 118858

U   branches/1.4/apps/app_forkcdr.c
U   branches/1.4/main/cdr.c

------------------------------------------------------------------------
r118858 | murf | 2008-05-28 19:19:02 -0500 (Wed, 28 May 2008) | 46 lines

(closes issue ASTERISK-10252)
(closes issue ASTERISK-11191)
(closes issue ASTERISK-12085)
Reported by: arkadia
Tested by: murf

These changes:

1. revert the changes made via bug 10668;
  I should have known that such changes,
  even tho they made sense at the time,
  seemed like an omission, etc, were actually
  integral to the CDR system via forkCDR.
  It makes sense to me now that forkCDR didn't
  natively end any CDR's, but rather depended
  on natively closing them all at hangup time
  via traversing and closing them all, whether
  locked or not. I still don't completely
  understand the benefits of setvar and answer
  operating on locked cdrs, but I've seen
  enough to revert those changes also, and
  stop messing up users who depended on that
  behavior. bug 12726 found reverting the changes
  fixed his changes, and after a long review
  and working on forkCDR, I can see why.
2. Apply the suggested enhancements proposed
  in 10668, but in a completely compatible
  way. ForkCDR will behave exactly as before,
  but now has new options that will allow some
  actions to be taken that will slightly
  modify the outcome and side-effects of
  forkCDR. Based on conversations I've had
  with various people, these small tweaks
  will allow some users to get the behavior
  they need. For instance, users executing
  forkCDR in an AGI script will find the
  answer time set, and DISPOSITION set,
  a situation not covered when the routines
 were first written.
3. A small problem in the cdr serializer
  would output answer and end times even
  when they were not set. This is now
  fixed.



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

http://svn.digium.com/view/asterisk?view=rev&revision=118858
By: Digium Subversion (svnbot) 2008-05-28 20:23:18

Repository: asterisk
Revision: 118880

_U  trunk/
U   trunk/apps/app_forkcdr.c
U   trunk/main/cdr.c

------------------------------------------------------------------------
r118880 | murf | 2008-05-28 20:22:59 -0500 (Wed, 28 May 2008) | 54 lines

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

........
r118858 | murf | 2008-05-28 18:25:28 -0600 (Wed, 28 May 2008) | 46 lines

(closes issue ASTERISK-10252)
(closes issue ASTERISK-11191)
(closes issue ASTERISK-12085)
Reported by: arkadia
Tested by: murf

These changes:

1. revert the changes made via bug 10668;
  I should have known that such changes,
  even tho they made sense at the time,
  seemed like an omission, etc, were actually
  integral to the CDR system via forkCDR.
  It makes sense to me now that forkCDR didn't
  natively end any CDR's, but rather depended
  on natively closing them all at hangup time
  via traversing and closing them all, whether
  locked or not. I still don't completely
  understand the benefits of setvar and answer
  operating on locked cdrs, but I've seen
  enough to revert those changes also, and
  stop messing up users who depended on that
  behavior. bug 12726 found reverting the changes
  fixed his changes, and after a long review
  and working on forkCDR, I can see why.
2. Apply the suggested enhancements proposed
  in 10668, but in a completely compatible
  way. ForkCDR will behave exactly as before,
  but now has new options that will allow some
  actions to be taken that will slightly
  modify the outcome and side-effects of
  forkCDR. Based on conversations I've had
  with various people, these small tweaks
  will allow some users to get the behavior
  they need. For instance, users executing
  forkCDR in an AGI script will find the
  answer time set, and DISPOSITION set,
  a situation not covered when the routines
 were first written.
3. A small problem in the cdr serializer
  would output answer and end times even
  when they were not set. This is now
  fixed.



........

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

http://svn.digium.com/view/asterisk?view=rev&revision=118880
By: Digium Subversion (svnbot) 2008-05-28 23:05:31

Repository: asterisk
Revision: 118909

_U  branches/1.6.0/
U   branches/1.6.0/apps/app_forkcdr.c
U   branches/1.6.0/main/cdr.c

------------------------------------------------------------------------
r118909 | murf | 2008-05-28 23:05:27 -0500 (Wed, 28 May 2008) | 62 lines

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

................
r118880 | murf | 2008-05-28 19:29:09 -0600 (Wed, 28 May 2008) | 54 lines

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

........
r118858 | murf | 2008-05-28 18:25:28 -0600 (Wed, 28 May 2008) | 46 lines

(closes issue ASTERISK-10252)
(closes issue ASTERISK-11191)
(closes issue ASTERISK-12085)
Reported by: arkadia
Tested by: murf

These changes:

1. revert the changes made via bug 10668;
  I should have known that such changes,
  even tho they made sense at the time,
  seemed like an omission, etc, were actually
  integral to the CDR system via forkCDR.
  It makes sense to me now that forkCDR didn't
  natively end any CDR's, but rather depended
  on natively closing them all at hangup time
  via traversing and closing them all, whether
  locked or not. I still don't completely
  understand the benefits of setvar and answer
  operating on locked cdrs, but I've seen
  enough to revert those changes also, and
  stop messing up users who depended on that
  behavior. bug 12726 found reverting the changes
  fixed his changes, and after a long review
  and working on forkCDR, I can see why.
2. Apply the suggested enhancements proposed
  in 10668, but in a completely compatible
  way. ForkCDR will behave exactly as before,
  but now has new options that will allow some
  actions to be taken that will slightly
  modify the outcome and side-effects of
  forkCDR. Based on conversations I've had
  with various people, these small tweaks
  will allow some users to get the behavior
  they need. For instance, users executing
  forkCDR in an AGI script will find the
  answer time set, and DISPOSITION set,
  a situation not covered when the routines
 were first written.
3. A small problem in the cdr serializer
  would output answer and end times even
  when they were not set. This is now
  fixed.



........

................

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

http://svn.digium.com/view/asterisk?view=rev&revision=118909
By: Dmytro Mishchenko (arkadia) 2008-06-02 00:32:08

1. I initially wrote how we can preserve original behavior:

"I understand your concerns about ForkCDR. But we may save its original behavior adding extra LOCK flag (e.g. AST_CDR_FLAG_FORKLOCK) in ast_cdr_setvar(), ast_cdr_answer(), ast_cdr_end(). This flag will be set by ForkCDR and will keep CDR unlocked in mentioned functions."

Is it the way to go? I can write a patch with such changes.

Summary of this change:
- ForkCDR will work in exactly the same way as in 1.2 and early 1.4
- ast_cdr_... API will be consistent in the way of treating AST_CDR_FLAG_LOCKED and be ready for applications like ForkCDRext.

2. I don't think its a good idea to introduce new options in branch 1.4 ForkCDR. We can add them to the trunk. Those who need new features in 1.4 will add and support them by themselves.
By: Steve Murphy (murf) 2008-06-02 09:46:45

Arkadia--

Several of the routines that act on CDR's, do so on the entire list, obeying
the LOCKED flag. The 3 did not, and at least for setting the end time, I can understand why it would ignore the LOCK... So that all forked CDR's would be ended at the same time. The original assumptions you mentioned in this bug report about how LOCK would be interpreted in the CDR code seemed reasonable, but were proved to be wrong. There are subtle effects on the CDR system of playing with these flags, and any changes will have to prove they won't muck things up.

As to not adding the flags to 1.4, well, I've heavily tested them, and they aren't hurting anyone by being available in 1.4. But, I know of at least one case, where they are helping to solve problems with situations that weren't considered when ForkCDR was written. So, I'd really rather leave them available in 1.4, for the sake of the few who might find them lifesavers.

You are trying to do something via ForkCDR, I perceive, that involves these LOCKS (or lack of them) getting in the way. What is it you are trying to do, maybe there's another way to accomplish your goal?
By: Dmytro Mishchenko (arkadia) 2008-06-02 10:52:57

What I'm doing is explained in 0070379 note. I'm calling Dial several times for different routes. I would like to collect dial status on every route.
So I need pairs: route+status. I don't want to collect it in one CDR.
So every Dial attempt is a call leg which overs with a CDR. ForkCDRext() is used to create new CDR for every dial attempt. In my case all CDRs ends at different time.

I also understand the case when ForkCDR() maybe used and people needs CDRs to be ended at the same time when the call is over. I'm saying we'll have this behavior if we add a new flag AST_CDR_FLAG_FORKLOCK in ast_cdr_.. API. Do you understand this idea? In ForkCDR() cdr will be marked with AST_CDR_FLAG_FORKLOCK flag and it'll pass in section locked with AST_CDR_FLAG_LOCKED.

E.g. ast_cdr_answer may look like this:

void ast_cdr_answer(struct ast_cdr *cdr)
{
       char *chan;
       while (cdr) {
            if (!ast_test_flag(cdr, AST_CDR_FLAG_LOCKED) ||        ast_test_flag(cdr, AST_CDR_FLAG_FORKLOCK)) {
   ....do the job...
             }
             cdr = cdr->next;
       }
}
By: Steve Murphy (murf) 2008-06-04 12:33:25

The "e" option on forkCDR won't let you end the previous CDR? Won't that
give you what you need?  Hmmm. Let's see... it would, but if it isn't
immediately posted, then you get the "answered" status upgraded to ANSWERED"
when a following Dial() does succeed... Hmmm.

So, I did my research. cdr_end() is OK, because it won't overwrite a previously
set non-zero timestamp.

cdr_answer() will set every cdr with disp < ANSWERED to ANSWERED.

setvar will set the var on every CDR in the chain, if recur is set in the
options. So I checked the CDR() func. and there's a recur option (r) and a last (l) option. the cdr write func doesn't obey the (l) flag, but that's a bug we can fix.

So, can I close this bug, if I:

(1) I can provide one more option ('A') to forkCDR that will prevent the answer time from being updated in LOCKED cdrs. (I'll use something like ANSWERLOCK.)

(2) Update the CDR() write func to obey the (l) flag. I'd think that this a true bug according to the spec, the doc string, and common sense. The only possible hitch might be if people are depending on this bug; then they will have to realize that it is not good to depend on behavior that is the result of a bug.

?

I can/will add this 1.4 and to trunk, and 1.6.0. I'll still consider it a bug fix, as the addition will not hurt current users, and the sparse documentation around forkCDR would allow your interpretation, given some slack.

The only possible hitch might be when I fix CDR() to obey the (l) flag on writing. If people are depending on that, then they will have to realize that it is not good to depend on behavior that is the result of a bug.
By: Dmytro Mishchenko (arkadia) 2008-06-05 16:50:39

>cdr_end() is OK, because it won't overwrite a previously
> set non-zero timestamp.

Yeah. But see my notes below about API. Also somebody may want to lock not ended CDR by some reason.

>cdr_answer() will set every cdr with disp < ANSWERED to ANSWERED.

I think that's the problem. I would like to see LOCKED CDRs untouched.

>setvar will set the var on every CDR in the chain, if recur is set in the
>options.

Note that it starts processing *cdr list from the head. Where in case of several CDR we have an oldest probably locked CDRs. Updating just an oldest locked CDR if recur==0 seems a little bit strange. I can understand why all or just the latest CDR needs to be updated.


It seems we are looking on the final solutions from two different sides:
I'm more concerned about consistence of the API ast_cdr..., and you are talking about proper work of higher level applications. I still suggest we start looking on this issue from API level.

AST_CDR_FLAG_LOCKED is a part of public API used by developers in their applications. Preferably this flag has constant and clear meaning like: if CDR is marked with this flag it is not updated by any ast_cdr_.... functions.

Are we agree on that?

Any LOCKED cdr that require exception from this rule should be explicitly tagged. E.g. ForkCDR() may have code like this:
ast_set_flag(cdr, AST_CDR_FLAG_RECURSIVE_SETVAR | AST_CDR_FLAG_RECURSIVE_ANSWER | AST_CDR_FLAG_RECURSIVE_END);

That will allow us to do a proper fix for CDR() and ForkCDR()
By: Steve Murphy (murf) 2008-06-05 18:43:09

Sorry, but all hope of an API consistency argument is gone. The API was never
documented, so for years, everyone made their own mental API based on its
behavior, which forms an 'implicit' documentation. And that unpublished, but
ubiquitous mental document based on experimentation, trial, and error is the
standard. There is no way you can change the behavior of these funcs and get
it merged into the code. Any changes you would like to make to solve your
problem must not disturb the current behavior. So, the API must stay as it is.

> Also somebody may want to lock not ended CDR by some reason.

They can file a bug, we'll make a new option, and they can achieve this that way.

> AST_CDR_FLAG_LOCKED is a part of public API used by developers in their
> applications. Preferably this flag has constant and clear meaning like:
> if CDR is marked with this flag it is not updated by any ast_cdr_....
> functions.

Sorry, but this is wrong. AST_CDR_FLAG_LOCKED is NOT part of a public API.
It's only minimally documented in the source code. The way it's used right
now is not up for negotiation. People have tons of code built around the
way things work. Any tweaks that need to be made, need to be via new options,
and must be transparent to current implementations.

Also, implementing new funcs and apps are definitely out. First of all, they
complicate the picture, by adding new but similar apps to the existing interface. Users will be confused about which to use in new implementations.
Next, new apps are an enhancement, and are not on the table for discussion for
1.4; If you want new stuff in trunk, we can talk, but if you want this in 1.4,
then we have to do it this way. Splitting up the LOCKED flag into 3 flags is
almost what I've done. I've intro'd a new flag for locking out changes to
disposition, and an option to set that flag, basically. I could do it for
end, but I'll wait till someone actually needs it before I'll add it. For
setvar, we have enough flags to cover most situations. the 'l' flag
will force a var set to be done only on the last CDR in the chain.
This is basically the CDR created by a previous call to ForkCDR().
So, you can get what you need that way.

So, given these restrictions, I ask again, will the proposed changes give you
what you need?
By: Dmytro Mishchenko (arkadia) 2008-06-06 02:40:58

>Sorry, but all hope of an API consistency argument is gone. The API was never
> documented, so for years, everyone made their own mental API based on its
> behavior, which forms an 'implicit' documentation. And that unpublished, but
> ubiquitous mental document based on experimentation, trial, and error is the
> standard. There is no way you can change the behavior of these funcs and get
> it merged into the code. Any changes you would like to make to solve your
> problem must not disturb the current behavior. So, the API must stay as it >is.

Okay, that may be the way to go too. If everyone agreed on that we keep AST_CDR_FLAG_LOCKED where is was originally.
Now can we introduce NEW flag AST_CDR_FLAG_LOCKEDFULL which will be used together with AST_CDR_FLAG_LOCKED, everywhere where AST_CDR_FLAG_LOCKED is used and additionally in ast_cdr_answer, ast_cdr_setvar and ast_cdr_end?

Thats the whole changes for cdr.c I'm talking about. I suggest we invite somebody to comment on these changes.


>> AST_CDR_FLAG_LOCKED is a part of public API used by developers in their
>> applications. Preferably this flag has constant and clear meaning like:
>> if CDR is marked with this flag it is not updated by any ast_cdr_....
>> functions.

>Sorry, but this is wrong. AST_CDR_FLAG_LOCKED is NOT part of a public API.
>It's only minimally documented in the source code.

I have another point of view: ast_cdr_... functions - are not static functions and are available for application developers. AST_CDR_FLAG_... comes together with these functions. Thats why I'm saying it IS a public API. Luck of documentation doesn't mean it's not an API.



>Also, implementing new funcs and apps are definitely out. First of all, they
> complicate the picture, by adding new but similar apps to the existing >interface. Users will be confused about which to use in new implementations.
> Next, new apps are an enhancement, and are not on the table for discussion >for 1.4; If you want new stuff in trunk, we can talk, but if you want this in >1.4, then we have to do it this way.

I'm not interested in any applications changes in that bug discussion. I'm talking about applications only to show the way how ast_cdr_... API can be used. Lets finish with cdr.c changes first. All applications can be discussed separately in new bug reports.

>So, given these restrictions, I ask again, will the proposed changes give you
>what you need?

can you show me changes you propose to do on cdr.c. It may be a dirty draft, not a tested patch if we want to save time.
By: Steve Murphy (murf) 2008-06-06 08:44:54

My proposed changes, in the rough, are in the attachment "10668.proposed.patch"
By: Dmytro Mishchenko (arkadia) 2008-06-06 10:41:01

On your patch:
ast_cdr_answer, ast_cdr_busy, ast_cdr_failed - are ok.

ast_cdr_noanswer - I don't get why original AST_CDR_FLAG_LOCKED was replaced with AST_CDR_FLAG_ANSLOCKED. I assume its a typo.

Can you add the same construction to:  ast_cdr_setvar and ast_cdr_end?
By: Steve Murphy (murf) 2008-06-06 14:26:13

As far as I can tell, the change I made to ast_cdr_noanswer was just the insertion of the two lines. It, like ast_cdr_busy(), etc, did not previously obey any kind of LOCKED flag.

As to locking end time, do you see this as a viable and useful thing? CDR's with no end times are pretty useless, and will generate warnings. You'll need some way to remove the lock before the channel hangs up... very messy. No, let's leave that one be.

And, setvar has opts last and recur, so it has 3 ways to set chained cdrs:
 1. just the cdr in question (no opt)(no respect for locked or unlocked).
 2. just the last cdr in the chain. (l option)
 3. All CDR's in the chain (r option) (whether locked or not).

So, are you proposing/wishing this:
    Add 'u' option to CDR() func, to set !LOCKED cdr's only? (useless
    with 'l' option, as last CDR in chain will never be LOCKED; with 'r'
    option, it will only set unlocked CDR's; with no other opts, will
    only set the current CDR if it is not locked). And, for CDR reading,
    'u' will return the value from the first unlocked CDR.

Is this really necessary, tho? Rather than add a lot of opts with subtle affects on the outcome, will the r/l opts be sufficient? Why not?
By: Dmytro Mishchenko (arkadia) 2008-06-06 15:33:08

>As far as I can tell, the change I made to ast_cdr_noanswer was just the >insertion of the two lines. It, like ast_cdr_busy(), etc, did not previously >obey any kind of LOCKED flag.

I was applying your patch to the trunk and it fails on last chunks. That why I though it was removed. Great that we have a general CDR lock here.

>And, setvar has opts last and recur, so it has 3 ways to set chained cdrs:
>   1. just the cdr in question (no opt)(no respect for locked or unlocked).
>   2. just the last cdr in the chain. (l option)

that is a func_cdr.c option

>   3. All CDR's in the chain (r option) (whether locked or not).

According code ast_cdr_setvar() by itself has only two ways of processing CDRs:
1. all cdrs (recursive)
2. first cdr in chain
Can we add possibilities to set variable only on non locked CDRs to this function too?


As for your proposition for adding 'u' to CDR() it looks okay to me.


By: Steve Murphy (murf) 2008-06-06 18:16:45

OK, that explains it; the patch was against 1.4, as that's the version
you reported in the bug report.

I apologize for my previous inaccuracy; yes, setvar has only the recur
option, and two modes of operation. But the only way to get at setvar from
the dialplan is to use the CDR func, and that provides the (l for last) 3rd
option. It just advances the cdr ptr to the last in the chain before calling
the setvar func.

Now, don't get me wrong. I am not proposing the 'u' option. I was
asking if you are proposing it. If you are, you need to explain to
me why you'd need this. I'd rather not include it.

As to changing the setvar func to make it so it doesn't touch LOCKED
cdrs, well, my reaction is no. The way ForkCDR is constructed,
all CDR's but the last in a chain will be locked. Thus, you get
this behavior without anything needing to be changed, if you
use the CDR() func with the 'l' option (small-L). (after I commit
the change to make CDR honor the 'l' in writing).
If you can show me a concrete need that is not otherwise
filled, I would like to leave setvar/CDR() as they are,
with only the 'l' flag fixed/honored on write. I'm not trying
to be contrary, it's just that this is released software,
and people are using it. I'm trying to be as absolutely
minimal as possible when it comes to changes.
By: Steve Murphy (murf) 2008-06-09 13:38:26

OK, I take it back. ForkCDR() is *not* the only place in the code that
chains CDRs. There is some going on in the chan_sip and chan_zap channel drivers.
I'm working on CDR bugs in 1.4, and trunk, and 1.6.0 that will remove these
cdr_append calls, at least in chan_zap.c; I may do the same in chan_sip.c.
Until then, I've added the 's' option (I didn't notice previously that the
CDR() func used the 'u' option already for raw time formatting.)

So, I've attached a new diff patch that includes the code nec. to
add and obey the 's' (skip LOCKED cdr's) option, in the CDR() func.
By: Dmytro Mishchenko (arkadia) 2008-06-09 15:05:32

> As to changing the setvar func to make it so it doesn't touch LOCKED
> cdrs, well, my reaction is no. The way ForkCDR is constructed,
> all CDR's but the last in a chain will be locked. Thus, you get
> this behavior without anything needing to be changed, if you
> use the CDR() func with the 'l' option (small-L). (after I commit
> the change to make CDR honor the 'l' in writing).

I'm not using CDR() at all. I'm using ast_cdr_setvar() directly from my applications.
Thats why I'm talking about adding LOCK to this function. Otherwise the code which meant to be there will be in user applications everywhere you need to set variable. I see you've got this point adding:

if (dont_touch_locked && ast_test_flag(cdr, AST_CDR_FLAG_LOCKED))
continue;

I prefer we use same kind of construction all over cdr.c like:

if (ast_test_flag(cdr, AST_CDR_FLAG_DONT_TOUCH_LOCKED) && ast_test_flag(cdr, AST_CDR_FLAG_LOCKED))
continue;

without introducing extra AST_CDR_FLAG_ANSLOCKED.

If you put this code in ast_cdr_answer, ast_cdr_setvar and ast_cdr_end I'll be happy. Nothing else needs to be done in cdr.c.
In this case ForkCDR and CDR() will work as before without any necessity to adjust them.
By: Steve Murphy (murf) 2008-06-09 17:09:46

Arkadia--

This is the first time you've explicitly said that you are calling the
ast_cdr_xxx() funcs from an app directly! Well, that's a different affair.

For you, then, I have added the "dont_touch_locked" int param to:
ast_cdr_end
ast_cdr_answer
ast_cdr_failed
ast_cdr_busy
ast_cdr_noanswer

All internal calls to these funcs set the new param to 0.
Each uses this param to decide whether to skip the CDR in question.
All other functionality stays the same. No existing dialplan code should be
affected by this.
However, anyone who's written an app that calls any of the above funcs,
will now have to modify the calls to these apps. If they add ",0" to their
calls, they should get exactly the same functionality.
I do not expect anyone to be affected, but if I am wrong, I hope they
will find the fix easy to make. Internal interfaces are likely to change
even in released code to allow bug fixing. In general, the CDR API is
internal to asterisk, and subject to change, even in released code,
in response to bug fix demands. Sorry!
Arkadia-- please note that even this code is not sacred. It's interface may
change in the future, in response to bugfix and security demands.

Please see if the attached ...proposed_patch3 meets your needs. Setvar
already has the dont_touch param.  The CDR() func and ForkCDR() app
will continue to have the new options, as you are not the only person
asking for at least some of these changes.
By: Dmytro Mishchenko (arkadia) 2008-06-10 04:21:06

Why do you prefer to add separate function argument "dont_touch_locked" and update a lot of code
instead of introducting cdr flag AST_CDR_FLAG_DONT_TOUCH_LOCKED and leave the API untouched?

>However, anyone who's written an app that calls any of the above funcs,
>will now have to modify the calls to these apps. If they add ",0" to their
>calls, they should get exactly the same functionality.

Why do you want to force developers to modify their code just to keep it working in the way it was working before? Why not to make their life more simple and don't change functions parameters, especially when it can be done easily as in this case.

>This is the first time you've explicitly said that you are calling the
> ast_cdr_xxx() funcs from an app directly! Well, that's a different affair.

I'm saying from the begging this bug report is about ast_cdr_... API only and cdr.c changes.
By: Steve Murphy (murf) 2008-06-10 14:25:48

Arkadia--

I like the idea of not affecting the function interfaces. I uploaded the patch4
file. the function interfaces have been returned to previous, and they now
rely on the cdr struct containing a DONT_TOUCH flag instead.

The DONT_TOUCH can be set via a new option to forkCDR.

> I'm saying from the beg[inn]ing this bug report is about ast_cdr_... API only and cdr.c changes.

Sorry if I've been a bit dense, but you **have** expressed interest in dialplan
app changes:

>> "app_forkcdr.c should be updated to end original locked CDR.
>> Also it'll be great to make optional cdr reset in this application,
>> but thats a theme for another patch."

as well as all the discussion over forkCDR and CDR()...

But, the app/func work I've been pushing is not just for you. I've gotten
phone calls, email, and have been involved in IRC discussions on these
sort of issues for about a year. And while you may only peripherally care
about the dialplan app/func changes, they will be key to folks who are having
problems in developing billing apps with the current restrictions of the
CDR system.

Also, I'd prefer to have users developing apps, use the dialplan
app/func interface, rather than coding in C in the source code. Your app
will be much more upwardly compatible and portable than keeping your own
patches up to date against a shifting code tree. Just the fact that you
are mucking around with the ast_cdr_xxx funcs tells me that the dialplan
interface is not sufficient for you. But you can't say I didn't try!

What do you think of the patch4 stuff? Is it OK to commit it?
By: Dmytro Mishchenko (arkadia) 2008-06-11 15:59:08

ast_cdr_busy(), ast_cdr_failed(), ast_cdr_noanswer() - AST_CDR_FLAG_LOCKED was already handled in these functions. In  10668.proposed.patch4 you are adding more checks of this flag.


+ if (ast_test_flag(cdr, AST_CDR_FLAG_ANSLOCKED))
+ continue;
+ if (ast_test_flag(cdr, AST_CDR_FLAG_DONT_TOUCH) && ast_test_flag(cdr, AST_CDR_FLAG_LOCKED))
+ continue;

This looks a little bit funny. Are you sure you need AST_CDR_FLAG_ANSLOCKED?
Can't we do the same with just AST_CDR_FLAG_DONT_TOUCH and AST_CDR_FLAG_LOCKED?


>The DONT_TOUCH can be set via a new option to forkCDR.
Great. In ForkCDR() description it'll be nice to recommendation to use this flag together with 'e'.
By: Steve Murphy (murf) 2008-06-11 17:08:30

As to ANS_LOCKED, it's for the guys who just want to shut down having the
various call disposition updates affect the CDR. They don't want to keep
end from getting updated. So, it's a more specific sort of goal
than the shotgun 'T' flag. It's for the folks who want to freeze just
the answer times without changing the way the end times are handled
normally.

I added your recommendation to the doc strings, about using the 'e' option
with 'T'.

I'll assume then, that we have come to closure on this; I will check this
bug again in a few hours, and if no further objections, I will close it
by committing the fixes to 1.4, trunk, and 1.6.0...
By: Digium Subversion (svnbot) 2008-06-12 08:40:54

Repository: asterisk
Revision: 122046

U   branches/1.4/CHANGES
U   branches/1.4/apps/app_forkcdr.c
U   branches/1.4/funcs/func_cdr.c
U   branches/1.4/include/asterisk/cdr.h
U   branches/1.4/main/cdr.c

------------------------------------------------------------------------
r122046 | murf | 2008-06-12 08:40:47 -0500 (Thu, 12 Jun 2008) | 37 lines

(closes issue ASTERISK-10252)
Reported by: arkadia
Tested by: murf, arkadia

Options added to forkCDR() app and the CDR() func to
remove some roadblocks for CDR applications.

The "show application ForkCDR" output was upgraded
to more fully explain the inner workings of forkCDR.

The A option was added to forkCDR to force the
CDR system to NOT change the disposition on the
original CDR, after the fork. This involves
ast_cdr_answer, _busy, _failed, and so on.

The T option was added to forkCDR to force
obedience of the cdr LOCKED flag in the
ast_cdr_end, all the disposition changing
funcs (ast_cdr_answer, etc), and in the
ast_cdr_setvar func.

The CHANGES file was updated to explain ALL
the new options added to satisfy this bug report
(and some requests made verbally and via
email, irc, etc, over the past months/year)

The 's' option was added to the CDR() func,
to force it to skip LOCKED cdr's in the
chain.

Again, the new options should be totally transparent
to existing apps! Current behavior of CDR,
forkCDR, and the rest of the CDR system should
not change one little bit. Until you add the
new options, at least!


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

http://svn.digium.com/view/asterisk?view=rev&revision=122046
By: Digium Subversion (svnbot) 2008-06-12 09:21:40

Repository: asterisk
Revision: 122091

_U  trunk/
U   trunk/CHANGES
U   trunk/apps/app_forkcdr.c
U   trunk/funcs/func_cdr.c
U   trunk/include/asterisk/cdr.h
U   trunk/main/cdr.c

------------------------------------------------------------------------
r122091 | murf | 2008-06-12 09:21:34 -0500 (Thu, 12 Jun 2008) | 45 lines

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

........
r122046 | murf | 2008-06-12 07:47:34 -0600 (Thu, 12 Jun 2008) | 37 lines

(closes issue ASTERISK-10252)
Reported by: arkadia
Tested by: murf, arkadia

Options added to forkCDR() app and the CDR() func to
remove some roadblocks for CDR applications.

The "show application ForkCDR" output was upgraded
to more fully explain the inner workings of forkCDR.

The A option was added to forkCDR to force the
CDR system to NOT change the disposition on the
original CDR, after the fork. This involves
ast_cdr_answer, _busy, _failed, and so on.

The T option was added to forkCDR to force
obedience of the cdr LOCKED flag in the
ast_cdr_end, all the disposition changing
funcs (ast_cdr_answer, etc), and in the
ast_cdr_setvar func.

The CHANGES file was updated to explain ALL
the new options added to satisfy this bug report
(and some requests made verbally and via
email, irc, etc, over the past months/year)

The 's' option was added to the CDR() func,
to force it to skip LOCKED cdr's in the
chain.

Again, the new options should be totally transparent
to existing apps! Current behavior of CDR,
forkCDR, and the rest of the CDR system should
not change one little bit. Until you add the
new options, at least!


........

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

http://svn.digium.com/view/asterisk?view=rev&revision=122091
By: Digium Subversion (svnbot) 2008-06-12 09:33:21

Repository: asterisk
Revision: 122126

_U  branches/1.6.0/
U   branches/1.6.0/CHANGES
U   branches/1.6.0/apps/app_forkcdr.c
U   branches/1.6.0/funcs/func_cdr.c
U   branches/1.6.0/include/asterisk/cdr.h
U   branches/1.6.0/main/cdr.c

------------------------------------------------------------------------
r122126 | murf | 2008-06-12 09:33:09 -0500 (Thu, 12 Jun 2008) | 53 lines

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

................
r122091 | murf | 2008-06-12 08:28:01 -0600 (Thu, 12 Jun 2008) | 45 lines

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

........
r122046 | murf | 2008-06-12 07:47:34 -0600 (Thu, 12 Jun 2008) | 37 lines

(closes issue ASTERISK-10252)
Reported by: arkadia
Tested by: murf, arkadia

Options added to forkCDR() app and the CDR() func to
remove some roadblocks for CDR applications.

The "show application ForkCDR" output was upgraded
to more fully explain the inner workings of forkCDR.

The A option was added to forkCDR to force the
CDR system to NOT change the disposition on the
original CDR, after the fork. This involves
ast_cdr_answer, _busy, _failed, and so on.

The T option was added to forkCDR to force
obedience of the cdr LOCKED flag in the
ast_cdr_end, all the disposition changing
funcs (ast_cdr_answer, etc), and in the
ast_cdr_setvar func.

The CHANGES file was updated to explain ALL
the new options added to satisfy this bug report
(and some requests made verbally and via
email, irc, etc, over the past months/year)

The 's' option was added to the CDR() func,
to force it to skip LOCKED cdr's in the
chain.

Again, the new options should be totally transparent
to existing apps! Current behavior of CDR,
forkCDR, and the rest of the CDR system should
not change one little bit. Until you add the
new options, at least!


........

................

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

http://svn.digium.com/view/asterisk?view=rev&revision=122126
By: Digium Subversion (svnbot) 2008-06-13 11:39:06

Repository: asterisk
Revision: 122590

_U  team/murf/CDRfix4/
U   team/murf/CDRfix4/CHANGES
U   team/murf/CDRfix4/acinclude.m4
U   team/murf/CDRfix4/apps/app_chanspy.c
A   team/murf/CDRfix4/apps/app_dahdibarge.c
A   team/murf/CDRfix4/apps/app_dahdiras.c
A   team/murf/CDRfix4/apps/app_dahdiscan.c
U   team/murf/CDRfix4/apps/app_dial.c
U   team/murf/CDRfix4/apps/app_disa.c
U   team/murf/CDRfix4/apps/app_flash.c
U   team/murf/CDRfix4/apps/app_forkcdr.c
U   team/murf/CDRfix4/apps/app_getcpeid.c
U   team/murf/CDRfix4/apps/app_meetme.c
U   team/murf/CDRfix4/apps/app_page.c
U   team/murf/CDRfix4/apps/app_parkandannounce.c
U   team/murf/CDRfix4/apps/app_read.c
U   team/murf/CDRfix4/apps/app_rpt.c
D   team/murf/CDRfix4/apps/app_zapbarge.c
D   team/murf/CDRfix4/apps/app_zapras.c
D   team/murf/CDRfix4/apps/app_zapscan.c
U   team/murf/CDRfix4/build_tools/menuselect-deps.in
A   team/murf/CDRfix4/channels/chan_dahdi.c
U   team/murf/CDRfix4/channels/chan_iax2.c
U   team/murf/CDRfix4/channels/chan_mgcp.c
U   team/murf/CDRfix4/channels/chan_misdn.c
D   team/murf/CDRfix4/channels/chan_zap.c
A   team/murf/CDRfix4/codecs/codec_dahdi.c
D   team/murf/CDRfix4/codecs/codec_zap.c
U   team/murf/CDRfix4/configure
U   team/murf/CDRfix4/configure.ac
U   team/murf/CDRfix4/contrib/utils/zones2indications.c
U   team/murf/CDRfix4/funcs/func_cdr.c
U   team/murf/CDRfix4/funcs/func_channel.c
U   team/murf/CDRfix4/include/asterisk/autoconfig.h.in
U   team/murf/CDRfix4/include/asterisk/cdr.h
U   team/murf/CDRfix4/include/asterisk/channel.h
A   team/murf/CDRfix4/include/asterisk/dahdi_compat.h
U   team/murf/CDRfix4/include/asterisk/indications.h
U   team/murf/CDRfix4/include/asterisk/options.h
U   team/murf/CDRfix4/main/app.c
U   team/murf/CDRfix4/main/asterisk.c
U   team/murf/CDRfix4/main/cdr.c
U   team/murf/CDRfix4/main/channel.c
U   team/murf/CDRfix4/main/file.c
U   team/murf/CDRfix4/main/indications.c
U   team/murf/CDRfix4/main/loader.c
U   team/murf/CDRfix4/makeopts.in
U   team/murf/CDRfix4/pbx/pbx_config.c
U   team/murf/CDRfix4/res/res_features.c
U   team/murf/CDRfix4/res/res_indications.c
U   team/murf/CDRfix4/res/res_musiconhold.c
U   team/murf/CDRfix4/res/snmp/agent.c

------------------------------------------------------------------------
r122590 | murf | 2008-06-13 11:39:02 -0500 (Fri, 13 Jun 2008) | 108 lines

Merged revisions 122046,122127,122130,122137,122208,122259,122311,122314,122589 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r122046 | murf | 2008-06-12 07:47:34 -0600 (Thu, 12 Jun 2008) | 37 lines

(closes issue ASTERISK-10252)
Reported by: arkadia
Tested by: murf, arkadia

Options added to forkCDR() app and the CDR() func to
remove some roadblocks for CDR applications.

The "show application ForkCDR" output was upgraded
to more fully explain the inner workings of forkCDR.

The A option was added to forkCDR to force the
CDR system to NOT change the disposition on the
original CDR, after the fork. This involves
ast_cdr_answer, _busy, _failed, and so on.

The T option was added to forkCDR to force
obedience of the cdr LOCKED flag in the
ast_cdr_end, all the disposition changing
funcs (ast_cdr_answer, etc), and in the
ast_cdr_setvar func.

The CHANGES file was updated to explain ALL
the new options added to satisfy this bug report
(and some requests made verbally and via
email, irc, etc, over the past months/year)

The 's' option was added to the CDR() func,
to force it to skip LOCKED cdr's in the
chain.

Again, the new options should be totally transparent
to existing apps! Current behavior of CDR,
forkCDR, and the rest of the CDR system should
not change one little bit. Until you add the
new options, at least!


........
r122127 | murf | 2008-06-12 08:51:44 -0600 (Thu, 12 Jun 2008) | 1 line

Arkadia tried to warn me, but the code added to ast_cdr_busy, _failed, and _noanswer was redundant. Didn't spot it until I was resolving conflicts in trunk. Ugh. Redundant code removed. It wasn't harmful. Just dumb.
........
r122130 | tilghman | 2008-06-12 09:11:30 -0600 (Thu, 12 Jun 2008) | 4 lines

Occasionally, the alertpipe loses its nonblocking status, so detect and correct
that situation before it causes a deadlock.  (Reported and tested by ctooley
via #asterisk-dev)

........
r122137 | tilghman | 2008-06-12 09:18:39 -0600 (Thu, 12 Jun 2008) | 8 lines

Flipflop the sections for two options, since the section for 'X' (exit context)
may otherwise absorb keypresses meant for 's' (admin/user menu).
(closes issue ASTERISK-12175)
Reported by: blitzrage
Patches:
      20080611__bug12836.diff.txt uploaded by Corydon76 (license 14)
Tested by: blitzrage

........
r122208 | jpeeler | 2008-06-12 09:46:08 -0600 (Thu, 12 Jun 2008) | 5 lines

(closes issue ASTERISK-11622)
Reported by: davidw
Patch by: Corydon76, modified by me to work properly with ParkAndAnnounce app


........
r122259 | russell | 2008-06-12 12:22:44 -0600 (Thu, 12 Jun 2008) | 3 lines

Fix some race conditions that cause ast_assert() to report that chan_iax2 tried
to remove an entry that wasn't in the scheduler

........
r122311 | mmichelson | 2008-06-12 12:50:58 -0600 (Thu, 12 Jun 2008) | 9 lines

Properly play a holdtime message if the announce-holdtime option is
set to "once."

(closes issue ASTERISK-12180)
Reported by: ramonpeek
Patches:
     patch001.diff uploaded by ramonpeek (license 266)


........
r122314 | jpeeler | 2008-06-12 13:08:20 -0600 (Thu, 12 Jun 2008) | 2 lines

Adds DAHDI support alongside Zaptel. DAHDI usage favored, but all Zap stuff should continue working. Release announcement to follow.

........
r122589 | twilson | 2008-06-13 10:29:07 -0600 (Fri, 13 Jun 2008) | 7 lines

This should fix the behavior of the 'T' dial feature being passed incorrectly to the transferee when builtin_atxfers are used.
Also, doing a builtin_atxfer to parking was broken and is fixed here as well.

(closes issue ASTERISK-11352)
Reported by: sergee
Tested by: otherwiseguy

........

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

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