[Spice-devel] [PATCH][spice-gtk] usb-device-manager: Add spice_usb_device_manager_get_devices_by_classes
Fabiano Fidêncio
fabiano at fidencio.org
Wed Jul 16 08:25:05 PDT 2014
On Wed, Jul 16, 2014 at 4:59 PM, Fabiano Fidêncio <fabiano at fidencio.org>
wrote:
>
>
>
> On Wed, Jul 16, 2014 at 4:49 PM, Marc-André Lureau <mlureau at redhat.com>
> wrote:
>
>> Hi
>>
>> ----- Original Message -----
>> > This function allows the applications to get USB devices filtered by the
>> > devices' classes. The applications that are going to use this function
>> > should do:
>> > - Create a list of SpiceUsbDeviceFilter and set, for each
>> > SpiceUsbDeviceFilter:
>> > - the device class code based on
>> > http://www.usb.org/developers/defined_class
>> > - whether the device is allowed or not to auto connect
>> > The function will create a string in the expcted format, representing
>> the
>> > filter, based on the list provided by the user and then call
>> > spice_usb_device_manager_get_devices_with_filter()
>>
>> I am not sure this kind of utility function belongs in spice-gtk. It
>> doesn't bring much to the existing call.
>>
>> Why is this change needed for?
>>
>
> Basically because the filter rule seems a bit cryptic and would be better
> if we could do that only by the device class code.
> Please, take a look on
> https://bugs.freedesktop.org/show_bug.cgi?id=63807#c2 for more info.
>
As Hans shares the same idea than Marc-André about the patch not bringing
any help to the apps, I'm going to close this bug as WONTFIX and, please,
ignore this patch.
> > ---
>> > doc/reference/spice-gtk-sections.txt | 1 +
>> > gtk/map-file | 1 +
>> > gtk/spice-glib-sym-file | 1 +
>> > gtk/usb-device-manager.c | 47
>> > +++++++++++++++++++++++++++++++++++-
>> > gtk/usb-device-manager.h | 13 ++++++++++
>> > 5 files changed, 62 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/doc/reference/spice-gtk-sections.txt
>> > b/doc/reference/spice-gtk-sections.txt
>> > index caaa92c..03dcec3 100644
>> > --- a/doc/reference/spice-gtk-sections.txt
>> > +++ b/doc/reference/spice-gtk-sections.txt
>> > @@ -298,6 +298,7 @@ SpiceUsbDeviceManagerClass
>> > spice_usb_device_manager_get
>> > spice_usb_device_manager_get_devices
>> > spice_usb_device_manager_get_devices_with_filter
>> > +spice_usb_device_manager_get_devices_by_classes
>> > spice_usb_device_manager_is_device_connected
>> > spice_usb_device_manager_disconnect_device
>> > spice_usb_device_manager_can_redirect_device
>> > diff --git a/gtk/map-file b/gtk/map-file
>> > index 90f14f1..0a31dc0 100644
>> > --- a/gtk/map-file
>> > +++ b/gtk/map-file
>> > @@ -110,6 +110,7 @@ spice_usb_device_manager_connect_device_finish;
>> > spice_usb_device_manager_disconnect_device;
>> > spice_usb_device_manager_get;
>> > spice_usb_device_manager_get_devices;
>> > +spice_usb_device_manager_get_devices_by_classes;
>> > spice_usb_device_manager_get_devices_with_filter;
>> > spice_usb_device_manager_get_type;
>> > spice_usb_device_manager_is_device_connected;
>> > diff --git a/gtk/spice-glib-sym-file b/gtk/spice-glib-sym-file
>> > index 878dd12..3c5a785 100644
>> > --- a/gtk/spice-glib-sym-file
>> > +++ b/gtk/spice-glib-sym-file
>> > @@ -85,6 +85,7 @@ spice_usb_device_manager_connect_device_finish
>> > spice_usb_device_manager_disconnect_device
>> > spice_usb_device_manager_get
>> > spice_usb_device_manager_get_devices
>> > +spice_usb_device_manager_get_devices_by_classes
>> > spice_usb_device_manager_get_devices_with_filter
>> > spice_usb_device_manager_get_type
>> > spice_usb_device_manager_is_device_connected
>> > diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
>> > index 5013b6c..71bffe4 100644
>> > --- a/gtk/usb-device-manager.c
>> > +++ b/gtk/usb-device-manager.c
>> > @@ -154,7 +154,6 @@ typedef struct _SpiceUsbDeviceInfo {
>> > gint ref;
>> > } SpiceUsbDeviceInfo;
>> >
>> > -
>> > static void channel_new(SpiceSession *session, SpiceChannel *channel,
>> > gpointer user_data);
>> > static void channel_destroy(SpiceSession *session, SpiceChannel
>> *channel,
>> > @@ -1379,6 +1378,52 @@ GPtrArray*
>> > spice_usb_device_manager_get_devices(SpiceUsbDeviceManager *self)
>> > }
>> >
>> > /**
>> > + * spice_usb_device_manager_get_devices_by_classes:
>> > + * @manager: the #SpiceUsbDeviceManager manager
>> > + * @classes: (element-type SpiceUsbDeviceFilter) (allow-none): a
>> %GList of
>> > classes for selecting
>> > + * which devices to return or %NULL to use the default filter, that
>> filters
>> > out HID (class 0x03)
>> > + * USB devices from auto connect and auto connects anything else.
>> > + *
>> > + * Returns: (element-type SpiceUsbDevice) (transfer full): a %GPtrArray
>> > array of %SpiceUsbDevice
>> > + */
>>
>> missing Since:
>>
>> > +GPtrArray*
>> > spice_usb_device_manager_get_devices_by_classes(SpiceUsbDeviceManager
>> *self,
>> > + GList
>> *classes)
>>
>> I am not convinced using GList of structs make this function easier to
>> use,
>> a string or a valist could do. Aee also below for my question about
>> struct content.
>>
>
> Maybe a GList of device codes? That would be exactly what I need,
> considering I don't have to set the allow field.
>
> > +{
>> > + GPtrArray *devices;
>> > + GList *l;
>> > + gchar *filter = NULL;
>> > +
>> > + /* Builds the filter based on the USB classes received from the
>> user. As
>> > this function
>> > + filters only by the device class, another fields as vendor id,
>> > product id and device
>> > + version bcd are automatically set to match any id/version.
>> > +
>> > + A filter rule has the form of
>> @class, at vendor, at product, at version, at allow
>> > and the rules,
>> > + themselves, are concatenated like @rule1|@rule2|@rule3 */
>> > + for (l = classes; l != NULL; l = l->next) {
>> > + SpiceUsbDeviceFilter *device_filter = l->data;
>> > + if (filter == NULL) {
>> > + filter = g_strdup_printf(
>> > + "0x%02x,-1,-1,-1,%d",
>> > + device_filter->code, device_filter->allow);
>> > + } else {
>> > + gchar *tmp_filter = NULL;
>> > +
>> > + tmp_filter = g_strdup_printf(
>> > + "%s|0x%02x,-1,-1,-1,%d",
>> > + filter, device_filter->code, device_filter->allow);
>> > +
>> > + g_free(filter);
>> > + filter = tmp_filter;
>> > + }
>> > + }
>> > +
>> > + devices = spice_usb_device_manager_get_devices_with_filter(self,
>> > filter);
>> > + g_free(filter);
>> > +
>> > + return devices;
>> > +}
>> > +
>> > +/**
>> > * spice_usb_device_manager_is_device_connected:
>> > * @manager: the #SpiceUsbDeviceManager manager
>> > * @device: a #SpiceUsbDevice
>> > diff --git a/gtk/usb-device-manager.h b/gtk/usb-device-manager.h
>> > index a7e3515..7a048be 100644
>> > --- a/gtk/usb-device-manager.h
>> > +++ b/gtk/usb-device-manager.h
>> > @@ -40,6 +40,7 @@ typedef struct _SpiceUsbDeviceManagerClass
>> > SpiceUsbDeviceManagerClass;
>> > typedef struct _SpiceUsbDeviceManagerPrivate
>> SpiceUsbDeviceManagerPrivate;
>> >
>> > typedef struct _SpiceUsbDevice SpiceUsbDevice;
>> > +typedef struct _SpiceUsbDeviceFilter SpiceUsbDeviceFilter;
>> >
>> > /**
>> > * SpiceUsbDeviceManager:
>> > @@ -85,6 +86,16 @@ struct _SpiceUsbDeviceManagerClass
>> > gchar _spice_reserved[SPICE_RESERVED_PADDING];
>> > };
>> >
>> > +/**
>> > + * SpiceUsbDeviceFilter:
>> > + * @code: the USB Class Code
>> > + * @allow: whether the device is allowed or not to auto connect
>>
>> I don't get why "allow" is necessary here. It make sense for
>> auto-connect-filter, but to query a list of devices?
>>
>
> Hmmm. I'm not sure if "allow" is necessary. Thinking a bit, probably not.
>
>
>>
>> > + */
>> > +struct _SpiceUsbDeviceFilter {
>> > + guint code;
>> > + gboolean allow;
>> > +};
>> > +
>> > GType spice_usb_device_get_type(void);
>> > GType spice_usb_device_manager_get_type(void);
>> >
>> > @@ -96,6 +107,8 @@ SpiceUsbDeviceManager
>> > *spice_usb_device_manager_get(SpiceSession *session,
>> > GPtrArray *spice_usb_device_manager_get_devices(SpiceUsbDeviceManager
>> > *manager);
>> > GPtrArray* spice_usb_device_manager_get_devices_with_filter(
>> > SpiceUsbDeviceManager *manager, const gchar *filter);
>> > +GPtrArray* spice_usb_device_manager_get_devices_by_classes(
>> > + SpiceUsbDeviceManager *manager, GList *classes);
>> >
>> > gboolean
>> spice_usb_device_manager_is_device_connected(SpiceUsbDeviceManager
>> > *manager,
>> > SpiceUsbDevice
>> > *device);
>> > --
>> > 1.9.3
>> >
>> > _______________________________________________
>> > Spice-devel mailing list
>> > Spice-devel at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
>> >
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>>
>
>
> --
> Fabiano Fidênciou
>
--
Fabiano Fidêncio
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20140716/5513a73d/attachment.html>
More information about the Spice-devel
mailing list