remove DeviceProvider from Notifier

3 files changed, 136 insertions, 151 deletions

diff --git a/src/notifier.c b/src/notifier.c
index 897154a..d32008f 100644
--- a/src/notifier.c
+++ b/src/notifier.c
@@ -19,10 +19,7 @@
 
 #include "dbus-battery-info.h"
 #include "dbus-shared.h"
-#include "device.h"
-#include "device-provider.h"
 #include "notifier.h"
-#include "service.h"
 
 #include <libnotify/notify.h>
 
@@ -33,40 +30,44 @@ G_DEFINE_TYPE(IndicatorPowerNotifier,
               indicator_power_notifier,
               G_TYPE_OBJECT)
 
+/**
+***  GObject Properties
+**/
+
 enum
 {
   PROP_0,
-  PROP_DEVICE_PROVIDER,
+  PROP_BATTERY,
   PROP_IS_WARNING,
   PROP_POWER_LEVEL,
   LAST_PROP
 };
 
-#define DEVICE_PROVIDER_NAME "device-provider"
+#define BATTERY_NAME "battery"
 #define IS_WARNING_NAME "is-warning"
 #define POWER_LEVEL_NAME "power-level"
 
 static GParamSpec * properties[LAST_PROP];
 
-static int n_notifiers = 0;
+/**
+***
+**/
 
-typedef enum
-{
-  POWER_LEVEL_OK,
-  POWER_LEVEL_LOW,
-  POWER_LEVEL_VERY_LOW,
-  POWER_LEVEL_CRITICAL
-}
-PowerLevel;
+static int n_notifiers = 0;
 
 struct _IndicatorPowerNotifierPrivate
 {
   IndicatorPowerDeviceProvider * device_provider;
 
+  /* The battery we're currently watching.
+     This may be a physical battery or it may be a "merged" battery
+     synthesized from multiple batteries present on the device.
+     See indicator_power_service_choose_primary_device() */
   IndicatorPowerDevice * battery;
+  PowerLevel power_level;
+
   NotifyNotification* notify_notification;
   gboolean is_warning;
-  PowerLevel power_level;
 
   DbusBattery * dbus_battery;
   GBinding * is_warning_binding;
@@ -77,41 +78,7 @@ struct _IndicatorPowerNotifierPrivate
 typedef IndicatorPowerNotifierPrivate priv_t;
 
 /***
-****
-***/
-
-/* implemented here rather than my_set_property() to guard from public use
-   because this is a read-only property */
-static void
-set_is_warning_property (IndicatorPowerNotifier * self, gboolean is_warning)
-{
-  priv_t * p = self->priv;
-
-  if (p->is_warning != is_warning)
-    {
-      p->is_warning = is_warning;
-
-      g_object_notify_by_pspec (G_OBJECT(self), properties[PROP_IS_WARNING]);
-    }
-}
-
-/* implemented here rather than my_set_property() to guard from public use
-   because this is a read-only property */
-static void
-set_power_level_property (IndicatorPowerNotifier * self, PowerLevel power_level)
-{
-  priv_t * p = self->priv;
-
-  if (p->power_level != power_level)
-    {
-      p->power_level = power_level;
-
-      g_object_notify_by_pspec (G_OBJECT(self), properties[PROP_POWER_LEVEL]);
-    }
-}
-
-/***
-****
+****  Notifications
 ***/
 
 static void
@@ -135,30 +102,38 @@ on_notification_clicked(NotifyNotification * notify_notification G_GNUC_UNUSED,
                         char * action G_GNUC_UNUSED,
                         gpointer gself G_GNUC_UNUSED)
 {
-  /* no-op */
+  /* no-op because notify_notification_add_action() doesn't like a NULL cb */
 }
 
 static void
-notification_show(IndicatorPowerNotifier * self,
-                  IndicatorPowerDevice   * device)
-                   
+notification_show(IndicatorPowerNotifier * self)
 {
-  priv_t * p = self->priv;
+  priv_t * p;
+  IndicatorPowerDevice * battery;
   char * body;
   NotifyNotification * nn;
 
-  // if there's already a notification, tear it down
   notification_clear (self);
 
-  // create the notification
-  body = g_strdup_printf(_("%d%% charge remaining"), (int)indicator_power_device_get_percentage(device));
-  p->notify_notification = nn = notify_notification_new(_("Battery Low"), body, NULL);
-  notify_notification_set_hint(nn, "x-canonical-snap-decisions", g_variant_new_boolean(TRUE));
-  notify_notification_set_hint(nn, "x-canonical-private-button-tint", g_variant_new_boolean(TRUE));
-  notify_notification_add_action(nn, "OK", _("OK"), on_notification_clicked, self, NULL);
+  p = self->priv;
+  battery = p->battery;
+  g_return_if_fail (battery != NULL);
+
+  /* create the notification */
+  body = g_strdup_printf(_("%d%% charge remaining"),
+                         (int)indicator_power_device_get_percentage(battery));
+  p->notify_notification = nn = notify_notification_new(_("Battery Low"),
+                                                        body,
+                                                        NULL);
+  notify_notification_set_hint(nn, "x-canonical-snap-decisions",
+                               g_variant_new_boolean(TRUE));
+  notify_notification_set_hint(nn, "x-canonical-private-button-tint",
+                               g_variant_new_boolean(TRUE));
+  notify_notification_add_action(nn, "OK", _("OK"),
+                                 on_notification_clicked, self, NULL);
   g_signal_connect_swapped(nn, "closed", G_CALLBACK(notification_clear), self);
 
-  // show the notification
+  /* show the notification */
   GError* error = NULL;
   notify_notification_show(nn, &error);
   if (error != NULL)
@@ -174,12 +149,16 @@ notification_show(IndicatorPowerNotifier * self,
   g_free (body);
 }
 
+/***
+****
+***/
+
 static PowerLevel
 get_power_level (const IndicatorPowerDevice * device)
 {
   static const double percent_critical = 2.0;
   static const double percent_very_low = 5.0;
-  static const double percent_low = 48.0;
+  static const double percent_low = 10.0;
   const gdouble p = indicator_power_device_get_percentage(device);
   PowerLevel ret;
 
@@ -195,50 +174,6 @@ get_power_level (const IndicatorPowerDevice * device)
   return ret;
 }
   
-static void
-on_devices_changed(IndicatorPowerNotifier * self)
-{
-  priv_t * p = self->priv;
-  GList * devices;
-  IndicatorPowerDevice * primary;
-
-  /* find the primary battery */
-  devices = indicator_power_device_provider_get_devices (p->device_provider);
-  primary = indicator_power_service_choose_primary_device (devices);
-  g_clear_object (&p->battery);
-  if ((primary != NULL) && (indicator_power_device_get_kind (primary) == UP_DEVICE_KIND_BATTERY))
-    p->battery = g_object_ref (primary);
-  g_clear_object(&primary);
-  g_list_free_full (devices, (GDestroyNotify)g_object_unref);
-
-  /* update our state based on the new primary device */
-  if (p->battery == NULL)
-    {
-      /* if there's no primary battery, put everything in standby mode */
-      set_power_level_property (self, POWER_LEVEL_OK);
-      notification_clear(self);
-    }
-  else
-    {
-      const PowerLevel power_level = get_power_level (p->battery);
-
-      if (p->power_level != power_level)
-        {
-          set_power_level_property (self, power_level);
-
-          /* maybe update the notifications */
-          if ((power_level == POWER_LEVEL_OK) ||
-              (indicator_power_device_get_state(p->battery) != UP_DEVICE_STATE_DISCHARGING))
-            {
-              notification_clear (self);
-            }
-          else
-            {
-              notification_show (self, p->battery);
-            }
-        }
-    }
-}
 
 /***
 ****  GObject virtual functions
@@ -255,8 +190,8 @@ my_get_property (GObject     * o,
 
   switch (property_id)
     {
-      case PROP_DEVICE_PROVIDER:
-        g_value_set_object (value, p->device_provider);
+      case PROP_BATTERY:
+        g_value_set_object (value, p->battery);
         break;
 
       case PROP_POWER_LEVEL:
@@ -282,8 +217,8 @@ my_set_property (GObject       * o,
 
   switch (property_id)
     {
-      case PROP_DEVICE_PROVIDER:
-        indicator_power_notifier_set_device_provider (self, g_value_get_object (value));
+      case PROP_BATTERY:
+        indicator_power_notifier_set_battery (self, g_value_get_object(value));
         break;
 
       default:
@@ -291,17 +226,43 @@ my_set_property (GObject       * o,
     }
 }
 
+/* read-only property, so not implemented in my_set_property() */
+static void
+set_is_warning_property (IndicatorPowerNotifier * self, gboolean is_warning)
+{
+  priv_t * p = self->priv;
+
+  if (p->is_warning != is_warning)
+    {
+      p->is_warning = is_warning;
+
+      g_object_notify_by_pspec (G_OBJECT(self), properties[PROP_IS_WARNING]);
+    }
+}
+
+/* read-only property, so not implemented in my_set_property() */
+static void
+set_power_level_property (IndicatorPowerNotifier * self, PowerLevel power_level)
+{
+  priv_t * p = self->priv;
+
+  if (p->power_level != power_level)
+    {
+      p->power_level = power_level;
+
+      g_object_notify_by_pspec (G_OBJECT(self), properties[PROP_POWER_LEVEL]);
+    }
+}
+
 static void
 my_dispose (GObject * o)
 {
   IndicatorPowerNotifier * self = INDICATOR_POWER_NOTIFIER(o);
   priv_t * p = self->priv;
-g_message ("%s %s dispose %p", G_STRLOC, G_STRFUNC, (void*)o);
-
-  indicator_power_notifier_set_bus(self, NULL);
-  notification_clear(self);
-  indicator_power_notifier_set_device_provider (self, NULL);
 
+  indicator_power_notifier_set_bus (self, NULL);
+  notification_clear (self);
+  indicator_power_notifier_set_device (self, NULL);
   g_clear_pointer (&p->power_level_binding, g_binding_unbind);
   g_clear_pointer (&p->is_warning_binding, g_binding_unbind);
   g_clear_object (&p->dbus_battery);
@@ -310,9 +271,8 @@ g_message ("%s %s dispose %p", G_STRLOC, G_STRFUNC, (void*)o);
 }
 
 static void
-my_finalize (GObject * o)
+my_finalize (GObject * o G_GNUC_UNUSED)
 {
-g_message ("%s %s finalize %p", G_STRLOC, G_STRFUNC, (void*)o);
   if (!--n_notifiers)
     notify_uninit();
 }
@@ -328,12 +288,9 @@ indicator_power_notifier_init (IndicatorPowerNotifier * self)
                                             INDICATOR_TYPE_POWER_NOTIFIER,
                                             IndicatorPowerNotifierPrivate);
   self->priv = p;
-g_message ("%s %s init %p", G_STRLOC, G_STRFUNC, (void*)self);
 
   p->dbus_battery = dbus_battery_skeleton_new ();
 
-  /* FIXME: own the busname and export the skeleton */
-
   p->is_warning_binding = g_object_bind_property (self,
                                                   IS_WARNING_NAME,
                                                   p->dbus_battery,
@@ -364,17 +321,17 @@ indicator_power_notifier_class_init (IndicatorPowerNotifierClass * klass)
 
   properties[PROP_0] = NULL;
 
-  properties[PROP_DEVICE_PROVIDER] = g_param_spec_object (
-    DEVICE_PROVIDER_NAME,
-    "Device Provider",
-    "Source for power devices",
+  properties[PROP_BATTERY] = g_param_spec_object (
+    BATTERY_NAME,
+    "Battery",
+    "The current battery",
     G_TYPE_OBJECT,
     G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
 
   properties[PROP_POWER_LEVEL] = g_param_spec_int (
     POWER_LEVEL_NAME,
     "Power Level",
-    "Power Level of the batteries",
+    "The battery's power level",
     POWER_LEVEL_OK,
     POWER_LEVEL_CRITICAL,
     POWER_LEVEL_OK,
@@ -397,50 +354,65 @@ indicator_power_notifier_class_init (IndicatorPowerNotifierClass * klass)
 IndicatorPowerNotifier *
 indicator_power_notifier_new (IndicatorPowerDeviceProvider * device_provider)
 {
-  GObject * o = g_object_new (INDICATOR_TYPE_POWER_NOTIFIER,
-                              DEVICE_PROVIDER_NAME, device_provider,
-                              NULL);
+  GObject * o = g_object_new (INDICATOR_TYPE_POWER_NOTIFIER, NULL);
 
   return INDICATOR_POWER_NOTIFIER (o);
 }
 
 void
-indicator_power_notifier_set_device_provider(IndicatorPowerNotifier * self,
-                                             IndicatorPowerDeviceProvider * dp)
+indicator_power_notifier_set_battery (IndicatorPowerNotifier * self,
+                                      IndicatorPowerDevice   * battery)
 {
   priv_t * p;
 
   g_return_if_fail(INDICATOR_IS_POWER_NOTIFIER(self));
-  g_return_if_fail(!dp || INDICATOR_IS_POWER_DEVICE_PROVIDER(dp));
-  p = self->priv;
+  g_return_if_fail((battery == NULL) || INDICATOR_IS_POWER_DEVICE(battery));
+  g_return_if_fail((battery == NULL) || (indicator_power_device_get_kind(battery) == UP_DEVICE_KIND_BATTERY));
 
-  if (p->device_provider != NULL)
+  if (p->battery != NULL)
     {
-      g_signal_handlers_disconnect_by_data(p->device_provider, self);
-      g_clear_object(&p->device_provider);
-      g_clear_object(&p->battery);
+      g_clear_object (&p->battery);
+      set_power_level_property (self, POWER_LEVEL_OK);
+      notification_clear (self);
     }
 
-  if (dp != NULL)
+  if (battery != NULL)
     {
-      p->device_provider = g_object_ref(dp);
+      const PowerLevel power_level = get_power_level (battery);
 
-      g_signal_connect_swapped(p->device_provider, "devices-changed",
-                               G_CALLBACK(on_devices_changed), self);
+      p->battery = g_object_ref(battery);
 
-      on_devices_changed(self);
+      if (p->power_level != power_level)
+        {
+          set_power_level_property (self, power_level);
+
+          if ((power_level == POWER_LEVEL_OK) ||
+              (indicator_power_device_get_state(battery) != UP_DEVICE_STATE_DISCHARGING))
+            {
+              notification_clear (self);
+            }
+          else
+            {
+              notification_show (self);
+            }
+        }
     }
 }
 
 void
 indicator_power_notifier_set_bus (IndicatorPowerNotifier * self,
-                                  GDBusConnection * bus)
+                                  GDBusConnection        * bus)
 {
   priv_t * p;
 
   g_return_if_fail(INDICATOR_IS_POWER_NOTIFIER(self));
+  g_return_if_fail((bus == NULL) || G_IS_DBUS_CONNECTION(bus));
+
   p = self->priv;
 
+  if (p->bus == bus)
+    return;
+
   if (p->bus != NULL)
     {
       if (p->dbus_battery != NULL)
diff --git a/src/notifier.h b/src/notifier.h
index c224b29..2602171 100644
--- a/src/notifier.h
+++ b/src/notifier.h
@@ -24,7 +24,7 @@
 #include <glib-object.h>
 #include <gio/gio.h> /* GDBusConnection */
 
-#include "device-provider.h"
+#include "device.h"
 
 G_BEGIN_DECLS
 
@@ -37,6 +37,15 @@ typedef struct _IndicatorPowerNotifier         IndicatorPowerNotifier;
 typedef struct _IndicatorPowerNotifierClass    IndicatorPowerNotifierClass;
 typedef struct _IndicatorPowerNotifierPrivate  IndicatorPowerNotifierPrivate;
 
+typedef enum
+{
+  POWER_LEVEL_OK,
+  POWER_LEVEL_LOW,
+  POWER_LEVEL_VERY_LOW,
+  POWER_LEVEL_CRITICAL
+}
+PowerLevel;
+
 /**
  * The Indicator Power Notifier.
  */
@@ -58,13 +67,13 @@ struct _IndicatorPowerNotifierClass
 
 GType indicator_power_notifier_get_type (void);
 
-IndicatorPowerNotifier * indicator_power_notifier_new (IndicatorPowerDeviceProvider * provider);
+IndicatorPowerNotifier * indicator_power_notifier_new (void);
 
 void indicator_power_notifier_set_bus (IndicatorPowerNotifier  * self,
                                        GDBusConnection         * connection);
 
-void indicator_power_notifier_set_device_provider (IndicatorPowerNotifier       * self,
-                                                   IndicatorPowerDeviceProvider * provider);
+void indicator_power_notifier_set_device (IndicatorPowerNotifier  * self,
+                                          IndicatorPowerDevice    * provider);
 
 
 G_END_DECLS
diff --git a/src/service.c b/src/service.c
index d8f1371..1c1f8f7 100644
--- a/src/service.c
+++ b/src/service.c
@@ -926,6 +926,12 @@ on_devices_changed (IndicatorPowerService * self)
   g_clear_object (&p->primary_device);
   p->primary_device = indicator_power_service_choose_primary_device (p->devices);
 
+  /* update the notifier's battery */
+  if ((p->primary_device != NULL) || (indicator_power_device_get_kind(p->primary_device) == UP_DEVICE_KIND_BATTERY))
+    indicator_power_notifier_set_battery (p->primary_device);
+  else
+    indicator_power_notifier_set_battery (NULL);
+
   /* update the battery-level action's state */
   if (p->primary_device == NULL)
     battery_level = 0;
@@ -1042,7 +1048,7 @@ indicator_power_service_init (IndicatorPowerService * self)
 
   p->settings = g_settings_new ("com.canonical.indicator.power");
 
-  p->notifier = indicator_power_notifier_new (NULL);
+  p->notifier = indicator_power_notifier_new ();
 
   uscreen_proxy = uscreen_get_proxy(&brightness_params);
   if (uscreen_proxy != NULL)
@@ -1145,8 +1151,6 @@ indicator_power_service_set_device_provider (IndicatorPowerService * self,
 
       on_devices_changed (self);
     }
-
-  indicator_power_notifier_set_device_provider (p->notifier, dp);
 }
 
 /* If a device has multiple batteries and uses only one of them at a time,