http://bugzilla.openmoko.org/cgi-bin/bugzilla/show_bug.cgi?id=100 From baf5dafacf0f037cd502dcebf3d0b4f807062f48 From: Andrzej Zaborowski Date: Fri, 27 Jul 2007 14:29:44 +0200 Subject: [PATCH] Multiline unsolicited responses parsing. +CMT, +CBM and +CDS responses two lines long in PDU mode and the parser has problem with them. As it was mentioned earlier there's no way for the parser to know if a new line is part of the most recent command response or unsolicited response, or a completely new unsolicited response, without looking at the contents of the first line. My idea is that the parser for the particular response decides if the response is too short, and returns -EAGAIN in which case the AT parser will append the following line to the response and resubmit it to the response parser. The disadvantage is that the generic parser gets one more special case to remember about, but it works quite well, hopefully there are no more cases where we will need to modify the parser. (We may need to use a separate multiline buffer (like the current mlbuf variable) to account for situations when a multiline unsolicited response comes in the middle of receiving a different response, which is also possible. Also, mlbuf should probably be moved into the parser state struct instead of being a global). This patch also moves initialisation of SMS options to after we are registered to the network, so that we don't get errors from the modem. Perhaps we want to have a dynamic list of callbacks to call on succesfull registration. Among other changes also prevents setting dev_state.on if the radio powering-on fails. --- include/gsmd/event.h | 8 +++++ include/gsmd/sms.h | 1 + src/gsmd/atcmd.c | 26 +++++++++++++++++ src/gsmd/sms_cb.c | 73 ++++++++++++++++++++++++------------------------ src/gsmd/unsolicited.c | 8 +++++ src/gsmd/usock.c | 21 ++++++++++++-- src/util/event.c | 10 +++---- 7 files changed, 103 insertions(+), 44 deletions(-) Index: gsm/include/gsmd/event.h =================================================================== --- gsm.orig/include/gsmd/event.h 2007-08-15 00:33:00.000000000 +0800 +++ gsm/include/gsmd/event.h 2007-08-15 00:33:33.000000000 +0800 @@ -53,8 +53,14 @@ __NUM_GSMD_CALL }; +/* Chapter 7.2 */ enum gsmd_netreg_state { - GSMD_NETREG_NONE = 0, + GSMD_NETREG_UNREG = 0, + GSMD_NETREG_REG_HOME, + GSMD_NETREG_UNREG_BUSY, + GSMD_NETREG_DENIED, + GSMD_NETREG_UNKNOWN, + GSMD_NETREG_REG_ROAMING, __NUM_GSMD_NETREG }; Index: gsm/include/gsmd/sms.h =================================================================== --- gsm.orig/include/gsmd/sms.h 2007-08-15 00:33:00.000000000 +0800 +++ gsm/include/gsmd/sms.h 2007-08-15 00:33:33.000000000 +0800 @@ -6,6 +6,7 @@ #include int sms_cb_init(struct gsmd *gsmd); +int sms_cb_network_init(struct gsmd *gsmd); #define MAX_PDU_SIZE 180 int sms_pdu_make_smssubmit(char *dest, const struct gsmd_sms_submit *src); Index: gsm/src/gsmd/atcmd.c =================================================================== --- gsm.orig/src/gsmd/atcmd.c 2007-08-15 00:31:08.000000000 +0800 +++ gsm/src/gsmd/atcmd.c 2007-08-15 00:33:33.000000000 +0800 @@ -197,6 +197,7 @@ static char mlbuf[MLPARSE_BUF_SIZE]; int rc = 0; static int mlbuf_len; + static int mlunsolicited = 0; int cme_error = 0; DEBUGP("buf=`%s'(%d)\n", buf, len); @@ -272,6 +273,16 @@ if (colon > buf+len) colon = NULL; rc = unsolicited_parse(g, buf, len, colon); + if (rc == -EAGAIN) { + /* The parser wants one more line of + * input. Wait for the next line, concatenate + * and resumbit to unsolicited_parse(). */ + DEBUGP("Multiline unsolicited code\n"); + mlbuf_len = len; + memcpy(mlbuf, buf, len); + mlunsolicited = 1; + return 0; + } /* if unsolicited parser didn't handle this 'reply', then we * need to continue and try harder and see what it is */ if (rc != -ENOENT) { @@ -356,6 +367,21 @@ len = sizeof(mlbuf) - mlbuf_len; memcpy(mlbuf + mlbuf_len, buf, len); mlbuf_len += len; + + if (mlunsolicited) { + rc = unsolicited_parse(g, mlbuf, mlbuf_len, + strchr(mlbuf, ':') + 1); + if (rc == -EAGAIN) { + /* The parser wants one more line of + * input. Wait for the next line, concatenate + * and resumbit to unsolicited_parse(). */ + DEBUGP("Multiline unsolicited code\n"); + return 0; + } + mlunsolicited = 0; + mlbuf_len = 0; + return rc; + } return 0; final_cb: Index: gsm/src/gsmd/sms_cb.c =================================================================== --- gsm.orig/src/gsmd/sms_cb.c 2007-08-15 00:33:26.000000000 +0800 +++ gsm/src/gsmd/sms_cb.c 2007-08-15 00:33:33.000000000 +0800 @@ -498,7 +498,7 @@ len = strtoul(comma + 1, &cr, 10); if (cr[0] != '\n') { talloc_free(ucmd); - return -EINVAL; + return -EAGAIN; } cr ++; @@ -567,8 +567,10 @@ msg = (struct gsmd_sms_list *) aux->data; len = strtoul(param, &cr, 10); - if (cr[0] != '\n') - return -EINVAL; + if (cr[0] != '\n') { + talloc_free(ucmd); + return -EAGAIN; + } cr ++; for (i = 0; cr[0] >= '0' && cr[1] >= '0' && i < MAX_PDU_SIZE; i ++) { @@ -631,8 +633,10 @@ msg = (struct gsmd_sms_list *) aux->data; len = strtoul(param, &cr, 10); - if (cr[0] != '\n') - return -EINVAL; + if (cr[0] != '\n') { + talloc_free(ucmd); + return -EAGAIN; + } cr ++; for (i = 0; cr[0] >= '0' && cr[1] >= '0' && i < MAX_PDU_SIZE; i ++) { @@ -671,37 +675,6 @@ unsolicited_register_array(gsm0705_unsolicit, ARRAY_SIZE(gsm0705_unsolicit)); - atcmd = atcmd_fill("AT+CSMS=0", 9 + 1, NULL, gsmd, 0); - if (!atcmd) - return -ENOMEM; - atcmd_submit(gsmd, atcmd); - - /* - * Set the New Message Indications properties to values that are - * likely supported. We will get a: - * +CMTI on a new incoming SMS, - * +CBM on a new incoming CB, - * +CDS on an SMS status report. - * - * FIXME: ask for supported +CNMI values first. - */ - atcmd = atcmd_fill("AT+CNMI=2,1,2,1,0", 17 + 1, NULL, gsmd, 0); - if (!atcmd) - return -ENOMEM; - atcmd_submit(gsmd, atcmd); - - /* Store into ME/TA and notify */ - atcmd = atcmd_fill("AT+CSBS=1", 9 + 1, NULL, gsmd, 0); - if (!atcmd) - return -ENOMEM; - atcmd_submit(gsmd, atcmd); - - /* Store into ME/TA and notify */ - atcmd = atcmd_fill("AT+CSDS=2", 9 + 1, NULL, gsmd, 0); - if (!atcmd) - return -ENOMEM; - atcmd_submit(gsmd, atcmd); - /* If text mode, set the encoding */ if (gsmd->flags & GSMD_FLAG_SMS_FMT_TEXT) { atcmd = atcmd_fill("AT+CSCS=\"IRA\"", 13 + 1, NULL, gsmd, 0); @@ -721,3 +694,31 @@ return atcmd_submit(gsmd, atcmd); } + +/* Called everytime the phone registers to the network and we want to start + * receiving messages. */ +int sms_cb_network_init(struct gsmd *gsmd) +{ + int ret = 0; + + ret |= gsmd_simplecmd(gsmd, "AT+CSMS=0"); + + /* + * Set the New Message Indications properties to values that are + * likely supported. We will get a: + * +CMTI on a new incoming SMS, + * +CBM on a new incoming CB, + * +CDS on an SMS status report. + * + * FIXME: ask for supported +CNMI values first. + */ + ret |= gsmd_simplecmd(gsmd, "AT+CNMI=2,1,2,1,0"); + + /* Store into ME/TA and notify */ + ret |= gsmd_simplecmd(gsmd, "AT+CSBS=1"); + + /* Store into ME/TA and notify */ + ret |= gsmd_simplecmd(gsmd, "AT+CSDS=2"); + + return ret; +} Index: gsm/src/gsmd/unsolicited.c =================================================================== --- gsm.orig/src/gsmd/unsolicited.c 2007-08-15 00:33:00.000000000 +0800 +++ gsm/src/gsmd/unsolicited.c 2007-08-15 00:33:33.000000000 +0800 @@ -163,6 +163,12 @@ } else aux->u.netreg.lac = aux->u.netreg.ci = 0; + /* Intialise things that depend on network registration */ + if (aux->u.netreg.state == GSMD_NETREG_REG_HOME || + aux->u.netreg.state == GSMD_NETREG_REG_ROAMING) { + sms_cb_network_init(gsmd); + } + return usock_evt_send(gsmd, ucmd, GSMD_EVT_NETREG); } @@ -378,6 +384,8 @@ colon = NULL; rc = i->parse(buf, len, colon, g); + if (rc == -EAGAIN) + return rc; if (rc < 0) gsmd_log(GSMD_ERROR, "error %d during parsing of " "an unsolicied response `%s'\n", Index: gsm/src/gsmd/usock.c =================================================================== --- gsm.orig/src/gsmd/usock.c 2007-08-15 00:33:00.000000000 +0800 +++ gsm/src/gsmd/usock.c 2007-08-15 00:33:33.000000000 +0800 @@ -239,6 +239,24 @@ return atcmd_submit(gu->gsmd, cmd); } +static int phone_powerup_cb(struct gsmd_atcmd *cmd, void *ctx, char *resp) +{ + struct gsmd_user *gu = ctx; + + /* We need to verify if there is some error */ + switch (cmd->ret) { + case 0: + gsmd_log(GSMD_DEBUG, "Radio powered-up\n"); + gu->gsmd->dev_state.on = 1; + break; + default: + /* something went wrong */ + gsmd_log(GSMD_DEBUG, "Radio power-up failed\n"); + break; + } + return 0; +} + static int usock_rcv_phone(struct gsmd_user *gu, struct gsmd_msg_hdr *gph, int len) { @@ -247,8 +265,7 @@ switch (gph->msg_subtype) { case GSMD_PHONE_POWERUP: cmd = atcmd_fill("AT+CFUN=1", 9+1, - &null_cmd_cb, gu, 0); - gu->gsmd->dev_state.on = 1; + &phone_powerup_cb, gu, 0); break; case GSMD_PHONE_POWERDOWN: Index: gsm/src/util/event.c =================================================================== --- gsm.orig/src/util/event.c 2007-08-15 00:31:46.000000000 +0800 +++ gsm/src/util/event.c 2007-08-15 00:33:33.000000000 +0800 @@ -60,19 +60,19 @@ printf("EVENT: Netreg "); switch (aux->u.netreg.state) { - case 0: + case GSMD_NETREG_UNREG: printf("not searching for network "); break; - case 1: + case GSMD_NETREG_REG_HOME: printf("registered (home network) "); break; - case 2: + case GSMD_NETREG_UNREG_BUSY: printf("searching for network "); break; - case 3: + case GSMD_NETREG_DENIED: printf("registration denied "); break; - case 5: + case GSMD_NETREG_REG_ROAMING: printf("registered (roaming) "); break; }