[Home]

Summary:ASTERISK-05973: [patch] fix inaccurate naming of header containing CallerID Number
Reporter:Russell Bryant (russell)Labels:
Date Opened:2006-01-04 00:49:22.000-0600Date Closed:2006-04-11 18:23:49
Priority:TrivialRegression?No
Status:Closed/CompleteComponents:Core/ManagerInterface
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) bug20060411__bug6130.diff.txt
( 1) manager_callerid.patch
Description:There was recently a patch committed from ASTERISK-5088 that added the "CallerID" header to the OriginateSuccess/OriginateFailure events.  However, I was unhappy with how "CallerID" was used here.

The "Status" events that are sent after the 'status' action is executed to request the status of all channels already used the "CallerID:" header to contain only CallerID name.  However, the "CallerID:" header is used to represent the entire CallerID string in the originate action.  I think it makes the most sense to reserve "CallerID:" to be used for the entire CallerID string so that headers have consistent meaning.

This patch changes the use of "CallerID:" to "CallerIDNum:" in the places where it is only used to contain the CallerID number.  I have also added information to UPGRADE.txt that contains the information on changes from 1.2.
Comments:By: Olle Johansson (oej) 2006-01-04 02:15:16.000-0600

...and I think cleaning up manager is a good thing (TM). But if we change the syntax, we need to change the version number of the manager interface to alert software connecting to Asterisk about which version they are talking to.

We need a policy for doing that. One way is that manager version numbers follows Asterisk version numbers and we can change freely between versions.
Another solution is to use the manager version for version of the protocol.

There's a lot of cleaning up to do, that will harm current clients.

Using the later method, we could set manager to version 2.0 in Asterisk 1.4 and clean it up heavily. Or create a new module that implements manager 2.0 and clean that up, while keeping manager 1 as is. The problem with that solution is the events generated from various modules, that need a new syntax or two calls to manager for each event....

What do you think?



By: Russell Bryant (russell) 2006-01-04 22:38:57.000-0600

I do not think this change is large enough warrant changing the manager protocol version number.  This is an inconsistency where "CallerID:" is used in more than one way.  I would even consider this a bug.

I see it kind of like IAX and IAX2.  We add new information elements and change information elements in IAX2, but that does not mean it is a new protocol.

Until extremely drastic changes are proposed to the manager protocol, I do not think changing the version number is necessary.

By: Olle Johansson (oej) 2006-01-05 02:20:56.000-0600

But still, a change will affect existing implementations. I think they need a way to figure this out to be compatible with several versions of Asterisk. So if this is not a manager 2.0, is it a 1.1 version?

When do we change the version and why? I think we should change the version number when we make changes to the current protocol and this is a change, even if it is a bug fix.

By: Stefan Reuter (srt) 2006-01-18 20:10:31.000-0600

The first part of this patch wouldnt break existing clients as the patch that added the CallerId has not yet been included in any release. As I understand the history of patch 5223 it was commited to trunk on 01-03-06, thats just a few weeks.

Regarding the Status event we might consider adding the CallerIDNum property and deprecating CallerID. Then it could be removed in 1.6.

By: Tilghman Lesher (tilghman) 2006-04-11 18:14:50

Here's an alternative approach:  leave the old parameter and duplicate it in the same event with the new name.  This should allow a transition period during the 1.4 release cycle to properly move up to the new API.

By: Russell Bryant (russell) 2006-04-11 18:19:30

That's fine with me.  I guess we would also have to update UPGRADE.txt as well.

I appologize that I've let this sit around so long.  Feel free to commit this change or assign it to me and I'll do it when I can.

By: Tilghman Lesher (tilghman) 2006-04-11 18:23:49

Committed to trunk