A proposal for quicker reading of the received SMS messages in smsc_at
Andrija Petrovic <[email protected]>
| Newsgroups | gmane.comp.mobile.kannel.devel |
|---|---|
| Message-ID | <[email protected]> |
Please, consider my proposal for enhancing the smsc_at functionality: The method currently implemented for reading SMSes from memory is way too slow and inefficient, compared to usage of the standard CMGL command. What at2_read_sms_memory tries to achieve in the current version is just what AT+CMGL command already achieves - in much less time. Therefore, instead of slowly traversing through the memory in search for present SMSes, AT+CMGL command is issued. It responds in almost identical format as AT+CMGR, with just an extra parameter - the index of the actual SMS being read. There are as many response lines as there are SMSes in memory. During the AT+CMGL response, a private List of SMS indices is being built, so those SMSes can be deleted once the AT+CMGL response is over. Moreover, there is no need for at2_check_memory, nor for the private fields sms_memory_capacity and sms_memory_usage in the PrivAT2data. It should be noted that once this patch is applied, the smsc_at becomes quite useful in a very safe use case: AT+CNMI is not issued, so there is no concern whether the SMS modem will respond to the incoming SMS notification (AT+CMTx) in a timely fashion. Instead of that, sim-buffering is set to true, with a 1 second keepalive interval. The use case described above has but one drawback: logs tend to become rather massive because of numerous DEBUG lines coming from communication with the SMS modem. Implementation of this use case with the current version of smsc_at brought us to delays in SMS responses of up to 18 minutes. With the improvement we propose here, the SMS response delay depends only on the GSM network throughput (6 SMS/minute max, AFAIK). Regards, Andrija
smsc_at.c.31.diff
(text/plain, 10.8 KB)
? gateway/gw/smsc/.smsc_at.c.swp
Index: gateway/gw/smsc/smsc_at.c
===================================================================
RCS file: /home/cvs/gateway/gw/smsc/smsc_at.c,v
retrieving revision 1.31
diff -u -b -B -a -u -r1.31 smsc_at.c
--- gateway/gw/smsc/smsc_at.c 23 May 2006 13:27:31 -0000 1.31
+++ gateway/gw/smsc/smsc_at.c 27 May 2006 11:31:54 -0000
@@ -621,7 +621,7 @@
time_t cur_time;
Msg *msg;
int len;
- int cmgr_flag = 0;
+ int cmgr_flag = 0, cmgl_flag=0;
time(&end_time);
if (timeout == 0)
@@ -683,11 +683,12 @@
}
if (octstr_search(line, octstr_imm("+CMT:"), 0) != -1 ||
octstr_search(line, octstr_imm("+CDS:"), 0) != -1 ||
+ ((octstr_search(line, octstr_imm("+CMGL:"), 0) != -1) && (cmgl_flag = 1)) ||
((octstr_search(line, octstr_imm("+CMGR:"), 0) != -1) && (cmgr_flag = 1)) ) {
- line2 = at2_wait_line(privdata, 1, 0);
+ line2 = at2_wait_line(privdata, 30, 0);
if (line2 == NULL) {
- error(0, "AT2[%s]: got +CMT but waiting for next line timed out",
+ error(0, "AT2[%s]: got an incoming message but waiting for next line timed out",
octstr_get_cstr(privdata->name));
} else {
octstr_append_cstr(line, "\n");
@@ -695,7 +696,7 @@
O_DESTROY(line2);
at2_pdu_extract(privdata, &pdu, line, smsc_number);
if (pdu == NULL) {
- error(0, "AT2[%s]: got +CMT but pdu_extract failed",
+ error(0, "AT2[%s]: got an incoming message but pdu_extract failed",
octstr_get_cstr(privdata->name));
} else {
/* count message even if I can't decode it */
@@ -711,7 +712,7 @@
octstr_get_cstr(privdata->name));
}
- if (!cmgr_flag) {
+ if (!(cmgr_flag|cmgl_flag)) {
if (privdata->phase2plus)
at2_write_line(privdata, "AT+CNMA");
}
@@ -719,6 +720,8 @@
O_DESTROY(pdu);
}
}
+ time(&end_time);
+ end_time += 3;
continue;
}
if ((octstr_search(line, octstr_imm("+CMGS:"),0) != -1) && (output)) {
@@ -900,135 +903,23 @@
static int at2_read_sms_memory(PrivAT2data* privdata)
{
- /* get memory status */
- if (at2_check_sms_memory(privdata) == -1) {
- debug("bb.smsc.at2", 0, "AT2[%s]: memory check error", octstr_get_cstr(privdata->name));
- return -1;
- }
-
- if (privdata->sms_memory_usage) {
- /*
- * that is - greater then 0, meaning there are some messages to fetch
- * now - I used to just loop over the first input_mem_sms_used locations,
- * but it doesn't hold, since under load, messages may be received while
- * we're in the loop, and get stored in locations towards the end of the list,
- * thus creating 'holes' in the memory.
- *
- * There are two ways we can fix this :
- * (a) Just read the last message location, delete it and return.
- * It's not a complete solution since holes can still be created if messages
- * are received between the memory check and the delete command,
- * and anyway - it will slow us down and won't hold well under pressure
- * (b) Just scan the entire memory each call, bottom to top.
- * This will be slow too, but it'll be reliable.
- *
- * We can massivly improve performance by stopping after input_mem_sms_used messages
- * have been read, but send_modem_command returns 0 for no message as well as for a
- * message read, and the only other way to implement it is by doing memory_check
- * after each read and stoping when input_mem_sms_used get to 0. This is slow
- * (modem commands take time) so we improve speed only if there are less then 10
- * messages in memory.
- *
- * I implemented the alternative - changed at2_wait_modem_command to return the
- * number of messages it collected.
- */
- int i;
- int message_count = 0; /* cound number of messages collected */
-
- debug("bb.smsc.at2", 0, "AT2[%s]: %d messages waiting in memory",
- octstr_get_cstr(privdata->name), privdata->sms_memory_usage);
-
- /*
- * loop till end of memory or collected enouch messages
- */
- for (i = 1; i <= privdata->sms_memory_capacity &&
- message_count < privdata->sms_memory_usage; ++i) {
-
- /* if (meanwhile) there are pending CMTI notifications, process these first
- * to not let CMTI and sim buffering sit in each others way */
- while (gwlist_len(privdata->pending_incoming_messages) > 0) {
- at2_read_pending_incoming_messages(privdata);
- }
- /* read the message and delete it */
- message_count += at2_read_delete_message(privdata, i);
+ at2_send_modem_command(privdata,"AT+CMGL=0",0,0);
+ while(gwlist_len(privdata->smses_for_deletion))
+ {
+ char command[40]={0};
+ sprintf(command,"AT+CMGD=%ld",(long)gwlist_extract_first(privdata->smses_for_deletion));
+ at2_send_modem_command(privdata,command,7,0);
}
+ at2_send_modem_command(privdata,"AT+CMGL=1",0,0);
+ while(gwlist_len(privdata->smses_for_deletion))
+ {
+ char command[40]={0};
+ sprintf(command,"AT+CMGD=%ld",(long)gwlist_extract_first(privdata->smses_for_deletion));
+ at2_send_modem_command(privdata,command,7,0);
}
-
- /*
- at2_send_modem_command(privdata, ModemTypes[privdata->modemid].init1, 0, 0);
- */
return 0;
}
-
-static int at2_check_sms_memory(PrivAT2data *privdata)
-{
- long values[4]; /* array to put response data in */
- int pos; /* position of parser in data stream */
- int ret;
- Octstr *search_cpms = NULL;
-
- /* select memory type and get report */
- if ((ret = at2_send_modem_command(privdata, "AT+CPMS?", 0, 0)) != 0) {
- debug("bb.smsc.at2.memory_check", 0, "failed to send mem select command to modem %d", ret);
- return -1;
- }
-
- search_cpms = octstr_create("+CPMS:");
-
- if ((pos = octstr_search(privdata->lines, search_cpms, 0)) != -1) {
- /* got back a +CPMS response */
- int index = 0; /* index in values array */
- pos += 6; /* position of parser in the stream - start after header */
-
- /* skip memory indication */
- pos = octstr_search(privdata->lines, octstr_imm(","), pos) + 1;
-
- /* find all the values */
- while (index < 4 && pos < octstr_len(privdata->lines) &&
- (pos = octstr_parse_long(&values[index], privdata->lines, pos, 10)) != -1) {
- ++pos; /* skip number seperator */
- ++index; /* increment array index */
- if (index == 2)
- /* skip second memory indication */
- pos = octstr_search(privdata->lines, octstr_imm(","), pos) + 1;
- }
-
- if (index < 4) {
- /* didn't get all memory data - I don't why, so I'll bail */
- debug("bb.smsc.at2", 0, "AT2[%s]: couldn't parse all memory locations : %d:'%s'.",
- octstr_get_cstr(privdata->name), index,
- &(octstr_get_cstr(privdata->lines)[pos]));
- O_DESTROY(search_cpms);
- return -1;
- }
-
- privdata->sms_memory_usage = values[0];
- privdata->sms_memory_capacity = values[1];
- /*
- privdata->output_mem_sms_used = values[2];
- privdata->output_mem_sms_capacity = values[3];
- */
-
- /* everything's cool */
- ret = 0;
-
- /* clear the buffer */
- O_DESTROY(privdata->lines);
-
- } else {
- debug("bb.smsc.at2", 0, "AT2[%s]: no correct header for CPMS response.",
- octstr_get_cstr(privdata->name));
-
- /* didn't get a +CPMS response - this is clearly an error */
- ret = -1;
- }
-
- O_DESTROY(search_cpms);
- return ret;
-}
-
-
static void at2_set_speed(PrivAT2data *privdata, int bps)
{
struct termios tios;
@@ -1226,6 +1117,7 @@
octstr_destroy(privdata->configfile);
gw_prioqueue_destroy(privdata->outgoing_queue, NULL);
gwlist_destroy(privdata->pending_incoming_messages, octstr_destroy_item);
+ gwlist_destroy(privdata->smses_for_deletion, NULL);
gw_free(conn->data);
conn->data = NULL;
mutex_lock(conn->flow_mutex);
@@ -1315,6 +1207,7 @@
privdata = gw_malloc(sizeof(PrivAT2data));
privdata->outgoing_queue = gw_prioqueue_create(sms_priority_compare);
privdata->pending_incoming_messages = gwlist_create();
+ privdata->smses_for_deletion = gwlist_create();
privdata->configfile = cfg_get_configfile(cfg);
@@ -1459,6 +1352,19 @@
pos++;
else
goto nomsg;
+ } else if ((pos = octstr_search(buffer, octstr_imm("+CMGL:"), 0)) != -1) {
+ long index;
+ pos = octstr_parse_long(&index, buffer, pos+6, 10);
+ if(pos == -1) {
+ error(0, "AT2[%s]: Failed to extract index from CMGL response",octstr_get_cstr(privdata->name));
+ goto nomsg;
+ }
+ gwlist_append(privdata->smses_for_deletion,(void*)index);
+ /* skip status field in +CMGL response */
+ if ((pos = octstr_search(buffer, octstr_imm(","), pos + 1)) != -1)
+ pos++;
+ else
+ goto nomsg;
} else
goto nomsg;
Index: gateway/gw/smsc/smsc_at.h
===================================================================
RCS file: /home/cvs/gateway/gw/smsc/smsc_at.h,v
retrieving revision 1.11
diff -u -b -B -a -u -r1.11 smsc_at.h
--- gateway/gw/smsc/smsc_at.h 24 Apr 2006 21:32:01 -0000 1.11
+++ gateway/gw/smsc/smsc_at.h 27 May 2006 11:31:54 -0000
@@ -136,13 +136,12 @@
Octstr *name;
Octstr *configfile;
int sms_memory_poll_interval;
- int sms_memory_capacity;
- int sms_memory_usage;
List *pending_incoming_messages;
int max_error_count;
Octstr *rawtcp_host;
int rawtcp_port;
int is_serial; /* false if device is rawtcp */
+ List *smses_for_deletion;
} PrivAT2data;
@@ -347,11 +346,6 @@
static int at2_read_sms_memory(PrivAT2data *privdata);
/*
- * Memory capacity and usage check
- */
-static int at2_check_sms_memory(PrivAT2data *privdata);
-
-/*
* This silly thing here will just translate a "swapped nibble"
* pseodo Hex encoding (from PDU) into something that people can
* actually understand.