Uhm, sorry, not the right patch. Please ignore<div> Carlo</div> Il martedì 4 aprile 2017, Carlo Lobrano <<a href="mailto:c.lobrano@gmail.com">c.lobrano@gmail.com</a>> ha scritto: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 Fixes: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=100374" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=100374</a> --- 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 | 91 +++++++++++++++++------ plugins/telit/tests/test-mm-modem-helpers-telit.c | 44 +++++++---- 2 files changed, 98 insertions(+), 37 deletions(-) diff --git a/plugins/telit/mm-modem-helpers-telit.c b/plugins/telit/mm-modem-helpers-telit.c index c8c1f4b..f2568d8 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,96 @@ 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, GError **error) { - GRegex *r = NULL; GMatchInfo *match_info = NULL; - gchar *retries_hex_str; - guint retries; + GRegex *r = NULL; + gchar *str_code = NULL; + gint retries = -1; + guint64 hex_code = 0x0; - r = g_regex_new ("\\+CSIM:\\s*[0-9]+,\\s*.*63C(.*)\"", G_REGEX_RAW, 0, NULL); + //TODO why do I need step? + 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; } - retries_hex_str = mm_get_string_unquoted_from_match_info (match_info, 1); - g_assert (NULL != retries_hex_str); + 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; + } - /* 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; + 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; } - g_free (retries_hex_str); + 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) { + // TODO unittest + 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); + } + +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/tests/test-mm-modem-helpers-telit.c b/plugins/telit/tests/test-mm-modem-helpers-telit.c index 40e3628..3d902b2 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, \"69FF\"", -1, "Could not parse response '+CSIM: 4, \"69FF\"'"}, + {"+CSIM: 4, \"6A86\"", -1, "Incorrect parameters in SIM request"}, + {"+CSIM: 4, \"6A88\"", -1, "SIM reference data not found"}, + {"+CSIM: 4, \"6AFF\"", -1, "Could not parse response '+CSIM: 4, \"6AFF\"'"}, { NULL, -1} }; static CSIMResponseTest invalid_csim_response_test_list [] = { /* Test missing final quotation mark */ - {"+CSIM: 8, xxxx63CF", -1}, + {"+CSIM: 8, xxxx63CF", -1, "Could not recognize response '+CSIM: 8, xxxx63CF'"}, /* Negative test for substring "63Cx" */ - {"+CSIM: 4, xxxx73CF\"", -1}, + {"+CSIM: 4, xxxx73CF\"", -1, "Could not parse response '+CSIM: 4, xxxx73CF\"'"}, /* Test missing first argument */ - {"+CSIM:xxxx63CF\"", -1}, + {"+CSIM:xxxx63CF\"", -1, "Could not recognize response '+CSIM:xxxx63CF\"'"}, { 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); } @@ -82,6 +99,7 @@ test_mm_telit_parse_csim_response (void) 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_cmpstr (error->message, ==, invalid_csim_response_test_list[i].error_message); g_assert_cmpint (res, ==, invalid_csim_response_test_list[i].result); if (NULL != error) { -- 2.7.4 </blockquote>