<div dir="ltr">Hi Aleksander and Dan,<div> </div><div>This patch is probably not the best solution, but incurs minimal changes. I hope there is no modem that puts leading zeros in integers and also doesn't quote hexadecimal strings.</div> <div> </div><div>Have you got any modem that put leading zeros in the <n>, <stat>, <AcT> fields? I wonder if the support for leading zeros can be scoped to the plugin level.</div><div> </div><div> Thanks,</div><div>Ben</div></div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Jul 31, 2013 at 11:07 PM, Ben Chan <<a href="mailto:benchan@chromium.org" target="_blank">benchan@chromium.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The format of CREG/CGREG/CEREG responses is not very precisely defined in or strictly enforced by the 3GPP specifications. That leads to the fact that some modems put leading zeros in integer type fields (e.g. <n>, <stat>, <AcT>), and not all modems put double quotes around string type fields (e.g. <lac>, <ci>) in those C*REG responses. For example, 0001 can be a valid value for both <stat> and <lac>. The original C*REG parsing code in ModemManager could potentially interpret '+CREG: <stat>,<lac>,<ci>,<AcT>' as '+CREG: <n>,<stat>,<lac>,<ci>'. This patch addresses this issue by refining the regular expressions returned by mm_3gpp_creg_regex_get() with the following assumptions: 1. If a modem puts leading zeros in integer type fields, it puts double quotes around string type fields. 2. If a modem omits double quotes around string type fields, it does not put leading zeros in integer type fields. --- src/mm-modem-helpers.c | 30 ++++++++++++--- src/tests/test-modem-helpers.c | 86 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 100 insertions(+), 16 deletions(-) diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c index cd4ef76..95d74dc 100644 --- a/src/mm-modem-helpers.c +++ b/src/mm-modem-helpers.c @@ -333,20 +333,22 @@ mm_filter_supported_capabilities (MMModemCapability all, #define CREG3 "\\+(CREG|CGREG|CEREG):\\s*0*([0-9]),\\s*([^,\\s]*)\\s*,\\s*([^,\\s]*)" /* +CREG: <n>,<stat>,<lac>,<ci> (GSM 07.07 solicited and some CREG=2 unsolicited) */ -#define CREG4 "\\+(CREG|CGREG|CEREG):\\s*0*([0-9]),\\s*0*([0-9])\\s*,\\s*([^,]*)\\s*,\\s*([^,\\s]*)" +#define CREG4 "\\+(CREG|CGREG|CEREG):\\s*([0-9]),\\s*([0-9])\\s*,\\s*([^,]*)\\s*,\\s*([^,\\s]*)" +#define CREG5 "\\+(CREG|CGREG|CEREG):\\s*0*([0-9]),\\s*0*([0-9])\\s*,\\s*(\"[^,]*\")\\s*,\\s*(\"[^,\\s]*\")" /* +CREG: <stat>,<lac>,<ci>,<AcT> (ETSI 27.007 CREG=2 unsolicited) */ -#define CREG5 "\\+(CREG|CGREG|CEREG):\\s*0*([0-9])\\s*,\\s*([^,\\s]*)\\s*,\\s*([^,\\s]*)\\s*,\\s*0*([0-9])" +#define CREG6 "\\+(CREG|CGREG|CEREG):\\s*([0-9])\\s*,\\s*([^,\\s]*)\\s*,\\s*([^,\\s]*)\\s*,\\s*([0-9])" +#define CREG7 "\\+(CREG|CGREG|CEREG):\\s*0*([0-9])\\s*,\\s*(\"[^,\\s]*\")\\s*,\\s*(\"[^,\\s]*\")\\s*,\\s*0*([0-9])" /* +CREG: <n>,<stat>,<lac>,<ci>,<AcT> (ETSI 27.007 solicited and some CREG=2 unsolicited) */ -#define CREG6 "\\+(CREG|CGREG|CEREG):\\s*0*([0-9]),\\s*0*([0-9])\\s*,\\s*([^,\\s]*)\\s*,\\s*([^,\\s]*)\\s*,\\s*0*([0-9])" +#define CREG8 "\\+(CREG|CGREG|CEREG):\\s*0*([0-9]),\\s*0*([0-9])\\s*,\\s*([^,\\s]*)\\s*,\\s*([^,\\s]*)\\s*,\\s*0*([0-9])" /* +CREG: <n>,<stat>,<lac>,<ci>,<AcT?>,<something> (Samsung Wave S8500) */ /* '<CR><LF>+CREG: 2,1,000B,2816, B, C2816<CR><LF><CR><LF>OK<CR><LF>' */ -#define CREG7 "\\+(CREG|CGREG):\\s*0*([0-9]),\\s*0*([0-9])\\s*,\\s*([^,\\s]*)\\s*,\\s*([^,\\s]*)\\s*,\\s*([^,\\s]*)\\s*,\\s*[^,\\s]*" +#define CREG9 "\\+(CREG|CGREG):\\s*0*([0-9]),\\s*0*([0-9])\\s*,\\s*([^,\\s]*)\\s*,\\s*([^,\\s]*)\\s*,\\s*([^,\\s]*)\\s*,\\s*[^,\\s]*" /* +CREG: <stat>,<lac>,<ci>,<AcT>,<RAC> (ETSI 27.007 v9.20 CREG=2 unsolicited with RAC) */ -#define CREG8 "\\+(CREG|CGREG):\\s*0*([0-9])\\s*,\\s*([^,\\s]*)\\s*,\\s*([^,\\s]*)\\s*,\\s*0*([0-9])\\s*,\\s*([^,\\s]*)" +#define CREG10 "\\+(CREG|CGREG):\\s*0*([0-9])\\s*,\\s*([^,\\s]*)\\s*,\\s*([^,\\s]*)\\s*,\\s*0*([0-9])\\s*,\\s*([^,\\s]*)" /* +CEREG: <stat>,<lac>,<rac>,<ci>,<AcT> (ETSI 27.007 v8.6 CREG=2 unsolicited with RAC) */ #define CEREG1 "\\+(CEREG):\\s*0*([0-9])\\s*,\\s*([^,\\s]*)\\s*,\\s*([^,\\s]*)\\s*,\\s*([^,\\s]*)\\s*,\\s*0*([0-9])" @@ -357,7 +359,7 @@ mm_filter_supported_capabilities (MMModemCapability all, GPtrArray * mm_3gpp_creg_regex_get (gboolean solicited) { - GPtrArray *array = g_ptr_array_sized_new (10); + GPtrArray *array = g_ptr_array_sized_new (12); GRegex *regex; /* #1 */ @@ -424,6 +426,22 @@ mm_3gpp_creg_regex_get (gboolean solicited) g_assert (regex); g_ptr_array_add (array, regex); + /* #9 */ + if (solicited) + regex = g_regex_new (CREG9 "$", G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL); + else + regex = g_regex_new ("\\r\\n" CREG9 "\\r\\n", G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL); + g_assert (regex); + g_ptr_array_add (array, regex); + + /* #10 */ + if (solicited) + regex = g_regex_new (CREG10 "$", G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL); + else + regex = g_regex_new ("\\r\\n" CREG10 "\\r\\n", G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL); + g_assert (regex); + g_ptr_array_add (array, regex); + /* CEREG #1 */ if (solicited) regex = g_regex_new (CEREG1 "$", G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL); diff --git a/src/tests/test-modem-helpers.c b/src/tests/test-modem-helpers.c index 5d94571..b8cd2d4 100644 --- a/src/tests/test-modem-helpers.c +++ b/src/tests/test-modem-helpers.c @@ -840,12 +840,52 @@ test_creg2_iridium_solicited (void *f, gpointer d) { RegTestData *data = (RegTestData *) d; const char *reply = "+CREG:002,001,\"18d8\",\"ffff\""; - const CregResult result = { 1, 0x18D8, 0xFFFF, MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN, 4, FALSE, FALSE }; + const CregResult result = { 1, 0x18D8, 0xFFFF, MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN, 5, FALSE, FALSE }; test_creg_match ("Iridium, CREG=2", TRUE, reply, data, &result); } static void +test_creg2_no_leading_zeros_solicited (void *f, gpointer d) +{ + RegTestData *data = (RegTestData *) d; + const char *reply = "+CREG:2,1,0001,0010"; + const CregResult result = { 1, 0x0001, 0x0010, MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN, 4, FALSE, FALSE }; + + test_creg_match ("solicited CREG=2 with no leading zeros in integer fields", TRUE, reply, data, &result); +} + +static void +test_creg2_leading_zeros_solicited (void *f, gpointer d) +{ + RegTestData *data = (RegTestData *) d; + const char *reply = "+CREG:002,001,\"0001\",\"0010\""; + const CregResult result = { 1, 0x0001, 0x0010, MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN, 5, FALSE, FALSE }; + + test_creg_match ("solicited CREG=2 with leading zeros in integer fields", TRUE, reply, data, &result); +} + +static void +test_creg2_no_leading_zeros_unsolicited (void *f, gpointer d) +{ + RegTestData *data = (RegTestData *) d; + const char *reply = "\r\n+CREG: 1,0001,0010,0\r\n"; + const CregResult result = { 1, 0x0001, 0x0010, MM_MODEM_ACCESS_TECHNOLOGY_GSM, 6, FALSE, FALSE }; + + test_creg_match ("unsolicited CREG=2 with no leading zeros in integer fields", FALSE, reply, data, &result); +} + +static void +test_creg2_leading_zeros_unsolicited (void *f, gpointer d) +{ + RegTestData *data = (RegTestData *) d; + const char *reply = "\r\n+CREG: 001,\"0001\",\"0010\",000\r\n"; + const CregResult result = { 1, 0x0001, 0x0010, MM_MODEM_ACCESS_TECHNOLOGY_GSM, 7, FALSE, FALSE }; + + test_creg_match ("unsolicited CREG=2 with leading zeros in integer fields", FALSE, reply, data, &result); +} + +static void test_cgreg1_solicited (void *f, gpointer d) { RegTestData *data = (RegTestData *) d; @@ -870,7 +910,7 @@ test_cgreg2_f3607gw_solicited (void *f, gpointer d) { RegTestData *data = (RegTestData *) d; const char *reply = "+CGREG: 2,1,\"8BE3\",\"00002B5D\",3"; - const CregResult result = { 1, 0x8BE3, 0x2B5D, MM_MODEM_ACCESS_TECHNOLOGY_EDGE , 6, TRUE, FALSE }; + const CregResult result = { 1, 0x8BE3, 0x2B5D, MM_MODEM_ACCESS_TECHNOLOGY_EDGE, 8, TRUE, FALSE }; test_creg_match ("Ericsson F3607gw CGREG=2", TRUE, reply, data, &result); } @@ -880,7 +920,7 @@ test_cgreg2_f3607gw_unsolicited (void *f, gpointer d) { RegTestData *data = (RegTestData *) d; const char *reply = "\r\n+CGREG: 1,\"8BE3\",\"00002B5D\",3\r\n"; - const CregResult result = { 1, 0x8BE3, 0x2B5D, MM_MODEM_ACCESS_TECHNOLOGY_EDGE , 5, TRUE, FALSE }; + const CregResult result = { 1, 0x8BE3, 0x2B5D, MM_MODEM_ACCESS_TECHNOLOGY_EDGE, 6, TRUE, FALSE }; test_creg_match ("Ericsson F3607gw CGREG=2", FALSE, reply, data, &result); } @@ -900,7 +940,7 @@ test_cgreg2_md400_unsolicited (void *f, gpointer d) { RegTestData *data = (RegTestData *) d; const char *reply = "\r\n+CGREG: 5,\"0502\",\"0404736D\",2\r\n"; - const CregResult result = { 5, 0x0502, 0x0404736D, MM_MODEM_ACCESS_TECHNOLOGY_UMTS, 5, TRUE, FALSE }; + const CregResult result = { 5, 0x0502, 0x0404736D, MM_MODEM_ACCESS_TECHNOLOGY_UMTS, 6, TRUE, FALSE }; test_creg_match ("Sony-Ericsson MD400 CGREG=2", FALSE, reply, data, &result); } @@ -941,7 +981,7 @@ test_creg2_s8500_wave_unsolicited (void *f, gpointer d) { RegTestData *data = (RegTestData *) d; const char *reply = "\r\n+CREG: 2,1,000B,2816, B, C2816\r\n"; - const CregResult result = { 1, 0x000B, 0x2816, MM_MODEM_ACCESS_TECHNOLOGY_GSM, 7, FALSE, FALSE }; + const CregResult result = { 1, 0x000B, 0x2816, MM_MODEM_ACCESS_TECHNOLOGY_GSM, 9, FALSE, FALSE }; test_creg_match ("Samsung Wave S8500 CREG=2", FALSE, reply, data, &result); } @@ -961,7 +1001,7 @@ test_cgreg2_unsolicited_with_rac (void *f, gpointer d) { RegTestData *data = (RegTestData *) d; const char *reply = "\r\n+CGREG: 1,\"1422\",\"00000142\",3,\"00\"\r\n"; - const CregResult result = { 1, 0x1422, 0x0142, MM_MODEM_ACCESS_TECHNOLOGY_EDGE, 8, TRUE, FALSE }; + const CregResult result = { 1, 0x1422, 0x0142, MM_MODEM_ACCESS_TECHNOLOGY_EDGE, 10, TRUE, FALSE }; test_creg_match ("CGREG=2 with RAC", FALSE, reply, data, &result); } @@ -991,7 +1031,7 @@ test_cereg2_solicited (void *f, gpointer d) { RegTestData *data = (RegTestData *) d; const char *reply = "\r\n+CEREG: 2,1, 1F00, 79D903 ,7\r\n"; - const CregResult result = { 1, 0x1F00, 0x79D903, MM_MODEM_ACCESS_TECHNOLOGY_LTE, 6, FALSE, TRUE }; + const CregResult result = { 1, 0x1F00, 0x79D903, MM_MODEM_ACCESS_TECHNOLOGY_LTE, 8, FALSE, TRUE }; test_creg_match ("CEREG=2", TRUE, reply, data, &result); } @@ -1001,17 +1041,37 @@ test_cereg2_unsolicited (void *f, gpointer d) { RegTestData *data = (RegTestData *) d; const char *reply = "\r\n+CEREG: 1, 1F00, 79D903 ,7\r\n"; - const CregResult result = { 1, 0x1F00, 0x79D903, MM_MODEM_ACCESS_TECHNOLOGY_LTE, 5, FALSE, TRUE }; + const CregResult result = { 1, 0x1F00, 0x79D903, MM_MODEM_ACCESS_TECHNOLOGY_LTE, 6, FALSE, TRUE }; test_creg_match ("CEREG=2", FALSE, reply, data, &result); } static void +test_cereg2_altair_lte_solicited (void *f, gpointer d) +{ + RegTestData *data = (RegTestData *) d; + const char *reply = "\r\n+CEREG: 1, 2, 0001, 00000100, 7\r\n"; + const CregResult result = { 2, 0x0001, 0x00000100, MM_MODEM_ACCESS_TECHNOLOGY_LTE, 8, FALSE, TRUE }; + + test_creg_match ("Altair LTE CEREG=2", FALSE, reply, data, &result); +} + +static void +test_cereg2_altair_lte_unsolicited (void *f, gpointer d) +{ + RegTestData *data = (RegTestData *) d; + const char *reply = "\r\n+CEREG: 2, 0001, 00000100, 7\r\n"; + const CregResult result = { 2, 0x0001, 0x00000100, MM_MODEM_ACCESS_TECHNOLOGY_LTE, 6, FALSE, TRUE }; + + test_creg_match ("Altair LTE CEREG=2", FALSE, reply, data, &result); +} + +static void test_cereg2_novatel_lte_solicited (void *f, gpointer d) { RegTestData *data = (RegTestData *) d; const char *reply = "\r\n+CEREG: 2,1, 1F00, 20 ,79D903 ,7\r\n"; - const CregResult result = { 1, 0x1F00, 0x79D903, MM_MODEM_ACCESS_TECHNOLOGY_LTE, 10, FALSE, TRUE }; + const CregResult result = { 1, 0x1F00, 0x79D903, MM_MODEM_ACCESS_TECHNOLOGY_LTE, 12, FALSE, TRUE }; test_creg_match ("Novatel LTE E362 CEREG=2", TRUE, reply, data, &result); } @@ -1021,7 +1081,7 @@ test_cereg2_novatel_lte_unsolicited (void *f, gpointer d) { RegTestData *data = (RegTestData *) d; const char *reply = "\r\n+CEREG: 1, 1F00, 20 ,79D903 ,7\r\n"; - const CregResult result = { 1, 0x1F00, 0x79D903, MM_MODEM_ACCESS_TECHNOLOGY_LTE, 9, FALSE, TRUE }; + const CregResult result = { 1, 0x1F00, 0x79D903, MM_MODEM_ACCESS_TECHNOLOGY_LTE, 11, FALSE, TRUE }; test_creg_match ("Novatel LTE E362 CEREG=2", FALSE, reply, data, &result); } @@ -2222,6 +2282,10 @@ int main (int argc, char **argv) g_test_suite_add (suite, TESTCASE (test_creg2_s8500_wave_unsolicited, reg_data)); g_test_suite_add (suite, TESTCASE (test_creg2_gobi_weird_solicited, reg_data)); g_test_suite_add (suite, TESTCASE (test_creg2_iridium_solicited, reg_data)); + g_test_suite_add (suite, TESTCASE (test_creg2_no_leading_zeros_solicited, reg_data)); + g_test_suite_add (suite, TESTCASE (test_creg2_leading_zeros_solicited, reg_data)); + g_test_suite_add (suite, TESTCASE (test_creg2_no_leading_zeros_unsolicited, reg_data)); + g_test_suite_add (suite, TESTCASE (test_creg2_leading_zeros_unsolicited, reg_data)); g_test_suite_add (suite, TESTCASE (test_cgreg1_solicited, reg_data)); g_test_suite_add (suite, TESTCASE (test_cgreg1_unsolicited, reg_data)); @@ -2235,6 +2299,8 @@ int main (int argc, char **argv) g_test_suite_add (suite, TESTCASE (test_cereg1_unsolicited, reg_data)); g_test_suite_add (suite, TESTCASE (test_cereg2_solicited, reg_data)); g_test_suite_add (suite, TESTCASE (test_cereg2_unsolicited, reg_data)); + g_test_suite_add (suite, TESTCASE (test_cereg2_altair_lte_solicited, reg_data)); + g_test_suite_add (suite, TESTCASE (test_cereg2_altair_lte_unsolicited, reg_data)); g_test_suite_add (suite, TESTCASE (test_cereg2_novatel_lte_solicited, reg_data)); g_test_suite_add (suite, TESTCASE (test_cereg2_novatel_lte_unsolicited, reg_data)); -- 1.8.3 </blockquote></div> </div>