[Home]

Summary:ASTERISK-13468: [patch] Adding JabberJoin and an argument to JabberSend for groupchats
Reporter:Fredrik Liljegren (fiddur)Labels:
Date Opened:2009-01-28 04:39:31.000-0600Date Closed:2009-12-07 12:00:46.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Resources/res_jabber
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) jabber.h.diff
( 1) res_jabber.c.diff
( 2) trunk-14352-1.diff
( 3) trunk-14352-2.diff
( 4) trunk-199278-1.diff
Description:Joining groupchats already has functions in res_jabber.c, they are just not available from the dialplan.

I added JabberJoin and a parameter to JabberSend to say if it is to a groupchat.  

****** ADDITIONAL INFORMATION ******

My c skills may be a bit rusty, but I've tried to follow the indentation and commenting used in the rest of res_jabber
Comments:By: phsultan (phsultan) 2009-02-06 09:30:17.000-0600

Hi fiddur, thanks a lot for implementing this. I had some code that addresses this feature, which implements JabberJoin (and allows to choose a nickname for the chatroom) and JabberLeave. I attached the patch, that applies to trunk since it's a new feature.

Regarding JabberSend, I found it clearer to write a new application (JabberSendGroup) that explicitly sends group messages.

In the attached patch, Asterisk (when connected as a component) can send messages to a chatroom using the provided (optional) nickname. As an example, suppose you connected Asterisk as a component named 'asterisk' to your XMPP server, you could :
- enter any chat room with any nickname ;
- send messages from any nickname to any chat room.

If you connect Asterisk as a regular XMPP client, messages sent to chat rooms will be seen as coming from the nickname you chose when joining the corresponding chat room.

Can you please give your feedback on it?
By: Fredrik Liljegren (fiddur) 2009-02-09 02:58:20.000-0600

I used XMPP notation of user@server/nick for nickname-choice, but having it as separate parameter to JabberJoin is better!
Actually, in regards to JabberJoin, I would like to have join-option directly in jabber.conf, to have asterisk on a group automatically after a restart.  As a component that's not necessary, but I haven't set that up yet.

Having JabberSendGroup is good; then I haven't got to encapsulate all strings that happesn to have a comma in them :)

What's the point of having nickname in JabberLeave - to connect to the same chat with several nicks?

Thanks for the implementation!
By: phsultan (phsultan) 2009-02-13 04:53:12.000-0600

> Actually, in regards to JabberJoin, I would like to have join-option directly in
> jabber.conf, to have asterisk on a group automatically after a restart. As a
> component that's not necessary, but I haven't set that up yet.

That's a great idea. Why do you think this is not necessary in component mode? Anyway, I'll update the patch to add this option.

> What's the point of having nickname in JabberLeave - to connect to the same chat
> with several nicks?

Yep. In component mode, you could have a single Asterisk server that connects/disconnects users in a chatroom.

I'll try to come with a new patch soon, thanks a lot for your constructive comments!
By: Fredrik Liljegren (fiddur) 2009-02-26 06:57:23.000-0600

>> Actually, in regards to JabberJoin, I would like to have join-option
>> directly in
>> jabber.conf, to have asterisk on a group automatically after a restart. As a
>> component that's not necessary, but I haven't set that up yet.

> That's a great idea. Why do you think this is not necessary in component
> mode? Anyway, I'll update the patch to add this option.

I just meant because you don't need to join the channels to send to them, as a component.
By: Leif Madsen (lmadsen) 2009-05-20 09:04:15

Just pinging this issue to see how much more needs to be done here before we can move this to Ready for Testing?
By: Fredrik Liljegren (fiddur) 2009-06-05 03:18:11

I tried applying trunk-14352-1.diff to current trunk... I had to do some manual patching, but the functionality was good after that.

phsultan: If you're not ready to commit the extra functionality, I could upload a patch to current trunk of the changes you made; I've done some testing allready and it's good for my needs at least!
By: phsultan (phsultan) 2009-06-05 04:43:08

Ok fiddur, thanks for your testing results. Please upload an updated patch so we can close the ticket.

Thanks again!
By: Fredrik Liljegren (fiddur) 2009-06-05 09:06:49

I have to ask first, why these two lines where removed from trunk:

static int aji_send_exec(struct ast_channel *chan, void *data);
static int aji_status_exec(struct ast_channel *chan, void *data);

..that is, the predeclaration of the exec-functions specifically.  I changed them to const char *data, but should the forward declarations of exec-functions be removed?
By: Leif Madsen (lmadsen) 2009-06-05 10:57:40

I wonder if that has something to do with the recent "Consti-fy the world" stuff... ?
By: Fredrik Liljegren (fiddur) 2009-06-05 11:37:40

Yes, that was where it was done...
https://reviewboard.asterisk.org/r/251/diff/?page=7

...but I don't understand why.  That is; I understand the changing of void * to const char *, but why remove the forward declarations of just these two functions?

....I'm asking through the review board.
By: Fredrik Liljegren (fiddur) 2009-06-05 13:14:38

Ok, I got the answer.  Sounds reasonable.  Here comes the patch.
By: Fredrik Liljegren (fiddur) 2009-06-08 01:19:26

I've tested JabberJoin with nickname set, JabberSend, JabberSendGroup and JabberLeave.  Works according to manual.  core show application Jabber... looks good.

JabberJoin to a room you're already in but with another nickname changes the nickname to the new specified.  That's ok, but undocumented.

JabberJoin without a nickname specified gets the nickname "asterisk".  What I see in the code, that's expected when you connect as a componen, but otherwise you should get the nickname from the jid... and my jabber.conf has type=client.  So I guess that's an error. ..haven't tried to debug that, yet :)
By: phsultan (phsultan) 2009-06-16 07:42:36

I just uploaded a new version, can you confirm the nickname issue is solved? Thanks!
By: Fredrik Liljegren (fiddur) 2009-06-16 10:03:04

Yes, at least partly.

JabberJoin without a nickname now gets the expected nickname.

JabberJoin to a room you're already in but with another nickname, changes the nickname.  I'd say that's good, but could be documented.

All in all, it works well!
By: Digium Subversion (svnbot) 2009-12-07 11:59:47.000-0600

Repository: asterisk
Revision: 233468

U   trunk/CHANGES
U   trunk/include/asterisk/jabber.h
U   trunk/res/res_jabber.c

------------------------------------------------------------------------
r233468 | jpeeler | 2009-12-07 11:59:46 -0600 (Mon, 07 Dec 2009) | 8 lines

Add applications JabberJoin, JabberLeave, JabberSendGroup for XMPP groupchat

(closes issue ASTERISK-13468)
Reported by: fiddur
Patches:
     trunk-14352-2.diff uploaded by phsultan (license 73)
Tested by: fiddur

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=233468
By: Digium Subversion (svnbot) 2009-12-07 12:00:44.000-0600

Repository: asterisk
Revision: 233469

_U  branches/1.6.2/

------------------------------------------------------------------------
r233469 | jpeeler | 2009-12-07 12:00:44 -0600 (Mon, 07 Dec 2009) | 14 lines

Blocked revisions 233468 via svnmerge

........
 r233468 | jpeeler | 2009-12-07 11:59:46 -0600 (Mon, 07 Dec 2009) | 8 lines
 
 Add applications JabberJoin, JabberLeave, JabberSendGroup for XMPP groupchat
 
 (closes issue ASTERISK-13468)
 Reported by: fiddur
 Patches:
       trunk-14352-2.diff uploaded by phsultan (license 73)
 Tested by: fiddur
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=233469