[PATCH 1/2] broadband-modem,helpers: implement AT+WS46=? response parser

Dan Williams dcbw at redhat.com
Fri Mar 24 20:11:46 UTC 2017


On Sun, 2017-03-12 at 21:05 +0100, Aleksander Morgado wrote:
> We want a parser that returns the full list of combinations found.

Looks good to me except the comment:

+    /* Fixup the ANY value, based on whether LTE is supported or not

the code under that comment seems to use all modes, not just LTE/4G.

Dan


> ---
>  src/mm-broadband-modem.c       | 111 ++++++++++++-------------------
> -
>  src/mm-modem-helpers.c         | 143
> +++++++++++++++++++++++++++++++++++++++++
>  src/mm-modem-helpers.h         |   3 +
>  src/tests/test-modem-helpers.c | 102 +++++++++++++++++++++++++++++
>  4 files changed, 290 insertions(+), 69 deletions(-)
> 
> diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
> index 17b3253a..aa84fc7e 100644
> --- a/src/mm-broadband-modem.c
> +++ b/src/mm-broadband-modem.c
> @@ -378,17 +378,28 @@ current_capabilities_ws46_test_ready
> (MMBaseModem *self,
>                                        LoadCapabilitiesContext *ctx)
>  {
>      const gchar *response;
> +    GArray      *modes;
> +    guint        i;
>  
>      /* Completely ignore errors in AT+WS46=? */
>      response = mm_base_modem_at_command_finish (self, res, NULL);
> -    if (response &&
> -        (strstr (response, "28") != NULL ||   /* 4G only */
> -         strstr (response, "30") != NULL ||   /* 2G/4G */
> -         strstr (response, "31") != NULL)) {  /* 3G/4G */
> -        /* Add LTE caps */
> -        ctx->caps |= MM_MODEM_CAPABILITY_LTE;
> +    if (!response)
> +        goto out;
> +
> +    modes = mm_3gpp_parse_ws46_test_response (response, NULL);
> +    if (!modes)
> +        goto out;
> +
> +    /* Add LTE caps if any of the reported modes supports 4G */
> +    for (i = 0; i < modes->len; i++) {
> +        if (g_array_index (modes, MMModemMode, i) &
> MM_MODEM_MODE_4G) {
> +            ctx->caps |= MM_MODEM_CAPABILITY_LTE;
> +            break;
> +        }
>      }
> +    g_array_unref (modes);
>  
> +out:
>      g_simple_async_result_set_op_res_gpointer (
>          ctx->result,
>          GUINT_TO_POINTER (ctx->caps),
> @@ -1430,78 +1441,40 @@ supported_modes_ws46_test_ready
> (MMBroadbandModem *self,
>                                   LoadSupportedModesContext *ctx)
>  {
>      const gchar *response;
> -    GError *error = NULL;
> +    GArray      *modes;
> +    GError      *error = NULL;
> +    guint        i;
>  
>      response = mm_base_modem_at_command_finish (MM_BASE_MODEM
> (self), res, &error);
> -    if (!error) {
> -        MMModemMode mode = MM_MODEM_MODE_NONE;
> -
> -        /*
> -         * More than one numeric ID may appear in the list, that's
> why
> -         * they are checked separately.
> -         *
> -         * NOTE: Do not skip WS46 prefix; it would break Cinterion
> handling.
> -         *
> -         * From 3GPP TS 27.007 v.11.2.0, section 5.9
> -         * 12	GSM Digital Cellular Systems (GERAN only)
> -         * 22	UTRAN only
> -         * 25	3GPP Systems (GERAN, UTRAN and E-UTRAN)
> -         * 28	E-UTRAN only
> -         * 29	GERAN and UTRAN
> -         * 30	GERAN and E-UTRAN
> -         * 31	UTRAN and E-UTRAN
> -         */
> -
> -        if (strstr (response, "12") != NULL) {
> -            mm_dbg ("Device allows (3GPP) 2G-only network mode");
> -            mode |= MM_MODEM_MODE_2G;
> -        }
> -
> -        if (strstr (response, "22") != NULL) {
> -            mm_dbg ("Device allows (3GPP) 3G-only network mode");
> -            mode |= MM_MODEM_MODE_3G;
> -        }
> -
> -        if (strstr (response, "28") != NULL) {
> -            mm_dbg ("Device allows (3GPP) 4G-only network mode");
> -            mode |= MM_MODEM_MODE_4G;
> -        }
> +    if (error) {
> +        mm_dbg ("Generic query of supported 3GPP networks with
> WS46=? failed: '%s'", error->message);
> +        g_error_free (error);
> +        goto out;
> +    }
>  
> -        if (strstr (response, "29") != NULL) {
> -            mm_dbg ("Device allows (3GPP) 2G/3G network mode");
> -            mode |= (MM_MODEM_MODE_2G | MM_MODEM_MODE_3G);
> -        }
> +    modes = mm_3gpp_parse_ws46_test_response (response, &error);
> +    if (!modes) {
> +        mm_dbg ("Parsing WS46=? response failed: '%s'", error-
> >message);
> +        g_error_free (error);
> +        goto out;
> +    }
>  
> -        if (strstr (response, "30") != NULL) {
> -            mm_dbg ("Device allows (3GPP) 2G/4G network mode");
> -            mode |= (MM_MODEM_MODE_2G | MM_MODEM_MODE_4G);
> -        }
> +    for (i = 0; i < modes->len; i++) {
> +        MMModemMode  mode;
> +        gchar       *str;
>  
> -        if (strstr (response, "31") != NULL) {
> -            mm_dbg ("Device allows (3GPP) 3G/4G network mode");
> -            mode |= (MM_MODEM_MODE_3G | MM_MODEM_MODE_4G);
> -        }
> +        mode = g_array_index (modes, MMModemMode, i);
>  
> -        if (strstr (response, "25") != NULL) {
> -            if (mm_iface_modem_is_3gpp_lte (MM_IFACE_MODEM (self)))
> {
> -                mm_dbg ("Device allows every supported 3GPP network
> mode (2G/3G/4G)");
> -                mode |= (MM_MODEM_MODE_2G | MM_MODEM_MODE_3G |
> MM_MODEM_MODE_4G);
> -            } else {
> -                mm_dbg ("Device allows every supported 3GPP network
> mode (2G/3G)");
> -                mode |= (MM_MODEM_MODE_2G | MM_MODEM_MODE_3G);
> -            }
> -        }
> +        ctx->mode |= mode;
>  
> -        /* If no expected ID found, log error */
> -        if (mode == MM_MODEM_MODE_NONE)
> -            mm_dbg ("Invalid list of supported networks reported by
> WS46=?: '%s'", response);
> -        else
> -            ctx->mode |= mode;
> -    } else {
> -        mm_dbg ("Generic query of supported 3GPP networks with
> WS46=? failed: '%s'", error->message);
> -        g_error_free (error);
> +        str = mm_modem_mode_build_string_from_mask (mode);
> +        mm_dbg ("Device allows (3GPP) mode combination: %s", str);
> +        g_free (str);
>      }
>  
> +    g_array_unref (modes);
> +
> +out:
>      /* Now keep on with the loading, we may need CDMA-specific
> checks */
>      ctx->run_ws46 = FALSE;
>      load_supported_modes_step (ctx);
> diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
> index bc6e48b4..b1c3326c 100644
> --- a/src/mm-modem-helpers.c
> +++ b/src/mm-modem-helpers.c
> @@ -725,6 +725,149 @@ mm_3gpp_cds_regex_get (void)
>  }
>  
>  /*******************************************************************
> ******/
> +/* AT+WS46=? response parser
> + *
> + * More than one numeric ID may appear in the list, that's why
> + * they are checked separately.
> + *
> + * NOTE: ignore WS46 prefix or it will break Cinterion handling.
> + *
> + * For the specific case of '25', we will check if any other mode
> supports
> + * 4G, and if there is none, we'll remove 4G caps from it.
> + */
> +
> +typedef struct {
> +    guint       ws46;
> +    MMModemMode mode;
> +} Ws46Mode;
> +
> +/* 3GPP TS 27.007 r14, section 5.9: select wireless network +WS46 */
> +static const Ws46Mode ws46_modes[] = {
> +    /* GSM Digital Cellular Systems (GERAN only) */
> +    { 12, MM_MODEM_MODE_2G },
> +    /* UTRAN only */
> +    { 22, MM_MODEM_MODE_3G },
> +    /* 3GPP Systems (GERAN, UTRAN and E-UTRAN) */
> +    { 25, MM_MODEM_MODE_ANY },
> +    /* E-UTRAN only */
> +    { 28, MM_MODEM_MODE_4G },
> +    /* GERAN and UTRAN */
> +    { 29, MM_MODEM_MODE_2G | MM_MODEM_MODE_3G },
> +    /* GERAN and E-UTRAN */
> +    { 30, MM_MODEM_MODE_2G | MM_MODEM_MODE_4G },
> +    /* UERAN and E-UTRAN */
> +    { 31, MM_MODEM_MODE_3G | MM_MODEM_MODE_4G },
> +};
> +
> +GArray *
> +mm_3gpp_parse_ws46_test_response (const gchar  *response,
> +                                  GError      **error)
> +{
> +    GArray     *modes = NULL;
> +    GRegex     *r;
> +    GError     *inner_error = NULL;
> +    GMatchInfo *match_info = NULL;
> +    gchar      *full_list = NULL;
> +    gchar     **split;
> +    guint       i;
> +    gboolean    supported_4g = FALSE;
> +    gboolean    supported_3g = FALSE;
> +    gboolean    supported_2g = FALSE;
> +
> +    r = g_regex_new ("(?:\\+WS46:)?\\s*\\((.*)\\)(?:\\r\\n)?", 0, 0,
> NULL);
> +    g_assert (r != NULL);
> +
> +    g_regex_match_full (r, response, strlen (response), 0, 0,
> &match_info, &inner_error);
> +    if (inner_error)
> +        goto out;
> +
> +    if (!g_match_info_matches (match_info)) {
> +        inner_error = g_error_new (MM_CORE_ERROR,
> MM_CORE_ERROR_FAILED, "Couldn't match response");
> +        goto out;
> +    }
> +
> +    if (!(full_list = mm_get_string_unquoted_from_match_info
> (match_info, 1))) {
> +        inner_error = g_error_new (MM_CORE_ERROR,
> MM_CORE_ERROR_FAILED, "Error parsing full list string");
> +        goto out;
> +    }
> +
> +    split = g_strsplit (full_list, ",", -1);
> +    modes = g_array_new (FALSE, FALSE, sizeof (MMModemMode));
> +
> +    for (i = 0; split && split[i]; i++) {
> +        guint val;
> +        guint j;
> +
> +        if (!mm_get_uint_from_str (split[i], &val)) {
> +            g_warning ("Invalid +WS46 mode reported: %s", split[i]);
> +            continue;
> +        }
> +
> +        for (j = 0; j < G_N_ELEMENTS (ws46_modes); j++) {
> +            if (ws46_modes[j].ws46 == val) {
> +                if (val != 25) {
> +                    if (ws46_modes[j].mode & MM_MODEM_MODE_4G)
> +                        supported_4g = TRUE;
> +                    if (ws46_modes[j].mode & MM_MODEM_MODE_3G)
> +                        supported_3g = TRUE;
> +                    if (ws46_modes[j].mode & MM_MODEM_MODE_2G)
> +                        supported_2g = TRUE;
> +                }
> +                g_array_append_val (modes, ws46_modes[j].mode);
> +                break;
> +            }
> +        }
> +
> +        if (j == G_N_ELEMENTS (ws46_modes))
> +            g_warning ("Unknown +WS46 mode reported: %s", split[i]);
> +    }
> +
> +    g_strfreev (split);
> +
> +    if (modes->len == 0) {
> +        inner_error = g_error_new (MM_CORE_ERROR,
> MM_CORE_ERROR_FAILED, "No valid modes reported");
> +        g_clear_pointer (&modes, g_array_unref);
> +        goto out;
> +    }
> +
> +    /* Fixup the ANY value, based on whether LTE is supported or not
> */
> +    for (i = 0; i < modes->len; i++) {
> +        MMModemMode *mode;
> +
> +        mode = &g_array_index (modes, MMModemMode, i);
> +        if (*mode == MM_MODEM_MODE_ANY) {
> +            *mode = 0;
> +            if (supported_2g)
> +                *mode |= MM_MODEM_MODE_2G;
> +            if (supported_3g)
> +                *mode |= MM_MODEM_MODE_3G;
> +            if (supported_4g)
> +                *mode |= MM_MODEM_MODE_4G;
> +
> +            if (*mode == 0) {
> +                inner_error = g_error_new (MM_CORE_ERROR,
> MM_CORE_ERROR_FAILED, "No way to fixup the ANY value");
> +                g_clear_pointer (&modes, g_array_unref);
> +                goto out;
> +            }
> +        }
> +    }
> +
> +out:
> +    g_free (full_list);
> +
> +    g_clear_pointer (&match_info, g_match_info_free);
> +    g_regex_unref (r);
> +
> +    if (inner_error) {
> +        g_propagate_error (error, inner_error);
> +        return NULL;
> +    }
> +
> +    g_assert (modes && modes->len);
> +    return modes;
> +}
> +
> +/*******************************************************************
> ******/
>  
>  static void
>  mm_3gpp_network_info_free (MM3gppNetworkInfo *info)
> diff --git a/src/mm-modem-helpers.h b/src/mm-modem-helpers.h
> index fb7982fe..33af48b6 100644
> --- a/src/mm-modem-helpers.h
> +++ b/src/mm-modem-helpers.h
> @@ -106,6 +106,9 @@ GRegex    *mm_3gpp_cusd_regex_get (void);
>  GRegex    *mm_3gpp_cmti_regex_get (void);
>  GRegex    *mm_3gpp_cds_regex_get (void);
>  
> +/* AT+WS46=? response parser: returns array of MMModemMode values */
> +GArray *mm_3gpp_parse_ws46_test_response (const gchar  *response,
> +                                          GError      **error);
>  
>  /* AT+COPS=? (network scan) response parser */
>  typedef struct {
> diff --git a/src/tests/test-modem-helpers.c b/src/tests/test-modem-
> helpers.c
> index fdcac10e..1fc8c353 100644
> --- a/src/tests/test-modem-helpers.c
> +++ b/src/tests/test-modem-helpers.c
> @@ -33,6 +33,102 @@
>      g_assert_cmpfloat (fabs (val1 - val2), <, tolerance)
>  
>  /*******************************************************************
> **********/
> +/* Test WS46=? responses */
> +
> +static void
> +test_ws46_response (const gchar       *str,
> +                    const MMModemMode *expected,
> +                    guint              n_expected)
> +{
> +    guint   i;
> +    GArray *modes;
> +    GError *error = NULL;
> +
> +    modes = mm_3gpp_parse_ws46_test_response (str, &error);
> +    g_assert_no_error (error);
> +    g_assert (modes != NULL);
> +    g_assert_cmpuint (modes->len, ==, n_expected);
> +
> +    for (i = 0; i < n_expected; i++) {
> +        guint j;
> +
> +        for (j = 0; j < modes->len; j++) {
> +            if (expected[i] == g_array_index (modes, MMModemMode,
> j))
> +                break;
> +        }
> +        g_assert_cmpuint (j, !=, modes->len);
> +    }
> +    g_array_unref (modes);
> +}
> +
> +static void
> +test_ws46_response_generic_2g3g4g (void)
> +{
> +    static const MMModemMode expected[] = {
> +        MM_MODEM_MODE_2G,
> +        MM_MODEM_MODE_3G,
> +        MM_MODEM_MODE_2G | MM_MODEM_MODE_3G | MM_MODEM_MODE_4G,
> +        MM_MODEM_MODE_4G,
> +        MM_MODEM_MODE_2G | MM_MODEM_MODE_3G,
> +    };
> +    const gchar *str = "+WS46: (12,22,25,28,29)";
> +
> +    test_ws46_response (str, expected, G_N_ELEMENTS (expected));
> +}
> +
> +static void
> +test_ws46_response_generic_2g3g (void)
> +{
> +    static const MMModemMode expected[] = {
> +        MM_MODEM_MODE_2G,
> +        MM_MODEM_MODE_3G,
> +        MM_MODEM_MODE_2G | MM_MODEM_MODE_3G,
> +    };
> +    const gchar *str = "+WS46: (12,22,25)";
> +
> +    test_ws46_response (str, expected, G_N_ELEMENTS (expected));
> +}
> +
> +static void
> +test_ws46_response_generic_2g3g_v2 (void)
> +{
> +    static const MMModemMode expected[] = {
> +        MM_MODEM_MODE_2G,
> +        MM_MODEM_MODE_3G,
> +        MM_MODEM_MODE_2G | MM_MODEM_MODE_3G,
> +    };
> +    const gchar *str = "+WS46: (12,22,29)";
> +
> +    test_ws46_response (str, expected, G_N_ELEMENTS (expected));
> +}
> +
> +static void
> +test_ws46_response_cinterion (void)
> +{
> +    static const MMModemMode expected[] = {
> +        MM_MODEM_MODE_2G,
> +        MM_MODEM_MODE_3G,
> +        MM_MODEM_MODE_2G | MM_MODEM_MODE_3G | MM_MODEM_MODE_4G,
> +        MM_MODEM_MODE_4G,
> +        MM_MODEM_MODE_2G | MM_MODEM_MODE_3G,
> +    };
> +    const gchar *str = "(12,22,25,28,29)";
> +
> +    test_ws46_response (str, expected, G_N_ELEMENTS (expected));
> +}
> +
> +static void
> +test_ws46_response_telit_le866 (void)
> +{
> +    static const MMModemMode expected[] = {
> +        MM_MODEM_MODE_4G,
> +    };
> +    const gchar *str = "(28)";
> +
> +    test_ws46_response (str, expected, G_N_ELEMENTS (expected));
> +}
> +
> +/*******************************************************************
> **********/
>  /* Test CMGL responses */
>  
>  static void
> @@ -3392,6 +3488,12 @@ int main (int argc, char **argv)
>      suite = g_test_get_root ();
>      reg_data = reg_test_data_new ();
>  
> +    g_test_suite_add (suite, TESTCASE
> (test_ws46_response_generic_2g3g4g, NULL));
> +    g_test_suite_add (suite, TESTCASE
> (test_ws46_response_generic_2g3g, NULL));
> +    g_test_suite_add (suite, TESTCASE
> (test_ws46_response_generic_2g3g_v2, NULL));
> +    g_test_suite_add (suite, TESTCASE (test_ws46_response_cinterion,
> NULL));
> +    g_test_suite_add (suite, TESTCASE
> (test_ws46_response_telit_le866, NULL));
> +
>      g_test_suite_add (suite, TESTCASE (test_cops_response_tm506,
> NULL));
>      g_test_suite_add (suite, TESTCASE (test_cops_response_gt3gplus,
> NULL));
>      g_test_suite_add (suite, TESTCASE (test_cops_response_ac881,
> NULL));


More information about the ModemManager-devel mailing list