[asterisk-dev] [Code Review] Add Connected line interception macros to Asterisk

[Russell Bryant]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/256/#review813
-----------------------------------------------------------

Ship it!


Other than this one comment, and some trailing whitespace in the header file changes, this looks good to go.

Nice work, Mark!


/trunk/main/channel.c
<http://reviewboard.digium.com/r/256/#comment2003>

    Hm, this seems kind of weird.  Why do we reset jitterbuffer state on a connected line update?  It seems as if the code is trying to infer more from the update than it should.
    
    There are other mechanisms, such as the SSRC changing in RTP, to detect when the source of the media has changed and jitterbuffer state should be reset.  I think that it should be removed from this code.
    
    This same comment goes for the handling of AST_CONTROL_REDIRECTING, even though that is not being changed in this patch.


- Russell


On 2009-06-01 13:48:56, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/256/
> -----------------------------------------------------------
> 
> (Updated 2009-06-01 13:48:56)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Connected line reception and transmission is a new feature in Asterisk trunk. One of its shortcomings is the inability to modify received connected line information while a dialplan application is running. 
> 
> Here is a common example where this may be useful. Many office environments require that a digit (such as '9') is dialled in order to reach the PSTN. The problem is that when one dials a PSTN number, the information received from the far end will not have this prefix digit. To make the lives of users easier, it would be really nice if the prefix digit could be displayed for them.
> 
> This proposed patch adds the ability to add connected line interception macros into the dialplan. When we are going to send a connected line update to someone, the administrator has the ability to run a macro which can do whatever manipulation of connected line information is necessary to appease everyone. There are four defined macros in the channelvariables.tex file.
> 
> ${CONNECTED_LINE_SEND_CALLEE_MACRO}        Macro to call before sending a connected line update to the callee
> ${CONNECTED_LINE_SEND_CALLEE_MACRO_ARGS}   Arguments to pass to ${CONNECTED_LINE_SEND_CALLEE_MACRO}
> ${CONNECTED_LINE_SEND_CALLER_MACRO}        Macro to call before sending a connected line update to the caller
> ${CONNECTED_LINE_SEND_CALLER_MACRO_ARGS}   Arguments to pass to ${CONNECTED_LINE_SEND_CALLER_MACRO}
> 
> So in the example I gave above, the administrator could write a simple macro like this:
> 
> [macro-addnine]
> exten => s,1,Set(CONNECTEDLINE(num,i)=9${CONNECTEDLINE(num)}); Adds '9' to the connected line number
> 
> Then, he could add a priority before any Dial to the PSTN:
> exten => blah,n,Set(CONNECTED_LINE_SEND_CALLER_MACRO=addnine)
> 
> Simple as that! And of course, one is not limited to modifying just the connected line number. You could modify the name, or even information not relating to connected line information.
> 
> Please review my code and see if there is anything that needs fixing. I realize as I am typing this that I have not placed anything in the CHANGES file, so I need to do that soon.i
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_dial.c 198559 
>   /trunk/apps/app_directed_pickup.c 198559 
>   /trunk/apps/app_macro.c 198559 
>   /trunk/apps/app_queue.c 198559 
>   /trunk/channels/chan_sip.c 198559 
>   /trunk/doc/tex/channelvariables.tex 198559 
>   /trunk/include/asterisk/app.h 198559 
>   /trunk/include/asterisk/channel.h 198559 
>   /trunk/main/app.c 198559 
>   /trunk/main/channel.c 198559 
>   /trunk/main/dial.c 198559 
>   /trunk/main/features.c 198559 
> 
> Diff: http://reviewboard.digium.com/r/256/diff
> 
> 
> Testing
> -------
> 
> I have tested this with a scenario similar to what I listed above. In addition, I tested features such as call pickup (both using the Pickup application and the pickupexten from features.conf), built-in blind and attended transfers, and SIP transfers.
> 
> 
> Thanks,
> 
> Mark
> 
>