add tests to confirm that the DBus object's PowerLevel property changes at the right times (and only at the right times) when the battery is draining

4 files changed, 189 insertions, 115 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d5d6a82..569100d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -63,7 +63,8 @@ add_custom_target (cppcheck COMMAND cppcheck --enable=all -q --error-exitcode=2
 ##
 
 if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
-  set(C_WARNING_ARGS "${C_WARNING_ARGS} -Weverything -Wno-c++98-compat")
+  set(C_WARNING_ARGS "${C_WARNING_ARGS} -Weverything")
+  set(C_WARNING_ARGS "${C_WARNING_ARGS} -Wno-c++98-compat -Wno-padded") # these are annoying
   set(C_WARNING_ARGS "${C_WARNING_ARGS} -Wno-documentation") # gtk-doc != doxygen
 else()
   set(C_WARNING_ARGS "${C_WARNING_ARGS} -Wall -Wextra -Wpedantic -Wformat=2")
diff --git a/src/notifier.c b/src/notifier.c
index 9dd7550..c805413 100644
--- a/src/notifier.c
+++ b/src/notifier.c
@@ -153,28 +153,34 @@ notification_show(IndicatorPowerNotifier * self)
 ****
 ***/
 
-static PowerLevel
-get_power_level (const IndicatorPowerDevice * device)
+static void
+on_battery_property_changed (IndicatorPowerNotifier * self)
 {
-  static const double percent_critical = 2.0;
-  static const double percent_very_low = 5.0;
-  static const double percent_low = 10.0;
+  priv_t * p;
+  PowerLevel power_level;
 
-  const gdouble p = indicator_power_device_get_percentage(device);
-  PowerLevel ret;
+  g_return_if_fail(INDICATOR_IS_POWER_NOTIFIER(self));
+  g_return_if_fail(INDICATOR_IS_POWER_DEVICE(self->priv->battery));
 
-  if (p <= percent_critical)
-    ret = POWER_LEVEL_CRITICAL;
-  else if (p <= percent_very_low)
-    ret = POWER_LEVEL_VERY_LOW;
-  else if (p <= percent_low)
-    ret = POWER_LEVEL_LOW;
-  else
-    ret = POWER_LEVEL_OK;
+  p = self->priv;
 
-  return ret;
+  power_level = indicator_power_notifier_get_power_level (p->battery);
+
+  if (p->power_level != power_level)
+    {
+      set_power_level_property (self, power_level);
+
+      if ((power_level == POWER_LEVEL_OK) ||
+          (indicator_power_device_get_state(p->battery) != UP_DEVICE_STATE_DISCHARGING))
+        {
+          notification_clear (self);
+        }
+      else
+        {
+          notification_show (self);
+        }
+    }
 }
-  
 
 /***
 ****  GObject virtual functions
@@ -263,10 +269,10 @@ my_dispose (GObject * o)
 
   indicator_power_notifier_set_bus (self, NULL);
   notification_clear (self);
-  indicator_power_notifier_set_battery (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);
+  indicator_power_notifier_set_battery (self, NULL);
 
   G_OBJECT_CLASS (indicator_power_notifier_parent_class)->dispose (o);
 }
@@ -327,7 +333,6 @@ indicator_power_notifier_init (IndicatorPowerNotifier * self)
           for (l=caps; l!=NULL && !klass->interactive; l=l->next)
             if (!g_strcmp0 ("actions", (const char*)l->data))
               klass->interactive = TRUE;
-          g_message ("%s klass->interactive is %d", G_STRLOC, (int)klass->interactive);
           g_list_free_full (caps, g_free);
         }
     }
@@ -400,8 +405,12 @@ indicator_power_notifier_set_battery (IndicatorPowerNotifier * self,
 
   p = self->priv;
 
+  if (p->battery == battery)
+    return;
+
   if (p->battery != NULL)
     {
+      g_signal_handlers_disconnect_by_data (p->battery, self);
       g_clear_object (&p->battery);
       set_power_level_property (self, POWER_LEVEL_OK);
       notification_clear (self);
@@ -409,24 +418,12 @@ indicator_power_notifier_set_battery (IndicatorPowerNotifier * self,
 
   if (battery != NULL)
     {
-      const PowerLevel power_level = get_power_level (battery);
-
-      p->battery = g_object_ref(battery);
-
-      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);
-            }
-        }
+      p->battery = g_object_ref (battery);
+      g_signal_connect_swapped (p->battery, "notify::"INDICATOR_POWER_DEVICE_PERCENTAGE,
+                                G_CALLBACK(on_battery_property_changed), self);
+      g_signal_connect_swapped (p->battery, "notify::"INDICATOR_POWER_DEVICE_STATE,
+                                G_CALLBACK(on_battery_property_changed), self);
+      on_battery_property_changed (self);
     }
 }
 
@@ -474,7 +471,28 @@ indicator_power_notifier_set_bus (IndicatorPowerNotifier * self,
     }
 }
 
+PowerLevel
+indicator_power_notifier_get_power_level (IndicatorPowerDevice * battery)
+{
+  static const double percent_critical = 2.0;
+  static const double percent_very_low = 5.0;
+  static const double percent_low = 10.0;
+  gdouble p;
+  PowerLevel ret;
 
+  g_return_val_if_fail(battery != NULL, POWER_LEVEL_OK);
+  g_return_val_if_fail(indicator_power_device_get_kind(battery) == UP_DEVICE_KIND_BATTERY, POWER_LEVEL_OK);
 
+  p = indicator_power_device_get_percentage(battery);
 
+  if (p <= percent_critical)
+    ret = POWER_LEVEL_CRITICAL;
+  else if (p <= percent_very_low)
+    ret = POWER_LEVEL_VERY_LOW;
+  else if (p <= percent_low)
+    ret = POWER_LEVEL_LOW;
+  else
+    ret = POWER_LEVEL_OK;
 
+  return ret;
+}
diff --git a/src/notifier.h b/src/notifier.h
index cab053f..0e3ad99 100644
--- a/src/notifier.h
+++ b/src/notifier.h
@@ -80,6 +80,7 @@ void indicator_power_notifier_set_bus (IndicatorPowerNotifier  * self,
 void indicator_power_notifier_set_battery (IndicatorPowerNotifier  * self,
                                            IndicatorPowerDevice    * battery);
 
+PowerLevel indicator_power_notifier_get_power_level (IndicatorPowerDevice * battery);
 
 G_END_DECLS
 
diff --git a/tests/test-notify.cc b/tests/test-notify.cc
index 6ac3d82..cf1f9d3 100644
--- a/tests/test-notify.cc
+++ b/tests/test-notify.cc
@@ -20,6 +20,7 @@
 
 #include "glib-fixture.h"
 
+#include "dbus-shared.h"
 #include "device.h"
 #include "notifier.h"
 
@@ -49,10 +50,11 @@ private:
   DbusTestService * service = nullptr;
   DbusTestDbusMock * mock = nullptr;
   DbusTestDbusMockObject * obj = nullptr;
-  GDBusConnection * bus = nullptr;
 
 protected:
 
+  GDBusConnection * bus = nullptr;
+
   static constexpr int NOTIFY_ID {1234};
 
   static constexpr int NOTIFICATION_CLOSED_EXPIRED   {1};
@@ -76,15 +78,15 @@ protected:
 
     // init DBusMock / dbus-test-runner
 
-    service = dbus_test_service_new(NULL);
+    service = dbus_test_service_new(nullptr);
 
-    GError * error = NULL;
+    GError * error = nullptr;
     mock = dbus_test_dbus_mock_new(NOTIFY_BUSNAME);
     obj = dbus_test_dbus_mock_get_object(mock, NOTIFY_PATH, NOTIFY_INTERFACE, &error);
     g_assert_no_error (error);
     
     dbus_test_dbus_mock_object_add_method(mock, obj, METHOD_GET_INFO,
-                                          NULL,
+                                          nullptr,
                                           G_VARIANT_TYPE("(ssss)"),
                                           "ret = ('mock-notify', 'test vendor', '1.0', '1.1')", // python
                                           &error);
@@ -102,7 +104,7 @@ protected:
     dbus_test_service_add_task(service, DBUS_TEST_TASK(mock));
     dbus_test_service_start_tasks(service);
 
-    bus = g_bus_get_sync(G_BUS_TYPE_SESSION, NULL, NULL);
+    bus = g_bus_get_sync(G_BUS_TYPE_SESSION, nullptr, nullptr);
     g_dbus_connection_set_exit_on_close(bus, FALSE);
     g_object_add_weak_pointer(G_OBJECT(bus), (gpointer *)&bus);
 
@@ -120,7 +122,7 @@ protected:
     // wait a little while for the scaffolding to shut down,
     // but don't block on it forever...
     unsigned int cleartry = 0;
-    while ((bus != NULL) && (cleartry < 50))
+    while ((bus != nullptr) && (cleartry < 50))
       {
         g_usleep(100000);
         while (g_main_pending())
@@ -136,88 +138,140 @@ protected:
 ****
 ***/
 
-// mock device provider
-
-// send notifications of a device going down from 50% to 3% by 1% increments
-
-// popup should appear exactly twice: once at 10%, once at 5%
-
+// simple test to confirm the NotifyFixture plumbing all works
 TEST_F(NotifyFixture, HelloWorld)
 {
 }
 
-#if 0
-using namespace unity::indicator::datetime;
-
-/***
-****
-***/
-
-using namespace unity::indicator::datetime;
-
-class SnapFixture: public GlibFixture
-{
-
-/***
-****
-***/
 
+// scaffolding to listen for PropertyChanged signals and remember them
 namespace
 {
-  gboolean quit_idle (gpointer gloop)
+  enum
   {
-    g_main_loop_quit(static_cast<GMainLoop*>(gloop));
-    return G_SOURCE_REMOVE;
+    FIELD_POWER_LEVEL = (1<<0),
+    FIELD_IS_WARNING  = (1<<1)
   };
+
+  struct ChangedParams
+  {
+    int32_t power_level = POWER_LEVEL_OK;
+    bool is_warning = false;
+    uint32_t fields = 0;
+  };
+
+  void on_battery_property_changed (GDBusConnection *connection G_GNUC_UNUSED,
+                                    const gchar *sender_name G_GNUC_UNUSED,
+                                    const gchar *object_path G_GNUC_UNUSED,
+                                    const gchar *interface_name G_GNUC_UNUSED,
+                                    const gchar *signal_name G_GNUC_UNUSED,
+                                    GVariant *parameters,
+                                    gpointer gchanged_params)
+  {
+    g_return_if_fail (g_variant_n_children (parameters) == 3);
+    auto changed_properties = g_variant_get_child_value (parameters, 1);
+    g_return_if_fail (g_variant_is_of_type (changed_properties, G_VARIANT_TYPE_DICTIONARY));
+    auto changed_params = static_cast<ChangedParams*>(gchanged_params);
+
+    gint32 power_level;
+    if (g_variant_lookup (changed_properties, "PowerLevel", "i", &power_level, nullptr))
+    {
+      changed_params->power_level = power_level;
+      changed_params->fields |= FIELD_POWER_LEVEL;
+    }
+
+    gboolean is_warning;
+    if (g_variant_lookup (changed_properties, "IsWarning", "b", &is_warning, nullptr))
+    {
+      changed_params->is_warning = is_warning;
+      changed_params->fields |= FIELD_IS_WARNING;
+    }
+
+    g_variant_unref (changed_properties);
+  }
+}
+
+TEST_F(NotifyFixture, PercentageToLevel)
+{
+  auto battery = indicator_power_device_new ("/object/path",
+                                             UP_DEVICE_KIND_BATTERY,
+                                             50.0,
+                                             UP_DEVICE_STATE_DISCHARGING,
+                                             30);
+
+  // confirm that the power levels trigger at the right percentages
+  for (int i=100; i>=0; --i)
+    {
+      g_object_set (battery, INDICATOR_POWER_DEVICE_PERCENTAGE, (gdouble)i, nullptr);
+      const auto level = indicator_power_notifier_get_power_level(battery);
+
+       if (i <= 2)
+         EXPECT_EQ (POWER_LEVEL_CRITICAL, level);
+       else if (i <= 5)
+         EXPECT_EQ (POWER_LEVEL_VERY_LOW, level);
+       else if (i <= 10)
+         EXPECT_EQ (POWER_LEVEL_LOW, level);
+       else
+         EXPECT_EQ (POWER_LEVEL_OK, level);
+     }
+
+  g_object_unref (battery);
 }
 
-TEST_F(SnapFixture, InteractiveDuration)
+
+TEST_F(NotifyFixture, LevelsDuringBatteryDrain)
 {
-  static constexpr int duration_minutes = 120;
-  auto settings = std::make_shared<Settings>();
-  settings->alarm_duration.set(duration_minutes);
-  auto timezones = std::make_shared<Timezones>();
-  auto clock = std::make_shared<LiveClock>(timezones);
-  Snap snap (clock, settings);
-
-  // GetCapabilities returns an array containing 'actions',
-  // so our snap decision will be interactive.
-  // For this test, it means we should get a timeout Notify Hint
-  // that matches duration_minutes
-  GError * error = nullptr;
-  dbus_test_dbus_mock_object_add_method(mock, obj, METHOD_GET_CAPS, nullptr, G_VARIANT_TYPE_STRING_ARRAY, "ret = ['actions', 'body']", &error);
-  g_assert_no_error (error);
-
-  // call the Snap Decision
-  auto func = [this](const Appointment&){g_idle_add(quit_idle, loop);};
-  snap(appt, func, func);
-
-  // confirm that Notify got called once
-  guint len = 0;
-  auto calls = dbus_test_dbus_mock_object_get_method_calls (mock, obj, METHOD_NOTIFY, &len, &error);
-  g_assert_no_error(error);
-  ASSERT_EQ(1, len);
-
-  // confirm that the app_name passed to Notify was APP_NAME
-  const auto& params = calls[0].params;
-  ASSERT_EQ(8, g_variant_n_children(params));
-  const char * str = nullptr;
-  g_variant_get_child (params, 0, "&s", &str);
-  ASSERT_STREQ(APP_NAME, str);
-
-  // confirm that the icon passed to Notify was "alarm-clock"
-  g_variant_get_child (params, 2, "&s", &str);
-  ASSERT_STREQ("alarm-clock", str);
-
-  // confirm that the hints passed to Notify included a timeout matching duration_minutes
-  int32_t i32;
-  bool b;
-  auto hints = g_variant_get_child_value (params, 6);
-  b = g_variant_lookup (hints, HINT_TIMEOUT, "i", &i32);
-  EXPECT_TRUE(b);
-  const auto duration = std::chrono::minutes(duration_minutes);
-  EXPECT_EQ(std::chrono::duration_cast<std::chrono::milliseconds>(duration).count(), i32);
-  g_variant_unref(hints);
+  auto battery = indicator_power_device_new ("/object/path",
+                                             UP_DEVICE_KIND_BATTERY,
+                                             50.0,
+                                             UP_DEVICE_STATE_DISCHARGING,
+                                             30);
+
+  // set up a notifier and give it the battery so changing the battery's
+  // charge should show up on the bus.
+  auto notifier = indicator_power_notifier_new ();
+  indicator_power_notifier_set_battery (notifier, battery);
+  indicator_power_notifier_set_bus (notifier, bus);
+  wait_msec();
+
+  ChangedParams changed_params;
+  auto subscription_tag = g_dbus_connection_signal_subscribe (bus,
+                                                              nullptr,
+                                                              "org.freedesktop.DBus.Properties",
+                                                              "PropertiesChanged",
+                                                              BUS_PATH"/Battery",
+                                                              nullptr,
+                                                              G_DBUS_SIGNAL_FLAGS_NONE,
+                                                              on_battery_property_changed,
+                                                              &changed_params,
+                                                              nullptr);
+
+  // confirm that draining the battery puts
+  // the power_level change through its paces                                                              
+  for (int i=100; i>=0; --i)
+    {
+      changed_params = ChangedParams();
+      EXPECT_TRUE (changed_params.fields == 0);
+
+      const auto old_level = indicator_power_notifier_get_power_level(battery);
+      g_object_set (battery, INDICATOR_POWER_DEVICE_PERCENTAGE, (gdouble)i, nullptr);
+      const auto new_level = indicator_power_notifier_get_power_level(battery);
+      wait_msec();
+
+      if (old_level == new_level)
+        {
+          EXPECT_EQ (0, changed_params.fields);
+        }
+      else
+        {
+          EXPECT_EQ (FIELD_POWER_LEVEL, changed_params.fields);
+          EXPECT_EQ (new_level, changed_params.power_level);
+        }
+    }
+
+  // cleanup
+  g_dbus_connection_signal_unsubscribe (bus, subscription_tag);
+  g_object_unref (notifier);
+  g_object_unref (battery);
 }
-#endif