Re: [MM] [PATCH] cli, libmm-glib: set default timeout on manager DBus proxy

[Aleksander Morgado <aleksander lanedo com>]

On 07/31/2012 01:46 AM, Ben Chan wrote:
> This patch fixes mmcli to set the default timeout on the manager proxy
> interface (MmGdbusOrgFreedesktopModemManager1) instead of MMManager
> itself. The later is not a GDBusProxy object.
> ---
> This is what 'mmcli -L' currently prints out:
> 
>   (mmcli:4689): GLib-GObject-WARNING **: invalid cast from `MMManager' to `GDBusProxy'
> 
>   (mmcli:4689): GLib-GIO-CRITICAL **: g_dbus_proxy_set_default_timeout: assertion `G_IS_DBUS_PROXY (proxy)' failed
> 
> Not sure if it's a good idea to expose the private GDBusProxy interface of
> MMManager. An alternative fix we should consider:
> - Implement 'mm_manager_set_default_timeout(MMManager *, gint timeout)' in libmm-glib/mm-manager.c
> - Implement 'gint mmcli_get_default_timeout()' in cli/mmcli.c
> - Call 'mm_manager_set_default_timeout (ctx->manaager, mmcli_get_default_timeout ())' instead of
>   'mmcli_force_operation_timeout' in cli/mmcli-manager.c
> 

Pushed the fix; thanks.

I added a new commit afterwards so that we have both a _peek() version
(transfer-none) and a _get() version (transfer-full), which is more
aligned with the reset of the code. Also, there shouldn't be any case
where those methods return NULL, as the proxy creation is part of the
sync/async initable sequence in the Manager object, so there won't be a
Manager object if the proxy doesn't get created.

I don't think that exposing the underlying GDBusProxy is a big deal; we
already do it for the Modem objects as they have per-interface proxy
objects. In fact, the user really needs to be able to access it, for
cases like this.


> 
>  cli/mmcli-manager.c     |    4 ++--
>  libmm-glib/mm-manager.c |   14 ++++++++++++++
>  libmm-glib/mm-manager.h |    2 ++
>  3 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/cli/mmcli-manager.c b/cli/mmcli-manager.c
> index 6dd00ac..4db353d 100644
> --- a/cli/mmcli-manager.c
> +++ b/cli/mmcli-manager.c
> @@ -249,7 +249,7 @@ get_manager_ready (GObject      *source,
>      ctx->manager = mmcli_get_manager_finish (result);
>  
>      /* Setup operation timeout */
> -    mmcli_force_operation_timeout (G_DBUS_PROXY (ctx->manager));
> +    mmcli_force_operation_timeout (mm_manager_get_proxy (ctx->manager));
>  
>      /* Request to set log level? */
>      if (set_logging_str) {
> @@ -331,7 +331,7 @@ mmcli_manager_run_synchronous (GDBusConnection *connection)
>      ctx->manager = mmcli_get_manager_sync (connection);
>  
>      /* Setup operation timeout */
> -    mmcli_force_operation_timeout (G_DBUS_PROXY (ctx->manager));
> +    mmcli_force_operation_timeout (mm_manager_get_proxy (ctx->manager));
>  
>      /* Request to set log level? */
>      if (set_logging_str) {
> diff --git a/libmm-glib/mm-manager.c b/libmm-glib/mm-manager.c
> index 3e5c6d0..e1b1105 100644
> --- a/libmm-glib/mm-manager.c
> +++ b/libmm-glib/mm-manager.c
> @@ -131,6 +131,20 @@ mm_manager_new_sync (GDBusConnection                *connection,
>      return (ret ? MM_MANAGER (ret) : NULL);
>  }
>  
> +/**
> + * mm_manager_get_proxy:
> + * @manager: A #MMManager.
> + *
> + * Gets the #GDBusProxy interface of the %manager.
> + *
> + * Returns: (transfer none): The #GDBusProxy interface of %manager or %NULL if the interface hasn't been created. Do not free the returned object, it is owned by @manager.
> + */
> +GDBusProxy *
> +mm_manager_get_proxy (MMManager *manager)
> +{
> +    return G_DBUS_PROXY (manager->priv->manager_iface_proxy);
> +}
> +
>  static void
>  set_logging_ready (MmGdbusOrgFreedesktopModemManager1 *manager_iface_proxy,
>                     GAsyncResult                       *res,
> diff --git a/libmm-glib/mm-manager.h b/libmm-glib/mm-manager.h
> index e95257b..0684075 100644
> --- a/libmm-glib/mm-manager.h
> +++ b/libmm-glib/mm-manager.h
> @@ -67,6 +67,8 @@ MMManager *mm_manager_new_sync (
>      GCancellable                   *cancellable,
>      GError                        **error);
>  
> +GDBusProxy *mm_manager_get_proxy (MMManager *manager);
> +
>  void mm_manager_set_logging (MMManager           *manager,
>                               const gchar         *level,
>                               GCancellable        *cancellable,
> 


-- 
Aleksander