[MM] Question regarding minimum probing time in MMPluginManager

2013-06-28 Thread Ben Chan
The following code in MMPluginManager seems to always wait until the
minimum probing time is consumed.

/* If we didn't use the minimum probing time, wait for it to finish */
else if (ctx-timeout_id  0) {
mm_dbg ((Plugin Manager) '%s' port probe finished, last one
in device, 
but minimum probing time not consumed yet ('%lf'
seconds elapsed),
g_udev_device_get_name (port_probe_ctx-port),
g_timer_elapsed (ctx-timer, NULL));

For a modem with known port layout (assuming we have a way to specify
in the plugin), once all port probes finish, should MMPluginManager
proceed without waiting for the minimum probing time to expire? Is
there a potential downside? The obvious upside is cutting the modem
startup time.

Thanks,
Ben
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] Question regarding minimum probing time in MMPluginManager

2013-06-28 Thread Ben Chan
Aleksander,

As always, we're very motivated to shave a second or a few hundred
milliseconds off the modem startup time :)

I guess a simple solution is to let a plugin specify the max number of
ports N, such that the plugin manager will skip the minimum probing
time after seeing the completion of N port probes.   A more generic
solution would be adding a hook in the plugin to decide whether or not
to end the probing process after each port probe finishes.  How do you
think?

Thanks,
Ben

On Fri, Jun 28, 2013 at 12:13 AM, Aleksander Morgado
aleksan...@lanedo.com wrote:

 The following code in MMPluginManager seems to always wait until the
 minimum probing time is consumed.

 /* If we didn't use the minimum probing time, wait for it to finish */
 else if (ctx-timeout_id  0) {
 mm_dbg ((Plugin Manager) '%s' port probe finished, last one
 in device, 
 but minimum probing time not consumed yet ('%lf'
 seconds elapsed),
 g_udev_device_get_name (port_probe_ctx-port),
 g_timer_elapsed (ctx-timer, NULL));

 For a modem with known port layout (assuming we have a way to specify
 in the plugin), once all port probes finish, should MMPluginManager
 proceed without waiting for the minimum probing time to expire? Is
 there a potential downside? The obvious upside is cutting the modem
 startup time.


 This is just to let the kernel some time to expose all the ports, and
 should only be a couple of seconds max. If the first probe is exposed
 and right away probed, and there is no other port around when the
 probing finishes, the whole device probing is assumed finished. Newer
 ports appearing afterwards should still get grabbed by the already
 created modem, but these ports will not take part in the probing
 decisions. But of course, if we have a way to specify the expected port
 layout in the plugin itself (in very device-specific plugins like the
 novatel-lte or altair-lte), then there is no point in waiting the
 minimum probing time if we already got the expected layout, of course.

 --
 Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] novatel-lte: propagate error when load_current_bands fails

2013-06-27 Thread Ben Chan
This patch fixes the following crash when
MMIfaceModem::load_current_bands_ready() dereferences a NULL GError
pointer, which happens when the novatel-lte plugin fails to load the
current bands but does not propagate the error.

Thread 0 *CRASHED* ( SIGSEGV @ 0x )

0x7f04d6c89c36   [ModemManager]- mm-iface-modem.c:3886  
load_current_bands_ready
0x7f04d6942236   [libgio-2.0.so.0.3200.4]  - gsimpleasyncresult.c:767   
g_simple_async_result_complete
0x7f04d6942338   [libgio-2.0.so.0.3200.4]  - gsimpleasyncresult.c:779   
complete_in_idle_cb
0x7f04d67fad74   [libglib-2.0.so.0.3200.4] - gmain.c:2539   
g_main_context_dispatch
0x7f04d67fb0f7   [libglib-2.0.so.0.3200.4] - gmain.c:3146   
g_main_context_iterate
0x7f04d67fb551   [libglib-2.0.so.0.3200.4] - gmain.c:3340   
g_main_loop_run
0x7f04d6c68795   [ModemManager]- main.c:142]main
0x7f04d6213464   [libc-2.15.so]- libc-start.c:234]  
__libc_start_main
0x7f04d6c68318   [ModemManager]+ 0x0001d318]
---
 plugins/novatel/mm-broadband-modem-novatel-lte.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c 
b/plugins/novatel/mm-broadband-modem-novatel-lte.c
index 0db1686..8cb127a 100644
--- a/plugins/novatel/mm-broadband-modem-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c
@@ -376,7 +376,9 @@ load_current_bands_finish (MMIfaceModem *self,
GAsyncResult *res,
GError **error)
 {
-/* Never fails */
+if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), 
error))
+return NULL;
+
 return (GArray *) g_array_ref (g_simple_async_result_get_op_res_gpointer (
G_SIMPLE_ASYNC_RESULT (res)));
 }
-- 
1.8.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] altair-lte: propagate error when load_{supported, current}_bands fails

2013-06-27 Thread Ben Chan
This patch fixes a potential crash when
MMIfaceModem::load_current_bands_ready() dereferences a NULL GError
pointer, which happens when the altair-lte plugin fails to load the
current bands but does not propagate the error. It also fixes a similar
issue with the plugin fails to load the supported bands, even though
MMIfaceModem::load_supported_bands_ready() checks for a NULL GError
pointer.
---
 plugins/altair/mm-broadband-modem-altair-lte.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/plugins/altair/mm-broadband-modem-altair-lte.c 
b/plugins/altair/mm-broadband-modem-altair-lte.c
index 4eeae1c..a2ae2ba 100644
--- a/plugins/altair/mm-broadband-modem-altair-lte.c
+++ b/plugins/altair/mm-broadband-modem-altair-lte.c
@@ -286,7 +286,9 @@ load_supported_bands_finish (MMIfaceModem *self,
  GAsyncResult *res,
  GError **error)
 {
-/* Never fails */
+if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), 
error))
+return NULL;
+
 return (GArray *) g_array_ref (g_simple_async_result_get_op_res_gpointer (
G_SIMPLE_ASYNC_RESULT (res)));
 }
@@ -365,7 +367,9 @@ load_current_bands_finish (MMIfaceModem *self,
GAsyncResult *res,
GError **error)
 {
-/* Never fails */
+if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), 
error))
+return NULL;
+
 return (GArray *) g_array_ref (g_simple_async_result_get_op_res_gpointer (
G_SIMPLE_ASYNC_RESULT (res)));
 }
-- 
1.8.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] altair-lte: avoid sending ATZ when enabling the modem

2013-06-26 Thread Ben Chan
This patch prevents an ATZ command, which causes the modem to reboot,
from being sent to the modem when the modem is being enabled.
---
 plugins/altair/mm-broadband-modem-altair-lte.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/plugins/altair/mm-broadband-modem-altair-lte.c 
b/plugins/altair/mm-broadband-modem-altair-lte.c
index e645776..4eeae1c 100644
--- a/plugins/altair/mm-broadband-modem-altair-lte.c
+++ b/plugins/altair/mm-broadband-modem-altair-lte.c
@@ -1044,4 +1044,11 @@ mm_broadband_modem_altair_lte_class_init 
(MMBroadbandModemAltairLteClass *klass)
 g_type_class_add_private (object_class, sizeof 
(MMBroadbandModemAltairLtePrivate));
 
 broadband_modem_class-setup_ports = setup_ports;
+
+/* The Altair LTE modem reboots itself upon receiving an ATZ command. We
+ * need to skip the default implementation in MMBroadbandModem to prevent
+ * an ATZ command from being issued as part of the modem initialization
+ * sequence when enabling the modem. */
+broadband_modem_class-enabling_modem_init = NULL;
+broadband_modem_class-enabling_modem_init_finish = NULL;
 }
-- 
1.8.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[mobile-broadband-provider-info PATCH] se: add Teliasonera

2013-06-23 Thread Ben Chan
---
 serviceproviders.xml | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/serviceproviders.xml b/serviceproviders.xml
index 6809f90..42912fb 100644
--- a/serviceproviders.xml
+++ b/serviceproviders.xml
@@ -9214,6 +9214,16 @@ conceived.
 
 !-- Sweden --
 country code=se
+   provider
+   nameTeliasonera/name
+   gsm
+   network-id mcc=240 mnc=01/
+   apn value=online.telia.se
+   plan type=postpaid/
+   usage type=internet/
+   /apn
+   /gsm
+   /provider
provider primary=true
name3/name
gsm
-- 
1.8.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[mobile-broadband-provider-info PATCH] in: add mtnl.net APN for MTNL

2013-06-23 Thread Ben Chan
---
 serviceproviders.xml | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/serviceproviders.xml b/serviceproviders.xml
index 6809f90..d68b55b 100644
--- a/serviceproviders.xml
+++ b/serviceproviders.xml
@@ -5305,6 +5305,16 @@ conceived.
gsm
network-id mcc=404 mnc=68/
network-id mcc=404 mnc=69/
+   apn value=mtnl.net
+   usage type=internet/
+   nameDelhi (3G Prepaid / Postpaid)/name
+   usernamemtnl/username
+   passwordmtnl123/password
+   /apn
+   apn value=mtnl.net
+   usage type=internet/
+   nameMumbai (3G Prepaid / Postpaid)/name
+   /apn
apn value=gprsmtnldel
plan type=postpaid/
usage type=internet/
-- 
1.8.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH v3] novatel-lte: increase number of retries for connection status checks

2013-05-28 Thread Ben Chan
This patch increases the number of retries, from 4 to 60, for connection
status check during a connection / disconnection request, which handles
some scenario when the connection / disconnection request takes more
than 5 seconds to complete.
---
 plugins/novatel/mm-broadband-bearer-novatel-lte.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c 
b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
index 31c5650..e0227da 100644
--- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
@@ -281,7 +281,7 @@ connect_3gpp (MMBroadbandBearer *self,
  callback,
  user_data,
  connect_3gpp);
-ctx-retries = 4;
+ctx-retries = 60;
 
 /* Get a 'net' data port */
 ctx-data = mm_base_modem_get_best_data_port (MM_BASE_MODEM (modem),
@@ -333,7 +333,7 @@ detailed_disconnect_context_new (MMBroadbandBearer *self,
  callback,
  user_data,
  detailed_disconnect_context_new);
-ctx-retries = 4;
+ctx-retries = 60;
 return ctx;
 }
 
-- 
1.8.2.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] novatel-lte: increase number of retries for connection status checks

2013-05-28 Thread Ben Chan
Sounds good to me. Changed to 60s.

Ben


On Tue, May 28, 2013 at 12:47 AM, Aleksander Morgado
aleksan...@lanedo.comwrote:

 On 28/05/13 06:04, Ben Chan wrote:
  60s seems a bit excessive as users most likely give up earlier.  Revised
  the patch to have a 30s timeout.
 

 Remember that MM lets the connection attempt to be cancelled at any time
 by Disconnect()-ing the Bearer during the connection attempt (best
 effort is done to perform the disconnection), so connection managers can
 actually let users give up whenever they want. Actually, IIRC there's a
 120s timeout in NetworkManager by default (to consider not only the
 dialling, but also additional steps in SimpleConnect() like
 authentication and such), with an effective timeout at the end of 60s in
 most cases as that's the timeout MM uses by default for the dialling
 step. Shouldn't we try to have the same timeout in MM side in all
 plugins? (i.e. 60s). What do you think?



  Thanks,
  Ben
 
 
  On Mon, May 27, 2013 at 12:52 AM, Aleksander Morgado
  aleksan...@lanedo.com mailto:aleksan...@lanedo.com wrote:
 
  On 26/05/13 08:45, Ben Chan wrote:
   This patch increases the number of retries, from 4 to 10, for
  connection
   status check during a connection / disconnection request, which
  handles
   some scenario when the connection / disconnection request takes
 more
   than 5 seconds to complete.
 
  We recently updated most plugins and the generic implementation to
 wait
  up to 60s to get the connection up. Do you really think that 10s is
  enough?
 
 
   ---
plugins/novatel/mm-broadband-bearer-novatel-lte.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
  
   diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c
  b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
   index 31c5650..677bc3c 100644
   --- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c
   +++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
   @@ -281,7 +281,7 @@ connect_3gpp (MMBroadbandBearer *self,
 callback,
 user_data,
 connect_3gpp);
   -ctx-retries = 4;
   +ctx-retries = 10;
  
/* Get a 'net' data port */
ctx-data = mm_base_modem_get_best_data_port (MM_BASE_MODEM
  (modem),
   @@ -333,7 +333,7 @@ detailed_disconnect_context_new
  (MMBroadbandBearer *self,
 callback,
 user_data,
  
  detailed_disconnect_context_new);
   -ctx-retries = 4;
   +ctx-retries = 10;
return ctx;
}
  
  
 
 
  --
  Aleksander
 
 


 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH v2] novatel-lte: normalize QMI status when included in DBus error message

2013-05-27 Thread Ben Chan
This patches normalize a response for the AT$NWQMISTATUS command, by
replacing white-space characters with a space, before the response is
included in a DBus error message.
---
 plugins/novatel/mm-broadband-bearer-novatel-lte.c | 33 ---
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c 
b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
index 31c5650..a28db66 100644
--- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
@@ -42,6 +42,22 @@ struct _MMBroadbandBearerNovatelLtePrivate {
 guint connection_poller;
 };
 
+static gchar *
+normalize_qmistatus (const gchar *status)
+{
+gchar *normalized_status, *iter;
+
+if (!status)
+return NULL;
+
+normalized_status = g_strdup (status);
+for (iter = normalized_status; *iter; iter++)
+if (g_ascii_isspace (*iter))
+*iter = ' ';
+
+return normalized_status;
+}
+
 /*/
 /* 3GPP Connection sequence */
 
@@ -146,6 +162,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem,
   DetailedConnectContext *ctx)
 {
 const gchar *result;
+gchar *normalized_result;
 GError *error = NULL;
 
 result = mm_base_modem_at_command_finish (MM_BASE_MODEM (ctx-modem),
@@ -188,10 +205,13 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem,
 }
 
 /* Already exhausted all retries */
+normalized_result = normalize_qmistatus (result);
 g_simple_async_result_set_error (ctx-result,
  MM_CORE_ERROR,
  MM_CORE_ERROR_FAILED,
- %s, result);
+ QMI connect failed: %s,
+ normalized_result);
+g_free (normalized_result);
 detailed_connect_context_complete_and_free (ctx);
 }
 
@@ -385,6 +405,7 @@ disconnect_3gpp_status_ready (MMBaseModem *modem,
 } else {
 mm_dbg (QMI connection status failed: %s, error-message);
 g_error_free (error);
+result = Unknown error;
 }
 
 if (ctx-retries  0) {
@@ -398,13 +419,17 @@ disconnect_3gpp_status_ready (MMBaseModem *modem,
 /* If $NWQMISTATUS reports a CONNECTED QMI state, returns an error such 
that
  * the modem state remains 'connected'. Otherwise, assumes the modem is
  * disconnected from the network successfully. */
-if (is_connected)
+if (is_connected) {
+gchar *normalized_result;
+
+normalized_result = normalize_qmistatus (result);
 g_simple_async_result_set_error (ctx-result,
  MM_CORE_ERROR,
  MM_CORE_ERROR_FAILED,
  QMI disconnect failed: %s,
- result);
-else
+ normalized_result);
+g_free (normalized_result);
+} else
 g_simple_async_result_set_op_res_gboolean (ctx-result, TRUE);
 
 detailed_disconnect_context_complete_and_free (ctx);
-- 
1.8.2.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH v2] novatel-lte: increase number of retries for connection status checks

2013-05-27 Thread Ben Chan
This patch increases the number of retries, from 4 to 30, for connection
status check during a connection / disconnection request, which handles
some scenario when the connection / disconnection request takes more
than 5 seconds to complete.
---
 plugins/novatel/mm-broadband-bearer-novatel-lte.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c 
b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
index 31c5650..e0227da 100644
--- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
@@ -281,7 +281,7 @@ connect_3gpp (MMBroadbandBearer *self,
  callback,
  user_data,
  connect_3gpp);
-ctx-retries = 4;
+ctx-retries = 30;
 
 /* Get a 'net' data port */
 ctx-data = mm_base_modem_get_best_data_port (MM_BASE_MODEM (modem),
@@ -333,7 +333,7 @@ detailed_disconnect_context_new (MMBroadbandBearer *self,
  callback,
  user_data,
  detailed_disconnect_context_new);
-ctx-retries = 4;
+ctx-retries = 30;
 return ctx;
 }
 
-- 
1.8.2.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] novatel-lte: increase number of retries for connection status checks

2013-05-27 Thread Ben Chan
60s seems a bit excessive as users most likely give up earlier.  Revised
the patch to have a 30s timeout.

Thanks,
Ben


On Mon, May 27, 2013 at 12:52 AM, Aleksander Morgado
aleksan...@lanedo.comwrote:

 On 26/05/13 08:45, Ben Chan wrote:
  This patch increases the number of retries, from 4 to 10, for connection
  status check during a connection / disconnection request, which handles
  some scenario when the connection / disconnection request takes more
  than 5 seconds to complete.

 We recently updated most plugins and the generic implementation to wait
 up to 60s to get the connection up. Do you really think that 10s is enough?


  ---
   plugins/novatel/mm-broadband-bearer-novatel-lte.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
 
  diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c
 b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
  index 31c5650..677bc3c 100644
  --- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c
  +++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
  @@ -281,7 +281,7 @@ connect_3gpp (MMBroadbandBearer *self,
callback,
user_data,
connect_3gpp);
  -ctx-retries = 4;
  +ctx-retries = 10;
 
   /* Get a 'net' data port */
   ctx-data = mm_base_modem_get_best_data_port (MM_BASE_MODEM (modem),
  @@ -333,7 +333,7 @@ detailed_disconnect_context_new (MMBroadbandBearer
 *self,
callback,
user_data,
 
 detailed_disconnect_context_new);
  -ctx-retries = 4;
  +ctx-retries = 10;
   return ctx;
   }
 
 


 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] novatel-lte: normalize QMI status when included in DBus error message

2013-05-26 Thread Ben Chan
This patches normalize a response for the AT$NWQMISTATUS command, by
replacing white-space characters with a space, before the response is
included in a DBus error message.
---
 plugins/novatel/mm-broadband-bearer-novatel-lte.c | 41 ++-
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c 
b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
index 31c5650..6329414 100644
--- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
@@ -42,6 +42,22 @@ struct _MMBroadbandBearerNovatelLtePrivate {
 guint connection_poller;
 };
 
+static gchar *
+normalize_qmistatus (const gchar *status)
+{
+gchar *normalized_status, *iter;
+
+if (!status)
+return NULL;
+
+normalized_status = g_strdup (status);
+for (iter = normalized_status; *iter; iter++)
+if (g_ascii_isspace (*iter))
+*iter = ' ';
+
+return normalized_status;
+}
+
 /*/
 /* 3GPP Connection sequence */
 
@@ -145,12 +161,12 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem,
   GAsyncResult *res,
   DetailedConnectContext *ctx)
 {
-const gchar *result;
+gchar *result;
 GError *error = NULL;
 
-result = mm_base_modem_at_command_finish (MM_BASE_MODEM (ctx-modem),
-  res,
-  error);
+result = normalize_qmistatus (mm_base_modem_at_command_finish 
(MM_BASE_MODEM (ctx-modem),
+   res,
+   error));
 if (!result) {
 mm_warn (QMI connection status failed: %s, error-message);
 if (!g_error_matches (error, MM_MOBILE_EQUIPMENT_ERROR, 
MM_MOBILE_EQUIPMENT_ERROR_UNKNOWN)) {
@@ -159,7 +175,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem,
 return;
 }
 g_error_free (error);
-result = Unknown error;
+result = g_strdup (Unknown error);
 } else if (is_qmistatus_connected (result)) {
 MMBearerIpConfig *config;
 
@@ -175,6 +191,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem,
 (GDestroyNotify)mm_bearer_connect_result_unref);
 g_object_unref (config);
 detailed_connect_context_complete_and_free (ctx);
+g_free (result);
 return;
 }
 
@@ -184,6 +201,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem,
 mm_dbg (Retrying status check in a second. %d retries left.,
 ctx-retries);
 g_timeout_add_seconds (1, (GSourceFunc)connect_3gpp_qmistatus, ctx);
+g_free (result);
 return;
 }
 
@@ -193,6 +211,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem,
  MM_CORE_ERROR_FAILED,
  %s, result);
 detailed_connect_context_complete_and_free (ctx);
+g_free (result);
 }
 
 static gboolean
@@ -366,18 +385,19 @@ disconnect_3gpp_status_ready (MMBaseModem *modem,
   GAsyncResult *res,
   DetailedDisconnectContext *ctx)
 {
-const gchar *result;
+gchar *result;
 GError *error = NULL;
 gboolean is_connected = FALSE;
 
-result = mm_base_modem_at_command_finish (MM_BASE_MODEM (modem),
-  res,
-  error);
+result = normalize_qmistatus (mm_base_modem_at_command_finish 
(MM_BASE_MODEM (modem),
+   res,
+   error));
 if (result) {
 mm_dbg (QMI connection status: %s, result);
 if (is_qmistatus_disconnected (result)) {
 g_simple_async_result_set_op_res_gboolean (ctx-result, TRUE);
 detailed_disconnect_context_complete_and_free (ctx);
+g_free (result);
 return;
 } else if (is_qmistatus_connected (result)) {
 is_connected = TRUE;
@@ -385,6 +405,7 @@ disconnect_3gpp_status_ready (MMBaseModem *modem,
 } else {
 mm_dbg (QMI connection status failed: %s, error-message);
 g_error_free (error);
+result = g_strdup (Unknown error);
 }
 
 if (ctx-retries  0) {
@@ -392,6 +413,7 @@ disconnect_3gpp_status_ready (MMBaseModem *modem,
 mm_dbg (Retrying status check in a second. %d retries left.,
 ctx-retries);
 g_timeout_add_seconds (1, (GSourceFunc)disconnect_3gpp_qmistatus, ctx);
+g_free (result);
 return;
 }
 
@@ -408,6 +430,7 @@ disconnect_3gpp_status_ready (MMBaseModem *modem,
 g_simple_async_result_set_op_res_gboolean (ctx-result, TRUE);
 
 

[MM] [PATCH] novatel-lte: increase number of retries for connection status checks

2013-05-26 Thread Ben Chan
This patch increases the number of retries, from 4 to 10, for connection
status check during a connection / disconnection request, which handles
some scenario when the connection / disconnection request takes more
than 5 seconds to complete.
---
 plugins/novatel/mm-broadband-bearer-novatel-lte.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c 
b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
index 31c5650..677bc3c 100644
--- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
@@ -281,7 +281,7 @@ connect_3gpp (MMBroadbandBearer *self,
  callback,
  user_data,
  connect_3gpp);
-ctx-retries = 4;
+ctx-retries = 10;
 
 /* Get a 'net' data port */
 ctx-data = mm_base_modem_get_best_data_port (MM_BASE_MODEM (modem),
@@ -333,7 +333,7 @@ detailed_disconnect_context_new (MMBroadbandBearer *self,
  callback,
  user_data,
  detailed_disconnect_context_new);
-ctx-retries = 4;
+ctx-retries = 10;
 return ctx;
 }
 
-- 
1.8.2.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] novatel: fix invalid comparison of unsigned expression

2013-05-20 Thread Ben Chan
This patch fixes the following invalid comparison of unsigned expression:

novatel/mm-plugin-novatel.c:148:29: error: comparison of unsigned
  expression = 0 is always true [-Werror,-Wtautological-compare]
if (ctx-nwdmat_retries = 0) {
~~~ ^  ~

Bug reported on https://code.google.com/p/chromium/issues/detail?id=242150
---
 plugins/novatel/mm-plugin-novatel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/novatel/mm-plugin-novatel.c 
b/plugins/novatel/mm-plugin-novatel.c
index 4d09f23..2b4497b 100644
--- a/plugins/novatel/mm-plugin-novatel.c
+++ b/plugins/novatel/mm-plugin-novatel.c
@@ -145,7 +145,7 @@ custom_init_step (CustomInitContext *ctx)
 return;
 }
 
-if (ctx-nwdmat_retries = 0) {
+if (ctx-nwdmat_retries  0) {
 ctx-nwdmat_retries--;
 mm_at_serial_port_queue_command (ctx-port,
  $NWDMAT=1,
-- 
1.8.2.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] device: check for NULL driver in add_port_driver

2013-05-17 Thread Ben Chan
This patch fixes a crash in MMDevice::add_port_driver() due to
g_str_equal() dereferencing a NULL driver returned by
mm_device_utils_get_port_driver().

Bug reported on https://code.google.com/p/chromium/issues/detail?id=241823
---
 src/mm-device.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/mm-device.c b/src/mm-device.c
index bfc4b36..54f6456 100644
--- a/src/mm-device.c
+++ b/src/mm-device.c
@@ -255,6 +255,8 @@ add_port_driver (MMDevice *self,
 guint i;
 
 driver = mm_device_utils_get_port_driver (udev_port);
+if (!driver)
+return;
 
 n_items = (self-priv-drivers ? g_strv_length (self-priv-drivers) : 0);
 if (n_items  0) {
-- 
1.8.2.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH v2] sierra: remove comparison of unsigned expression = 0

2013-05-06 Thread Ben Chan
This patch removes an unnecessary check of unsigned expression = 0,
which also fixes the following clang warnings:

sierra/mm-broadband-modem-sierra.c:570:18: error: comparison of
unsigned expression = 0 is always true
[-Werror,-Wtautological-compare]
mode = 0 
 ^  ~

Bug reported on https://code.google.com/p/chromium/issues/detail?id=235989
Patched by Yunlian Jiang yunl...@chromium.org
---
 plugins/sierra/mm-broadband-modem-sierra.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/plugins/sierra/mm-broadband-modem-sierra.c 
b/plugins/sierra/mm-broadband-modem-sierra.c
index 7cb8e6c..f8cfb74 100644
--- a/plugins/sierra/mm-broadband-modem-sierra.c
+++ b/plugins/sierra/mm-broadband-modem-sierra.c
@@ -566,9 +566,7 @@ selrat_query_ready (MMBaseModem *self,
 if (g_regex_match_full (r, response, strlen (response), 0, 0, match_info, 
error)) {
 guint mode;
 
-if (mm_get_uint_from_match_info (match_info, 1, mode) 
-mode = 0 
-mode = 7) {
+if (mm_get_uint_from_match_info (match_info, 1, mode)  mode = 7) {
 switch (mode) {
 case 0:
 result.allowed = MM_MODEM_MODE_ANY;
-- 
1.8.2.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM][PATCH] remove comparison of unsigned expression = 0

2013-05-06 Thread Ben Chan
Hi Aleksander / Dan,

I've rebased Yunlian's original patch to MM HEAD:
https://mail.gnome.org/archives/networkmanager-list/2013-May/msg00026.html

Could you please take another look?

Thanks,
Ben

On Tue, Apr 30, 2013 at 1:34 PM, Ben Chan benc...@chromium.org wrote:

 I fixed the cid check in a different way:

 https://mail.gnome.org/archives/networkmanager-list/2013-April/msg00223.html

 Also revise the quality normalization:

 https://mail.gnome.org/archives/networkmanager-list/2013-April/msg00227.html



 On Tue, Apr 30, 2013 at 12:09 AM, Aleksander Morgado 
 aleksan...@lanedo.com wrote:

 On 29/04/13 18:23, Ben Chan wrote:
  How about the cid case?  cid == 0 seems to be an invalid value?
 

 Ah, true, cid == 0 wouldn't make sense there.

 --
 Aleksander



___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] broadband-bearer: fix cid check in disconnect_3gpp

2013-05-01 Thread Ben Chan
Aleksander / Dan, is this the right fix?

Thanks,
Ben


On Tue, Apr 30, 2013 at 12:54 PM, Ben Chan benc...@chromium.org wrote:

 A value 0 is used to denote an invalid/uninitialized CID. This patch
 fixes a CID check in disconnect_3gpp() of MMBroadbandBearer such that it
 disables all PDP contexts via AT+CGACT=0 when no specific CID is used
 (i.e. cid == 0).
 ---
  src/mm-broadband-bearer.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/src/mm-broadband-bearer.c b/src/mm-broadband-bearer.c
 index 292420c..193f44a 100644
 --- a/src/mm-broadband-bearer.c
 +++ b/src/mm-broadband-bearer.c
 @@ -1508,7 +1508,7 @@ disconnect_3gpp (MMBroadbandBearer *self,
 user_data);

  /* If no specific CID was used, disable all PDP contexts */
 -ctx-cgact_command = (cid = 0 ?
 +ctx-cgact_command = (cid  0 ?
g_strdup_printf (+CGACT=0,%d, cid) :
g_strdup_printf (+CGACT=0));

 --
 1.8.2.1


___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] broadband-bearer: fix cid check in disconnect_3gpp

2013-04-30 Thread Ben Chan
A value 0 is used to denote an invalid/uninitialized CID. This patch
fixes a CID check in disconnect_3gpp() of MMBroadbandBearer such that it
disables all PDP contexts via AT+CGACT=0 when no specific CID is used
(i.e. cid == 0).
---
 src/mm-broadband-bearer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mm-broadband-bearer.c b/src/mm-broadband-bearer.c
index 292420c..193f44a 100644
--- a/src/mm-broadband-bearer.c
+++ b/src/mm-broadband-bearer.c
@@ -1508,7 +1508,7 @@ disconnect_3gpp (MMBroadbandBearer *self,
user_data);
 
 /* If no specific CID was used, disable all PDP contexts */
-ctx-cgact_command = (cid = 0 ?
+ctx-cgact_command = (cid  0 ?
   g_strdup_printf (+CGACT=0,%d, cid) :
   g_strdup_printf (+CGACT=0));
 
-- 
1.8.2.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] iface-modem-3gpp: handle non-deferrable registration state updates

2013-04-30 Thread Ben Chan
Hi Aleksander / Dan,

Would a transition from 'registered' to 'idle'/'searching' considered a
'service' loss from the connection manager's perspective (e.g. the service
disappears and then reappears in connection manager)?  In practice,  a
+CEREG change may not necessarily mean that the service disappears. But I
guess such a glitch can be smoothed out in the connection manager layer
instead of the modem manager layer. I'm happy to update the logic as
suggested if that's the expected behavior.

Thanks,
Ben


On Tue, Apr 30, 2013 at 5:23 AM, Aleksander Morgado
aleksan...@lanedo.comwrote:

 Hey Ben,

 Resurrecting old patch...

 On 05/03/13 03:37, Ben Chan wrote:
  This patch changes MMIfaceModem3gpp to differentiate between deferrable
  and non-deferrable 3GPP registration state updates. Periodic or
  unsolicited registration state updates are deferrable, while internal
  updates, e.g. due to modem being disabled, are non-deferrable.

 I think that we should *not* defer the registration state update unless
 the modem was connected. This is, if the modem goes from registered to
 idle or searching, that update must be published in the interface right
 away. What do you think? Are you able to update the logic like that?

 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] iface-modem-3gpp: handle non-deferrable registration state updates

2013-04-30 Thread Ben Chan
+Dan


On Tue, Apr 30, 2013 at 1:03 PM, Ben Chan benc...@chromium.org wrote:

 Hi Aleksander / Dan,

 Would a transition from 'registered' to 'idle'/'searching' considered a
 'service' loss from the connection manager's perspective (e.g. the service
 disappears and then reappears in connection manager)?  In practice,  a
 +CEREG change may not necessarily mean that the service disappears. But I
 guess such a glitch can be smoothed out in the connection manager layer
 instead of the modem manager layer. I'm happy to update the logic as
 suggested if that's the expected behavior.

 Thanks,
 Ben



 On Tue, Apr 30, 2013 at 5:23 AM, Aleksander Morgado aleksan...@lanedo.com
  wrote:

 Hey Ben,

 Resurrecting old patch...

 On 05/03/13 03:37, Ben Chan wrote:
  This patch changes MMIfaceModem3gpp to differentiate between deferrable
  and non-deferrable 3GPP registration state updates. Periodic or
  unsolicited registration state updates are deferrable, while internal
  updates, e.g. due to modem being disabled, are non-deferrable.

 I think that we should *not* defer the registration state update unless
 the modem was connected. This is, if the modem goes from registered to
 idle or searching, that update must be published in the interface right
 away. What do you think? Are you able to update the logic like that?

 --
 Aleksander



___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] broadband-modem: update signal quality normalization

2013-04-30 Thread Ben Chan
This patch updates normalize_ciev_cind_signal_quality() in
MMBroadbandModem to remove an unnecessary check on 'quality = 0' and
also makes sure the normalized signal quality is capped at 100 when no
maximum is specified.

This is revised from a patch originally authored by
Yunlian Jiang yunl...@chromium.org.

Bug reported on https://code.google.com/p/chromium/issues/detail?id=235989
---
 src/mm-broadband-modem.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 764b4ed..45d9027 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -1651,11 +1651,10 @@ normalize_ciev_cind_signal_quality (guint quality,
 guint min,
 guint max)
 {
-if (!max 
-quality = 0) {
+if (!max) {
 /* If we didn't get a max, assume it was 5. Note that we do allow
  * 0, meaning no signal at all. */
-return (quality * 20);
+return (quality = 5) ? (quality * 20) : 100;
 }
 
 if (quality = min 
-- 
1.8.2.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM][PATCH] remove comparison of unsigned expression = 0

2013-04-30 Thread Ben Chan
I fixed the cid check in a different way:
https://mail.gnome.org/archives/networkmanager-list/2013-April/msg00223.html

Also revise the quality normalization:
https://mail.gnome.org/archives/networkmanager-list/2013-April/msg00227.html



On Tue, Apr 30, 2013 at 12:09 AM, Aleksander Morgado
aleksan...@lanedo.comwrote:

 On 29/04/13 18:23, Ben Chan wrote:
  How about the cid case?  cid == 0 seems to be an invalid value?
 

 Ah, true, cid == 0 wouldn't make sense there.

 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM][PATCH] remove comparison of unsigned expression = 0

2013-04-29 Thread Ben Chan
How about the cid case?  cid == 0 seems to be an invalid value?
On Apr 28, 2013 10:55 PM, Aleksander Morgado aleksan...@lanedo.com
wrote:

 On 27/04/13 08:00, Ben Chan wrote:
  [Ben]  Aleksander, do you think the original code was meant to check
  quality  0?
 
   +if (!max) {
/* If we didn't get a max, assume it was 5. Note that we do
 allow
 * 0, meaning no signal at all. */

 According to the comment just after that, zero is allowed.

 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM][PATCH] remove comparison of unsigned expression = 0

2013-04-27 Thread Ben Chan
On Fri, Apr 26, 2013 at 10:29 PM, Yunlian Jiang yunl...@google.com wrote:

 This patch removes a few unnecessary checks of unsigned expression = 0, 
 which also fixes the following clang warnings:

 mm-broadband-bearer.c:1511:31: error: comparison of unsigned expression = 0 
 is always true [-Werror,-Wtautological-compare]
 ctx-cgact_command = (cid = 0 ?
   ~~~ ^  ~
 1 error generated.

 mm-broadband-modem.c:1647:17: error: comparison of unsigned expression = 0 
 is always true [-Werror,-Wtautological-compare]
 quality = 0) {
 ~~~ ^  ~

 sierra/mm-broadband-modem-sierra.c:570:18: error: comparison of unsigned 
 expression = 0 is always true [-Werror,-Wtautological-compare]
 mode = 0 
  ^  ~

 Bug reported on https://code.google.com/p/chromium/issues/detail?id=235989

 ---

 diff --git a/plugins/sierra/mm-broadband-modem-sierra.c 
 b/plugins/sierra/mm-broadband-modem-sierra.c
 index 301f41e..affc01f 100644
 --- a/plugins/sierra/mm-broadband-modem-sierra.c
 +++ b/plugins/sierra/mm-broadband-modem-sierra.c
 @@ -567,7 +567,6 @@ selrat_query_ready (MMBaseModem *self,
  guint mode;

  if (mm_get_uint_from_match_info (match_info, 1, mode) 
 -mode = 0 
  mode = 7) {
  switch (mode) {
  case 0:
 diff --git a/src/mm-broadband-bearer.c b/src/mm-broadband-bearer.c
 index 292420c..fcaad56 100644
 --- a/src/mm-broadband-bearer.c
 +++ b/src/mm-broadband-bearer.c
 @@ -1508,9 +1508,7 @@ disconnect_3gpp (MMBroadbandBearer *self,
 user_data);

  /* If no specific CID was used, disable all PDP contexts */
 -ctx-cgact_command = (cid = 0 ?

[Ben]  Aleksander, do you think the original code was meant to check cid  0?

 -  g_strdup_printf (+CGACT=0,%d, cid) :
 -  g_strdup_printf (+CGACT=0));
 +ctx-cgact_command = g_strdup_printf (+CGACT=0,%d, cid);

  /* If the primary port is NOT connected (doesn't have to be the data 
 port),
   * we'll send CGACT there */
 diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
 index 92b2050..bfca363 100644
 --- a/src/mm-broadband-modem.c
 +++ b/src/mm-broadband-modem.c
 @@ -1643,8 +1643,7 @@ normalize_ciev_cind_signal_quality (guint quality,
  guint min,
  guint max)
  {
 -if (!max 
 -quality = 0) {

[Ben]  Aleksander, do you think the original code was meant to check
quality  0?

 +if (!max) {
  /* If we didn't get a max, assume it was 5. Note that we do allow
   * 0, meaning no signal at all. */
  return (quality * 20);
 --

 1.8.2.1


 ___
 networkmanager-list mailing list
 networkmanager-list@gnome.org
 https://mail.gnome.org/mailman/listinfo/networkmanager-list

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] qcdm: remove unnecessary NULL check on free()

2013-04-24 Thread Ben Chan
This patch removes a few unnecessary NULL checks on free(), which also fixes
the following clang warnings:

result.c:59:27: error: if statement has empty body [-Werror,-Wempty-body]
if (v-u.u8_array);
  ^
result.c:59:27: note: put the semicolon on a separate line to silence this 
warning
result.c:62:28: error: if statement has empty body [-Werror,-Wempty-body]
if (v-u.u16_array);
   ^
result.c:62:28: note: put the semicolon on a separate line to silence this 
warning

Bug reported on https://code.google.com/p/chromium/issues/detail?id=219280
Patched by Yunlian Jiang yunl...@chromium.org
---
 libqcdm/src/result.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/libqcdm/src/result.c b/libqcdm/src/result.c
index d51a050..d508fb7 100644
--- a/libqcdm/src/result.c
+++ b/libqcdm/src/result.c
@@ -53,14 +53,11 @@ static void
 val_free (Val *v)
 {
 if (v-type == VAL_TYPE_STRING) {
-if (v-u.s)
-free (v-u.s);
+free (v-u.s);
 } else if (v-type == VAL_TYPE_U8_ARRAY) {
-if (v-u.u8_array);
-free (v-u.u8_array);
+free (v-u.u8_array);
 } else if (v-type == VAL_TYPE_U16_ARRAY) {
-if (v-u.u16_array);
-free (v-u.u16_array);
+free (v-u.u16_array);
 }
 free (v-key);
 memset (v, 0, sizeof (*v));
-- 
1.8.2.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH v2] bearer: allow specifying default IP family for bearers

2013-04-22 Thread Ben Chan
This patch adds a 'bearer-default-ip-family' property to MMBearer, which
specifies the default IP family to use for a bearer when no explicit
value is given via the simple connect properties. The default IP family
is set to IPv4 in MMBearer but can be overridden by a MMBearer subclass,
which allows a modem plugin to specify an appropriate default value.
---
 libmm-glib/mm-bearer-properties.c |  8 +---
 src/mm-bearer-mbim.c  | 14 +++---
 src/mm-bearer-qmi.c   | 18 +-
 src/mm-bearer.c   | 25 +
 src/mm-bearer.h   |  2 ++
 src/mm-broadband-bearer.c | 16 
 6 files changed, 64 insertions(+), 19 deletions(-)

diff --git a/libmm-glib/mm-bearer-properties.c 
b/libmm-glib/mm-bearer-properties.c
index 6526ed0..2f59fee 100644
--- a/libmm-glib/mm-bearer-properties.c
+++ b/libmm-glib/mm-bearer-properties.c
@@ -674,13 +674,7 @@ mm_bearer_properties_init (MMBearerProperties *self)
 self-priv-allow_roaming = TRUE;
 self-priv-rm_protocol = MM_MODEM_CDMA_RM_PROTOCOL_UNKNOWN;
 self-priv-allowed_auth = MM_BEARER_ALLOWED_AUTH_UNKNOWN;
-
-/* At some point in the future, this default should probably be changed
- * to IPV4V6. However, presently support for this PDP type is rare. An
- * even better approach would likely be to query which PDP types the
- * modem supports (using AT+CGDCONT=?), and set the default accordingly
- */
-self-priv-ip_type = MM_BEARER_IP_FAMILY_IPV4;
+self-priv-ip_type = MM_BEARER_IP_FAMILY_UNKNOWN;
 }
 
 static void
diff --git a/src/mm-bearer-mbim.c b/src/mm-bearer-mbim.c
index d96a1b1..bddc9da 100644
--- a/src/mm-bearer-mbim.c
+++ b/src/mm-bearer-mbim.c
@@ -618,6 +618,7 @@ connect_context_step (ConnectContext *ctx)
 const gchar *password;
 MbimAuthProtocol auth;
 MbimContextIpType ip_type;
+MMBearerIpFamily ip_family;
 GError *error = NULL;
 
 /* Setup parameters to use */
@@ -658,7 +659,14 @@ connect_context_step (ConnectContext *ctx)
 }
 }
 
-switch (mm_bearer_properties_get_ip_type (ctx-properties)) {
+ip_family = mm_bearer_properties_get_ip_type (ctx-properties);
+if (ip_family == MM_BEARER_IP_FAMILY_UNKNOWN) {
+ip_family = mm_bearer_get_default_ip_family (MM_BEARER 
(ctx-self));
+mm_dbg (No specific IP family requested, defaulting to %s,
+mm_bearer_ip_family_get_string (ip_family));
+}
+
+switch (ip_family) {
 case MM_BEARER_IP_FAMILY_IPV4:
 ip_type = MBIM_CONTEXT_IP_TYPE_IPV4;
 break;
@@ -670,8 +678,8 @@ connect_context_step (ConnectContext *ctx)
 break;
 case MM_BEARER_IP_FAMILY_UNKNOWN:
 default:
-mm_dbg (No specific IP family requested, defaulting to IPv4);
-ip_type = MBIM_CONTEXT_IP_TYPE_IPV4;
+/* A valid default IP family should have been specified */
+g_assert_not_reached ();
 break;
 }
 
diff --git a/src/mm-bearer-qmi.c b/src/mm-bearer-qmi.c
index 8f9568d..9e8d2bc 100644
--- a/src/mm-bearer-qmi.c
+++ b/src/mm-bearer-qmi.c
@@ -846,11 +846,21 @@ _connect (MMBearer *self,
 
 if (properties) {
 MMBearerAllowedAuth auth;
+MMBearerIpFamily ip_family;
 
 ctx-apn = g_strdup (mm_bearer_properties_get_apn (properties));
 ctx-user = g_strdup (mm_bearer_properties_get_user (properties));
 ctx-password = g_strdup (mm_bearer_properties_get_password 
(properties));
-switch (mm_bearer_properties_get_ip_type (properties)) {
+
+ip_family = mm_bearer_properties_get_ip_type (properties);
+if (ip_family == MM_BEARER_IP_FAMILY_UNKNOWN) {
+ip_family = mm_bearer_get_default_ip_family (self);
+mm_dbg (No specific IP family requested, defaulting to %s,
+mm_bearer_ip_family_get_string (ip_family));
+ctx-no_ip_family_preference = TRUE;
+}
+
+switch (ip_family) {
 case MM_BEARER_IP_FAMILY_IPV4:
 ctx-ipv4 = TRUE;
 ctx-ipv6 = FALSE;
@@ -865,10 +875,8 @@ _connect (MMBearer *self,
 break;
 case MM_BEARER_IP_FAMILY_UNKNOWN:
 default:
-mm_dbg (No specific IP family requested, defaulting to IPv4);
-ctx-no_ip_family_preference = TRUE;
-ctx-ipv4 = TRUE;
-ctx-ipv6 = FALSE;
+/* A valid default IP family should have been specified */
+g_assert_not_reached ();
 break;
 }
 
diff --git a/src/mm-bearer.c b/src/mm-bearer.c
index 9e96bde..d047e11 100644
--- a/src/mm-bearer.c
+++ b/src/mm-bearer.c
@@ -57,6 +57,7 @@ enum {
 PROP_MODEM,
 PROP_STATUS,
 PROP_CONFIG,
+PROP_DEFAULT_IP_FAMILY,
 PROP_LAST
 };
 
@@ -73,6 +74,8 @@ struct _MMBearerPrivate {
 MMBearerStatus status;
 /* 

Re: [MM] [PATCH] bearer: allow specifying default IP family for bearers

2013-04-22 Thread Ben Chan
Thanks. Submitted a revised patch.

- Ben


On Mon, Apr 22, 2013 at 12:42 AM, Aleksander Morgado
aleksan...@lanedo.comwrote:

 On 04/22/2013 03:50 AM, Ben Chan wrote:
  This patch adds a 'bearer-default-ip-family' property to MMBearer, which
  specifies the default IP family to use for a bearer when no explicit
  value is given via the simple connect properties. The default IP family
  is set to IPv4 in MMBearer but can be overridden by a MMBearer subclass,
  which allows a modem plugin to specify an appropriate default value.

 The logic to use the new property was added only in the
 MMBroadbandBearer subclass; all the other available MMBearer subclasses
 (QMI, MBIM) should also get modified to use this new default value.

 Some more comments below.


  ---
   libmm-glib/mm-bearer-properties.c |  8 +---
   src/mm-bearer.c   | 25 +
   src/mm-bearer.h   |  2 ++
   src/mm-broadband-bearer.c | 25 +
   4 files changed, 49 insertions(+), 11 deletions(-)
 
  diff --git a/libmm-glib/mm-bearer-properties.c
 b/libmm-glib/mm-bearer-properties.c
  index 6526ed0..2f59fee 100644
  --- a/libmm-glib/mm-bearer-properties.c
  +++ b/libmm-glib/mm-bearer-properties.c
  @@ -674,13 +674,7 @@ mm_bearer_properties_init (MMBearerProperties *self)
   self-priv-allow_roaming = TRUE;
   self-priv-rm_protocol = MM_MODEM_CDMA_RM_PROTOCOL_UNKNOWN;
   self-priv-allowed_auth = MM_BEARER_ALLOWED_AUTH_UNKNOWN;
  -
  -/* At some point in the future, this default should probably be
 changed
  - * to IPV4V6. However, presently support for this PDP type is rare.
 An
  - * even better approach would likely be to query which PDP types the
  - * modem supports (using AT+CGDCONT=?), and set the default
 accordingly
  - */
  -self-priv-ip_type = MM_BEARER_IP_FAMILY_IPV4;
  +self-priv-ip_type = MM_BEARER_IP_FAMILY_UNKNOWN;
   }
 
   static void
  diff --git a/src/mm-bearer.c b/src/mm-bearer.c
  index 9e96bde..d047e11 100644
  --- a/src/mm-bearer.c
  +++ b/src/mm-bearer.c
  @@ -57,6 +57,7 @@ enum {
   PROP_MODEM,
   PROP_STATUS,
   PROP_CONFIG,
  +PROP_DEFAULT_IP_FAMILY,
   PROP_LAST
   };
 
  @@ -73,6 +74,8 @@ struct _MMBearerPrivate {
   MMBearerStatus status;
   /* Configuration of the bearer */
   MMBearerProperties *config;
  +/* Default IP family of this bearer */
  +MMBearerIpFamily default_ip_family;
 
   /* Cancellable for connect() */
   GCancellable *connect_cancellable;
  @@ -844,6 +847,12 @@ mm_bearer_get_config (MMBearer *self)
   NULL);
   }
 
  +MMBearerIpFamily
  +mm_bearer_get_default_ip_family (MMBearer *self)
  +{
  +return self-priv-default_ip_family;
  +}
  +
 
  
 /*/
 
   static void
  @@ -969,6 +978,9 @@ set_property (GObject *object,
   g_variant_unref (dictionary);
   break;
   }
  +case PROP_DEFAULT_IP_FAMILY:
  +self-priv-default_ip_family = g_value_get_enum (value);
  +break;
   default:
   G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
   break;
  @@ -999,6 +1011,9 @@ get_property (GObject *object,
   case PROP_CONFIG:
   g_value_set_object (value, self-priv-config);
   break;
  +case PROP_DEFAULT_IP_FAMILY:
  +g_value_set_enum (value, self-priv-default_ip_family);
  +break;
   default:
   G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
   break;
  @@ -1015,6 +1030,7 @@ mm_bearer_init (MMBearer *self)
   self-priv-status = MM_BEARER_STATUS_DISCONNECTED;
   self-priv-reason_3gpp = CONNECTION_FORBIDDEN_REASON_NONE;
   self-priv-reason_cdma = CONNECTION_FORBIDDEN_REASON_NONE;
  +self-priv-default_ip_family = MM_BEARER_IP_FAMILY_IPV4;
 
   /* Set defaults */
   mm_gdbus_bearer_set_interface (MM_GDBUS_BEARER (self), NULL);
  @@ -,6 +1127,15 @@ mm_bearer_class_init (MMBearerClass *klass)
MM_TYPE_BEARER_PROPERTIES,
G_PARAM_READWRITE);
   g_object_class_install_property (object_class, PROP_CONFIG,
 properties[PROP_CONFIG]);
  +
  +properties[PROP_DEFAULT_IP_FAMILY] =
  +g_param_spec_enum (MM_BEARER_DEFAULT_IP_FAMILY,
  +   Bearer default IP family,
  +   IP family to use for this bearer when no IP
 family is specified,
  +   MM_TYPE_BEARER_IP_FAMILY,
  +   MM_BEARER_IP_FAMILY_IPV4,
  +   G_PARAM_READWRITE);
  +g_object_class_install_property (object_class,
 PROP_DEFAULT_IP_FAMILY, properties[PROP_DEFAULT_IP_FAMILY]);
   }
 
 
  
 /*/
  diff --git a/src/mm-bearer.h b/src/mm-bearer.h
  index c617297..cc71bfd

[MM] [PATCH] bearer: allow specifying default IP family for bearers

2013-04-21 Thread Ben Chan
This patch adds a 'bearer-default-ip-family' property to MMBearer, which
specifies the default IP family to use for a bearer when no explicit
value is given via the simple connect properties. The default IP family
is set to IPv4 in MMBearer but can be overridden by a MMBearer subclass,
which allows a modem plugin to specify an appropriate default value.
---
 libmm-glib/mm-bearer-properties.c |  8 +---
 src/mm-bearer.c   | 25 +
 src/mm-bearer.h   |  2 ++
 src/mm-broadband-bearer.c | 25 +
 4 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/libmm-glib/mm-bearer-properties.c 
b/libmm-glib/mm-bearer-properties.c
index 6526ed0..2f59fee 100644
--- a/libmm-glib/mm-bearer-properties.c
+++ b/libmm-glib/mm-bearer-properties.c
@@ -674,13 +674,7 @@ mm_bearer_properties_init (MMBearerProperties *self)
 self-priv-allow_roaming = TRUE;
 self-priv-rm_protocol = MM_MODEM_CDMA_RM_PROTOCOL_UNKNOWN;
 self-priv-allowed_auth = MM_BEARER_ALLOWED_AUTH_UNKNOWN;
-
-/* At some point in the future, this default should probably be changed
- * to IPV4V6. However, presently support for this PDP type is rare. An
- * even better approach would likely be to query which PDP types the
- * modem supports (using AT+CGDCONT=?), and set the default accordingly
- */
-self-priv-ip_type = MM_BEARER_IP_FAMILY_IPV4;
+self-priv-ip_type = MM_BEARER_IP_FAMILY_UNKNOWN;
 }
 
 static void
diff --git a/src/mm-bearer.c b/src/mm-bearer.c
index 9e96bde..d047e11 100644
--- a/src/mm-bearer.c
+++ b/src/mm-bearer.c
@@ -57,6 +57,7 @@ enum {
 PROP_MODEM,
 PROP_STATUS,
 PROP_CONFIG,
+PROP_DEFAULT_IP_FAMILY,
 PROP_LAST
 };
 
@@ -73,6 +74,8 @@ struct _MMBearerPrivate {
 MMBearerStatus status;
 /* Configuration of the bearer */
 MMBearerProperties *config;
+/* Default IP family of this bearer */
+MMBearerIpFamily default_ip_family;
 
 /* Cancellable for connect() */
 GCancellable *connect_cancellable;
@@ -844,6 +847,12 @@ mm_bearer_get_config (MMBearer *self)
 NULL);
 }
 
+MMBearerIpFamily
+mm_bearer_get_default_ip_family (MMBearer *self)
+{
+return self-priv-default_ip_family;
+}
+
 /*/
 
 static void
@@ -969,6 +978,9 @@ set_property (GObject *object,
 g_variant_unref (dictionary);
 break;
 }
+case PROP_DEFAULT_IP_FAMILY:
+self-priv-default_ip_family = g_value_get_enum (value);
+break;
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
 break;
@@ -999,6 +1011,9 @@ get_property (GObject *object,
 case PROP_CONFIG:
 g_value_set_object (value, self-priv-config);
 break;
+case PROP_DEFAULT_IP_FAMILY:
+g_value_set_enum (value, self-priv-default_ip_family);
+break;
 default:
 G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
 break;
@@ -1015,6 +1030,7 @@ mm_bearer_init (MMBearer *self)
 self-priv-status = MM_BEARER_STATUS_DISCONNECTED;
 self-priv-reason_3gpp = CONNECTION_FORBIDDEN_REASON_NONE;
 self-priv-reason_cdma = CONNECTION_FORBIDDEN_REASON_NONE;
+self-priv-default_ip_family = MM_BEARER_IP_FAMILY_IPV4;
 
 /* Set defaults */
 mm_gdbus_bearer_set_interface (MM_GDBUS_BEARER (self), NULL);
@@ -,6 +1127,15 @@ mm_bearer_class_init (MMBearerClass *klass)
  MM_TYPE_BEARER_PROPERTIES,
  G_PARAM_READWRITE);
 g_object_class_install_property (object_class, PROP_CONFIG, 
properties[PROP_CONFIG]);
+
+properties[PROP_DEFAULT_IP_FAMILY] =
+g_param_spec_enum (MM_BEARER_DEFAULT_IP_FAMILY,
+   Bearer default IP family,
+   IP family to use for this bearer when no IP family 
is specified,
+   MM_TYPE_BEARER_IP_FAMILY,
+   MM_BEARER_IP_FAMILY_IPV4,
+   G_PARAM_READWRITE);
+g_object_class_install_property (object_class, PROP_DEFAULT_IP_FAMILY, 
properties[PROP_DEFAULT_IP_FAMILY]);
 }
 
 /*/
diff --git a/src/mm-bearer.h b/src/mm-bearer.h
index c617297..cc71bfd 100644
--- a/src/mm-bearer.h
+++ b/src/mm-bearer.h
@@ -58,6 +58,7 @@ typedef struct _MMBearerPrivate MMBearerPrivate;
 #define MM_BEARER_MODEM  bearer-modem
 #define MM_BEARER_STATUS bearer-status
 #define MM_BEARER_CONFIG bearer-config
+#define MM_BEARER_DEFAULT_IP_FAMILY bearer-deafult-ip-family
 
 typedef enum { /* underscore_name=mm_bearer_status */
 MM_BEARER_STATUS_DISCONNECTED,
@@ -103,6 +104,7 @@ const gchar*mm_bearer_get_path(MMBearer 
*bearer);
 MMBearerStatus  mm_bearer_get_status  (MMBearer *bearer);
 MMBearerProperties *mm_bearer_peek_config (MMBearer *self);
 

[MM] [PATCH] device: handle NULL returned by g_udev_device_get_driver() gracefully

2013-04-10 Thread Ben Chan
This patch fixes a crash in mm_device_grab_port() when doing a string
comparison on a NULL returned by g_udev_device_get_driver().

Thread 0 *CRASHED* ( SIGSEGV @ 0x )

0x76b760b4   [libc-2.15.so]   - strcmp.c:38  
strcmp
0x76c66a7d   [libglib-2.0.so.0.3200.4]- ghash.c:1704 
g_str_equal
0x76ee0e5d   [ModemManager]   - mm-device.c:147  
mm_device_grab_port
0x76edf9d9   [ModemManager]   - mm-manager.c:313 
device_added
0x76e95b2d   [libgudev-1.0.so.0.1.0]  - extras/gudev/gudevmarshal.c:84   
g_udev_marshal_VOID__STRING_OBJECT
0x76d1fb2b   [libgobject-2.0.so.0.3200.4] - gclosure.c:777   
g_closure_invoke
0x76d2b88b   [libgobject-2.0.so.0.3200.4] - gsignal.c:3551   
signal_emit_unlocked_R
0x76d313c5   [libgobject-2.0.so.0.3200.4] - gsignal.c:3300   
g_signal_emit_valist
0x76d31569   [libgobject-2.0.so.0.3200.4] - gsignal.c:3356   
g_signal_emit
0x76e93bdd   [libgudev-1.0.so.0.1.0]  - extras/gudev/gudevclient.c:105   
monitor_event
0x76c9beb7   [libglib-2.0.so.0.3200.4]- giounix.c:166
g_io_unix_dispatch
0x76c714c1   [libglib-2.0.so.0.3200.4]- gmain.c:2539 
g_main_context_dispatch
0x76c71745   [libglib-2.0.so.0.3200.4]- gmain.c:3146 
g_main_context_iterate
0x76c71a59   [libglib-2.0.so.0.3200.4]- gmain.c:3340 
g_main_loop_run
0x76ede8ed   [ModemManager]   - main.c:142   
main
0x76b35f79   [libc-2.15.so]   - libc-start.c:226 
__libc_start_main
0x76edea49   [ModemManager]   + 0x00014a49
0x76eb4eab   [ld-2.15.so] + 0xaeab
---
 src/mm-device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mm-device.c b/src/mm-device.c
index 7800439..a2755a7 100644
--- a/src/mm-device.c
+++ b/src/mm-device.c
@@ -145,7 +145,7 @@ get_device_ids (GUdevDevice *device,
 success = TRUE;
 goto out;
 } else if (g_str_has_prefix (parent_subsys, usb) 
-   g_str_equal (g_udev_device_get_driver (parent), 
qmi_wwan)) {
+   !g_strcmp0 (g_udev_device_get_driver (parent), 
qmi_wwan)) {
 /* Need to look for vendor/product in the parent of the QMI 
device */
 GUdevDevice *qmi_parent;
 
-- 
1.8.1.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] serial,blacklist: fix file mode (0755 - 0644)

2013-04-04 Thread Ben Chan
---
 src/77-mm-usb-device-blacklist.rules | 0
 src/mm-at-serial-port.c  | 0
 2 files changed, 0 insertions(+), 0 deletions(-)
 mode change 100755 = 100644 src/77-mm-usb-device-blacklist.rules
 mode change 100755 = 100644 src/mm-at-serial-port.c

diff --git a/src/77-mm-usb-device-blacklist.rules 
b/src/77-mm-usb-device-blacklist.rules
old mode 100755
new mode 100644
diff --git a/src/mm-at-serial-port.c b/src/mm-at-serial-port.c
old mode 100755
new mode 100644
-- 
1.8.1.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] broadband-bearer: handle NULL and character escaping of APN value

2013-04-04 Thread Ben Chan
---
 src/mm-broadband-bearer.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/mm-broadband-bearer.c b/src/mm-broadband-bearer.c
index c34867d..fe894c9 100644
--- a/src/mm-broadband-bearer.c
+++ b/src/mm-broadband-bearer.c
@@ -753,7 +753,7 @@ find_cid_ready (MMBaseModem *modem,
 DetailedConnectContext *ctx)
 {
 GVariant *result;
-gchar *command;
+gchar *apn, *command;
 GError *error = NULL;
 const gchar *pdp_type;
 
@@ -783,10 +783,12 @@ find_cid_ready (MMBaseModem *modem,
 }
 
 ctx-cid = g_variant_get_uint32 (result);
-command = g_strdup_printf (+CGDCONT=%u,\%s\,\%s\,
+apn = mm_at_serial_port_quote_string (mm_bearer_properties_get_apn 
(mm_bearer_peek_config (MM_BEARER (ctx-self;
+command = g_strdup_printf (+CGDCONT=%u,\%s\,%s,
ctx-cid,
pdp_type,
-   mm_bearer_properties_get_apn 
(mm_bearer_peek_config (MM_BEARER (ctx-self;
+   apn);
+g_free (apn);
 mm_base_modem_at_command_full (ctx-modem,
ctx-primary,
command,
-- 
1.8.1.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM][PATCH v4] altair-lte: initial altair lte plugin

2013-04-03 Thread Ben Chan
Hi Ori

I guess the copyright note in mm-broadband-bearer-altair-lte.c may simply
be Copyright (C) 2013 Altair Semiconductor as it's a new file, right?

- Ben


On Wed, Apr 3, 2013 at 4:56 AM, Aleksander Morgado aleksan...@lanedo.comwrote:

 On 04/02/2013 01:57 AM, Ori Inbar wrote:
  Hello all
  This is a log for the plugin run :
 
  2013-04-01T16:19:30.614176-07:00 localhost ModemManager[1366]: debug
 (Plugin Manager) [/sys/devices/s5p-ehci/usb1/1-1] Checking device support...
  2013-04-01T16:19:30.614408-07:00 localhost ModemManager[1366]: debug
 (Novatel LTE) [ttyACM0] filtered by vendor/product IDs
  2013-04-01T16:19:30.614428-07:00 localhost ModemManager[1366]: debug
 (Samsung) [ttyACM0] filtered by vendor/product IDs
  2013-04-01T16:19:30.614445-07:00 localhost ModemManager[1366]: debug
 (Huawei) [ttyACM0] filtered by vendor/product IDs
  2013-04-01T16:19:30.614460-07:00 localhost ModemManager[1366]: debug
 (Plugin Manager) [ttyACM0] Found '1' plugins to try...
  2013-04-01T16:19:30.614473-07:00 localhost ModemManager[1366]: debug
 (Plugin Manager) [ttyACM0]   Will try with plugin 'Altair LTE'
 


 Thanks! Looks pretty good :)

 --
 Aleksander
 ___
 networkmanager-list mailing list
 networkmanager-list@gnome.org
 https://mail.gnome.org/mailman/listinfo/networkmanager-list

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] broadband-modem: fix enable flag in UnsolicitedRegistrationEventsContext

2013-03-05 Thread Ben Chan
---
 src/mm-broadband-modem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 9d127c9..b3ec35a 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -3712,7 +3712,7 @@ unsolicited_registration_events_context_new 
(MMBroadbandModem *self,
  callback,
  user_data,
  
unsolicited_registration_events_context_new);
-ctx-enable = FALSE;
+ctx-enable = enable;
 ctx-run_cs = cs_supported;
 ctx-run_ps = ps_supported;
 ctx-run_eps = eps_supported;
-- 
1.8.1.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] broadband-modem: fix disabling of unsolicited registration events

2013-03-04 Thread Ben Chan
---
 src/mm-broadband-modem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 9f45904..9d127c9 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -3924,7 +3924,7 @@ unsolicited_registration_events_context_step 
(UnsolicitedRegistrationEventsConte
 mm_base_modem_at_sequence_full (
 MM_BASE_MODEM (ctx-self),
 mm_base_modem_peek_port_primary (MM_BASE_MODEM (ctx-self)),
-cs_registration_sequence,
+ctx-enable ? cs_registration_sequence : 
cs_unregistration_sequence,
 NULL,  /* response processor context */
 NULL,  /* response processor context free */
 NULL,  /* cancellable */
@@ -3939,7 +3939,7 @@ unsolicited_registration_events_context_step 
(UnsolicitedRegistrationEventsConte
 mm_base_modem_at_sequence_full (
 MM_BASE_MODEM (ctx-self),
 mm_base_modem_peek_port_primary (MM_BASE_MODEM (ctx-self)),
-ps_registration_sequence,
+ctx-enable ? ps_registration_sequence : 
ps_unregistration_sequence,
 NULL,  /* response processor context */
 NULL,  /* response processor context free */
 NULL,  /* cancellable */
@@ -3954,7 +3954,7 @@ unsolicited_registration_events_context_step 
(UnsolicitedRegistrationEventsConte
 mm_base_modem_at_sequence_full (
 MM_BASE_MODEM (ctx-self),
 mm_base_modem_peek_port_primary (MM_BASE_MODEM (ctx-self)),
-eps_registration_sequence,
+ctx-enable ? eps_registration_sequence : 
eps_unregistration_sequence,
 NULL,  /* response processor context */
 NULL,  /* response processor context free */
 NULL,  /* cancellable */
-- 
1.8.1.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] iface-modem-3gpp: handle non-deferrable registration state updates

2013-03-04 Thread Ben Chan
This patch changes MMIfaceModem3gpp to differentiate between deferrable
and non-deferrable 3GPP registration state updates. Periodic or
unsolicited registration state updates are deferrable, while internal
updates, e.g. due to modem being disabled, are non-deferrable.
---
 src/mm-iface-modem-3gpp.c | 40 ++--
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
index 6c8bb14..7049ec8 100644
--- a/src/mm-iface-modem-3gpp.c
+++ b/src/mm-iface-modem-3gpp.c
@@ -1093,7 +1093,8 @@ run_deferred_registration_state_update (MMIfaceModem3gpp 
*self)
 
 static void
 update_registration_state (MMIfaceModem3gpp *self,
-   MMModem3gppRegistrationState new_state)
+   MMModem3gppRegistrationState new_state,
+   gboolean deferrable)
 {
 MMModem3gppRegistrationState old_state = 
MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
 RegistrationStateContext *ctx;
@@ -1106,18 +1107,20 @@ update_registration_state (MMIfaceModem3gpp *self,
 g_assert (ctx);
 
 if (ctx-deferred_update_id != 0) {
-/* If there is already a deferred 'registration loss' state update and 
the new update
- * is not a registered state, update the deferred state update without 
extending the
- * timeout. */
-if (new_state != MM_MODEM_3GPP_REGISTRATION_STATE_HOME 
-new_state != MM_MODEM_3GPP_REGISTRATION_STATE_ROAMING) {
-mm_info (Modem %s: 3GPP Registration state changed (%s - %s), 
update deferred,
- g_dbus_object_get_object_path (G_DBUS_OBJECT (self)),
- mm_modem_3gpp_registration_state_get_string (old_state),
- mm_modem_3gpp_registration_state_get_string (new_state));
-
-ctx-deferred_new_state = new_state;
-return;
+if (deferrable) {
+/* If there is already a deferred 'registration loss' state update 
and the new update
+ * is not a registered state, update the deferred state update 
without extending the
+ * timeout. */
+if (new_state != MM_MODEM_3GPP_REGISTRATION_STATE_HOME 
+new_state != MM_MODEM_3GPP_REGISTRATION_STATE_ROAMING) {
+mm_info (Modem %s: 3GPP Registration state changed (%s - 
%s), update deferred,
+ g_dbus_object_get_object_path (G_DBUS_OBJECT (self)),
+ mm_modem_3gpp_registration_state_get_string 
(old_state),
+ mm_modem_3gpp_registration_state_get_string 
(new_state));
+
+ctx-deferred_new_state = new_state;
+return;
+}
 }
 
 /* Otherwise, cancel any deferred registration state update */
@@ -1153,7 +1156,8 @@ update_registration_state (MMIfaceModem3gpp *self,
 if ((old_state == MM_MODEM_3GPP_REGISTRATION_STATE_HOME ||
  old_state == MM_MODEM_3GPP_REGISTRATION_STATE_ROAMING) 
 (new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ||
- new_state == MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN)) {
+ new_state == MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN) 
+deferrable) {
 mm_info (Modem %s: 3GPP Registration state changed (%s - %s), update 
deferred,
  g_dbus_object_get_object_path (G_DBUS_OBJECT (self)),
  mm_modem_3gpp_registration_state_get_string (old_state),
@@ -1191,7 +1195,7 @@ mm_iface_modem_3gpp_update_cs_registration_state 
(MMIfaceModem3gpp *self,
 
 ctx = get_registration_state_context (self);
 ctx-cs = state;
-update_registration_state (self, get_consolidated_reg_state (ctx));
+update_registration_state (self, get_consolidated_reg_state (ctx), TRUE);
 }
 
 void
@@ -1210,7 +1214,7 @@ mm_iface_modem_3gpp_update_ps_registration_state 
(MMIfaceModem3gpp *self,
 
 ctx = get_registration_state_context (self);
 ctx-ps = state;
-update_registration_state (self, get_consolidated_reg_state (ctx));
+update_registration_state (self, get_consolidated_reg_state (ctx), TRUE);
 }
 
 void
@@ -1229,7 +1233,7 @@ mm_iface_modem_3gpp_update_eps_registration_state 
(MMIfaceModem3gpp *self,
 
 ctx = get_registration_state_context (self);
 ctx-eps = state;
-update_registration_state (self, get_consolidated_reg_state (ctx));
+update_registration_state (self, get_consolidated_reg_state (ctx), TRUE);
 }
 
 /*/
@@ -1492,7 +1496,7 @@ interface_disabling_step (DisablingContext *ctx)
 ctx-step++;
 
 case DISABLING_STEP_REGISTRATION_STATE:
-update_registration_state (ctx-self, 
MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN);
+update_registration_state (ctx-self, 
MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN, FALSE);
 mm_iface_modem_3gpp_update_access_technologies (ctx-self, 

Re: [MM] [PATCH v2] iface-modem-3gpp: clear deferred registration state update when disabling

2013-03-01 Thread Ben Chan
On Thu, Feb 28, 2013 at 11:46 PM, Aleksander Morgado
aleksan...@lanedo.comwrote:

 On 02/28/2013 08:11 PM, Ben Chan wrote:
  ---
   src/mm-iface-modem-3gpp.c | 15 +++
   1 file changed, 15 insertions(+)
 

 I pushed it after modifying it to include the
 clear_deferred_registration_state_update() call in the first
 DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS step. As said previously, we
 don't really mind if we get an unsolicited registration event during the
 disabling process, as we're disabling anyway.


I'm a bit confused. If an unsolicited registration event is received after
clearing deferred_update_id, deferred_update_id is set to a new value,
which will be scheduled after the modem is disabled. That will cause the
modem state to become enabled due to the following code in
mm-iface-modem-3gpp.c:

mm_iface_modem_update_subsystem_state (
MM_IFACE_MODEM (self),
SUBSYSTEM_3GPP,
(new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ?
 MM_MODEM_STATE_SEARCHING :
 MM_MODEM_STATE_ENABLED),
MM_MODEM_STATE_CHANGE_REASON_UNKNOWN);
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH v2] iface-modem-3gpp: clear deferred registration state update when disabling

2013-03-01 Thread Ben Chan
Yep, that should work.


On Fri, Mar 1, 2013 at 12:18 AM, Aleksander Morgado
aleksan...@lanedo.comwrote:

 On 03/01/2013 09:06 AM, Ben Chan wrote:
 
 
 
  On Thu, Feb 28, 2013 at 11:46 PM, Aleksander Morgado
  aleksan...@lanedo.com mailto:aleksan...@lanedo.com wrote:
 
  On 02/28/2013 08:11 PM, Ben Chan wrote:
   ---
src/mm-iface-modem-3gpp.c | 15 +++
1 file changed, 15 insertions(+)
  
 
  I pushed it after modifying it to include the
  clear_deferred_registration_state_update() call in the first
  DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS step. As said
 previously, we
  don't really mind if we get an unsolicited registration event during
 the
  disabling process, as we're disabling anyway.
 
 
  I'm a bit confused. If an unsolicited registration event is received
  after clearing deferred_update_id, deferred_update_id is set to a new
  value, which will be scheduled after the modem is disabled. That will
  cause the modem state to become enabled due to the following code in
  mm-iface-modem-3gpp.c:
 
  mm_iface_modem_update_subsystem_state (
  MM_IFACE_MODEM (self),
  SUBSYSTEM_3GPP,
  (new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ?
   MM_MODEM_STATE_SEARCHING :
   MM_MODEM_STATE_ENABLED),
  MM_MODEM_STATE_CHANGE_REASON_UNKNOWN);
 

 Ah, good point; didn't consider that the deferred update could get
 re-scheduled... In that case we should actually clear it *after*
 DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS, once we know we
 will not process those unsolicited events. How does this look?

 diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
 index 1d3fdce..6c8bb14 100644
 --- a/src/mm-iface-modem-3gpp.c
 +++ b/src/mm-iface-modem-3gpp.c
 @@ -1347,6 +1347,7 @@ typedef enum {
  DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS,
  DISABLING_STEP_DISABLE_UNSOLICITED_REGISTRATION_EVENTS,
  DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS,
 +DISABLING_STEP_CLEANUP_DEFERRED_REGISTRATION_UPDATE,
  DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS,
  DISABLING_STEP_DISABLE_UNSOLICITED_EVENTS,
  DISABLING_STEP_REGISTRATION_STATE,
 @@ -1419,8 +1420,6 @@ interface_disabling_step (DisablingContext *ctx)
  case DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS:
  /* Disable periodic registration checks, if they were set */
  periodic_registration_check_disable (ctx-self);
 -/* Prevent any deferred registration state update from
 happening after the modem is disabled */
 -clear_deferred_registration_state_update (ctx-self);
  /* Fall down to next step */
  ctx-step++;

 @@ -1462,6 +1461,12 @@ interface_disabling_step (DisablingContext *ctx)
  /* Fall down to next step */
  ctx-step++;

 +case DISABLING_STEP_CLEANUP_DEFERRED_REGISTRATION_UPDATE:
 +/* Prevent any deferred registration state update from
 happening after the modem is disabled */
 +clear_deferred_registration_state_update (ctx-self);
 +/* Fall down to next step */
 +ctx-step++;
 +
  case DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS:
  if (MM_IFACE_MODEM_3GPP_GET_INTERFACE
 (ctx-self)-cleanup_unsolicited_events 
  MM_IFACE_MODEM_3GPP_GET_INTERFACE
 (ctx-self)-cleanup_unsolicited_events_finish) {

 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] iface-modem-3gpp: clear deferred registration state update when disabling

2013-02-28 Thread Ben Chan
One question inline

On Feb 28, 2013 12:23 AM, Aleksander Morgado aleksan...@lanedo.com
wrote:

 On 02/27/2013 11:04 PM, Ben Chan wrote:
  ---
   src/mm-iface-modem-3gpp.c | 18 +-
   1 file changed, 17 insertions(+), 1 deletion(-)

 When I reviewed your original patch I actually checked for when we were
 removing the timeout when disabling, and assumed it was done when we set
 the GObject's data to NULL, but I mixed the two data objects we handle
 here, as one of them (the one where this new timeout id is stored) isn't
 removed during disabling. So, yes, we need this method to remove the
 timeout. Now, I would add the clearing method just when we start the
 disabling sequence, next to the periodic_registration_check_disable()
 call. Or was it added in CLEANUP_UNSOLICITED_REGISTRATION_EVENTS for
 some reason?


Could an unsolicited registration state update happen between invocations
of  interface_disabling_step()? I was wondering if the deferred_update_id
should be cleared after unsolicited registration event handling is removed.

 Also some minor things below.

 
  diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
  index fa8da1a..3f3a803 100644
  --- a/src/mm-iface-modem-3gpp.c
  +++ b/src/mm-iface-modem-3gpp.c
  @@ -1325,6 +1325,18 @@ periodic_registration_check_enable
(MMIfaceModem3gpp *self)
 
(GDestroyNotify)registration_check_context_free);
   }
 
  +static void
  +clear_deferred_registration_state_update (MMIfaceModem3gpp *self)
  +{
  +RegistrationStateContext *ctx = get_registration_state_context
(self);
  +
  +ctx-deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
  +if (ctx-deferred_update_id) {
  +g_source_remove (ctx-deferred_update_id);
  +ctx-deferred_update_id = 0;
  +}
  +}
  +
 
 /*/
 
   typedef struct _DisablingContext DisablingContext;
  @@ -1435,7 +1447,10 @@ interface_disabling_step (DisablingContext *ctx)
   ctx-step++;
   }
 
  -case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS:
  +case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS: {

 No real need to add braces here, unless we're adding new variables.

  +/* Prevent any deferred registration state update from
happending after the modem is disabled */


 happending doesn't seem an English word to me :)


Oops... typo :)


  +clear_deferred_registration_state_update (ctx-self);
  +
   if (MM_IFACE_MODEM_3GPP_GET_INTERFACE
(ctx-self)-cleanup_unsolicited_registration_events 
   MM_IFACE_MODEM_3GPP_GET_INTERFACE
(ctx-self)-cleanup_unsolicited_registration_events_finish) {
   MM_IFACE_MODEM_3GPP_GET_INTERFACE
(ctx-self)-cleanup_unsolicited_registration_events (
  @@ -1446,6 +1461,7 @@ interface_disabling_step (DisablingContext *ctx)
   }
   /* Fall down to next step */
   ctx-step++;
  +}
 
   case DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS:
   if (MM_IFACE_MODEM_3GPP_GET_INTERFACE
(ctx-self)-cleanup_unsolicited_events 
 


 --
 Aleksander
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH v2] iface-modem-3gpp: clear deferred registration state update when disabling

2013-02-28 Thread Ben Chan
---
 src/mm-iface-modem-3gpp.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
index fa8da1a..2430056 100644
--- a/src/mm-iface-modem-3gpp.c
+++ b/src/mm-iface-modem-3gpp.c
@@ -1325,6 +1325,18 @@ periodic_registration_check_enable (MMIfaceModem3gpp 
*self)
  (GDestroyNotify)registration_check_context_free);
 }
 
+static void
+clear_deferred_registration_state_update (MMIfaceModem3gpp *self)
+{
+RegistrationStateContext *ctx = get_registration_state_context (self);
+
+ctx-deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
+if (ctx-deferred_update_id) {
+g_source_remove (ctx-deferred_update_id);
+ctx-deferred_update_id = 0;
+}
+}
+
 /*/
 
 typedef struct _DisablingContext DisablingContext;
@@ -1436,6 +1448,9 @@ interface_disabling_step (DisablingContext *ctx)
 }
 
 case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS:
+/* Prevent any deferred registration state update from happening after 
the modem is disabled */
+clear_deferred_registration_state_update (ctx-self);
+
 if (MM_IFACE_MODEM_3GPP_GET_INTERFACE 
(ctx-self)-cleanup_unsolicited_registration_events 
 MM_IFACE_MODEM_3GPP_GET_INTERFACE 
(ctx-self)-cleanup_unsolicited_registration_events_finish) {
 MM_IFACE_MODEM_3GPP_GET_INTERFACE 
(ctx-self)-cleanup_unsolicited_registration_events (
-- 
1.8.1.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] iface-modem-3gpp: clear deferred registration state update when disabling

2013-02-27 Thread Ben Chan
---
 src/mm-iface-modem-3gpp.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
index fa8da1a..3f3a803 100644
--- a/src/mm-iface-modem-3gpp.c
+++ b/src/mm-iface-modem-3gpp.c
@@ -1325,6 +1325,18 @@ periodic_registration_check_enable (MMIfaceModem3gpp 
*self)
  (GDestroyNotify)registration_check_context_free);
 }
 
+static void
+clear_deferred_registration_state_update (MMIfaceModem3gpp *self)
+{
+RegistrationStateContext *ctx = get_registration_state_context (self);
+
+ctx-deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
+if (ctx-deferred_update_id) {
+g_source_remove (ctx-deferred_update_id);
+ctx-deferred_update_id = 0;
+}
+}
+
 /*/
 
 typedef struct _DisablingContext DisablingContext;
@@ -1435,7 +1447,10 @@ interface_disabling_step (DisablingContext *ctx)
 ctx-step++;
 }
 
-case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS:
+case DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS: {
+/* Prevent any deferred registration state update from happending 
after the modem is disabled */
+clear_deferred_registration_state_update (ctx-self);
+
 if (MM_IFACE_MODEM_3GPP_GET_INTERFACE 
(ctx-self)-cleanup_unsolicited_registration_events 
 MM_IFACE_MODEM_3GPP_GET_INTERFACE 
(ctx-self)-cleanup_unsolicited_registration_events_finish) {
 MM_IFACE_MODEM_3GPP_GET_INTERFACE 
(ctx-self)-cleanup_unsolicited_registration_events (
@@ -1446,6 +1461,7 @@ interface_disabling_step (DisablingContext *ctx)
 }
 /* Fall down to next step */
 ctx-step++;
+}
 
 case DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS:
 if (MM_IFACE_MODEM_3GPP_GET_INTERFACE 
(ctx-self)-cleanup_unsolicited_events 
-- 
1.8.1.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] iface-modem: fix modem state consolidation upon bearer disconnection

2013-02-19 Thread Ben Chan
Patch iface-modem: fix invalid modem state consolidation (commit
69aff6183a9e6532b4074c89831d6dcfa81ddcce) incorrectly consolidates the
modem state upon the disconnection of a bearer. The modem state remains
'connected' after the last bearer is disconnected. This patch fixes
that.
---
 src/mm-iface-modem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
index 63ef6fc..f02b0bd 100644
--- a/src/mm-iface-modem.c
+++ b/src/mm-iface-modem.c
@@ -270,7 +270,7 @@ bearer_status_changed (MMBearer *bearer,
 new_state = MM_MODEM_STATE_DISCONNECTING;
 break;
 case MM_BEARER_STATUS_DISCONNECTED:
-new_state = get_current_consolidated_state (self, state);
+new_state = get_current_consolidated_state (self, 
MM_MODEM_STATE_UNKNOWN);
 break;
 }
 
-- 
1.8.1.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] iface-modem: fix invalid modem state consolidation

2013-02-19 Thread Ben Chan
I've found a bug in this patch and submitted a fix. Upon the disconnection
of bearers, the consolidation of modem state shouldn't take the curent
state into account, but simply goes through all subsystems.

Thanks,
Ben


On Thu, Jan 24, 2013 at 12:12 AM, Aleksander Morgado
aleksan...@lanedo.comwrote:

 On 24/01/13 02:16, Ben Chan wrote:
  This patch fixes get_current_consolidated_state() in MMIfaceModem to
  avoid invalid state transitions, e.g. from 'enabling' to 'disabled'.
 
  https://code.google.com/p/chromium-os/issues/detail?id=38173

 I modified the commit log to include the information from the bugreport
 and then pushed it. Thanks!


 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] modem: use +CEREG to determine EPS network registration status

2013-02-15 Thread Ben Chan
Thanks Aleksander.

I committed a a patch to the Chromium OS tree to enable EPS registration
check for the novatel-lte plugin. I'd like to see if you and Dan would like
to enable it for any LTE-capable modem by default. If so, I will submit a
follow-up patch to the modem code instead of the novatel-lte plugin.

Ben

On Thu, Feb 14, 2013 at 11:56 PM, Aleksander Morgado
aleksan...@lanedo.comwrote:


 
 
  Will you be able to test my patch v2 on some of the modems you
  mentioned?
 
  Sure, can do.
 
  The V2 patch doesn't seem to cause problems with the ADU960, but then
  again, I don't have access to the LTE networks it can use (ATT and
  Lightsquared).  After patching the code to unconditionally enable CEREG,
  the device responds correctly to CEREG requests:
 
  AT+CEREG=2
  OK
  AT+CEREG?
  +CEREG: 2,1
  OK
 
  when I'm registered on T-Mobile's GSM network.  So while I'd really like
  to have us try CEREG unconditionally if we can, I'm fine with the V2
  patch as it is.


 I just pushed the v2 patch now. Whenever we can test it properly I guess
 we can enable this by default, so not a big deal.

 Ben, will you send a patch to enable it in the novatel-lte plugin?

 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH v4] iface-modem-3gpp: defer registration state update when registration is lost

2013-02-15 Thread Ben Chan
This patch defers the update of 3GPP registration state by 15 seconds
when the registration state changes from 'registered' (home / roaming)
to 'searching'. This allows a temporary loss of 3GPP registration to
recover itself when relying on ModemManager to explicitly disconnect and
reconnect to the network.
---
I rebase patch v3 on top of HEAD, so that this v4 patch can be applied cleanly 
on HEAD.

 src/mm-iface-modem-3gpp.c | 122 ++
 1 file changed, 102 insertions(+), 20 deletions(-)

diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
index 7117e9c..fa8da1a 100644
--- a/src/mm-iface-modem-3gpp.c
+++ b/src/mm-iface-modem-3gpp.c
@@ -29,6 +29,7 @@
 #include mm-log.h
 
 #define REGISTRATION_CHECK_TIMEOUT_SEC 30
+#define DEFERRED_REGISTRATION_STATE_UPDATE_TIMEOUT_SEC 15
 
 #define SUBSYSTEM_3GPP 3gpp
 
@@ -73,6 +74,8 @@ typedef struct {
 MMModem3gppRegistrationState cs;
 MMModem3gppRegistrationState ps;
 MMModem3gppRegistrationState eps;
+MMModem3gppRegistrationState deferred_new_state;
+guint deferred_update_id;
 gboolean manual_registration;
 GCancellable *pending_registration_cancellable;
 gboolean reloading_operator;
@@ -85,6 +88,9 @@ registration_state_context_free (RegistrationStateContext 
*ctx)
 g_cancellable_cancel (ctx-pending_registration_cancellable);
 g_object_unref (ctx-pending_registration_cancellable);
 }
+if (ctx-deferred_update_id) {
+g_source_remove (ctx-deferred_update_id);
+}
 g_slice_free (RegistrationStateContext, ctx);
 }
 
@@ -104,6 +110,7 @@ get_registration_state_context (MMIfaceModem3gpp *self)
 ctx-cs = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
 ctx-ps = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
 ctx-eps = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
+ctx-deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
 
 g_object_set_qdata_full (
 G_OBJECT (self),
@@ -1033,25 +1040,98 @@ update_registration_reload_current_operator_ready 
(MMIfaceModem3gpp *self,
 }
 
 static void
+update_non_registered_state (MMIfaceModem3gpp *self,
+ MMModem3gppRegistrationState old_state,
+ MMModem3gppRegistrationState new_state)
+{
+/* Not registered neither in home nor roaming network */
+mm_iface_modem_3gpp_clear_current_operator (self);
+
+/* The property in the interface is bound to the property
+ * in the skeleton, so just updating here is enough */
+g_object_set (self,
+  MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, new_state,
+  NULL);
+
+mm_iface_modem_update_subsystem_state (
+MM_IFACE_MODEM (self),
+SUBSYSTEM_3GPP,
+(new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ?
+ MM_MODEM_STATE_SEARCHING :
+ MM_MODEM_STATE_ENABLED),
+MM_MODEM_STATE_CHANGE_REASON_UNKNOWN);
+}
+
+static gboolean
+run_deferred_registration_state_update (MMIfaceModem3gpp *self)
+{
+MMModem3gppRegistrationState old_state = 
MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
+MMModem3gppRegistrationState new_state;
+RegistrationStateContext *ctx;
+
+ctx = get_registration_state_context (self);
+ctx-deferred_update_id = 0;
+
+g_object_get (self,
+  MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, old_state,
+  NULL);
+new_state = ctx-deferred_new_state;
+
+/* Only set new state if different */
+if (new_state == old_state)
+return FALSE;
+
+mm_info (Modem %s: (deferred) 3GPP Registration state changed (%s - %s),
+ g_dbus_object_get_object_path (G_DBUS_OBJECT (self)),
+ mm_modem_3gpp_registration_state_get_string (old_state),
+ mm_modem_3gpp_registration_state_get_string (new_state));
+
+update_non_registered_state (self, old_state, new_state);
+
+return FALSE;
+}
+
+static void
 update_registration_state (MMIfaceModem3gpp *self,
MMModem3gppRegistrationState new_state)
 {
 MMModem3gppRegistrationState old_state = 
MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
+RegistrationStateContext *ctx;
 
 g_object_get (self,
   MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, old_state,
   NULL);
 
+ctx = get_registration_state_context (self);
+g_assert (ctx);
+
+if (ctx-deferred_update_id != 0) {
+/* If there is already a deferred 'registration loss' state update and 
the new update
+ * is not a registered state, update the deferred state update without 
extending the
+ * timeout. */
+if (new_state != MM_MODEM_3GPP_REGISTRATION_STATE_HOME 
+new_state != MM_MODEM_3GPP_REGISTRATION_STATE_ROAMING) {
+mm_info (Modem %s: 3GPP Registration state changed (%s - %s), 
update deferred,
+ g_dbus_object_get_object_path (G_DBUS_OBJECT (self)),
+

[MM] [PATCH v3] iface-modem-3gpp: defer registration state update when registration is lost

2013-02-14 Thread Ben Chan
This patch defers the update of 3GPP registration state by 15 seconds
when the registration state changes from 'registered' (home / roaming)
to 'searching'. This allows a temporary loss of 3GPP registration to
recover itself when relying on ModemManager to explicitly disconnect and
reconnect to the network.
---
This patch addresses the comments from Aleksander Morgado 
aleksan...@lanedo.com.

https://mail.gnome.org/archives/networkmanager-list/2013-February/msg00074.html

 src/mm-iface-modem-3gpp.c | 122 ++
 1 file changed, 102 insertions(+), 20 deletions(-)

diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
index 7a88b2f..d1278d4 100644
--- a/src/mm-iface-modem-3gpp.c
+++ b/src/mm-iface-modem-3gpp.c
@@ -29,6 +29,7 @@
 #include mm-log.h
 
 #define REGISTRATION_CHECK_TIMEOUT_SEC 30
+#define DEFERRED_REGISTRATION_STATE_UPDATE_TIMEOUT_SEC 15
 
 #define SUBSYSTEM_3GPP 3gpp
 
@@ -72,6 +73,8 @@ mm_iface_modem_3gpp_bind_simple_status (MMIfaceModem3gpp 
*self,
 typedef struct {
 MMModem3gppRegistrationState cs;
 MMModem3gppRegistrationState ps;
+MMModem3gppRegistrationState deferred_new_state;
+guint deferred_update_id;
 gboolean manual_registration;
 GCancellable *pending_registration_cancellable;
 gboolean reloading_operator;
@@ -84,6 +87,9 @@ registration_state_context_free (RegistrationStateContext 
*ctx)
 g_cancellable_cancel (ctx-pending_registration_cancellable);
 g_object_unref (ctx-pending_registration_cancellable);
 }
+if (ctx-deferred_update_id) {
+g_source_remove (ctx-deferred_update_id);
+}
 g_slice_free (RegistrationStateContext, ctx);
 }
 
@@ -102,6 +108,7 @@ get_registration_state_context (MMIfaceModem3gpp *self)
 ctx = g_slice_new0 (RegistrationStateContext);
 ctx-cs = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
 ctx-ps = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
+ctx-deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
 
 g_object_set_qdata_full (
 G_OBJECT (self),
@@ -1019,25 +1026,98 @@ update_registration_reload_current_operator_ready 
(MMIfaceModem3gpp *self,
 }
 
 static void
+update_non_registered_state (MMIfaceModem3gpp *self,
+ MMModem3gppRegistrationState old_state,
+ MMModem3gppRegistrationState new_state)
+{
+/* Not registered neither in home nor roaming network */
+mm_iface_modem_3gpp_clear_current_operator (self);
+
+/* The property in the interface is bound to the property
+ * in the skeleton, so just updating here is enough */
+g_object_set (self,
+  MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, new_state,
+  NULL);
+
+mm_iface_modem_update_subsystem_state (
+MM_IFACE_MODEM (self),
+SUBSYSTEM_3GPP,
+(new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ?
+ MM_MODEM_STATE_SEARCHING :
+ MM_MODEM_STATE_ENABLED),
+MM_MODEM_STATE_CHANGE_REASON_UNKNOWN);
+}
+
+static gboolean
+run_deferred_registration_state_update (MMIfaceModem3gpp *self)
+{
+MMModem3gppRegistrationState old_state = 
MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
+MMModem3gppRegistrationState new_state;
+RegistrationStateContext *ctx;
+
+ctx = get_registration_state_context (self);
+ctx-deferred_update_id = 0;
+
+g_object_get (self,
+  MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, old_state,
+  NULL);
+new_state = ctx-deferred_new_state;
+
+/* Only set new state if different */
+if (new_state == old_state)
+return FALSE;
+
+mm_info (Modem %s: (deferred) 3GPP Registration state changed (%s - %s),
+ g_dbus_object_get_object_path (G_DBUS_OBJECT (self)),
+ mm_modem_3gpp_registration_state_get_string (old_state),
+ mm_modem_3gpp_registration_state_get_string (new_state));
+
+update_non_registered_state (self, old_state, new_state);
+
+return FALSE;
+}
+
+static void
 update_registration_state (MMIfaceModem3gpp *self,
MMModem3gppRegistrationState new_state)
 {
 MMModem3gppRegistrationState old_state = 
MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
+RegistrationStateContext *ctx;
 
 g_object_get (self,
   MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, old_state,
   NULL);
 
+ctx = get_registration_state_context (self);
+g_assert (ctx);
+
+if (ctx-deferred_update_id != 0) {
+/* If there is already a deferred 'registration loss' state update and 
the new update
+ * is not a registered state, update the deferred state update without 
extending the
+ * timeout. */
+if (new_state != MM_MODEM_3GPP_REGISTRATION_STATE_HOME 
+new_state != MM_MODEM_3GPP_REGISTRATION_STATE_ROAMING) {
+mm_info (Modem %s: 3GPP Registration state changed (%s - %s), 
update 

Re: [MM] [PATCH] iface-modem-3gpp: defer registration state update when registration is lost

2013-02-14 Thread Ben Chan
Thanks Aleksander. Submitted patch v3.


On Thu, Feb 14, 2013 at 2:01 AM, Aleksander Morgado
aleksan...@lanedo.comwrote:

 On 02/12/2013 02:18 AM, Ben Chan wrote:
  This patch defers the update of 3GPP registration state by 15 seconds
  when the registration state changes from 'registered' (home / roaming)
  to 'searching'. This allows a temporary loss of 3GPP registration to
  recover itself when relying on ModemManager to explicitly disconnect and
  reconnect to the network.


 I tried to test this with a QMI modem and an external antenna, but
 disconnecting the antenna while connected just gets me to signal quality
 0, it doesn't get neither unregistered nor disconnected. Are you able to
 grab me some debug logs of a test case where this happens?

 See some additional comments inline below.


  ---
   src/mm-iface-modem-3gpp.c | 108
 +-
   1 file changed, 88 insertions(+), 20 deletions(-)
 
  diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
  index 7a88b2f..f5d703d 100644
  --- a/src/mm-iface-modem-3gpp.c
  +++ b/src/mm-iface-modem-3gpp.c
  @@ -29,6 +29,7 @@
   #include mm-log.h
 
   #define REGISTRATION_CHECK_TIMEOUT_SEC 30
  +#define DEFERRED_REGISTRATION_STATE_UPDATE_TIMEOUT_SEC 15
 
   #define SUBSYSTEM_3GPP 3gpp
 
  @@ -72,6 +73,8 @@ mm_iface_modem_3gpp_bind_simple_status
 (MMIfaceModem3gpp *self,
   typedef struct {
   MMModem3gppRegistrationState cs;
   MMModem3gppRegistrationState ps;
  +MMModem3gppRegistrationState deferred_new_state;
  +guint deferred_update_id;
   gboolean manual_registration;
   GCancellable *pending_registration_cancellable;
   gboolean reloading_operator;
  @@ -84,6 +87,9 @@ registration_state_context_free
 (RegistrationStateContext *ctx)
   g_cancellable_cancel (ctx-pending_registration_cancellable);
   g_object_unref (ctx-pending_registration_cancellable);
   }
  +if (ctx-deferred_update_id) {
  +g_source_remove (ctx-deferred_update_id);
  +}
   g_slice_free (RegistrationStateContext, ctx);
   }
 
  @@ -102,6 +108,7 @@ get_registration_state_context (MMIfaceModem3gpp
 *self)
   ctx = g_slice_new0 (RegistrationStateContext);
   ctx-cs = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
   ctx-ps = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
  +ctx-deferred_new_state =
 MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
 
   g_object_set_qdata_full (
   G_OBJECT (self),
  @@ -1019,25 +1026,84 @@
 update_registration_reload_current_operator_ready (MMIfaceModem3gpp *self,
   }
 
   static void
  +update_non_registered_state (MMIfaceModem3gpp *self,
  + MMModem3gppRegistrationState old_state,
  + MMModem3gppRegistrationState new_state)
  +{
  +/* Not registered neither in home nor roaming network */
  +mm_iface_modem_3gpp_clear_current_operator (self);
  +
  +/* The property in the interface is bound to the property
  + * in the skeleton, so just updating here is enough */
  +g_object_set (self,
  +  MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, new_state,
  +  NULL);
  +
  +mm_iface_modem_update_subsystem_state (
  +MM_IFACE_MODEM (self),
  +SUBSYSTEM_3GPP,
  +(new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ?
  + MM_MODEM_STATE_SEARCHING :
  + MM_MODEM_STATE_ENABLED),
  +MM_MODEM_STATE_CHANGE_REASON_UNKNOWN);
  +}
  +
  +static gboolean
  +run_deferred_registration_state_update (MMIfaceModem3gpp *self)
  +{
  +MMModem3gppRegistrationState old_state =
 MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
  +MMModem3gppRegistrationState new_state;
  +RegistrationStateContext *ctx;
  +
  +ctx = get_registration_state_context (self);
  +ctx-deferred_update_id = 0;
  +
  +g_object_get (self,
  +  MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, old_state,
  +  NULL);
  +new_state = ctx-deferred_new_state;
  +
  +/* Only set new state if it is known and different from the old one
 */
  +if (new_state == MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN ||
  +new_state == old_state)
  +return FALSE;

 Why do we bail out if UNKNOWN here? If we end up 15s without getting
 re-registered and the original new state that we got was UNKNOWN, we
 would not be doing anything, so the state would still say 'registered'
 in the interface, while we are not.

  +
  +mm_info (Modem %s: (deferred) 3GPP Registration state changed (%s
 - %s),
  + g_dbus_object_get_object_path (G_DBUS_OBJECT (self)),
  + mm_modem_3gpp_registration_state_get_string (old_state),
  + mm_modem_3gpp_registration_state_get_string (new_state));
  +
  +update_non_registered_state (self, old_state, new_state);
  +
  +return FALSE;
  +}
  +
  +static void
   update_registration_state (MMIfaceModem3gpp

Re: [MM] [PATCH] modem: use +CEREG to determine EPS network registration status

2013-02-12 Thread Ben Chan
Thanks Aleksander.  My patch has a typo bug
in unsolicited_registration_events_context_step where the eps_error variable
in UnsolicitedRegistrationEventsContext is not properly set to NULL after
the ownership of GError object pointed by eps_error is transfered. I've
submitted a revised patch.

- Ben

On Mon, Feb 11, 2013 at 11:47 PM, Aleksander Morgado
aleksan...@lanedo.comwrote:

 On 02/11/2013 07:44 PM, Ben Chan wrote:
  Aleksander and Dan,
 
  How do you feel about this patch?
 


 Doesn't look bad, but I really need to read it carefully as it changed
 lots of things. The idea is that plugins can specify whether they
 require CEREG by setting MM_IFACE_MODEM_3GPP_EPS_NETWORK_SUPPORTED to
 TRUE themselves, instead of automatically trying it when LTE support is
 found, right? I now wonder how other AT-controlled LTE modems out there,
 not just the Novatel LTE, handle this. Is anyone willing to give it a
 try with other LTE modems?


___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] iface-modem-3gpp: defer registration state update when registration is lost

2013-02-11 Thread Ben Chan
This patch defers the update of 3GPP registration state by 15 seconds
when the registration state changes from 'registered' (home / roaming)
to 'searching'. This allows a temporary loss of 3GPP registration to
recover itself when relying on ModemManager to explicitly disconnect and
reconnect to the network.
---
 src/mm-iface-modem-3gpp.c | 108 +-
 1 file changed, 88 insertions(+), 20 deletions(-)

diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
index 7a88b2f..f5d703d 100644
--- a/src/mm-iface-modem-3gpp.c
+++ b/src/mm-iface-modem-3gpp.c
@@ -29,6 +29,7 @@
 #include mm-log.h
 
 #define REGISTRATION_CHECK_TIMEOUT_SEC 30
+#define DEFERRED_REGISTRATION_STATE_UPDATE_TIMEOUT_SEC 15
 
 #define SUBSYSTEM_3GPP 3gpp
 
@@ -72,6 +73,8 @@ mm_iface_modem_3gpp_bind_simple_status (MMIfaceModem3gpp 
*self,
 typedef struct {
 MMModem3gppRegistrationState cs;
 MMModem3gppRegistrationState ps;
+MMModem3gppRegistrationState deferred_new_state;
+guint deferred_update_id;
 gboolean manual_registration;
 GCancellable *pending_registration_cancellable;
 gboolean reloading_operator;
@@ -84,6 +87,9 @@ registration_state_context_free (RegistrationStateContext 
*ctx)
 g_cancellable_cancel (ctx-pending_registration_cancellable);
 g_object_unref (ctx-pending_registration_cancellable);
 }
+if (ctx-deferred_update_id) {
+g_source_remove (ctx-deferred_update_id);
+}
 g_slice_free (RegistrationStateContext, ctx);
 }
 
@@ -102,6 +108,7 @@ get_registration_state_context (MMIfaceModem3gpp *self)
 ctx = g_slice_new0 (RegistrationStateContext);
 ctx-cs = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
 ctx-ps = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
+ctx-deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
 
 g_object_set_qdata_full (
 G_OBJECT (self),
@@ -1019,25 +1026,84 @@ update_registration_reload_current_operator_ready 
(MMIfaceModem3gpp *self,
 }
 
 static void
+update_non_registered_state (MMIfaceModem3gpp *self,
+ MMModem3gppRegistrationState old_state,
+ MMModem3gppRegistrationState new_state)
+{
+/* Not registered neither in home nor roaming network */
+mm_iface_modem_3gpp_clear_current_operator (self);
+
+/* The property in the interface is bound to the property
+ * in the skeleton, so just updating here is enough */
+g_object_set (self,
+  MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, new_state,
+  NULL);
+
+mm_iface_modem_update_subsystem_state (
+MM_IFACE_MODEM (self),
+SUBSYSTEM_3GPP,
+(new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ?
+ MM_MODEM_STATE_SEARCHING :
+ MM_MODEM_STATE_ENABLED),
+MM_MODEM_STATE_CHANGE_REASON_UNKNOWN);
+}
+
+static gboolean
+run_deferred_registration_state_update (MMIfaceModem3gpp *self)
+{
+MMModem3gppRegistrationState old_state = 
MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
+MMModem3gppRegistrationState new_state;
+RegistrationStateContext *ctx;
+
+ctx = get_registration_state_context (self);
+ctx-deferred_update_id = 0;
+
+g_object_get (self,
+  MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, old_state,
+  NULL);
+new_state = ctx-deferred_new_state;
+
+/* Only set new state if it is known and different from the old one */
+if (new_state == MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN ||
+new_state == old_state)
+return FALSE;
+
+mm_info (Modem %s: (deferred) 3GPP Registration state changed (%s - %s),
+ g_dbus_object_get_object_path (G_DBUS_OBJECT (self)),
+ mm_modem_3gpp_registration_state_get_string (old_state),
+ mm_modem_3gpp_registration_state_get_string (new_state));
+
+update_non_registered_state (self, old_state, new_state);
+
+return FALSE;
+}
+
+static void
 update_registration_state (MMIfaceModem3gpp *self,
MMModem3gppRegistrationState new_state)
 {
 MMModem3gppRegistrationState old_state = 
MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
+RegistrationStateContext *ctx;
 
 g_object_get (self,
   MM_IFACE_MODEM_3GPP_REGISTRATION_STATE, old_state,
   NULL);
 
+ctx = get_registration_state_context (self);
+g_assert (ctx);
+
+/* Cancel any deferred registration state update */
+if (ctx-deferred_update_id != 0) {
+g_source_remove (ctx-deferred_update_id);
+ctx-deferred_update_id = 0;
+}
+
 /* Only set new state if different */
 if (new_state == old_state)
 return;
 
 if (new_state == MM_MODEM_3GPP_REGISTRATION_STATE_HOME ||
 new_state == MM_MODEM_3GPP_REGISTRATION_STATE_ROAMING) {
-RegistrationStateContext *ctx;
-
-ctx = get_registration_state_context (self);
-
 /* If already 

[MM] [PATCH] modem: use +CEREG to determine EPS network registration status

2013-02-06 Thread Ben Chan
This patch adds the support for solicited/unsolicited EPS network
registration status via AT+CEREG, which is configurable via the
'iface-modem-3gpp-eps-network-supported' property of the
MMIfaceModem3gpp interface and is disabled by default.
---
 src/mm-broadband-modem-qmi.c   |   3 +
 src/mm-broadband-modem.c   | 165 +++--
 src/mm-iface-modem-3gpp.c  |  51 -
 src/mm-iface-modem-3gpp.h  |  10 ++-
 src/mm-modem-helpers.c |  82 +++-
 src/mm-modem-helpers.h |   1 +
 src/tests/test-modem-helpers.c | 121 +++---
 7 files changed, 365 insertions(+), 68 deletions(-)

diff --git a/src/mm-broadband-modem-qmi.c b/src/mm-broadband-modem-qmi.c
index aa82f0e..05be718 100644
--- a/src/mm-broadband-modem-qmi.c
+++ b/src/mm-broadband-modem-qmi.c
@@ -3984,6 +3984,7 @@ static void
 modem_3gpp_run_registration_checks (MMIfaceModem3gpp *self,
 gboolean cs_supported,
 gboolean ps_supported,
+gboolean eps_supported,
 GAsyncReadyCallback callback,
 gpointer user_data)
 {
@@ -4138,6 +4139,7 @@ static void
 modem_3gpp_disable_unsolicited_registration_events (MMIfaceModem3gpp *self,
 gboolean cs_supported,
 gboolean ps_supported,
+gboolean eps_supported,
 GAsyncReadyCallback 
callback,
 gpointer user_data)
 {
@@ -4183,6 +4185,7 @@ static void
 modem_3gpp_enable_unsolicited_registration_events (MMIfaceModem3gpp *self,
gboolean cs_supported,
gboolean ps_supported,
+   gboolean eps_supported,
GAsyncReadyCallback 
callback,
gpointer user_data)
 {
diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 3778b56..505c42a 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -86,6 +86,7 @@ enum {
 PROP_MODEM_3GPP_REGISTRATION_STATE,
 PROP_MODEM_3GPP_CS_NETWORK_SUPPORTED,
 PROP_MODEM_3GPP_PS_NETWORK_SUPPORTED,
+PROP_MODEM_3GPP_EPS_NETWORK_SUPPORTED,
 PROP_MODEM_CDMA_CDMA1X_REGISTRATION_STATE,
 PROP_MODEM_CDMA_EVDO_REGISTRATION_STATE,
 PROP_MODEM_CDMA_CDMA1X_NETWORK_SUPPORTED,
@@ -129,6 +130,7 @@ struct _MMBroadbandModemPrivate {
 MMModem3gppRegistrationState modem_3gpp_registration_state;
 gboolean modem_3gpp_cs_network_supported;
 gboolean modem_3gpp_ps_network_supported;
+gboolean modem_3gpp_eps_network_supported;
 /* Implementation helpers */
 GPtrArray *modem_3gpp_registration_regex;
 
@@ -3280,6 +3282,7 @@ registration_state_changed (MMAtSerialPort *port,
 gulong lac = 0, cell_id = 0;
 MMModemAccessTechnology act = MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN;
 gboolean cgreg = FALSE;
+gboolean cereg = FALSE;
 GError *error = NULL;
 
 if (!mm_3gpp_parse_creg_response (match_info,
@@ -3288,6 +3291,7 @@ registration_state_changed (MMAtSerialPort *port,
   cell_id,
   act,
   cgreg,
+  cereg,
   error)) {
 mm_warn (error parsing unsolicited registration: %s,
  error  error-message ? error-message : (unknown));
@@ -3298,6 +3302,8 @@ registration_state_changed (MMAtSerialPort *port,
 /* Report new registration state */
 if (cgreg)
 mm_iface_modem_3gpp_update_ps_registration_state (MM_IFACE_MODEM_3GPP 
(self), state);
+else if (cereg)
+mm_iface_modem_3gpp_update_eps_registration_state (MM_IFACE_MODEM_3GPP 
(self), state);
 else
 mm_iface_modem_3gpp_update_cs_registration_state (MM_IFACE_MODEM_3GPP 
(self), state);
 
@@ -3484,12 +3490,16 @@ typedef struct {
 GSimpleAsyncResult *result;
 gboolean cs_supported;
 gboolean ps_supported;
+gboolean eps_supported;
 gboolean run_cs;
 gboolean run_ps;
+gboolean run_eps;
 gboolean running_cs;
 gboolean running_ps;
+gboolean running_eps;
 GError *cs_error;
 GError *ps_error;
+GError *eps_error;
 } RunRegistrationChecksContext;
 
 static void
@@ -3500,6 +3510,8 @@ run_registration_checks_context_complete_and_free 
(RunRegistrationChecksContext
 g_error_free (ctx-cs_error);
 if (ctx-ps_error)
 g_error_free (ctx-ps_error);
+if (ctx-eps_error)
+g_error_free (ctx-eps_error);
 

[MM] [PATCH] novatel-lte: retry $NWQMISTATUS check upon error during disconnect

2013-01-30 Thread Ben Chan
$NWQMISTATUS sometimes returns 'ERROR'. This patch modifies the Novatel
LTE plugin to retry $NWQMISTATUS (up to 5 times) to determine if the
disconnect operation succeeds. It also changes the plugin to assume that
the disconnect operation succeeds if $NWQMISTATUS fails to report the
current connection status.
---
 plugins/novatel/mm-broadband-bearer-novatel-lte.c | 68 +--
 1 file changed, 51 insertions(+), 17 deletions(-)

diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c 
b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
index 80c7464..4b67cd6 100644
--- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
@@ -325,6 +325,7 @@ typedef struct {
 MMAtSerialPort *secondary;
 MMPort *data;
 GSimpleAsyncResult *result;
+gint retries;
 } DetailedDisconnectContext;
 
 static DetailedDisconnectContext *
@@ -348,6 +349,7 @@ detailed_disconnect_context_new (MMBroadbandBearer *self,
  callback,
  user_data,
  detailed_disconnect_context_new);
+ctx-retries = 4;
 return ctx;
 }
 
@@ -373,34 +375,72 @@ disconnect_3gpp_finish (MMBroadbandBearer *self,
 return !g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT 
(res), error);
 }
 
+static gboolean disconnect_3gpp_qmistatus (DetailedDisconnectContext *ctx);
+
 static void
-disconnect_3gpp_status_complete (MMBaseModem *modem,
- GAsyncResult *res,
- DetailedDisconnectContext *ctx)
+disconnect_3gpp_status_ready (MMBaseModem *modem,
+  GAsyncResult *res,
+  DetailedDisconnectContext *ctx)
 {
 const gchar *result;
 GError *error = NULL;
+gboolean is_connected = FALSE;
 
 result = mm_base_modem_at_command_finish (MM_BASE_MODEM (modem),
   res,
   error);
-if (error) {
-mm_dbg(QMI connection status failed: %s, error-message);
+if (result) {
+mm_dbg (QMI connection status: %s, result);
+if (is_qmistatus_disconnected (result)) {
+g_simple_async_result_set_op_res_gboolean (ctx-result, TRUE);
+detailed_disconnect_context_complete_and_free (ctx);
+return;
+} else if (is_qmistatus_connected (result)) {
+is_connected = TRUE;
+}
+} else {
+mm_dbg (QMI connection status failed: %s, error-message);
 g_error_free (error);
 }
 
-if (result  is_qmistatus_disconnected (result))
-g_simple_async_result_set_op_res_gboolean (ctx-result, TRUE);
-else
+if (ctx-retries  0) {
+ctx-retries--;
+mm_dbg (Retrying status check in a second. %d retries left.,
+ctx-retries);
+g_timeout_add_seconds (1, (GSourceFunc)disconnect_3gpp_qmistatus, ctx);
+return;
+}
+
+/* If $NWQMISTATUS reports a CONNECTED QMI state, returns an error such 
that
+ * the modem state remains 'connected'. Otherwise, assumes the modem is
+ * disconnected from the network successfully. */
+if (is_connected)
 g_simple_async_result_set_error (ctx-result,
  MM_CORE_ERROR,
  MM_CORE_ERROR_FAILED,
- Error checking if disconnected (%s),
- result ? result : no result);
+ QMI disconnect failed: %s,
+ result);
+else
+g_simple_async_result_set_op_res_gboolean (ctx-result, TRUE);
 
 detailed_disconnect_context_complete_and_free (ctx);
 }
 
+static gboolean
+disconnect_3gpp_qmistatus (DetailedDisconnectContext *ctx)
+{
+mm_base_modem_at_command (
+ctx-modem,
+$NWQMISTATUS,
+3, /* timeout */
+FALSE, /* allow_cached */
+(GAsyncReadyCallback)disconnect_3gpp_status_ready, /* callback */
+ctx); /* user_data */
+
+return FALSE;
+}
+
+
 static void
 disconnect_3gpp_check_status (MMBaseModem *modem,
   GAsyncResult *res,
@@ -416,13 +456,7 @@ disconnect_3gpp_check_status (MMBaseModem *modem,
 g_error_free (error);
 }
 
-mm_base_modem_at_command (
-ctx-modem,
-$NWQMISTATUS,
-3, /* timeout */
-FALSE, /* allow_cached */
-(GAsyncReadyCallback)disconnect_3gpp_status_complete,
-ctx); /* user_data */
+disconnect_3gpp_qmistatus (ctx);
 }
 
 static void
-- 
1.8.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] novatel-lte: use +CFUN=4 for power down

2013-01-30 Thread Ben Chan
---
 plugins/novatel/mm-broadband-modem-novatel-lte.c | 26 
 1 file changed, 26 insertions(+)

diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c 
b/plugins/novatel/mm-broadband-modem-novatel-lte.c
index d32c397..9a21c4b 100644
--- a/plugins/novatel/mm-broadband-modem-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c
@@ -149,6 +149,30 @@ modem_init (MMIfaceModem *self,
 }
 
 /*/
+/* Modem power down (Modem interface) */
+
+static gboolean
+modem_power_down_finish (MMIfaceModem *self,
+ GAsyncResult *res,
+ GError **error)
+{
+return !!mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, 
error);
+}
+
+static void
+modem_power_down (MMIfaceModem *self,
+  GAsyncReadyCallback callback,
+  gpointer user_data)
+{
+mm_base_modem_at_command (MM_BASE_MODEM (self),
+  +CFUN=4,
+  6,
+  FALSE,
+  callback,
+  user_data);
+}
+
+/*/
 /* Create Bearer (Modem interface) */
 
 static MMBearer *
@@ -745,6 +769,8 @@ iface_modem_init (MMIfaceModem *iface)
 {
 iface-modem_init = modem_init;
 iface-modem_init_finish = modem_init_finish;
+iface-modem_power_down = modem_power_down;
+iface-modem_power_down_finish = modem_power_down_finish;
 iface-create_bearer = modem_create_bearer;
 iface-create_bearer_finish = modem_create_bearer_finish;
 iface-create_sim = modem_create_sim;
-- 
1.8.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] iface-modem: add a missing step increment in interface_initialization_step

2013-01-30 Thread Ben Chan
---
 src/mm-iface-modem.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
index 10144aa..722664c 100644
--- a/src/mm-iface-modem.c
+++ b/src/mm-iface-modem.c
@@ -3979,6 +3979,8 @@ interface_initialization_step (InitializationContext *ctx)
 g_object_unref (sim);
 return;
 }
+/* Fall down to next step */
+ctx-step++;
 
 case INITIALIZATION_STEP_OWN_NUMBERS:
 /* Own numbers is meant to be loaded only once during the whole
-- 
1.8.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] iface-modem: rearrange initialization steps

2013-01-24 Thread Ben Chan
This patch rearranges the initialization steps in MMIfaceModem such that
the following SIM related operations happen at the end of the
initialization:
- INITIALIZATION_STEP_UNLOCK_REQUIRED
- INITIALIZATION_STEP_SIM
- INITIALIZATION_STEP_OWN_NUMBERS

The rationale of this change is that the SIM interface of some modems
may require some time to initialize before it responds to SIM related
AT commands. By rearranging the initialization steps to execute non-SIM
related AT commands first, some of the latency for the SIM
initialization can be absorbed.
---
 src/mm-iface-modem.c | 118 +--
 1 file changed, 59 insertions(+), 59 deletions(-)

diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
index 6b812e2..3827a99 100644
--- a/src/mm-iface-modem.c
+++ b/src/mm-iface-modem.c
@@ -3401,12 +3401,12 @@ typedef enum {
 INITIALIZATION_STEP_REVISION,
 INITIALIZATION_STEP_EQUIPMENT_ID,
 INITIALIZATION_STEP_DEVICE_ID,
-INITIALIZATION_STEP_UNLOCK_REQUIRED,
-INITIALIZATION_STEP_SIM,
-INITIALIZATION_STEP_OWN_NUMBERS,
 INITIALIZATION_STEP_SUPPORTED_MODES,
 INITIALIZATION_STEP_SUPPORTED_BANDS,
 INITIALIZATION_STEP_POWER_STATE,
+INITIALIZATION_STEP_UNLOCK_REQUIRED,
+INITIALIZATION_STEP_SIM,
+INITIALIZATION_STEP_OWN_NUMBERS,
 INITIALIZATION_STEP_LAST
 } InitializationStep;
 
@@ -3872,62 +3872,6 @@ interface_initialization_step (InitializationContext 
*ctx)
 /* Fall down to next step */
 ctx-step++;
 
-case INITIALIZATION_STEP_UNLOCK_REQUIRED:
-/* Only check unlock required if we were previously not unlocked */
-if (mm_gdbus_modem_get_unlock_required (ctx-skeleton) != 
MM_MODEM_LOCK_NONE) {
-mm_iface_modem_update_lock_info (ctx-self,
- MM_MODEM_LOCK_UNKNOWN, /* ask */
- 
(GAsyncReadyCallback)modem_update_lock_info_ready,
- ctx);
-return;
-}
-/* Fall down to next step */
-ctx-step++;
-
-case INITIALIZATION_STEP_SIM:
-/* If the modem doesn't need any SIM, skip */
-if (MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-create_sim 
-MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-create_sim_finish) {
-MMSim *sim = NULL;
-
-g_object_get (ctx-self,
-  MM_IFACE_MODEM_SIM, sim,
-  NULL);
-if (!sim) {
-MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-create_sim (
-MM_IFACE_MODEM (ctx-self),
-(GAsyncReadyCallback)sim_new_ready,
-ctx);
-return;
-}
-
-/* If already available the sim object, relaunch initialization.
- * This will try to load any missing property value that couldn't 
be
- * retrieved before due to having the SIM locked. */
-mm_sim_initialize (sim,
-   NULL, /* TODO: cancellable */
-   (GAsyncReadyCallback)sim_reinit_ready,
-   ctx);
-g_object_unref (sim);
-return;
-}
-
-case INITIALIZATION_STEP_OWN_NUMBERS:
-/* Own numbers is meant to be loaded only once during the whole
- * lifetime of the modem. Therefore, if we already have them loaded,
- * don't try to load them again. */
-if (mm_gdbus_modem_get_own_numbers (ctx-skeleton) == NULL 
-MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers 
-MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers_finish) 
{
-MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers (
-ctx-self,
-(GAsyncReadyCallback)load_own_numbers_ready,
-ctx);
-return;
-}
-/* Fall down to next step */
-ctx-step++;
-
 case INITIALIZATION_STEP_SUPPORTED_MODES:
 g_assert (MM_IFACE_MODEM_GET_INTERFACE 
(ctx-self)-load_supported_modes != NULL);
 g_assert (MM_IFACE_MODEM_GET_INTERFACE 
(ctx-self)-load_supported_modes_finish != NULL);
@@ -3995,6 +3939,62 @@ interface_initialization_step (InitializationContext 
*ctx)
 /* Fall down to next step */
 ctx-step++;
 
+case INITIALIZATION_STEP_UNLOCK_REQUIRED:
+/* Only check unlock required if we were previously not unlocked */
+if (mm_gdbus_modem_get_unlock_required (ctx-skeleton) != 
MM_MODEM_LOCK_NONE) {
+mm_iface_modem_update_lock_info (ctx-self,
+ MM_MODEM_LOCK_UNKNOWN, /* ask */
+ 
(GAsyncReadyCallback)modem_update_lock_info_ready,
+ ctx);
+return;
+}
+/* Fall down to next step */
+ctx-step++;
+
+case 

[MM] [PATCH] iface-modem: fix invalid modem state consolidation

2013-01-23 Thread Ben Chan
This patch fixes get_current_consolidated_state() in MMIfaceModem to
avoid invalid state transitions, e.g. from 'enabling' to 'disabled'.

https://code.google.com/p/chromium-os/issues/detail?id=38173
---
 src/mm-iface-modem.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
index 6b812e2..f6af878 100644
--- a/src/mm-iface-modem.c
+++ b/src/mm-iface-modem.c
@@ -205,7 +205,7 @@ mm_iface_modem_wait_for_final_state (MMIfaceModem *self,
 
 /*/
 
-static MMModemState get_current_consolidated_state (MMIfaceModem *self);
+static MMModemState get_current_consolidated_state (MMIfaceModem *self, 
MMModemState modem_state);
 
 typedef struct {
 MMBearer *self;
@@ -270,7 +270,7 @@ bearer_status_changed (MMBearer *bearer,
 new_state = MM_MODEM_STATE_DISCONNECTING;
 break;
 case MM_BEARER_STATUS_DISCONNECTED:
-new_state = get_current_consolidated_state (self);
+new_state = get_current_consolidated_state (self, state);
 break;
 }
 
@@ -1184,7 +1184,7 @@ mm_iface_modem_update_state (MMIfaceModem *self,
 
 /* Enabled may really be searching or registered */
 if (new_state == MM_MODEM_STATE_ENABLED)
-new_state = get_current_consolidated_state (self);
+new_state = get_current_consolidated_state (self, new_state);
 
 /* Update state only if different */
 if (new_state != old_state) {
@@ -1260,9 +1260,9 @@ subsystem_state_array_free (GArray *array)
 }
 
 static MMModemState
-get_current_consolidated_state (MMIfaceModem *self)
+get_current_consolidated_state (MMIfaceModem *self, MMModemState modem_state)
 {
-MMModemState consolidated = MM_MODEM_STATE_DISABLED;
+MMModemState consolidated = modem_state;
 GArray *subsystem_states;
 
 if (G_UNLIKELY (!state_update_context_quark))
@@ -1293,6 +1293,7 @@ get_current_consolidated_state (MMIfaceModem *self)
 
 static MMModemState
 get_updated_consolidated_state (MMIfaceModem *self,
+MMModemState modem_state,
 const gchar *subsystem,
 MMModemState subsystem_state)
 {
@@ -1345,7 +1346,7 @@ get_updated_consolidated_state (MMIfaceModem *self,
 g_array_append_val (subsystem_states, s);
 }
 
-return get_current_consolidated_state (self);
+return get_current_consolidated_state (self, modem_state);
 }
 
 void
@@ -1364,7 +1365,7 @@ mm_iface_modem_update_subsystem_state (MMIfaceModem *self,
 /* We may have different subsystems being handled (e.g. 3GPP and CDMA), and
  * the registration status value is unique, so if we get subsystem-specific
  * state updates, we'll need to merge all to get a consolidated one. */
-consolidated = get_updated_consolidated_state (self, subsystem, new_state);
+consolidated = get_updated_consolidated_state (self, state, subsystem, 
new_state);
 
 /* Don't update registration-related states while disabling/enabling */
 if (state == MM_MODEM_STATE_ENABLING ||
-- 
1.8.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] iface-modem, iface-modem-simple: log time taken for succeeded operations

2013-01-21 Thread Ben Chan
This patch logs the time taken for succeeded initialization, enabling,
disabling, connection, and disconnection operations, which helps
evaluate how to optimize modem plugins.
---
 libmm-glib/mm-common-helpers.c |7 +++
 libmm-glib/mm-common-helpers.h |2 ++
 src/mm-iface-modem-simple.c|   18 +-
 src/mm-iface-modem.c   |   24 
 4 files changed, 50 insertions(+), 1 deletions(-)

diff --git a/libmm-glib/mm-common-helpers.c b/libmm-glib/mm-common-helpers.c
index 3f100b6..406a99a 100644
--- a/libmm-glib/mm-common-helpers.c
+++ b/libmm-glib/mm-common-helpers.c
@@ -960,3 +960,10 @@ mm_utils_check_for_single_value (guint32 value)
 
 return TRUE;
 }
+
+gdouble
+mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal *start_time)
+{
+return (end_time-tv_sec - start_time-tv_sec) +
+   (end_time-tv_usec - start_time-tv_usec) / 1.0e6;
+}
diff --git a/libmm-glib/mm-common-helpers.h b/libmm-glib/mm-common-helpers.h
index 6155b58..42bbe20 100644
--- a/libmm-glib/mm-common-helpers.h
+++ b/libmm-glib/mm-common-helpers.h
@@ -102,4 +102,6 @@ gchar*mm_utils_bin2hexstr (const guint8 *bin, gsize 
len);
 
 gboolean  mm_utils_check_for_single_value (guint32 value);
 
+gdouble mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal 
*start_time);
+
 #endif /* MM_COMMON_HELPERS_H */
diff --git a/src/mm-iface-modem-simple.c b/src/mm-iface-modem-simple.c
index 12aa152..b0bcf46 100644
--- a/src/mm-iface-modem-simple.c
+++ b/src/mm-iface-modem-simple.c
@@ -196,6 +196,8 @@ typedef struct {
 
 /* Results to set */
 MMBearer *bearer;
+
+GTimeVal start_time;
 } ConnectionContext;
 
 static void
@@ -203,7 +205,6 @@ connection_context_free (ConnectionContext *ctx)
 {
 g_assert (ctx-state_changed_id == 0);
 g_assert (ctx-state_changed_wait_id == 0);
-
 g_variant_unref (ctx-dictionary);
 if (ctx-properties)
 g_object_unref (ctx-properties);
@@ -516,6 +517,8 @@ bearer_list_find_disconnected (MMBearer *bearer,
 static void
 connection_step (ConnectionContext *ctx)
 {
+GTimeVal end_time;
+
 switch (ctx-step) {
 case CONNECTION_STEP_FIRST:
 /* Fall down to next step */
@@ -738,6 +741,9 @@ connection_step (ConnectionContext *ctx)
 case CONNECTION_STEP_LAST:
 mm_info (Simple connect state (%d/%d): All done,
  ctx-step, CONNECTION_STEP_LAST);
+g_get_current_time (end_time);
+mm_info (Connection steps took %.3f second(s).,
+ mm_utils_subtract_time (end_time, ctx-start_time));
 /* All done, yey! */
 mm_gdbus_modem_simple_complete_connect (
 ctx-skeleton,
@@ -828,6 +834,7 @@ handle_connect (MmGdbusModemSimple *skeleton,
 ctx-invocation = g_object_ref (invocation);
 ctx-self = g_object_ref (self);
 ctx-dictionary = g_variant_ref (dictionary);
+g_get_current_time (ctx-start_time);
 
 mm_base_modem_authorize (MM_BASE_MODEM (self),
  invocation,
@@ -846,6 +853,7 @@ typedef struct {
 gchar *bearer_path;
 GList *bearers;
 MMBearer *current;
+GTimeVal start_time;
 } DisconnectionContext;
 
 static void
@@ -887,6 +895,12 @@ disconnect_next_bearer (DisconnectionContext *ctx)
 
 /* No more bearers? all done! */
 if (!ctx-bearers) {
+GTimeVal end_time;
+
+g_get_current_time (end_time);
+mm_info (Disconnection steps took %.3f second(s).,
+ mm_utils_subtract_time (end_time, ctx-start_time));
+
 mm_gdbus_modem_simple_complete_disconnect (ctx-skeleton,
ctx-invocation);
 disconnection_context_free (ctx);
@@ -977,6 +991,8 @@ handle_disconnect (MmGdbusModemSimple *skeleton,
 ctx-bearer_path = g_strdup (ctx-bearer_path);
 }
 
+g_get_current_time (ctx-start_time);
+
 mm_base_modem_authorize (MM_BASE_MODEM (self),
  invocation,
  MM_AUTHORIZATION_DEVICE_CONTROL,
diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
index 6b812e2..2edbcf4 100644
--- a/src/mm-iface-modem.c
+++ b/src/mm-iface-modem.c
@@ -2911,6 +2911,7 @@ struct _DisablingContext {
 MMModemState previous_state;
 GSimpleAsyncResult *result;
 MmGdbusModem *skeleton;
+GTimeVal start_time;
 };
 
 static void
@@ -2935,6 +2936,8 @@ mm_iface_modem_disable_finish (MMIfaceModem *self,
 static void
 interface_disabling_step (DisablingContext *ctx)
 {
+GTimeVal end_time;
+
 switch (ctx-step) {
 case DISABLING_STEP_FIRST:
 /* Fall down to next step */
@@ -2954,6 +2957,10 @@ interface_disabling_step (DisablingContext *ctx)
 ctx-step++;
 
 case DISABLING_STEP_LAST:
+g_get_current_time (end_time);
+mm_info (Disabling steps took %.3f second(s).,
+ mm_utils_subtract_time (end_time, ctx-start_time));
+
 /* We are done without errors! */
 

Re: [MM] [PATCH] iface-modem,iface-modem-simple: log time taken for succeeded operations

2013-01-21 Thread Ben Chan
There may be better ways to collect this kind of performance data than
outputting them to ModemManager log. Is there a commonly used method
for ModemManager to report performance metrics in a way that other tools
can collect and analyze them?

- Ben


On Mon, Jan 21, 2013 at 8:30 PM, Ben Chan benc...@chromium.org wrote:

 This patch logs the time taken for succeeded initialization, enabling,
 disabling, connection, and disconnection operations, which helps
 evaluate how to optimize modem plugins.
 ---
  libmm-glib/mm-common-helpers.c |7 +++
  libmm-glib/mm-common-helpers.h |2 ++
  src/mm-iface-modem-simple.c|   18 +-
  src/mm-iface-modem.c   |   24 
  4 files changed, 50 insertions(+), 1 deletions(-)

 diff --git a/libmm-glib/mm-common-helpers.c
 b/libmm-glib/mm-common-helpers.c
 index 3f100b6..406a99a 100644
 --- a/libmm-glib/mm-common-helpers.c
 +++ b/libmm-glib/mm-common-helpers.c
 @@ -960,3 +960,10 @@ mm_utils_check_for_single_value (guint32 value)

  return TRUE;
  }
 +
 +gdouble
 +mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal
 *start_time)
 +{
 +return (end_time-tv_sec - start_time-tv_sec) +
 +   (end_time-tv_usec - start_time-tv_usec) / 1.0e6;
 +}
 diff --git a/libmm-glib/mm-common-helpers.h
 b/libmm-glib/mm-common-helpers.h
 index 6155b58..42bbe20 100644
 --- a/libmm-glib/mm-common-helpers.h
 +++ b/libmm-glib/mm-common-helpers.h
 @@ -102,4 +102,6 @@ gchar*mm_utils_bin2hexstr (const guint8 *bin,
 gsize len);

  gboolean  mm_utils_check_for_single_value (guint32 value);

 +gdouble mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal
 *start_time);
 +
  #endif /* MM_COMMON_HELPERS_H */
 diff --git a/src/mm-iface-modem-simple.c b/src/mm-iface-modem-simple.c
 index 12aa152..b0bcf46 100644
 --- a/src/mm-iface-modem-simple.c
 +++ b/src/mm-iface-modem-simple.c
 @@ -196,6 +196,8 @@ typedef struct {

  /* Results to set */
  MMBearer *bearer;
 +
 +GTimeVal start_time;
  } ConnectionContext;

  static void
 @@ -203,7 +205,6 @@ connection_context_free (ConnectionContext *ctx)
  {
  g_assert (ctx-state_changed_id == 0);
  g_assert (ctx-state_changed_wait_id == 0);
 -
  g_variant_unref (ctx-dictionary);
  if (ctx-properties)
  g_object_unref (ctx-properties);
 @@ -516,6 +517,8 @@ bearer_list_find_disconnected (MMBearer *bearer,
  static void
  connection_step (ConnectionContext *ctx)
  {
 +GTimeVal end_time;
 +
  switch (ctx-step) {
  case CONNECTION_STEP_FIRST:
  /* Fall down to next step */
 @@ -738,6 +741,9 @@ connection_step (ConnectionContext *ctx)
  case CONNECTION_STEP_LAST:
  mm_info (Simple connect state (%d/%d): All done,
   ctx-step, CONNECTION_STEP_LAST);
 +g_get_current_time (end_time);
 +mm_info (Connection steps took %.3f second(s).,
 + mm_utils_subtract_time (end_time, ctx-start_time));
  /* All done, yey! */
  mm_gdbus_modem_simple_complete_connect (
  ctx-skeleton,
 @@ -828,6 +834,7 @@ handle_connect (MmGdbusModemSimple *skeleton,
  ctx-invocation = g_object_ref (invocation);
  ctx-self = g_object_ref (self);
  ctx-dictionary = g_variant_ref (dictionary);
 +g_get_current_time (ctx-start_time);

  mm_base_modem_authorize (MM_BASE_MODEM (self),
   invocation,
 @@ -846,6 +853,7 @@ typedef struct {
  gchar *bearer_path;
  GList *bearers;
  MMBearer *current;
 +GTimeVal start_time;
  } DisconnectionContext;

  static void
 @@ -887,6 +895,12 @@ disconnect_next_bearer (DisconnectionContext *ctx)

  /* No more bearers? all done! */
  if (!ctx-bearers) {
 +GTimeVal end_time;
 +
 +g_get_current_time (end_time);
 +mm_info (Disconnection steps took %.3f second(s).,
 + mm_utils_subtract_time (end_time, ctx-start_time));
 +
  mm_gdbus_modem_simple_complete_disconnect (ctx-skeleton,
 ctx-invocation);
  disconnection_context_free (ctx);
 @@ -977,6 +991,8 @@ handle_disconnect (MmGdbusModemSimple *skeleton,
  ctx-bearer_path = g_strdup (ctx-bearer_path);
  }

 +g_get_current_time (ctx-start_time);
 +
  mm_base_modem_authorize (MM_BASE_MODEM (self),
   invocation,
   MM_AUTHORIZATION_DEVICE_CONTROL,
 diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
 index 6b812e2..2edbcf4 100644
 --- a/src/mm-iface-modem.c
 +++ b/src/mm-iface-modem.c
 @@ -2911,6 +2911,7 @@ struct _DisablingContext {
  MMModemState previous_state;
  GSimpleAsyncResult *result;
  MmGdbusModem *skeleton;
 +GTimeVal start_time;
  };

  static void
 @@ -2935,6 +2936,8 @@ mm_iface_modem_disable_finish (MMIfaceModem *self,
  static void
  interface_disabling_step (DisablingContext *ctx)
  {
 +GTimeVal end_time

[MM] [PATCH v2] modem: log time taken for succeeded operations

2013-01-21 Thread Ben Chan
This patch logs the time taken for succeeded initialization, enabling,
disabling, connection, and disconnection operations, which helps
evaluate how to optimize modem plugins.
---
 libmm-glib/mm-common-helpers.c |7 +++
 libmm-glib/mm-common-helpers.h |2 ++
 src/mm-broadband-modem.c   |   22 ++
 src/mm-iface-modem-simple.c|   17 +
 src/mm-iface-modem.c   |   24 
 5 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/libmm-glib/mm-common-helpers.c b/libmm-glib/mm-common-helpers.c
index 3f100b6..406a99a 100644
--- a/libmm-glib/mm-common-helpers.c
+++ b/libmm-glib/mm-common-helpers.c
@@ -960,3 +960,10 @@ mm_utils_check_for_single_value (guint32 value)
 
 return TRUE;
 }
+
+gdouble
+mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal *start_time)
+{
+return (end_time-tv_sec - start_time-tv_sec) +
+   (end_time-tv_usec - start_time-tv_usec) / 1.0e6;
+}
diff --git a/libmm-glib/mm-common-helpers.h b/libmm-glib/mm-common-helpers.h
index 6155b58..42bbe20 100644
--- a/libmm-glib/mm-common-helpers.h
+++ b/libmm-glib/mm-common-helpers.h
@@ -102,4 +102,6 @@ gchar*mm_utils_bin2hexstr (const guint8 *bin, gsize 
len);
 
 gboolean  mm_utils_check_for_single_value (guint32 value);
 
+gdouble mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal 
*start_time);
+
 #endif /* MM_COMMON_HELPERS_H */
diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 0a7af2c..135e725 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -7460,6 +7460,7 @@ typedef struct {
 DisablingStep step;
 MMModemState previous_state;
 gboolean disabled;
+GTimeVal start_time;
 } DisablingContext;
 
 static void disabling_step (DisablingContext *ctx);
@@ -7618,6 +7619,8 @@ disabling_wait_for_final_state_ready (MMIfaceModem *self,
 static void
 disabling_step (DisablingContext *ctx)
 {
+GTimeVal end_time;
+
 /* Don't run new steps if we're cancelled */
 if (disabling_context_complete_and_free_if_cancelled (ctx))
 return;
@@ -7746,6 +7749,9 @@ disabling_step (DisablingContext *ctx)
 case DISABLING_STEP_LAST:
 mm_info (Modem fully disabled...);
 ctx-disabled = TRUE;
+g_get_current_time (end_time);
+mm_info (Modem disabling steps took %.3f second(s).,
+ mm_utils_subtract_time (end_time, ctx-start_time));
 /* All disabled without errors! */
 g_simple_async_result_set_op_res_gboolean (G_SIMPLE_ASYNC_RESULT 
(ctx-result), TRUE);
 disabling_context_complete_and_free (ctx);
@@ -7768,6 +7774,7 @@ disable (MMBaseModem *self,
 ctx-result = g_simple_async_result_new (G_OBJECT (self), callback, 
user_data, disable);
 ctx-cancellable = (cancellable ? g_object_ref (cancellable) : NULL);
 ctx-step = DISABLING_STEP_FIRST;
+g_get_current_time (ctx-start_time);
 
 disabling_step (ctx);
 }
@@ -7798,6 +7805,7 @@ typedef struct {
 EnablingStep step;
 MMModemState previous_state;
 gboolean enabled;
+GTimeVal start_time;
 } EnablingContext;
 
 static void enabling_step (EnablingContext *ctx);
@@ -7940,6 +7948,8 @@ enabling_wait_for_final_state_ready (MMIfaceModem *self,
 static void
 enabling_step (EnablingContext *ctx)
 {
+GTimeVal end_time;
+
 /* Don't run new steps if we're cancelled */
 if (enabling_context_complete_and_free_if_cancelled (ctx))
 return;
@@ -8068,6 +8078,9 @@ enabling_step (EnablingContext *ctx)
 case ENABLING_STEP_LAST:
 mm_info (Modem fully enabled...);
 ctx-enabled = TRUE;
+g_get_current_time (end_time);
+mm_info (Modem enabling steps took %.3f second(s).,
+ mm_utils_subtract_time (end_time, ctx-start_time));
 /* All enabled without errors! */
 g_simple_async_result_set_op_res_gboolean (G_SIMPLE_ASYNC_RESULT 
(ctx-result), TRUE);
 enabling_context_complete_and_free (ctx);
@@ -8118,6 +8131,7 @@ enable (MMBaseModem *self,
 ctx-result = result;
 ctx-cancellable = g_object_ref (cancellable);
 ctx-step = ENABLING_STEP_FIRST;
+g_get_current_time (ctx-start_time);
 enabling_step (ctx);
 return;
 }
@@ -8179,6 +8193,7 @@ typedef struct {
 GSimpleAsyncResult *result;
 InitializeStep step;
 gpointer ports_ctx;
+GTimeVal start_time;
 } InitializeContext;
 
 static void initialize_step (InitializeContext *ctx);
@@ -8356,6 +8371,8 @@ INTERFACE_INIT_READY_FN (iface_modem_firmware,  
MM_IFACE_MODEM_FIRMWARE,  FALSE)
 static void
 initialize_step (InitializeContext *ctx)
 {
+GTimeVal end_time;
+
 /* Don't run new steps if we're cancelled */
 if (initialize_context_complete_and_free_if_cancelled (ctx))
 return;
@@ -8523,6 +8540,10 @@ initialize_step (InitializeContext *ctx)
 
 mm_info (Modem fully initialized);
 
+g_get_current_time (end_time);
+mm_info 

Re: [MM] [PATCH] iface-modem,iface-modem-simple: log time taken for succeeded operations

2013-01-21 Thread Ben Chan
Submitted a revised patch.


On Mon, Jan 21, 2013 at 8:39 PM, Ben Chan benc...@chromium.org wrote:

 There may be better ways to collect this kind of performance data than
 outputting them to ModemManager log. Is there a commonly used method
 for ModemManager to report performance metrics in a way that other tools
 can collect and analyze them?

 - Ben


 On Mon, Jan 21, 2013 at 8:30 PM, Ben Chan benc...@chromium.org wrote:

 This patch logs the time taken for succeeded initialization, enabling,
 disabling, connection, and disconnection operations, which helps
 evaluate how to optimize modem plugins.
 ---
  libmm-glib/mm-common-helpers.c |7 +++
  libmm-glib/mm-common-helpers.h |2 ++
  src/mm-iface-modem-simple.c|   18 +-
  src/mm-iface-modem.c   |   24 
  4 files changed, 50 insertions(+), 1 deletions(-)

 diff --git a/libmm-glib/mm-common-helpers.c
 b/libmm-glib/mm-common-helpers.c
 index 3f100b6..406a99a 100644
 --- a/libmm-glib/mm-common-helpers.c
 +++ b/libmm-glib/mm-common-helpers.c
 @@ -960,3 +960,10 @@ mm_utils_check_for_single_value (guint32 value)

  return TRUE;
  }
 +
 +gdouble
 +mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal
 *start_time)
 +{
 +return (end_time-tv_sec - start_time-tv_sec) +
 +   (end_time-tv_usec - start_time-tv_usec) / 1.0e6;
 +}
 diff --git a/libmm-glib/mm-common-helpers.h
 b/libmm-glib/mm-common-helpers.h
 index 6155b58..42bbe20 100644
 --- a/libmm-glib/mm-common-helpers.h
 +++ b/libmm-glib/mm-common-helpers.h
 @@ -102,4 +102,6 @@ gchar*mm_utils_bin2hexstr (const guint8 *bin,
 gsize len);

  gboolean  mm_utils_check_for_single_value (guint32 value);

 +gdouble mm_utils_subtract_time (const GTimeVal *end_time, const GTimeVal
 *start_time);
 +
  #endif /* MM_COMMON_HELPERS_H */
 diff --git a/src/mm-iface-modem-simple.c b/src/mm-iface-modem-simple.c
 index 12aa152..b0bcf46 100644
 --- a/src/mm-iface-modem-simple.c
 +++ b/src/mm-iface-modem-simple.c
 @@ -196,6 +196,8 @@ typedef struct {

  /* Results to set */
  MMBearer *bearer;
 +
 +GTimeVal start_time;
  } ConnectionContext;

  static void
 @@ -203,7 +205,6 @@ connection_context_free (ConnectionContext *ctx)
  {
  g_assert (ctx-state_changed_id == 0);
  g_assert (ctx-state_changed_wait_id == 0);
 -
  g_variant_unref (ctx-dictionary);
  if (ctx-properties)
  g_object_unref (ctx-properties);
 @@ -516,6 +517,8 @@ bearer_list_find_disconnected (MMBearer *bearer,
  static void
  connection_step (ConnectionContext *ctx)
  {
 +GTimeVal end_time;
 +
  switch (ctx-step) {
  case CONNECTION_STEP_FIRST:
  /* Fall down to next step */
 @@ -738,6 +741,9 @@ connection_step (ConnectionContext *ctx)
  case CONNECTION_STEP_LAST:
  mm_info (Simple connect state (%d/%d): All done,
   ctx-step, CONNECTION_STEP_LAST);
 +g_get_current_time (end_time);
 +mm_info (Connection steps took %.3f second(s).,
 + mm_utils_subtract_time (end_time, ctx-start_time));
  /* All done, yey! */
  mm_gdbus_modem_simple_complete_connect (
  ctx-skeleton,
 @@ -828,6 +834,7 @@ handle_connect (MmGdbusModemSimple *skeleton,
  ctx-invocation = g_object_ref (invocation);
  ctx-self = g_object_ref (self);
  ctx-dictionary = g_variant_ref (dictionary);
 +g_get_current_time (ctx-start_time);

  mm_base_modem_authorize (MM_BASE_MODEM (self),
   invocation,
 @@ -846,6 +853,7 @@ typedef struct {
  gchar *bearer_path;
  GList *bearers;
  MMBearer *current;
 +GTimeVal start_time;
  } DisconnectionContext;

  static void
 @@ -887,6 +895,12 @@ disconnect_next_bearer (DisconnectionContext *ctx)

  /* No more bearers? all done! */
  if (!ctx-bearers) {
 +GTimeVal end_time;
 +
 +g_get_current_time (end_time);
 +mm_info (Disconnection steps took %.3f second(s).,
 + mm_utils_subtract_time (end_time, ctx-start_time));
 +
  mm_gdbus_modem_simple_complete_disconnect (ctx-skeleton,
 ctx-invocation);
  disconnection_context_free (ctx);
 @@ -977,6 +991,8 @@ handle_disconnect (MmGdbusModemSimple *skeleton,
  ctx-bearer_path = g_strdup (ctx-bearer_path);
  }

 +g_get_current_time (ctx-start_time);
 +
  mm_base_modem_authorize (MM_BASE_MODEM (self),
   invocation,
   MM_AUTHORIZATION_DEVICE_CONTROL,
 diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
 index 6b812e2..2edbcf4 100644
 --- a/src/mm-iface-modem.c
 +++ b/src/mm-iface-modem.c
 @@ -2911,6 +2911,7 @@ struct _DisablingContext {
  MMModemState previous_state;
  GSimpleAsyncResult *result;
  MmGdbusModem *skeleton;
 +GTimeVal start_time;
  };

  static void
 @@ -2935,6 +2936,8 @@ mm_iface_modem_disable_finish (MMIfaceModem

[MM] [PATCH] iface-modem: schedule signal quality check more often initially

2013-01-17 Thread Ben Chan
This patch modifies MMIfaceModem to schedule the periodic signal quality
check with a period of 3s instead of 30s (up to 5 periods) initially
until a non-zero signal quality value is obtained and then switch back
to the 30s period.
---
 src/mm-iface-modem.c |   36 +---
 1 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
index be4b70d..edb9b7d 100644
--- a/src/mm-iface-modem.c
+++ b/src/mm-iface-modem.c
@@ -27,9 +27,10 @@
 #include mm-log.h
 #include mm-context.h
 
-#define SIGNAL_QUALITY_RECENT_TIMEOUT_SEC 60
-#define SIGNAL_QUALITY_CHECK_TIMEOUT_SEC  30
-#define ACCESS_TECHNOLOGIES_CHECK_TIMEOUT_SEC 30
+#define SIGNAL_QUALITY_RECENT_TIMEOUT_SEC60
+#define SIGNAL_QUALITY_INITIAL_CHECK_TIMEOUT_SEC 3
+#define SIGNAL_QUALITY_CHECK_TIMEOUT_SEC 30
+#define ACCESS_TECHNOLOGIES_CHECK_TIMEOUT_SEC30
 
 #define STATE_UPDATE_CONTEXT_TAG  state-update-context-tag
 #define SIGNAL_QUALITY_UPDATE_CONTEXT_TAG 
signal-quality-update-context-tag
@@ -996,6 +997,8 @@ mm_iface_modem_update_signal_quality (MMIfaceModem *self,
 /*/
 
 typedef struct {
+guint interval;
+guint initial_retries;
 guint timeout_source;
 gboolean running;
 } SignalQualityCheckContext;
@@ -1008,6 +1011,8 @@ signal_quality_check_context_free 
(SignalQualityCheckContext *ctx)
 g_free (ctx);
 }
 
+static gboolean periodic_signal_quality_check (MMIfaceModem *self);
+
 static void
 signal_quality_check_ready (MMIfaceModem *self,
 GAsyncResult *res)
@@ -1029,8 +1034,20 @@ signal_quality_check_ready (MMIfaceModem *self,
  * mm_iface_modem_shutdown when this function is invoked as a callback of
  * load_signal_quality. */
 ctx = g_object_get_qdata (G_OBJECT (self), 
signal_quality_check_context_quark);
-if (ctx)
+if (ctx) {
+if (ctx-interval == SIGNAL_QUALITY_INITIAL_CHECK_TIMEOUT_SEC 
+(signal_quality != 0 || --ctx-initial_retries == 0)) {
+ctx-interval = SIGNAL_QUALITY_CHECK_TIMEOUT_SEC;
+if (ctx-timeout_source) {
+mm_dbg (Periodic signal quality checks rescheduled (interval 
= %ds), ctx-interval);
+g_source_remove(ctx-timeout_source);
+ctx-timeout_source = g_timeout_add_seconds (ctx-interval,
+ 
(GSourceFunc)periodic_signal_quality_check,
+ self);
+}
+}
 ctx-running = FALSE;
+}
 }
 
 static gboolean
@@ -1043,7 +1060,7 @@ periodic_signal_quality_check (MMIfaceModem *self)
 /* Only launch a new one if not one running already OR if the last one run
  * was more than 15s ago. */
 if (!ctx-running ||
-(time (NULL) - get_last_signal_quality_update_time (self)  15)) {
+(time (NULL) - get_last_signal_quality_update_time (self)  
(ctx-interval / 2))) {
 ctx-running = TRUE;
 MM_IFACE_MODEM_GET_INTERFACE (self)-load_signal_quality (
 self,
@@ -1097,9 +1114,14 @@ periodic_signal_quality_check_enable (MMIfaceModem *self)
 }
 
 /* Create context and keep it as object data */
-mm_dbg (Periodic signal quality checks enabled);
 ctx = g_new0 (SignalQualityCheckContext, 1);
-ctx-timeout_source = g_timeout_add_seconds 
(SIGNAL_QUALITY_CHECK_TIMEOUT_SEC,
+/* Schedule the signal quality check using a shorter period, up to 5
+ * periods, initially until a non-zero signal quality value is obtained
+ * and then switch back to the normal period. */
+ctx-interval = SIGNAL_QUALITY_INITIAL_CHECK_TIMEOUT_SEC;
+ctx-initial_retries = 5;
+mm_dbg (Periodic signal quality checks enabled (interval = %ds), 
ctx-interval);
+ctx-timeout_source = g_timeout_add_seconds (ctx-interval,
  
(GSourceFunc)periodic_signal_quality_check,
  self);
 g_object_set_qdata_full (G_OBJECT (self),
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH 1/2] core: add 'hotplugged' flag to indicate if modem is newly plugged in

2013-01-17 Thread Ben Chan
This patch adds a 'hotplugged' flag to MMBaseModem to indicate if a
modem is newly plugged in. A plugin can use this information to
determine if, for example, the modem needs to be soft reset using the
ATZ command.

Dan Williams d...@redhat.com contributed the idea of implementation.
---
 src/mm-base-modem.c |   18 ++
 src/mm-base-modem.h |4 
 src/mm-device.c |   21 +
 src/mm-device.h |4 +++-
 src/mm-manager.c|   15 ---
 src/mm-plugin.c |2 ++
 6 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/src/mm-base-modem.c b/src/mm-base-modem.c
index c66ec53..86ff812 100644
--- a/src/mm-base-modem.c
+++ b/src/mm-base-modem.c
@@ -67,6 +67,7 @@ struct _MMBaseModemPrivate {
 guint vendor_id;
 guint product_id;
 
+gboolean hotplugged;
 gboolean valid;
 
 guint max_timeouts;
@@ -411,6 +412,23 @@ mm_base_modem_initialize (MMBaseModem *self,
 }
 
 void
+mm_base_modem_set_hotplugged (MMBaseModem *self,
+  gboolean hotplugged)
+{
+g_return_if_fail (MM_IS_BASE_MODEM (self));
+
+self-priv-hotplugged = hotplugged;
+}
+
+gboolean
+mm_base_modem_get_hotplugged (MMBaseModem *self)
+{
+g_return_val_if_fail (MM_IS_BASE_MODEM (self), FALSE);
+
+return self-priv-hotplugged;
+}
+
+void
 mm_base_modem_set_valid (MMBaseModem *self,
  gboolean new_valid)
 {
diff --git a/src/mm-base-modem.h b/src/mm-base-modem.h
index bdacf86..9d4e3a1 100644
--- a/src/mm-base-modem.h
+++ b/src/mm-base-modem.h
@@ -143,6 +143,10 @@ MMAtSerialPort   *mm_base_modem_get_best_at_port  
(MMBaseModem *self, GError
 MMPort   *mm_base_modem_get_best_data_port(MMBaseModem *self);
 GList*mm_base_modem_get_data_ports(MMBaseModem *self);
 
+void mm_base_modem_set_hotplugged (MMBaseModem *self,
+   gboolean hotplugged);
+gboolean mm_base_modem_get_hotplugged (MMBaseModem *self);
+
 void mm_base_modem_set_valid(MMBaseModem *self,
  gboolean valid);
 gboolean mm_base_modem_get_valid(MMBaseModem *self);
diff --git a/src/mm-device.c b/src/mm-device.c
index e8330d6..a37b26d 100644
--- a/src/mm-device.c
+++ b/src/mm-device.c
@@ -68,6 +68,9 @@ struct _MMDevicePrivate {
 
 /* When exported, a reference to the object manager */
 GDBusObjectManagerServer *object_manager;
+
+/* Whether the device was hot-plugged. */
+gboolean hotplugged;
 };
 
 /*/
@@ -593,14 +596,24 @@ mm_device_get_port_probe_list (MMDevice *self)
 return copy;
 }
 
+gboolean
+mm_device_get_hotplugged (MMDevice *self)
+{
+return self-priv-hotplugged;
+}
+
 /*/
 
 MMDevice *
-mm_device_new (GUdevDevice *udev_device)
+mm_device_new (GUdevDevice *udev_device, gboolean hotplugged)
 {
-return MM_DEVICE (g_object_new (MM_TYPE_DEVICE,
-MM_DEVICE_UDEV_DEVICE, udev_device,
-NULL));
+MMDevice *device;
+
+device = MM_DEVICE (g_object_new (MM_TYPE_DEVICE,
+  MM_DEVICE_UDEV_DEVICE, udev_device,
+  NULL));
+device-priv-hotplugged = hotplugged;
+return device;
 }
 
 static void
diff --git a/src/mm-device.h b/src/mm-device.h
index 4849905..e7065ed 100644
--- a/src/mm-device.h
+++ b/src/mm-device.h
@@ -58,7 +58,7 @@ struct _MMDeviceClass {
 
 GType mm_device_get_type (void);
 
-MMDevice *mm_device_new (GUdevDevice *udev_device);
+MMDevice *mm_device_new (GUdevDevice *udev_device, gboolean hotplugged);
 
 void mm_device_grab_port(MMDevice*self,
  GUdevDevice *udev_port);
@@ -96,4 +96,6 @@ GList   *mm_device_get_port_probe_list  (MMDevice *self);
 
 const gchar *mm_device_utils_get_port_driver (GUdevDevice *udev_port);
 
+gbooleanmm_device_get_hotplugged (MMDevice *self);
+
 #endif /* MM_DEVICE_H */
diff --git a/src/mm-manager.c b/src/mm-manager.c
index c5185b3..688348f 100644
--- a/src/mm-manager.c
+++ b/src/mm-manager.c
@@ -216,7 +216,8 @@ find_physical_device (GUdevDevice *child)
 
 static void
 device_added (MMManager *manager,
-  GUdevDevice *port)
+  GUdevDevice *port,
+  gboolean hotplugged)
 {
 MMDevice *device;
 const char *subsys, *name, *physdev_path, *physdev_subsys;
@@ -292,7 +293,7 @@ device_added (MMManager *manager,
 FindDeviceSupportContext *ctx;
 
 /* Keep the device listed in the Manager */
-device = mm_device_new (physdev);
+device = mm_device_new (physdev, hotplugged);
 g_hash_table_insert (manager-priv-devices,
  g_strdup (physdev_path),
  device);
@@ -394,7 +395,7 @@ handle_uevent 

[MM] [PATCH 2/2] novatel-lte: skip soft reset if modem is newly plugged in

2013-01-17 Thread Ben Chan
Soft resetting a Novatel LTE modem can a significant amount of time
(3-4 seconds). If the modem is newly plugged in, skip the unnecessary
soft reset when enabling the modem.
---
 plugins/novatel/mm-broadband-modem-novatel-lte.c |  108 ++
 1 files changed, 108 insertions(+), 0 deletions(-)

diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c 
b/plugins/novatel/mm-broadband-modem-novatel-lte.c
index 08954b5..d32c397 100644
--- a/plugins/novatel/mm-broadband-modem-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c
@@ -43,6 +43,112 @@ G_DEFINE_TYPE_EXTENDED (MMBroadbandModemNovatelLte, 
mm_broadband_modem_novatel_l
 G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_3GPP, 
iface_modem_3gpp_init));
 
 /*/
+/* Initializing the modem (Modem interface) */
+
+static gboolean
+modem_init_finish (MMIfaceModem *self,
+   GAsyncResult *res,
+   GError **error)
+{
+return !g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT 
(res), error);
+}
+
+static void
+modem_init_sequence_ready (MMBaseModem *self,
+   GAsyncResult *res,
+   GSimpleAsyncResult *simple)
+{
+GError *error = NULL;
+
+mm_base_modem_at_sequence_full_finish (MM_BASE_MODEM (self), res, NULL, 
error);
+if (error)
+g_simple_async_result_take_error (simple, error);
+else {
+MMAtSerialPort *secondary;
+
+/* Disable echo in secondary port as well, if any */
+secondary = mm_base_modem_peek_port_secondary (MM_BASE_MODEM (self));
+if (secondary)
+/* No need to wait for the reply */
+mm_base_modem_at_command_full (MM_BASE_MODEM (self),
+   secondary,
+   E0,
+   3,
+   FALSE,
+   FALSE, /* raw */
+   NULL, /* cancellable */
+   NULL,
+   NULL);
+
+g_simple_async_result_set_op_res_gboolean (simple, TRUE);
+}
+
+g_simple_async_result_complete (simple);
+g_object_unref (simple);
+}
+
+static const MMBaseModemAtCommand modem_init_sequence[] = {
+/* Init command. ITU rec v.250 (6.1.1) says:
+ *   The DTE should not include additional commands on the same command 
line
+ *   after the Z command because such commands may be ignored.
+ * So run ATZ alone.
+ */
+{ Z,   6, FALSE, mm_base_modem_response_processor_no_result_continue 
},
+
+/* Ensure echo is off after the init command */
+{ E0 V1,   3, FALSE, NULL },
+
+/* Some phones (like Blackberries) don't support +CMEE=1, so make it
+ * optional.  It completely violates 3GPP TS 27.007 (9.1) but what can we 
do...
+ */
+{ +CMEE=1, 3, FALSE, NULL },
+
+/* Additional OPTIONAL initialization */
+{ X4 C1,  3, FALSE, NULL },
+
+{ NULL }
+};
+
+static void
+modem_init (MMIfaceModem *self,
+GAsyncReadyCallback callback,
+gpointer user_data)
+{
+MMAtSerialPort *primary;
+GSimpleAsyncResult *result;
+guint init_sequence_index;
+
+result = g_simple_async_result_new (G_OBJECT (self),
+callback,
+user_data,
+modem_init);
+
+primary = mm_base_modem_peek_port_primary (MM_BASE_MODEM (self));
+if (!primary) {
+g_simple_async_result_set_error (
+result,
+MM_CORE_ERROR,
+MM_CORE_ERROR_FAILED,
+Need primary AT port to run modem init sequence);
+g_simple_async_result_complete_in_idle (result);
+g_object_unref (result);
+return;
+}
+
+/* Skip ATZ if the device was hotplugged. */
+init_sequence_index = mm_base_modem_get_hotplugged (MM_BASE_MODEM (self)) 
? 1 : 0;
+
+mm_base_modem_at_sequence_full (MM_BASE_MODEM (self),
+primary,
+modem_init_sequence[init_sequence_index],
+NULL,  /* response_processor_context */
+NULL,  /* response_processor_context_free 
*/
+NULL, /* cancellable */
+
(GAsyncReadyCallback)modem_init_sequence_ready,
+result);
+}
+
+/*/
 /* Create Bearer (Modem interface) */
 
 static MMBearer *
@@ -637,6 +743,8 @@ mm_broadband_modem_novatel_lte_init 
(MMBroadbandModemNovatelLte *self)
 static void
 iface_modem_init (MMIfaceModem *iface)
 {
+

Re: [MM/QMI] Unable to select firmware when there is no SIM

2013-01-07 Thread Ben Chan
Do you have any preference/suggestion on how we should approach this
problem? I guess we may need to add additional checks in the code where it
currently assumes a modem object in a fully initialized state.

- Ben


On Sun, Jan 6, 2013 at 3:10 AM, Aleksander Morgado aleksan...@lanedo.comwrote:

 Hey hey Ben,

 
  I encounter an issue with firmware selection on my Novatel Gobi 3000
  (E396) modem. When the UMTS firmware is initially selected and there is
  no SIM card inserted, ModemManager can't fully initialize the modem and
  firmware interface, which prevents me from selecting the CDMA firmware.
 
  Is it possible to continue the initialization of the modem and firmware
  interface when the SIM check fails?
 


 I guess we can allow that, yes. The modem itself would be in 'failed'
 state when there is no SIM though; the only thing we have to do is to
 let the Firmware interface get initialized even if we're in 'failed' state.

 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM/QMI] Unable to select firmware when there is no SIM

2013-01-07 Thread Ben Chan
Thanks for the patch and it works.

- Ben


On Mon, Jan 7, 2013 at 1:26 PM, Aleksander Morgado aleksan...@lanedo.comwrote:

 On 07/01/13 19:53, Ben Chan wrote:
  Do you have any preference/suggestion on how we should approach this
  problem? I guess we may need to add additional checks in the code where
  it currently assumes a modem object in a fully initialized state.
 

 I don't think there should be many Firmware-related code (if any) where
 we require a specific modem state. The attached patch (untested!!)
 should allow launching the Firmware interface when the modem is in
 Failed state. It should at least give some hints of where to start.

 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM/QMI] Unable to select firmware when there is no SIM

2013-01-07 Thread Ben Chan
The patch addresses the firmware selection issue I mentioned. I think it
can be pushed as it is. Thanks!


On Mon, Jan 7, 2013 at 10:55 PM, Aleksander Morgado
aleksan...@lanedo.comwrote:

 On 07/01/13 23:34, Ben Chan wrote:
  Thanks for the patch and it works.
 

 Oh; cool. Nothing else needed then?

 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM/QMI] Unable to select firmware when there is no SIM

2013-01-04 Thread Ben Chan
Hi Aleksander,

I encounter an issue with firmware selection on my Novatel Gobi 3000 (E396)
modem. When the UMTS firmware is initially selected and there is no SIM
card inserted, ModemManager can't fully initialize the modem and firmware
interface, which prevents me from selecting the CDMA firmware.

Is it possible to continue the initialization of the modem and firmware
interface when the SIM check fails?

Thanks,
Ben
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] iface-modem-3gpp, iface-modem-cdma: check for deallocated RegistrationCheckContext

2013-01-03 Thread Ben Chan
This patch fixes a crash in periodic_registration_checks_ready() due to
access of an already deallocated RegistrationCheckContext.

Thread 0 *CRASHED* ( SIGSEGV @ 0x )

0x7fc344d355cd   [ModemManager] - mm-iface-modem-cdma.c:1112   
periodic_registration_checks_ready
0x7fc3449ea266   [libgio-2.0.so.0.3200.4]   - gsimpleasyncresult.c:767 
g_simple_async_result_complete
0x7fc3449ea368   [libgio-2.0.so.0.3200.4]   - gsimpleasyncresult.c:779 
complete_in_idle_cb
0x7fc344851dc4   [libglib-2.0.so.0.3200.4]  - gmain.c:2539 
g_main_context_dispatch
0x7fc344852147   [libglib-2.0.so.0.3200.4]  - gmain.c:3146 
g_main_context_iterate
0x7fc3448525a1   [libglib-2.0.so.0.3200.4]  - gmain.c:3340 
g_main_loop_run
0x7fc344d0f154   [ModemManager] - main.c:158   main
0x7fc34426a474   [libc-2.15.so] - libc-start.c:234 
__libc_start_main
0x7fc344d0eb68   [ModemManager] + 0x0001bb68
---
 src/mm-iface-modem-3gpp.c |3 ++-
 src/mm-iface-modem-cdma.c |3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
index 1e0cd30..7a88b2f 100644
--- a/src/mm-iface-modem-3gpp.c
+++ b/src/mm-iface-modem-3gpp.c
@@ -1147,7 +1147,8 @@ periodic_registration_checks_ready (MMIfaceModem3gpp 
*self,
 
 /* Remove the running tag */
 ctx = g_object_get_qdata (G_OBJECT (self), 
registration_check_context_quark);
-ctx-running = FALSE;
+if (ctx)
+ctx-running = FALSE;
 }
 
 static gboolean
diff --git a/src/mm-iface-modem-cdma.c b/src/mm-iface-modem-cdma.c
index 8478df5..d582fb3 100644
--- a/src/mm-iface-modem-cdma.c
+++ b/src/mm-iface-modem-cdma.c
@@ -1109,7 +1109,8 @@ periodic_registration_checks_ready (MMIfaceModemCdma 
*self,
 
 /* Remove the running tag */
 ctx = g_object_get_qdata (G_OBJECT (self), 
registration_check_context_quark);
-ctx-running = FALSE;
+if (ctx)
+ctx-running = FALSE;
 }
 
 static gboolean
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] iface-modem, novatel-lte: disable network scan in LTE mode

2013-01-03 Thread Ben Chan
In LTE mode, AT+COPS=? simply replies OK (confirmed with the modem vendor)

- Ben

On Thu, Jan 3, 2013 at 7:46 AM, Aleksander Morgado aleksan...@lanedo.comwrote:

 AT+COPS=?
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] iface-modem, novatel-lte: disable network scan in LTE mode

2013-01-03 Thread Ben Chan
Also, based on my experiments, the modem sometimes seems to hang when
running AT+COPS=?. This patch avoids AT+COPS=? from being issued when the
modem is in LTE mdoe.


On Thu, Jan 3, 2013 at 11:10 AM, Ben Chan benc...@chromium.org wrote:

 In LTE mode, AT+COPS=? simply replies OK (confirmed with the modem
 vendor)

 - Ben

 On Thu, Jan 3, 2013 at 7:46 AM, Aleksander Morgado 
 aleksan...@lanedo.comwrote:

 AT+COPS=?



___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] iface-modem, novatel-lte: disable network scan in LTE mode

2013-01-03 Thread Ben Chan
On Thu, Jan 3, 2013 at 1:17 PM, Aleksander Morgado aleksan...@lanedo.comwrote:

 On 03/01/13 20:13, Ben Chan wrote:
  Also, based on my experiments, the modem sometimes seems to hang when
  running AT+COPS=?. This patch avoids AT+COPS=? from being issued when
  the modem is in LTE mdoe.

 From what I can see, it also avoids the Scan when the access tech is
 unknown:

 +if ((access_tech == MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN) ||
 +(access_tech  MM_MODEM_ACCESS_TECHNOLOGY_LTE)) {

 So you're really limiting the scanning for networks only to when the
 modem is already connected to a given network which is not LTE. I don't
 think we shouldn't ban from scanning when access tech is UNKNOWN.


I include UNKNOWN just to be safe as there may be a period of time when the
access technology is not yet determined.

Also, the reply to AT+COPS=? may really take a loong time; you sure it
 gets stuck?


From what I've seen before, the scan operation timed out and the modem
didn't respond to AT commands anymore. However, that's not frequently
reproducible.


 I wonder how the scan works when the Novatel LTE modem is handled by the
 QMI driver. IIRC Dan said we wouldn't need this specific plugin any more
 once the modem is handled by the QMI implementation.


I haven't tried the QMI driver for this modem, but wonder if it behaves in
a similar way as the limitation is most likely a firmware issue.


 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] iface-modem, novatel-lte: disable network scan in LTE mode

2013-01-03 Thread Ben Chan
On Thu, Jan 3, 2013 at 1:37 PM, Aleksander Morgado aleksan...@lanedo.comwrote:


   Also, based on my experiments, the modem sometimes seems to hang
 when
   running AT+COPS=?. This patch avoids AT+COPS=? from being issued
 when
   the modem is in LTE mdoe.
 
  From what I can see, it also avoids the Scan when the access tech is
  unknown:
 
  +if ((access_tech == MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN) ||
  +(access_tech  MM_MODEM_ACCESS_TECHNOLOGY_LTE)) {
 
  So you're really limiting the scanning for networks only to when the
  modem is already connected to a given network which is not LTE. I
 don't
  think we shouldn't ban from scanning when access tech is UNKNOWN.
 
 
  I include UNKNOWN just to be safe as there may be a period of time when
  the access technology is not yet determined.
 

 That's what I mean; it should be perfectly valid to be able to scan for
 networks when the modem is not yet registered anywhere, i.e. when the
 access tech is unknown.


Let me remove the check for unknown access technology.


   Also, the reply to AT+COPS=? may really take a loong time; you sure
 it
  gets stuck?
 
 
  From what I've seen before, the scan operation timed out and the modem
  didn't respond to AT commands anymore. However, that's not frequently
  reproducible.
 

 So this patch is just avoiding the need to parse an empty list as the
 AT+COPS=? command replies just OK by default when in LTE mode. Is it
 really worth the complication of subclassing the scan method just to
 avoid that? :-/


The only purpose of this patch is to skip AT+COPS=? in LTE mode as that
causes the modem to sometimes behave weirdly :-/



 
  I wonder how the scan works when the Novatel LTE modem is handled by
 the
  QMI driver. IIRC Dan said we wouldn't need this specific plugin any
 more
  once the modem is handled by the QMI implementation.
 
 
  I haven't tried the QMI driver for this modem, but wonder if it behaves
  in a similar way as the limitation is most likely a firmware issue.
 

 It would be great if you could check the overall behaviour of the
 Novatel LTE modem with the QMI driver, to see if there is any reason not
 to switch to it...


Will do


  --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH v2] iface-modem, novatel-lte: disable network scan in LTE mode

2013-01-03 Thread Ben Chan
---
 plugins/novatel/mm-broadband-modem-novatel-lte.c |   72 +-
 src/mm-iface-modem.c |   20 ++
 src/mm-iface-modem.h |3 +
 3 files changed, 94 insertions(+), 1 deletions(-)

diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c 
b/plugins/novatel/mm-broadband-modem-novatel-lte.c
index 249a4a1..d667182 100644
--- a/plugins/novatel/mm-broadband-modem-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c
@@ -29,15 +29,18 @@
 #include mm-sim-novatel-lte.h
 #include mm-errors-types.h
 #include mm-iface-modem.h
+#include mm-iface-modem-3gpp.h
 #include mm-iface-modem-messaging.h
 #include mm-log.h
 #include mm-modem-helpers.h
 #include mm-serial-parsers.h
 
 static void iface_modem_init (MMIfaceModem *iface);
+static void iface_modem_3gpp_init (MMIfaceModem3gpp *iface);
 
 G_DEFINE_TYPE_EXTENDED (MMBroadbandModemNovatelLte, 
mm_broadband_modem_novatel_lte, MM_TYPE_BROADBAND_MODEM, 0,
-G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM, 
iface_modem_init));
+G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM, 
iface_modem_init)
+G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_3GPP, 
iface_modem_3gpp_init));
 
 /*/
 /* Create Bearer (Modem interface) */
@@ -519,6 +522,66 @@ reset (MMIfaceModem *self,
 }
 
 /*/
+/* Scan networks (3GPP interface) */
+
+static GList *
+scan_networks_finish (MMIfaceModem3gpp *self,
+  GAsyncResult *res,
+  GError **error)
+{
+const gchar *result;
+
+result = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, 
error);
+if (!result)
+return NULL;
+
+return mm_3gpp_parse_cops_test_response (result, error);
+}
+
+static void
+scan_networks (MMIfaceModem3gpp *self,
+   GAsyncReadyCallback callback,
+   gpointer user_data)
+{
+MMModemAccessTechnology access_tech;
+
+mm_dbg (scanning for networks (Novatel LTE)...);
+
+access_tech = mm_iface_modem_get_access_technologies (MM_IFACE_MODEM 
(self));
+/* The Novatel LTE modem does not properly support AT+COPS=? in LTE mode.
+ * Thus, do not try to scan networks when the current access technologies
+ * include LTE.
+ */
+if (access_tech  MM_MODEM_ACCESS_TECHNOLOGY_LTE) {
+GSimpleAsyncResult *simple;
+gchar *access_tech_string;
+
+access_tech_string = mm_modem_access_technology_build_string_from_mask 
(access_tech);
+mm_warn (Couldn't scan for networks with access technologies: %s, 
access_tech_string);
+simple = g_simple_async_result_new (G_OBJECT (self),
+callback,
+user_data,
+scan_networks);
+g_simple_async_result_set_error (simple,
+ MM_CORE_ERROR,
+ MM_CORE_ERROR_UNSUPPORTED,
+ Couldn't scan for networks with 
access technologies: %s,
+ access_tech_string);
+g_simple_async_result_complete_in_idle (simple);
+g_object_unref (simple);
+g_free (access_tech_string);
+return;
+}
+
+mm_base_modem_at_command (MM_BASE_MODEM (self),
+  +COPS=?,
+  120,
+  FALSE,
+  callback,
+  user_data);
+}
+
+/*/
 
 MMBroadbandModemNovatelLte *
 mm_broadband_modem_novatel_lte_new (const gchar *device,
@@ -564,6 +627,13 @@ iface_modem_init (MMIfaceModem *iface)
 }
 
 static void
+iface_modem_3gpp_init (MMIfaceModem3gpp *iface)
+{
+iface-scan_networks = scan_networks;
+iface-scan_networks_finish = scan_networks_finish;
+}
+
+static void
 mm_broadband_modem_novatel_lte_class_init (MMBroadbandModemNovatelLteClass 
*klass)
 {
 }
diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
index d25b03d..e94bc42 100644
--- a/src/mm-iface-modem.c
+++ b/src/mm-iface-modem.c
@@ -3930,6 +3930,26 @@ mm_iface_modem_shutdown (MMIfaceModem *self)
 
 /*/
 
+MMModemAccessTechnology
+mm_iface_modem_get_access_technologies (MMIfaceModem *self)
+{
+MMModemAccessTechnology access_tech = MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN;
+MmGdbusModem *skeleton;
+
+g_object_get (self,
+  MM_IFACE_MODEM_DBUS_SKELETON, skeleton,
+  NULL);
+
+if (skeleton) {
+access_tech = mm_gdbus_modem_get_access_technologies (skeleton);
+g_object_unref 

[MM] [PATCH v3] iface-modem, novatel-lte: disable network scan in LTE mode

2013-01-03 Thread Ben Chan
---
 plugins/novatel/mm-broadband-modem-novatel-lte.c |  102 +-
 src/mm-iface-modem.c |   20 
 src/mm-iface-modem.h |3 +
 3 files changed, 124 insertions(+), 1 deletions(-)

diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c 
b/plugins/novatel/mm-broadband-modem-novatel-lte.c
index 249a4a1..08954b5 100644
--- a/plugins/novatel/mm-broadband-modem-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c
@@ -29,15 +29,18 @@
 #include mm-sim-novatel-lte.h
 #include mm-errors-types.h
 #include mm-iface-modem.h
+#include mm-iface-modem-3gpp.h
 #include mm-iface-modem-messaging.h
 #include mm-log.h
 #include mm-modem-helpers.h
 #include mm-serial-parsers.h
 
 static void iface_modem_init (MMIfaceModem *iface);
+static void iface_modem_3gpp_init (MMIfaceModem3gpp *iface);
 
 G_DEFINE_TYPE_EXTENDED (MMBroadbandModemNovatelLte, 
mm_broadband_modem_novatel_lte, MM_TYPE_BROADBAND_MODEM, 0,
-G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM, 
iface_modem_init));
+G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM, 
iface_modem_init)
+G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_3GPP, 
iface_modem_3gpp_init));
 
 /*/
 /* Create Bearer (Modem interface) */
@@ -519,6 +522,96 @@ reset (MMIfaceModem *self,
 }
 
 /*/
+/* Scan networks (3GPP interface) */
+
+static GList *
+scan_networks_finish (MMIfaceModem3gpp *self,
+  GAsyncResult *res,
+  GError **error)
+{
+if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), 
error))
+return NULL;
+
+return g_simple_async_result_get_op_res_gpointer (G_SIMPLE_ASYNC_RESULT 
(res));
+}
+
+static void
+cops_query_ready (MMBroadbandModemNovatelLte *self,
+  GAsyncResult *res,
+  GSimpleAsyncResult *operation_result)
+{
+const gchar *response;
+GError *error = NULL;
+GList *scan_result;
+
+response = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, 
error);
+if (error) {
+g_simple_async_result_take_error (operation_result, error);
+g_simple_async_result_complete (operation_result);
+g_object_unref (operation_result);
+return;
+}
+
+scan_result = mm_3gpp_parse_cops_test_response (response, error);
+if (error) {
+g_simple_async_result_take_error (operation_result, error);
+g_simple_async_result_complete (operation_result);
+g_object_unref (operation_result);
+return;
+}
+
+g_simple_async_result_set_op_res_gpointer (operation_result,
+   scan_result,
+   NULL);
+g_simple_async_result_complete (operation_result);
+g_object_unref (operation_result);
+}
+
+static void
+scan_networks (MMIfaceModem3gpp *self,
+   GAsyncReadyCallback callback,
+   gpointer user_data)
+{
+GSimpleAsyncResult *result;
+MMModemAccessTechnology access_tech;
+
+mm_dbg (scanning for networks (Novatel LTE)...);
+
+result = g_simple_async_result_new (G_OBJECT (self),
+callback,
+user_data,
+scan_networks);
+
+access_tech = mm_iface_modem_get_access_technologies (MM_IFACE_MODEM 
(self));
+/* The Novatel LTE modem does not properly support AT+COPS=? in LTE mode.
+ * Thus, do not try to scan networks when the current access technologies
+ * include LTE.
+ */
+if (access_tech  MM_MODEM_ACCESS_TECHNOLOGY_LTE) {
+gchar *access_tech_string;
+
+access_tech_string = mm_modem_access_technology_build_string_from_mask 
(access_tech);
+mm_warn (Couldn't scan for networks with access technologies: %s, 
access_tech_string);
+g_simple_async_result_set_error (result,
+ MM_CORE_ERROR,
+ MM_CORE_ERROR_UNSUPPORTED,
+ Couldn't scan for networks with 
access technologies: %s,
+ access_tech_string);
+g_simple_async_result_complete_in_idle (result);
+g_object_unref (result);
+g_free (access_tech_string);
+return;
+}
+
+mm_base_modem_at_command (MM_BASE_MODEM (self),
+  +COPS=?,
+  120,
+  FALSE,
+  (GAsyncReadyCallback)cops_query_ready,
+  result);
+}
+
+/*/
 
 MMBroadbandModemNovatelLte *
 

Re: [MM] [PATCH v2] iface-modem, novatel-lte: disable network scan in LTE mode

2013-01-03 Thread Ben Chan
Thanks for the notes.  Submitted patch v3.

- Ben


On Thu, Jan 3, 2013 at 3:58 PM, Aleksander Morgado aleksan...@lanedo.comwrote:


  +static GList *
  +scan_networks_finish (MMIfaceModem3gpp *self,
  +  GAsyncResult *res,
  +  GError **error)
  +{
  +const gchar *result;
  +
  +result = mm_base_modem_at_command_finish (MM_BASE_MODEM (self),
 res, error);
  +if (!result)
  +return NULL;
  +
  +return mm_3gpp_parse_cops_test_response (result, error);
  +}
  +
  +static void
  +scan_networks (MMIfaceModem3gpp *self,
  +   GAsyncReadyCallback callback,
  +   gpointer user_data)
  +{
  +MMModemAccessTechnology access_tech;
  +
  +mm_dbg (scanning for networks (Novatel LTE)...);
  +
  +access_tech = mm_iface_modem_get_access_technologies
 (MM_IFACE_MODEM (self));
  +/* The Novatel LTE modem does not properly support AT+COPS=? in LTE
 mode.
  + * Thus, do not try to scan networks when the current access
 technologies
  + * include LTE.
  + */
  +if (access_tech  MM_MODEM_ACCESS_TECHNOLOGY_LTE) {
  +GSimpleAsyncResult *simple;
  +gchar *access_tech_string;
  +
  +access_tech_string =
 mm_modem_access_technology_build_string_from_mask (access_tech);
  +mm_warn (Couldn't scan for networks with access technologies:
 %s, access_tech_string);
  +simple = g_simple_async_result_new (G_OBJECT (self),
  +callback,
  +user_data,
  +scan_networks);
  +g_simple_async_result_set_error (simple,
  + MM_CORE_ERROR,
  + MM_CORE_ERROR_UNSUPPORTED,
  + Couldn't scan for networks
 with access technologies: %s,
  + access_tech_string);
  +g_simple_async_result_complete_in_idle (simple);
  +g_object_unref (simple);
  +g_free (access_tech_string);
  +return;
  +}
  +
  +mm_base_modem_at_command (MM_BASE_MODEM (self),
  +  +COPS=?,
  +  120,
  +  FALSE,
  +  callback,
  +  user_data);
  +}

 You cannot mix in the same async method a code execution path using
 mm_base_modem_at_command() with another one using GSimpleAsyncResult and
 completion in idle.  When you use mm_base_modem_at_command(), in
 finish() you're expected to use mm_base_modem_at_command_finish(), like
 you did. But when you use GSimpleAsyncResult and completion in idle you
 should use g_simple_async_result_propagate_error() in finish(). The fact
 that it may work as expected is due to how at_command_finish() is
 implemented; but you shouldn't rely on that.

 So you'll need to use GSimpleAsyncResult for both cases, i.e. provide a
 _ready() GAsyncReadyCallback in mm_base_modem_at_command() to which you
 pass 'simple' as user_data; and then complete the 'simple' from within
 the _ready() method. In this way, you can safely call
 g_simple_async_result_propagate_error() in finish().

 See for example:

 http://cgit.freedesktop.org/ModemManager/ModemManager/tree/plugins/simtech/mm-broadband-modem-simtech.c#n416

 And btw, a hint for when you just always need to report an error in idle
 in the async method (avoids the need to create a GSimpleAsyncResult
 yourself):

 http://developer.gnome.org/gio/unstable/GSimpleAsyncResult.html#g-simple-async-report-error-in-idle

 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] iface-modem, novatel-lte: disable network scan in LTE mode

2013-01-02 Thread Ben Chan
---
 plugins/novatel/mm-broadband-modem-novatel-lte.c |   73 +-
 src/mm-iface-modem.c |   20 ++
 src/mm-iface-modem.h |3 +
 3 files changed, 95 insertions(+), 1 deletions(-)

diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c 
b/plugins/novatel/mm-broadband-modem-novatel-lte.c
index 249a4a1..5b1a63b 100644
--- a/plugins/novatel/mm-broadband-modem-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c
@@ -29,15 +29,18 @@
 #include mm-sim-novatel-lte.h
 #include mm-errors-types.h
 #include mm-iface-modem.h
+#include mm-iface-modem-3gpp.h
 #include mm-iface-modem-messaging.h
 #include mm-log.h
 #include mm-modem-helpers.h
 #include mm-serial-parsers.h
 
 static void iface_modem_init (MMIfaceModem *iface);
+static void iface_modem_3gpp_init (MMIfaceModem3gpp *iface);
 
 G_DEFINE_TYPE_EXTENDED (MMBroadbandModemNovatelLte, 
mm_broadband_modem_novatel_lte, MM_TYPE_BROADBAND_MODEM, 0,
-G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM, 
iface_modem_init));
+G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM, 
iface_modem_init)
+G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_3GPP, 
iface_modem_3gpp_init));
 
 /*/
 /* Create Bearer (Modem interface) */
@@ -519,6 +522,67 @@ reset (MMIfaceModem *self,
 }
 
 /*/
+/* Scan networks (3GPP interface) */
+
+static GList *
+scan_networks_finish (MMIfaceModem3gpp *self,
+  GAsyncResult *res,
+  GError **error)
+{
+const gchar *result;
+
+result = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, 
error);
+if (!result)
+return NULL;
+
+return mm_3gpp_parse_cops_test_response (result, error);
+}
+
+static void
+scan_networks (MMIfaceModem3gpp *self,
+   GAsyncReadyCallback callback,
+   gpointer user_data)
+{
+MMModemAccessTechnology access_tech;
+
+mm_dbg (scanning for networks (Novatel LTE)...);
+
+access_tech = mm_iface_modem_get_access_technologies (MM_IFACE_MODEM 
(self));
+/* The Novatel LTE modem does not properly support AT+COPS=? in LTE mode.
+ * Thus, do not try to scan networks when the current access technologies
+ * include LTE or cannot be determined.
+ */
+if ((access_tech == MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN) ||
+(access_tech  MM_MODEM_ACCESS_TECHNOLOGY_LTE)) {
+GSimpleAsyncResult *simple;
+gchar *access_tech_string;
+
+access_tech_string = mm_modem_access_technology_build_string_from_mask 
(access_tech);
+mm_warn (Couldn't scan for networks with access technologies: %s, 
access_tech_string);
+simple = g_simple_async_result_new (G_OBJECT (self),
+callback,
+user_data,
+scan_networks);
+g_simple_async_result_set_error (simple,
+ MM_CORE_ERROR,
+ MM_CORE_ERROR_UNSUPPORTED,
+ Couldn't scan for networks with 
access technologies: %s,
+ access_tech_string);
+g_simple_async_result_complete_in_idle (simple);
+g_object_unref (simple);
+g_free (access_tech_string);
+return;
+}
+
+mm_base_modem_at_command (MM_BASE_MODEM (self),
+  +COPS=?,
+  120,
+  FALSE,
+  callback,
+  user_data);
+}
+
+/*/
 
 MMBroadbandModemNovatelLte *
 mm_broadband_modem_novatel_lte_new (const gchar *device,
@@ -564,6 +628,13 @@ iface_modem_init (MMIfaceModem *iface)
 }
 
 static void
+iface_modem_3gpp_init (MMIfaceModem3gpp *iface)
+{
+iface-scan_networks = scan_networks;
+iface-scan_networks_finish = scan_networks_finish;
+}
+
+static void
 mm_broadband_modem_novatel_lte_class_init (MMBroadbandModemNovatelLteClass 
*klass)
 {
 }
diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
index d25b03d..e94bc42 100644
--- a/src/mm-iface-modem.c
+++ b/src/mm-iface-modem.c
@@ -3930,6 +3930,26 @@ mm_iface_modem_shutdown (MMIfaceModem *self)
 
 /*/
 
+MMModemAccessTechnology
+mm_iface_modem_get_access_technologies (MMIfaceModem *self)
+{
+MMModemAccessTechnology access_tech = MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN;
+MmGdbusModem *skeleton;
+
+g_object_get (self,
+  MM_IFACE_MODEM_DBUS_SKELETON, skeleton,
+  NULL);
+
+if (skeleton) {
+

Re: [MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed

2012-12-12 Thread Ben Chan
Thanks for the patches!

Ben

On Wed, Dec 12, 2012 at 3:59 AM, Aleksander Morgado
aleksan...@lanedo.comwrote:


  Aleksander, thanks for the patch.  I've been running suspend/resume test
  with the patch. Seems like those glib warnings (and the crash) are gone
  except for the following:

 So, I pushed now the patch for both the Icera and HSO plugins; see
 commit dc9bbef.

 
  (ModemManager:2858): GLib-GObject-WARNING **: gsignal.c:2576: instance
  `0x78624028' has no handler with id `148'
 

 Also fixed this one, which was popping up when trying to reproduce the
 issue in an Option/HSO modem; see commit f20922b.

 Cheers,

 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed

2012-12-03 Thread Ben Chan
Aleksander, thanks for the patch.  I've been running suspend/resume test
with the patch. Seems like those glib warnings (and the crash) are gone
except for the following:

(ModemManager:2858): GLib-GObject-WARNING **: gsignal.c:2576: instance
`0x78624028' has no handler with id `148'

Thanks,
Ben

On Fri, Nov 30, 2012 at 5:05 AM, Aleksander Morgado
aleksan...@lanedo.comwrote:

 On 30/11/12 13:32, Aleksander Morgado wrote:
   * Whenever we wait for an unsolicited message to keep on processing an
  operation, we also need to consider the case where the port is forced to
  be closed due to being removed. At least *Icera* and *HSO* are affected
  by this issue.

 Ben, are you able to retest with the attached patch applied?

 Cheers,

 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed

2012-11-29 Thread Ben Chan
The crash happens to be a reuse of an already disposed MMSerialPort object.
I'm now tracing the new/dispose paths.

Ben

/* Don't read any input if the current command isn't done being sent
yet */
info = g_queue_peek_nth (priv-queue, 0);
   49f60:   6940ldr r0, [r0, #20]
  - 0x151F60
   49f62:   f7c9 ebf6   blx 13750 _init+0x1254

==20047== Invalid read of size 4
==20047==at 0x151F60: ??? (in /usr/sbin/ModemManager)
==20047==  Address 0x14 is not stack'd, malloc'd or (recently) free'd
==20047==
==20047==
==20047== Process terminating with default action of signal 11 (SIGSEGV)
==20047==  Access not within mapped region at address 0x14
==20047==at 0x151F60: ??? (in /usr/sbin/ModemManager)
==20047==  If you believe this happened as a result of a stack
==20047==  overflow in your program's main thread (unlikely but
==20047==  possible), you can try to increase the size of the
==20047==  main thread stack using the --main-stacksize= flag.
==20047==  The main thread stack size used in this run was 8388608.

(ModemManager:2858): GLib-GObject-CRITICAL **: g_object_ref: assertion
`G_IS_OBJECT (object)' failed
(ModemManager:2858): GLib-GObject-CRITICAL **: g_object_ref: assertion
`G_IS_OBJECT (object)' failed
(ModemManager:2858): GLib-GObject-CRITICAL **: g_object_unref: assertion
`G_IS_OBJECT (object)' failed
(ModemManager:2858): GLib-GObject-CRITICAL **: g_object_unref: assertion
`G_IS_OBJECT (object)' failed
(ModemManager:2858): GLib-GObject-WARNING **: gsignal.c:2576: instance
`0x78624028' has no handler with id `148'
(ModemManager:2858): GLib-GObject-WARNING **: invalid unclassed pointer in
cast to `MMSerialPort'
(ModemManager:2858): GLib-GObject-CRITICAL **: g_type_instance_get_private:
assertion `instance != NULL  instance-g_class != NULL' failed

On Thu, Nov 29, 2012 at 11:26 AM, Dan Williams d...@redhat.com wrote:

 On Tue, 2012-11-27 at 13:19 -0800, Ben Chan wrote:
  Yes, it's related to the data_available (mm-serial-port.c:767) crash
  (crosbug.com/35391).  I'm running suspend/resume stress test with
  ModemManager under valgrind.

 Any way to get MM running with --debug in those logs?  Also, in your
 sources, does line 767 match up with:

 info = g_queue_peek_nth (priv-queue, 0);

 or some other line?

 One other thing to do, put an

 mm_info ((%s): disposing, mm_port_get_device (MM_PORT (self)));

 into dispose() and lets see when it gets disposed and if anything tries
 to open it during/after dispose.  Drop another of these into
 mm_serial_port_new().

 Dan

 
  Thanks,
  Ben
 
  On Tue, Nov 27, 2012 at 1:13 PM, Aleksander Morgado
  aleksan...@lanedo.com wrote:
  On 11/27/2012 09:39 PM, Ben Chan wrote:
   ---
src/mm-serial-port.c |   10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
 
 
 
  Don't think it's the correct approach.
 
  I don't think we ever run g_object_run_dispose() ourselves for
  port
  objects, and that means that whenever we got the port object
  disposed it
  was because it was the last valid reference.
 
  And that means that you won't be able to read priv-disposed
  afterwards
  as that would mean reading already freed memory.
 
  There clearly is a missing reference around, or a timeout or
  other
  source which has the port as user_data and is not being
  properly cleaned
  up (e.g. removing the timeout or event source when the port is
  disposed). Running it under valgrind should confirm this. Is
  this
  related to the data_available() crashes during suspend/resume?
 
 
  
   diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c
   index 0a8820d..a33c745 100644
   --- a/src/mm-serial-port.c
   +++ b/src/mm-serial-port.c
   @@ -69,6 +69,7 @@ static guint signals[LAST_SIGNAL] = { 0 };
  
typedef struct {
guint32 open_count;
   +gboolean disposed;
gboolean forced_close;
int fd;
GHashTable *reply_cache;
   @@ -849,6 +850,12 @@ mm_serial_port_open (MMSerialPort
  *self, GError **error)
  
device = mm_port_get_device (MM_PORT (self));
  
   +/* If the MMSerialPort object has been disposed, just
  return an error. */
   +if (priv-disposed) {
   +mm_info ((%s) skipped opening serial port that has
  been disposed, device);
   +return FALSE;
   +}
   +
if (priv-open_count) {
/* Already open */
goto success;
   @@ -1537,6 +1544,9 @@ dispose (GObject *object)
{
MMSerialPortPrivate *priv = MM_SERIAL_PORT_GET_PRIVATE
  (object);
  
   +/* Mark

[MM] [PATCH] serial-port: avoid opening a serial port that has been disposed

2012-11-27 Thread Ben Chan
---
 src/mm-serial-port.c |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c
index 0a8820d..dee2fec 100644
--- a/src/mm-serial-port.c
+++ b/src/mm-serial-port.c
@@ -849,6 +849,13 @@ mm_serial_port_open (MMSerialPort *self, GError **error)
 
 device = mm_port_get_device (MM_PORT (self));
 
+/* If we forced closing the port, the MMSerialPort object has been 
disposed.
+ * Just return an error. */
+if (priv-forced_close) {
+mm_info ((%s) skipped opening serial port that has been disposed, 
device);
+return FALSE;
+}
+
 if (priv-open_count) {
 /* Already open */
 goto success;
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] serial-port: avoid opening a serial port that has been disposed

2012-11-27 Thread Ben Chan
I guess the major concern would be using priv-forced_close as an indicator
of whether the serial port is allowed to reopen or not. How about using
another variable to tag the MMSerialPort object as being disposed?  There
are a few call sites calling mm_serial_port_open(), so it seems better to
check inside mm_serial_port_open().

How do you think?  I can revise the patch if that makes sense.

Thanks,
Ben

On Tue, Nov 27, 2012 at 12:03 PM, Dan Williams d...@redhat.com wrote:

 On Tue, 2012-11-27 at 10:57 -0800, Ben Chan wrote:
  ---
   src/mm-serial-port.c |7 +++
   1 files changed, 7 insertions(+), 0 deletions(-)

 I guess I'd rather fix the code that's attempting to re-open the port if
 it's not supposed to do that.  This has the effect of making ports
 single-use only, which wasn't originally the intention.  Forcing a port
 closed regardless of the open count doesn't necessarily mean the port
 object itself has been disposed.

 Dan

  diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c
  index 0a8820d..dee2fec 100644
  --- a/src/mm-serial-port.c
  +++ b/src/mm-serial-port.c
  @@ -849,6 +849,13 @@ mm_serial_port_open (MMSerialPort *self, GError
 **error)
 
   device = mm_port_get_device (MM_PORT (self));
 
  +/* If we forced closing the port, the MMSerialPort object has been
 disposed.
  + * Just return an error. */
  +if (priv-forced_close) {
  +mm_info ((%s) skipped opening serial port that has been
 disposed, device);
  +return FALSE;
  +}
  +
   if (priv-open_count) {
   /* Already open */
   goto success;



___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed

2012-11-27 Thread Ben Chan
---
 src/mm-serial-port.c |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c
index 0a8820d..a33c745 100644
--- a/src/mm-serial-port.c
+++ b/src/mm-serial-port.c
@@ -69,6 +69,7 @@ static guint signals[LAST_SIGNAL] = { 0 };
 
 typedef struct {
 guint32 open_count;
+gboolean disposed;
 gboolean forced_close;
 int fd;
 GHashTable *reply_cache;
@@ -849,6 +850,12 @@ mm_serial_port_open (MMSerialPort *self, GError **error)
 
 device = mm_port_get_device (MM_PORT (self));
 
+/* If the MMSerialPort object has been disposed, just return an error. */
+if (priv-disposed) {
+mm_info ((%s) skipped opening serial port that has been disposed, 
device);
+return FALSE;
+}
+
 if (priv-open_count) {
 /* Already open */
 goto success;
@@ -1537,6 +1544,9 @@ dispose (GObject *object)
 {
 MMSerialPortPrivate *priv = MM_SERIAL_PORT_GET_PRIVATE (object);
 
+/* Mark the MMSerialPort object as disposed to prevent it from being 
re-opened. */
+priv-disposed = TRUE;
+
 if (priv-timeout_id) {
 g_source_remove (priv-timeout_id);
 priv-timeout_id = 0;
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed

2012-11-27 Thread Ben Chan
Yes, it's related to the data_available (mm-serial-port.c:767) crash (
crosbug.com/35391).  I'm running suspend/resume stress test with
ModemManager under valgrind.

Thanks,
Ben

On Tue, Nov 27, 2012 at 1:13 PM, Aleksander Morgado
aleksan...@lanedo.comwrote:

 On 11/27/2012 09:39 PM, Ben Chan wrote:
  ---
   src/mm-serial-port.c |   10 ++
   1 files changed, 10 insertions(+), 0 deletions(-)


 Don't think it's the correct approach.

 I don't think we ever run g_object_run_dispose() ourselves for port
 objects, and that means that whenever we got the port object disposed it
 was because it was the last valid reference.

 And that means that you won't be able to read priv-disposed afterwards
 as that would mean reading already freed memory.

 There clearly is a missing reference around, or a timeout or other
 source which has the port as user_data and is not being properly cleaned
 up (e.g. removing the timeout or event source when the port is
 disposed). Running it under valgrind should confirm this. Is this
 related to the data_available() crashes during suspend/resume?


 
  diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c
  index 0a8820d..a33c745 100644
  --- a/src/mm-serial-port.c
  +++ b/src/mm-serial-port.c
  @@ -69,6 +69,7 @@ static guint signals[LAST_SIGNAL] = { 0 };
 
   typedef struct {
   guint32 open_count;
  +gboolean disposed;
   gboolean forced_close;
   int fd;
   GHashTable *reply_cache;
  @@ -849,6 +850,12 @@ mm_serial_port_open (MMSerialPort *self, GError
 **error)
 
   device = mm_port_get_device (MM_PORT (self));
 
  +/* If the MMSerialPort object has been disposed, just return an
 error. */
  +if (priv-disposed) {
  +mm_info ((%s) skipped opening serial port that has been
 disposed, device);
  +return FALSE;
  +}
  +
   if (priv-open_count) {
   /* Already open */
   goto success;
  @@ -1537,6 +1544,9 @@ dispose (GObject *object)
   {
   MMSerialPortPrivate *priv = MM_SERIAL_PORT_GET_PRIVATE (object);
 
  +/* Mark the MMSerialPort object as disposed to prevent it from
 being re-opened. */
  +priv-disposed = TRUE;
  +
   if (priv-timeout_id) {
   g_source_remove (priv-timeout_id);
   priv-timeout_id = 0;
 


 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] broadband-modem: check for NULL response in parse_caps_cpin and parse_caps_cgmm

2012-11-26 Thread Ben Chan
---
 src/mm-broadband-modem.c |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 9161c82..070526f 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -465,6 +465,9 @@ parse_caps_cpin (MMBaseModem *self,
  GVariant **result,
  GError **result_error)
 {
+if (!response)
+return FALSE;
+
 if (strcasestr (response, SIM PIN) ||
 strcasestr (response, SIM PUK) ||
 strcasestr (response, PH-SIM PIN) ||
@@ -498,6 +501,9 @@ parse_caps_cgmm (MMBaseModem *self,
  GVariant **result,
  GError **result_error)
 {
+if (!response)
+return FALSE;
+
 /* This check detects some really old Motorola GPRS dongles and phones */
 if (strstr (response, GSM900) ||
 strstr (response, GSM1800) ||
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH v2] broadband-modem: check for NULL response in parse_caps_{cpin, cgmm, gcap}

2012-11-26 Thread Ben Chan
---
 src/mm-broadband-modem.c |   11 ++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 9161c82..174de9d 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -435,10 +435,13 @@ parse_caps_gcap (MMBaseModem *self,
 const ModemCaps *cap = modem_caps;
 guint32 ret = 0;
 
+if (!response)
+return FALSE;
+
 /* Some modems (Huawei E160g) won't respond to +GCAP with no SIM, but
  * will respond to ATI.  Ignore the error and continue.
  */
-if (response  strstr (response, +CME ERROR:))
+if (strstr (response, +CME ERROR:))
 return FALSE;
 
 while (cap-name) {
@@ -465,6 +468,9 @@ parse_caps_cpin (MMBaseModem *self,
  GVariant **result,
  GError **result_error)
 {
+if (!response)
+return FALSE;
+
 if (strcasestr (response, SIM PIN) ||
 strcasestr (response, SIM PUK) ||
 strcasestr (response, PH-SIM PIN) ||
@@ -498,6 +504,9 @@ parse_caps_cgmm (MMBaseModem *self,
  GVariant **result,
  GError **result_error)
 {
+if (!response)
+return FALSE;
+
 /* This check detects some really old Motorola GPRS dongles and phones */
 if (strstr (response, GSM900) ||
 strstr (response, GSM1800) ||
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] [PATCH] novatel-lte: use $NWMDN to read own number when +CNUM fails

2012-11-05 Thread Ben Chan
The number(s) returned by +CNUM include the number return by $NWMDN.
Ideally, we should be able to just use +CNUM. But probably due to some
firmware issue, +CNUM sometimes returns ERROR unexpectedly shortly after
the SIM interface becomes ready. It also returns ERROR for an unactivated
SIM.$NWMDN seems to always behave properly, so that this patch uses it
as a fallback.   And also because $NWMDN only returns one number, I still
try +CNUM first when possible.

Thanks,
Ben

On Mon, Nov 5, 2012 at 4:48 AM, Aleksander Morgado aleksan...@lanedo.comwrote:

 On 04/11/12 02:00, Ben Chan wrote:
  +CNUM may return ERROR when the modem fails to read the own numbers from
  the SIM card or when the SIM card hasn't been activated. Use $NWMDN to
  read the MDN as a fallback, which distinguishes these two cases.

 Just wondering, shouldn't the plugin try to read always both? OwnNumbers
 is an array of strings, you can both the one retrieved from +CNUM (if
 any) and the MDN in there.

 --
 Aleksander

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] novatel-lte: use $NWMDN to read own number when +CNUM fails

2012-11-03 Thread Ben Chan
+CNUM may return ERROR when the modem fails to read the own numbers from
the SIM card or when the SIM card hasn't been activated. Use $NWMDN to
read the MDN as a fallback, which distinguishes these two cases.
---
 plugins/novatel/mm-broadband-modem-novatel-lte.c |  103 ++
 1 files changed, 103 insertions(+), 0 deletions(-)

diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c 
b/plugins/novatel/mm-broadband-modem-novatel-lte.c
index 6f2e7e2..249a4a1 100644
--- a/plugins/novatel/mm-broadband-modem-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c
@@ -158,6 +158,107 @@ modem_after_sim_unlock (MMIfaceModem *self,
 }
 
 /*/
+/* Load own numbers (Modem interface) */
+
+static GStrv
+load_own_numbers_finish (MMIfaceModem *self,
+ GAsyncResult *res,
+ GError **error)
+{
+GVariant *result;
+GStrv own_numbers;
+
+result = mm_base_modem_at_sequence_finish (MM_BASE_MODEM (self), res, 
NULL, error);
+if (!result)
+return NULL;
+
+own_numbers = (GStrv) g_variant_dup_strv (result, NULL);
+return own_numbers;
+}
+
+static gboolean
+response_processor_cnum_ignore_at_errors (MMBaseModem *self,
+  gpointer none,
+  const gchar *command,
+  const gchar *response,
+  gboolean last_command,
+  const GError *error,
+  GVariant **result,
+  GError **result_error)
+{
+GStrv own_numbers;
+
+if (error) {
+/* Ignore AT errors (ie, ERROR or CMx ERROR) */
+if (error-domain != MM_MOBILE_EQUIPMENT_ERROR || last_command)
+*result_error = g_error_copy (error);
+
+return FALSE;
+}
+
+own_numbers = mm_3gpp_parse_cnum_exec_response (response, result_error);
+if (!own_numbers)
+return FALSE;
+
+*result = g_variant_new_strv ((const gchar *const *) own_numbers, -1);
+g_strfreev (own_numbers);
+return TRUE;
+}
+
+static gboolean
+response_processor_nwmdn_ignore_at_errors (MMBaseModem *self,
+   gpointer none,
+   const gchar *command,
+   const gchar *response,
+   gboolean last_command,
+   const GError *error,
+   GVariant **result,
+   GError **result_error)
+{
+GArray *array;
+GStrv own_numbers;
+gchar *mdn;
+
+if (error) {
+/* Ignore AT errors (ie, ERROR or CMx ERROR) */
+if (error-domain != MM_MOBILE_EQUIPMENT_ERROR || last_command)
+*result_error = g_error_copy (error);
+
+return FALSE;
+}
+
+mdn = g_strdup (mm_strip_tag (response, $NWMDN:));
+array = g_array_new (TRUE, TRUE, sizeof (gchar *));
+g_array_append_val (array, mdn);
+own_numbers = (GStrv) g_array_free (array, FALSE);
+
+*result = g_variant_new_strv ((const gchar *const *) own_numbers, -1);
+g_strfreev (own_numbers);
+return TRUE;
+}
+
+static const MMBaseModemAtCommand own_numbers_commands[] = {
+{ +CNUM,  3, TRUE, response_processor_cnum_ignore_at_errors },
+{ $NWMDN, 3, TRUE, response_processor_nwmdn_ignore_at_errors },
+{ NULL }
+};
+
+static void
+load_own_numbers (MMIfaceModem *self,
+  GAsyncReadyCallback callback,
+  gpointer user_data)
+{
+mm_dbg (loading (Novatel LTE) own numbers...);
+mm_base_modem_at_sequence (
+MM_BASE_MODEM (self),
+own_numbers_commands,
+NULL, /* response_processor_context */
+NULL, /* response_processor_context_free */
+callback,
+user_data);
+}
+
+/*/
 /* Load supported bands (Modem interface) */
 
 /*
@@ -449,6 +550,8 @@ iface_modem_init (MMIfaceModem *iface)
 iface-create_sim_finish = modem_create_sim_finish;
 iface-modem_after_sim_unlock = modem_after_sim_unlock;
 iface-modem_after_sim_unlock_finish = modem_after_sim_unlock_finish;
+iface-load_own_numbers = load_own_numbers;
+iface-load_own_numbers_finish = load_own_numbers_finish;
 iface-load_supported_bands = load_supported_bands;
 iface-load_supported_bands_finish = load_supported_bands_finish;
 iface-load_current_bands = load_current_bands;
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM PATCH] iface-modem: load own numbers after SIM initialization

2012-10-31 Thread Ben Chan
In 3GPP, own numbers are loaded from the SIM card, the loading of own
numbers should be scheduled after the SIM card is ready.
---
 src/mm-iface-modem.c |   34 +-
 1 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c
index 082846e..d25b03d 100644
--- a/src/mm-iface-modem.c
+++ b/src/mm-iface-modem.c
@@ -3128,10 +3128,10 @@ typedef enum {
 INITIALIZATION_STEP_REVISION,
 INITIALIZATION_STEP_EQUIPMENT_ID,
 INITIALIZATION_STEP_DEVICE_ID,
-INITIALIZATION_STEP_OWN_NUMBERS,
 INITIALIZATION_STEP_UNLOCK_REQUIRED,
 INITIALIZATION_STEP_UNLOCK_RETRIES,
 INITIALIZATION_STEP_SIM,
+INITIALIZATION_STEP_OWN_NUMBERS,
 INITIALIZATION_STEP_SUPPORTED_MODES,
 INITIALIZATION_STEP_SUPPORTED_BANDS,
 INITIALIZATION_STEP_LAST
@@ -3649,22 +3649,6 @@ interface_initialization_step (InitializationContext 
*ctx)
 /* Fall down to next step */
 ctx-step++;
 
-case INITIALIZATION_STEP_OWN_NUMBERS:
-/* Own numbers is meant to be loaded only once during the whole
- * lifetime of the modem. Therefore, if we already have them loaded,
- * don't try to load them again. */
-if (mm_gdbus_modem_get_own_numbers (ctx-skeleton) == NULL 
-MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers 
-MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers_finish) 
{
-MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers (
-ctx-self,
-(GAsyncReadyCallback)load_own_numbers_ready,
-ctx);
-return;
-}
-/* Fall down to next step */
-ctx-step++;
-
 case INITIALIZATION_STEP_UNLOCK_REQUIRED:
 /* Only check unlock required if we were previously not unlocked */
 if (mm_gdbus_modem_get_unlock_required (ctx-skeleton) != 
MM_MODEM_LOCK_NONE) {
@@ -3710,6 +3694,22 @@ interface_initialization_step (InitializationContext 
*ctx)
 return;
 }
 
+case INITIALIZATION_STEP_OWN_NUMBERS:
+/* Own numbers is meant to be loaded only once during the whole
+ * lifetime of the modem. Therefore, if we already have them loaded,
+ * don't try to load them again. */
+if (mm_gdbus_modem_get_own_numbers (ctx-skeleton) == NULL 
+MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers 
+MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers_finish) 
{
+MM_IFACE_MODEM_GET_INTERFACE (ctx-self)-load_own_numbers (
+ctx-self,
+(GAsyncReadyCallback)load_own_numbers_ready,
+ctx);
+return;
+}
+/* Fall down to next step */
+ctx-step++;
+
 case INITIALIZATION_STEP_SUPPORTED_MODES:
 g_assert (MM_IFACE_MODEM_GET_INTERFACE 
(ctx-self)-load_supported_modes != NULL);
 g_assert (MM_IFACE_MODEM_GET_INTERFACE 
(ctx-self)-load_supported_modes_finish != NULL);
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM PATCH] novatel-lte: increase the wait after SIM unlock to 3 seconds

2012-10-31 Thread Ben Chan
After repeated stress tests on a few Novatel E362 modems and SIM cards,
it is revealed that a 3-second wait after SIM unlock is necessary for
reliably reading ICCID and IMSI through the SIM interface.
---
 plugins/novatel/mm-broadband-modem-novatel-lte.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/novatel/mm-broadband-modem-novatel-lte.c 
b/plugins/novatel/mm-broadband-modem-novatel-lte.c
index da855fc..6f2e7e2 100644
--- a/plugins/novatel/mm-broadband-modem-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-modem-novatel-lte.c
@@ -152,9 +152,9 @@ modem_after_sim_unlock (MMIfaceModem *self,
 user_data,
 modem_after_sim_unlock);
 
-/* A 2-second wait is necessary for SIM to become ready.
+/* A 3-second wait is necessary for SIM to become ready.
  * Otherwise, a subsequent AT+CRSM command will likely fail. */
-g_timeout_add_seconds (2, (GSourceFunc)after_sim_unlock_wait_cb, result);
+g_timeout_add_seconds (3, (GSourceFunc)after_sim_unlock_wait_cb, result);
 }
 
 /*/
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM PATCH 2/2] novatel-lte: retry $NWQMISTATUS upon an unknown error during connecting

2012-10-19 Thread Ben Chan
The $NWQMISTATUS command sometimes replies an ERROR shortly after
calling the $NWQMICONNECT command, but then replies the proper QMI
status if we retry it. This behavior is observed on an E362 modem with
4.08 firmware.

(ttyUSB0): -- 'AT$NWQMICONNECT=,,CR'
(ttyUSB0): -- 'CRLFOKCRLF'
(ttyUSB0): -- 'AT$NWQMISTATUSCR'
(ttyUSB0): -- 'CRLFERRORCRLF'
Got failure code 100: Unknown error
QMI connection status failed: Unknown error
---
 plugins/novatel/mm-broadband-bearer-novatel-lte.c |   14 --
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c 
b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
index e570024..e1944bb 100644
--- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
@@ -191,12 +191,14 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem,
   error);
 if (!result) {
 mm_warn (QMI connection status failed: %s, error-message);
-g_simple_async_result_take_error (ctx-result, error);
-detailed_connect_context_complete_and_free (ctx);
-return;
-}
-
-if (is_qmistatus_connected (result)) {
+if (!g_error_matches (error, MM_MOBILE_EQUIPMENT_ERROR, 
MM_MOBILE_EQUIPMENT_ERROR_UNKNOWN)) {
+g_simple_async_result_take_error (ctx-result, error);
+detailed_connect_context_complete_and_free (ctx);
+return;
+}
+g_error_free (error);
+result = Unknown error;
+} else if (is_qmistatus_connected (result)) {
 MMBearerIpConfig *config;
 
 mm_dbg(Connected);
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM PATCH 1/2] novatel-lte: handle $NWQMISTATUS responses for firmware 4.08

2012-10-19 Thread Ben Chan
In firmware 4.08, the $NWQMISTATUS command returns different values for
QMI state to indicate the current connection state. This patch modifies
the code to handle $NWQMISTATUS responses in firmware 1.41 and 4.08.
---
 plugins/novatel/mm-broadband-bearer-novatel-lte.c |   24 +++-
 1 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/plugins/novatel/mm-broadband-bearer-novatel-lte.c 
b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
index e7e76f6..e570024 100644
--- a/plugins/novatel/mm-broadband-bearer-novatel-lte.c
+++ b/plugins/novatel/mm-broadband-bearer-novatel-lte.c
@@ -33,6 +33,7 @@
 #include mm-modem-helpers.h
 
 #define CONNECTION_CHECK_TIMEOUT_SEC 5
+#define QMISTATUS_TAG $NWQMISTATUS:
 
 G_DEFINE_TYPE (MMBroadbandBearerNovatelLte, mm_broadband_bearer_novatel_lte, 
MM_TYPE_BROADBAND_BEARER);
 
@@ -121,6 +122,20 @@ connect_3gpp_finish (MMBroadbandBearer *self,
 
 static gboolean connect_3gpp_qmistatus (DetailedConnectContext *ctx);
 
+static gboolean
+is_qmistatus_connected(const gchar *str)
+{
+str = mm_strip_tag (str, QMISTATUS_TAG);
+return g_strrstr (str, QMI State: CONNECTED) || g_strrstr (str, QMI 
State: QMI_WDS_PKT_DATA_CONNECTED);
+}
+
+static gboolean
+is_qmistatus_disconnected(const gchar *str)
+{
+str = mm_strip_tag (str, QMISTATUS_TAG);
+return g_strrstr (str, QMI State: DISCONNECTED) || g_strrstr (str, QMI 
State: QMI_WDS_PKT_DATA_DISCONNECTED);
+}
+
 static void
 poll_connection_ready (MMBaseModem *modem,
GAsyncResult *res,
@@ -136,8 +151,7 @@ poll_connection_ready (MMBaseModem *modem,
 return;
 }
 
-result = mm_strip_tag (result, $NWQMISTATUS:);
-if (g_strrstr (result, QMI State: DISCONNECTED)) {
+if (is_qmistatus_disconnected (result)) {
 mm_bearer_report_disconnection (MM_BEARER (bearer));
 g_source_remove (bearer-priv-connection_poller);
 bearer-priv-connection_poller = 0;
@@ -182,8 +196,7 @@ connect_3gpp_qmistatus_ready (MMBaseModem *modem,
 return;
 }
 
-result = mm_strip_tag (result, $NWQMISTATUS:);
-if (g_strrstr (result, QMI State: CONNECTED)) {
+if (is_qmistatus_connected (result)) {
 MMBearerIpConfig *config;
 
 mm_dbg(Connected);
@@ -372,8 +385,7 @@ disconnect_3gpp_status_complete (MMBaseModem *modem,
 g_error_free (error);
 }
 
-result = mm_strip_tag (result, $NWQMISTATUS:);
-if (result  g_strrstr (result, QMI State: DISCONNECTED))
+if (result  is_qmistatus_disconnected (result))
 g_simple_async_result_set_op_res_gboolean (ctx-result, TRUE);
 else
 g_simple_async_result_set_error (ctx-result,
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[PATCH] icera: improve parsing of access technologies in %NWSTATE response

2012-10-16 Thread Ben Chan
---
 plugins/icera/mm-broadband-modem-icera.c |   40 +++---
 1 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/plugins/icera/mm-broadband-modem-icera.c 
b/plugins/icera/mm-broadband-modem-icera.c
index db70252..8267ba6 100644
--- a/plugins/icera/mm-broadband-modem-icera.c
+++ b/plugins/icera/mm-broadband-modem-icera.c
@@ -322,27 +322,27 @@ ipdpact_received (MMAtSerialPort *port,
 static MMModemAccessTechnology
 nwstate_to_act (const gchar *str)
 {
+MMModemAccessTechnology technologies = 0;
+
 /* small 'g' means CS, big 'G' means PS */
-if (!strcmp (str, 2g))
-return MM_MODEM_ACCESS_TECHNOLOGY_GSM;
-else if (!strcmp (str, 2G-GPRS))
-return MM_MODEM_ACCESS_TECHNOLOGY_GPRS;
-else if (!strcmp (str, 2G-EDGE))
-return MM_MODEM_ACCESS_TECHNOLOGY_EDGE;
-else if (!strcmp (str, 3G))
-return MM_MODEM_ACCESS_TECHNOLOGY_UMTS;
-else if (!strcmp (str, 3g))
-return MM_MODEM_ACCESS_TECHNOLOGY_UMTS;
-else if (!strcmp (str, R99))
-return MM_MODEM_ACCESS_TECHNOLOGY_UMTS;
-else if (!strcmp (str, 3G-HSDPA) || !strcmp (str, HSDPA))
-return MM_MODEM_ACCESS_TECHNOLOGY_HSDPA;
-else if (!strcmp (str, 3G-HSUPA) || !strcmp (str, HSUPA))
-return MM_MODEM_ACCESS_TECHNOLOGY_HSUPA;
-else if (!strcmp (str, 3G-HSDPA-HSUPA) || !strcmp (str, HSDPA-HSUPA))
-return MM_MODEM_ACCESS_TECHNOLOGY_HSPA;
-
-return MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN;
+if (strstr (str, 2g) || strstr (str, 2G))
+technologies |= MM_MODEM_ACCESS_TECHNOLOGY_GSM;
+if (strstr (str, GPRS))
+technologies |= MM_MODEM_ACCESS_TECHNOLOGY_GPRS;
+if (strstr (str, EDGE))
+technologies |= MM_MODEM_ACCESS_TECHNOLOGY_EDGE;
+if (strstr (str, 3g) || strstr (str, 3G) || strstr (str, R99))
+technologies |= MM_MODEM_ACCESS_TECHNOLOGY_UMTS;
+if (strstr (str, HSDPA-HSUPA) || strstr (str, HSUPA-HSDPA))
+technologies |= MM_MODEM_ACCESS_TECHNOLOGY_HSPA;
+if (strstr (str, HSDPA))
+technologies |= MM_MODEM_ACCESS_TECHNOLOGY_HSDPA;
+if (strstr (str, HSUPA))
+technologies |= MM_MODEM_ACCESS_TECHNOLOGY_HSUPA;
+if (strstr (str, HSPA+))
+technologies |= MM_MODEM_ACCESS_TECHNOLOGY_HSPA_PLUS;
+
+return technologies;
 }
 
 static void
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH] icera: improve parsing of access technologies in %NWSTATE response

2012-10-16 Thread Ben Chan
On Tue, Oct 16, 2012 at 12:49 AM, Aleksander Morgado
aleksan...@lanedo.com wrote:

 I pushed this patch as is, but then I realized that the report of
 current access technologies is supposed to give which is the *current*
 access technology being active. We allow reporting more than one for the
 cases where several access technologies are given simultaneously (e.g.
 cdma1x + evdo + lte). So not sure if there is much benefit in giving
 e.g. 4 different technologies like umts, hsdpa, hsupa, hspa when the
 modem reports 3G-HSDPA-HSUPA. In this example, umts shouldn't really
 be given I guess, and also we know that HSPA is just HSDPA+HSUPA, so
 giving HSPA only was enough... so... should I revert the patch? :-/


I guess that makes sense :)  The previous patch tries to address your
comment in crosbug.com/33483. Let's revert it. The real issue in
crosbug.com/33483 is that nwstate_to_act doesn't handle HSPA+, so I
think we can fix it as follows:

Thanks,
Ben

From f90c8bea9dae89aceb6d7d0cf037cbc4bc73d2df Mon Sep 17 00:00:00 2001
From: Ben Chan benc...@chromium.org
Date: Tue, 16 Oct 2012 00:55:40 -0700
Subject: [PATCH] icera: parse HSPA+ access technology in %NWSTATE response

---
 plugins/icera/mm-broadband-modem-icera.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/plugins/icera/mm-broadband-modem-icera.c
b/plugins/icera/mm-broadband-modem-icera.c
index db70252..697e8cc 100644
--- a/plugins/icera/mm-broadband-modem-icera.c
+++ b/plugins/icera/mm-broadband-modem-icera.c
@@ -341,6 +341,8 @@ nwstate_to_act (const gchar *str)
 return MM_MODEM_ACCESS_TECHNOLOGY_HSUPA;
 else if (!strcmp (str, 3G-HSDPA-HSUPA) || !strcmp (str, HSDPA-HSUPA))
 return MM_MODEM_ACCESS_TECHNOLOGY_HSPA;
+else if (!strcmp (str, 3G-HSDPA-HSUPA-HSPA+) || !strcmp (str,
HSDPA-HSUPA-HSPA+))
+return MM_MODEM_ACCESS_TECHNOLOGY_HSPA_PLUS;

 return MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN;
 }
-- 
1.7.7.3
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] glib assertions

2012-09-23 Thread Ben Chan
 FYI, while running suspend/resume stress tests on various modems, MM
 gives a few glib assertion warnings.  I'm going to run gdb to get a
 backtrace, but wonder if you know where the issue could potentially
 be.

 (ModemManager:648): GLib-GIO-CRITICAL **:
 g_dbus_connection_emit_signal: assertion `G_IS_DBUS_CONNECTION
 (connection)' failed
 (ModemManager:648): GLib-GObject-CRITICAL **: g_object_ref: assertion
 `G_IS_OBJECT (object)' failed
 (ModemManager:648): GLib-GObject-CRITICAL **: g_object_unref:
 assertion `G_IS_OBJECT (object)' failed

 Also, I had to take out the following assertion in MMBroadbandModem
 disabling_step() to prevent a crash. It seems like a race when
 mm_base_modem_disable() is being called on MMBaseModem and
 MMBroardbandModem is being disposed. Should we turn the assertion into
 a condition check instead?

 case DISABLING_STEP_IFACE_MODEM:
 g_assert (ctx-self-priv-modem_dbus_skeleton != NULL);

 Thread 0 (crashed)
  0  libc-2.15.so!raise [raise.c : 64 + 0x10]
 rbx = 0x7fcfdec08260   r12 = 0x7fcf52c4
 r13 = 0x7fcfdec073e0   r14 = 0x7fcfdd8d3960
 r15 = 0x7fcfdeabd800   rip = 0x7fcfdd224aa5
 rsp = 0x7fff300c3e18   rbp = 0x7fcfdd8d4260
 Found by: given as instruction pointer in context
  1  libc-2.15.so!abort [abort.c : 91 + 0x9]
 rbx = 0x7fcfdec08260   r12 = 0x7fcf52c4
 r13 = 0x7fcfdec073e0   r14 = 0x7fcfdd8d3960
 r15 = 0x7fcfdeabd800   rip = 0x7fcfdd225f07
 rsp = 0x7fff300c3e20   rbp = 0x7fcfdd8d4260
 Found by: call frame info
  2  libglib-2.0.so.0.3200.4!g_assertion_message [gtestutils.c : 1861 + 0x4]
 rbx = 0x7fcfdec08260   r12 = 0x7fcf52c4
 r13 = 0x7fcfdec073e0   r14 = 0x7fcfdd8d3960
 r15 = 0x7fcfdeabd800   rip = 0x7fcfdd81b83d
 rsp = 0x7fff300c3f50   rbp = 0x7fcfdd8d4260
 Found by: call frame info
  3  libglib-2.0.so.0.3200.4!g_assertion_message_expr [gtestutils.c : 1872 +
 0xc]
 rbx = 0x7fcfdddc52d0   r12 = 0x7fcf52c4
 r13 = 0x   r14 = 0x7fcfdd8d3960
 r15 = 0x7fcfdeabd800   rip = 0x7fcfdd81bd12
 rsp = 0x7fff300c3fe0   rbp = 0x1b45
 Found by: call frame info
  4  ModemManager!disabling_step [mm-broadband-modem.c : 6981 + 0x20]
 rbx = 0x7fcfdec0ed00   r12 = 0x0041
 r13 = 0x7fcfdeac4f00   r14 = 0x7fcfdd8d3960
 r15 = 0x7fcfdeabd800   rip = 0x7fcfddda6571
 rsp = 0x7fff300c4010   rbp = 0x7fcfdeaf4200
 Found by: call frame info
  5  ModemManager!handle_enable_auth_ready [mm-iface-modem.c : 1216 + 0x4]
 rbx = 0x7fcfdec0fd30   r12 = 0x0041
 r13 = 0x7fcfdeac4f00   r14 = 0x7fcfdd8d3960
 r15 = 0x7fcfdeabd800   rip = 0x7fcfddd84379
 rsp = 0x7fff300c4020   rbp = 0x7fcfdeaf4200
 Found by: call frame info
  6  libgio-2.0.so.0.3200.4!g_simple_async_result_complete
 [gsimpleasyncresult.c : 767 + 0xd]
 rbx = 0x7fcfdeb03e30   r12 = 0x0041
 r13 = 0x7fcfdeac4f00   r14 = 0x7fcfdd8d3960
 r15 = 0x7fcfdeabd800   rip = 0x7fcfddbd0447
 rsp = 0x7fff300c4050   rbp = 0x7fcfdec0e650
 Found by: call frame info
  7  ModemManager!authorize_ready [mm-base-modem.c : 1015 + 0x7]
 rbx = 0x7fcfdeb03e30   r12 = 0x0041
 r13 = 0x7fcfdeac4f00   r14 = 0x7fcfdd8d3960
 r15 = 0x7fcfdeabd800   rip = 0x7fcfddd7a161
 rsp = 0x7fff300c4070   rbp = 0x7fcfdec0e650
 Found by: call frame info
  8  libgio-2.0.so.0.3200.4!g_simple_async_result_complete
 [gsimpleasyncresult.c : 767 + 0xd]
 rbx = 0x7fcfdeb040f0   r12 = 0x0041
 r13 = 0x7fcfdeac4f00   r14 = 0x7fcfdd8d3960
 r15 = 0x7fcfdeabd800   rip = 0x7fcfddbd0447
 rsp = 0x7fff300c4090   rbp = 0x7fcfdec0e650
 Found by: call frame info
  9  libgio-2.0.so.0.3200.4!complete_in_idle_cb [gsimpleasyncresult.c :
 779 + 0x4]
 rbx = 0x7fcfdec0e650   r12 = 0x0041
 r13 = 0x7fcfdeac4f00   r14 = 0x7fcfdd8d3960
 r15 = 0x7fcfdeabd800   rip = 0x7fcfddbd0549
 rsp = 0x7fff300c40b0   rbp = 0x0001
 Found by: call frame info
 10  libglib-2.0.so.0.3200.4!g_main_context_dispatch [gmain.c : 2539 + 0x3]
 rbx = 0x7fcfdec0e650   r12 = 0x0041
 r13 = 0x7fcfdeac4f00   r14 = 0x7fcfdd8d3960
 r15 = 0x7fcfdeabd800   rip = 0x7fcfdd7fa5c3
 rsp = 0x7fff300c40c0   rbp = 0x0001
 Found by: call frame info
 11  libglib-2.0.so.0.3200.4!g_main_context_iterate [gmain.c : 3146 + 0x7]
 rbx = 0x7fcfdeabd800   r12 = 0x7fcfdd807d70
 r13 = 0x   r14 = 0x0002
 r15 = 0x0001   rip = 0x7fcfdd7fa940
 rsp = 0x7fff300c4150   rbp = 0x0001
 Found by: call frame info
 12  

[MM] [PATCH] broadband-modem: increase timeout for ATZ

2012-09-22 Thread Ben Chan
Some modem like Novatel LTE can take up to 5 seconds to complete the ATZ
command. Increase the timeout for ATZ from 3 seconds to 6 seconds.
---
 src/mm-broadband-modem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 4a2fdb8..10100c8 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -2295,7 +2295,7 @@ static const MMBaseModemAtCommand modem_init_sequence[] = 
{
  *   after the Z command because such commands may be ignored.
  * So run ATZ alone.
  */
-{ Z, 3, FALSE, mm_base_modem_response_processor_no_result_continue },
+{ Z, 6, FALSE, mm_base_modem_response_processor_no_result_continue },
 
 /* Ensure echo is off after the init command */
 { E0 V1,  3, FALSE, NULL },
-- 
1.7.12

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] broadband-modem: disconnect bearers only if the bearer list still exists

2012-09-22 Thread Ben Chan
This patch fixes a crash when MMBroadbandModem tries to access an
already cleared bearer list during the disabling steps.

Thread 0 *CRASHED* ( SIGSEGV @ 0x )

0x7f6eed4c40a3   [ModemManager] - mm-bearer-list.c:259   
mm_bearer_list_disconnect_all_bearers
0x7f6eed4cd6f8   [ModemManager] - mm-iface-modem.c:1216  
handle_enable_auth_ready
0x7f6eed332676   [libgio-2.0.so.0.3000.2]   - gsimpleasyncresult.c:749   
g_simple_async_result_complete
0x7f6eed4c5750   [ModemManager] - mm-base-modem.c:1015   
authorize_ready
0x7f6eed332676   [libgio-2.0.so.0.3000.2]   - gsimpleasyncresult.c:749   
g_simple_async_result_complete
0x7f6eed332788   [libgio-2.0.so.0.3000.2]   - gsimpleasyncresult.c:761   
complete_in_idle_cb
0x7f6eecf36f44   [libglib-2.0.so.0.3000.2]  - gmain.c:2441   
g_main_context_dispatch
0x7f6eecf37597   [libglib-2.0.so.0.3000.2]  - gmain.c:3089   
g_main_context_iterate
0x7f6eecf37b51   [libglib-2.0.so.0.3000.2]  - gmain.c:3297   
g_main_loop_run
0x7f6eed4b5ad1   [ModemManager] - main.c:150 main
0x7f6eec95141c   [libc-2.15.so] - libc-start.c:234   
__libc_start_main
0x7f6eed4b55e8   [ModemManager] + 0x0001a5e8
---
 src/mm-broadband-modem.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 4a2fdb8..64df821 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -6883,11 +6883,15 @@ disabling_step (DisablingContext *ctx)
 ctx-step++;
 
 case DISABLING_STEP_DISCONNECT_BEARERS:
-mm_bearer_list_disconnect_all_bearers (
-ctx-self-priv-modem_bearer_list,
-(GAsyncReadyCallback)bearer_list_disconnect_all_bearers_ready,
-ctx);
-return;
+if (ctx-self-priv-modem_bearer_list) {
+mm_bearer_list_disconnect_all_bearers (
+ctx-self-priv-modem_bearer_list,
+(GAsyncReadyCallback)bearer_list_disconnect_all_bearers_ready,
+ctx);
+return;
+}
+/* Fall down to next step */
+ctx-step++;
 
 case DISABLING_STEP_IFACE_SIMPLE:
 /* Fall down to next step */
-- 
1.7.12

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] glib assertions

2012-09-22 Thread Ben Chan
 = 0x7fcfdeac7970   r12 = 0x0001
r13 = 0x   r14 = 0x
r15 = 0x   rip = 0x7fcfdd7fad7a
rsp = 0x7fff300c41a0   rbp = 0x0001
Found by: call frame info
13  ModemManager!main [main.c : 150 + 0x4]
rbx = 0x7fcfdeabd010   r12 = 0x0001
r13 = 0x   r14 = 0x
r15 = 0x   rip = 0x7fcfddd6a492
rsp = 0x7fff300c41c0   rbp = 0x0001
Found by: call frame info
14  libc-2.15.so!__libc_start_main [libc-start.c : 234 + 0x16]
rbx = 0x   r12 = 0x7fcfddd69f80
r13 = 0x7fff300c42e0   r14 = 0x
r15 = 0x   rip = 0x7fcfdd21141d
rsp = 0x7fff300c4210   rbp = 0x
Found by: call frame info
15  ModemManager + 0x1afa8
rbx = 0x   r12 = 0x7fcfddd69f80
r13 = 0x7fff300c42e0   r14 = 0x
r15 = 0x   rip = 0x7fcfddd69fa9
rsp = 0x7fff300c42d0   rbp = 0x
Found by: call frame info

Thanks,
Ben
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH v2] modem-helpers: handle the case when operator name is Unknown

2012-09-20 Thread Ben Chan
Some modems report Unknown as the operator name when failed to obtain
the actual value:

-- 'AT+COPS=3,0;+COPS?CR'
-- 'CRLF+COPS: 0,0,Unknown,0CRLFCRLFOKCRLF'

This patch prevents Unknown from being treated as a valid operator name.
---
 src/mm-modem-helpers.c |   10 +-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
index db0bfbd..6692b6f 100644
--- a/src/mm-modem-helpers.c
+++ b/src/mm-modem-helpers.c
@@ -1468,7 +1468,15 @@ mm_3gpp_parse_operator (const gchar *reply,
  */
 if (!g_utf8_validate (operator, -1, NULL)) {
 g_free (operator);
-operator = NULL;
+return NULL;
+}
+
+/* Some modems (Novatel LTE) return the operator name as Unknown when
+ * it fails to obtain the operator name. Return NULL in such case.
+ */
+if (g_ascii_strcasecmp (operator, unknown) == 0) {
+g_free (operator);
+return NULL;
 }
 }
 
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [MM] Power down during init

2012-09-19 Thread Ben Chan

 How long? I think the timeout is 3 seconds now so maybe it should be
 increased.


could be up to 30s, which is likely a firmware issue.

- Ben
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[MM] [PATCH] modem-helpers: handle the case when operator name is Unknown

2012-09-19 Thread Ben Chan
Some modems report Unknown as the operator name when failed to obtain
the actual value:

-- 'AT+COPS=3,0;+COPS?CR'
-- 'CRLF+COPS: 0,0,Unknown,0CRLFCRLFOKCRLF'

This patch prevents Unknown from being treated as a valid operator name.
---
 src/mm-modem-helpers.c |   12 
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
index db0bfbd..9aa360d 100644
--- a/src/mm-modem-helpers.c
+++ b/src/mm-modem-helpers.c
@@ -1456,6 +1456,8 @@ mm_3gpp_parse_operator (const gchar *reply,
 }
 
 if (operator) {
+gchar *casefolded_operator;
+
 /* 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.
@@ -1468,8 +1470,18 @@ mm_3gpp_parse_operator (const gchar *reply,
  */
 if (!g_utf8_validate (operator, -1, NULL)) {
 g_free (operator);
+return NULL;
+}
+
+/* Some modems (Novatel LTE) return the operator name as Unknown when
+ * it fails to obtain the operator name. Return NULL in such case.
+ */
+casefolded_operator = g_utf8_casefold (operator, -1);
+if (g_utf8_collate (casefolded_operator, unknown) == 0) {
+g_free (operator);
 operator = NULL;
 }
+g_free (casefolded_operator);
 }
 
 return operator;
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


  1   2   >