cinterion: improve ^SMONG response parser:

* Allow whitespaces prefixing the values row.
  * Allow more than one \r\n between the title and the table header.

Reported-by: Colin Helliwell <colin.helliwell@ln-systems.com>

plugins/cinterion/mm-broadband-modem-cinterion.c

@@ -618,98 +618,29 @@ load_access_technologies_finish (MMIfaceModem *self,
     return TRUE;
 }
 
-static MMModemAccessTechnology
-get_access_technology_from_smong_gprs_status (const gchar *gprs_status,
-                                              GError **error)
-{
-    if (strlen (gprs_status) == 1) {
-        switch (gprs_status[0]) {
-        case '0':
-            return MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN;
-        case '1':
-        case '2':
-            return MM_MODEM_ACCESS_TECHNOLOGY_GPRS;
-        case '3':
-        case '4':
-            return MM_MODEM_ACCESS_TECHNOLOGY_EDGE;
-        default:
-            break;
-        }
-    }
-
-    g_set_error (error,
-                 MM_CORE_ERROR,
-                 MM_CORE_ERROR_INVALID_ARGS,
-                 "Couldn't get network capabilities, "
-                 "invalid GPRS status value: '%s'",
-                 gprs_status);
-    return MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN;
-}
-
 static void
 smong_query_ready (MMBroadbandModemCinterion *self,
                    GAsyncResult *res,
                    GSimpleAsyncResult *operation_result)
 {
-    const gchar *response;
+    const gchar             *response;
-    GError *error = NULL;
+    GError                  *error = NULL;
-    GMatchInfo *match_info = NULL;
+    MMModemAccessTechnology  access_tech;
-    GRegex *regex;
 
     response = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, &error);
     if (!response) {
         /* Let the error be critical. */
         g_simple_async_result_take_error (operation_result, error);
-        g_simple_async_result_complete (operation_result);
+    } else if (!mm_cinterion_parse_smong_response (response, &access_tech, &error)) {
-        g_object_unref (operation_result);
-        return;
-    }
-
-    /* The AT^SMONG command returns a cell info table, where the second
-     * column identifies the "GPRS status", which is exactly what we want.
-     * So we'll try to read that second number in the values row.
-     *
-     * AT^SMONG
-     * GPRS Monitor
-     * BCCH  G  PBCCH  PAT MCC  MNC  NOM  TA      RAC    # Cell #
-     * 0776  1  -      -   214   03  2    00      01
-     * OK
-     */
-    regex = g_regex_new (".*GPRS Monitor\\r\\n"
-                         "BCCH\\s*G.*\\r\\n"
-                         "(\\d*)\\s*(\\d*)\\s*", 0, 0, NULL);
-    if (g_regex_match_full (regex, response, strlen (response), 0, 0, &match_info, NULL)) {
-        gchar *gprs_status;
-        MMModemAccessTechnology act;
-
-        gprs_status = g_match_info_fetch (match_info, 2);
-        act = get_access_technology_from_smong_gprs_status (gprs_status, &error);
-        g_free (gprs_status);
-
-        if (error)
-            g_simple_async_result_take_error (operation_result, error);
-        else {
-            /* We'll default to use SMONG then */
-            self->priv->sind_psinfo = FALSE;
-            g_simple_async_result_set_op_res_gpointer (operation_result,
-                                                       GUINT_TO_POINTER (act),
-                                                       NULL);
-        }
-    } else {
         /* We'll reset here the flag to try to use SIND/psinfo the next time */
         self->priv->sind_psinfo = TRUE;
-
+        g_simple_async_result_take_error (operation_result, error);
-        g_simple_async_result_set_error (operation_result,
+    } else {
-                                         MM_CORE_ERROR,
+        /* We'll default to use SMONG then */
-                                         MM_CORE_ERROR_INVALID_ARGS,
+        self->priv->sind_psinfo = FALSE;
-                                         "Couldn't get network capabilities, "
+        g_simple_async_result_set_op_res_gpointer (operation_result, GUINT_TO_POINTER (access_tech), NULL);
-                                         "invalid SMONG reply: '%s'",
-                                         response);
     }
 
-    g_match_info_free (match_info);
-    g_regex_unref (regex);
-
     g_simple_async_result_complete (operation_result);
     g_object_unref (operation_result);
 }

plugins/cinterion/mm-modem-helpers-cinterion.c

@@ -546,3 +546,80 @@ mm_cinterion_parse_swwan_response (const gchar  *response,
 
     return status;
 }
+
+/*****************************************************************************/
+/* ^SMONG response parser */
+
+static MMModemAccessTechnology
+get_access_technology_from_smong_gprs_status (guint    gprs_status,
+                                              GError **error)
+{
+    switch (gprs_status) {
+    case 0:
+        return MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN;
+    case 1:
+    case 2:
+        return MM_MODEM_ACCESS_TECHNOLOGY_GPRS;
+    case 3:
+    case 4:
+        return MM_MODEM_ACCESS_TECHNOLOGY_EDGE;
+    default:
+        break;
+    }
+
+    g_set_error (error,
+                 MM_CORE_ERROR,
+                 MM_CORE_ERROR_INVALID_ARGS,
+                 "Couldn't get network capabilities, "
+                 "unsupported GPRS status value: '%u'",
+                 gprs_status);
+    return MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN;
+}
+
+gboolean
+mm_cinterion_parse_smong_response (const gchar              *response,
+                                   MMModemAccessTechnology  *access_tech,
+                                   GError                  **error)
+{
+    GError     *inner_error = NULL;
+    GMatchInfo *match_info = NULL;
+    GRegex     *regex;
+
+    /* The AT^SMONG command returns a cell info table, where the second
+     * column identifies the "GPRS status", which is exactly what we want.
+     * So we'll try to read that second number in the values row.
+     *
+     * AT^SMONG
+     * GPRS Monitor
+     * BCCH  G  PBCCH  PAT MCC  MNC  NOM  TA      RAC    # Cell #
+     * 0776  1  -      -   214   03  2    00      01
+     * OK
+     */
+    regex = g_regex_new (".*GPRS Monitor(?:\r\n)*"
+                         "BCCH\\s*G.*\\r\\n"
+                         "\\s*(\\d+)\\s*(\\d+)\\s*",
+                         G_REGEX_DOLLAR_ENDONLY | G_REGEX_RAW,
+                         0, NULL);
+    g_assert (regex);
+
+    if (g_regex_match_full (regex, response, strlen (response), 0, 0, &match_info, &inner_error)) {
+        guint value = 0;
+
+        if (!mm_get_uint_from_match_info (match_info, 2, &value))
+            inner_error = g_error_new (MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                                       "Couldn't read 'GPRS status' field from AT^SMONG response");
+        else if (access_tech)
+            *access_tech = get_access_technology_from_smong_gprs_status (value, &inner_error);
+    }
+
+    g_match_info_free (match_info);
+    g_regex_unref (regex);
+
+    if (inner_error) {
+        g_propagate_error (error, inner_error);
+        return FALSE;
+    }
+
+    g_assert (access_tech != MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN);
+    return TRUE;
+}

plugins/cinterion/mm-modem-helpers-cinterion.h

@@ -20,6 +20,7 @@
 
 #include <glib.h>
 
+#include <ModemManager.h>
 #include <mm-base-bearer.h>
 
 /*****************************************************************************/
@@ -74,4 +75,11 @@ MMBearerConnectionStatus mm_cinterion_parse_swwan_response (const gchar  *respon
                                                             guint         swwan_index,
                                                             GError      **error);
 
+/*****************************************************************************/
+/* ^SMONG response parser */
+
+gboolean mm_cinterion_parse_smong_response (const gchar              *response,
+                                            MMModemAccessTechnology  *access_tech,
+                                            GError                  **error);
+
 #endif  /* MM_MODEM_HELPERS_CINTERION_H */

plugins/cinterion/tests/test-modem-helpers-cinterion.c

@@ -564,6 +564,47 @@ test_sind_response_simstatus (void)
     common_test_sind_response ("^SIND: simstatus,1,5", "simstatus", 1, 5);
 }
 
+/*****************************************************************************/
+/* Test ^SMONG responses */
+
+static void
+common_test_smong_response (const gchar             *response,
+                            MMModemAccessTechnology  expected_access_tech)
+{
+    GError                  *error = NULL;
+    gboolean                 res;
+    MMModemAccessTechnology  access_tech;
+
+    res = mm_cinterion_parse_smong_response (response, &access_tech, &error);
+    g_assert_no_error (error);
+    g_assert (res == TRUE);
+
+    g_assert_cmpuint (access_tech, ==, expected_access_tech);
+}
+
+static void
+test_smong_response_tc63i (void)
+{
+    const gchar *response =
+        "\r\n"
+        "GPRS Monitor\r\n"
+        "BCCH  G  PBCCH  PAT MCC  MNC  NOM  TA      RAC                               # Cell #\r\n"
+        "0073  1  -      -   262   02  2    00 01\r\n";
+    common_test_smong_response (response, MM_MODEM_ACCESS_TECHNOLOGY_GPRS);
+}
+
+static void
+test_smong_response_other (void)
+{
+    const gchar *response =
+        "\r\n"
+        "GPRS Monitor\r\n"
+        "\r\n"
+        "BCCH  G  PBCCH  PAT MCC  MNC  NOM  TA      RAC                              # Cell #\r\n"
+        "  44  1  -      -   234   10  -    -       -                                             \r\n";
+    common_test_smong_response (response, MM_MODEM_ACCESS_TECHNOLOGY_GPRS);
+}
+
 /*****************************************************************************/
 
 void
@@ -600,6 +641,8 @@ int main (int argc, char **argv)
     g_test_add_func ("/MM/cinterion/cnmi/other",              test_cnmi_other);
     g_test_add_func ("/MM/cinterion/swwan/pls8",              test_swwan_pls8);
     g_test_add_func ("/MM/cinterion/sind/response/simstatus", test_sind_response_simstatus);
+    g_test_add_func ("/MM/cinterion/smong/response/tc63i",    test_smong_response_tc63i);
+    g_test_add_func ("/MM/cinterion/smong/response/other",    test_smong_response_other);
 
     return g_test_run ();
 }