[PATCH 1/2] huawei: fix ^SYSINFO parsing

Aleksander Morgado aleksander at lanedo.com
Sun Oct 20 08:07:36 PDT 2013


On 18/10/13 11:56, Ben Chan wrote:
> 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.
> ---

Pushed, thanks!

>  plugins/huawei/mm-broadband-modem-huawei.c       | 82 +++---------------------
>  plugins/huawei/mm-modem-helpers-huawei.c         | 67 +++++++++++++++++++
>  plugins/huawei/mm-modem-helpers-huawei.h         | 11 ++++
>  plugins/huawei/tests/test-modem-helpers-huawei.c | 64 ++++++++++++++++++
>  4 files changed, 151 insertions(+), 73 deletions(-)
> 
> diff --git a/plugins/huawei/mm-broadband-modem-huawei.c b/plugins/huawei/mm-broadband-modem-huawei.c
> index 4f231df..f449a39 100644
> --- a/plugins/huawei/mm-broadband-modem-huawei.c
> +++ b/plugins/huawei/mm-broadband-modem-huawei.c
> @@ -114,70 +114,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,
>                   guint *out_srv_domain,
> @@ -308,15 +244,15 @@ run_sysinfo_ready (MMBaseModem *self,
>  
>      result = g_new0 (SysinfoResult, 1);
>      result->extended = FALSE;
> -    if (!sysinfo_parse (response,
> -                          &result->srv_status,
> -                          &result->srv_domain,
> -                          &result->roam_status,
> -                          &result->sys_mode,
> -                          &result->sim_state,
> -                          &result->sys_submode_valid,
> -                          &result->sys_submode,
> -                          &error)) {
> +    if (!mm_huawei_parse_sysinfo_response (response,
> +                                           &result->srv_status,
> +                                           &result->srv_domain,
> +                                           &result->roam_status,
> +                                           &result->sys_mode,
> +                                           &result->sim_state,
> +                                           &result->sys_submode_valid,
> +                                           &result->sys_submode,
> +                                           &error)) {
>          mm_dbg ("^SYSINFO parsing failed: %s", error->message);
>          g_simple_async_result_take_error (simple, error);
>          g_simple_async_result_complete (simple);
> diff --git a/plugins/huawei/mm-modem-helpers-huawei.c b/plugins/huawei/mm-modem-helpers-huawei.c
> index bb977c2..b869c31 100644
> --- a/plugins/huawei/mm-modem-helpers-huawei.c
> +++ b/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;
> +}
> diff --git a/plugins/huawei/mm-modem-helpers-huawei.h b/plugins/huawei/mm-modem-helpers-huawei.h
> index 4d15e8a..f08ff8a 100644
> --- a/plugins/huawei/mm-modem-helpers-huawei.h
> +++ b/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 */
> diff --git a/plugins/huawei/tests/test-modem-helpers-huawei.c b/plugins/huawei/tests/test-modem-helpers-huawei.c
> index 8d99215..fa791fe 100644
> --- a/plugins/huawei/tests/test-modem-helpers-huawei.c
> +++ b/plugins/huawei/tests/test-modem-helpers-huawei.c
> @@ -129,6 +129,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 ();
>  }
> 


-- 
Aleksander


More information about the ModemManager-devel mailing list