Select the primary device based on the spec's criteria. Add tests to confirm.

3 files changed, 152 insertions, 56 deletions

diff --git a/src/indicator-power.c b/src/indicator-power.c
index ebfc2c1..852ccd5 100644
--- a/src/indicator-power.c
+++ b/src/indicator-power.c
@@ -352,76 +352,95 @@ build_menu (IndicatorPower *self)
   gtk_widget_show_all (GTK_WIDGET (priv->menu));
 }
 
-static IndicatorPowerDevice*
-get_primary_device (GSList * devices)
+/* sort devices from most interesting to least interesting on this criteria:
+   1. discharging items from least time remaining until most time remaining
+   2. discharging items with an unknown time remaining
+   3. charging items from most time left to charge to least time left to charge
+   4. charging items with an unknown time remaining
+   5. everything else */
+static gint
+device_compare_func (gconstpointer ga, gconstpointer gb)
 {
-  IndicatorPowerDevice * primary_device = NULL;
-  IndicatorPowerDevice * primary_device_charging = NULL;
-  IndicatorPowerDevice * primary_device_discharging = NULL;
-  gboolean charging = FALSE;
-  gboolean discharging = FALSE;
-  guint64 min_discharging_time = G_MAXUINT64;
-  guint64 max_charging_time = 0;
-  GSList * l;
-
-  for (l=devices; l!=NULL; l=l->next)
+  int ret;
+  int state;
+  const IndicatorPowerDevice * a = INDICATOR_POWER_DEVICE(ga);
+  const IndicatorPowerDevice * b = INDICATOR_POWER_DEVICE(gb);
+  const int a_state = indicator_power_device_get_state (a);
+  const int b_state = indicator_power_device_get_state (b);
+  const gdouble a_percentage = indicator_power_device_get_percentage (a);
+  const gdouble b_percentage = indicator_power_device_get_percentage (b);
+  const time_t a_time = indicator_power_device_get_time (a);
+  const time_t b_time = indicator_power_device_get_time (b);
+
+  ret = 0;
+
+  state = UP_DEVICE_STATE_DISCHARGING;
+  if (!ret && ((a_state == state) || (b_state == state)))
     {
-      IndicatorPowerDevice * device = INDICATOR_POWER_DEVICE(l->data);
-      const UpDeviceKind kind = indicator_power_device_get_kind (device);
-      const UpDeviceState state = indicator_power_device_get_state (device);
-      const gdouble percentage  = indicator_power_device_get_percentage (device);
-      const time_t time = indicator_power_device_get_time (device);
-
-      /* Try to fix the case when we get a empty battery bay as a real battery */
-      if (state == UP_DEVICE_STATE_UNKNOWN &&
-          percentage == 0)
-        continue;
-
-      /* not battery */
-      if (kind != UP_DEVICE_KIND_BATTERY)
-        continue;
-
-      if (state == UP_DEVICE_STATE_DISCHARGING)
+      if (a_state != state) /* b is discharging */
         {
-          discharging = TRUE;
-          if (time < min_discharging_time)
-            {
-              min_discharging_time = time;
-              primary_device_discharging = device;
-            }
+          ret = 1;
         }
-      else if (state == UP_DEVICE_STATE_CHARGING)
+      else if (b_state != state) /* a is discharging */
         {
-          charging = TRUE;
-          if (time == 0) /* Battery broken */
-            {
-              primary_device_charging = device;
-            }
-          if (time > max_charging_time)
-            {
-              max_charging_time = time;
-              primary_device_charging = device;
-            }
+          ret = -1;
         }
-      else
+      else /* both are discharging; least-time-left goes first */
         {
-          primary_device = device;
+          if (!a_time || !b_time) /* known time always trumps unknown time */
+            ret = a_time ? -1 : 1;
+          else if (a_time != b_time)
+            ret = a_time < b_time ? -1 : 1;
+          else
+            ret = a_percentage < b_percentage ? -1 : 1;
         }
     }
 
-  if (discharging)
+  state = UP_DEVICE_STATE_CHARGING;
+  if (!ret && (((a_state == state) && a_time) || ((b_state == state) && b_time)))
     {
-      primary_device = primary_device_discharging;
+      if (b_state != state) /* a is charging */
+        {
+          ret = 1;
+        }
+      if (a_state != state) /* b is charging */
+        {
+          ret = -1;
+        }
+      else /* both are discharging; most-time-to-charge goes first */
+        {
+          if (!a_time || !b_time) /* known time always trumps unknown time */
+            ret = a_time ? -1 : 1;
+          else if (a_time != b_time)
+            ret = a_time > b_time ? -1 : 1;
+          else
+            ret = a_percentage < b_percentage ? -1 : 1;
+        }
     }
-  else if (charging)
+
+  if (!ret)
+    ret = a_state - b_state;
+
+  return ret;
+}
+
+IndicatorPowerDevice *
+indicator_power_choose_primary_device (GSList * devices)
+{
+  IndicatorPowerDevice * primary = NULL;
+
+  if (devices != NULL)
     {
-      primary_device = primary_device_charging;
-    }
+      GSList * tmp;
 
-  if (primary_device != NULL)
-    g_object_ref (primary_device);
+      tmp = g_slist_copy (devices);
+      tmp = g_slist_sort (tmp, device_compare_func);
+      primary = g_object_ref (tmp->data);
+
+      g_slist_free (tmp);
+    }
 
-  return primary_device;
+  return primary;
 }
 
 static void
@@ -460,7 +479,7 @@ indicator_power_set_devices (IndicatorPower * self, GSList * devices)
   g_slist_foreach (devices, (GFunc)g_object_ref, NULL);
   dispose_devices (self);
   priv->devices = g_slist_copy (devices);
-  priv->device = get_primary_device (priv->devices);
+  priv->device = indicator_power_choose_primary_device (devices);
 
   /* and our menus/visibility from the new device list */
   if (priv->device != NULL)
diff --git a/src/indicator-power.h b/src/indicator-power.h
index a696b40..34d0d0e 100644
--- a/src/indicator-power.h
+++ b/src/indicator-power.h
@@ -55,4 +55,6 @@ GType indicator_power_get_type (void) G_GNUC_CONST;
 void indicator_power_set_devices (IndicatorPower         * power,
                                   GSList                 * devices);
 
+IndicatorPowerDevice* indicator_power_choose_primary_device (GSList * devices);
+
 G_END_DECLS
diff --git a/tests/test-device.cc b/tests/test-device.cc
index 4c6a6f2..c310a58 100644
--- a/tests/test-device.cc
+++ b/tests/test-device.cc
@@ -19,6 +19,7 @@ with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 #include <gtest/gtest.h>
 #include "device.h"
+#include "indicator-power.h"
 
 namespace
 {
@@ -530,3 +531,77 @@ TEST_F(DeviceTest, Labels)
   g_setenv ("LANG", real_lang, TRUE);
   g_free (real_lang);
 }
+
+static void
+set_device_charge_state (IndicatorPowerDevice * device, int state, int time, double pct)
+{
+  g_object_set (device, INDICATOR_POWER_DEVICE_STATE, state,
+                        INDICATOR_POWER_DEVICE_PERCENTAGE, pct,
+                        INDICATOR_POWER_DEVICE_TIME, guint64(time),
+                        NULL);
+}
+
+
+/* The menu title should tell you at a glance what you need to know most: what
+   device will lose power soonest (and optionally when), or otherwise which
+   device will take longest to charge (and optionally how long it will take). */
+TEST_F(DeviceTest, ChoosePrimary)
+{
+  GSList * devices;
+  IndicatorPowerDevice * a;
+  IndicatorPowerDevice * b;
+
+  a = indicator_power_device_new ("/org/freedesktop/UPower/devices/mouse",
+                                  UP_DEVICE_KIND_MOUSE,
+                                  0.0,
+                                  UP_DEVICE_STATE_DISCHARGING,
+                                  0);
+  b = indicator_power_device_new ("/org/freedesktop/UPower/devices/battery",
+                                  UP_DEVICE_KIND_BATTERY,
+                                  0.0,
+                                  UP_DEVICE_STATE_DISCHARGING,
+                                  0);
+
+  devices = NULL;
+  devices = g_slist_append (devices, a);
+  devices = g_slist_append (devices, b);
+
+  /* Both discharging, same charge %, different times left before empty.
+     Confirm that the one with less time is chosen. */
+  set_device_charge_state (a, UP_DEVICE_STATE_DISCHARGING,  99, 50.0);
+  set_device_charge_state (b, UP_DEVICE_STATE_DISCHARGING, 100, 50.0);
+  ASSERT_EQ (a, indicator_power_choose_primary_device(devices));
+
+  /* Both discharging, different charge % and times left.
+     Confirm that the one with less time is chosen. */
+  set_device_charge_state (a, UP_DEVICE_STATE_DISCHARGING,  99, 50.0);
+  set_device_charge_state (b, UP_DEVICE_STATE_DISCHARGING, 100, 49.0);
+  ASSERT_EQ (a, indicator_power_choose_primary_device(devices));
+
+  /* Both discharging, different charge %, same times left.
+     Confirm that the one with less charge is chosen. */
+  set_device_charge_state (a, UP_DEVICE_STATE_DISCHARGING, 100, 49.0);
+  set_device_charge_state (b, UP_DEVICE_STATE_DISCHARGING, 100, 50.0);
+  ASSERT_EQ (a, indicator_power_choose_primary_device(devices));
+
+  /* Both are charging, have the same charge percentage, and differnt times left (to charge).
+   * Confirm that the one with the most time left is chosen. */
+  set_device_charge_state (a, UP_DEVICE_STATE_CHARGING,     49, 50.0);
+  set_device_charge_state (b, UP_DEVICE_STATE_CHARGING,     50, 50.0);
+  ASSERT_EQ (b, indicator_power_choose_primary_device(devices));
+
+  /* Both are charging, with different charges and time left.
+     Confirm that the one with the most time left is chosen. */
+  set_device_charge_state (a, UP_DEVICE_STATE_CHARGING,     49, 50.0);
+  set_device_charge_state (b, UP_DEVICE_STATE_CHARGING,     50, 49.0);
+  ASSERT_EQ (b, indicator_power_choose_primary_device(devices));
+
+  /* Both are charging, have the same time left, and different charges.
+   * Confirm that the one with less charge is chosen. */
+  set_device_charge_state (a, UP_DEVICE_STATE_CHARGING,     50, 50.0);
+  set_device_charge_state (b, UP_DEVICE_STATE_CHARGING,     50, 49.0);
+  ASSERT_EQ (b, indicator_power_choose_primary_device(devices));
+
+  // cleanup
+  g_slist_free_full (devices, g_object_unref);
+}