[asterisk-dev] [Code Review] 3601: accountcode: Slightly change accountcode propagation.

rmudgett reviewboard at asterisk.org
Tue Jun 10 17:41:13 CDT 2014 


> On June 10, 2014, 5:13 p.m., Scott Griepentrog wrote:
> > /branches/12/main/bridge_channel.c, lines 602-610
> > <https://reviewboard.asterisk.org/r/3601/diff/1/?file=59438#file59438line602>
> >
> >     I'm not understanding the need for this to be split out with separate join/leave handling.  It seems like an un-necessary complication.  Would it not work to use a method similar to the linkedid propagation and copy account codes from earliest channel, just never overwriting an existing code?

The processing is different because of the different conditions.  Joining channels have not been added to the bridge channel list yet and the optional leaving channel is being swapped with the joining channel.  The leaving case is needed when a bridge goes from a 3-party bridge to a 2-party bridge and the bridge may be marked as destroyed.  The leaving case was not handled before this patch so the peeraccount codes could be mismatched with the peer channel.

Accountcodes to not propagate like linkedids where the oldest linkedid gets set on all channels in the bridge.  The accountcode/peeraccount are more like caller-id/connected-line respectively.  When a channel is replaced with another channel, the peeraccount codes need to be updated so they match what's accross the bridge.

I could have replaced the ast_bridge_channel_update_accountcodes with a joining and leaving version.  However, that would change the API unnecessarily.


- rmudgett


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3601/#review12119
-----------------------------------------------------------


On June 6, 2014, 11 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3601/
> -----------------------------------------------------------
> 
> (Updated June 6, 2014, 11 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AFS-65 and ASTERISK-17954
>     https://issues.asterisk.org/jira/browse/AFS-65
>     https://issues.asterisk.org/jira/browse/ASTERISK-17954
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Accountcode propagation:
> 
> The current behavior is to simply set the accountcode of an outgoing
> channel to the accountcode of the channel initiating the call.  It was
> done this way a long time ago to allow the accountcode set on the SIP/100
> channel to be propagated to a local channel so the dialplan execution on
> the Local;2 channel would have the SIP/100 accountcode available.
> 
> SIP/100 -> Local;1/Local;2 -> SIP/200
> 
> Propagating the SIP/100 accountcode to the local channels is very useful.
> However, a side effect of this is it will overwrite any accountcode set by
> the SIP channel driver configuration for the SIP/200 channel with the
> accountcode from the Local;2 channel.  Without any dialplan manipulation,
> all channels in this call would have the same accountcode.  I believe this
> is the basis for the ASTERISK-17954 issue.
> 
> Using dialplan, you can set a different accountcode on the SIP/200 channel
> either by setting the accountcode on the Local;2 channel or by the Dial
> application's b(pre-dial), M(macro) or U(gosub) options, or by the
> FollowMe application's b(pre-dial) option, or by the Queue application's
> macro or gosub options.
> 
> This patch changes the behavior slightly.  The outgoing channel gets the
> accountcode of the initiating channel _only_ if the outgoing channel
> doesn't already have an accountcode set.
> 
> The most visible effect the change would have is if the channel driver
> automatically set the accountcode on channel creation.  In the example
> above if SIP/100 and SIP/200 are created with their own accountcode from
> the SIP channel driver configuration, the SIP/200 accountcode would not
> get overwritten by the SIP/100 accountcode and you would not need to use
> dialplan to set it back.  Of course the converse is that dialplan would
> need to be used to propagate the SIP/100 accountcode to SIP/200.
> 
> * Changed the accountcode propagation from simply setting the accountcode
> of the channel initiating the call onto the outgoing channel to setting
> the accountcode only if the outgoing channel doesn't already have one.
> In other words, if a channel already has an accountcode it will not be
> changed unless explicitly set by CHANNEL(accountcode) or an originate
> method that can specify an accountcode.  The most visible effect the
> change has is if the channel driver is configured with an accountcode
> for new channels.
> 
> * Moved most accountcode propagation behavior into ast_request() which
> eliminates several inconsistencies throughout the code.  (app_dial,
> app_followme, app_queue, and the dialing API)
> 
> * Fixed the basic bridge sub-class updating peeraccount codes when the
> number of channels in the bridge drops back down to two parties.
> 
> * Refactored ast_bridge_channel_update_accountcodes() to handle channels
> joining/leaving the bridge.
> 
> * Fixed the basic bridge to not call the base pull method twice.
> 
> * Fixed CEL extracting the wrong ie value for the peeraccount.
> 
> * Fixed some incorrect CEL event ie types.  These seem to only be used
> by the unit tests.
> 
> 
> Diffs
> -----
> 
>   /branches/12/main/pbx.c 415428 
>   /branches/12/main/event.c 415428 
>   /branches/12/main/dial.c 415428 
>   /branches/12/main/channel.c 415428 
>   /branches/12/main/cel.c 415428 
>   /branches/12/main/bridge_channel.c 415428 
>   /branches/12/main/bridge_basic.c 415428 
>   /branches/12/include/asterisk/bridge_channel.h 415428 
>   /branches/12/apps/app_queue.c 415428 
>   /branches/12/apps/app_followme.c 415428 
>   /branches/12/apps/app_dial.c 415428 
>   /branches/12/UPGRADE.txt 415428 
> 
> Diff: https://reviewboard.asterisk.org/r/3601/diff/
> 
> 
> Testing
> -------
> 
> Set the accountcode on the initial channel and let the channel driver supply or not the accountcode for the other channel.
> Without the channel driver supplying the accountcode, the outgoing channel got the same accountcode as the initial channel.
> With the channel driver supplying the accountcode, the outgoing channel kept its accountcode.
> 
> Let the channel driver supply the accountcode for all endpoint channel.
> Performed a DTMF attended transfer with consultation and creation of a three party bridge.
> When the transferrer channel left the three party bridge, the remaining two channels got the peeraccount updated to the other party.
> 
> Throughout each of these tests, the CEL log had the expected peeraccount value.  Note the peeraccount value is meaningless if the bridge contains more than two parties.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140610/1f895a4d/attachment-0001.html>

More information about the asterisk-dev mailing list