Re: [PATCH 1/2] modem-helpers: if operator not in UCS2, see if already valid UTF-8

2017-04-15 Thread Colin Helliwell 

> On 14 April 2017 at 23:15 Aleksander Morgado  wrote:
> 
> On Fri, Apr 14, 2017 at 4:57 PM, Colin Helliwell
> 
>  wrote:
> 
> > Might have hung this off the wrong thread, but it's along the same lines.
> > I've just noticed that whilst the mmcli status now decodes the operator, 
> > the --sim status doesn't:
> > 
> > # mmcli -m 0
> > ...
> >  3GPP | imei: '358606050452806'
> >  | enabled locks: 'none'
> >  | operator id: '23415'
> >  | operator name: 'vodafone UK'
> >  | subscription: 'unknown'
> > 
> > | registration: 'home'
> > 
> > -
> > ...
> > 
> > # mmcli -m 0 --sim=0
> > 
> > SIM '/org/freedesktop/ModemManager1/SIM/0'
> > 
> > -
> >  Properties | imsi : '234159108784146'
> >  | id : '89441000301658713096'
> >  | operator id : '23415'
> >  | operator name : 'unknown'
> 
> Could you gather debug logs so that we can see whether this is due to
> the string being wrongly decoded or just not provided by the SIM?
> 

Yep, it was something I spotted late on yesterday and fired in in case it was a 
quicky. 
At first I thought they both would have been coming from the same source 
command/response, though when I looked at the code quickly it was seeming 
perhaps not (which your comment also suggests).
 - I'll try to dig into it further today.
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel

Re: [PATCH 1/2] modem-helpers: if operator not in UCS2, see if already valid UTF-8

2017-04-14 Thread Colin Helliwell 
Might have hung this off the wrong thread, but it's along the same lines.
I've just noticed that whilst the mmcli status now decodes the operator, the 
--sim status doesn't:

# mmcli -m 0
...
  3GPP |   imei: '358606050452806'
   |  enabled locks: 'none'
   |operator id: '23415'
   |  operator name: 'vodafone UK'
   |   subscription: 'unknown'
   |   registration: 'home'
  -
...

# mmcli -m 0 --sim=0
SIM '/org/freedesktop/ModemManager1/SIM/0'
  -
  Properties |  imsi : '234159108784146'
 |id : '89441000301658713096'
 |   operator id : '23415'
 | operator name : 'unknown'
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel

Re: [PATCH 1/2] modem-helpers: if operator not in UCS2, see if already valid UTF-8

2017-04-02 Thread Aleksander Morgado 
On 01/04/17 06:01, Dan Williams wrote:
> 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.
> 

Pushed to git master.

> 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 */
>> -   _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 (_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,

Re: [PATCH 1/2] modem-helpers: if operator not in UCS2, see if already valid UTF-8

2017-03-31 Thread Dan Williams 
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 */
> -   _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 (_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.
> -  

[PATCH 1/2] modem-helpers: if operator not in UCS2, see if already valid UTF-8

2017-03-25 Thread Aleksander Morgado 
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.
---
 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 */
-   _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 (_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