[asterisk-dev] [Code Review] Add replacement for SIP_CAUSE

[Mark Michelson]

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


There are a bunch of places that need to be updated to handle reading AST_CONTROL_PVT_CAUSE_CODE. To name a few, wait_for_answer() in app_queue.c, handle_frame() in dial.c, ast_generic_bridge() in channel.c, remote_bridge_loop() and local_bridge_loop() in rtp_engine.c, and possibly wait_for_winner() in app_followme.c. In addition, you should also add the control frame to the switch statement in waitstream_core() of file.c so that there is no warning message about an unexpected control subclass printed.


trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1822/#comment10705>

    Get rid of this.



trunk/main/channel.c
<https://reviewboard.asterisk.org/r/1822/#comment10702>

    This case is out of place here. This switch statement is here for the intention of getting tones to play to a channel. There is another switch statement higher up in ast_indicate_data() where AST_CONTROL_CONNECTED_LINE and AST_CONTROL_REDIRECTING have some special handling done. That seems like a better place for the AST_CONTROL_PVT_CAUSE_CODE to go. Having it up there would also allow you to return earlier, meaning not as many wasted cycles either.



trunk/main/channel.c
<https://reviewboard.asterisk.org/r/1822/#comment10698>

    This pattern is unnecessary here. You can safely call pbx_builtin_setvar_helper() without having to do the ref-and-lock waltz.


- Mark


On March 20, 2012, 10:43 a.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1822/
> -----------------------------------------------------------
> 
> (Updated March 20, 2012, 10:43 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Add PVT_CAUSE as a drop-in replacement for SIP_CAUSE that does not incur the overhead of the MASTER_CHANNEL dialplan function.  This feature uses control frames to pass the data and creates a mechanism by which any channel driver can report cause information.  This implementation includes only SIP, but implementations for other channel drivers will be available in the next month.
> 
> 
> This addresses bug SWP-4221.
>     https://issues.asterisk.org/jira/browse/SWP-4221
> 
> 
> Diffs
> -----
> 
>   trunk/apps/app_dial.c 359891 
>   trunk/channels/chan_sip.c 359891 
>   trunk/include/asterisk/frame.h 359891 
>   trunk/main/channel.c 359891 
>   trunk/main/features.c 359891 
> 
> Diff: https://reviewboard.asterisk.org/r/1822/diff
> 
> 
> Testing
> -------
> 
> Verified that this functions identically to SIP_CAUSE in single-channel dials, forked dials, and forked dials behind a local dial.
> 
> Sample dialplan:
> [foo]
> exten => s,1,Dial(SIP/bar)
>  
> exten => h,1,noop()
> exten => h,n,set(PVT_CAUSE_STRING=${HASHKEYS(PVT_CAUSE)})
> ; start loop
> exten => h,n(pvt_begin),noop()
>  
> ; check exit condition (no more array to check)
> exten => h,n,gotoif($[${LEN(${PVT_CAUSE_STRING})} = 0]?pvt_exit)
>  
> ; pull the next item
> exten => h,n,set(ARRAY(item)=${PVT_CAUSE_STRING})
> exten => h,n,set(PVT_CAUSE_STRING=${PVT_CAUSE_STRING:${LEN(${item})}})
>  
> ; display the channel ID and cause code
> exten => h,n,noop(got channel ID ${item} with pvt cause ${HASH(PVT_CAUSE,${item})})
>  
> ; check exit condition (no more array to check)
> exten => h,n,gotoif($[${LEN(${PVT_CAUSE_STRING})} = 0]?pvt_exit)
>  
> ; we still have entries to process, so strip the leading comma
> exten => h,n,set(PVT_CAUSE_STRING=${PVT_CAUSE_STRING:1})
> ; go back to the beginning of the loop
> exten => h,n,goto(pvt_begin)
> exten => h,n(pvt_exit),noop(All PVT_CAUSE entries processed)
> 
> 
> Thanks,
> 
> opticron
> 
>

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