[PATCH] telit: add error_code recognition to +CSIM parser
Carlo Lobrano
c.lobrano at gmail.com
Tue Apr 4 07:13:08 UTC 2017
Sure, good point!
At least for the error codes it's easy, I'll think something for the hex
with the retry value as well.
I'll update the patch
Carlo
Il lunedì 3 aprile 2017, Dan Williams <dcbw at redhat.com> ha scritto:
> 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);
> > }
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20170404/4e20721e/attachment-0001.html>
More information about the ModemManager-devel
mailing list