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
? 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.
