[asterisk-dev] [Code Review] New application JabberReceive, implement SendText in chan_gtalk and chan_jingle
Russell Bryant
russell at digium.com
Fri Dec 12 09:14:17 CST 2008
> On 2008-12-11 22:02:02, Russell Bryant wrote:
> > http://svn.digium.com/svn/asterisk/trunk/res/res_jabber.c, lines 821-822
> > <http://reviewboard.digium.com/r/88/diff/1/?file=1872#file1872line821>
> >
> > This is an interesting way to implement this feature. I especially like that it is minimally invasive. However, I do see some downsides to the way this currently works.
> >
> > The first problem I see is that there is technically a race condition related to receiving messages. In the examples, you send a message, and then you run an application to receive a message. Unfortunately, there is a period of time in between those two where a message might come in, and if your channel isn't ready to receive it, it will get lost.
> >
> > The way that I envisioned doing this is to have an option that enabled receiving messages globally. Then, when messages are received, you could cache them for some configurable period of time. You could also have a configurable cache size limit. Finally, the receive application could do a couple of things:
> >
> > 1) Check to see if the message you want to receive has already come in and is in the cache. If so, remove it and return it.
> >
> > 2) Add yourself to a list of channels waiting for some configurable amount of time for a message to come in. When messages come in, before caching them, they get offered to callers waiting for messages.
> >
> >
> > Those are just some thoughts I had about this functionality. The downside is that the code would get a lot more complicated. The benefit is that it handles the possibility of a message arriving before the dialplan has reached the point of being ready to handle it.
>
> wrote:
> Maybe we can check the call to ast_queue_frame to ensure the incoming message will be delivered to a listening channel?
>
> All incoming messages that come to Asterisk on its XMPP client(s) are handled in aji_handle_message. We could acknowledge the message if we actually queue it as a text frame (when calling ast_queue_frame, as the return codes don't provide much more information), for example by sending a notification that Asterisk 'is composing' a reply. Or we could inform the user that his message won't be processed and possibly ask him to send it again.
>
> The cache mechanism you describe prevents us from losing messages, which is more powerful than just informing the user about the message delivery process. On the other hand, Asterisk frames along with the useful queuing functionality makes the latter option possibly acceptable (apart from the fact that it will save me some work too :)). What's your opinion?
>
> Russell, thanks a lot for taking the time to review this code, and for your feedback as well.
What if it's not a user, but an automated system? Asking the other side to resend the message is not very ideal.
Another benefit of this other route is that it avoids the channel_walk_locked() loop, which is very expensive. An optimization would be the new ast_channel_search() API call. However, this other route avoids the need for searching the channel list at all, and will make a big difference on heavily loaded systems.
I would be happy to help with this new approach if you would like to discuss implementation specifics in more detail on the -dev list.
- Russell
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/88/#review201
-----------------------------------------------------------
On 2008-12-11 21:50:49, Philippe Sultan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/88/
> -----------------------------------------------------------
>
> (Updated 2008-12-11 21:50:49)
>
>
> Review request for Asterisk Developers.
>
>
> Summary
> -------
>
> JabberReceive is a dialplan application that makes Asterisk wait for an XMPP message from a given user (identified with his JID), and store the content in a dialplan variable.
>
> It can be used for example to notify a user (via XMPP) that a call is coming, and read input back from him.
> Taken from the documentation :
> In the following example, calls targeted to extension 1234 (be it
> accessed from SIP, DAHDI or whatever channel type) are controlled by
> user bob at jabber.org. Asterisk notifies him that a call is coming, and
> asks him to take an action. This dialog takes place over an XMPP chat.
>
> context from-ext {
> 1234 => {
> Answer();
> JabberSend(asterisk-xmpp,bob at jabber.org,Call from $CALLERID(num) - choose an option to process the call);
> JabberSend(asterisk-xmpp,bob at jabber.org,1 : forward to cellphone);
> JabberSend(asterisk-xmpp,bob at jabber.org,2 : forward to work phone);
> JabberSend(asterisk-xmpp,bob at jabber.org,Default action : forward to your voicemail);
> JabberReceive(bob at jabber.org,OPTION,20);
> switch (${OPTION}) {
> case 1:
> JabberSend(asterisk-xmpp,bob at jabber.org,(Calling cellphone...);
> Dial(SIP/987654321);
> break;
> case 2:
> JabberSend(asterisk-xmpp,bob at jabber.org,(Calling workphone...);
> Dial(SIP/${EXTEN});
> break;
> default:
> Voicemail(${EXTEN}|u)
> }
> }
> }
>
> The diff also includes an implementation of SendText as XMPP messages in both chan_jingle and chan_gtalk.
>
> The corresponding bug contains more use cases : http://bugs.digium.com/view.php?id=12569
>
>
> This addresses bug 12569.
> http://bugs.digium.com/view.php?id=12569
>
>
> Diffs
> -----
>
> http://svn.digium.com/svn/asterisk/trunk/channels/chan_gtalk.c 163162
> http://svn.digium.com/svn/asterisk/trunk/channels/chan_jingle.c 163162
> http://svn.digium.com/svn/asterisk/trunk/doc/jabber.txt 163162
> http://svn.digium.com/svn/asterisk/trunk/res/res_jabber.c 163162
>
> Diff: http://reviewboard.digium.com/r/88/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Philippe
>
>
More information about the asterisk-dev
mailing list