Summary:ASTERISK-07096: [patch] ANI not set with callerid= in some channel drivers
Reporter:gkloepfer (gkloepfer)Labels:
Date Opened:2006-06-05 10:38:28Date Closed:2006-07-02 23:25:36
Versions:Frequency of
Environment:Attachments:( 0) cid-ani.patch
( 1) patch_trunk_34852
Description:There is an inconsistency where the ANI is set by the callerid= configuration option for some channel drivers (Zap/, for example) and not others (SIP).  This inconsistency affects CDR because the CDR prefers ANI over CALLERID(num) as the calling extension, so in some cases the CDR will list the Caller-ID as modified by the Asterisk dialplan, and in other cases what is specified in the "callerid=" configuration option for the channel.

This can be seen in the dialplan by displaying the contents of CALLERID(num) and CALLERID(ANI) using both a Zap/ channel (from a channel bank) and a SIP/ channel with callerid= in the {channel}.conf files set.  In the Zap/ case the ANI will be assigned, while in the SIP/ case, the ANI will be null.


This was discussed (briefly) on the Asterisk-Users mailing list and Kevin Fleming requested that I report it as a bug.

It appears that the Zap/ driver was improved some time ago and a function ast_set_callerid and some logic was defined to elegantly set the various variables pertaining to Caller-ID (name, number, and ANI) within the channel driver.  This improvement doesn't appear to have been carried-over to the other channel drivers (SIP, etc.).

I believe I can fix the issue by making all channel drivers set the Caller-ID variables in a consistent manner to the way chan_zap.c does, but it may take a few days for me to get a proposed patch together and tested.  Please be patient.
Comments:By: gkloepfer (gkloepfer) 2006-06-05 15:06:13

I just attached a proposed patch.  The patch addresses this issue in the following channel interfaces:  chan_agent, chan_features, chan_h323, chan_iax2, chan_local, chan_mgcp, chan_misdn, chan_modem, chan_phone, chan_sip, and chan_skinny.

IMPORTANT:  The only channel interfaces I could easily test are chan_local and chan_sip.  The others I believe to be correct.

Also, note that the ast_set_callerid() channel function has the side effect of potientially generating a manager event whenever the function is called.  In all the locations I changed the code, there was not a manager event generated when the callerid number and ANI were changed.  So while this patch may be operationally correct, it may need to be changed if the additional manager events are inappropriate.

Feedback appreciated.

By: Serge Vecher (serge-v) 2006-06-12 19:21:05

gkloepfer: I think this is more of a trunk patch than 1.2, since this looks more of an enhancement than a bug fix. Ok if I mark this as trunk?

By: gkloepfer (gkloepfer) 2006-06-12 19:51:11

I believe this is actually a bugfix, although I can understand why it could also be considered an enhancement.

Basically what I'm reporting is that the behavior of the callerid= option in zapata.conf for Zap channels functions inconsistently from the callerid= option in other channel drivers.

Whether this is an enhancement or bugfix I think would depend on what the expected behavior of those options would be with respect to ANI and CDR.  If the expected behavior is for ANI to default to null until it is explicitly set by Set(CALLERID(ANI)=<something>) then the fix I'm recommending is an enhancement and the bug is in chan_zap.c.  If the expectation is that the ANI defaults to what is set by the callerid= options in the {channel}.conf files, then this is probably a bugfix.

The impact on the CDR is that the CDR prefers recording the ANI as the originating number, and secondarily the Caller ID (if the ANI doesn't exist).  If different channels operate inconsistently, then the CDR output is unpredictable unless the ANI is explicitly set for every call being originated.

For what it's worth, I'm using the the patch I submitted in a production Asterisk environment and so far things are working as expected.

I'm open to discussion on this - unfortunately I didn't get much of a response on the list.

By: Serge Vecher (serge-v) 2006-06-12 19:55:37

russell: need your ruling here.

By: Russell Bryant (russell) 2006-06-18 16:56:18

These changes are acceptable for 1.2.  The attached patch still applies cleanly to the 1.2 branch, but we will need an additional equivalent patch for the trunk.


By: Serge Vecher (serge-v) 2006-06-19 10:56:40

gkloepfer: please provide a separate patch for the latest trunk so that this can be merged it. Thanks!

By: gkloepfer (gkloepfer) 2006-06-19 13:21:54

Patch for trunk version 34852 has been uploaded to the attached files.  Note that I am not able to easily test this in our environment (I don't have a spare system to just run trunk).  It compiles cleanly, and the logic looks right.  If someone can test it, that would be fantastic.

The easiest way to test this is to set the Caller ID for the device in the {whatever}.conf file (sip.conf is a good example).  Cause all outgoing calls from that device to go to a test context(context=test-cid-patch, for example):

exten => _X.,1,Answer
exten => _X.,n,NoOp(${CALLERID(num)} / ${CALLERID(name)} / ${CALLERID(ani)})
exten => _X.,n,Set(CALLERID(num)=123456)
exten => _X.,n,NoOp(${CALLERID(num)} / ${CALLERID(name)} / ${CALLERID(ani)})
exten => _X.,n,Hangup

If you watch the console output, the first NoOp should indicate the CallerID number in the first and last fields with the name in the middle.  In the second NoOp, the first field should be 123456, second should be the name yet, and the third will continue to be the original CallerID string.

If no callerid= is added to the {whatever}.conf for the device, then the first and last fields should be blank on the first NoOp, and the first and last fields should be 123456 on the second NoOp.

Hope this helps.

By: Russell Bryant (russell) 2006-07-02 23:25:35

added to the trunk and 1.2 branch, thanks!