Uhm, sorry, not the right patch. Please ignore Carlo
Il martedì 4 aprile 2017, Carlo Lobrano <[email protected]> ha scritto: > 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: 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 | 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 > >
_______________________________________________ ModemManager-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
