GNOME.org
 

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

On 02/14/2013 09:08 PM, 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.
---
This patch addresses the comments from Aleksander Morgado <aleksander lanedo com>.

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




Looks better now.

Dan, any further comments? You were concerned about the value of the
timeout IIRC, should we increase it? I don't think there should be any
problem with having a longer timeout, as we're just hiding the real
registration state to the whole system, so a longer timeout here
shouldn't break anything.



 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 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 */
+        g_source_remove (ctx->deferred_update_id);
+        ctx->deferred_update_id = 0;
+        ctx->deferred_new_state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
+    }
+
     /* 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 reloading operator, skip it */
         if (ctx->reloading_operator)
             return;
@@ -1056,27 +1136,29 @@ update_registration_state (MMIfaceModem3gpp *self,
         return;
     }
 
+    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)) {
+        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;
+        ctx->deferred_update_id = g_timeout_add_seconds (
+            DEFERRED_REGISTRATION_STATE_UPDATE_TIMEOUT_SEC,
+            (GSourceFunc)run_deferred_registration_state_update,
+            self);
+        return;
+    }
+
     mm_info ("Modem %s: 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));
 
-    /* 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);
+    update_non_registered_state (self, old_state, new_state);
 }
 
 void




-- 
Aleksander
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]