On Sat, 2017-03-25 at 21:39 +0100, Aleksander Morgado wrote:
> The method doing the operator name normalization takes as input the
> current configured modem charset. If this is UCS2, we will now just
> assume this is a hint: the string may or may not come in hex/UCS2.
>
> This logic makes the custom operator name loading in Huawei unneeded,
> if the modem is configured in UCS2, we still properly process
> operator
> names coming in plain ASCII.
LGTM.
Dan
> ---
> plugins/huawei/mm-broadband-modem-huawei.c | 51 --------------------
> ----------
> src/mm-modem-helpers.c | 26 +++++++++------
> src/tests/test-modem-helpers.c | 45
> ++++++++++++++++++++++++++
> 3 files changed, 62 insertions(+), 60 deletions(-)
>
> diff --git a/plugins/huawei/mm-broadband-modem-huawei.c
> b/plugins/huawei/mm-broadband-modem-huawei.c
> index 74e680b4..9f13b0f2 100644
> --- a/plugins/huawei/mm-broadband-modem-huawei.c
> +++ b/plugins/huawei/mm-broadband-modem-huawei.c
> @@ -2172,55 +2172,6 @@ modem_3gpp_disable_unsolicited_events
> (MMIfaceModem3gpp *self,
> }
>
> /*******************************************************************
> **********/
> -/* Operator Name loading (3GPP interface) */
> -
> -static gchar *
> -modem_3gpp_load_operator_name_finish (MMIfaceModem3gpp *self,
> - GAsyncResult *res,
> - GError **error)
> -{
> - const gchar *result;
> - gchar *operator_name = NULL;
> -
> - result = mm_base_modem_at_command_finish (MM_BASE_MODEM (self),
> res, error);
> - if (!result)
> - return NULL;
> -
> - if (!mm_3gpp_parse_cops_read_response (result,
> - NULL, /* mode */
> - NULL, /* format */
> - &operator_name,
> - NULL, /* act */
> - error))
> - return NULL;
> -
> - /* Despite +CSCS? may claim supporting UCS2, Huawei modems
> always report the
> - * operator name in ASCII in a +COPS response. Thus, we ignore
> the current
> - * charset claimed by the modem and assume the charset is IRA
> when parsing
> - * the operator name.
> - */
> - mm_3gpp_normalize_operator_name (&operator_name,
> MM_MODEM_CHARSET_IRA);
> - if (operator_name)
> - mm_dbg ("loaded Operator Name: %s", operator_name);
> -
> - return operator_name;
> -}
> -
> -static void
> -modem_3gpp_load_operator_name (MMIfaceModem3gpp *self,
> - GAsyncReadyCallback callback,
> - gpointer user_data)
> -{
> - mm_dbg ("loading Operator Name (huawei)...");
> - mm_base_modem_at_command (MM_BASE_MODEM (self),
> - "+COPS=3,0;+COPS?",
> - 3,
> - FALSE,
> - callback,
> - user_data);
> -}
> -
> -/*******************************************************************
> **********/
> /* Create Bearer (Modem interface) */
>
> typedef struct {
> @@ -4603,8 +4554,6 @@ iface_modem_3gpp_init (MMIfaceModem3gpp *iface)
> iface->enable_unsolicited_events_finish =
> modem_3gpp_enable_unsolicited_events_finish;
> iface->disable_unsolicited_events =
> modem_3gpp_disable_unsolicited_events;
> iface->disable_unsolicited_events_finish =
> modem_3gpp_disable_unsolicited_events_finish;
> - iface->load_operator_name = modem_3gpp_load_operator_name;
> - iface->load_operator_name_finish =
> modem_3gpp_load_operator_name_finish;
> }
>
> static void
> diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
> index fc95e28f..9266a5a0 100644
> --- a/src/mm-modem-helpers.c
> +++ b/src/mm-modem-helpers.c
> @@ -3131,20 +3131,28 @@ mm_3gpp_normalize_operator_name
> (gchar **operator,
> if (*operator == NULL)
> return;
>
> - /* Some modems (Option & HSO) return the operator name as a
> hexadecimal
> - * string of the bytes of the operator name as encoded by the
> current
> - * character set.
> - */
> + /* Despite +CSCS? may claim supporting UCS2, Some modems (e.g.
> Huawei)
> + * always report the operator name in ASCII in a +COPS response.
> */
> if (cur_charset == MM_MODEM_CHARSET_UCS2) {
> + gchar *tmp;
> +
> + tmp = g_strdup (*operator);
> /* In this case we're already checking UTF-8 validity */
> - *operator = mm_charset_take_and_convert_to_utf8 (*operator,
> MM_MODEM_CHARSET_UCS2);
> + tmp = mm_charset_take_and_convert_to_utf8 (tmp,
> cur_charset);
> + if (tmp) {
> + g_clear_pointer (operator, g_free);
> + *operator = tmp;
> + goto out;
> + }
> }
> - /* Ensure the operator name is valid UTF-8 so that we can send
> it
> - * through D-Bus and such.
> - */
> - else if (!g_utf8_validate (*operator, -1, NULL))
> +
> + /* Charset is unknown or there was an error in conversion; try
> to see
> + * if the contents we got are valid UTF-8 already. */
> + if (!g_utf8_validate (*operator, -1, NULL))
> g_clear_pointer (operator, g_free);
>
> +out:
> +
> /* Some modems (Novatel LTE) return the operator name as
> "Unknown" when
> * it fails to obtain the operator name. Return NULL in such
> case.
> */
> diff --git a/src/tests/test-modem-helpers.c b/src/tests/test-modem-
> helpers.c
> index 1fc8c353..98f30f83 100644
> --- a/src/tests/test-modem-helpers.c
> +++ b/src/tests/test-modem-helpers.c
> @@ -866,6 +866,49 @@ test_cops_query (void)
> }
>
> /*******************************************************************
> **********/
> +
> +typedef struct {
> + const gchar *input;
> + MMModemCharset charset;
> + const gchar *normalized;
> +} NormalizeOperatorTest;
> +
> +static const NormalizeOperatorTest normalize_operator_tests[] = {
> + /* charset unknown */
> + { "Verizon", MM_MODEM_CHARSET_UNKNOWN, "Verizon" },
> + /* charset configured as IRA (ASCII) */
> + { "Verizon", MM_MODEM_CHARSET_IRA, "Verizon" },
> + /* charset configured as GSM7 */
> + { "Verizon", MM_MODEM_CHARSET_GSM, "Verizon" },
> + /* charset configured as UCS2 */
> + { "0056006500720069007A006F006E", MM_MODEM_CHARSET_UCS2,
> "Verizon" },
> + { "Verizon", MM_MODEM_CHARSET_UCS2,
> "Verizon" },
> +};
> +
> +static void
> +common_test_normalize_operator (const NormalizeOperatorTest *t)
> +{
> + gchar *str;
> +
> + str = g_strdup (t->input);
> + mm_3gpp_normalize_operator_name (&str, t->charset);
> + if (!t->normalized)
> + g_assert (!str);
> + else
> + g_assert_cmpstr (str, ==, t->normalized);
> + g_free (str);
> +}
> +
> +static void
> +test_normalize_operator (void)
> +{
> + guint i;
> +
> + for (i = 0; i < G_N_ELEMENTS (normalize_operator_tests); i++)
> + common_test_normalize_operator
> (&normalize_operator_tests[i]);
> +}
> +
> +/*******************************************************************
> **********/
> /* Test CREG/CGREG responses and unsolicited messages */
>
> typedef struct {
> @@ -3527,6 +3570,8 @@ int main (int argc, char **argv)
>
> g_test_suite_add (suite, TESTCASE (test_cops_query, NULL));
>
> + g_test_suite_add (suite, TESTCASE (test_normalize_operator,
> NULL));
> +
> g_test_suite_add (suite, TESTCASE (test_creg1_solicited,
> reg_data));
> g_test_suite_add (suite, TESTCASE (test_creg1_unsolicited,
> reg_data));
> g_test_suite_add (suite, TESTCASE (test_creg2_mercury_solicited,
> reg_data));
_______________________________________________
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
