[asterisk-dev] [Code Review] 2845: Restore Dial, Queue, and FollowMe 'I' option support.

[Mark Michelson]

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



/branches/12/include/asterisk/bridge.h
<https://reviewboard.asterisk.org/r/2845/#comment18835>

    Defining AST_BRIDGE_IMPART_DEPARTABLE as 0 is potentially error-prone. As it is, the flag cannot be tested directly, since ast_test_flag(flags, AST_BRIDGE_IMPART_DEPARTABLE) will always return false.
    
    See my comment later in ast_bridge_impart().



/branches/12/main/bridge.c
<https://reviewboard.asterisk.org/r/2845/#comment18836>

    This is where the definition of AST_BRIDGE_IMPART_DEPARTABLE causes awkward code to be written. This would be much more plain as
    
    bridge_channel->depart_wait = (flags & AST_BRIDGE_IMPART_DEPARTABLE);
    
    but such a statement is not possible at the moment.
    
    The other option would be to create a mask for the DEPARTABLE/INDEPENDENT values and have
    
    bridge_channel->depart_wait = (flags & AST_BRIDGE_IMPART_MASK) == AST_BRIDGE_IMPART_DEPARTABLE;



/branches/12/main/channel.c
<https://reviewboard.asterisk.org/r/2845/#comment18838>

    I think this should be a debug rather than a verbose message.


- Mark Michelson


On Sept. 11, 2013, 9:31 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2845/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2013, 9:31 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22072
>     https://issues.asterisk.org/jira/browse/ASTERISK-22072
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The Dial, Queue, and FollowMe applications need to inhibit the bridging initial connected line exchange in order to support the 'I' option.
> 
> * Replaced the pass_reference flag on ast_bridge_join() with a flags parameter to pass other flags defined by enum ast_bridge_join_flags.
> 
> * Replaced the independent flag on ast_bridge_impart() with a flags parameter to pass other flags defined by enum ast_bridge_impart_flags.
> 
> * Since the Dial, Queue, and FollowMe applications are now the only callers of ast_bridge_call() and ast_bridge_call_with_flags(), changed the calling contract to require the initial COLP exchange to already have been done by the caller.
> 
> An independent, but associated change is:
> * Reduce stack usage in ast_indicate_data() and add a dropping redundant connected line verbose message.
> 
> 
> Diffs
> -----
> 
>   /branches/12/apps/app_agent_pool.c 398875 
>   /branches/12/apps/confbridge/conf_chan_announce.c 398875 
>   /branches/12/channels/chan_sip.c 398875 
>   /branches/12/include/asterisk/bridge.h 398875 
>   /branches/12/include/asterisk/bridge_channel.h 398875 
>   /branches/12/include/asterisk/features.h 398875 
>   /branches/12/main/bridge.c 398875 
>   /branches/12/main/bridge_basic.c 398875 
>   /branches/12/main/bridge_channel.c 398875 
>   /branches/12/main/channel.c 398875 
>   /branches/12/main/core_local.c 398875 
>   /branches/12/main/core_unreal.c 398875 
>   /branches/12/main/features.c 398875 
>   /branches/12/res/parking/parking_applications.c 398875 
>   /branches/12/res/parking/parking_bridge_features.c 398875 
>   /branches/12/res/parking/parking_tests.c 398875 
>   /branches/12/res/res_pjsip_refer.c 398875 
>   /branches/12/res/stasis/control.c 398875 
>   /branches/12/tests/test_cdr.c 398875 
>   /branches/12/tests/test_cel.c 398875 
> 
> Diff: https://reviewboard.asterisk.org/r/2845/diff/
> 
> 
> Testing
> -------
> 
> Tested Dial, Queue, and FollowMe with and without the 'I' option.  The connected line information was as expected.
> 
> In the process of testing the Queue I option, I came across a couple of assertions which I filed as ASTERISK-22506 and ASTERISK-22507.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130912/acc28f50/attachment-0001.htm>