* New upstream release.

  - Show the proper icon for mice and ups devices. (LP: #1066208)

11 files changed, 512 insertions, 228 deletions

diff --git a/ChangeLog b/ChangeLog
index 20338f9..5fcc09a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,81 @@
 # Generated by Makefile. Do not edit.
 
+2012-11-04  Charles Kerr  <charles.kerr@canonical.com>
+
+	12.10.4
+
+2012-11-04  Charles Kerr  <charles.kerr@canonical.com>
+
+	fix logic error in primary device selection; add unit tests to confirm the fix. (lp:~charlesk/indicator-power/lp-1071757)
+
+2012-10-29  Charles Kerr  <charles.kerr@canonical.com>
+
+	copyediting: fix comment text
+
+2012-10-28  Charles Kerr  <charles.kerr@canonical.com>
+
+	in TestDevice, make the log variable names a little more consistent
+
+2012-10-28  Charles Kerr  <charles.kerr@canonical.com>
+
+	set DeviceTest's gwarning/gcritical log func to one that prints no messages but accumulates the log count. This way we can both (a) silence in-console warnings that look like bugs in the test but are actually desirable warnings generated by fuzz testing, and (b) assert that we get exactly as many warnings/criticals as we expect to get.
+
+2012-10-28  Charles Kerr  <charles.kerr@canonical.com>
+
+	expand DeviceTest.ChoosePrimary to test choosing from a wider variety of device states.
+
+2012-10-26  Charles Kerr  <charles.kerr@canonical.com>
+
+	fix logic error when deciding which primary device to select.
+
+2012-10-26  Charles Kerr  <charles.kerr@canonical.com>
+
+	add test (currently failing) to detect the logic error reported in bug #1071757
+
+2012-10-26  Charles Kerr  <charles.kerr@canonical.com>
+
+	choose the correct primary device, and choose the correct icon for it.
+
+2012-10-26  Charles Kerr  <charles.kerr@canonical.com>
+
+	12.10.3
+
+2012-10-26  Charles Kerr  <charles.kerr@canonical.com>
+
+	choose the correct primary device, and choose the correct icon for it.
+
+2012-10-26  Charles Kerr  <charles.kerr@canonical.com>
+
+	fix copy-paste comment error
+
+2012-10-26  Charles Kerr  <charles.kerr@canonical.com>
+
+	fix comment typo
+
+2012-10-26  Charles Kerr  <charles.kerr@canonical.com>
+
+	Select the primary device based on the spec's criteria. Add tests to confirm.
+
+2012-10-26  Charles Kerr  <charles.kerr@canonical.com>
+
+	silence the known/nonbug gtk and glib warnings
+
+2012-10-26  Charles Kerr  <charles.kerr@canonical.com>
+
+	add unit tests to confirm the device refcounts are correct s.t. IndicatorPower isn't leaking devices
+
+2012-10-26  Charles Kerr  <charles.kerr@canonical.com>
+
+	fix leaked icon and label; found by running valgrind on tests/test-indicator
+
+2012-10-26  Charles Kerr  <charles.kerr@canonical.com>
+
+	Fix unhandled enum 'unknown device' that was smoked out by the unit tests.
+
+2012-10-25  Charles Kerr  <charles.kerr@canonical.com>
+
+	the device kind string wasn't being used in some icon names.
+
 2012-10-03  Charles Kerr  <charles.kerr@canonical.com>
 
 	12.10.2
diff --git a/NEWS b/NEWS
index ff89f0e..e068a49 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,11 @@
+12.10.4
+ - Fix primary device selection bug introduced in 12.10.3 (LP: #1071757)
+
+12.10.3
+ - Show the proper icon for mice and ups devices. (LP: #1066208)
+ - Follow the spec when choosing the primary device (LP: #1071645)
+ - Fix minor memory leaks
+
 12.10.2
  - Show a more accurate view of discharging batteries (LP: #1054146)
  - Fix a GIcon memory leak
diff --git a/build-aux/ltmain.sh b/build-aux/ltmain.sh
index c2852d8..0096fe6 100644
--- a/build-aux/ltmain.sh
+++ b/build-aux/ltmain.sh
@@ -70,7 +70,7 @@
 #         compiler:		$LTCC
 #         compiler flags:		$LTCFLAGS
 #         linker:		$LD (gnu? $with_gnu_ld)
-#         $progname:	(GNU libtool) 2.4.2 Debian-2.4.2-1ubuntu1
+#         $progname:	(GNU libtool) 2.4.2 Debian-2.4.2-1ubuntu2
 #         automake:	$automake_version
 #         autoconf:	$autoconf_version
 #
@@ -80,7 +80,7 @@
 
 PROGRAM=libtool
 PACKAGE=libtool
-VERSION="2.4.2 Debian-2.4.2-1ubuntu1"
+VERSION="2.4.2 Debian-2.4.2-1ubuntu2"
 TIMESTAMP=""
 package_revision=1.3337
 
diff --git a/configure b/configure
index 2c42ec3..78ed247 100755
--- a/configure
+++ b/configure
@@ -1,6 +1,6 @@
 #! /bin/sh
 # Guess values for system-dependent variables and create Makefiles.
-# Generated by GNU Autoconf 2.69 for indicator-power 12.10.2.
+# Generated by GNU Autoconf 2.69 for indicator-power 12.10.4.
 #
 # Report bugs to <http://bugs.launchpad.net/indicator-power>.
 #
@@ -590,8 +590,8 @@ MAKEFLAGS=
 # Identity of this package.
 PACKAGE_NAME='indicator-power'
 PACKAGE_TARNAME='indicator-power'
-PACKAGE_VERSION='12.10.2'
-PACKAGE_STRING='indicator-power 12.10.2'
+PACKAGE_VERSION='12.10.4'
+PACKAGE_STRING='indicator-power 12.10.4'
 PACKAGE_BUGREPORT='http://bugs.launchpad.net/indicator-power'
 PACKAGE_URL='http://launchpad.net/indicator-power'
 
@@ -1442,7 +1442,7 @@ if test "$ac_init_help" = "long"; then
   # Omit some internal or obsolete options to make the list less imposing.
   # This message is too long to be a string in the A/UX 3.1 sh.
   cat <<_ACEOF
-\`configure' configures indicator-power 12.10.2 to adapt to many kinds of systems.
+\`configure' configures indicator-power 12.10.4 to adapt to many kinds of systems.
 
 Usage: $0 [OPTION]... [VAR=VALUE]...
 
@@ -1512,7 +1512,7 @@ fi
 
 if test -n "$ac_init_help"; then
   case $ac_init_help in
-     short | recursive ) echo "Configuration of indicator-power 12.10.2:";;
+     short | recursive ) echo "Configuration of indicator-power 12.10.4:";;
    esac
   cat <<\_ACEOF
 
@@ -1666,7 +1666,7 @@ fi
 test -n "$ac_init_help" && exit $ac_status
 if $ac_init_version; then
   cat <<\_ACEOF
-indicator-power configure 12.10.2
+indicator-power configure 12.10.4
 generated by GNU Autoconf 2.69
 
 Copyright (C) 2012 Free Software Foundation, Inc.
@@ -2202,7 +2202,7 @@ cat >config.log <<_ACEOF
 This file contains any messages produced by compilers while
 running configure, to aid debugging if configure makes a mistake.
 
-It was created by indicator-power $as_me 12.10.2, which was
+It was created by indicator-power $as_me 12.10.4, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   $ $0 $@
@@ -3026,7 +3026,7 @@ fi
 
 # Define the identity of the package.
  PACKAGE='indicator-power'
- VERSION='12.10.2'
+ VERSION='12.10.4'
 
 
 cat >>confdefs.h <<_ACEOF
@@ -25191,7 +25191,7 @@ cat >>$CONFIG_STATUS <<\_ACEOF || ac_write_fail=1
 # report actual input values of CONFIG_FILES etc. instead of their
 # values after options handling.
 ac_log="
-This file was extended by indicator-power $as_me 12.10.2, which was
+This file was extended by indicator-power $as_me 12.10.4, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   CONFIG_FILES    = $CONFIG_FILES
@@ -25258,7 +25258,7 @@ _ACEOF
 cat >>$CONFIG_STATUS <<_ACEOF || ac_write_fail=1
 ac_cs_config="`$as_echo "$ac_configure_args" | sed 's/^ //; s/[\\""\`\$]/\\\\&/g'`"
 ac_cs_version="\\
-indicator-power config.status 12.10.2
+indicator-power config.status 12.10.4
 configured by $0, generated by GNU Autoconf 2.69,
   with options \\"\$ac_cs_config\\"
 
diff --git a/configure.ac b/configure.ac
index 5fbf2e5..9a0e06c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
 AC_INIT([indicator-power],
-        [12.10.2],
+        [12.10.4],
         [http://bugs.launchpad.net/indicator-power],
         [indicator-power],
         [http://launchpad.net/indicator-power])
diff --git a/debian/changelog b/debian/changelog
index 495fb03..76ab8f1 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+indicator-power (12.10.4-0ubuntu1) UNRELEASED; urgency=low
+
+  * New upstream release.
+    - Show the proper icon for mice and ups devices. (LP: #1066208)
+
+ -- Sebastien Bacher <seb128@ubuntu.com>  Mon, 05 Nov 2012 12:38:15 +0100
+
 indicator-power (12.10.2-0ubuntu1) quantal; urgency=low
 
   * New upstream release.
diff --git a/src/device.c b/src/device.c
index 898e18f..490ff1a 100644
--- a/src/device.c
+++ b/src/device.c
@@ -334,7 +334,7 @@ indicator_power_device_get_icon_names (const IndicatorPowerDevice * device)
   gdouble percentage = indicator_power_device_get_percentage (device);
   const UpDeviceKind kind = indicator_power_device_get_kind (device);
   const UpDeviceState state = indicator_power_device_get_state (device);
-  const gchar * kind_str = kind_str = up_device_kind_to_string (kind);
+  const gchar * kind_str = up_device_kind_to_string (kind);
 
   GPtrArray * names = g_ptr_array_new ();
 
@@ -351,19 +351,19 @@ indicator_power_device_get_icon_names (const IndicatorPowerDevice * device)
   else switch (state)
     {
       case UP_DEVICE_STATE_EMPTY:
-        g_ptr_array_add (names, g_strdup("battery-empty-symbolic"));
+        g_ptr_array_add (names, g_strdup_printf("%s-empty-symbolic", kind_str));
         g_ptr_array_add (names, g_strdup_printf("gpm-%s-empty", kind_str));
         g_ptr_array_add (names, g_strdup_printf("gpm-%s-000", kind_str));
-        g_ptr_array_add (names, g_strdup("battery-empty"));
+        g_ptr_array_add (names, g_strdup_printf("%s-empty", kind_str));
         break;
 
       case UP_DEVICE_STATE_FULLY_CHARGED:
-        g_ptr_array_add (names, g_strdup("battery-full-charged-symbolic"));
-        g_ptr_array_add (names, g_strdup("battery-full-charging-symbolic"));
+        g_ptr_array_add (names, g_strdup_printf("%s-full-charged-symbolic", kind_str));
+        g_ptr_array_add (names, g_strdup_printf("%s-full-charging-symbolic", kind_str));
         g_ptr_array_add (names, g_strdup_printf("gpm-%s-full", kind_str));
         g_ptr_array_add (names, g_strdup_printf("gpm-%s-100", kind_str));
-        g_ptr_array_add (names, g_strdup("battery-full-charged"));
-        g_ptr_array_add (names, g_strdup("battery-full-charging"));
+        g_ptr_array_add (names, g_strdup_printf("%s-full-charged", kind_str));
+        g_ptr_array_add (names, g_strdup_printf("%s-full-charging", kind_str));
         break;
 
       case UP_DEVICE_STATE_CHARGING:
@@ -374,9 +374,9 @@ indicator_power_device_get_icon_names (const IndicatorPowerDevice * device)
 
         suffix_str = get_device_icon_suffix (percentage);
         index_str = get_device_icon_index (percentage);
-        g_ptr_array_add (names, g_strdup_printf ("battery-%s-charging-symbolic", suffix_str));
+        g_ptr_array_add (names, g_strdup_printf ("%s-%s-charging-symbolic", kind_str, suffix_str));
         g_ptr_array_add (names, g_strdup_printf ("gpm-%s-%s-charging", kind_str, index_str));
-        g_ptr_array_add (names, g_strdup_printf ("battery-%s-charging", suffix_str));
+        g_ptr_array_add (names, g_strdup_printf ("%s-%s-charging", kind_str, suffix_str));
         break;
 
       case UP_DEVICE_STATE_DISCHARGING:
@@ -392,14 +392,14 @@ indicator_power_device_get_icon_names (const IndicatorPowerDevice * device)
         index_str = get_device_icon_index (percentage);
         g_ptr_array_add (names, g_strdup_printf ("%s-%s", kind_str, index_str));
         g_ptr_array_add (names, g_strdup_printf ("gpm-%s-%s", kind_str, index_str));
-        g_ptr_array_add (names, g_strdup_printf ("battery-%s-symbolic", suffix_str));
-        g_ptr_array_add (names, g_strdup_printf ("battery-%s", suffix_str));
+        g_ptr_array_add (names, g_strdup_printf ("%s-%s-symbolic", kind_str, suffix_str));
+        g_ptr_array_add (names, g_strdup_printf ("%s-%s", kind_str, suffix_str));
         break;
 
       default:
-        g_ptr_array_add (names, g_strdup("battery-missing-symbolic"));
-        g_ptr_array_add (names, g_strdup("gpm-battery-missing"));
-        g_ptr_array_add (names, g_strdup("battery-missing"));
+        g_ptr_array_add (names, g_strdup_printf("%s-missing-symbolic", kind_str));
+        g_ptr_array_add (names, g_strdup_printf("gpm-%s-missing", kind_str));
+        g_ptr_array_add (names, g_strdup_printf("%s-missing", kind_str));
     }
 
     g_ptr_array_add (names, NULL); /* terminates the strv */
@@ -528,6 +528,10 @@ device_kind_to_localised_string (UpDeviceKind kind)
       /* TRANSLATORS: tablet device */
       text = _("Computer");
       break;
+    case UP_DEVICE_KIND_UNKNOWN:
+      /* TRANSLATORS: unknown device */
+      text = _("Unknown");
+      break;
     default:
       g_warning ("enum unrecognised: %i", kind);
       text = up_device_kind_to_string (kind);
diff --git a/src/indicator-power.c b/src/indicator-power.c
index 9829839..4118dcc 100644
--- a/src/indicator-power.c
+++ b/src/indicator-power.c
@@ -150,6 +150,8 @@ indicator_power_dispose (GObject *object)
 
   dispose_devices (self);
 
+  g_clear_object (&priv->label);
+  g_clear_object (&priv->status_image);
   g_clear_object (&priv->dbus_listener);
   g_clear_object (&priv->settings);
 
@@ -350,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 (a_state != state) /* b is charging */
+        {
+          ret = 1;
+        }
+      else if (b_state != state) /* a 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
@@ -458,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)
@@ -489,6 +510,7 @@ get_label (IndicatorObject *io)
     {
       /* Create the label if it doesn't exist already */
       priv->label = GTK_LABEL (gtk_label_new (""));
+      g_object_ref_sink (priv->label);
       gtk_widget_set_visible (GTK_WIDGET (priv->label), FALSE);
     }
 
@@ -508,6 +530,7 @@ get_image (IndicatorObject *io)
     gicon = g_themed_icon_new (DEFAULT_ICON);
     priv->status_image = GTK_IMAGE (gtk_image_new_from_gicon (gicon,
                                                               GTK_ICON_SIZE_LARGE_TOOLBAR));
+    g_object_ref_sink (priv->status_image);
     g_object_unref (gicon);
   }
 
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 eb087dc..18bdc08 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
 {
@@ -36,15 +37,40 @@ namespace
 
 class DeviceTest : public ::testing::Test
 {
+  private:
+
+    guint log_handler_id;
+
+    int log_count_ipower_actual;
+
+    static void log_count_func (const gchar *log_domain,
+                                GLogLevelFlags log_level,
+                                const gchar *message,
+                                gpointer user_data)
+    {
+      reinterpret_cast<DeviceTest*>(user_data)->log_count_ipower_actual++;
+    }
+
+  protected:
+
+    int log_count_ipower_expected;
+
   protected:
 
     virtual void SetUp()
     {
+      const GLogLevelFlags flags = GLogLevelFlags(G_LOG_LEVEL_CRITICAL|G_LOG_LEVEL_WARNING);
+      log_handler_id = g_log_set_handler ("Indicator-Power", flags, log_count_func, this);
+      log_count_ipower_expected = 0;
+      log_count_ipower_actual = 0;
+
       ensure_glib_initialized ();
     }
 
     virtual void TearDown()
     {
+      ASSERT_EQ (log_count_ipower_expected, log_count_ipower_actual);
+      g_log_remove_handler ("Indicator-Power", log_handler_id);
     }
 
   protected:
@@ -204,6 +230,7 @@ TEST_F(DeviceTest, BadAccessors)
   indicator_power_device_get_state (device);
   indicator_power_device_get_percentage (device);
   indicator_power_device_get_object_path (device);
+  log_count_ipower_expected += 5;
 
   // test that these functions can handle being passed non-device GObjects
   device = reinterpret_cast<IndicatorPowerDevice*>(g_cancellable_new ());
@@ -212,6 +239,8 @@ TEST_F(DeviceTest, BadAccessors)
   indicator_power_device_get_state (device);
   indicator_power_device_get_percentage (device);
   indicator_power_device_get_object_path (device);
+  log_count_ipower_expected += 5;
+
   g_object_unref (device);
 }
 
@@ -225,6 +254,7 @@ TEST_F(DeviceTest, IconNames)
   GObject * o = G_OBJECT(device);
 
   // bad arguments
+  log_count_ipower_expected++;
   ASSERT_TRUE (indicator_power_device_get_icon_names (NULL) == NULL);
 
   // power
@@ -239,154 +269,205 @@ TEST_F(DeviceTest, IconNames)
   check_icon_names (device, "gpm-monitor-symbolic;"
                             "gpm-monitor");
 
-  // empty battery
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_EMPTY,
-                   NULL);
-  check_icon_names (device, "battery-empty-symbolic;"
-                            "gpm-battery-empty;"
-                            "gpm-battery-000;"
-                            "battery-empty");
-
-  // charged battery
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_FULLY_CHARGED,
-                   NULL);
-  check_icon_names (device, "battery-full-charged-symbolic;"
-                            "battery-full-charging-symbolic;"
-                            "gpm-battery-full;"
-                            "gpm-battery-100;"
-                            "battery-full-charged;"
-                            "battery-full-charging");
-
-  // charging battery, 95%
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_CHARGING,
-                   INDICATOR_POWER_DEVICE_PERCENTAGE, 95.0,
-                   NULL);
-  check_icon_names (device, "battery-caution-charging-symbolic;"
-                            "gpm-battery-000-charging;"
-                            "battery-caution-charging");
-
-  // charging battery, 85%
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_CHARGING,
-                   INDICATOR_POWER_DEVICE_PERCENTAGE, 85.0,
-                   NULL);
-  check_icon_names (device, "battery-caution-charging-symbolic;"
-                            "gpm-battery-000-charging;"
-                            "battery-caution-charging");
-
-  // charging battery, 50%
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_CHARGING,
-                   INDICATOR_POWER_DEVICE_PERCENTAGE, 50.0,
-                   NULL);
-  check_icon_names (device, "battery-caution-charging-symbolic;"
-                            "gpm-battery-000-charging;"
-                            "battery-caution-charging");
-
-  // charging battery, 25%
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_CHARGING,
-                   INDICATOR_POWER_DEVICE_PERCENTAGE, 25.0,
-                   NULL);
-  check_icon_names (device, "battery-caution-charging-symbolic;"
-                            "gpm-battery-000-charging;"
-                            "battery-caution-charging");
-
-  // charging battery, 5%
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_CHARGING,
-                   INDICATOR_POWER_DEVICE_PERCENTAGE, 5.0,
-                   NULL);
-  check_icon_names (device, "battery-caution-charging-symbolic;"
-                            "gpm-battery-000-charging;"
-                            "battery-caution-charging");
-
-
-  // discharging battery, 95%
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
-                   INDICATOR_POWER_DEVICE_PERCENTAGE, 95.0,
-                   NULL);
-  check_icon_names (device, "battery-100;"
-                            "gpm-battery-100;"
-                            "battery-full-symbolic;"
-                            "battery-full");
-
-  // discharging battery, 85%
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
-                   INDICATOR_POWER_DEVICE_PERCENTAGE, 85.0,
-                   NULL);
-  check_icon_names (device, "battery-080;"
-                            "gpm-battery-080;"
-                            "battery-full-symbolic;"
-                            "battery-full");
-
-  // discharging battery, 50% -- 1 hour left
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
-                   INDICATOR_POWER_DEVICE_PERCENTAGE, 50.0,
-                   INDICATOR_POWER_DEVICE_TIME, (guint64)(60*60),
-                   NULL);
-  check_icon_names (device, "battery-060;"
-                            "gpm-battery-060;"
-                            "battery-good-symbolic;"
-                            "battery-good");
-
-  // discharging battery, 25% -- 1 hour left
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
-                   INDICATOR_POWER_DEVICE_PERCENTAGE, 25.0,
-                   INDICATOR_POWER_DEVICE_TIME, (guint64)(60*60),
-                   NULL);
-  check_icon_names (device, "battery-040;"
-                            "gpm-battery-040;"
-                            "battery-good-symbolic;"
-                            "battery-good");
-
-  // discharging battery, 25% -- 15 minutes left
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
-                   INDICATOR_POWER_DEVICE_PERCENTAGE, 25.0,
-                   INDICATOR_POWER_DEVICE_TIME, (guint64)(60*15),
-                   NULL);
-  check_icon_names (device, "battery-020;"
-                            "gpm-battery-020;"
-                            "battery-low-symbolic;"
-                            "battery-low");
+  // devices that hold a charge
+  struct {
+    int kind;
+    const gchar * kind_str;
+  } devices[] = {
+    { UP_DEVICE_KIND_BATTERY, "battery" },
+    { UP_DEVICE_KIND_UPS, "ups" },
+    { UP_DEVICE_KIND_MOUSE, "mouse" },
+    { UP_DEVICE_KIND_KEYBOARD, "keyboard" },
+    { UP_DEVICE_KIND_PHONE, "phone" }
+  };
+
+  GString * expected = g_string_new (NULL);
+  for (int i=0, n=G_N_ELEMENTS(devices); i<n; i++)
+    {
+      const int kind = devices[i].kind;
+      const gchar * kind_str = devices[i].kind_str;
 
-  // discharging battery, 5% -- 1 hour left
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
-                   INDICATOR_POWER_DEVICE_PERCENTAGE, 5.0,
-                   INDICATOR_POWER_DEVICE_TIME, (guint64)(60*60),
+      // empty
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_EMPTY,
+                       NULL);
+ 
+      g_string_append_printf (expected, "%s-empty-symbolic;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-empty;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-000;", kind_str);
+      g_string_append_printf (expected, "%s-empty", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+
+      // charged
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_FULLY_CHARGED,
+                       NULL);
+      g_string_append_printf (expected, "%s-full-charged-symbolic;", kind_str);
+      g_string_append_printf (expected, "%s-full-charging-symbolic;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-full;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-100;", kind_str);
+      g_string_append_printf (expected, "%s-full-charged;", kind_str);
+      g_string_append_printf (expected, "%s-full-charging", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+
+      // charging, 95%
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_CHARGING,
+                       INDICATOR_POWER_DEVICE_PERCENTAGE, 95.0,
+                       NULL);
+
+      g_string_append_printf (expected, "%s-caution-charging-symbolic;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-000-charging;", kind_str);
+      g_string_append_printf (expected, "%s-caution-charging", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+
+      // charging, 85%
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_CHARGING,
+                       INDICATOR_POWER_DEVICE_PERCENTAGE, 85.0,
+                       NULL);
+      g_string_append_printf (expected, "%s-caution-charging-symbolic;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-000-charging;", kind_str);
+      g_string_append_printf (expected, "%s-caution-charging", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+
+      // charging, 50%
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_CHARGING,
+                       INDICATOR_POWER_DEVICE_PERCENTAGE, 50.0,
+                       NULL);
+      g_string_append_printf (expected, "%s-caution-charging-symbolic;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-000-charging;", kind_str);
+      g_string_append_printf (expected, "%s-caution-charging", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+
+      // charging, 25%
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_CHARGING,
+                       INDICATOR_POWER_DEVICE_PERCENTAGE, 25.0,
+                       NULL);
+      g_string_append_printf (expected, "%s-caution-charging-symbolic;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-000-charging;", kind_str);
+      g_string_append_printf (expected, "%s-caution-charging", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+
+      // charging, 5%
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_CHARGING,
+                       INDICATOR_POWER_DEVICE_PERCENTAGE, 5.0,
+                       NULL);
+      g_string_append_printf (expected, "%s-caution-charging-symbolic;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-000-charging;", kind_str);
+      g_string_append_printf (expected, "%s-caution-charging", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+
+      // discharging, 95%
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
+                       INDICATOR_POWER_DEVICE_PERCENTAGE, 95.0,
                    NULL);
-  check_icon_names (device, "battery-040;"
-                            "gpm-battery-040;"
-                            "battery-good-symbolic;"
-                            "battery-good");
-
-  // discharging battery, 5% -- 15 minutes left
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
-                   INDICATOR_POWER_DEVICE_PERCENTAGE, 5.0,
-                   INDICATOR_POWER_DEVICE_TIME, (guint64)(60*15),
+      g_string_append_printf (expected, "%s-100;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-100;", kind_str);
+      g_string_append_printf (expected, "%s-full-symbolic;", kind_str);
+      g_string_append_printf (expected, "%s-full", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+
+      // discharging, 85%
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
+                       INDICATOR_POWER_DEVICE_PERCENTAGE, 85.0,
+                       NULL);
+      g_string_append_printf (expected, "%s-080;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-080;", kind_str);
+      g_string_append_printf (expected, "%s-full-symbolic;", kind_str);
+      g_string_append_printf (expected, "%s-full", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+
+      // discharging, 50% -- 1 hour left
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
+                       INDICATOR_POWER_DEVICE_PERCENTAGE, 50.0,
+                       INDICATOR_POWER_DEVICE_TIME, (guint64)(60*60),
+                       NULL);
+      g_string_append_printf (expected, "%s-060;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-060;", kind_str);
+      g_string_append_printf (expected, "%s-good-symbolic;", kind_str);
+      g_string_append_printf (expected, "%s-good", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+
+      // discharging, 25% -- 1 hour left
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
+                       INDICATOR_POWER_DEVICE_PERCENTAGE, 25.0,
+                       INDICATOR_POWER_DEVICE_TIME, (guint64)(60*60),
+                       NULL);
+      g_string_append_printf (expected, "%s-040;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-040;", kind_str);
+      g_string_append_printf (expected, "%s-good-symbolic;", kind_str);
+      g_string_append_printf (expected, "%s-good", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+
+      // discharging, 25% -- 15 minutes left
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
+                       INDICATOR_POWER_DEVICE_PERCENTAGE, 25.0,
+                       INDICATOR_POWER_DEVICE_TIME, (guint64)(60*15),
+                       NULL);
+      g_string_append_printf (expected, "%s-020;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-020;", kind_str);
+      g_string_append_printf (expected, "%s-low-symbolic;", kind_str);
+      g_string_append_printf (expected, "%s-low", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+
+      // discharging, 5% -- 1 hour left
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
+                       INDICATOR_POWER_DEVICE_PERCENTAGE, 5.0,
+                       INDICATOR_POWER_DEVICE_TIME, (guint64)(60*60),
                    NULL);
-  check_icon_names (device, "battery-000;"
-                            "gpm-battery-000;"
-                            "battery-caution-symbolic;"
-                            "battery-caution"); 
-
-  // state unknown
-  g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
-                   INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_UNKNOWN, 
-                   NULL);
-  check_icon_names (device, "battery-missing-symbolic;"
-                            "gpm-battery-missing;"
-                            "battery-missing");
+      g_string_append_printf (expected, "%s-040;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-040;", kind_str);
+      g_string_append_printf (expected, "%s-good-symbolic;", kind_str);
+      g_string_append_printf (expected, "%s-good", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+
+      // discharging, 5% -- 15 minutes left
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
+                       INDICATOR_POWER_DEVICE_PERCENTAGE, 5.0,
+                       INDICATOR_POWER_DEVICE_TIME, (guint64)(60*15),
+                       NULL);
+      g_string_append_printf (expected, "%s-000;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-000;", kind_str);
+      g_string_append_printf (expected, "%s-caution-symbolic;", kind_str);
+      g_string_append_printf (expected, "%s-caution", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+
+      // state unknown
+      g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_UNKNOWN, 
+                       NULL);
+      g_string_append_printf (expected, "%s-missing-symbolic;", kind_str);
+      g_string_append_printf (expected, "gpm-%s-missing;", kind_str);
+      g_string_append_printf (expected, "%s-missing", kind_str);
+      check_icon_names (device, expected->str);
+      g_string_truncate (expected, 0);
+  }
+  g_string_free (expected, TRUE);
 
   // cleanup
   g_object_unref(o);
@@ -400,9 +481,11 @@ TEST_F(DeviceTest, Labels)
   g_setenv ("LANG", "en_US.UTF-8", TRUE);
 
   // bad args: NULL device
+  log_count_ipower_expected++;
   check_strings (NULL, NULL, NULL, NULL);
 
   // bad args: a GObject that isn't a device
+  log_count_ipower_expected++;
   GObject * o = G_OBJECT(g_cancellable_new());
   check_strings ((IndicatorPowerDevice*)o, NULL, NULL, NULL);
   g_object_unref (o);
@@ -479,3 +562,70 @@ TEST_F(DeviceTest, Labels)
   g_setenv ("LANG", real_lang, TRUE);
   g_free (real_lang);
 }
+
+/* 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 * device_list;
+  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);
+
+  /* device states + time left to {discharge,charge} + % of charge left,
+     sorted in order of preference wrt the spec's criteria.
+     So tests[i] should be picked over any test with an index greater than i */
+  struct {
+    int state;
+    guint64 time;
+    double percentage;
+  } tests[] = {
+    { UP_DEVICE_STATE_DISCHARGING, 49, 50.0 },
+    { UP_DEVICE_STATE_DISCHARGING, 50, 50.0 },
+    { UP_DEVICE_STATE_DISCHARGING, 50, 100.0 },
+    { UP_DEVICE_STATE_DISCHARGING, 51, 50.0 },
+    { UP_DEVICE_STATE_CHARGING, 50, 50.0 },
+    { UP_DEVICE_STATE_CHARGING, 49, 50.0 },
+    { UP_DEVICE_STATE_CHARGING, 49, 100.0 },
+    { UP_DEVICE_STATE_CHARGING, 48, 50.0 },
+    { UP_DEVICE_STATE_FULLY_CHARGED, 0, 50.0 }
+  };
+
+  device_list = NULL;
+  device_list = g_slist_append (device_list, a);
+  device_list = g_slist_append (device_list, b);
+
+  for (int i=0, n=G_N_ELEMENTS(tests); i<n; i++)
+    {
+      for (int j=i+1; j<n; j++)
+        {
+          g_object_set (a, INDICATOR_POWER_DEVICE_STATE, tests[i].state,
+                           INDICATOR_POWER_DEVICE_TIME, guint64(tests[i].time),
+                           INDICATOR_POWER_DEVICE_PERCENTAGE, tests[i].percentage,
+                           NULL);
+          g_object_set (b, INDICATOR_POWER_DEVICE_STATE, tests[j].state,
+                           INDICATOR_POWER_DEVICE_TIME, guint64(tests[j].time),
+                           INDICATOR_POWER_DEVICE_PERCENTAGE, tests[j].percentage,
+                           NULL);
+          ASSERT_EQ (a, indicator_power_choose_primary_device(device_list));
+
+          /* reverse the list to check that list order doesn't matter */
+          device_list = g_slist_reverse (device_list);
+          ASSERT_EQ (a, indicator_power_choose_primary_device(device_list));
+        }
+    }
+    
+  // cleanup
+  g_slist_free_full (device_list, g_object_unref);
+}
diff --git a/tests/test-indicator.cc b/tests/test-indicator.cc
index b9f7321..824a5ac 100644
--- a/tests/test-indicator.cc
+++ b/tests/test-indicator.cc
@@ -29,6 +29,16 @@ with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 namespace
 {
+  void quiet_log_func (const gchar *log_domain,
+                       GLogLevelFlags log_level,
+                       const gchar *message,
+                       gpointer user_data)
+  {
+    // instantiating an indicator w/o a window causes lots
+    // of glib/gtk warnings... silence them so that they don't
+    // obscure any other warnings generated by the tests.
+  }
+
   void ensure_glib_initialized ()
   {
     static bool initialized = false;
@@ -37,6 +47,8 @@ namespace
     {
       initialized = true;
       g_type_init();
+      g_log_set_handler ("Gtk", (GLogLevelFlags)(G_LOG_LEVEL_CRITICAL|G_LOG_LEVEL_WARNING), quiet_log_func, NULL);
+      g_log_set_handler ("GLib-GObject", (GLogLevelFlags)(G_LOG_LEVEL_CRITICAL|G_LOG_LEVEL_WARNING), quiet_log_func, NULL);
     }
   }
 }
@@ -71,6 +83,8 @@ class IndicatorTest : public ::testing::Test
 
     virtual void TearDown()
     {
+      ASSERT_EQ (1, G_OBJECT(battery_device)->ref_count);
+      ASSERT_EQ (1, G_OBJECT(ac_device)->ref_count);
       g_object_unref (battery_device);
       g_object_unref (ac_device);
     }