[asterisk-dev] [Code Review] 3780: res_pjsip_outbound_publish / res_pjsip_publish_asterisk: Add outbound PUBLISH support with 'asterisk' event type.

[Mark Michelson]

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



/trunk/res/res_pjsip_outbound_publish.c
<https://reviewboard.asterisk.org/r/3780/#comment23289>

    There are two response codes that RFC 3903 Section 5 describes handling for:
    
    423 Interval Too Brief: we MAY opt to re-send the PUBLISH with a higher expiration in this case.
    
    412 Conditional Request Failed: this indicates that we sent a PUBLISH on an expired or otherwise unknown publish session. This means that we need to kill off the current publish session and send a fresh new initial PUBLISH in its place.
    
    PJSIP does not automatically handle either of these cases.



/trunk/res/res_pjsip_outbound_publish.c
<https://reviewboard.asterisk.org/r/3780/#comment23288>

    This isn't *quite* enough to deal with the bizarreness of auto-refresh.
    
    When the publication code sends a publish refresh, if the PUBLISH request cannot be created (perhaps because there is a pending transaction), then your callback will be called with param->code == 400, param->reason set to the error text, param->rdata == NULL, and param->expiration == -1.
    
    If I'm seeing PJSIP's publication code correctly, the following can occur.
    
    * We send an initial publication that expires in 60 seconds.
    * We get a 200 OK for the publish.
    * A refresh of the PUBLISH is scheduled.
    * 59 seconds pass with no state changes.
    * The resource changes states.
    * We send a PUBLISH with updated state.
    * PJSIP marks the publication as having a pending transaction.
    * The timer fires, causing PJSIP to attempt to send a publication refresh.
    * The refresh fails to get created since there is a pending transaction.
    * PJSIP calls into the callback.
    
    At this point, publish->state->sending is non-NULL since we have sent out a PUBLISH, but the callback does not correspond to a response to our PUBLISH.
    
    Unfortunately, they've made it quite difficult to be able to detect that your callback is being called because the refresh failed to be created.
    
    * param->rdata can be NULL for multiple reasons. It could be NULL if your callback gets called because the transaction for your outbound PUBLISH timed out, for instance.
    * param->expiration can be -1 if the response to our PUBLISH has no Expires header in it (bad form, but someone may do it). Or again, if the PUBLISH transaction times out, it'll be set to -1.
    * param->code is set to 400 if the refresh fails to get created. 400 is obviously also a possibility for a response to our PUBLISH.
    * Don't try to use param->reason to deduce what's going on. Just don't.
    
    So...not really sure what the best way to handle this is.



/trunk/res/res_pjsip_outbound_publish.c
<https://reviewboard.asterisk.org/r/3780/#comment23290>

    Two issues:
    
    1) You don't need to allocate a pool. You can just use the pool from pjsip_publishc_get_pool() since it has not been destroyed yet.
    
    2) You don't need tmp since all of the URIs you're parsing are NULL-terminated already and parsing does not, AFAIK, modify the string passed in.


- Mark Michelson


On July 29, 2014, 2:40 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3780/
> -----------------------------------------------------------
> 
> (Updated July 29, 2014, 2:40 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This adds two PJSIP modules which add outbound PUBLISH support and an 'asterisk' event type.
> 
> The res_pjsip_outbound_publish module is a common module which provides basic logic for setting up outbound PUBLISH clients, handling authentication requests, handling configuration, and lifetime. Extra modules implement specific event types which are registered with res_pjsip_outbound_publish. Since it takes care of configuration when an outbound PUBLISH is configured extra configuration can be passed to the event type implementation to further configure itself.
> 
> The res_pjsip_publish_asterisk module implements inbound and outbound support for an 'asterisk' event type. This event type conveys device and mailbox state between Asterisk instances using a JSON content body. As internal device or mailbox state changes the module sends a PUBLISH message to other configured instances. When a PUBLISH is received the contents are examined and a device or mailbox state change queued up within Asterisk. To restrict what is sent and received filtering is available using regular expressions which can reduce SIP traffic.
> 
> A wiki page is available at https://wiki.asterisk.org/wiki/display/~jcolp/Exchanging+Device+and+Mailbox+State+Using+PJSIP which has some configuration details with some examples. This should also be reviewed.
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_pjsip_pubsub.exports.in 419753 
>   /trunk/res/res_pjsip_pubsub.c 419753 
>   /trunk/res/res_pjsip_publish_asterisk.c PRE-CREATION 
>   /trunk/res/res_pjsip_outbound_publish.exports.in PRE-CREATION 
>   /trunk/res/res_pjsip_outbound_publish.c PRE-CREATION 
>   /trunk/include/asterisk/res_pjsip_pubsub.h 419753 
>   /trunk/include/asterisk/res_pjsip_outbound_publish.h PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/3780/diff/
> 
> 
> Testing
> -------
> 
> Set up two Asterisk instances, configured both sides to publish to eachother, made calls and manipulated voicemail. Watched PUBLISH messages go between them and state change.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140729/d8eda053/attachment-0001.html>