[PATCH] modem-helpers: fix parsing of CREG/CGREG/CEREG responses

Ben Chan benchan at chromium.org
Wed Jul 31 23:48:07 PDT 2013


Hi Aleksander and Dan,

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.

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.

Thanks,
Ben


On Wed, Jul 31, 2013 at 11:07 PM, Ben Chan <benchan at chromium.org> wrote:

> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/modemmanager-devel/attachments/20130731/ad32ef29/attachment-0001.html>


More information about the ModemManager-devel mailing list