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

opticron reviewboard at asterisk.org
Fri Mar 23 10:01:29 CDT 2012



> On March 22, 2012, 2:34 p.m., Mark Michelson wrote:
> > 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.

Done.


> On March 22, 2012, 2:34 p.m., Mark Michelson wrote:
> > trunk/channels/chan_sip.c, line 26110
> > <https://reviewboard.asterisk.org/r/1822/diff/1/?file=26414#file26414line26110>
> >
> >     Get rid of this.

Oops, done.


> On March 22, 2012, 2:34 p.m., Mark Michelson wrote:
> > trunk/main/channel.c, lines 4274-4289
> > <https://reviewboard.asterisk.org/r/1822/diff/1/?file=26416#file26416line4274>
> >
> >     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.

This needs to be handled after the attempt to indicate this frame to the channel driver.  Local is the only one that handles this frame internally by passing it down the line.  If it was handled above, the local channel driver would never get a chance to pass it along.


- opticron


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


On March 23, 2012, 10 a.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1822/
> -----------------------------------------------------------
> 
> (Updated March 23, 2012, 10 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/funcs/func_frame_trace.c 360259 
>   trunk/include/asterisk/frame.h 360259 
>   trunk/main/channel.c 360259 
>   trunk/main/dial.c 360259 
>   trunk/main/features.c 360259 
>   trunk/main/file.c 360259 
>   trunk/main/rtp_engine.c 360259 
>   trunk/CHANGES 360259 
>   trunk/UPGRADE.txt 360259 
>   trunk/apps/app_dial.c 360259 
>   trunk/apps/app_followme.c 360259 
>   trunk/apps/app_queue.c 360259 
>   trunk/channels/chan_sip.c 360259 
> 
> 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/20120323/38a1098c/attachment-0001.htm>


More information about the asterisk-dev mailing list