huawei: fix ^SYSINFO parsing

The original sysinfo_parse() in MMBroadbandModemHuawei incorrectly sets
'out_sys_submode_valid' to TRUE even when <sys_submode> is not present
in a ^SYSINFO response. This patch moves the code to
mm-modem-helpers-huawei.c, fixes the regex for parsing ^SYSINFO
responses, and adds unit tests.

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

@@ -113,70 +113,6 @@ struct _MMBroadbandModemHuaweiPrivate {
 
 /*****************************************************************************/
 
-static gboolean
-sysinfo_parse (const char *reply,
-               guint *out_srv_status,
-               guint *out_srv_domain,
-               guint *out_roam_status,
-               guint *out_sys_mode,
-               guint *out_sim_state,
-               gboolean *out_sys_submode_valid,
-               guint *out_sys_submode,
-               GError **error)
-{
-    gboolean matched;
-    GRegex *r;
-    GMatchInfo *match_info = NULL;
-    GError *match_error = NULL;
-
-    g_assert (out_srv_status != NULL);
-    g_assert (out_srv_domain != NULL);
-    g_assert (out_roam_status != NULL);
-    g_assert (out_sys_mode != NULL);
-    g_assert (out_sim_state != NULL);
-    g_assert (out_sys_submode_valid != NULL);
-    g_assert (out_sys_submode != NULL);
-
-    /* Format:
-     *
-     * ^SYSINFO: <srv_status>,<srv_domain>,<roam_status>,<sys_mode>,<sim_state>[,<reserved>,<sys_submode>]
-     */
-
-    /* Can't just use \d here since sometimes you get "^SYSINFO:2,1,0,3,1,,3" */
-    r = g_regex_new ("\\^SYSINFO:\\s*(\\d+),(\\d+),(\\d+),(\\d+),(\\d+),?(\\d*),?(\\d*)$", 0, 0, NULL);
-    g_assert (r != NULL);
-
-    matched = g_regex_match_full (r, reply, -1, 0, 0, &match_info, &match_error);
-    if (!matched) {
-        if (match_error) {
-            g_propagate_error (error, match_error);
-            g_prefix_error (error, "Could not parse ^SYSINFO results: ");
-        } else {
-            g_set_error_literal (error,
-                                 MM_CORE_ERROR,
-                                 MM_CORE_ERROR_FAILED,
-                                 "Couldn't match ^SYSINFO reply");
-        }
-    } else {
-        mm_get_uint_from_match_info (match_info, 1, out_srv_status);
-        mm_get_uint_from_match_info (match_info, 2, out_srv_domain);
-        mm_get_uint_from_match_info (match_info, 3, out_roam_status);
-        mm_get_uint_from_match_info (match_info, 4, out_sys_mode);
-        mm_get_uint_from_match_info (match_info, 5, out_sim_state);
-
-        /* Remember that g_match_info_get_match_count() includes match #0 */
-        if (g_match_info_get_match_count (match_info) >= 8) {
-            *out_sys_submode_valid = TRUE;
-            mm_get_uint_from_match_info (match_info, 7, out_sys_submode);
-        }
-    }
-
-    if (match_info)
-        g_match_info_free (match_info);
-    g_regex_unref (r);
-    return matched;
-}
-
 static gboolean
 sysinfoex_parse (const char *reply,
                  guint *out_srv_status,
@@ -308,15 +244,15 @@ run_sysinfo_ready (MMBaseModem *self,
 
     result = g_new0 (SysinfoResult, 1);
     result->extended = FALSE;
-    if (!sysinfo_parse (response,
+    if (!mm_huawei_parse_sysinfo_response (response,
-                          &result->srv_status,
+                                           &result->srv_status,
-                          &result->srv_domain,
+                                           &result->srv_domain,
-                          &result->roam_status,
+                                           &result->roam_status,
-                          &result->sys_mode,
+                                           &result->sys_mode,
-                          &result->sim_state,
+                                           &result->sim_state,
-                          &result->sys_submode_valid,
+                                           &result->sys_submode_valid,
-                          &result->sys_submode,
+                                           &result->sys_submode,
-                          &error)) {
+                                           &error)) {
         mm_dbg ("^SYSINFO parsing failed: %s", error->message);
         g_simple_async_result_take_error (simple, error);
         g_simple_async_result_complete (simple);

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

@@ -112,3 +112,70 @@ mm_huawei_parse_ndisstatqry_response (const gchar *response,
 
     return TRUE;
 }
+
+/*****************************************************************************/
+/* ^SYSINFO response parser */
+
+gboolean
+mm_huawei_parse_sysinfo_response (const char *reply,
+                                  guint *out_srv_status,
+                                  guint *out_srv_domain,
+                                  guint *out_roam_status,
+                                  guint *out_sys_mode,
+                                  guint *out_sim_state,
+                                  gboolean *out_sys_submode_valid,
+                                  guint *out_sys_submode,
+                                  GError **error)
+{
+    gboolean matched;
+    GRegex *r;
+    GMatchInfo *match_info = NULL;
+    GError *match_error = NULL;
+
+    g_assert (out_srv_status != NULL);
+    g_assert (out_srv_domain != NULL);
+    g_assert (out_roam_status != NULL);
+    g_assert (out_sys_mode != NULL);
+    g_assert (out_sim_state != NULL);
+    g_assert (out_sys_submode_valid != NULL);
+    g_assert (out_sys_submode != NULL);
+
+    /* Format:
+     *
+     * ^SYSINFO: <srv_status>,<srv_domain>,<roam_status>,<sys_mode>,<sim_state>[,<reserved>,<sys_submode>]
+     */
+
+    /* Can't just use \d here since sometimes you get "^SYSINFO:2,1,0,3,1,,3" */
+    r = g_regex_new ("\\^SYSINFO:\\s*(\\d+),(\\d+),(\\d+),(\\d+),(\\d+),?(\\d+)?,?(\\d+)?$", 0, 0, NULL);
+    g_assert (r != NULL);
+
+    matched = g_regex_match_full (r, reply, -1, 0, 0, &match_info, &match_error);
+    if (!matched) {
+        if (match_error) {
+            g_propagate_error (error, match_error);
+            g_prefix_error (error, "Could not parse ^SYSINFO results: ");
+        } else {
+            g_set_error_literal (error,
+                                 MM_CORE_ERROR,
+                                 MM_CORE_ERROR_FAILED,
+                                 "Couldn't match ^SYSINFO reply");
+        }
+    } else {
+        mm_get_uint_from_match_info (match_info, 1, out_srv_status);
+        mm_get_uint_from_match_info (match_info, 2, out_srv_domain);
+        mm_get_uint_from_match_info (match_info, 3, out_roam_status);
+        mm_get_uint_from_match_info (match_info, 4, out_sys_mode);
+        mm_get_uint_from_match_info (match_info, 5, out_sim_state);
+
+        /* Remember that g_match_info_get_match_count() includes match #0 */
+        if (g_match_info_get_match_count (match_info) >= 8) {
+            *out_sys_submode_valid = TRUE;
+            mm_get_uint_from_match_info (match_info, 7, out_sys_submode);
+        }
+    }
+
+    if (match_info)
+        g_match_info_free (match_info);
+    g_regex_unref (r);
+    return matched;
+}

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

@@ -27,4 +27,15 @@ gboolean mm_huawei_parse_ndisstatqry_response (const gchar *response,
                                                gboolean *ipv6_connected,
                                                GError **error);
 
+/* ^SYSINFO response parser */
+gboolean mm_huawei_parse_sysinfo_response (const char *reply,
+                                           guint *out_srv_status,
+                                           guint *out_srv_domain,
+                                           guint *out_roam_status,
+                                           guint *out_sys_mode,
+                                           guint *out_sim_state,
+                                           gboolean *out_sys_submode_valid,
+                                           guint *out_sys_submode,
+                                           GError **error);
+
 #endif  /* MM_MODEM_HELPERS_HUAWEI_H */

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

@@ -128,6 +128,69 @@ test_ndisstatqry (void)
     }
 }
 
+/*****************************************************************************/
+/* Test ^SYSINFO responses */
+
+typedef struct {
+    const gchar *str;
+    guint expected_srv_status;
+    guint expected_srv_domain;
+    guint expected_roam_status;
+    guint expected_sys_mode;
+    guint expected_sim_state;
+    gboolean expected_sys_submode_valid;
+    guint expected_sys_submode;
+} SysinfoTest;
+
+static const SysinfoTest sysinfo_tests[] = {
+    { "^SYSINFO:2,4,5,3,1",      2, 4, 5, 3, 1, FALSE, 0 },
+    { "^SYSINFO:2,4,5,3,1,",     2, 4, 5, 3, 1, FALSE, 0 },
+    { "^SYSINFO:2,4,5,3,1,,",    2, 4, 5, 3, 1, FALSE, 0 },
+    { "^SYSINFO:2,4,5,3,1,6",    2, 4, 5, 3, 1, FALSE, 6 },
+    { "^SYSINFO:2,4,5,3,1,6,",   2, 4, 5, 3, 1, FALSE, 6 },
+    { "^SYSINFO:2,4,5,3,1,,3",   2, 4, 5, 3, 1, TRUE,  3 },
+    { "^SYSINFO:2,4,5,3,1,0,3",  2, 4, 5, 3, 1, TRUE,  3 },
+    { "^SYSINFO: 2,4,5,3,1,0,3", 2, 4, 5, 3, 1, TRUE,  3 },
+    { NULL,                      0, 0, 0, 0, 0, FALSE, 0 }
+};
+
+static void
+test_sysinfo (void)
+{
+    guint i;
+
+    for (i = 0; sysinfo_tests[i].str; i++) {
+        GError *error = NULL;
+        guint srv_status = 0;
+        guint srv_domain = 0;
+        guint roam_status = 0;
+        guint sys_mode = 0;
+        guint sim_state = 0;
+        gboolean sys_submode_valid = FALSE;
+        guint sys_submode = 0;
+
+        g_assert (mm_huawei_parse_sysinfo_response (sysinfo_tests[i].str,
+                                                    &srv_status,
+                                                    &srv_domain,
+                                                    &roam_status,
+                                                    &sys_mode,
+                                                    &sim_state,
+                                                    &sys_submode_valid,
+                                                    &sys_submode,
+                                                    &error) == TRUE);
+        g_assert_no_error (error);
+
+        g_assert (srv_status == sysinfo_tests[i].expected_srv_status);
+        g_assert (srv_domain == sysinfo_tests[i].expected_srv_domain);
+        g_assert (roam_status == sysinfo_tests[i].expected_roam_status);
+        g_assert (sys_mode == sysinfo_tests[i].expected_sys_mode);
+        g_assert (sim_state == sysinfo_tests[i].expected_sim_state);
+        g_assert (sys_submode_valid == sysinfo_tests[i].expected_sys_submode_valid);
+        if (sys_submode_valid)
+            g_assert (sys_submode == sysinfo_tests[i].expected_sys_submode);
+    }
+}
+
 /*****************************************************************************/
 
 int main (int argc, char **argv)
@@ -138,6 +201,7 @@ int main (int argc, char **argv)
     g_test_init (&argc, &argv, NULL);
 
     g_test_add_func ("/MM/huawei/ndisstatqry", test_ndisstatqry);
+    g_test_add_func ("/MM/huawei/sysinfo", test_sysinfo);
 
     return g_test_run ();
 }