[asterisk-dev] [Code Review] 2466: Pimp my SIP: Call forwarding & diversion headers

[Joshua Colp]

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



/team/group/pimp_my_sip/include/asterisk/res_sip.h
<https://reviewboard.asterisk.org/r/2466/#comment16046>

    chan_sip has an option called "promiscredir" which follows the explicit SIP URI and does not just go to the user portion within the dialplan. Is this something we want to do as well here? (albeit maybe not the same way)



/team/group/pimp_my_sip/res/res_sip_diversion.c
<https://reviewboard.asterisk.org/r/2466/#comment16047>

    reason can be passed in const



/team/group/pimp_my_sip/res/res_sip_diversion.c
<https://reviewboard.asterisk.org/r/2466/#comment16056>

    Heck, you could drop the parsed variable and just return the result of pjsip_parse_hdr directly per my next comment.



/team/group/pimp_my_sip/res/res_sip_diversion.c
<https://reviewboard.asterisk.org/r/2466/#comment16053>

    Redundant.



/team/group/pimp_my_sip/res/res_sip_diversion.c
<https://reviewboard.asterisk.org/r/2466/#comment16057>

    Having this return a value is silly as it only ever returns 1.



/team/group/pimp_my_sip/res/res_sip_diversion.c
<https://reviewboard.asterisk.org/r/2466/#comment16058>

    Did you swipe this from pjsip source code?



/team/group/pimp_my_sip/res/res_sip_diversion.c
<https://reviewboard.asterisk.org/r/2466/#comment16059>

    Can this not be moved up before creating a header, duplicating, etc?



/team/group/pimp_my_sip/res/res_sip_diversion.c
<https://reviewboard.asterisk.org/r/2466/#comment16074>

    You can use PJSIP_IS_STATUS_IN_CLASS in here with 300 as the code class


- Joshua Colp


On April 24, 2013, 8:09 p.m., Kevin Harwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2466/
> -----------------------------------------------------------
> 
> (Updated April 24, 2013, 8:09 p.m.)
> 
> 
> Review request for Asterisk Developers, Joshua Colp and Mark Michelson.
> 
> 
> Bugs: ASTERISK-21426
>     https://issues.asterisk.org/jira/browse/ASTERISK-21426
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Adds call forwarding support (Josh's patch) to the new SIP work being done in Asterisk.  This also includes the ability to add a diversion header to an outgoing response/request when appropriate.  The diversion header feature can be turned off by setting the send_diversion=false (defaults to true) on an endpoint within the configuration file.
> 
> 
> Diffs
> -----
> 
>   /team/group/pimp_my_sip/channels/chan_gulp.c 386451 
>   /team/group/pimp_my_sip/include/asterisk/res_sip.h 386451 
>   /team/group/pimp_my_sip/res/res_sip.c 386451 
>   /team/group/pimp_my_sip/res/res_sip/sip_configuration.c 386451 
>   /team/group/pimp_my_sip/res/res_sip_diversion.c PRE-CREATION 
>   /team/group/pimp_my_sip/res/res_sip_session.c 386451 
> 
> Diff: https://reviewboard.asterisk.org/r/2466/diff/
> 
> 
> Testing
> -------
> 
> Wrote a few testsuite tests to make sure calls are being forwarded and the diversion header is added/propagated appropriately.
> 
> 
> Thanks,
> 
> Kevin Harwell
> 
>

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