[gnome-settings-daemon/wip/hadess/low-device-warn-once] power: Don't warn more than once per warning level for devices

[Bastien Nocera <hadess src gnome org>]

commit 5b7b0b2b58e2a077a60ed0b79d55b019bb4253e0
Author: Bastien Nocera <hadess hadess net>
Date:   Fri Nov 20 17:44:09 2020 +0100

    power: Don't warn more than once per warning level for devices
    
    A lot of wireless input devices, such as Logitech mice and touchpads, or
    Bluetooth LE input devices, will disconnect and reconnect frequently
    from the computer when unused.
    
    This causes a problem when the battery on the device is low because
    a new notification will be generated each time the device reconnects, as
    if it was the first time it connected.
    
    Saving the last warning-level for every external peripheral ensures that
    we only emit one low battery notification for each warning-level, when
    the threshold is crossed, rather than at every reconnect.
    
    The last warning-level is not stored, so a new session, or a reboot will
    cause devices to warn again once.
    
    Closes: #108

 plugins/power/gsd-power-manager.c | 50 ++++++++++++++++++++-
 plugins/power/test.py             | 95 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 144 insertions(+), 1 deletion(-)
---
diff --git a/plugins/power/gsd-power-manager.c b/plugins/power/gsd-power-manager.c
index e45a24e7..9d34dce2 100644
--- a/plugins/power/gsd-power-manager.c
+++ b/plugins/power/gsd-power-manager.c
@@ -161,6 +161,7 @@ struct _GsdPowerManager
         NotifyNotification      *notification_low;
         NotifyNotification      *notification_sleep_warning;
         GsdPowerActionType       sleep_action_type;
+        GHashTable              *devices_notified_ht; /* key = serial str, value = UpDeviceLevel */
         gboolean                 battery_is_low; /* laptop battery low, or UPS discharging */
 
         /* Brightness */
@@ -256,10 +257,16 @@ static void
 engine_device_add (GsdPowerManager *manager, UpDevice *device)
 {
         UpDeviceKind kind;
+        UpDeviceLevel warning_level;
+        g_autofree char *serial = NULL;
 
         /* Batteries and UPSes are already handled through
          * the composite battery */
-        g_object_get (device, "kind", &kind, NULL);
+        g_object_get (device,
+                      "kind", &kind,
+                      "warning-level", &warning_level,
+                      "serial", &serial,
+                      NULL);
         if (kind == UP_DEVICE_KIND_BATTERY ||
             kind == UP_DEVICE_KIND_UPS ||
             kind == UP_DEVICE_KIND_LINE_POWER)
@@ -449,6 +456,39 @@ manager_critical_action_stop_sound_cb (GsdPowerManager *manager)
         return FALSE;
 }
 
+static gboolean
+engine_device_should_warn (GsdPowerManager *manager,
+                           UpDeviceKind kind,
+                           UpDeviceLevel warning,
+                           const char *serial)
+{
+        gpointer last_warning_ptr;
+        UpDeviceLevel last_warning;
+        gboolean ret = TRUE;
+
+        if (!serial)
+                return TRUE;
+
+        if (kind == UP_DEVICE_KIND_BATTERY ||
+            kind == UP_DEVICE_KIND_UPS)
+                return TRUE;
+
+        if (g_hash_table_lookup_extended (manager->devices_notified_ht, serial,
+                                          NULL, &last_warning_ptr)) {
+                last_warning = GPOINTER_TO_INT (last_warning_ptr);
+
+                if (last_warning >= warning)
+                        ret = FALSE;
+        }
+
+        if (warning != UP_DEVICE_LEVEL_UNKNOWN)
+                g_hash_table_insert (manager->devices_notified_ht,
+                                     g_strdup (serial),
+                                     GINT_TO_POINTER (warning));
+
+        return ret;
+}
+
 static void
 engine_charge_low (GsdPowerManager *manager, UpDevice *device)
 {
@@ -894,14 +934,19 @@ engine_charge_action (GsdPowerManager *manager, UpDevice *device)
 static void
 engine_device_warning_changed_cb (UpDevice *device, GParamSpec *pspec, GsdPowerManager *manager)
 {
+        g_autofree char *serial = NULL;
         UpDeviceLevel warning;
         UpDeviceKind kind;
 
         g_object_get (device,
+                      "serial", &serial,
                       "warning-level", &warning,
                       "kind", &kind,
                       NULL);
 
+        if (!engine_device_should_warn (manager, kind, warning, serial))
+                return;
+
         if (warning == UP_DEVICE_LEVEL_DISCHARGING) {
                 g_debug ("** EMIT: discharging");
                 engine_ups_discharging (manager, device);
@@ -2571,6 +2616,8 @@ on_rr_screen_acquired (GObject      *object,
                                   manager);
 
         manager->devices_array = g_ptr_array_new_with_free_func (g_object_unref);
+        manager->devices_notified_ht = g_hash_table_new_full (g_str_hash, g_str_equal,
+                                                              g_free, NULL);
 
         /* create a fake virtual composite battery */
         manager->device_composite = up_client_get_display_device (manager->up_client);
@@ -2815,6 +2862,7 @@ gsd_power_manager_stop (GsdPowerManager *manager)
 
         g_clear_pointer (&manager->devices_array, g_ptr_array_unref);
         g_clear_object (&manager->device_composite);
+        g_clear_pointer (&manager->devices_notified_ht, g_hash_table_destroy);
 
         g_clear_object (&manager->screensaver_proxy);
 
diff --git a/plugins/power/test.py b/plugins/power/test.py
index 557ae7d0..7bb190a1 100755
--- a/plugins/power/test.py
+++ b/plugins/power/test.py
@@ -942,6 +942,101 @@ class PowerPluginTest6(PowerPluginBase):
         # verify notification
         self.assertRegex(notify_log, b'[0-9.]+ Notify "Power" .* ".*" ".*Wireless mouse .*low.* 
power.*\([0-9.]+%\).*"')
 
+    def test_notify_device_spam(self):
+        '''no repeat notifications for device batteries'''
+
+        # Set internal battery to discharging
+        self.set_composite_battery_discharging()
+
+        # Add a device battery
+        bat2_path = '/org/freedesktop/UPower/devices/' + 'mock_MOUSE_BAT1'
+        self.obj_upower.AddObject(bat2_path,
+                                  'org.freedesktop.UPower.Device',
+                                  {
+                                      'PowerSupply': dbus.Boolean(False, variant_level=1),
+                                      'IsPresent': dbus.Boolean(True, variant_level=1),
+                                      'Model': dbus.String('Bat1', variant_level=1),
+                                      'Serial': dbus.String('12345678', variant_level=1),
+                                      'Percentage': dbus.Double(10.0, variant_level=1),
+                                      'State': dbus.UInt32(UPowerGlib.DeviceState.DISCHARGING, 
variant_level=1),
+                                      'Type': dbus.UInt32(UPowerGlib.DeviceKind.MOUSE, variant_level=1),
+                                      'WarningLevel': dbus.UInt32(UPowerGlib.DeviceLevel.LOW, 
variant_level=1),
+                                   }, dbus.Array([], signature='(ssss)'))
+
+        obj_bat2 = self.system_bus_con.get_object('org.freedesktop.UPower', bat2_path)
+        self.obj_upower.EmitSignal('', 'DeviceAdded', 'o', [bat2_path],
+                                   dbus_interface='org.freedesktop.DBus.Mock')
+        time.sleep(1)
+
+        self.check_plugin_log('EMIT: charge-low', 2)
+        time.sleep(0.5)
+
+        # we should have gotten a notification by now
+        notify_log = self.p_notify.stdout.read()
+
+        # verify notification
+        self.assertRegex(notify_log, b'[0-9.]+ Notify "Power" .* ".*" ".*Wireless mouse .*low.* 
power.*\([0-9.]+%\).*"')
+
+        # Disconnect mouse
+        self.obj_upower.RemoveObject(bat2_path)
+        time.sleep(0.5)
+
+        # Reconnect mouse
+        self.obj_upower.AddObject(bat2_path,
+                                  'org.freedesktop.UPower.Device',
+                                  {
+                                      'PowerSupply': dbus.Boolean(False, variant_level=1),
+                                      'IsPresent': dbus.Boolean(True, variant_level=1),
+                                      'Model': dbus.String('Bat1', variant_level=1),
+                                      'Serial': dbus.String('12345678', variant_level=1),
+                                      'Percentage': dbus.Double(10.0, variant_level=1),
+                                      'State': dbus.UInt32(UPowerGlib.DeviceState.DISCHARGING, 
variant_level=1),
+                                      'Type': dbus.UInt32(UPowerGlib.DeviceKind.MOUSE, variant_level=1),
+                                      'WarningLevel': dbus.UInt32(UPowerGlib.DeviceLevel.LOW, 
variant_level=1),
+                                   }, dbus.Array([], signature='(ssss)'))
+
+        obj_bat2 = self.system_bus_con.get_object('org.freedesktop.UPower', bat2_path)
+        self.obj_upower.EmitSignal('', 'DeviceAdded', 'o', [bat2_path],
+                                   dbus_interface='org.freedesktop.DBus.Mock')
+        time.sleep(1)
+
+        # we shouldn't have gotten a notification by now
+        notify_log = self.p_notify.stdout.read()
+        self.assertIsNone(notify_log)
+
+        # Disconnect mouse
+        self.obj_upower.RemoveObject(bat2_path)
+        time.sleep(0.5)
+
+        # Reconnect mouse with critical battery level
+        self.obj_upower.AddObject(bat2_path,
+                                  'org.freedesktop.UPower.Device',
+                                  {
+                                      'PowerSupply': dbus.Boolean(False, variant_level=1),
+                                      'IsPresent': dbus.Boolean(True, variant_level=1),
+                                      'Model': dbus.String('Bat1', variant_level=1),
+                                      'Serial': dbus.String('12345678', variant_level=1),
+                                      'Percentage': dbus.Double(5.0, variant_level=1),
+                                      'State': dbus.UInt32(UPowerGlib.DeviceState.DISCHARGING, 
variant_level=1),
+                                      'Type': dbus.UInt32(UPowerGlib.DeviceKind.MOUSE, variant_level=1),
+                                      'WarningLevel': dbus.UInt32(UPowerGlib.DeviceLevel.CRITICAL, 
variant_level=1),
+                                   }, dbus.Array([], signature='(ssss)'))
+
+        obj_bat2 = self.system_bus_con.get_object('org.freedesktop.UPower', bat2_path)
+        self.obj_upower.EmitSignal('', 'DeviceAdded', 'o', [bat2_path],
+                                   dbus_interface='org.freedesktop.DBus.Mock')
+        time.sleep(1)
+
+        # Verify new warning
+        self.check_plugin_log('EMIT: charge-critical', 2)
+        time.sleep(0.5)
+
+        # we should have gotten a notification by now
+        notify_log = self.p_notify.stdout.read()
+
+        # verify notification
+        self.assertRegex(notify_log, b'[0-9.]+ Notify "Power" .* ".*" ".*Wireless mouse .*very low.* 
power.*\([0-9.]+%\).*"')
+
     def test_notify_device_battery_coarse_level(self):
         '''critical power level notification for device batteries with coarse level'''