Sorry for my last email - I did not see this one before I sent it.
Here are my comments for the patch you submitted - You are not following the Kannel standards as for debugging messages - take a look at other examples for debug() calls in the code. - please dont use strcpy() or other c-string handling functions, as those are easy to break - especially as you are not doing any sanity checks in the input. use the octstr class functions instead. - the whole think of treating +CMTI as a completly different case then the other notifictions is not a good idea IMHO, the recursive call is not nice, but cannot be avoided. all in all I think the code is not as clean as it should be, and I would really like it to be well integrated with the sim-buffering functions which implement much of the needed functionality. my suggestion would be to break the sim-buffering functions into parts and use those for the relevant calls. it will also be eaiser to integrate cross memory type queries that way. -1 for this patch. -- Oded Arbel m-Wise mobile solutions [EMAIL PROTECTED] +972-9-9581711 (116) +972-67-340014 ::.. If glory comes after death, I'm not in a hurry. (Latin proverb) > -----Original Message----- > From: Rene Kluwen / Chimit Software Solutions > [mailto:[EMAIL PROTECTED]] > Sent: Wednesday, September 18, 2002 11:18 PM > To: [EMAIL PROTECTED] > Subject: CMTI-patch? > > > Hello devel, > > I heard the preffered method is submitting patches is cvs diff -u. > Even though I have a little bit of experience with both SMS as well > as WAP (stack) development, I am new to Kannel. > I must say, the code is very well modularized and also well > documented (a thank-you to the authors ;]). > > Below is a patch to have kannel respond to +CMTI, sms incoming > notifications. > For me, this was neccesary because my phone (Siemens SL45i) does not > send full pdu's to the TE, but rather sends indications at last. > > See below the patch. My question about it is: > Am I not breaking things with this? I realize that > at2_wait_modem_command is called cross-recursively with this patch. > Error status is not checked... Because in case of an error, there is > little that I can do about it (except maybe a log message). > > Also, maybe the return value of at2_wait_modem_command can be > erroneous. But I am still not that much into the code to see if that > is correct. Feedback is appreciated. For me, at least - so far it > works well. > > To test this patch, set init-string = "AT+CNMI=1,1,0,2,1" (the > second parameter has to be 1). > Also, sim-buffering is set to false at best, since both methods > could intervene each other. > > Any feedback? > > =================================================================== > RCS file: /home/cvs/gateway/gw/smsc/smsc_at2.c,v > retrieving revision 1.5 > diff -u -r1.5 smsc_at2.c > --- gw/smsc/smsc_at2.c 6 Sep 2002 09:35:09 -0000 1.5 > +++ gw/smsc/smsc_at2.c 18 Sep 2002 21:04:35 -0000 > @@ -493,6 +493,11 @@ > int len; > int cmgr_flag = 0; > > + // Chimit: we use cmd to request a message from store at > an incoming SMS indication > + // cmti_pos is going to hold the memory position > at which the message came in > + char cmd[20]; > + char cmti_pos[20]; > + > time(&end_time); > if (timeout == 0) > timeout = 3; > @@ -534,6 +539,24 @@ > ret = 1; > goto end; > } > + // Chimit: These 18 lines handle incoming message > indication for phones that do not > + // support full PDU notification, like > Siemens SL45i. > + // Todo: Some phones have messages come > in at another message store than > + // the preferred message store for > reading (ericsson, as I may recall?). > + // This is not handled in this code. > + if (octstr_search(line, octstr_imm("+CMTI:"), 0) != -1) { > + // we received an incoming message indication > + debug("smsc_at2.c", 0, "+CMTI incoming SMS > indication: %s", octstr_get_cstr(line)); > + // determine which memory position > + strcpy(cmti_pos, octstr_get_cstr(line) + 12); > + // now read the message > + sprintf(cmd, "AT+CMGR=%s", cmti_pos); > + at2_send_modem_command(privdata, cmd, 7, 0); > + // and delete it > + sprintf(cmd, "AT+CMGD=%s", cmti_pos); > + at2_send_modem_command(privdata, cmd, 7, 0); > + continue; > + } > if (octstr_search(line, octstr_imm("+CMT:"), 0) != -1 || > octstr_search(line, octstr_imm("+CDS:"), 0) != -1 || > ((octstr_search(line, octstr_imm("+CMGR:"), > 0) != -1) && (cmgr_flag = 1)) ) { > > > -- > Best regards, > Rene mailto:[EMAIL PROTECTED] > > >
