[PATCH] telit: add error_code recognition to +CSIM parser
Carlo Lobrano
c.lobrano at gmail.com
Tue Apr 4 12:20:54 UTC 2017
Uhm, sorry, not the right patch. Please ignore
Carlo
Il martedì 4 aprile 2017, Carlo Lobrano <c.lobrano at gmail.com> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20170404/a22da76b/attachment-0001.html>
More information about the ModemManager-devel
mailing list