[Home]

Summary:ASTERISK-12822: [patch] SMS app ignores parameter 'p' - initial pause
Reporter:Alec Davis (alecdavis)Labels:
Date Opened:2008-10-03 23:41:15Date Closed:2008-10-14 13:47:58
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_sms
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app_sms.13oct.diff.txt
Description:Can be tested without any actual sms equipment.

dialplan code:
exten => 8507,1,NoOp(SMS Test extension)
exten => 8507,n,Answer()
exten => 8507,n,SMS(sms,atp350)
exten => 8507,n,Hangup()

modified app_sms.c -> sms_exec:
       h.smsc = ast_test_flag(&sms_flags, OPTION_BE_SMSC);
       h.protocol = ast_test_flag(&sms_flags, OPTION_TWO) ? 2 : 1;

       if (!ast_strlen_zero(sms_opts[OPTION_ARG_PAUSE])){
               h.opause_0 = atoi(sms_opts[OPTION_ARG_PAUSE]);
               ast_log(LOG_NOTICE, "Initial Delay [%d]ms override\n", h.opause_0);        /* Debug: this line is never shown!!!!!!! */
       }
       if (h.opause_0 < 25 || h.opause_0 > 2000){
               ast_log(LOG_WARNING, "Initial Delay [%d]ms out of range. Default 300ms\n", h.opause_0);          /* added for DEBUG */
               h.opause_0 = 300;       /* default 300ms */
       }

console output:
   -- Executing [8507@trusted:1] NoOp("IAX2/aldhome-1248", "SMS Test extension") in new stack
   -- Executing [8507@trusted:2] Answer("IAX2/aldhome-1248", "") in new stack
   -- Executing [8507@trusted:3] SMS("IAX2/aldhome-1248", "sms,atp350") in new stack
sms argc 2 queue <sms> opts <atp350> addr <> body <>
[Oct  4 17:47:03] WARNING[16702]: app_sms.c:1806 sms_exec: Initial Delay [0]ms out of range. Default 300ms
   -- SMS TX 7F 00
Comments:By: Alec Davis (alecdavis) 2008-10-04 00:34:44

My dialplan syntax was wrong, but still initial delay is ignored.
As soon as the line is answered you hear the initial FSK tones, when there should be a delay of 2 seconds.

dialplan code:
exten => 8507,1,NoOp(SMS Test extension)
exten => 8507,n,Answer()
exten => 8507,n,SMS(sms,atp(2000))
exten => 8507,n,NoOp(SMS Finished)
exten => 8507,n,Hangup(21)

console output:
 -- Executing [8507@trusted:1] NoOp("IAX2/aldhome-4760", "SMS Test extension") in new stack
   -- Executing [8507@trusted:2] Answer("IAX2/aldhome-4760", "") in new stack
   -- Executing [8507@trusted:3] SMS("IAX2/aldhome-4760", "sms,atp(2000)") in new stack
sms argc 2 queue <sms> opts <atp(2000> addr <> body <>
[Oct  4 18:42:07] NOTICE[18624]: app_sms.c:1803 sms_exec: Initial Delay [2000]ms override
   -- SMS TX 7F 00

Alec
By: Alec Davis (alecdavis) 2008-10-04 02:13:34

Fixed: app_sms.c looks like it was initially debugged for protocol 1 not protocol 2.
Moved variables that are proto dependant into if/else structure.
Added test for Proto2 start message 0x7F.
Tested with dialplan SMS(sms,at) and with SMS(sms,atp(2000)).

<Code removed.  Code must always be an attachment.>

my app_sms.c has too much debug, so didn't do a diff sorry.

Alec


By: Alec Davis (alecdavis) 2008-10-04 03:03:34

diff attached. Includes previous patches from Croyden76.
By: Alec Davis (alecdavis) 2008-10-04 03:15:05

Current working implementation, dialplan code below.
Note: the additional exten => h,1 priorities, this is needed as vodafone NZ hangs up the call before were finished, maybe more debug required.

[cellular]
exten => s,1,NoOp(Callerid = ${CALLERID(all)})
exten => s,n,GotoIf($["${CALLERID(num)}"="042111"]?SMSMessage)
exten => s,n,Macro(dialout,${TRUNK},4888)
exten => s,n,Hangup()

exten => s,n(SMSMessage),NoOp( SMS MESSAGE )
exten => s,n,Answer()
exten => s,n,SMS(sms,atp(500))
exten => s,n,NoOp(Normally wont get here)

exten => h,1,NoOp( Send SMS to Email )
exten => h,n,System(/usr/lib/asterisk/sms-email)
exten => h,n,Hangup()
By: Alec Davis (alecdavis) 2008-10-05 17:51:19

License is pending, believe it's beyond my control.

update: License now OK, thanks.


By: Alec Davis (alecdavis) 2008-10-08 03:59:41

app_sms.08oct.diff.txt
print 'protocol %d' after 'initial delay %d ms'
removed some extraneous braces.
By: Tilghman Lesher (tilghman) 2008-10-08 14:02:18

The "extraneous braces" are now required, as per the coding guidelines.

Some of the formatting of the original patch is also invalid, according to the coding guidelines.  There needs to be spaces around braces, i.e. "} else {" not "}else{".


By: Alec Davis (alecdavis) 2008-10-08 15:38:48

Patch uploaded.

Tested and works.

regarding coding guidelines I stuck to the same format as the existing format, not really prepared at this stage to make the whole application compliant, but I do agree with braces everywhere approach.

reference:
http://svn.digium.com/view/asterisk/trunk/doc/CODING-GUIDELINES?view=markup
* General rules
.....
Try to match the existing formatting of the file you are working on.
....


By: Alec Davis (alecdavis) 2008-10-13 03:19:42

Please remove all previous non format compliant patches.

Uploaded new revised patch. app_sms.13oct.diff.txt
By: Leif Madsen (lmadsen) 2008-10-14 11:02:23

Assigned to Corydon76 for a code review. Set back to confirmed when complete.
By: Digium Subversion (svnbot) 2008-10-14 13:47:56

Repository: asterisk
Revision: 148985

U   trunk/apps/app_sms.c

------------------------------------------------------------------------
r148985 | tilghman | 2008-10-14 13:47:56 -0500 (Tue, 14 Oct 2008) | 6 lines

App is ignoring 'p' parameter -- initial pause.
(closes issue ASTERISK-12822)
Reported by: alecdavis
Patches:
      app_sms.13oct.diff.txt uploaded by alecdavis (license 585)

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

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