After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 668611 - Warn about unknown devices
Warn about unknown devices
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: wacom
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Joaquim Rocha
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2012-01-24 20:25 UTC by Bastien Nocera
Modified: 2013-05-28 16:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (2.87 KB, patch)
2012-06-06 16:14 UTC, Olivier Fourdan
needs-work Details | Review
wacom: Warn when devices use the fallback configuration (3.10 KB, patch)
2013-04-25 15:50 UTC, Joaquim Rocha
none Details | Review
wacom: Warn when devices use the fallback configuration (3.15 KB, patch)
2013-04-25 15:50 UTC, Joaquim Rocha
needs-work Details | Review
wacom: Notify user when a device is using the fallback configuration (3.05 KB, patch)
2013-05-02 12:33 UTC, Joaquim Rocha
needs-work Details | Review
wacom: Notify user when a device is using the fallback configuration (4.36 KB, patch)
2013-05-22 12:03 UTC, Joaquim Rocha
none Details | Review
wacom: Warn when devices use the fallback configuration (2.75 KB, patch)
2013-05-28 09:16 UTC, Joaquim Rocha
none Details | Review
wacom: Warn when devices use the fallback configuration (3.18 KB, patch)
2013-05-28 09:17 UTC, Joaquim Rocha
accepted-commit_now Details | Review
wacom: Notify user when a device is using the fallback configuration (3.18 KB, patch)
2013-05-28 10:45 UTC, Joaquim Rocha
committed Details | Review
wacom: Warn when devices use the fallback configuration (2.75 KB, patch)
2013-05-28 11:15 UTC, Joaquim Rocha
committed Details | Review

Description Bastien Nocera 2012-01-24 20:25:57 UTC
When we fallback, warn. But do it just once.
Comment 1 Olivier Fourdan 2012-06-06 16:14:02 UTC
Created attachment 215755 [details] [review]
Proposed patch

Adds the notification for unknown tablet in Wacom panel.

Requires gsd-wacom-device.[ch] from bug #677562
Comment 2 Bastien Nocera 2012-06-08 09:55:03 UTC
Review of attachment 215755 [details] [review]:

I'm fairly certain we don't want a dialogue popping up here.
Comment 3 Bastien Nocera 2013-04-04 12:36:29 UTC
Maintainer change
Comment 4 Joaquim Rocha 2013-04-22 10:15:15 UTC
(In reply to comment #2)
> Review of attachment 215755 [details] [review]:
> 
> I'm fairly certain we don't want a dialogue popping up here.

Should this be a desktop notification instead?
Comment 5 Joaquim Rocha 2013-04-25 15:50:44 UTC
Created attachment 242426 [details] [review]
wacom: Warn when devices use the fallback configuration

It just warns once per device.
Comment 6 Joaquim Rocha 2013-04-25 15:50:56 UTC
Created attachment 242427 [details] [review]
wacom: Warn when devices use the fallback configuration

It just warns once per device.
Comment 7 Joaquim Rocha 2013-04-25 15:52:21 UTC
Sorry for the two equal patches but I was having fun with git-bz and in the first one I had it so it wouldn't attach the bug URL.
Comment 8 Jakub Steiner 2013-04-30 13:47:52 UTC
I don't know how to test this, but a warning dialog that is in the way is certainly not something we'd want. 

Does this mean we have no controls for the device? If the purpose for this is to communicate some of the controls on the panel *might* not work as expected, I'd rather see this communicated in the device name "Unknown Wacom tablet" "Unknown Tablet Model" or somesuch, rather than a nag screen.
Comment 9 Joaquim Rocha 2013-04-30 14:16:50 UTC
Hi Jakub, what I wanted to know is whether a desktop notification is a good way to warn the user.
The title and body for this could be "Unknown Tablet Connected // The device may not work as expected." or something like that.
Comment 10 Jakub Steiner 2013-04-30 14:18:33 UTC
So this happens when you attach the tablet rather than when you first go to configure it? Then yes, a system notification is appropriate.
Comment 11 Joaquim Rocha 2013-05-02 12:33:42 UTC
Created attachment 243038 [details] [review]
wacom: Notify user when a device is using the fallback configuration

It notifies the user only once per device.
Comment 12 Bastien Nocera 2013-05-21 13:01:46 UTC
Review of attachment 243038 [details] [review]:

::: plugins/wacom/gsd-wacom-manager.c
@@ +959,3 @@
+{
+        gchar *msg_body;
+        NotifyNotification *notification;

line feed here.

@@ +960,3 @@
+        gchar *msg_body;
+        NotifyNotification *notification;
+        msg_body = g_strdup_printf (_("The \"%s\" device may not "

One line.

@@ +961,3 @@
+        NotifyNotification *notification;
+        msg_body = g_strdup_printf (_("The \"%s\" device may not "
+                                      "work as expected."),

It's an unknown model, but we already recognised it as a wacom tablet.

@@ +963,3 @@
+                                      "work as expected."),
+                                    device_name);
+        notification = notify_notification_new (_("Unknown Tablet Connected"),

One line.

@@ +969,3 @@
+        notify_notification_set_urgency (notification, NOTIFY_URGENCY_NORMAL);
+        notify_notification_set_app_name (notification, _("Wacom Settings"));
+        notify_notification_show (notification, NULL);

Definitely some cleaning up to do when the notification gets closed.
Comment 13 Bastien Nocera 2013-05-21 13:08:22 UTC
Review of attachment 242427 [details] [review]:

::: plugins/wacom/gsd-wacom-manager.c
@@ +970,3 @@
+                warned_devices = manager->priv->warned_devices;
+
+                if (device_name != NULL &&

Why not check the device_name at the top of this block? You're not adding the device to the hash table if it's empty anyway.

Are you also sure that the device name isn't a fallback device name itself? Eg. having 2 different unknown tablets could have the same device name, no?

@@ +972,3 @@
+                if (device_name != NULL &&
+                    !g_hash_table_contains (warned_devices, device_name)) {
+                        g_warning ("Using the fallback configuration for "

It's not a fallback configuration, it's a fallback definition.

@@ +977,3 @@
+                                   device_name);
+                        g_hash_table_insert (warned_devices,
+                                             (gpointer) g_strdup (device_name),

No need to cast to gpointer.
Comment 14 Joaquim Rocha 2013-05-22 12:03:20 UTC
Created attachment 245029 [details] [review]
wacom: Notify user when a device is using the fallback configuration

New version addressing Bastien's comments and rebased with master.
Comment 15 Joaquim Rocha 2013-05-28 09:16:20 UTC
Created attachment 245437 [details] [review]
wacom: Warn when devices use the fallback configuration
Comment 16 Joaquim Rocha 2013-05-28 09:17:07 UTC
Created attachment 245438 [details] [review]
wacom: Warn when devices use the fallback configuration
Comment 17 Bastien Nocera 2013-05-28 10:34:34 UTC
Review of attachment 245438 [details] [review]:

Fine.
Comment 18 Joaquim Rocha 2013-05-28 10:45:19 UTC
Created attachment 245445 [details] [review]
wacom: Notify user when a device is using the fallback configuration
Comment 19 Joaquim Rocha 2013-05-28 11:15:13 UTC
Created attachment 245449 [details] [review]
wacom: Warn when devices use the fallback configuration

Re-attaching the right warn patch because I have mistakenly attached it before.
This is already pushed after Bastien had marked it as so before I messed up the attachment.