Re: [PATCH 2/2] usb-device-widget: Add counter of free channels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 20, 2016 at 9:59 AM, Victor Toso <lists@xxxxxxxxxxxxxx> wrote:
> Hi,
>
> On Tue, Jan 19, 2016 at 10:36:38PM +0100, Fabiano Fidêncio wrote:
>> As the message showed when the last usbredir channel is taken can be a
>> bit confusing, let's add a counter of free channels to the widget's
>> label.
>> In order to add the counter, two new helper functions got introduced
>> spice_usb_device_manager_get_{free,total}_channels().
>>
>> Related: rhbz#1298772
>> ---
>>  src/map-file             |  2 ++
>>  src/spice-glib-sym-file  |  2 ++
>>  src/usb-device-manager.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  src/usb-device-manager.h |  6 ++++++
>>  src/usb-device-widget.c  | 34 +++++++++++++++++++++++--------
>>  5 files changed, 88 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/map-file b/src/map-file
>> index 62cdb51..babe0be 100644
>> --- a/src/map-file
>> +++ b/src/map-file
>> @@ -130,6 +130,8 @@ spice_usb_device_manager_disconnect_device;
>>  spice_usb_device_manager_get;
>>  spice_usb_device_manager_get_devices;
>>  spice_usb_device_manager_get_devices_with_filter;
>> +spice_usb_device_manager_get_total_channels;
>> +spice_usb_device_manager_get_free_channels;
>>  spice_usb_device_manager_get_type;
>>  spice_usb_device_manager_is_device_connected;
>>  spice_usb_device_widget_get_type;
>> diff --git a/src/spice-glib-sym-file b/src/spice-glib-sym-file
>> index ae365cd..784f50e 100644
>> --- a/src/spice-glib-sym-file
>> +++ b/src/spice-glib-sym-file
>> @@ -107,6 +107,8 @@ spice_usb_device_manager_disconnect_device
>>  spice_usb_device_manager_get
>>  spice_usb_device_manager_get_devices
>>  spice_usb_device_manager_get_devices_with_filter
>> +spice_usb_device_manager_get_total_channels
>> +spice_usb_device_manager_get_free_channels
>>  spice_usb_device_manager_get_type
>>  spice_usb_device_manager_is_device_connected
>>  spice_usbredir_channel_get_type
>> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
>> index a22d926..21d3a25 100644
>> --- a/src/usb-device-manager.c
>> +++ b/src/usb-device-manager.c
>> @@ -1689,6 +1689,58 @@ spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager  *self,
>>  }
>>
>>  /**
>> + * spice_usb_device_manager_get_total_channels:
>> + * @self: the #SpiceUsbDeviceManager manager
>> + *
>> + * Get the total number of redirecting channels.
>> + *
>> + * Returns: The total number of redirecting channels.
>> + */
>> +int
>> +spice_usb_device_manager_get_total_channels(SpiceUsbDeviceManager *self)
>> +{
>> +#ifdef USE_USBREDIR
>> +    SpiceUsbDeviceManagerPrivate *priv = self->priv;
>
> Here you are accessing priv before the check bellow.
>
>> +
>> +    g_return_val_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self), FALSE);
>> +
>> +    return priv->channels->len;
>> +#else
>> +    return 0;
>> +#endif
>> +}
>> +
>> +/**
>> + * spice_usb_device_manager_get_total_channels:
>> + * @self: the #SpiceUsbDeviceManager manager
>> + *
>> + * Get the number of redirecting channels that are available.
>> + *
>> + * Returns: The number of redirecting channels that are available.
>> + */
>> +int
>> +spice_usb_device_manager_get_free_channels(SpiceUsbDeviceManager *self)
>> +{
>> +#ifdef USE_USBREDIR
>> +    SpiceUsbDeviceManagerPrivate *priv = self->priv;
>
> Ditto

I will incorporate these suggestions and send a v2 (as soon as we have
more comments).

>
> Looks good otherwise

I still don't like that much the string being used (%d of %d channels
free) doesn't sound clear enough for me, but I don't have a better
idea either :-\

>
>> +    int i, free_channels = 0;
>> +
>> +    g_return_val_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self), FALSE);
>> +
>> +    for (i = 0; i < priv->channels->len; i++) {
>> +        SpiceUsbredirChannel *channel = g_ptr_array_index(priv->channels, i);
>> +
>> +        if (!spice_usbredir_channel_get_device(channel))
>> +            free_channels++;
>> +    }
>> +
>> +    return free_channels;
>> +#else
>> +    return 0;
>> +#endif
>> +}
>> +
>> +/**
>>   * spice_usb_device_get_description:
>>   * @device: #SpiceUsbDevice to get the description of
>>   * @format: (allow-none): an optional printf() format string with
>> diff --git a/src/usb-device-manager.h b/src/usb-device-manager.h
>> index e05ebae..90fa413 100644
>> --- a/src/usb-device-manager.h
>> +++ b/src/usb-device-manager.h
>> @@ -127,6 +127,12 @@ spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager  *self,
>>                                               SpiceUsbDevice         *device,
>>                                               GError                **err);
>>
>> +int
>> +spice_usb_device_manager_get_total_channels(SpiceUsbDeviceManager *self);
>> +
>> +int
>> +spice_usb_device_manager_get_free_channels(SpiceUsbDeviceManager *self);
>> +
>>  G_END_DECLS
>>
>>  #endif /* __SPICE_USB_DEVICE_MANAGER_H__ */
>> diff --git a/src/usb-device-widget.c b/src/usb-device-widget.c
>> index 830bdce..fffee44 100644
>> --- a/src/usb-device-widget.c
>> +++ b/src/usb-device-widget.c
>> @@ -72,6 +72,7 @@ struct _SpiceUsbDeviceWidgetPrivate {
>>      gchar *device_format_string;
>>      SpiceUsbDeviceManager *manager;
>>      GtkWidget *info_bar;
>> +    GtkWidget *label;
>>      gchar *err_msg;
>>      gsize device_count;
>>      gboolean is_info_message;
>> @@ -182,8 +183,7 @@ static GObject *spice_usb_device_widget_constructor(
>>      SpiceUsbDeviceWidgetPrivate *priv;
>>      GPtrArray *devices = NULL;
>>      GError *err = NULL;
>> -    GtkWidget *label;
>> -    gchar *str;
>> +    gchar *str, *markup_str;
>>      int i;
>>
>>      {
>> @@ -198,12 +198,12 @@ static GObject *spice_usb_device_widget_constructor(
>>      if (!priv->session)
>>          g_error("SpiceUsbDeviceWidget constructed without a session");
>>
>> -    label = gtk_label_new(NULL);
>> -    str = g_strdup_printf("<b>%s</b>", _("Select USB devices to redirect"));
>> -    gtk_label_set_markup(GTK_LABEL (label), str);
>> -    g_free(str);
>> -    gtk_misc_set_alignment(GTK_MISC(label), 0.0, 0.5);
>> -    gtk_box_pack_start(GTK_BOX(self), label, FALSE, FALSE, 0);
>> +    priv->label = gtk_label_new(NULL);
>> +    markup_str = g_strdup_printf("<b>%s</b>", _("Select USB devices to redirect"));
>> +    gtk_label_set_markup(GTK_LABEL (priv->label), markup_str);
>> +    g_free(markup_str);
>> +    gtk_misc_set_alignment(GTK_MISC(priv->label), 0.0, 0.5);
>> +    gtk_box_pack_start(GTK_BOX(self), priv->label, FALSE, FALSE, 0);
>>
>>      priv->manager = spice_usb_device_manager_get(priv->session, &err);
>>      if (err) {
>> @@ -214,6 +214,14 @@ static GObject *spice_usb_device_widget_constructor(
>>          return obj;
>>      }
>>
>> +    str = g_strdup_printf(_("Select USB devices to redirect (%d of %d channels free)"),
>> +                          spice_usb_device_manager_get_free_channels(priv->manager),
>> +                          spice_usb_device_manager_get_total_channels(priv->manager));
>> +    markup_str = g_strdup_printf("<b>%s</b>", str);
>> +    gtk_label_set_markup(GTK_LABEL (priv->label), markup_str);
>> +    g_free(markup_str);
>> +    g_free(str);
>> +
>>      g_signal_connect(priv->manager, "device-added",
>>                       G_CALLBACK(device_added_cb), self);
>>      g_signal_connect(priv->manager, "device-removed",
>> @@ -469,6 +477,7 @@ static void checkbox_clicked_cb(GtkWidget *check, gpointer user_data)
>>      SpiceUsbDeviceWidget *self = SPICE_USB_DEVICE_WIDGET(user_data);
>>      SpiceUsbDeviceWidgetPrivate *priv = self->priv;
>>      SpiceUsbDevice *device;
>> +    gchar *str, *markup_str;
>>
>>      device = g_object_get_data(G_OBJECT(check), "usb-device");
>>
>> @@ -485,6 +494,15 @@ static void checkbox_clicked_cb(GtkWidget *check, gpointer user_data)
>>          spice_usb_device_manager_disconnect_device(priv->manager,
>>                                                     device);
>>      }
>> +
>> +    str = g_strdup_printf(_("Select USB devices to redirect (%d of %d channels free)"),
>> +                          spice_usb_device_manager_get_free_channels(priv->manager),
>> +                          spice_usb_device_manager_get_total_channels(priv->manager));
>> +    markup_str = g_strdup_printf("<b>%s</b>", str);
>> +    gtk_label_set_markup(GTK_LABEL (priv->label), markup_str);
>> +    g_free(markup_str);
>> +    g_free(str);
>> +
>>      spice_usb_device_widget_update_status(self);
>>  }
>>
>> --
>> 2.5.0
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]