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

[Scott Griepentrog]

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



/branches/12/main/bridge_channel.c
<https://reviewboard.asterisk.org/r/3601/#comment22141>

    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?


- Scott Griepentrog


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/f6839989/attachment-0001.html>