[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 07:59:54 PDT 2014
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.
> ---
> > 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20140716/8231eb22/attachment-0001.html>
More information about the Spice-devel
mailing list