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

Carlo Lobrano c.lobrano at gmail.com
Tue Apr 4 12:55:36 UTC 2017


- Refactored 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

- Updated correspondent tests.
- Finally, some minor changes in other files for better error logging

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-broadband-modem-telit.c          |  6 +-
 plugins/telit/mm-modem-helpers-telit.c            | 93 ++++++++++++++++-------
 plugins/telit/mm-modem-helpers-telit.h            |  3 +-
 plugins/telit/tests/test-mm-modem-helpers-telit.c | 72 +++++++++---------
 4 files changed, 105 insertions(+), 69 deletions(-)

diff --git a/plugins/telit/mm-broadband-modem-telit.c b/plugins/telit/mm-broadband-modem-telit.c
index cce0229..f316e30 100644
--- a/plugins/telit/mm-broadband-modem-telit.c
+++ b/plugins/telit/mm-broadband-modem-telit.c
@@ -541,13 +541,13 @@ csim_query_ready (MMBaseModem *self,
     response = mm_base_modem_at_command_finish (self, res, &error);
 
     if (!response) {
-        mm_warn ("No respose for step %d: %s", ctx->step, error->message);
+        mm_warn ("load unlock retries: no respose for step %d: %s", ctx->step, error->message);
         g_error_free (error);
         goto next_step;
     }
 
-    if ( (unlock_retries = mm_telit_parse_csim_response (ctx->step, response, &error)) < 0) {
-        mm_warn ("Parse error in step %d: %s.", ctx->step, error->message);
+    if ( (unlock_retries = mm_telit_parse_csim_response (response, &error)) < 0) {
+        mm_warn ("load unlock retries: parse error in step %d: %s.", ctx->step, error->message);
         g_error_free (error);
         goto next_step;
     }
diff --git a/plugins/telit/mm-modem-helpers-telit.c b/plugins/telit/mm-modem-helpers-telit.c
index c8c1f4b..cb3ff24 100644
--- a/plugins/telit/mm-modem-helpers-telit.c
+++ b/plugins/telit/mm-modem-helpers-telit.c
@@ -17,6 +17,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <stdio.h>
+#include <errno.h>
 
 #include <ModemManager.h>
 #define _LIBMM_INSIDE_MMCLI
@@ -113,54 +114,92 @@ mm_telit_get_band_flag (GArray *bands_array,
 
 /*****************************************************************************/
 /* +CSIM response parser */
+#define MM_TELIT_MIN_SIM_RETRY_HEX 0x63C0
+#define MM_TELIT_MAX_SIM_RETRY_HEX 0x63CF
 
 gint
-mm_telit_parse_csim_response (const guint step,
-                              const gchar *response,
+mm_telit_parse_csim_response (const gchar *response,
                               GError **error)
 {
-    GRegex *r = NULL;
     GMatchInfo *match_info = NULL;
-    gchar *retries_hex_str;
-    guint retries;
-
-    r = g_regex_new ("\\+CSIM:\\s*[0-9]+,\\s*.*63C(.*)\"", G_REGEX_RAW, 0, NULL);
+    GRegex *r = NULL;
+    gchar *str_code = NULL;
+    gint retries = -1;
+    guint64 hex_code = 0x0;
 
+    r = g_regex_new ("\\+CSIM:\\s*[0-9]+,\\s*.*([0-9a-fA-F]{4})\"", 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;
+                     "Could not recognize response '%s'", response);
+        goto out;
     }
 
-    if (!g_match_info_matches (match_info)) {
+    if (match_info == NULL || !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;
+                     "Could not parse response '%s'", response);
+        goto out;
+    }
+
+    str_code = mm_get_string_unquoted_from_match_info (match_info, 1);
+    if (str_code == NULL) {
+        g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                     "Could not find expected string code in response '%s'", response);
+        goto out;
     }
 
-    retries_hex_str = mm_get_string_unquoted_from_match_info (match_info, 1);
-    g_assert (NULL != retries_hex_str);
+    errno = 0;
+    hex_code = g_ascii_strtoull (str_code, NULL, 16);
+    if (hex_code == G_MAXUINT64 && errno == ERANGE) {
+        g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                     "Could not recognize expected hex code in response '%s'", response);
+        goto out;
+    }
 
-    /* convert hex value to uint */
-    if (sscanf (retries_hex_str, "%x", &retries) != 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;
+    switch(hex_code) {
+        case 0x6300:
+            g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                         "SIM verification failed");
+            break;
+        case 0x6983:
+            g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                         "SIM authentication method blocked");
+            break;
+        case 0x6984:
+            g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                         "SIM reference data invalidated");
+            break;
+        case 0x6A86:
+            g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                         "Incorrect parameters in SIM request");
+            break;
+        case 0x6A88:
+            g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                         "SIM reference data not found");
+            break;
+        default:
+            break;
+    }
+
+    if (g_error_matches (*error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED)) {
+        goto out;
+    }
+
+    if (hex_code < MM_TELIT_MIN_SIM_RETRY_HEX || hex_code > MM_TELIT_MAX_SIM_RETRY_HEX) {
+        g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                     "SIM retries out of bound in response '%s'", response);
+    } else {
+        retries = (gint)(hex_code - MM_TELIT_MIN_SIM_RETRY_HEX);
     }
 
-    g_free (retries_hex_str);
+out:
+    if (str_code != NULL)
+        g_free (str_code);
     g_match_info_free (match_info);
     g_regex_unref (r);
 
     return retries;
 }
+
 #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/mm-modem-helpers-telit.h b/plugins/telit/mm-modem-helpers-telit.h
index 5f0e880..b693732 100644
--- a/plugins/telit/mm-modem-helpers-telit.h
+++ b/plugins/telit/mm-modem-helpers-telit.h
@@ -61,8 +61,7 @@ typedef struct {
 } TelitToMMBandMap;
 
 /* +CSIM response parser */
-gint mm_telit_parse_csim_response (const guint step,
-                                   const gchar *response,
+gint mm_telit_parse_csim_response (const gchar *response,
                                    GError **error);
 
 typedef enum {
diff --git a/plugins/telit/tests/test-mm-modem-helpers-telit.c b/plugins/telit/tests/test-mm-modem-helpers-telit.c
index 40e3628..8b7a808 100644
--- a/plugins/telit/tests/test-mm-modem-helpers-telit.c
+++ b/plugins/telit/tests/test-mm-modem-helpers-telit.c
@@ -28,66 +28,64 @@
 typedef struct {
     gchar *response;
     gint result;
+    gchar *error_message;
 } CSIMResponseTest;
 
-static CSIMResponseTest valid_csim_response_test_list [] = {
+static CSIMResponseTest 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: 10, \"63CF\"", 15, NULL},
     /* the parser expect a quotation mark at the end
-     * of the response, but not at the begin */
-    {"+CSIM: 10, 63CF\"", 15},
-    { NULL, -1}
-};
-
-static CSIMResponseTest invalid_csim_response_test_list [] = {
-    /* Test missing final quotation mark */
-    {"+CSIM: 8, xxxx63CF", -1},
-    /* Negative test for substring "63Cx" */
-    {"+CSIM: 4, xxxx73CF\"", -1},
-    /* Test missing first argument */
-    {"+CSIM:xxxx63CF\"", -1},
+     * of the response, but not at the beginning */
+    {"+CSIM: 4, 63CF\"", 15, NULL},
+    /* Valid +CSIM 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, \"6A86\"", -1, "Incorrect parameters in SIM request"},
+    {"+CSIM: 4, \"6A88\"", -1, "SIM reference data not found"},
+    /* Test error: out of bound */
+    {"+CSIM: 4, \"63BF\"", -1, "SIM retries out of bound in response '+CSIM: 4, \"63BF\"'"},
+    {"+CSIM: 4, \"63D0\"", -1, "SIM retries out of bound in response '+CSIM: 4, \"63D0\"'"},
+    /* Test error: missing final quotation mark */
+    {"+CSIM: 8, xxxx63CF", -1, "Could not recognize response '+CSIM: 8, xxxx63CF'"},
+    /* Test error: missing first argument */
+    {"+CSIM:xxxx63CF\"", -1, "Could not recognize response '+CSIM:xxxx63CF\"'"},
     { NULL, -1}
 };
 
 static void
 test_mm_telit_parse_csim_response (void)
 {
-    const gint step = 1;
     guint i;
     gint res;
     GError* error = NULL;
 
     /* Test valid responses */
-    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);
-        g_assert_cmpint (res, ==, valid_csim_response_test_list[i].result);
-    }
-
-    /* Test invalid responses */
-    for (i = 0; invalid_csim_response_test_list[i].response != NULL; i++) {
-        res = mm_telit_parse_csim_response (step, invalid_csim_response_test_list[i].response, &error);
-
-        g_assert_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED);
-        g_assert_cmpint (res, ==, invalid_csim_response_test_list[i].result);
-
-        if (NULL != error) {
+    for (i = 0; csim_response_test_list[i].response != NULL; i++) {
+        res = mm_telit_parse_csim_response (csim_response_test_list[i].response, &error);
+
+        if (csim_response_test_list[i].error_message == NULL) {
+            g_assert_no_error (error);
+        } else {
+            g_assert_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED);
+            g_assert_cmpstr (error->message, ==, csim_response_test_list[i].error_message);
             g_error_free (error);
             error = NULL;
         }
+
+        g_assert_cmpint (res, ==, csim_response_test_list[i].result);
     }
 }
 
-- 
2.7.4



More information about the ModemManager-devel mailing list