[PATCH v4] telit: add error_code recognition to +CSIM parser
Carlo Lobrano
c.lobrano at gmail.com
Wed Apr 19 12:22:26 UTC 2017
>
> Pushed to git master. I also pushed a follow up commit to use
> mm_get_uint_from_hex_str():
> https://cgit.freedesktop.org/ModemManager/ModemManager/commit/?id=
> d09bc8baaa9fe93a72bb715530b1403a7a81c891
>
> Thanks!
>
Awesome, thanks to you!
On 19 April 2017 at 10:46, Aleksander Morgado <aleksander at aleksander.es>
wrote:
> On 19/04/17 10:00, Carlo Lobrano wrote:
> > - 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
>
> Pushed to git master. I also pushed a follow up commit to use
> mm_get_uint_from_hex_str():
> https://cgit.freedesktop.org/ModemManager/ModemManager/commit/?id=
> d09bc8baaa9fe93a72bb715530b1403a7a81c891
>
> Thanks!
>
> > ---
> > plugins/telit/mm-broadband-modem-telit.c | 6 +-
> > plugins/telit/mm-modem-helpers-telit.c | 98
> ++++++++++++++++-------
> > plugins/telit/mm-modem-helpers-telit.h | 3 +-
> > plugins/telit/tests/test-mm-modem-helpers-telit.c | 73
> ++++++++---------
> > 4 files changed, 104 insertions(+), 76 deletions(-)
> >
> > diff --git a/plugins/telit/mm-broadband-modem-telit.c
> b/plugins/telit/mm-broadband-modem-telit.c
> > index 8b87310..d154fc6 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..1dd2542 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,91 @@ 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;
> > + GRegex *r = NULL;
> > + gchar *str_code = NULL;
> > + gint retries = -1;
> > + guint64 hex_code = 0x0;
> > + GError *inner_error = NULL;
> >
> > - r = g_regex_new ("\\+CSIM:\\s*[0-9]+,\\s*.*63C(.*)\"",
> G_REGEX_RAW, 0, NULL);
> > + r = g_regex_new ("\\+CSIM:\\s*[0-9]+,\\s*\".*([0-9a-fA-F]{4})\"",
> G_REGEX_RAW, 0, NULL);
> > + g_regex_match (r, response, 0, &match_info);
> >
> > - 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;
> > + if (!g_match_info_matches (match_info)) {
> > + inner_error = g_error_new (MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
> > + "Could not recognize +CSIM response
> '%s'", response);
> > + 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;
> > + str_code = mm_get_string_unquoted_from_match_info (match_info, 1);
> > + if (str_code == NULL) {
> > + inner_error = g_error_new (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) {
> > + inner_error = g_error_new (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:
> > + inner_error = g_error_new (MM_CORE_ERROR,
> MM_CORE_ERROR_FAILED,
> > + "SIM verification failed");
> > + goto out;
> > + case 0x6983:
> > + inner_error = g_error_new (MM_CORE_ERROR,
> MM_CORE_ERROR_FAILED,
> > + "SIM authentication method
> blocked");
> > + goto out;
> > + case 0x6984:
> > + inner_error = g_error_new (MM_CORE_ERROR,
> MM_CORE_ERROR_FAILED,
> > + "SIM reference data
> invalidated");
> > + goto out;
> > + case 0x6A86:
> > + inner_error = g_error_new (MM_CORE_ERROR,
> MM_CORE_ERROR_FAILED,
> > + "Incorrect parameters in SIM
> request");
> > + goto out;
> > + case 0x6A88:
> > + inner_error = g_error_new (MM_CORE_ERROR,
> MM_CORE_ERROR_FAILED,
> > + "SIM reference data not found");
> > + goto out;
> > + default:
> > + break;
> > }
> >
> > - g_free (retries_hex_str);
> > - g_match_info_free (match_info);
> > + if (hex_code < MM_TELIT_MIN_SIM_RETRY_HEX || hex_code >
> MM_TELIT_MAX_SIM_RETRY_HEX) {
> > + inner_error = g_error_new (MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
> > + "Unknown error returned '0x%04lx'",
> hex_code);
> > + goto out;
> > + }
> > +
> > + retries = (gint)(hex_code - MM_TELIT_MIN_SIM_RETRY_HEX);
> > +
> > +out:
> > g_regex_unref (r);
> > + g_match_info_free (match_info);
> > + g_free (str_code);
> > +
> > + if (inner_error) {
> > + g_propagate_error (error, inner_error);
> > + return -1;
> > + }
> >
> > + g_assert (retries >= 0);
> > 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..3008010 100644
> > --- a/plugins/telit/tests/test-mm-modem-helpers-telit.c
> > +++ b/plugins/telit/tests/test-mm-modem-helpers-telit.c
> > @@ -28,66 +28,57 @@
> > 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,\"000063C1\"", 1, NULL},
> > + {"+CSIM:8,\"000063CA\"", 10, NULL},
> > + {"+CSIM:8,\"000063CF\"", 15, NULL},
> > /* The parser accepts spaces */
> > - {"+CSIM:8,\"xxxx63C1\"", 1},
> > - {"+CSIM:8, \"xxxx63C1\"", 1},
> > - {"+CSIM: 8, \"xxxx63C1\"", 1},
> > - {"+CSIM: 8, \"xxxx63C1\"", 1},
> > + {"+CSIM:8, \"000063C1\"", 1, NULL},
> > + {"+CSIM: 8, \"000063C1\"", 1, NULL},
> > + {"+CSIM: 8, \"000063C1\"", 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},
> > - /* 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},
> > - { NULL, -1}
> > + {"+CSIM: 10, \"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: missing first argument */
> > + {"+CSIM:000063CF\"", -1, "Could not recognize +CSIM response
> '+CSIM:000063CF\"'"},
> > + /* Test error: missing quotation mark */
> > + {"+CSIM: 8, 000063CF", -1, "Could not recognize +CSIM response
> '+CSIM: 8, 000063CF'"},
> > + /* Test generic error */
> > + {"+CSIM: 4, \"63BF\"", -1, "Unknown error returned '0x63bf'"},
> > + {"+CSIM: 4, \"63D0\"", -1, "Unknown error returned '0x63d0'"}
> > };
> >
> > 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);
> > + for (i = 0; i < G_N_ELEMENTS (csim_response_test_list); i++) {
> > + res = mm_telit_parse_csim_response (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) {
> > - g_error_free (error);
> > - error = NULL;
> > + 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_clear_error (&error);
> > }
> > +
> > + g_assert_cmpint (res, ==, csim_response_test_list[i].result);
> > }
> > }
> >
> >
>
>
> --
> Aleksander
> https://aleksander.es
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20170419/f8da3f8d/attachment-0001.html>
More information about the ModemManager-devel
mailing list