[PATCH] huawei: fix parsing of ^NDISSTATQRY? response

Dan Williams dcbw at redhat.com
Mon Aug 5 21:08:16 PDT 2013


On Mon, 2013-08-05 at 19:46 -0700, Ben Chan wrote:
> This patch modifies mm_huawei_parse_ndisstatqry_response() to properly
> handle a ^NDISSTATQRY? response when it is mixed with another unsolicited
> response, e.g.
> 
>   ^RSSI: 16\r\n^NDISSTATQRY: 0,,,"IPV4"

We'd expect the unsolicited responses to be cleared out of the response
string already by the serial parser.  Is that not happening?  At least
for ^RSSI, the regex expects a leading \r\n though.  Are there any logs
you've got that show a more complete command sequence?

Dan

> This patch is originally developed by:
>   Franko Fang <fangxiaozhi at huawei.com>
> 
> And then revised by:
>   Ben Chan <benchan at chromium.org>
> ---
>  plugins/huawei/mm-modem-helpers-huawei.c         |  6 +-
>  plugins/huawei/tests/test-modem-helpers-huawei.c | 70 +++++++++++++++---------
>  2 files changed, 48 insertions(+), 28 deletions(-)
> 
> diff --git a/plugins/huawei/mm-modem-helpers-huawei.c b/plugins/huawei/mm-modem-helpers-huawei.c
> index d9b038d..e1e05c4 100644
> --- a/plugins/huawei/mm-modem-helpers-huawei.c
> +++ b/plugins/huawei/mm-modem-helpers-huawei.c
> @@ -20,6 +20,7 @@
>  #define _LIBMM_INSIDE_MM
>  #include <libmm-glib.h>
>  
> +#include "mm-log.h"
>  #include "mm-modem-helpers-huawei.h"
>  
>  /*****************************************************************************/
> @@ -37,7 +38,10 @@ mm_huawei_parse_ndisstatqry_response (const gchar *response,
>      GMatchInfo *match_info;
>      GError *inner_error = NULL;
>  
> -    if (!response || !g_str_has_prefix (response, "^NDISSTATQRY:")) {
> +    if (response)
> +        response = g_strstr_len (response, -1, "^NDISSTATQRY:");
> +
> +    if (!response) {
>          g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED, "Missing ^NDISSTATQRY prefix");
>          return FALSE;
>      }
> diff --git a/plugins/huawei/tests/test-modem-helpers-huawei.c b/plugins/huawei/tests/test-modem-helpers-huawei.c
> index c1bc0ed..5e2e83d 100644
> --- a/plugins/huawei/tests/test-modem-helpers-huawei.c
> +++ b/plugins/huawei/tests/test-modem-helpers-huawei.c
> @@ -17,6 +17,9 @@
>  #include <glib-object.h>
>  #include <locale.h>
>  
> +#include <ModemManager.h>
> +
> +#include "mm-errors-types.h"
>  #include "mm-modem-helpers-huawei.h"
>  
>  /*****************************************************************************/
> @@ -24,6 +27,7 @@
>  
>  typedef struct {
>      const gchar *str;
> +    gboolean expected_result;
>      gboolean expected_ipv4_available;
>      gboolean expected_ipv4_connected;
>      gboolean expected_ipv6_available;
> @@ -31,39 +35,48 @@ typedef struct {
>  } NdisstatqryTest;
>  
>  static const NdisstatqryTest ndisstatqry_tests[] = {
> -    { "^NDISSTATQRY: 1,,,IPV4\r\n", TRUE,  TRUE,  FALSE, FALSE },
> -    { "^NDISSTATQRY: 0,,,IPV4\r\n", TRUE,  FALSE, FALSE, FALSE },
> -    { "^NDISSTATQRY: 1,,,IPV6\r\n", FALSE, FALSE, TRUE,  TRUE  },
> -    { "^NDISSTATQRY: 0,,,IPV6\r\n", FALSE, FALSE, TRUE,  FALSE },
> +    { "^NDISSTATQRY: 1,,,IPV4\r\n", TRUE,  TRUE,  TRUE,  FALSE, FALSE },
> +    { "^NDISSTATQRY: 0,,,IPV4\r\n", TRUE,  TRUE,  FALSE, FALSE, FALSE },
> +    { "^NDISSTATQRY: 1,,,IPV6\r\n", TRUE,  FALSE, FALSE, TRUE,  TRUE  },
> +    { "^NDISSTATQRY: 0,,,IPV6\r\n", TRUE,  FALSE, FALSE, TRUE,  FALSE },
>      { "^NDISSTATQRY: 1,,,IPV4\r\n"
> -      "^NDISSTATQRY: 1,,,IPV6\r\n", TRUE,  TRUE,  TRUE,  TRUE  },
> +      "^NDISSTATQRY: 1,,,IPV6\r\n", TRUE,  TRUE,  TRUE,  TRUE,  TRUE  },
>      { "^NDISSTATQRY: 1,,,IPV4\r\n"
> -      "^NDISSTATQRY: 0,,,IPV6\r\n", TRUE,  TRUE,  TRUE,  FALSE },
> +      "^NDISSTATQRY: 0,,,IPV6\r\n", TRUE,  TRUE,  TRUE,  TRUE,  FALSE },
>      { "^NDISSTATQRY: 0,,,IPV4\r\n"
> -      "^NDISSTATQRY: 1,,,IPV6\r\n", TRUE,  FALSE, TRUE,  TRUE  },
> +      "^NDISSTATQRY: 1,,,IPV6\r\n", TRUE,  TRUE,  FALSE, TRUE,  TRUE  },
>      { "^NDISSTATQRY: 0,,,IPV4\r\n"
> -      "^NDISSTATQRY: 0,,,IPV6\r\n", TRUE,  FALSE, TRUE,  FALSE },
> -    { "^NDISSTATQRY: 1,,,IPV4",     TRUE,  TRUE,  FALSE, FALSE },
> -    { "^NDISSTATQRY: 0,,,IPV4",     TRUE,  FALSE, FALSE, FALSE },
> -    { "^NDISSTATQRY: 1,,,IPV6",     FALSE, FALSE, TRUE,  TRUE  },
> -    { "^NDISSTATQRY: 0,,,IPV6",     FALSE, FALSE, TRUE,  FALSE },
> +      "^NDISSTATQRY: 0,,,IPV6\r\n", TRUE,  TRUE,  FALSE, TRUE,  FALSE },
> +    { "^NDISSTATQRY: 1,,,IPV4",     TRUE,  TRUE,  TRUE,  FALSE, FALSE },
> +    { "^NDISSTATQRY: 0,,,IPV4",     TRUE,  TRUE,  FALSE, FALSE, FALSE },
> +    { "^NDISSTATQRY: 1,,,IPV6",     TRUE,  FALSE, FALSE, TRUE,  TRUE  },
> +    { "^NDISSTATQRY: 0,,,IPV6",     TRUE,  FALSE, FALSE, TRUE,  FALSE },
>      { "^NDISSTATQRY: 1,,,IPV4\r\n"
> -      "^NDISSTATQRY: 1,,,IPV6",     TRUE,  TRUE,  TRUE,  TRUE  },
> +      "^NDISSTATQRY: 1,,,IPV6",     TRUE,  TRUE,  TRUE,  TRUE,  TRUE  },
>      { "^NDISSTATQRY: 1,,,IPV4\r\n"
> -      "^NDISSTATQRY: 0,,,IPV6",     TRUE,  TRUE,  TRUE,  FALSE },
> +      "^NDISSTATQRY: 0,,,IPV6",     TRUE,  TRUE,  TRUE,  TRUE,  FALSE },
>      { "^NDISSTATQRY: 0,,,IPV4\r\n"
> -      "^NDISSTATQRY: 1,,,IPV6",     TRUE,  FALSE, TRUE,  TRUE  },
> +      "^NDISSTATQRY: 1,,,IPV6",     TRUE,  TRUE,  FALSE, TRUE,  TRUE  },
>      { "^NDISSTATQRY: 0,,,IPV4\r\n"
> -      "^NDISSTATQRY: 0,,,IPV6",     TRUE,  FALSE, TRUE,  FALSE },
> -    { NULL,                         FALSE, FALSE, FALSE, FALSE }
> +      "^NDISSTATQRY: 0,,,IPV6",     TRUE,  TRUE,  FALSE, TRUE,  FALSE },
> +    { "^RSSI: 16\r\n"
> +      "^NDISSTATQRY: 0,,,IPV4",     TRUE,  TRUE,  FALSE, FALSE,  FALSE },
> +    { "^RSSI: 16\r\n"
> +      "^NDISSTATQRY: 0,,,IPV4\r\n"
> +      "^NDISSTATQRY: 0,,,IPV6",     TRUE,  TRUE,  FALSE, TRUE,  FALSE },
> +    { "^RSSI: 16\r\n",              FALSE, FALSE, FALSE, FALSE, FALSE },
> +    { "OK\r\n",                     FALSE, FALSE, FALSE, FALSE, FALSE },
> +    { "\r\n",                       FALSE, FALSE, FALSE, FALSE, FALSE },
> +    { "",                           FALSE, FALSE, FALSE, FALSE, FALSE },
> +    { NULL,                         FALSE, FALSE, FALSE, FALSE, FALSE }
>  };
>  
>  static void
>  test_ndisstatqry (void)
>  {
> -    guint i;
> +    size_t i;
>  
> -    for (i = 0; ndisstatqry_tests[i].str; i++) {
> +    for (i = 0; i < sizeof (ndisstatqry_tests) / sizeof (ndisstatqry_tests[0]); i++) {
>          GError *error = NULL;
>          gboolean ipv4_available;
>          gboolean ipv4_connected;
> @@ -76,15 +89,18 @@ test_ndisstatqry (void)
>                        &ipv4_connected,
>                        &ipv6_available,
>                        &ipv6_connected,
> -                      &error) == TRUE);
> -        g_assert_no_error (error);
> +                      &error) == ndisstatqry_tests[i].expected_result);
> +        if (ndisstatqry_tests[i].expected_result) {
> +            g_assert_no_error (error);
> +            g_assert (ipv4_available == ndisstatqry_tests[i].expected_ipv4_available);
> +            if (ipv4_available)
> +                g_assert (ipv4_connected == ndisstatqry_tests[i].expected_ipv4_connected);
> +            g_assert (ipv6_available == ndisstatqry_tests[i].expected_ipv6_available);
> +            if (ipv6_available)
> +                g_assert (ipv6_connected == ndisstatqry_tests[i].expected_ipv6_connected);
> +        } else
> +            g_assert_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED);
>  
> -        g_assert (ipv4_available == ndisstatqry_tests[i].expected_ipv4_available);
> -        if (ipv4_available)
> -            g_assert (ipv4_connected == ndisstatqry_tests[i].expected_ipv4_connected);
> -        g_assert (ipv6_available == ndisstatqry_tests[i].expected_ipv6_available);
> -        if (ipv6_available)
> -            g_assert (ipv6_connected == ndisstatqry_tests[i].expected_ipv6_connected);
>      }
>  }
>  




More information about the ModemManager-devel mailing list