Re: [PATCH 6/7] time: Poll for timezone

[Dan Williams <dcbw redhat com>]

On Fri, 2012-01-20 at 15:21 -0500, Thomas Tuttle wrote:
> Signed-off-by: Thomas Tuttle <ttuttle chromium org>
> ---
>  src/mm-modem-base.c |   88 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/mm-modem-base.h |    3 ++
>  src/mm-modem-time.c |   15 +++++++++
>  src/mm-modem-time.h |    8 +++++
>  4 files changed, 114 insertions(+), 0 deletions(-)
> 
> diff --git a/src/mm-modem-base.c b/src/mm-modem-base.c
> index e50e13f..dd1420e 100644
> --- a/src/mm-modem-base.c
> +++ b/src/mm-modem-base.c
> @@ -80,6 +80,9 @@ typedef struct {
>      GHashTable *ports;
>  
>      GHashTable *tz_data;
> +    gboolean tz_poll_running;

Do we really need tz_poll_running?  As long as tz_poll_source is cleared
to zero when the poll is stopped, you could just check for
(tz_poll_source > 0) to find out if the poll is running instead.  Also,
could we rename that to tz_poll_id instead?  _source (at least to me)
implies that it's a GSource which is slightly lower-level mechanism for
adding timeout/idle handlers (ie with g_timeout_source_new()) that's
overkill for this application.

Dan

> +    guint tz_poll_source;
> +    guint tz_poll_count;
>  } MMModemBasePrivate;
>  
> 
> @@ -503,6 +506,80 @@ value_destroy (gpointer data)
>      g_slice_free (GValue, v);
>  }
>  
> +static void
> +timezone_poll_done (MMModem *modem, GError *error, gpointer user_data)
> +{
> +    /* do nothing; modem will call mm_modem_base_set_network_timezone if
> +       it has timezone data, and then we will stop nagging it. */
> +}
> +
> +static gboolean
> +timezone_poll_callback (gpointer data)
> +{
> +    MMModemBase *self = (MMModemBase *) data;
> +    MMModemBasePrivate *priv = MM_MODEM_BASE_GET_PRIVATE (self);
> +    gboolean result;
> +
> +    if (priv->tz_poll_count == 0)
> +        goto stop_polling;
> +
> +    priv->tz_poll_count--;
> +    result = mm_modem_time_poll_network_timezone (MM_MODEM_TIME (self),
> +                                                  timezone_poll_done,
> +                                                  NULL);
> +    if (!result)
> +        goto stop_polling;
> +
> +    return TRUE;
> +
> +stop_polling:
> +    mm_modem_base_set_network_timezone (self, NULL, NULL, NULL);
> +    return FALSE;
> +}
> +
> +#define TIMEZONE_POLL_INTERVAL_SEC 5
> +#define TIMEZONE_POLL_RETRIES 6
> +
> +void
> +mm_modem_base_set_network_timezone_polling (MMModemBase *self,
> +                                            gboolean should_poll)
> +{
> +    MMModemBasePrivate *priv;
> +
> +    g_return_if_fail (self != NULL);
> +    g_return_if_fail (MM_IS_MODEM_BASE (self));
> +
> +    priv = MM_MODEM_BASE_GET_PRIVATE (self);
> +
> +    if (should_poll == priv->tz_poll_running)
> +        return;
> +
> +    if (should_poll) {
> +        priv->tz_poll_count = TIMEZONE_POLL_RETRIES;
> +        priv->tz_poll_source = g_timeout_add_seconds (TIMEZONE_POLL_INTERVAL_SEC,
> +                                                      timezone_poll_callback,
> +                                                      self);
> +    } else
> +        g_source_remove (priv->tz_poll_source);
> +
> +    priv->tz_poll_running = should_poll;
> +}
> +
> +static void
> +modem_state_changed (MMModemBase *self, GParamSpec *pspec, gpointer user_data)
> +{
> +    MMModemState state;
> +    gboolean registered;
> +
> +    state = mm_modem_get_state (MM_MODEM (self));
> +    registered = (state >= MM_MODEM_STATE_REGISTERED);
> +
> +    mm_modem_base_set_network_timezone_polling (self, registered);
> +
> +    if (!registered)
> +        mm_modem_base_set_network_timezone (self, NULL, NULL, NULL);
> +}
> +
>  void
>  mm_modem_base_set_network_timezone (MMModemBase *self, gint *offset,
>                                      gint *dst_offset, gint *leap_seconds)
> @@ -530,6 +607,8 @@ mm_modem_base_set_network_timezone (MMModemBase *self, gint *offset,
>          g_hash_table_remove (priv->tz_data, "leap_seconds");
>  
>      g_object_notify (G_OBJECT (self), MM_MODEM_TIME_NETWORK_TIMEZONE);
> +
> +    mm_modem_base_set_network_timezone_polling (self, FALSE);
>  }
>  
>  /*************************************************************************/
> @@ -782,6 +861,7 @@ mm_modem_base_init (MMModemBase *self)
>  
>      priv->ports = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_object_unref);
>      priv->tz_data = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, value_destroy);
> +    priv->tz_poll_running = FALSE;
>  
>      mm_properties_changed_signal_register_property (G_OBJECT (self),
>                                                      MM_MODEM_ENABLED,
> @@ -815,6 +895,11 @@ mm_modem_base_init (MMModemBase *self)
>                                                      MM_MODEM_TIME_NETWORK_TIMEZONE,
>                                                      NULL,
>                                                      MM_DBUS_INTERFACE_MODEM_TIME);
> +
> +    g_signal_connect (self,
> +                      "notify::" MM_MODEM_STATE,
> +                      G_CALLBACK (modem_state_changed),
> +                      NULL);
>  }
>  
>  static void
> @@ -990,6 +1075,9 @@ finalize (GObject *object)
>  
>      mm_auth_provider_cancel_for_owner (priv->authp, object);
>  
> +    if (priv->tz_poll_running)
> +        g_source_remove (priv->tz_poll_source);
> +
>      g_hash_table_destroy (priv->ports);
>      g_hash_table_destroy (priv->tz_data);
>      g_free (priv->driver);
> diff --git a/src/mm-modem-base.h b/src/mm-modem-base.h
> index a7b8afe..a386bd7 100644
> --- a/src/mm-modem-base.h
> +++ b/src/mm-modem-base.h
> @@ -81,6 +81,9 @@ void mm_modem_base_set_network_timezone (MMModemBase *self,
>                                           gint *dst_offset,
>                                           gint *leap_seconds);
>  
> +void mm_modem_base_set_network_timezone_polling (MMModemBase *self,
> +                                                 gboolean should_poll);
> +
>  const char *mm_modem_base_get_manf (MMModemBase *self);
>  void        mm_modem_base_set_manf (MMModemBase *self, const char *manf);
>  
> diff --git a/src/mm-modem-time.c b/src/mm-modem-time.c
> index a1878c8..1bc5f27 100644
> --- a/src/mm-modem-time.c
> +++ b/src/mm-modem-time.c
> @@ -87,6 +87,21 @@ mm_modem_time_get_network_time (MMModemTime *self,
>          async_get_call_not_supported (self, callback, user_data);
>  }
>  
> +gboolean
> +mm_modem_time_poll_network_timezone (MMModemTime *self,
> +                                     MMModemFn callback,
> +                                     gpointer user_data)
> +{
> +    g_return_val_if_fail (MM_IS_MODEM_TIME (self), FALSE);
> +    g_return_val_if_fail (callback != NULL, FALSE);
> +
> +    if (MM_MODEM_TIME_GET_INTERFACE (self)->poll_network_timezone)
> +        return MM_MODEM_TIME_GET_INTERFACE (self)->poll_network_timezone
> +                (self, callback, user_data);
> +    else
> +        return FALSE;
> +}
> +
>  static void
>  impl_modem_time_get_network_time (MMModemTime *self,
>                                    DBusGMethodInvocation *context)
> diff --git a/src/mm-modem-time.h b/src/mm-modem-time.h
> index 485e234..eb37520 100644
> --- a/src/mm-modem-time.h
> +++ b/src/mm-modem-time.h
> @@ -41,6 +41,10 @@ struct _MMModemTime {
>      void (*get_network_time) (MMModemTime *self,
>                                MMModemTimeGetNetworkTimeFn callback,
>                                gpointer user_data);
> +
> +    gboolean (*poll_network_timezone) (MMModemTime *self,
> +                                       MMModemFn callback,
> +                                       gpointer user_data);
>  };
>  
>  GType mm_modem_time_get_type (void);
> @@ -49,4 +53,8 @@ void mm_modem_time_get_network_time (MMModemTime *self,
>                                       MMModemTimeGetNetworkTimeFn callback,
>                                       gpointer user_data);
>  
> +gboolean mm_modem_time_poll_network_timezone (MMModemTime *self,
> +                                              MMModemFn callback,
> +                                              gpointer user_data);
> +
>  #endif /* MM_MODEM_TIME_H */