[Merge] ~phablet-team/plano/+git/modemmanager-snappy-vivid:he910 into ~phablet-team/plano/+git/modemmanager-snappy-vivid:plano

Simon Fels simon.busch at canonical.com
Fri Jan 15 06:12:35 UTC 2016


Review: Needs Fixing

Small comments in line but otherwise LGTM

Diff comments:

> diff --git a/debian/patches/0001-Add-Dell-modem-plugin-for-certain-types.patch b/debian/patches/0001-Add-Dell-modem-plugin-for-certain-types.patch
> index 0a3122a..de3bb15 100644
> --- a/debian/patches/0001-Add-Dell-modem-plugin-for-certain-types.patch
> +++ b/debian/patches/0001-Add-Dell-modem-plugin-for-certain-types.patch
> @@ -199,6 +266,88 @@ Index: build/plugins/dell/mm-broadband-modem-dell.c
>  +}
>  +
>  +/*****************************************************************************/
> ++/* Create Bearer (Modem interface) */
> ++
> ++static unsigned
> ++get_uint_property (GObject *obj, const gchar *property_name)
> ++{
> ++    GValue value = G_VALUE_INIT;
> ++
> ++    g_value_init (&value, G_TYPE_UINT);
> ++    g_object_get_property (obj, property_name, &value);
> ++    return g_value_get_uint (&value);
> ++}
> ++
> ++static MMBaseBearer *
> ++modem_create_bearer_finish (MMIfaceModem *self,
> ++                            GAsyncResult *res,
> ++                            GError **error)
> ++{
> ++    MMBaseBearer *bearer;
> ++    unsigned pid;
> ++
> ++    pid = get_uint_property (G_OBJECT (self), MM_BASE_MODEM_PRODUCT_ID);
> ++
> ++    if (pid == 0x81ba)

Can we add a constant for this?

> ++        return iface_modem_parent->create_bearer_finish (self, res, error);
> ++
> ++    bearer = g_simple_async_result_get_op_res_gpointer (G_SIMPLE_ASYNC_RESULT (res));
> ++    mm_dbg ("New Dell bearer created at DBus path '%s'", mm_base_bearer_get_path (bearer));
> ++
> ++    return g_object_ref (bearer);
> ++}
> ++
> ++static void
> ++broadband_bearer_dell_new_ready (GObject *source,
> ++                                   GAsyncResult *res,
> ++                                   GSimpleAsyncResult *simple)
> ++{
> ++    MMBaseBearer *bearer = NULL;
> ++    GError *error = NULL;
> ++
> ++    bearer = mm_broadband_bearer_dell_new_finish (res, &error);
> ++    if (!bearer)
> ++        g_simple_async_result_take_error (simple, error);
> ++    else
> ++        g_simple_async_result_set_op_res_gpointer (simple,
> ++                                                   bearer,
> ++                                                   (GDestroyNotify)g_object_unref);
> ++    g_simple_async_result_complete (simple);
> ++    g_object_unref (simple);
> ++}
> ++
> ++static void
> ++modem_create_bearer (MMIfaceModem *self,
> ++                     MMBearerProperties *properties,
> ++                     GAsyncReadyCallback callback,
> ++                     gpointer user_data)
> ++{
> ++    GSimpleAsyncResult *result;
> ++    unsigned vid, pid;
> ++
> ++    vid = get_uint_property (G_OBJECT (self), MM_BASE_MODEM_VENDOR_ID);
> ++    pid = get_uint_property (G_OBJECT (self), MM_BASE_MODEM_PRODUCT_ID);
> ++
> ++    mm_dbg ("Creating Dell bearer for VID %x PID %x...", vid, pid);
> ++
> ++    if (pid == 0x81ba) {

See comment above.

> ++        iface_modem_parent->create_bearer (self, properties, callback, user_data);
> ++        return;
> ++    }
> ++
> ++    result = g_simple_async_result_new (G_OBJECT (self),
> ++                                        callback,
> ++                                        user_data,
> ++                                        modem_create_bearer);
> ++
> ++    mm_broadband_bearer_dell_new (MM_BROADBAND_MODEM (self),
> ++                                  properties,
> ++                                  NULL, /* cancellable */
> ++                                  (GAsyncReadyCallback)broadband_bearer_dell_new_ready,
> ++                                  result);
> ++}
> ++
> ++/*****************************************************************************/
>  +
>  +MMBroadbandModemDell *
>  +mm_broadband_modem_dell_new (const gchar *device,
> diff --git a/plugins/dell/mm-broadband-modem-dell.c b/plugins/dell/mm-broadband-modem-dell.c
> index ecc02aa..8325068 100644
> --- a/plugins/dell/mm-broadband-modem-dell.c
> +++ b/plugins/dell/mm-broadband-modem-dell.c
> @@ -147,6 +212,88 @@ disabling_stopped (MMBroadbandModem *self,
>  }
>  
>  /*****************************************************************************/
> +/* Create Bearer (Modem interface) */
> +
> +static unsigned
> +get_uint_property (GObject *obj, const gchar *property_name)
> +{
> +    GValue value = G_VALUE_INIT;
> +
> +    g_value_init (&value, G_TYPE_UINT);
> +    g_object_get_property (obj, property_name, &value);
> +    return g_value_get_uint (&value);
> +}
> +
> +static MMBaseBearer *
> +modem_create_bearer_finish (MMIfaceModem *self,
> +                            GAsyncResult *res,
> +                            GError **error)
> +{
> +    MMBaseBearer *bearer;
> +    unsigned pid;
> +
> +    pid = get_uint_property (G_OBJECT (self), MM_BASE_MODEM_PRODUCT_ID);
> +
> +    if (pid == 0x81ba)
> +        return iface_modem_parent->create_bearer_finish (self, res, error);
> +
> +    bearer = g_simple_async_result_get_op_res_gpointer (G_SIMPLE_ASYNC_RESULT (res));
> +    mm_dbg ("New Dell bearer created at DBus path '%s'", mm_base_bearer_get_path (bearer));
> +
> +    return g_object_ref (bearer);
> +}
> +
> +static void
> +broadband_bearer_dell_new_ready (GObject *source,
> +                                   GAsyncResult *res,
> +                                   GSimpleAsyncResult *simple)
> +{
> +    MMBaseBearer *bearer = NULL;
> +    GError *error = NULL;
> +
> +    bearer = mm_broadband_bearer_dell_new_finish (res, &error);
> +    if (!bearer)
> +        g_simple_async_result_take_error (simple, error);
> +    else
> +        g_simple_async_result_set_op_res_gpointer (simple,
> +                                                   bearer,
> +                                                   (GDestroyNotify)g_object_unref);
> +    g_simple_async_result_complete (simple);
> +    g_object_unref (simple);
> +}
> +
> +static void
> +modem_create_bearer (MMIfaceModem *self,
> +                     MMBearerProperties *properties,
> +                     GAsyncReadyCallback callback,
> +                     gpointer user_data)
> +{
> +    GSimpleAsyncResult *result;
> +    unsigned vid, pid;
> +
> +    vid = get_uint_property (G_OBJECT (self), MM_BASE_MODEM_VENDOR_ID);
> +    pid = get_uint_property (G_OBJECT (self), MM_BASE_MODEM_PRODUCT_ID);
> +
> +    mm_dbg ("Creating Dell bearer for VID %x PID %x...", vid, pid);
> +
> +    if (pid == 0x81ba) {

Needs a constant

> +        iface_modem_parent->create_bearer (self, properties, callback, user_data);
> +        return;
> +    }
> +
> +    result = g_simple_async_result_new (G_OBJECT (self),
> +                                        callback,
> +                                        user_data,
> +                                        modem_create_bearer);
> +
> +    mm_broadband_bearer_dell_new (MM_BROADBAND_MODEM (self),
> +                                  properties,
> +                                  NULL, /* cancellable */
> +                                  (GAsyncReadyCallback)broadband_bearer_dell_new_ready,
> +                                  result);
> +}
> +
> +/*****************************************************************************/
>  
>  MMBroadbandModemDell *
>  mm_broadband_modem_dell_new (const gchar *device,


-- 
https://code.launchpad.net/~phablet-team/plano/+git/modemmanager-snappy-vivid/+merge/282619
Your team Ubuntu Phablet Team is subscribed to branch ~phablet-team/plano/+git/modemmanager-snappy-vivid:plano.



More information about the Ubuntu-reviews mailing list