[asterisk-dev] [Code Review] 3040: bridge transfers: Make sure ATTENDEDTRANSFER variable gets set for all the expected channels when doing bridge attended transfers. Also make sure BLINDTRANSFER is set when doing externally intiated transfers (such as SIP or PJSIP transfers) in a three or more party call.

[Jonathan Rose]

> On Dec. 9, 2013, 11 p.m., rmudgett wrote:
> > /branches/12/include/asterisk/bridge.h, lines 965-968
> > <https://reviewboard.asterisk.org/r/3040/diff/2/?file=48994#file48994line965>
> >
> >     Suggest changing the attended parameter name to imply that it is a boolean value: is_attended.

change'd


> On Dec. 9, 2013, 11 p.m., rmudgett wrote:
> > /branches/12/main/bridge.c, lines 125-129
> > <https://reviewboard.asterisk.org/r/3040/diff/2/?file=48995#file48995line125>
> >
> >     Word wrap the long comment line or shorten the wordy sentence.

/* Variable name - stores peer information about the most recent attended transfer */

Shorten'd.


> On Dec. 9, 2013, 11 p.m., rmudgett wrote:
> > /branches/12/main/bridge.c, lines 3960-3962
> > <https://reviewboard.asterisk.org/r/3040/diff/2/?file=48995#file48995line3960>
> >
> >     A blank line should be added between variable declarations and code as a separator.

Blank'd


> On Dec. 9, 2013, 11 p.m., rmudgett wrote:
> > /branches/12/main/bridge.c, lines 3985-3988
> > <https://reviewboard.asterisk.org/r/3040/diff/2/?file=48995#file48995line3985>
> >
> >     Suggest changing the attended parameter name to imply that it is a boolean value: is_attended.

change'd 2: change harder


> On Dec. 9, 2013, 11 p.m., rmudgett wrote:
> > /branches/12/main/bridge.c, lines 4200-4202
> > <https://reviewboard.asterisk.org/r/3040/diff/2/?file=48995#file48995line4200>
> >
> >     Use ast_bridge_peers() rather than inlining it.
> >     
> >     Actually, you don't need to lock the bridge at all because the only caller already has it locked.

Turns out the already locked requirement is already in the documentation too.


> On Dec. 9, 2013, 11 p.m., rmudgett wrote:
> > /branches/12/main/bridge_basic.c, lines 2932-2933
> > <https://reviewboard.asterisk.org/r/3040/diff/2/?file=48996#file48996line2932>
> >
> >     The comment no longer makes sence here.

Uncommentary'd


- Jonathan


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


On Dec. 4, 2013, 6:11 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3040/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2013, 6:11 p.m.)
> 
> 
> Review request for Asterisk Developers, Mark Michelson and rmudgett.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> There were still a few cases in which ATTENDEDTRANSFER and BLINDTRANSFER wouldn't be set on channels involved with blind and attended transfers. This would happen with features that were initialized by channel driver specific mechanisms in multiparty calls. This patch resolves those cases while attempted to keep the behavior for setting those variables as consistent as possible.
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/parking/parking_manager.c 403068 
>   /branches/12/res/parking/parking_bridge_features.c 403068 
>   /branches/12/main/bridge_basic.c 403068 
>   /branches/12/main/bridge.c 403068 
>   /branches/12/include/asterisk/bridge.h 403068 
> 
> Diff: https://reviewboard.asterisk.org/r/3040/diff/
> 
> 
> Testing
> -------
> 
> Test One - single peer tansfers
> 1. Started a two way call between two chan_sip channels.
> 2. SIP atxfer'd it to an extension that would display the ATTENDEDTRANSFER and BLINDTRANSFER variables
> 3. repeated test with SIP blindxfer and compared the results.
> 
> Without patch:
> Blind transfer variable would be present on blind transfer, attended transfer variable would be present on attended transfer.
> 
> With patch:
> No behavioral change
> 
> Test Two - transferring three party calls
> 1. Started a two way call between two chan_sip channels.
> 2. Feature ATXfer'd to a local channel that would screech something about weasels every few seconds. Merged into a 3 way call with *3.
> 3. SIP atxfer'd the bridge to an extension that would display ATTENDEDTRANSFER and BLINDTRANSFER variables
> 3. repeated test with SIP blindxfer and compared the results.
> 
> Without patch:
> Attended transfer variables weren't set at all.
> Blind transfer variables would be set on the channels that were already in the bridge, but not on the local channel that was used to transfer.
> 
> With patch:
> Attended transfer variable set for all channels in the bridge as well as the local channel used for the transfer.
> Blind transer variables set for all channels in the bridge as well as the local channel used for the transfer.
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

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