[Home]

Summary:ASTERISK-05974: [test-this-branch] Add sendtext manager application
Reporter:Matt Riddell (zx81)Labels:
Date Opened:2006-01-04 06:50:17.000-0600Date Closed:2006-10-31 04:42:54.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) sendtext_manager7.patch
( 1) sendtext_manager8.patch
Description:This patch allows you to send a text message using the manager interface so that you can send messages to phones while they are on calls.  I.E. it will allow for updating the LCD on PA1688 based phones with call costs etc in real time.

I'm pretty sure I've done things right but it will still need a reasonable code review - don't worry it's pretty simple!

:)

Matt Riddell
Comments:By: Matt Riddell (zx81) 2006-01-04 07:20:34.000-0600

Added action id and explanation that this only works during a call
By: Matt Riddell (zx81) 2006-01-04 07:21:17.000-0600

is now against 7785

Could someone delete the first of the two patches?


By: Olle Johansson (oej) 2006-01-04 07:43:02.000-0600

Did you test it?

This part seems interesting:
     astman_send_ack(s, m, "Message Sent");
+        if (id && !ast_strlen_zero(id))
+                ast_cli(s->fd, "ActionID: %s\r\n",id);
+        return res;

Adding action ID after you acknowledged... Hmmm... ;-)
By: Matt Riddell (zx81) 2006-01-04 07:55:05.000-0600

should I do it like this:

       if (id && !ast_strlen_zero(id))
               ast_cli(s->fd, "ActionID: %s\r\n",id);
       res = ast_sendtext(c, textmsg);
       ast_mutex_unlock(&c->lock);
       if (res>0)
               astman_send_ack(s, m, "Message Sent");
       return res;

BTW: I did test it before inclusion of ActionID  :D

Oh, and is there something specific I should respond if res<=0?

or is Message Not Sent ok?


By: Olle Johansson (oej) 2006-01-04 08:56:13.000-0600

ZX81: "Message sent" and "message not sent" is not really good machine-parsable return values. "Failure" or "success" is better.
By: Matt Riddell (zx81) 2006-01-04 10:40:29.000-0600

Ask and you shall receive:

       if (res>0){
               astman_send_ack(s, m, "Message Success");
       } else {
               astman_send_ack(s, m, "Message Failure");
       }

Update: Version 6 fixes a locking problem where it was already locking inside sendtext.  Now it unlocks first.  (1 line swap)


By: Matt Riddell (zx81) 2006-01-09 07:20:05.000-0600

Anything else I need to do to get this in?
By: Olle Johansson (oej) 2006-01-09 08:19:09.000-0600

I was more thinking only "Failure" not "Message Failure". They now from the event and action ID what it is about, we just need to tell them what happened. We have to make it easy to write a parser by re-using stuff the same way.
By: Matt Riddell (zx81) 2006-01-09 09:23:19.000-0600

Version 7 uploaded with requested change from Message Failure/Success to Failure/Success

(Is now against 7888)


By: Olle Johansson (oej) 2006-01-09 09:54:34.000-0600

Recommended for svn trunk.
By: Olle Johansson (oej) 2006-01-09 09:55:35.000-0600

ZX81: Thanks for following through with all changes and such!
By: Matt Riddell (zx81) 2006-01-09 10:22:05.000-0600

:)

No Problem
By: Stefan Reuter (srt) 2006-01-18 19:56:51.000-0600

There are two problems with this patch:
1. It adds an additional ActionID, so you actually get it twice. You dont have to handle the ActionID by yourself its done by astman_send_ack.
2. You should indicate failure by using astman_send_error instead of astman_send_ack in the case of a failure.

and last you should probably use ast_strlen_zero (it also handles NULL) instead of strlen to check for mandatory properties.


By: Matt Riddell (zx81) 2006-01-19 07:41:35.000-0600

Done, Done and Done.

Patch is now against 8259.

Only changes were those requested.

Once this gets in, I can convert the other strlen's in the manager to ast_strlen_zero's and submit a patch


By: Stefan Reuter (srt) 2006-01-19 07:52:38.000-0600

+1 for getting this commited now
By: Olle Johansson (oej) 2006-02-22 09:17:01.000-0600

Included in the branch "test-this-branch" for testing...
By: Matt Riddell (zx81) 2006-03-28 20:42:17.000-0600

so, is it tested now?  :)


By: Matt Riddell (zx81) 2006-04-14 08:04:31

How's this going?  Any update?
By: Serge Vecher (serge-v) 2006-04-14 13:28:16

ZX81: the best option for soliciting feedback on new feauters is to use the asterisk lists which have much more exposure than the bug-tracker. Please use these tools to advertise your patch and get feedback. Thank you.
By: Olle Johansson (oej) 2006-04-16 22:30:11

There where some worries about channel locking and sending text messages, but I'll check again if it's ok to merge.
By: Matt O'Gorman (mogorman) 2006-04-26 17:29:14

prod oej how did it go
By: Serge Vecher (serge-v) 2006-05-04 14:19:05

Still need confirmation of successful tests in order for this to move forward.
By: Serge Vecher (serge-v) 2006-06-05 11:04:59

ZX81: any chance of updating this patch so this can be considered for 1.4 beta?
By: Matt Riddell (zx81) 2006-06-05 16:14:37

Will try tomorrow.  A little snowed under and not currently running HEAD as MySQL Realtime/CDR is/was broken.
By: jmls (jmls) 2006-10-31 04:32:40.000-0600

ZX81: were you able to update the patch ?
By: Olle Johansson (oej) 2006-10-31 04:42:28.000-0600

Committed to svn trunk.
By: Olle Johansson (oej) 2006-10-31 04:42:52.000-0600

Rev 46630