[asterisk-dev] [Code Review] 2623: New SIP Channel Driver - Add CLI/AMI initiated NOTIFY requests

[Joshua Colp]

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



./trunk/include/asterisk/res_sip.h
<https://reviewboard.asterisk.org/r/2623/#comment17534>

    I know you didn't write this but... this isn't actually true. Since sorcery can pull these from anywhere they may or may not already be loaded. Might as well make it reflect reality.



./trunk/res/res_sip_notify.c
<https://reviewboard.asterisk.org/r/2623/#comment17535>

    Why a pjsua header file?



./trunk/res/res_sip_notify.c
<https://reviewboard.asterisk.org/r/2623/#comment17540>

    If you move configuration to the configuration framework you won't need these, I'll explain below.



./trunk/res/res_sip_notify.c
<https://reviewboard.asterisk.org/r/2623/#comment17537>

    The real reason this is needed is because you are consuming straight from the configuration system instead of interpreting and creating data structures.
    
    Further down I say you should move to the configuration framework (for different reasons) but it would also eliminate this.
    
    When reading in the configuration you could construct a full content value there, and also parse the content type as you need.



./trunk/res/res_sip_notify.c
<https://reviewboard.asterisk.org/r/2623/#comment17538>

    This is incorrect. It goes to an endpoint.



./trunk/res/res_sip_notify.c
<https://reviewboard.asterisk.org/r/2623/#comment17539>

    This is all sorts of unsafe. If being used at the same time as a reload it will break.
    
    I'd suggest moving this over to the configuration framework if possible.


- Joshua Colp


On June 14, 2013, 4:05 p.m., Kevin Harwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2623/
> -----------------------------------------------------------
> 
> (Updated June 14, 2013, 4:05 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21436
>     https://issues.asterisk.org/jira/browse/ASTERISK-21436
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Added the ability to send unsolicited NOTIFY requests to a particular endpoint with a configured payload.  Added both CLI and AMI support.  For a given endpoint, this module will iterate over all its contacts sending the appropriate NOTIFY request to each.
> 
> 
> Diffs
> -----
> 
>   ./trunk/res/res_sip_notify.c PRE-CREATION 
>   ./trunk/res/res_sip/sip_options.c 391775 
>   ./trunk/res/res_sip/sip_configuration.c 391775 
>   ./trunk/res/res_sip/include/res_sip_private.h 391775 
>   ./trunk/res/res_sip.exports.in 391775 
>   ./trunk/include/asterisk/res_sip.h 391775 
> 
> Diff: https://reviewboard.asterisk.org/r/2623/diff/
> 
> 
> Testing
> -------
> 
> From the CLI issued the command to send a NOTIFY request to an endpoint with payload data that had been configured in sip_notify.conf.  Also connected via AMI and initiated an action to send a NOTIFY request to an endpoint with a given event type.
> 
> 
> Thanks,
> 
> Kevin Harwell
> 
>

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