[PATCH] telit: add error_code recognition to +CSIM parser

Dan Williams dcbw at redhat.com
Mon Apr 3 15:23:10 UTC 2017


On Mon, 2017-04-03 at 17:01 +0200, Carlo Lobrano wrote:
> Updated mm_telit_parse_csim_response in order to correctly recognize
> the
> following +CSIM error codes:
> 
> * 6300 - Verification failed
> * 6983 - Authentication method blocked
> * 6984 - Reference data invalidated
> * 6A86 - Incorrect parameters
> * 6A88 - Reference data not found
> 
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=100374
> ---
> 
> As a side note, I observed that sometimes the modem replies with
> error
> code
> 
>     6A86 - Incorrect parameters
> 
> when #QSS: 3 has not been received yet. This seems to be a modem's
> bug
> because the very same request is accepted as correct when issued
> later,
> namely when the SIM is ready.
> 
> ---
>  plugins/telit/mm-modem-helpers-telit.c            | 96
> ++++++++++++++++++++---
>  plugins/telit/tests/test-mm-modem-helpers-telit.c | 43 +++++++---
>  2 files changed, 113 insertions(+), 26 deletions(-)
> 
> diff --git a/plugins/telit/mm-modem-helpers-telit.c
> b/plugins/telit/mm-modem-helpers-telit.c
> index c8c1f4b..cc8cf0a 100644
> --- a/plugins/telit/mm-modem-helpers-telit.c
> +++ b/plugins/telit/mm-modem-helpers-telit.c
> @@ -121,45 +121,115 @@ mm_telit_parse_csim_response (const guint
> step,
>  {
>      GRegex *r = NULL;
>      GMatchInfo *match_info = NULL;
> -    gchar *retries_hex_str;
> -    guint retries;
> +    gchar *error_code = NULL;
> +    gchar *retries_hex_str = NULL;
> +    gint rtv = -1;
>  
> +    /* Check for error codes */
> +    r = g_regex_new ("\\+CSIM:\\s*[0-9]+,\\s*.*6300\"", G_REGEX_RAW,
> 0, NULL);
> +    if (g_regex_match (r, response, 0, &match_info)) {
> +        g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
> +                     "SIM verification failed");
> +        goto out;
> +    }

Can we do a single regex, capture the 4-hex-character response as part
of the regex, run g_ascii_strtoull() on it to get a uint, and then use
switch() to compare to known error codes instead?

Dan

> +    g_match_info_free (match_info);
> +    match_info = NULL;
> +    g_regex_unref (r);
> +    r = NULL;
> +
> +    r = g_regex_new ("\\+CSIM:\\s*[0-9]+,\\s*.*69(.*)\"",
> G_REGEX_RAW, 0, NULL);
> +    if (g_regex_match (r, response, 0, &match_info) &&
> +        match_info != NULL &&
> +        g_match_info_matches (match_info)) {
> +        error_code = mm_get_string_unquoted_from_match_info
> (match_info, 1);
> +
> +        if (error_code == NULL) {
> +            g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
> +                         "Could not parse CSIM error code in
> response '%s'",
> +                         response);
> +        } else if (strstr (error_code, "83") != NULL) {
> +            g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
> +                         "SIM authentication method blocked");
> +        } else if (strstr (error_code, "84") != NULL) {
> +            g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
> +                         "SIM reference data invalidated");
> +        } else {
> +             g_set_error (error, MM_CORE_ERROR,
> MM_CORE_ERROR_FAILED,
> +                         "Unknown error code '69%s'", error_code);
> +        }
> +
> +        goto out;
> +    }
> +
> +    g_match_info_free (match_info);
> +    match_info = NULL;
> +    g_regex_unref (r);
> +    r = NULL;
> +
> +    r = g_regex_new ("\\+CSIM:\\s*[0-9]+,\\s*.*6A(.*)\"",
> G_REGEX_RAW, 0, NULL);
> +    if (g_regex_match (r, response, 0, &match_info) &&
> +        match_info != NULL &&
> +        g_match_info_matches (match_info)) {
> +        error_code = mm_get_string_unquoted_from_match_info
> (match_info, 1);
> +
> +        if (error_code == NULL) {
> +            g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
> +                         "Could not parse CSIM error code in
> response '%s'",
> +                         response);
> +        } else if (strstr (error_code, "86") != NULL) {
> +            g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
> +                         "Incorrect parameters in SIM request");
> +        } else if (strstr (error_code, "88") != NULL) {
> +            g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
> +                         "SIM reference data not found");
> +        } else {
> +             g_set_error (error, MM_CORE_ERROR,
> MM_CORE_ERROR_FAILED,
> +                         "Unknown error code '6A%s'", error_code);
> +        }
> +
> +        goto out;
> +    }
> +
> +    g_match_info_free (match_info);
> +    match_info = NULL;
> +    g_regex_unref (r);
> +    r = NULL;
> +
> +    /* No errors found. Get SIM retries */
>      r = g_regex_new ("\\+CSIM:\\s*[0-9]+,\\s*.*63C(.*)\"",
> G_REGEX_RAW, 0, NULL);
>  
>      if (!g_regex_match (r, response, 0, &match_info)) {
>          g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
>                       "Could not parse reponse '%s'", response);
> -        g_match_info_free (match_info);
> -        g_regex_unref (r);
> -        return -1;
> +        goto out;
>      }
>  
>      if (!g_match_info_matches (match_info)) {
>          g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
>                       "Could not find matches in response '%s'",
> response);
> -        g_match_info_free (match_info);
> -        g_regex_unref (r);
> -        return -1;
> +        goto out;
>      }
>  
>      retries_hex_str = mm_get_string_unquoted_from_match_info
> (match_info, 1);
>      g_assert (NULL != retries_hex_str);
>  
>      /* convert hex value to uint */
> -    if (sscanf (retries_hex_str, "%x", &retries) != 1) {
> +    if (sscanf (retries_hex_str, "%x", &rtv) != 1) {
>           g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
>                       "Could not get retry value from match '%s'",
>                       retries_hex_str);
> -        g_match_info_free (match_info);
> -        g_regex_unref (r);
> -        return -1;
> +        goto out;
>      }
>  
>      g_free (retries_hex_str);
> +
> +out:
> +    g_free (error_code),
>      g_match_info_free (match_info);
>      g_regex_unref (r);
>  
> -    return retries;
> +    return rtv;
>  }
>  #define
> SUPP_BAND_RESPONSE_REGEX          "#BND:\\s*\\((?P<Bands2G>[0-9\\-
> ,]*)\\)(,\\s*\\((?P<Bands3G>[0-9\\-,]*)\\))?(,\\s*\\((?P<Bands4G>[0-
> 9\\-,]*)\\))?"
>  #define
> CURR_BAND_RESPONSE_REGEX          "#BND:\\s*(?P<Bands2G>\\d+)(,\\s*(?
> P<Bands3G>\\d+))?(,\\s*(?P<Bands4G>\\d+))?"
> diff --git a/plugins/telit/tests/test-mm-modem-helpers-telit.c
> b/plugins/telit/tests/test-mm-modem-helpers-telit.c
> index 40e3628..4907ce5 100644
> --- a/plugins/telit/tests/test-mm-modem-helpers-telit.c
> +++ b/plugins/telit/tests/test-mm-modem-helpers-telit.c
> @@ -28,36 +28,46 @@
>  typedef struct {
>      gchar *response;
>      gint result;
> +    gchar *error_message;
>  } CSIMResponseTest;
>  
>  static CSIMResponseTest valid_csim_response_test_list [] = {
>      /* The parser expects that 2nd arg contains
>       * substring "63Cx" where x is an HEX string
>       * representing the retry value */
> -    {"+CSIM:8,\"xxxx63C1\"", 1},
> -    {"+CSIM:8,\"xxxx63CA\"", 10},
> -    {"+CSIM:8,\"xxxx63CF\"", 15},
> +    {"+CSIM:8,\"xxxx63C1\"", 1, NULL},
> +    {"+CSIM:8,\"xxxx63CA\"", 10, NULL},
> +    {"+CSIM:8,\"xxxx63CF\"", 15, NULL},
>      /* The parser accepts spaces */
> -    {"+CSIM:8,\"xxxx63C1\"", 1},
> -    {"+CSIM:8, \"xxxx63C1\"", 1},
> -    {"+CSIM: 8, \"xxxx63C1\"", 1},
> -    {"+CSIM:  8, \"xxxx63C1\"", 1},
> +    {"+CSIM:8,\"xxxx63C1\"", 1, NULL},
> +    {"+CSIM:8, \"xxxx63C1\"", 1, NULL},
> +    {"+CSIM: 8, \"xxxx63C1\"", 1, NULL},
> +    {"+CSIM:  8, \"xxxx63C1\"", 1, NULL},
>      /* the parser expects an int as first argument (2nd arg's
> length),
>       * but does not check if it is correct */
> -    {"+CSIM: 10, \"63CF\"", 15},
> +    {"+CSIM: 4, \"63CF\"", 15, NULL},
>      /* the parser expect a quotation mark at the end
>       * of the response, but not at the begin */
> -    {"+CSIM: 10, 63CF\"", 15},
> +    {"+CSIM: 4, 63CF\"", 15, NULL},
> +    {"+CSIM: 4, 63CF\"", 15, NULL},
> +    /* SIM Error codes */
> +    {"+CSIM: 4, \"6300\"", -1, "SIM verification failed"},
> +    {"+CSIM: 4, \"6983\"", -1, "SIM authentication method blocked"},
> +    {"+CSIM: 4, \"6984\"", -1, "SIM reference data invalidated"},
> +    {"+CSIM: 4, \"69XX\"", -1, "Unknown error code '69XX'"},
> +    {"+CSIM: 4, \"6A86\"", -1, "Incorrect parameters in SIM
> request"},
> +    {"+CSIM: 4, \"6A88\"", -1, "SIM reference data not found"},
> +    {"+CSIM: 4, \"6AXX\"", -1, "Unknown error code '6AXX'"},
>      { NULL, -1}
>  };
>  
>  static CSIMResponseTest invalid_csim_response_test_list [] = {
>      /* Test missing final quotation mark */
> -    {"+CSIM: 8, xxxx63CF", -1},
> +    {"+CSIM: 8, xxxx63CF", -1, NULL},
>      /* Negative test for substring "63Cx" */
> -    {"+CSIM: 4, xxxx73CF\"", -1},
> +    {"+CSIM: 4, xxxx73CF\"", -1, NULL},
>      /* Test missing first argument */
> -    {"+CSIM:xxxx63CF\"", -1},
> +    {"+CSIM:xxxx63CF\"", -1, NULL},
>      { NULL, -1}
>  };
>  
> @@ -73,7 +83,14 @@ test_mm_telit_parse_csim_response (void)
>      for (i = 0; valid_csim_response_test_list[i].response != NULL;
> i++) {
>          res = mm_telit_parse_csim_response (step,
> valid_csim_response_test_list[i].response, &error);
>  
> -        g_assert_no_error (error);
> +        if (valid_csim_response_test_list[i].error_message == NULL)
> {
> +            g_assert_no_error (error);
> +        } else {
> +            g_assert_cmpstr (error->message, ==,
> valid_csim_response_test_list[i].error_message);
> +            g_error_free (error);
> +            error = NULL;
> +        }
> +
>          g_assert_cmpint (res, ==,
> valid_csim_response_test_list[i].result);
>      }
>  


More information about the ModemManager-devel mailing list