Re: [MM] [PATCH v3] iface-modem-3gpp: defer registration state update when registration is lost
I've rebased patch v3 on top of HEAD, so that patch v4 can be applied cleanly on HEAD. Dan, how do you think about the timeout? Thanks, Ben On Thu, Feb 14, 2013 at 11:48 PM, Aleksander Morgado wrote: > > 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. > > ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
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
> .
>
> 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_REGI
[MM] [PATCH v3] iface-modem-3gpp: defer registration state update when registration is lost
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
.
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 defer
