[asterisk-dev] [Code Review] Support text messages outside of a call

[David Vossel]

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


Looks good so far, here's a few comments.


/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1042/#comment6271>

    If you're giving up on AUTH, shouldn't it just return here?  By falling through it looks like you're going to attempt to transmit again.



/trunk/include/asterisk/message.h
<https://reviewboard.asterisk.org/r/1042/#comment6275>

    Is there anyway you can abstract away needing to know that the ast_msg is an ao2 object?  Maybe an alloc and destroy function?!



/trunk/include/asterisk/message.h
<https://reviewboard.asterisk.org/r/1042/#comment6274>

    Some documentation would be nice for this function.  It was not immediately obvious to me how the msg ref would be handled on failure.  I probably would have tried to unref it myself, and this function does that for you.



/trunk/main/message.c
<https://reviewboard.asterisk.org/r/1042/#comment6273>

    I don't understand how we are guaranteed this module will still be in memory here.
    
    I understand adding the ref guarantees that the mod pointer will stay around until after the message is sent, but what is guaranteeing that the mod pointer is valid before the message is sent?  There is no ref given to that pointer before then.
    
    Is it not possible the module could be unloaded yet the tech_holder object pointing to that module still exist in this function because we have a ref for the tech_holder?


- David


On 2010-12-13 13:37:44, Russell Bryant wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1042/
> -----------------------------------------------------------
> 
> (Updated 2010-12-13 13:37:44)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This branch contains a proposal for adding protocol independent support for processing text messages into and out of the dialplan, outside of a call.  The file doc/asterisk-messaging.txt contains more details on the proposal.  The introduction of the document is quoted here:
> 
> "    Asterisk has some limited support today for messaging.  The support that
> exists primarily includes passing text messages in the context of a call.  The
> SIP and IAX2 protocols have support for this, but that's it.
> 
>     There are a couple of other messaging protocols that are supported: Skype
> and XMPP (Jabber).  The support of these is very minimal and not very integrated
> into the architecture of Asterisk since these messages are not in the context of
> a phone call.  They provide a combination of dialplan and manager interface
> interfaces that are specific to each protocol.  There just is no current
> architectural concept of dealing with text messages.
> 
>     The purpose of this proposal is to introduce text messaging into the
> architecture of Asterisk.  For messaging support to exist in the true spirit of
> Asterisk architecture, the design needs to achieve the following two goals:
> 
>     a) Protocol Independence
>     b) Scriptable message routing
> 
>     The rest of this document goes through some details about how these goals
> will be achieved in a way that is both architecturally compatible with Asterisk
> as well as practical to implement."
> 
> ----------
> 
> In addition to the documented proposal, I have made some good progress on implementation.  While the document includes some ideas for future enhancements, what is there so far should be usable.
> 
>  - core modifications to allow sending incoming messages through the dialplan
>  - core modifications to allow outbound messages from the dialplan
>  - modifications to res_jabber to allow inbound and outbound messages in the new architecture
>  - changes to chan_sip to support inbound and outbound MESSAGE outside of a call
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_jabber.c 298284 
>   /trunk/include/asterisk/jabber.h 298284 
>   /trunk/include/asterisk/message.h PRE-CREATION 
>   /trunk/main/asterisk.c 298284 
>   /trunk/main/channel.c 298284 
>   /trunk/main/message.c PRE-CREATION 
>   /trunk/include/asterisk/_private.h 298284 
>   /trunk/include/asterisk/channel.h 298284 
>   /trunk/configs/jabber.conf.sample 298284 
>   /trunk/configs/sip.conf.sample 298284 
>   /trunk/doc/asterisk-messaging.txt PRE-CREATION 
>   /trunk/channels/sip/include/sip.h 298284 
>   /trunk/channels/chan_sip.c 298284 
> 
> Diff: https://reviewboard.asterisk.org/r/1042/diff
> 
> 
> Testing
> -------
> 
> svn/testsuite/asterisk/team/russell/messaging:
>   - This branch of the testsuite contains my tests for this branch, which include:
>     - tests/sip/message_disabled
>       - Ensure MESSAGE outside of a call is rejected when disabled.
>     - tests/sip/message_unauth
>       - When enabled, test sending a MESSAGE to Asterisk and send another back out from the dialplan.
>     - tests/sip/message_auth
>       - Same as the last test, but authenticate MESSAGE both inbound and outbound.
>     - tests/sip/message_from_call
>       - Set up a normal SIP call and send an out of call MESSAGE from the dialplan processing the call
> 
> I have also written some simple apps using the pjsua Python module from pjsip that can send and receive messages sent through Asterisk.
> 
> Lastly, I have done some manual testing of XMPP messages in and out of Asterisk using this code.
> 
> 
> Thanks,
> 
> Russell
> 
>

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