[Spice-devel] [PATCH v2 1/2] usb-device-{manager, widget}: Add counter of free channels

Fabiano FidĂȘncio fidencio at redhat.com
Thu Jan 21 11:58:39 PST 2016


On Thu, Jan 21, 2016 at 8:46 PM, Jonathon Jongsma <jjongsma at redhat.com> wrote:
> On Thu, 2016-01-21 at 19:09 +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, a new property for SpiceUsbDeviceManager
>> was introduced ("free-channels").
>>
>> Related: rhbz#1298772
>> ---
>>  src/usb-device-manager.c | 28 ++++++++++++++++++++++++++++
>>  src/usb-device-widget.c  | 37 ++++++++++++++++++++++++++++---------
>>  2 files changed, 56 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
>> index c62f56e..9e0e785 100644
>> --- a/src/usb-device-manager.c
>> +++ b/src/usb-device-manager.c
>> @@ -89,6 +89,7 @@ enum {
>>      PROP_AUTO_CONNECT,
>>      PROP_AUTO_CONNECT_FILTER,
>>      PROP_REDIRECT_ON_CONNECT,
>> +    PROP_FREE_CHANNELS,
>>  };
>>
>>  enum
>> @@ -390,6 +391,19 @@ static void spice_usb_device_manager_get_property(GObject
>>      *gobject,
>>      case PROP_REDIRECT_ON_CONNECT:
>>          g_value_set_string(value, priv->redirect_on_connect);
>>          break;
>> +    case PROP_FREE_CHANNELS: {
>> +        int free_channels = 0;
>> +#if USE_USBREDIR
>> +        for (int 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++;
>> +        }
>> +#endif
>> +        g_value_set_int(value, free_channels);
>> +        break;
>> +    }
>>      default:
>>          G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
>>          break;
>> @@ -550,6 +564,20 @@ static void
>> spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
>>                                      pspec);
>>
>>      /**
>> +     * SpiceUsbDeviceManager:n-free-channels:
>> +     *
>> +     * Get a list of avaialable channels for redirecting USB devices.
>> +     */
>> +    pspec = g_param_spec_int("free-channels", "Free channels",
>> +               "The number of available channels for redirecting USB
>> devices",
>> +               0,
>> +               G_MAXINT,
>> +               0,
>> +               G_PARAM_READABLE);
>> +    g_object_class_install_property(gobject_class, PROP_FREE_CHANNELS,
>> +                                    pspec);
>> +
>> +    /**
>>       * SpiceUsbDeviceManager::device-added:
>>       * @manager: the #SpiceUsbDeviceManager that emitted the signal
>>       * @device: #SpiceUsbDevice boxed object corresponding to the added
>> device
>> diff --git a/src/usb-device-widget.c b/src/usb-device-widget.c
>> index fe983c9..0866adb 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;
>>  };
>> @@ -181,9 +182,8 @@ static GObject *spice_usb_device_widget_constructor(
>>      SpiceUsbDeviceWidgetPrivate *priv;
>>      GPtrArray *devices = NULL;
>>      GError *err = NULL;
>> -    GtkWidget *label;
>> -    gchar *str;
>> -    int i;
>> +    gchar *str, *markup_str;
>> +    int i, free_channels;
>>
>>      {
>>          /* Always chain up to the parent constructor */
>> @@ -197,12 +197,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) {
>> @@ -213,6 +213,14 @@ static GObject *spice_usb_device_widget_constructor(
>>          return obj;
>>      }
>>
>> +    g_object_get(priv->manager, "free-channels", &free_channels, NULL);
>> +    str = g_strdup_printf(_("Select USB devices to redirect (%d channels
>> free)"),
>
> This doesn't work very well with all values of %d. For instance, in english, "1
> channels" isn't correct. We probably should use ngettext here.
>
>> +                          free_channels);
>> +    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",
>> @@ -463,6 +471,8 @@ 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;
>> +    int free_channels;
>>
>>      device = g_object_get_data(G_OBJECT(check), "usb-device");
>>
>> @@ -479,6 +489,15 @@ static void checkbox_clicked_cb(GtkWidget *check,
>> gpointer user_data)
>>          spice_usb_device_manager_disconnect_device(priv->manager,
>>                                                     device);
>>      }
>> +
>> +    g_object_get(priv->manager, "free-channels", &free_channels, NULL);
>> +    str = g_strdup_printf(_("Select USB devices to redirect (%d channels
>> free)"),
>> +                          free_channels);
>> +    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);
>
> Here we're basically duplicating what you did in the constructor. What about
> moving this into the spice_usb_device_widget_update_status() function so that
> it's not duplicated?

I will do the changes and submit a v3 (with this patch only ...)

>
>>  }
>>
>
> Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>
>


More information about the Spice-devel mailing list