From 066f1fb2eb23c4abce7ad9e56f9ecbc600ccddde Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 16 Jul 2014 01:06:59 -0500 Subject: in device.h, add #include to pick up needed GIcon declaration --- src/device.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/device.h b/src/device.h index 3a10f89..77d34ef 100644 --- a/src/device.h +++ b/src/device.h @@ -25,6 +25,7 @@ License along with this library. If not, see #define __INDICATOR_POWER_DEVICE_H__ #include +#include /* GIcon */ G_BEGIN_DECLS -- cgit v1.2.3 From 74b8522acb04f547cf291d33c237478a98f6e5d8 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 16 Jul 2014 01:07:29 -0500 Subject: in debian/control and CMake files, add libnotify --- CMakeLists.txt | 1 + debian/control | 1 + 2 files changed, 2 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9f80294..e9acbf1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -37,6 +37,7 @@ pkg_check_modules(SERVICE_DEPS REQUIRED gio-2.0>=2.36 gio-unix-2.0>=2.36 gudev-1.0>=204 + libnotify>=0.7.6 url-dispatcher-1>=1) include_directories (SYSTEM ${SERVICE_DEPS_INCLUDE_DIRS}) diff --git a/debian/control b/debian/control index e72d68f..68df8bc 100644 --- a/debian/control +++ b/debian/control @@ -6,6 +6,7 @@ Build-Depends: debhelper (>= 9), dh-autoreconf, autopoint, intltool, + libnotify-dev (>= 0.7.6), libgtest-dev, libglib2.0-dev (>= 2.36), libgudev-1.0-dev, -- cgit v1.2.3 From 6ea2db56307d04ac8dba4bcf929cd716f4840359 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 16 Jul 2014 01:08:54 -0500 Subject: add first draft of low battery notifications --- src/CMakeLists.txt | 1 + src/main.c | 12 +- src/notifier.c | 336 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/notifier.h | 68 +++++++++++ tests/CMakeLists.txt | 2 + tests/test-notify.cc | 59 +++++++++ 6 files changed, 474 insertions(+), 4 deletions(-) create mode 100644 src/notifier.c create mode 100644 src/notifier.h create mode 100644 tests/test-notify.cc diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a39b945..4747b12 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -10,6 +10,7 @@ set(SERVICE_MANUAL_SOURCES ib-brightness-uscreen-control.c device-provider.c device.c + notifier.c service.c) # generated sources diff --git a/src/main.c b/src/main.c index 7363284..70b7f2a 100644 --- a/src/main.c +++ b/src/main.c @@ -25,6 +25,7 @@ #include "device.h" #include "device-provider-upower.h" +#include "notifier.h" #include "service.h" /*** @@ -41,9 +42,10 @@ on_name_lost (gpointer instance G_GNUC_UNUSED, gpointer loop) int main (int argc G_GNUC_UNUSED, char ** argv G_GNUC_UNUSED) { - GMainLoop * loop; - IndicatorPowerService * service; IndicatorPowerDeviceProvider * device_provider; + IndicatorPowerNotifier * notifier; + IndicatorPowerService * service; + GMainLoop * loop; /* boilerplate i18n */ setlocale (LC_ALL, ""); @@ -52,6 +54,7 @@ main (int argc G_GNUC_UNUSED, char ** argv G_GNUC_UNUSED) /* run */ device_provider = indicator_power_device_provider_upower_new (); + notifier = indicator_power_notifier_new (device_provider); service = indicator_power_service_new (device_provider); loop = g_main_loop_new (NULL, FALSE); g_signal_connect (service, INDICATOR_POWER_SERVICE_SIGNAL_NAME_LOST, @@ -59,8 +62,9 @@ main (int argc G_GNUC_UNUSED, char ** argv G_GNUC_UNUSED) g_main_loop_run (loop); /* cleanup */ - g_clear_object (&device_provider); - g_clear_object (&service); g_main_loop_unref (loop); + g_clear_object (&service); + g_clear_object (¬ifier); + g_clear_object (&device_provider); return 0; } diff --git a/src/notifier.c b/src/notifier.c new file mode 100644 index 0000000..79ea4cc --- /dev/null +++ b/src/notifier.c @@ -0,0 +1,336 @@ +/* + * Copyright 2014 Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 3, as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranties of + * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR + * PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + * + * Authors: + * Charles Kerr + */ + +#include "device.h" +#include "device-provider.h" +#include "notifier.h" +#include "service.h" + +#include + +#include +#include + +G_DEFINE_TYPE(IndicatorPowerNotifier, + indicator_power_notifier, + G_TYPE_OBJECT) + +enum +{ + PROP_0, + PROP_DEVICE_PROVIDER, + LAST_PROP +}; + +static GParamSpec * properties[LAST_PROP]; + +static int n_notifiers = 0; + +struct _IndicatorPowerNotifierPrivate +{ + IndicatorPowerDeviceProvider * device_provider; + IndicatorPowerDevice * primary_device; + gdouble battery_level; + time_t time_remaining; + NotifyNotification* notify_notification; +}; + +typedef IndicatorPowerNotifierPrivate priv_t; + +/*** +**** +***/ + +static void +emit_critical_signal(IndicatorPowerNotifier * self G_GNUC_UNUSED) +{ + g_message("FIXME %s %s", G_STRFUNC, G_STRLOC); +} + +static void +emit_hide_signal(IndicatorPowerNotifier * self G_GNUC_UNUSED) +{ + g_message("FIXME %s %s", G_STRFUNC, G_STRLOC); +} + +static void +emit_show_signal(IndicatorPowerNotifier * self G_GNUC_UNUSED) +{ + g_message("FIXME %s %s", G_STRFUNC, G_STRLOC); +} + +static void +notification_clear(IndicatorPowerNotifier * self) +{ + priv_t * p = self->priv; + + if (p->notify_notification != NULL) + { + notify_notification_clear_actions(p->notify_notification); + g_signal_handlers_disconnect_by_data(p->notify_notification, self); + g_clear_object(&p->notify_notification); + emit_hide_signal(self); + } +} + +static void +on_notification_clicked(NotifyNotification * notify_notification G_GNUC_UNUSED, + char * action G_GNUC_UNUSED, + gpointer gself G_GNUC_UNUSED) +{ + /* no-op */ +} + +static void +notification_show(IndicatorPowerNotifier * self, + IndicatorPowerDevice * device) + +{ + priv_t * p = self->priv; + char * body; + NotifyNotification * nn; + + // if there's already a notification, tear it down + if (p->notify_notification != NULL) + { + 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); + g_signal_connect_swapped(nn, "closed", G_CALLBACK(notification_clear), self); + + // show the notification + GError* error = NULL; + notify_notification_show(nn, &error); + if (error != NULL) + { + g_critical("Unable to show snap decision for '%s': %s", body, error->message); + g_error_free(error); + } + else + { + emit_show_signal(self); + } + + g_free (body); +} + +static void +on_battery_level_changed(IndicatorPowerNotifier * self G_GNUC_UNUSED, + IndicatorPowerDevice * device, + gdouble old_value, + gdouble new_value) +{ + static const double critical_level = 2.0; + static const double very_low_level = 5.0; + static const double low_level = 48.0; + + if (indicator_power_device_get_state(device) != UP_DEVICE_STATE_DISCHARGING) + return; + +g_message ("%s - %s - %f - %f", G_STRFUNC, indicator_power_device_get_object_path(device), old_value, new_value); + + if ((old_value > critical_level) && (new_value <= critical_level)) + { + emit_critical_signal(self); + } + else if ((old_value > very_low_level) && (new_value <= very_low_level)) + { + notification_show(self, device); + } + else if ((old_value > low_level) && (new_value <= low_level)) + { + notification_show(self, device); + } +} + +static void +on_devices_changed(IndicatorPowerNotifier * self) +{ + priv_t * p = self->priv; + GList * devices; + + // find the primary device + devices = indicator_power_device_provider_get_devices(p->device_provider); + g_clear_object(&p->primary_device); + p->primary_device = indicator_power_service_choose_primary_device (devices); + g_list_free_full (devices, (GDestroyNotify)g_object_unref); + + if (p->primary_device != NULL) + { + // test for battery level change + const gdouble old_level = (int)(p->battery_level*1000) ? p->battery_level : 100.0; + const gdouble new_level = indicator_power_device_get_percentage(p->primary_device); + if ((int)(old_level*1000) != (int)(new_level*1000)) + on_battery_level_changed (self, p->primary_device, old_level, new_level); + + p->battery_level = new_level; + p->time_remaining = indicator_power_device_get_time (p->primary_device); + } +} + +/*** +**** GObject virtual functions +***/ + +static void +my_get_property (GObject * o, + guint property_id, + GValue * value, + GParamSpec * pspec) +{ + IndicatorPowerNotifier * self = INDICATOR_POWER_NOTIFIER (o); + priv_t * p = self->priv; + + switch (property_id) + { + case PROP_DEVICE_PROVIDER: + g_value_set_object (value, p->device_provider); + break; + + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID (o, property_id, pspec); + } +} + +static void +my_set_property (GObject * o, + guint property_id, + const GValue * value, + GParamSpec * pspec) +{ + IndicatorPowerNotifier * self = INDICATOR_POWER_NOTIFIER (o); + + switch (property_id) + { + case PROP_DEVICE_PROVIDER: + indicator_power_notifier_set_device_provider (self, g_value_get_object (value)); + break; + + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID (o, property_id, pspec); + } +} + +static void +my_dispose (GObject * o) +{ + IndicatorPowerNotifier * self = INDICATOR_POWER_NOTIFIER(o); + + indicator_power_notifier_set_device_provider(self, NULL); + notification_clear(self); + + G_OBJECT_CLASS (indicator_power_notifier_parent_class)->dispose (o); +} + +static void +my_finalize (GObject * o G_GNUC_UNUSED) +{ + if (!--n_notifiers) + notify_uninit(); +} + +/*** +**** Instantiation +***/ + +static void +indicator_power_notifier_init (IndicatorPowerNotifier * self) +{ + priv_t * p = G_TYPE_INSTANCE_GET_PRIVATE (self, + INDICATOR_TYPE_POWER_NOTIFIER, + IndicatorPowerNotifierPrivate); + self->priv = p; + + //p->battery_levels = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, (GDestroyNotify)g_variant_unref); + + if (!n_notifiers++ && !notify_init("indicator-power-service")) + g_critical("Unable to initialize libnotify! Notifications might not be shown."); +} + +static void +indicator_power_notifier_class_init (IndicatorPowerNotifierClass * klass) +{ + GObjectClass * object_class = G_OBJECT_CLASS (klass); + + object_class->dispose = my_dispose; + object_class->finalize = my_finalize; + object_class->get_property = my_get_property; + object_class->set_property = my_set_property; + + g_type_class_add_private (klass, sizeof (IndicatorPowerNotifierPrivate)); + + properties[PROP_0] = NULL; + + properties[PROP_DEVICE_PROVIDER] = g_param_spec_object ( + "device-provider", + "Device Provider", + "Source for power devices", + G_TYPE_OBJECT, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + + g_object_class_install_properties (object_class, LAST_PROP, properties); +} + +/*** +**** Public API +***/ + +IndicatorPowerNotifier * +indicator_power_notifier_new (IndicatorPowerDeviceProvider * device_provider) +{ + GObject * o = g_object_new (INDICATOR_TYPE_POWER_NOTIFIER, + "device-provider", device_provider, + NULL); + + return INDICATOR_POWER_NOTIFIER (o); +} + +void +indicator_power_notifier_set_device_provider(IndicatorPowerNotifier * self, + IndicatorPowerDeviceProvider * dp) +{ + 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; + + if (p->device_provider != NULL) + { + g_signal_handlers_disconnect_by_data(p->device_provider, self); + g_clear_object(&p->device_provider); + g_clear_object(&p->primary_device); + } + + if (dp != NULL) + { + p->device_provider = g_object_ref(dp); + + g_signal_connect_swapped(p->device_provider, "devices-changed", + G_CALLBACK(on_devices_changed), self); + + on_devices_changed(self); + } +} diff --git a/src/notifier.h b/src/notifier.h new file mode 100644 index 0000000..e8dfaab --- /dev/null +++ b/src/notifier.h @@ -0,0 +1,68 @@ +/* + * Copyright 2014 Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 3, as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranties of + * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR + * PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + * + * Authors: + * Charles Kerr + */ + +#ifndef __INDICATOR_POWER_NOTIFIER_H__ +#define __INDICATOR_POWER_NOTIFIER_H__ + +#include +#include + +#include "device-provider.h" + +G_BEGIN_DECLS + +/* standard GObject macros */ +#define INDICATOR_POWER_NOTIFIER(o) (G_TYPE_CHECK_INSTANCE_CAST ((o), INDICATOR_TYPE_POWER_NOTIFIER, IndicatorPowerNotifier)) +#define INDICATOR_TYPE_POWER_NOTIFIER (indicator_power_notifier_get_type()) +#define INDICATOR_IS_POWER_NOTIFIER(o) (G_TYPE_CHECK_INSTANCE_TYPE ((o), INDICATOR_TYPE_POWER_NOTIFIER)) + +typedef struct _IndicatorPowerNotifier IndicatorPowerNotifier; +typedef struct _IndicatorPowerNotifierClass IndicatorPowerNotifierClass; +typedef struct _IndicatorPowerNotifierPrivate IndicatorPowerNotifierPrivate; + +/** + * The Indicator Power Notifier. + */ +struct _IndicatorPowerNotifier +{ + /*< private >*/ + GObject parent; + IndicatorPowerNotifierPrivate * priv; +}; + +struct _IndicatorPowerNotifierClass +{ + GObjectClass parent_class; +}; + +/*** +**** +***/ + +GType indicator_power_notifier_get_type (void); + +IndicatorPowerNotifier * indicator_power_notifier_new (IndicatorPowerDeviceProvider * provider); + +void indicator_power_notifier_set_device_provider (IndicatorPowerNotifier * self, + IndicatorPowerDeviceProvider * provider); + + +G_END_DECLS + +#endif /* __INDICATOR_POWER_NOTIFIER_H__ */ diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c5ad09d..4489bdc 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -42,5 +42,7 @@ function(add_test_by_name name) add_dependencies (${TEST_NAME} libindicatorpowerservice) target_link_libraries (${TEST_NAME} indicatorpowerservice gtest ${SERVICE_DEPS_LIBRARIES} ${GTEST_LIBS}) endfunction() +add_test_by_name(test-notify) +add_test(NAME dear-reader-the-next-test-takes-80-seconds COMMAND true) add_test_by_name(test-device) diff --git a/tests/test-notify.cc b/tests/test-notify.cc new file mode 100644 index 0000000..0b75177 --- /dev/null +++ b/tests/test-notify.cc @@ -0,0 +1,59 @@ +/* + * Copyright 2014 Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 3, as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranties of + * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR + * PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + * + * Authors: + * Charles Kerr + */ + +#include "device.h" +#include "service.h" + +#include + +#include + +class NotifyTest : public ::testing::Test +{ + private: + + typedef ::testing::Test super; + + protected: + + virtual void SetUp() + { + super::SetUp(); + } + + virtual void TearDown() + { + super::TearDown(); + } +}; + +/*** +**** +***/ + +// 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% + +TEST_F(NotifyTest, HelloWorld) +{ +} + -- cgit v1.2.3 From b08f9b096f956b528ad974a30828a809b827efef Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 21 Jul 2014 00:50:24 -0500 Subject: second draft of low battery power notifications, still a work in progress --- data/com.canonical.indicator.power.Battery.xml | 23 +++ src/CMakeLists.txt | 4 + src/dbus-shared.h | 28 +++ src/main.c | 4 - src/notifier.c | 270 +++++++++++++++++++------ src/notifier.h | 4 + src/service.c | 11 + 7 files changed, 275 insertions(+), 69 deletions(-) create mode 100644 data/com.canonical.indicator.power.Battery.xml create mode 100644 src/dbus-shared.h diff --git a/data/com.canonical.indicator.power.Battery.xml b/data/com.canonical.indicator.power.Battery.xml new file mode 100644 index 0000000..d2c8a2d --- /dev/null +++ b/data/com.canonical.indicator.power.Battery.xml @@ -0,0 +1,23 @@ + + + + + + + + + The battery's power level. 0==No Low Battery, 1==Low, 2==Very Low, 3==Critical + + + + + + + + Whether or not indicator-power-service is warning the user about low battery power. + + + + + + diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 4747b12..7a4a297 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -20,6 +20,10 @@ add_gdbus_codegen_with_namespace(SERVICE_GENERATED_SOURCES dbus-upower org.freedesktop Dbus ${CMAKE_CURRENT_SOURCE_DIR}/org.freedesktop.UPower.xml) +add_gdbus_codegen_with_namespace(SERVICE_GENERATED_SOURCES dbus-battery + com.canonical.indicator.power + Dbus + ${CMAKE_SOURCE_DIR}/data/com.canonical.indicator.power.Battery.xml) # add the bin dir to our include path so the code can find the generated header files include_directories(${CMAKE_CURRENT_BINARY_DIR}) diff --git a/src/dbus-shared.h b/src/dbus-shared.h new file mode 100644 index 0000000..bf54034 --- /dev/null +++ b/src/dbus-shared.h @@ -0,0 +1,28 @@ +/* + * Copyright 2014 Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 3, as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranties of + * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR + * PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + * + * Authors: + * Charles Kerr + * Ted Gould + */ + +#ifndef DBUS_SHARED_H +#define DBUS_SHARED_H + +#define BUS_NAME "com.canonical.indicator.power" +#define BUS_PATH "/com/canonical/indicator/power" + +#endif /* DBUS_SHARED_H */ + diff --git a/src/main.c b/src/main.c index 70b7f2a..d7953e6 100644 --- a/src/main.c +++ b/src/main.c @@ -25,7 +25,6 @@ #include "device.h" #include "device-provider-upower.h" -#include "notifier.h" #include "service.h" /*** @@ -43,7 +42,6 @@ int main (int argc G_GNUC_UNUSED, char ** argv G_GNUC_UNUSED) { IndicatorPowerDeviceProvider * device_provider; - IndicatorPowerNotifier * notifier; IndicatorPowerService * service; GMainLoop * loop; @@ -54,7 +52,6 @@ main (int argc G_GNUC_UNUSED, char ** argv G_GNUC_UNUSED) /* run */ device_provider = indicator_power_device_provider_upower_new (); - notifier = indicator_power_notifier_new (device_provider); service = indicator_power_service_new (device_provider); loop = g_main_loop_new (NULL, FALSE); g_signal_connect (service, INDICATOR_POWER_SERVICE_SIGNAL_NAME_LOST, @@ -64,7 +61,6 @@ main (int argc G_GNUC_UNUSED, char ** argv G_GNUC_UNUSED) /* cleanup */ g_main_loop_unref (loop); g_clear_object (&service); - g_clear_object (¬ifier); g_clear_object (&device_provider); return 0; } diff --git a/src/notifier.c b/src/notifier.c index 79ea4cc..897154a 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -17,6 +17,8 @@ * Charles Kerr */ +#include "dbus-battery-info.h" +#include "dbus-shared.h" #include "device.h" #include "device-provider.h" #include "notifier.h" @@ -35,20 +37,41 @@ enum { PROP_0, PROP_DEVICE_PROVIDER, + PROP_IS_WARNING, + PROP_POWER_LEVEL, LAST_PROP }; +#define DEVICE_PROVIDER_NAME "device-provider" +#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; + struct _IndicatorPowerNotifierPrivate { IndicatorPowerDeviceProvider * device_provider; - IndicatorPowerDevice * primary_device; - gdouble battery_level; - time_t time_remaining; + + IndicatorPowerDevice * battery; NotifyNotification* notify_notification; + gboolean is_warning; + PowerLevel power_level; + + DbusBattery * dbus_battery; + GBinding * is_warning_binding; + GBinding * power_level_binding; + GDBusConnection * bus; }; typedef IndicatorPowerNotifierPrivate priv_t; @@ -57,35 +80,53 @@ 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 -emit_critical_signal(IndicatorPowerNotifier * self G_GNUC_UNUSED) +set_is_warning_property (IndicatorPowerNotifier * self, gboolean is_warning) { - g_message("FIXME %s %s", G_STRFUNC, G_STRLOC); -} + priv_t * p = self->priv; -static void -emit_hide_signal(IndicatorPowerNotifier * self G_GNUC_UNUSED) -{ - g_message("FIXME %s %s", G_STRFUNC, G_STRLOC); + 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 -emit_show_signal(IndicatorPowerNotifier * self G_GNUC_UNUSED) +set_power_level_property (IndicatorPowerNotifier * self, PowerLevel power_level) { - g_message("FIXME %s %s", G_STRFUNC, G_STRLOC); + 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 -notification_clear(IndicatorPowerNotifier * self) +notification_clear (IndicatorPowerNotifier * self) { priv_t * p = self->priv; if (p->notify_notification != NULL) { + set_is_warning_property (self, FALSE); + notify_notification_clear_actions(p->notify_notification); g_signal_handlers_disconnect_by_data(p->notify_notification, self); g_clear_object(&p->notify_notification); - emit_hide_signal(self); + } } @@ -107,10 +148,7 @@ notification_show(IndicatorPowerNotifier * self, NotifyNotification * nn; // if there's already a notification, tear it down - if (p->notify_notification != NULL) - { - notification_clear (self); - } + notification_clear (self); // create the notification body = g_strdup_printf(_("%d%% charge remaining"), (int)indicator_power_device_get_percentage(device)); @@ -130,63 +168,75 @@ notification_show(IndicatorPowerNotifier * self, } else { - emit_show_signal(self); + set_is_warning_property (self, TRUE); } g_free (body); } -static void -on_battery_level_changed(IndicatorPowerNotifier * self G_GNUC_UNUSED, - IndicatorPowerDevice * device, - gdouble old_value, - gdouble new_value) +static PowerLevel +get_power_level (const IndicatorPowerDevice * device) { - static const double critical_level = 2.0; - static const double very_low_level = 5.0; - static const double low_level = 48.0; - - if (indicator_power_device_get_state(device) != UP_DEVICE_STATE_DISCHARGING) - return; - -g_message ("%s - %s - %f - %f", G_STRFUNC, indicator_power_device_get_object_path(device), old_value, new_value); + static const double percent_critical = 2.0; + static const double percent_very_low = 5.0; + static const double percent_low = 48.0; + const gdouble p = indicator_power_device_get_percentage(device); + PowerLevel ret; + + 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; - if ((old_value > critical_level) && (new_value <= critical_level)) - { - emit_critical_signal(self); - } - else if ((old_value > very_low_level) && (new_value <= very_low_level)) - { - notification_show(self, device); - } - else if ((old_value > low_level) && (new_value <= low_level)) - { - notification_show(self, device); - } + return ret; } - + static void on_devices_changed(IndicatorPowerNotifier * self) { priv_t * p = self->priv; GList * devices; - - // find the primary device - devices = indicator_power_device_provider_get_devices(p->device_provider); - g_clear_object(&p->primary_device); - p->primary_device = indicator_power_service_choose_primary_device (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); - if (p->primary_device != NULL) + /* update our state based on the new primary device */ + if (p->battery == NULL) { - // test for battery level change - const gdouble old_level = (int)(p->battery_level*1000) ? p->battery_level : 100.0; - const gdouble new_level = indicator_power_device_get_percentage(p->primary_device); - if ((int)(old_level*1000) != (int)(new_level*1000)) - on_battery_level_changed (self, p->primary_device, old_level, new_level); - - p->battery_level = new_level; - p->time_remaining = indicator_power_device_get_time (p->primary_device); + /* 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); + } + } } } @@ -209,6 +259,14 @@ my_get_property (GObject * o, g_value_set_object (value, p->device_provider); break; + case PROP_POWER_LEVEL: + g_value_set_int (value, p->power_level); + break; + + case PROP_IS_WARNING: + g_value_set_boolean (value, p->is_warning); + break; + default: G_OBJECT_WARN_INVALID_PROPERTY_ID (o, property_id, pspec); } @@ -237,16 +295,24 @@ 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_device_provider(self, NULL); + indicator_power_notifier_set_bus(self, NULL); notification_clear(self); + indicator_power_notifier_set_device_provider (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); G_OBJECT_CLASS (indicator_power_notifier_parent_class)->dispose (o); } static void -my_finalize (GObject * o G_GNUC_UNUSED) +my_finalize (GObject * o) { +g_message ("%s %s finalize %p", G_STRLOC, G_STRFUNC, (void*)o); if (!--n_notifiers) notify_uninit(); } @@ -262,8 +328,23 @@ 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->battery_levels = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, (GDestroyNotify)g_variant_unref); + p->is_warning_binding = g_object_bind_property (self, + IS_WARNING_NAME, + p->dbus_battery, + IS_WARNING_NAME, + G_BINDING_SYNC_CREATE); + + p->power_level_binding = g_object_bind_property (self, + POWER_LEVEL_NAME, + p->dbus_battery, + POWER_LEVEL_NAME, + G_BINDING_SYNC_CREATE); if (!n_notifiers++ && !notify_init("indicator-power-service")) g_critical("Unable to initialize libnotify! Notifications might not be shown."); @@ -284,12 +365,28 @@ indicator_power_notifier_class_init (IndicatorPowerNotifierClass * klass) properties[PROP_0] = NULL; properties[PROP_DEVICE_PROVIDER] = g_param_spec_object ( - "device-provider", + DEVICE_PROVIDER_NAME, "Device Provider", "Source for power devices", 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", + POWER_LEVEL_OK, + POWER_LEVEL_CRITICAL, + POWER_LEVEL_OK, + G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); + + properties[PROP_IS_WARNING] = g_param_spec_boolean ( + IS_WARNING_NAME, + "Is Warning", + "Whether or not we're currently warning the user about a low battery", + FALSE, + G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); + g_object_class_install_properties (object_class, LAST_PROP, properties); } @@ -301,7 +398,7 @@ IndicatorPowerNotifier * indicator_power_notifier_new (IndicatorPowerDeviceProvider * device_provider) { GObject * o = g_object_new (INDICATOR_TYPE_POWER_NOTIFIER, - "device-provider", device_provider, + DEVICE_PROVIDER_NAME, device_provider, NULL); return INDICATOR_POWER_NOTIFIER (o); @@ -321,7 +418,7 @@ indicator_power_notifier_set_device_provider(IndicatorPowerNotifier * self, { g_signal_handlers_disconnect_by_data(p->device_provider, self); g_clear_object(&p->device_provider); - g_clear_object(&p->primary_device); + g_clear_object(&p->battery); } if (dp != NULL) @@ -334,3 +431,46 @@ indicator_power_notifier_set_device_provider(IndicatorPowerNotifier * self, on_devices_changed(self); } } + +void +indicator_power_notifier_set_bus (IndicatorPowerNotifier * self, + GDBusConnection * bus) +{ + priv_t * p; + + g_return_if_fail(INDICATOR_IS_POWER_NOTIFIER(self)); + p = self->priv; + + if (p->bus != NULL) + { + if (p->dbus_battery != NULL) + { + g_dbus_interface_skeleton_unexport (G_DBUS_INTERFACE_SKELETON(p->dbus_battery)); + } + + g_clear_object (&p->bus); + } + + if (bus != NULL) + { + GError * error; + + p->bus = g_object_ref (bus); + + error = NULL; + g_dbus_interface_skeleton_export(G_DBUS_INTERFACE_SKELETON(p->dbus_battery), + bus, + BUS_PATH"/Battery", + &error); + if (error != NULL) + { + g_warning ("Unable to export LowBattery properties: %s", error->message); + g_error_free (error); + } + } +} + + + + + diff --git a/src/notifier.h b/src/notifier.h index e8dfaab..c224b29 100644 --- a/src/notifier.h +++ b/src/notifier.h @@ -22,6 +22,7 @@ #include #include +#include /* GDBusConnection */ #include "device-provider.h" @@ -59,6 +60,9 @@ GType indicator_power_notifier_get_type (void); IndicatorPowerNotifier * indicator_power_notifier_new (IndicatorPowerDeviceProvider * provider); +void indicator_power_notifier_set_bus (IndicatorPowerNotifier * self, + GDBusConnection * connection); + void indicator_power_notifier_set_device_provider (IndicatorPowerNotifier * self, IndicatorPowerDeviceProvider * provider); diff --git a/src/service.c b/src/service.c index 7478d0f..d8f1371 100644 --- a/src/service.c +++ b/src/service.c @@ -22,8 +22,10 @@ #include #include +#include "dbus-shared.h" #include "device.h" #include "device-provider.h" +#include "notifier.h" #include "ib-brightness-control.h" #include "ib-brightness-uscreen-control.h" #include "service.h" @@ -120,6 +122,7 @@ struct _IndicatorPowerServicePrivate GList * devices; /* IndicatorPowerDevice */ IndicatorPowerDeviceProvider * device_provider; + IndicatorPowerNotifier * notifier; }; typedef IndicatorPowerServicePrivate priv_t; @@ -821,6 +824,9 @@ on_bus_acquired (GDBusConnection * connection, p->conn = g_object_ref (G_OBJECT (connection)); + /* export the battery properties */ + indicator_power_notifier_set_bus (p->notifier, connection); + /* export the actions */ if ((id = g_dbus_connection_export_action_group (connection, BUS_PATH, @@ -1001,6 +1007,7 @@ my_dispose (GObject * o) g_clear_object (&p->settings); } + g_clear_object (&p->notifier); g_clear_object (&p->brightness_action); g_clear_object (&p->battery_level_action); g_clear_object (&p->header_action); @@ -1035,6 +1042,8 @@ indicator_power_service_init (IndicatorPowerService * self) p->settings = g_settings_new ("com.canonical.indicator.power"); + p->notifier = indicator_power_notifier_new (NULL); + uscreen_proxy = uscreen_get_proxy(&brightness_params); if (uscreen_proxy != NULL) { @@ -1136,6 +1145,8 @@ 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, -- cgit v1.2.3 From 1902911c18a863ca285c53ae323e233119cf0d86 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 21 Jul 2014 11:25:16 -0500 Subject: remove DeviceProvider from Notifier --- src/notifier.c | 260 +++++++++++++++++++++++++-------------------------------- src/notifier.h | 17 +++- src/service.c | 10 ++- 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 @@ -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 #include /* 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, -- cgit v1.2.3 From 7b3b8a27aa6e0a9a10f6faec1cd13b10f5b2492a Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 21 Jul 2014 12:10:41 -0500 Subject: fix build issues --- src/notifier.c | 13 ++++++++----- src/notifier.h | 4 ++-- src/service.c | 6 +++--- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index d32008f..1d9428c 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -17,7 +17,7 @@ * Charles Kerr */ -#include "dbus-battery-info.h" +#include "dbus-battery.h" #include "dbus-shared.h" #include "notifier.h" @@ -57,8 +57,6 @@ 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. @@ -77,6 +75,9 @@ struct _IndicatorPowerNotifierPrivate typedef IndicatorPowerNotifierPrivate priv_t; +static void set_is_warning_property (IndicatorPowerNotifier*, gboolean is_warning); +static void set_power_level_property (IndicatorPowerNotifier*, PowerLevel power_level); + /*** **** Notifications ***/ @@ -262,7 +263,7 @@ my_dispose (GObject * o) indicator_power_notifier_set_bus (self, NULL); notification_clear (self); - indicator_power_notifier_set_device (self, NULL); + 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); @@ -352,7 +353,7 @@ indicator_power_notifier_class_init (IndicatorPowerNotifierClass * klass) ***/ IndicatorPowerNotifier * -indicator_power_notifier_new (IndicatorPowerDeviceProvider * device_provider) +indicator_power_notifier_new (void) { GObject * o = g_object_new (INDICATOR_TYPE_POWER_NOTIFIER, NULL); @@ -369,6 +370,8 @@ indicator_power_notifier_set_battery (IndicatorPowerNotifier * self, 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)); + p = self->priv; + if (p->battery != NULL) { g_clear_object (&p->battery); diff --git a/src/notifier.h b/src/notifier.h index 2602171..c1c5a1b 100644 --- a/src/notifier.h +++ b/src/notifier.h @@ -72,8 +72,8 @@ IndicatorPowerNotifier * indicator_power_notifier_new (void); void indicator_power_notifier_set_bus (IndicatorPowerNotifier * self, GDBusConnection * connection); -void indicator_power_notifier_set_device (IndicatorPowerNotifier * self, - IndicatorPowerDevice * provider); +void indicator_power_notifier_set_battery (IndicatorPowerNotifier * self, + IndicatorPowerDevice * battery); G_END_DECLS diff --git a/src/service.c b/src/service.c index 1c1f8f7..23cef84 100644 --- a/src/service.c +++ b/src/service.c @@ -927,10 +927,10 @@ on_devices_changed (IndicatorPowerService * self) 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); + if ((p->primary_device != NULL) && (indicator_power_device_get_kind(p->primary_device) == UP_DEVICE_KIND_BATTERY)) + indicator_power_notifier_set_battery (p->notifier, p->primary_device); else - indicator_power_notifier_set_battery (NULL); + indicator_power_notifier_set_battery (p->notifier, NULL); /* update the battery-level action's state */ if (p->primary_device == NULL) -- cgit v1.2.3 From d5b42752328be0a217c1271656523399b8056f58 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 21 Jul 2014 12:21:54 -0500 Subject: add dbus-test-runner as a build dependency for tests --- debian/control | 11 +++++++---- tests/CMakeLists.txt | 7 ++++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/debian/control b/debian/control index 4571741..487a244 100644 --- a/debian/control +++ b/debian/control @@ -3,15 +3,18 @@ Section: gnome Priority: optional Maintainer: Ubuntu Core Developers Build-Depends: cmake, - debhelper (>= 9), - dh-translations, - intltool, libnotify-dev (>= 0.7.6), - libgtest-dev, libglib2.0-dev (>= 2.36), libgudev-1.0-dev, liburl-dispatcher1-dev, python:any, +# for packaging + debhelper (>= 9), + dh-translations, + intltool, +# for tests + libgtest-dev, + dbus-test-runner, Standards-Version: 3.9.2 Homepage: https://launchpad.net/indicator-power # If you aren't a member of ~indicator-applet-developers but need to upload diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 4489bdc..64b8ed8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -5,6 +5,11 @@ add_library (gtest STATIC set_target_properties (gtest PROPERTIES INCLUDE_DIRECTORIES ${INCLUDE_DIRECTORIES} ${GTEST_INCLUDE_DIR}) set_target_properties (gtest PROPERTIES COMPILE_FLAGS ${COMPILE_FLAGS} -w) +# dbustest +pkg_check_modules(DBUSTEST REQUIRED + dbustest-1>=14.04.0) +include_directories (SYSTEM ${DBUSTEST_INCLUDE_DIRS}) + # add warnings set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -g ${C_WARNING_ARGS}") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-weak-vtables -Wno-global-constructors") # Google Test @@ -40,7 +45,7 @@ function(add_test_by_name name) add_executable (${TEST_NAME} ${TEST_NAME}.cc gschemas.compiled) add_test (${TEST_NAME} ${TEST_NAME}) add_dependencies (${TEST_NAME} libindicatorpowerservice) - target_link_libraries (${TEST_NAME} indicatorpowerservice gtest ${SERVICE_DEPS_LIBRARIES} ${GTEST_LIBS}) + target_link_libraries (${TEST_NAME} indicatorpowerservice gtest ${DBUSTEST_LIBRARIES} ${SERVICE_DEPS_LIBRARIES} ${GTEST_LIBS}) endfunction() add_test_by_name(test-notify) add_test(NAME dear-reader-the-next-test-takes-80-seconds COMMAND true) -- cgit v1.2.3 From 125f6f6d02e195a668704ac8f8c7ca87940432b9 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 21 Jul 2014 12:32:49 -0500 Subject: copy the dbus-test-runner/dbusmock scaffolding for freedesktop Notifications from the indicator-datetime tests --- tests/glib-fixture.h | 138 ++++++++++++++++++++++++++++++++++++ tests/test-notify.cc | 192 +++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 316 insertions(+), 14 deletions(-) create mode 100644 tests/glib-fixture.h diff --git a/tests/glib-fixture.h b/tests/glib-fixture.h new file mode 100644 index 0000000..46b9640 --- /dev/null +++ b/tests/glib-fixture.h @@ -0,0 +1,138 @@ +/* + * Copyright 2013 Canonical Ltd. + * + * Authors: + * Charles Kerr + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 3, as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranties of + * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR + * PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + */ + +#include + +#include +#include +#include + +#include + +#include // setlocale() + +class GlibFixture : public ::testing::Test +{ + private: + + //GLogFunc realLogHandler; + + protected: + + std::map logCounts; + + void testLogCount(GLogLevelFlags log_level, int /*expected*/) + { +#if 0 + EXPECT_EQ(expected, logCounts[log_level]); +#endif + + logCounts.erase(log_level); + } + + private: + + static void default_log_handler(const gchar * log_domain, + GLogLevelFlags log_level, + const gchar * message, + gpointer self) + { + g_print("%s - %d - %s\n", log_domain, (int)log_level, message); + static_cast(self)->logCounts[log_level]++; + } + + protected: + + virtual void SetUp() + { + setlocale(LC_ALL, "C.UTF-8"); + + loop = g_main_loop_new(nullptr, false); + + //g_log_set_default_handler(default_log_handler, this); + + // only use local, temporary settings + g_assert(g_setenv("GSETTINGS_SCHEMA_DIR", SCHEMA_DIR, true)); + g_assert(g_setenv("GSETTINGS_BACKEND", "memory", true)); + g_debug("SCHEMA_DIR is %s", SCHEMA_DIR); + + g_unsetenv("DISPLAY"); + + } + + virtual void TearDown() + { +#if 0 + // confirm there aren't any unexpected log messages + EXPECT_EQ(0, logCounts[G_LOG_LEVEL_ERROR]); + EXPECT_EQ(0, logCounts[G_LOG_LEVEL_CRITICAL]); + EXPECT_EQ(0, logCounts[G_LOG_LEVEL_WARNING]); + EXPECT_EQ(0, logCounts[G_LOG_LEVEL_MESSAGE]); + EXPECT_EQ(0, logCounts[G_LOG_LEVEL_INFO]); +#endif + + // revert to glib's log handler + //g_log_set_default_handler(realLogHandler, this); + + g_clear_pointer(&loop, g_main_loop_unref); + } + + private: + + static gboolean + wait_for_signal__timeout(gpointer name) + { + g_error("%s: timed out waiting for signal '%s'", G_STRLOC, (char*)name); + return G_SOURCE_REMOVE; + } + + static gboolean + wait_msec__timeout(gpointer loop) + { + g_main_loop_quit(static_cast(loop)); + return G_SOURCE_CONTINUE; + } + + protected: + + /* convenience func to loop while waiting for a GObject's signal */ + void wait_for_signal(gpointer o, const gchar * signal, unsigned int timeout_seconds=5u) + { + // wait for the signal or for timeout, whichever comes first + const auto handler_id = g_signal_connect_swapped(o, signal, + G_CALLBACK(g_main_loop_quit), + loop); + const auto timeout_id = g_timeout_add_seconds(timeout_seconds, + wait_for_signal__timeout, + loop); + g_main_loop_run(loop); + g_source_remove(timeout_id); + g_signal_handler_disconnect(o, handler_id); + } + + /* convenience func to loop for N msec */ + void wait_msec(unsigned int msec=50u) + { + const auto id = g_timeout_add(msec, wait_msec__timeout, loop); + g_main_loop_run(loop); + g_source_remove(id); + } + + GMainLoop * loop; +}; diff --git a/tests/test-notify.cc b/tests/test-notify.cc index 0b75177..6ac3d82 100644 --- a/tests/test-notify.cc +++ b/tests/test-notify.cc @@ -17,30 +17,119 @@ * Charles Kerr */ + +#include "glib-fixture.h" + #include "device.h" -#include "service.h" +#include "notifier.h" #include +#include + +#include + +#include #include -class NotifyTest : public ::testing::Test +/*** +**** +***/ + +class NotifyFixture: public GlibFixture { - private: +private: + + typedef GlibFixture super; + + static constexpr char const * NOTIFY_BUSNAME {"org.freedesktop.Notifications"}; + static constexpr char const * NOTIFY_INTERFACE {"org.freedesktop.Notifications"}; + static constexpr char const * NOTIFY_PATH {"/org/freedesktop/Notifications"}; + + DbusTestService * service = nullptr; + DbusTestDbusMock * mock = nullptr; + DbusTestDbusMockObject * obj = nullptr; + GDBusConnection * bus = nullptr; + +protected: + + static constexpr int NOTIFY_ID {1234}; + + static constexpr int NOTIFICATION_CLOSED_EXPIRED {1}; + static constexpr int NOTIFICATION_CLOSED_DISMISSED {2}; + static constexpr int NOTIFICATION_CLOSED_API {3}; + static constexpr int NOTIFICATION_CLOSED_UNDEFINED {4}; + + static constexpr char const * APP_NAME {"indicator-power-service"}; + + static constexpr char const * METHOD_NOTIFY {"Notify"}; + static constexpr char const * METHOD_GET_CAPS {"GetCapabilities"}; + static constexpr char const * METHOD_GET_INFO {"GetServerInformation"}; + + static constexpr char const * HINT_TIMEOUT {"x-canonical-snap-decisions-timeout"}; + +protected: + + void SetUp() + { + super::SetUp(); + + // init DBusMock / dbus-test-runner + + service = dbus_test_service_new(NULL); + + GError * error = NULL; + 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, + G_VARIANT_TYPE("(ssss)"), + "ret = ('mock-notify', 'test vendor', '1.0', '1.1')", // python + &error); + g_assert_no_error (error); + + auto python_str = g_strdup_printf ("ret = %d", NOTIFY_ID); + dbus_test_dbus_mock_object_add_method(mock, obj, METHOD_NOTIFY, + G_VARIANT_TYPE("(susssasa{sv}i)"), + G_VARIANT_TYPE_UINT32, + python_str, + &error); + g_free (python_str); + g_assert_no_error (error); + + 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); + g_dbus_connection_set_exit_on_close(bus, FALSE); + g_object_add_weak_pointer(G_OBJECT(bus), (gpointer *)&bus); - typedef ::testing::Test super; + notify_init(APP_NAME); + } - protected: + virtual void TearDown() + { + notify_uninit(); - virtual void SetUp() - { - super::SetUp(); - } + g_clear_object(&mock); + g_clear_object(&service); + g_object_unref(bus); - virtual void TearDown() - { - super::TearDown(); - } + // 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)) + { + g_usleep(100000); + while (g_main_pending()) + g_main_iteration(true); + cleartry++; + } + + super::TearDown(); + } }; /*** @@ -53,7 +142,82 @@ class NotifyTest : public ::testing::Test // popup should appear exactly twice: once at 10%, once at 5% -TEST_F(NotifyTest, HelloWorld) +TEST_F(NotifyFixture, HelloWorld) { } +#if 0 +using namespace unity::indicator::datetime; + +/*** +**** +***/ + +using namespace unity::indicator::datetime; + +class SnapFixture: public GlibFixture +{ + +/*** +**** +***/ + +namespace +{ + gboolean quit_idle (gpointer gloop) + { + g_main_loop_quit(static_cast(gloop)); + return G_SOURCE_REMOVE; + }; +} + +TEST_F(SnapFixture, InteractiveDuration) +{ + static constexpr int duration_minutes = 120; + auto settings = std::make_shared(); + settings->alarm_duration.set(duration_minutes); + auto timezones = std::make_shared(); + auto clock = std::make_shared(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(duration).count(), i32); + g_variant_unref(hints); +} +#endif + -- cgit v1.2.3 From f5bf7f99724796a21dc05676d613ed47c624bfd1 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 21 Jul 2014 13:48:18 -0500 Subject: don't show clickable actions if the Notify server doesn't support them. --- src/notifier.c | 60 +++++++++++++++++++++++++++++++++++++++++----------------- src/notifier.h | 11 ++++++++--- 2 files changed, 51 insertions(+), 20 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index 1d9428c..06ab119 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -53,8 +53,6 @@ static GParamSpec * properties[LAST_PROP]; *** **/ -static int n_notifiers = 0; - struct _IndicatorPowerNotifierPrivate { /* The battery we're currently watching. @@ -110,22 +108,22 @@ static void notification_show(IndicatorPowerNotifier * self) { priv_t * p; - IndicatorPowerDevice * battery; char * body; NotifyNotification * nn; notification_clear (self); + /* only show clickable notifications if the Notify server supports them */ + if (!INDICATOR_POWER_NOTIFIER_GET_CLASS(self)->interactive) + return; + 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); + body = g_strdup_printf(_("%.0f%% charge remaining"), + indicator_power_device_get_percentage(p->battery)); + nn = notify_notification_new(_("Battery Low"), body, NULL); + p->notify_notification = nn; notify_notification_set_hint(nn, "x-canonical-snap-decisions", g_variant_new_boolean(TRUE)); notify_notification_set_hint(nn, "x-canonical-private-button-tint", @@ -272,9 +270,11 @@ my_dispose (GObject * o) } static void -my_finalize (GObject * o G_GNUC_UNUSED) +my_finalize (GObject * o) { - if (!--n_notifiers) + IndicatorPowerNotifierClass * klass = INDICATOR_POWER_NOTIFIER_GET_CLASS(o); + + if (!--klass->instance_count) notify_uninit(); } @@ -285,9 +285,12 @@ my_finalize (GObject * o G_GNUC_UNUSED) static void indicator_power_notifier_init (IndicatorPowerNotifier * self) { - priv_t * p = G_TYPE_INSTANCE_GET_PRIVATE (self, - INDICATOR_TYPE_POWER_NOTIFIER, - IndicatorPowerNotifierPrivate); + priv_t * p; + IndicatorPowerNotifierClass * klass; + + p = G_TYPE_INSTANCE_GET_PRIVATE (self, + INDICATOR_TYPE_POWER_NOTIFIER, + IndicatorPowerNotifierPrivate); self->priv = p; p->dbus_battery = dbus_battery_skeleton_new (); @@ -304,8 +307,28 @@ indicator_power_notifier_init (IndicatorPowerNotifier * self) POWER_LEVEL_NAME, G_BINDING_SYNC_CREATE); - if (!n_notifiers++ && !notify_init("indicator-power-service")) - g_critical("Unable to initialize libnotify! Notifications might not be shown."); + klass = INDICATOR_POWER_NOTIFIER_GET_CLASS(self); + + if (!klass->instance_count++) + { + if (!notify_init("indicator-power-service")) + { + g_critical("Unable to initialize libnotify! Notifications might not be shown."); + } + else + { + /* See if the notification server supports clickable actions... */ + GList * caps; + GList * l; + klass->interactive = FALSE; + caps = notify_get_server_caps(); + 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); + } + } } static void @@ -346,6 +369,9 @@ indicator_power_notifier_class_init (IndicatorPowerNotifierClass * klass) G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); g_object_class_install_properties (object_class, LAST_PROP, properties); + + klass->instance_count = 0; + klass->interactive = FALSE; } /*** diff --git a/src/notifier.h b/src/notifier.h index c1c5a1b..cab053f 100644 --- a/src/notifier.h +++ b/src/notifier.h @@ -29,9 +29,10 @@ G_BEGIN_DECLS /* standard GObject macros */ -#define INDICATOR_POWER_NOTIFIER(o) (G_TYPE_CHECK_INSTANCE_CAST ((o), INDICATOR_TYPE_POWER_NOTIFIER, IndicatorPowerNotifier)) -#define INDICATOR_TYPE_POWER_NOTIFIER (indicator_power_notifier_get_type()) -#define INDICATOR_IS_POWER_NOTIFIER(o) (G_TYPE_CHECK_INSTANCE_TYPE ((o), INDICATOR_TYPE_POWER_NOTIFIER)) +#define INDICATOR_POWER_NOTIFIER(o) (G_TYPE_CHECK_INSTANCE_CAST ((o), INDICATOR_TYPE_POWER_NOTIFIER, IndicatorPowerNotifier)) +#define INDICATOR_TYPE_POWER_NOTIFIER (indicator_power_notifier_get_type()) +#define INDICATOR_IS_POWER_NOTIFIER(o) (G_TYPE_CHECK_INSTANCE_TYPE ((o), INDICATOR_TYPE_POWER_NOTIFIER)) +#define INDICATOR_POWER_NOTIFIER_GET_CLASS(o) (G_TYPE_INSTANCE_GET_CLASS ((o), INDICATOR_TYPE_POWER_NOTIFIER, IndicatorPowerNotifierClass)) typedef struct _IndicatorPowerNotifier IndicatorPowerNotifier; typedef struct _IndicatorPowerNotifierClass IndicatorPowerNotifierClass; @@ -59,6 +60,10 @@ struct _IndicatorPowerNotifier struct _IndicatorPowerNotifierClass { GObjectClass parent_class; + + /*< private >*/ + gint instance_count; + gboolean interactive; }; /*** -- cgit v1.2.3 From 8c4b964ee9fd6578f3b922357c58b933bf14e6df Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 21 Jul 2014 14:15:28 -0500 Subject: copyediting --- src/notifier.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index 06ab119..9dd7550 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -43,9 +43,9 @@ enum LAST_PROP }; -#define BATTERY_NAME "battery" -#define IS_WARNING_NAME "is-warning" -#define POWER_LEVEL_NAME "power-level" +#define PROP_BATTERY_NAME "battery" +#define PROP_IS_WARNING_NAME "is-warning" +#define PROP_POWER_LEVEL_NAME "power-level" static GParamSpec * properties[LAST_PROP]; @@ -101,7 +101,7 @@ on_notification_clicked(NotifyNotification * notify_notification G_GNUC_UNUSED, char * action G_GNUC_UNUSED, gpointer gself G_GNUC_UNUSED) { - /* no-op because notify_notification_add_action() doesn't like a NULL cb */ + /* no-op; notify_notification_add_action() doesn't like NULL callbacks */ } static void @@ -110,6 +110,7 @@ notification_show(IndicatorPowerNotifier * self) priv_t * p; char * body; NotifyNotification * nn; + GError * error; notification_clear (self); @@ -133,7 +134,7 @@ notification_show(IndicatorPowerNotifier * self) g_signal_connect_swapped(nn, "closed", G_CALLBACK(notification_clear), self); /* show the notification */ - GError* error = NULL; + error = NULL; notify_notification_show(nn, &error); if (error != NULL) { @@ -158,6 +159,7 @@ 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 = 10.0; + const gdouble p = indicator_power_device_get_percentage(device); PowerLevel ret; @@ -296,15 +298,15 @@ indicator_power_notifier_init (IndicatorPowerNotifier * self) p->dbus_battery = dbus_battery_skeleton_new (); p->is_warning_binding = g_object_bind_property (self, - IS_WARNING_NAME, + PROP_IS_WARNING_NAME, p->dbus_battery, - IS_WARNING_NAME, + PROP_IS_WARNING_NAME, G_BINDING_SYNC_CREATE); p->power_level_binding = g_object_bind_property (self, - POWER_LEVEL_NAME, + PROP_POWER_LEVEL_NAME, p->dbus_battery, - POWER_LEVEL_NAME, + PROP_POWER_LEVEL_NAME, G_BINDING_SYNC_CREATE); klass = INDICATOR_POWER_NOTIFIER_GET_CLASS(self); @@ -346,14 +348,14 @@ indicator_power_notifier_class_init (IndicatorPowerNotifierClass * klass) properties[PROP_0] = NULL; properties[PROP_BATTERY] = g_param_spec_object ( - BATTERY_NAME, + PROP_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, + PROP_POWER_LEVEL_NAME, "Power Level", "The battery's power level", POWER_LEVEL_OK, @@ -362,7 +364,7 @@ indicator_power_notifier_class_init (IndicatorPowerNotifierClass * klass) G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); properties[PROP_IS_WARNING] = g_param_spec_boolean ( - IS_WARNING_NAME, + PROP_IS_WARNING_NAME, "Is Warning", "Whether or not we're currently warning the user about a low battery", FALSE, @@ -433,6 +435,7 @@ indicator_power_notifier_set_bus (IndicatorPowerNotifier * self, GDBusConnection * bus) { priv_t * p; + GDBusInterfaceSkeleton * skel; g_return_if_fail(INDICATOR_IS_POWER_NOTIFIER(self)); g_return_if_fail((bus == NULL) || G_IS_DBUS_CONNECTION(bus)); @@ -442,12 +445,12 @@ indicator_power_notifier_set_bus (IndicatorPowerNotifier * self, if (p->bus == bus) return; + skel = G_DBUS_INTERFACE_SKELETON(p->dbus_battery); + if (p->bus != NULL) { - if (p->dbus_battery != NULL) - { - g_dbus_interface_skeleton_unexport (G_DBUS_INTERFACE_SKELETON(p->dbus_battery)); - } + if (skel != NULL) + g_dbus_interface_skeleton_unexport (skel); g_clear_object (&p->bus); } @@ -459,7 +462,7 @@ indicator_power_notifier_set_bus (IndicatorPowerNotifier * self, p->bus = g_object_ref (bus); error = NULL; - g_dbus_interface_skeleton_export(G_DBUS_INTERFACE_SKELETON(p->dbus_battery), + g_dbus_interface_skeleton_export(skel, bus, BUS_PATH"/Battery", &error); -- cgit v1.2.3 From 2cb851b018c5e7a0278dab75f73bb031c7c42422 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 21 Jul 2014 16:08:29 -0500 Subject: 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 --- CMakeLists.txt | 3 +- src/notifier.c | 92 ++++++++++++++--------- src/notifier.h | 1 + tests/test-notify.cc | 208 ++++++++++++++++++++++++++++++++------------------- 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(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(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->alarm_duration.set(duration_minutes); - auto timezones = std::make_shared(); - auto clock = std::make_shared(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(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 -- cgit v1.2.3 From 1bd1b700892a786fd01410a4b81b2f2cc93a89c1 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 21 Jul 2014 17:13:15 -0500 Subject: add tests for events that change whether or not a 'low battery' notification is being shown --- src/notifier.c | 23 +++++-------- tests/test-notify.cc | 97 ++++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 100 insertions(+), 20 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index c805413..fc54e62 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -157,28 +157,23 @@ static void on_battery_property_changed (IndicatorPowerNotifier * self) { priv_t * p; - PowerLevel power_level; g_return_if_fail(INDICATOR_IS_POWER_NOTIFIER(self)); g_return_if_fail(INDICATOR_IS_POWER_DEVICE(self->priv->battery)); p = self->priv; - power_level = indicator_power_notifier_get_power_level (p->battery); + set_power_level_property (self, + indicator_power_notifier_get_power_level (p->battery)); - if (p->power_level != power_level) + if ((indicator_power_device_get_state(p->battery) == UP_DEVICE_STATE_DISCHARGING) && + (p->power_level != POWER_LEVEL_OK)) { - 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); - } + notification_show (self); + } + else + { + notification_clear (self); } } diff --git a/tests/test-notify.cc b/tests/test-notify.cc index cf1f9d3..d5d7e9f 100644 --- a/tests/test-notify.cc +++ b/tests/test-notify.cc @@ -47,12 +47,11 @@ private: static constexpr char const * NOTIFY_INTERFACE {"org.freedesktop.Notifications"}; static constexpr char const * NOTIFY_PATH {"/org/freedesktop/Notifications"}; +protected: + DbusTestService * service = nullptr; DbusTestDbusMock * mock = nullptr; DbusTestDbusMockObject * obj = nullptr; - -protected: - GDBusConnection * bus = nullptr; static constexpr int NOTIFY_ID {1234}; @@ -189,6 +188,10 @@ namespace g_variant_unref (changed_properties); } + + static const double percent_critical = 2.0; + static const double percent_very_low = 5.0; + static const double percent_low = 10.0; } TEST_F(NotifyFixture, PercentageToLevel) @@ -205,11 +208,11 @@ TEST_F(NotifyFixture, PercentageToLevel) g_object_set (battery, INDICATOR_POWER_DEVICE_PERCENTAGE, (gdouble)i, nullptr); const auto level = indicator_power_notifier_get_power_level(battery); - if (i <= 2) + if (i <= percent_critical) EXPECT_EQ (POWER_LEVEL_CRITICAL, level); - else if (i <= 5) + else if (i <= percent_very_low) EXPECT_EQ (POWER_LEVEL_VERY_LOW, level); - else if (i <= 10) + else if (i <= percent_low) EXPECT_EQ (POWER_LEVEL_LOW, level); else EXPECT_EQ (POWER_LEVEL_OK, level); @@ -275,3 +278,85 @@ TEST_F(NotifyFixture, LevelsDuringBatteryDrain) g_object_unref (battery); } + +// confirm that notifications pop up +// when a discharging battery's power level drops +TEST_F(NotifyFixture, EventsThatChangeNotifications) +{ + // GetCapabilities returns an array containing 'actions', so that we'll + // get snap decisions and the 'IsWarning' property + 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); + + auto battery = indicator_power_device_new ("/object/path", + UP_DEVICE_KIND_BATTERY, + percent_low + 1.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); + 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); + + // test setup case + wait_msec(); + EXPECT_EQ (0, changed_params.power_level); + + // change the percent past the 'low' threshold and confirm that + // a) the power level changes + // b) we get a notification + changed_params = ChangedParams(); + g_object_set (battery, INDICATOR_POWER_DEVICE_PERCENTAGE, (gdouble)percent_low, nullptr); + wait_msec(); + EXPECT_EQ (FIELD_POWER_LEVEL|FIELD_IS_WARNING, changed_params.fields); + EXPECT_EQ (indicator_power_notifier_get_power_level(battery), changed_params.power_level); + EXPECT_TRUE (changed_params.is_warning); + + // now test that the warning changes if the level goes down even lower... + changed_params = ChangedParams(); + g_object_set (battery, INDICATOR_POWER_DEVICE_PERCENTAGE, (gdouble)percent_very_low, nullptr); + wait_msec(); + EXPECT_EQ (FIELD_POWER_LEVEL, changed_params.fields); + EXPECT_EQ (POWER_LEVEL_VERY_LOW, changed_params.power_level); + + // ...and that the warning is taken down if the battery is plugged back in... + changed_params = ChangedParams(); + g_object_set (battery, INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_CHARGING, nullptr); + wait_msec(); + EXPECT_EQ (FIELD_IS_WARNING, changed_params.fields); + EXPECT_FALSE (changed_params.is_warning); + + // ...and that it comes back if we unplug again... + changed_params = ChangedParams(); + g_object_set (battery, INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING, nullptr); + wait_msec(); + EXPECT_EQ (FIELD_IS_WARNING, changed_params.fields); + EXPECT_TRUE (changed_params.is_warning); + + // ...and that it's taken down if the power level is OK + changed_params = ChangedParams(); + g_object_set (battery, INDICATOR_POWER_DEVICE_PERCENTAGE, (gdouble)percent_low+1, nullptr); + wait_msec(); + EXPECT_EQ (FIELD_POWER_LEVEL|FIELD_IS_WARNING, changed_params.fields); + EXPECT_EQ (POWER_LEVEL_OK, changed_params.power_level); + EXPECT_FALSE (changed_params.is_warning); + + // cleanup + g_dbus_connection_signal_unsubscribe (bus, subscription_tag); + g_object_unref (notifier); + g_object_unref (battery); +} -- cgit v1.2.3 From 51f39c86f79fef81af296ee03d12a1883b7e015f Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 21 Jul 2014 17:27:49 -0500 Subject: in notifier.c, use g_clear_object to unref the GBindings --- src/notifier.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index fc54e62..80db10d 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -264,8 +264,8 @@ my_dispose (GObject * o) indicator_power_notifier_set_bus (self, NULL); notification_clear (self); - g_clear_pointer (&p->power_level_binding, g_binding_unbind); - g_clear_pointer (&p->is_warning_binding, g_binding_unbind); + g_clear_object (&p->power_level_binding); + g_clear_object (&p->is_warning_binding); g_clear_object (&p->dbus_battery); indicator_power_notifier_set_battery (self, NULL); -- cgit v1.2.3 From 34925a76df4ff222f02a65adec9edf1f626c45ad Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 21 Jul 2014 18:04:55 -0500 Subject: in debian/control, add libdbustest1-dev to Build-Deps for tests --- debian/control | 1 + 1 file changed, 1 insertion(+) diff --git a/debian/control b/debian/control index 487a244..228d305 100644 --- a/debian/control +++ b/debian/control @@ -14,6 +14,7 @@ Build-Depends: cmake, intltool, # for tests libgtest-dev, + libdbustest1-dev, dbus-test-runner, Standards-Version: 3.9.2 Homepage: https://launchpad.net/indicator-power -- cgit v1.2.3 From f5db19df10ace0919ec13f83a751cc891d8a1741 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 21 Jul 2014 18:12:11 -0500 Subject: in debian/control, add python3-dbusmock to Build-Deps for tests --- debian/control | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/debian/control b/debian/control index 228d305..00c0864 100644 --- a/debian/control +++ b/debian/control @@ -14,8 +14,9 @@ Build-Depends: cmake, intltool, # for tests libgtest-dev, - libdbustest1-dev, + python3-dbusmock, dbus-test-runner, + libdbustest1-dev, Standards-Version: 3.9.2 Homepage: https://launchpad.net/indicator-power # If you aren't a member of ~indicator-applet-developers but need to upload -- cgit v1.2.3 From 9c3d863d5e0ebe35ae69dedb5219519f0ced9339 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 22 Jul 2014 09:53:53 -0500 Subject: copyediting: code cleanup --- src/notifier.c | 18 ++++-- tests/test-notify.cc | 163 +++++++++++++++++++++++++++++---------------------- 2 files changed, 106 insertions(+), 75 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index 80db10d..bf52739 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -73,8 +73,11 @@ struct _IndicatorPowerNotifierPrivate typedef IndicatorPowerNotifierPrivate priv_t; -static void set_is_warning_property (IndicatorPowerNotifier*, gboolean is_warning); -static void set_power_level_property (IndicatorPowerNotifier*, PowerLevel power_level); +static void set_is_warning_property (IndicatorPowerNotifier*, + gboolean is_warning); + +static void set_power_level_property (IndicatorPowerNotifier*, + PowerLevel power_level); /*** **** Notifications @@ -101,7 +104,7 @@ on_notification_clicked(NotifyNotification * notify_notification G_GNUC_UNUSED, char * action G_GNUC_UNUSED, gpointer gself G_GNUC_UNUSED) { - /* no-op; notify_notification_add_action() doesn't like NULL callbacks */ + /* no-op: notify_notification_add_action() doesn't like NULL callbacks */ } static void @@ -183,9 +186,9 @@ on_battery_property_changed (IndicatorPowerNotifier * self) static void my_get_property (GObject * o, - guint property_id, - GValue * value, - GParamSpec * pspec) + guint property_id, + GValue * value, + GParamSpec * pspec) { IndicatorPowerNotifier * self = INDICATOR_POWER_NOTIFIER (o); priv_t * p = self->priv; @@ -296,6 +299,8 @@ indicator_power_notifier_init (IndicatorPowerNotifier * self) IndicatorPowerNotifierPrivate); self->priv = p; + /* bind the read-only properties so they'll get pushed to the bus */ + p->dbus_battery = dbus_battery_skeleton_new (); p->is_warning_binding = g_object_bind_property (self, @@ -330,6 +335,7 @@ indicator_power_notifier_init (IndicatorPowerNotifier * self) klass->interactive = TRUE; g_list_free_full (caps, g_free); } + g_debug ("Will show popups on low battery: %d", (int)klass->interactive); } } diff --git a/tests/test-notify.cc b/tests/test-notify.cc index d5d7e9f..74a08dc 100644 --- a/tests/test-notify.cc +++ b/tests/test-notify.cc @@ -81,13 +81,16 @@ protected: GError * error = nullptr; mock = dbus_test_dbus_mock_new(NOTIFY_BUSNAME); - obj = dbus_test_dbus_mock_get_object(mock, NOTIFY_PATH, NOTIFY_INTERFACE, &error); + 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, nullptr, G_VARIANT_TYPE("(ssss)"), - "ret = ('mock-notify', 'test vendor', '1.0', '1.1')", // python + "ret = ('mock-notify', 'test vendor', '1.0', '1.1')", &error); g_assert_no_error (error); @@ -142,8 +145,55 @@ TEST_F(NotifyFixture, HelloWorld) { } +/*** +**** +***/ + + +namespace +{ + static constexpr double percent_critical {2.0}; + static constexpr double percent_very_low {5.0}; + static constexpr double percent_low {10.0}; + + void set_battery_percentage (IndicatorPowerDevice * battery, gdouble p) + { + g_object_set (battery, INDICATOR_POWER_DEVICE_PERCENTAGE, p, nullptr); + } +} + +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) + { + set_battery_percentage (battery, i); + const auto level = indicator_power_notifier_get_power_level(battery); + + if (i <= percent_critical) + EXPECT_EQ (POWER_LEVEL_CRITICAL, level); + else if (i <= percent_very_low) + EXPECT_EQ (POWER_LEVEL_VERY_LOW, level); + else if (i <= percent_low) + EXPECT_EQ (POWER_LEVEL_LOW, level); + else + EXPECT_EQ (POWER_LEVEL_OK, level); + } + + g_object_unref (battery); +} + +/*** +**** +***/ -// scaffolding to listen for PropertyChanged signals and remember them +// scaffolding to monitor PropertyChanged signals namespace { enum @@ -168,60 +218,28 @@ namespace 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 dict = g_variant_get_child_value (parameters, 1); + g_return_if_fail (g_variant_is_of_type (dict, G_VARIANT_TYPE_DICTIONARY)); auto changed_params = static_cast(gchanged_params); gint32 power_level; - if (g_variant_lookup (changed_properties, "PowerLevel", "i", &power_level, nullptr)) + if (g_variant_lookup (dict, "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)) + if (g_variant_lookup (dict, "IsWarning", "b", &is_warning, nullptr)) { changed_params->is_warning = is_warning; changed_params->fields |= FIELD_IS_WARNING; } - g_variant_unref (changed_properties); + g_variant_unref (dict); } - - static const double percent_critical = 2.0; - static const double percent_very_low = 5.0; - static const double percent_low = 10.0; -} - -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 <= percent_critical) - EXPECT_EQ (POWER_LEVEL_CRITICAL, level); - else if (i <= percent_very_low) - EXPECT_EQ (POWER_LEVEL_VERY_LOW, level); - else if (i <= percent_low) - EXPECT_EQ (POWER_LEVEL_LOW, level); - else - EXPECT_EQ (POWER_LEVEL_OK, level); - } - - g_object_unref (battery); } - TEST_F(NotifyFixture, LevelsDuringBatteryDrain) { auto battery = indicator_power_device_new ("/object/path", @@ -238,16 +256,16 @@ TEST_F(NotifyFixture, LevelsDuringBatteryDrain) 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); + auto sub_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 @@ -257,7 +275,7 @@ TEST_F(NotifyFixture, LevelsDuringBatteryDrain) 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); + set_battery_percentage (battery, i); const auto new_level = indicator_power_notifier_get_power_level(battery); wait_msec(); @@ -273,20 +291,27 @@ TEST_F(NotifyFixture, LevelsDuringBatteryDrain) } // cleanup - g_dbus_connection_signal_unsubscribe (bus, subscription_tag); + g_dbus_connection_signal_unsubscribe (bus, sub_tag); g_object_unref (notifier); g_object_unref (battery); } +/*** +**** +***/ -// confirm that notifications pop up -// when a discharging battery's power level drops TEST_F(NotifyFixture, EventsThatChangeNotifications) { // GetCapabilities returns an array containing 'actions', so that we'll // get snap decisions and the 'IsWarning' property 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); + 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); auto battery = indicator_power_device_new ("/object/path", @@ -301,16 +326,16 @@ TEST_F(NotifyFixture, EventsThatChangeNotifications) indicator_power_notifier_set_battery (notifier, battery); indicator_power_notifier_set_bus (notifier, bus); 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); + auto sub_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); // test setup case wait_msec(); @@ -320,7 +345,7 @@ TEST_F(NotifyFixture, EventsThatChangeNotifications) // a) the power level changes // b) we get a notification changed_params = ChangedParams(); - g_object_set (battery, INDICATOR_POWER_DEVICE_PERCENTAGE, (gdouble)percent_low, nullptr); + set_battery_percentage (battery, percent_low); wait_msec(); EXPECT_EQ (FIELD_POWER_LEVEL|FIELD_IS_WARNING, changed_params.fields); EXPECT_EQ (indicator_power_notifier_get_power_level(battery), changed_params.power_level); @@ -328,7 +353,7 @@ TEST_F(NotifyFixture, EventsThatChangeNotifications) // now test that the warning changes if the level goes down even lower... changed_params = ChangedParams(); - g_object_set (battery, INDICATOR_POWER_DEVICE_PERCENTAGE, (gdouble)percent_very_low, nullptr); + set_battery_percentage (battery, percent_very_low); wait_msec(); EXPECT_EQ (FIELD_POWER_LEVEL, changed_params.fields); EXPECT_EQ (POWER_LEVEL_VERY_LOW, changed_params.power_level); @@ -349,14 +374,14 @@ TEST_F(NotifyFixture, EventsThatChangeNotifications) // ...and that it's taken down if the power level is OK changed_params = ChangedParams(); - g_object_set (battery, INDICATOR_POWER_DEVICE_PERCENTAGE, (gdouble)percent_low+1, nullptr); + set_battery_percentage (battery, percent_low+1); wait_msec(); EXPECT_EQ (FIELD_POWER_LEVEL|FIELD_IS_WARNING, changed_params.fields); EXPECT_EQ (POWER_LEVEL_OK, changed_params.power_level); EXPECT_FALSE (changed_params.is_warning); // cleanup - g_dbus_connection_signal_unsubscribe (bus, subscription_tag); + g_dbus_connection_signal_unsubscribe (bus, sub_tag); g_object_unref (notifier); g_object_unref (battery); } -- cgit v1.2.3 From dd5f50b07bea7a5dec515a753cf66f060e1d79c6 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 22 Jul 2014 09:54:36 -0500 Subject: when a device changes, update the header action state even if the menus haven't been created yet. This fixes a startup condition found by the tests. --- src/service.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/service.c b/src/service.c index 23cef84..0cd448b 100644 --- a/src/service.c +++ b/src/service.c @@ -595,14 +595,14 @@ rebuild_now (IndicatorPowerService * self, guint sections) struct ProfileMenuInfo * desktop = &p->menus[PROFILE_DESKTOP]; struct ProfileMenuInfo * greeter = &p->menus[PROFILE_DESKTOP_GREETER]; - if (p->conn == NULL) /* we haven't built the menus yet */ - return; - if (sections & SECTION_HEADER) { g_simple_action_set_state (p->header_action, create_header_state (self)); } + if (p->conn == NULL) /* we haven't built the menus yet */ + return; + if (sections & SECTION_DEVICES) { rebuild_section (desktop->submenu, 0, create_desktop_devices_section (self, PROFILE_DESKTOP)); -- cgit v1.2.3 From 399eab24252eb7c83042de726ebbc4cf9fbb7637 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 22 Jul 2014 09:58:29 -0500 Subject: add indicator-power-service-cmdline-battery, a manual test utility to test the indicator's behavior when a battery's state changes --- tests/CMakeLists.txt | 8 ++ tests/device-provider-mock.c | 107 +++++++++++++++++++ tests/device-provider-mock.h | 79 +++++++++++++++ tests/indicator-power-service-cmdline-battery.cc | 124 +++++++++++++++++++++++ 4 files changed, 318 insertions(+) create mode 100644 tests/device-provider-mock.c create mode 100644 tests/device-provider-mock.h create mode 100644 tests/indicator-power-service-cmdline-battery.cc diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 64b8ed8..a0d24af 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -51,3 +51,11 @@ add_test_by_name(test-notify) add_test(NAME dear-reader-the-next-test-takes-80-seconds COMMAND true) add_test_by_name(test-device) +### +### + +set (APP_NAME indicator-power-service-cmdline-battery) +add_executable (${APP_NAME} ${APP_NAME}.cc device-provider-mock.c) +add_dependencies (${APP_NAME} libindicatorpowerservice) +target_link_libraries (${APP_NAME} indicatorpowerservice ${SERVICE_DEPS_LIBRARIES}) + diff --git a/tests/device-provider-mock.c b/tests/device-provider-mock.c new file mode 100644 index 0000000..afca178 --- /dev/null +++ b/tests/device-provider-mock.c @@ -0,0 +1,107 @@ +/* + * Copyright 2014 Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 3, as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranties of + * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR + * PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + * + * Authors: + * Charles Kerr + */ + +#include "device.h" +#include "device-provider.h" +#include "device-provider-mock.h" + +/*** +**** GObject boilerplate +***/ + +static void indicator_power_device_provider_interface_init ( + IndicatorPowerDeviceProviderInterface * iface); + +G_DEFINE_TYPE_WITH_CODE ( + IndicatorPowerDeviceProviderMock, + indicator_power_device_provider_mock, + G_TYPE_OBJECT, + G_IMPLEMENT_INTERFACE (INDICATOR_TYPE_POWER_DEVICE_PROVIDER, + indicator_power_device_provider_interface_init)) + +/*** +**** IndicatorPowerDeviceProvider virtual functions +***/ + +static GList * +my_get_devices (IndicatorPowerDeviceProvider * provider) +{ + IndicatorPowerDeviceProviderMock * self = INDICATOR_POWER_DEVICE_PROVIDER_MOCK(provider); + + return g_list_copy_deep (self->devices, (GCopyFunc)g_object_ref, NULL); +} + +/*** +**** GObject virtual functions +***/ + +static void +my_dispose (GObject * o) +{ + IndicatorPowerDeviceProviderMock * self = INDICATOR_POWER_DEVICE_PROVIDER_MOCK(o); + + g_list_free_full (self->devices, g_object_unref); + + G_OBJECT_CLASS (indicator_power_device_provider_mock_parent_class)->dispose (o); +} + +/*** +**** Instantiation +***/ + +static void +indicator_power_device_provider_mock_class_init (IndicatorPowerDeviceProviderMockClass * klass) +{ + GObjectClass * object_class; + + object_class = G_OBJECT_CLASS (klass); + object_class->dispose = my_dispose; +} + +static void +indicator_power_device_provider_interface_init (IndicatorPowerDeviceProviderInterface * iface) +{ + iface->get_devices = my_get_devices; +} + +static void +indicator_power_device_provider_mock_init (IndicatorPowerDeviceProviderMock * self) +{ +} + +/*** +**** Public API +***/ + +IndicatorPowerDeviceProvider * +indicator_power_device_provider_mock_new (void) +{ + gpointer o = g_object_new (INDICATOR_TYPE_POWER_DEVICE_PROVIDER_MOCK, NULL); + + return INDICATOR_POWER_DEVICE_PROVIDER (o); +} + +void +indicator_power_device_provider_add_device (IndicatorPowerDeviceProviderMock * provider, + IndicatorPowerDevice * device) +{ + provider->devices = g_list_append (provider->devices, g_object_ref(device)); + + g_signal_connect_swapped (device, "notify", G_CALLBACK(indicator_power_device_provider_emit_devices_changed), provider); +} diff --git a/tests/device-provider-mock.h b/tests/device-provider-mock.h new file mode 100644 index 0000000..4d06924 --- /dev/null +++ b/tests/device-provider-mock.h @@ -0,0 +1,79 @@ +/* + * Copyright 2014 Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 3, as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranties of + * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR + * PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + * + * Authors: + * Charles Kerr + */ + +#ifndef __INDICATOR_POWER_DEVICE_PROVIDER_MOCK__H__ +#define __INDICATOR_POWER_DEVICE_PROVIDER_MOCK__H__ + +#include /* parent class */ + +#include "device.h" +#include "device-provider.h" + +G_BEGIN_DECLS + +#define INDICATOR_TYPE_POWER_DEVICE_PROVIDER_MOCK \ + (indicator_power_device_provider_mock_get_type()) + +#define INDICATOR_POWER_DEVICE_PROVIDER_MOCK(o) \ + (G_TYPE_CHECK_INSTANCE_CAST ((o), \ + INDICATOR_TYPE_POWER_DEVICE_PROVIDER_MOCK, \ + IndicatorPowerDeviceProviderMock)) + +#define INDICATOR_POWER_DEVICE_PROVIDER_MOCK_GET_CLASS(o) \ + (G_TYPE_INSTANCE_GET_CLASS ((o), \ + INDICATOR_TYPE_POWER_DEVICE_PROVIDER_MOCK, \ + IndicatorPowerDeviceProviderMockClass)) + +#define INDICATOR_IS_POWER_DEVICE_PROVIDER_MOCK(o) \ + (G_TYPE_CHECK_INSTANCE_TYPE ((o), \ + INDICATOR_TYPE_POWER_DEVICE_PROVIDER_MOCK)) + +typedef struct _IndicatorPowerDeviceProviderMock + IndicatorPowerDeviceProviderMock; +typedef struct _IndicatorPowerDeviceProviderMockPriv + IndicatorPowerDeviceProviderMockPriv; +typedef struct _IndicatorPowerDeviceProviderMockClass + IndicatorPowerDeviceProviderMockClass; + +/** + * An IndicatorPowerDeviceProvider which gets its devices from Mock. + */ +struct _IndicatorPowerDeviceProviderMock +{ + GObject parent_instance; + + /*< private >*/ + GList * devices; +}; + +struct _IndicatorPowerDeviceProviderMockClass +{ + GObjectClass parent_class; +}; + +GType indicator_power_device_provider_mock_get_type (void); + +IndicatorPowerDeviceProvider * indicator_power_device_provider_mock_new (void); + +void indicator_power_device_provider_add_device (IndicatorPowerDeviceProviderMock * provider, + IndicatorPowerDevice * device); + +G_END_DECLS + +#endif /* __INDICATOR_POWER_DEVICE_PROVIDER_MOCK__H__ */ diff --git a/tests/indicator-power-service-cmdline-battery.cc b/tests/indicator-power-service-cmdline-battery.cc new file mode 100644 index 0000000..a7a86a1 --- /dev/null +++ b/tests/indicator-power-service-cmdline-battery.cc @@ -0,0 +1,124 @@ +/* + * Copyright 2014 Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 3, as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranties of + * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR + * PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + * + * Authors: + * Charles Kerr + */ + +#include + +#include // setlocale() +#include // bindtextdomain() +#include // STDIN_FILENO + +#include + +#include "device-provider-mock.h" + +#include "service.h" + +/*** +**** +***/ + +static void +on_name_lost (gpointer instance G_GNUC_UNUSED, gpointer loop) +{ + g_message ("exiting: service couldn't acquire or lost ownership of busname"); + g_main_loop_quit ((GMainLoop*)loop); +} + +static IndicatorPowerDevice * battery = nullptr; + +static GMainLoop * loop = nullptr; + +static gboolean on_command_stream_available (GIOChannel *source, + GIOCondition /*condition*/, + gpointer /*user_data*/) +{ + gchar * str = nullptr; + GError * error = nullptr; + auto status = g_io_channel_read_line (source, &str, nullptr, nullptr, &error); + g_assert_no_error (error); + + if (status == G_IO_STATUS_NORMAL) + { + g_strstrip (str); + + if (!g_strcmp0 (str, "charging")) + { + g_object_set (battery, INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_CHARGING, nullptr); + } + else if (!g_strcmp0 (str, "discharging")) + { + g_object_set (battery, INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING, nullptr); + } + else + { + g_object_set (battery, INDICATOR_POWER_DEVICE_PERCENTAGE, atof(str), nullptr); + } + } + else if (status == G_IO_STATUS_EOF) + { + g_main_loop_quit (loop); + } + + g_free (str); + return G_SOURCE_CONTINUE; +} + +/* this is basically indicator-power-service with a custom provider */ +int +main (int argc G_GNUC_UNUSED, char ** argv G_GNUC_UNUSED) +{ + g_message ("This app is basically the same as indicator-power-service but,\n" + "instead of the system's real devices, sees a single fake battery\n" + "which can be manipulated by typing commands:\n" + "'charging', 'discharging', a charge percentage, or ctrl-c."); + + IndicatorPowerDeviceProvider * device_provider; + IndicatorPowerService * service; + + g_assert(g_setenv("GSETTINGS_SCHEMA_DIR", SCHEMA_DIR, true)); + g_assert(g_setenv("GSETTINGS_BACKEND", "memory", true)); + + /* boilerplate i18n */ + setlocale (LC_ALL, ""); + bindtextdomain (GETTEXT_PACKAGE, GNOMELOCALEDIR); + textdomain (GETTEXT_PACKAGE); + + /* read lines from the command line */ + auto channel = g_io_channel_unix_new (STDIN_FILENO); + auto watch_tag = g_io_add_watch (channel, G_IO_IN, on_command_stream_available, nullptr); + + /* run */ + battery = indicator_power_device_new ("/some/path", UP_DEVICE_KIND_BATTERY, 50.0, UP_DEVICE_STATE_DISCHARGING, 30*60); + device_provider = indicator_power_device_provider_mock_new (); + indicator_power_device_provider_add_device (INDICATOR_POWER_DEVICE_PROVIDER_MOCK(device_provider), battery); + service = indicator_power_service_new (device_provider); + loop = g_main_loop_new (NULL, FALSE); + g_signal_connect (service, INDICATOR_POWER_SERVICE_SIGNAL_NAME_LOST, + G_CALLBACK(on_name_lost), loop); + g_main_loop_run (loop); + + /* cleanup */ + g_main_loop_unref (loop); + g_source_remove (watch_tag); + g_io_channel_unref (channel); + g_clear_object (&service); + g_clear_object (&device_provider); + g_clear_object (&battery); + return 0; +} -- cgit v1.2.3 From 7ecf18c675f4cc65d6e8314e492dba214ec6ae9c Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 22 Jul 2014 11:27:07 -0500 Subject: for now, only show Ephemeral notifications when the battery gets low. --- src/notifier.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index bf52739..2d2730e 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -99,46 +99,36 @@ notification_clear (IndicatorPowerNotifier * self) } } -static void -on_notification_clicked(NotifyNotification * notify_notification G_GNUC_UNUSED, - char * action G_GNUC_UNUSED, - gpointer gself G_GNUC_UNUSED) -{ - /* no-op: notify_notification_add_action() doesn't like NULL callbacks */ -} - static void notification_show(IndicatorPowerNotifier * self) { priv_t * p; char * body; - NotifyNotification * nn; GError * error; notification_clear (self); +#if 0 /* using Ephemeral no-button notifications for right now; + however this will likely change so I'm not tearing the + NotifierClass.interactive code out just yet */ + /* only show clickable notifications if the Notify server supports them */ if (!INDICATOR_POWER_NOTIFIER_GET_CLASS(self)->interactive) return; +#endif p = self->priv; /* create the notification */ body = g_strdup_printf(_("%.0f%% charge remaining"), indicator_power_device_get_percentage(p->battery)); - nn = notify_notification_new(_("Battery Low"), body, NULL); - p->notify_notification = nn; - 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); + p->notify_notification = notify_notification_new(_("Battery Low"), body, NULL); + g_signal_connect_swapped(p->notify_notification, "closed", + G_CALLBACK(notification_clear), self); /* show the notification */ error = NULL; - notify_notification_show(nn, &error); + notify_notification_show(p->notify_notification, &error); if (error != NULL) { g_critical("Unable to show snap decision for '%s': %s", body, error->message); -- cgit v1.2.3 From 584aa80f415597a2970bfd29fa0d48b9a8842086 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 22 Jul 2014 11:59:03 -0500 Subject: Make the when-does-power-level-change tests work whether or not Ephemeral notifications are being used. --- tests/test-notify.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test-notify.cc b/tests/test-notify.cc index 74a08dc..1fc843e 100644 --- a/tests/test-notify.cc +++ b/tests/test-notify.cc @@ -281,11 +281,11 @@ TEST_F(NotifyFixture, LevelsDuringBatteryDrain) if (old_level == new_level) { - EXPECT_EQ (0, changed_params.fields); + EXPECT_EQ (0, (changed_params.fields & FIELD_POWER_LEVEL)); } else { - EXPECT_EQ (FIELD_POWER_LEVEL, changed_params.fields); + EXPECT_EQ (FIELD_POWER_LEVEL, (changed_params.fields & FIELD_POWER_LEVEL)); EXPECT_EQ (new_level, changed_params.power_level); } } -- cgit v1.2.3 From 79a3d892c39ba831e44871ca99a064084bd792e9 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 22 Jul 2014 11:59:59 -0500 Subject: Filter out some redundant warnings -- e.g., don't notify when the power percentage drops from 10% to 9%, only when it crosses a PowerLevel threshold --- src/notifier.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index 2d2730e..326f5a1 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -61,6 +61,7 @@ struct _IndicatorPowerNotifierPrivate See indicator_power_service_choose_primary_device() */ IndicatorPowerDevice * battery; PowerLevel power_level; + gboolean discharging; NotifyNotification* notify_notification; gboolean is_warning; @@ -150,17 +151,25 @@ static void on_battery_property_changed (IndicatorPowerNotifier * self) { priv_t * p; + PowerLevel old_power_level; + PowerLevel new_power_level; + gboolean old_discharging; + gboolean new_discharging; g_return_if_fail(INDICATOR_IS_POWER_NOTIFIER(self)); g_return_if_fail(INDICATOR_IS_POWER_DEVICE(self->priv->battery)); p = self->priv; - set_power_level_property (self, - indicator_power_notifier_get_power_level (p->battery)); + old_power_level = p->power_level; + new_power_level = indicator_power_notifier_get_power_level (p->battery); - if ((indicator_power_device_get_state(p->battery) == UP_DEVICE_STATE_DISCHARGING) && - (p->power_level != POWER_LEVEL_OK)) + old_discharging = p->discharging; + new_discharging = indicator_power_device_get_state(p->battery) == UP_DEVICE_STATE_DISCHARGING; + + if (new_discharging && + (new_power_level != POWER_LEVEL_OK) && + ((new_power_level != old_power_level) || (new_discharging != old_discharging))) { notification_show (self); } @@ -168,6 +177,9 @@ on_battery_property_changed (IndicatorPowerNotifier * self) { notification_clear (self); } + + set_power_level_property (self, new_power_level); + p->discharging = new_discharging; } /*** -- cgit v1.2.3 From dd322344959c31ebbda939bf74547ed85e3233f9 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 22 Jul 2014 12:01:45 -0500 Subject: add manual test case for using the indicator-power-service-cmdline-battery test utility --- tests/manual | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/manual b/tests/manual index d3a22e1..a542cac 100644 --- a/tests/manual +++ b/tests/manual @@ -22,3 +22,20 @@ Test-case indicator-power/unity8-items-check
The menu is populated with items
+Test-case indicator-power/low-battery-popups +
+
Open a terminal
+
Stop the currently-running power indicator: "stop indicator-power"
+
Start the fake battery harness in the tests/build/ directory: "indicator-power-service-cmdline-battery"
+
Battery indicator should update, showing a discharging battery with a 50% charge
+
Type: "10" (no quotes) and press Enter
+
A popup should appear saying 'Battery low - 10% charge remaining'
+
Battery indicator's icon should show a low charge
+
Battery indicator's "Charge level" menuitem should show a 10% charge
+
Type: "9" (no quotes) and press Enter
+
The 'Battery low' popup should NOT appear, since we've already been notified
+
Battery indicator's "Charge level" menuitem should show a 9% charge
+
Type: "5" (no quotes) and press Enter
+
No 'Battery low' popup SHOULD appear, since 5% is the next warning threshold
+
Battery indicator's "Charge level" menuitem should show a 5% charge
+
-- cgit v1.2.3 From 1f98321fa6139105a1b427a2dac2b4e2b0642dd6 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 22 Jul 2014 12:19:20 -0500 Subject: make the notification popup decision logic simpler & easier to read --- src/notifier.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index 326f5a1..81662ca 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -167,9 +167,11 @@ on_battery_property_changed (IndicatorPowerNotifier * self) old_discharging = p->discharging; new_discharging = indicator_power_device_get_state(p->battery) == UP_DEVICE_STATE_DISCHARGING; - if (new_discharging && - (new_power_level != POWER_LEVEL_OK) && - ((new_power_level != old_power_level) || (new_discharging != old_discharging))) + /* pop up a notification for a battery whenever either: + a) it's already discharging, and its PowerLevel worsens, OR + b) it's already got a bad PowerLevel and its state becomes 'discharging */ + if ((new_discharging && (new_power_level > old_power_level)) || + ((new_power_level != POWER_LEVEL_OK) && new_discharging && !old_discharging)) { notification_show (self); } -- cgit v1.2.3 From a8ea5a0304e5ee14ee93532f0bc960c99039356a Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 23 Jul 2014 16:04:02 -0500 Subject: remove now-unused code that tests whether the Notify server supports actions --- src/notifier.c | 23 ----------------------- src/notifier.h | 1 - 2 files changed, 24 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index 81662ca..f9f3ff6 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -109,15 +109,6 @@ notification_show(IndicatorPowerNotifier * self) notification_clear (self); -#if 0 /* using Ephemeral no-button notifications for right now; - however this will likely change so I'm not tearing the - NotifierClass.interactive code out just yet */ - - /* only show clickable notifications if the Notify server supports them */ - if (!INDICATOR_POWER_NOTIFIER_GET_CLASS(self)->interactive) - return; -#endif - p = self->priv; /* create the notification */ @@ -327,19 +318,6 @@ indicator_power_notifier_init (IndicatorPowerNotifier * self) { g_critical("Unable to initialize libnotify! Notifications might not be shown."); } - else - { - /* See if the notification server supports clickable actions... */ - GList * caps; - GList * l; - klass->interactive = FALSE; - caps = notify_get_server_caps(); - for (l=caps; l!=NULL && !klass->interactive; l=l->next) - if (!g_strcmp0 ("actions", (const char*)l->data)) - klass->interactive = TRUE; - g_list_free_full (caps, g_free); - } - g_debug ("Will show popups on low battery: %d", (int)klass->interactive); } } @@ -383,7 +361,6 @@ indicator_power_notifier_class_init (IndicatorPowerNotifierClass * klass) g_object_class_install_properties (object_class, LAST_PROP, properties); klass->instance_count = 0; - klass->interactive = FALSE; } /*** diff --git a/src/notifier.h b/src/notifier.h index 0e3ad99..dddb6e9 100644 --- a/src/notifier.h +++ b/src/notifier.h @@ -63,7 +63,6 @@ struct _IndicatorPowerNotifierClass /*< private >*/ gint instance_count; - gboolean interactive; }; /*** -- cgit v1.2.3 From a62291be0e2ae4e16647d3cdb5c329ef61356fc7 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 23 Jul 2014 16:04:20 -0500 Subject: copyediting --- src/notifier.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/notifier.c b/src/notifier.c index f9f3ff6..364628c 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -96,7 +96,6 @@ notification_clear (IndicatorPowerNotifier * self) notify_notification_clear_actions(p->notify_notification); g_signal_handlers_disconnect_by_data(p->notify_notification, self); g_clear_object(&p->notify_notification); - } } -- cgit v1.2.3 From bf934569374d2f15567e955c058c271d1072df7b Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 23 Jul 2014 16:09:00 -0500 Subject: make the notifications click-to-dismiss by adding the 'interactive notification' hint --- src/notifier.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/notifier.c b/src/notifier.c index 364628c..8c12dfc 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -26,6 +26,8 @@ #include #include +#define HINT_INTERACTIVE "x-canonical-switch-to-application" + G_DEFINE_TYPE(IndicatorPowerNotifier, indicator_power_notifier, G_TYPE_OBJECT) @@ -114,6 +116,9 @@ notification_show(IndicatorPowerNotifier * self) body = g_strdup_printf(_("%.0f%% charge remaining"), indicator_power_device_get_percentage(p->battery)); p->notify_notification = notify_notification_new(_("Battery Low"), body, NULL); + notify_notification_set_hint(p->notify_notification, + HINT_INTERACTIVE, + g_variant_new_boolean(TRUE)); g_signal_connect_swapped(p->notify_notification, "closed", G_CALLBACK(notification_clear), self); -- cgit v1.2.3 From aba9e6261a6f02c501fa690e12babbbc8eea3a53 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 23 Jul 2014 16:18:33 -0500 Subject: copyediting: slightly better comments. --- src/notifier.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index 8c12dfc..62952e6 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -58,9 +58,10 @@ static GParamSpec * properties[LAST_PROP]; struct _IndicatorPowerNotifierPrivate { /* 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() */ + This may be a physical battery or it may be an aggregated + battery from multiple batteries present on the device. + See indicator_power_service_choose_primary_device() and + bug #880881 */ IndicatorPowerDevice * battery; PowerLevel power_level; gboolean discharging; @@ -68,10 +69,10 @@ struct _IndicatorPowerNotifierPrivate NotifyNotification* notify_notification; gboolean is_warning; - DbusBattery * dbus_battery; - GBinding * is_warning_binding; - GBinding * power_level_binding; GDBusConnection * bus; + DbusBattery * dbus_battery; /* com.canonical.indicator.power.Battery skeleton */ + GBinding * is_warning_binding; /* pushes our property to dbus_battery */ + GBinding * power_level_binding; /* pushes our property to dbus_battery */ }; typedef IndicatorPowerNotifierPrivate priv_t; @@ -162,7 +163,7 @@ on_battery_property_changed (IndicatorPowerNotifier * self) old_discharging = p->discharging; new_discharging = indicator_power_device_get_state(p->battery) == UP_DEVICE_STATE_DISCHARGING; - /* pop up a notification for a battery whenever either: + /* pop up a 'low battery' notification if either: a) it's already discharging, and its PowerLevel worsens, OR b) it's already got a bad PowerLevel and its state becomes 'discharging */ if ((new_discharging && (new_power_level > old_power_level)) || -- cgit v1.2.3 From d185409f22c859e3c29bf387955d195a5cd884c3 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 23 Jul 2014 16:22:27 -0500 Subject: remove instance_count from IndicatorPowerNotifierClass --- src/notifier.c | 18 ++++++++---------- src/notifier.h | 3 --- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index 62952e6..c6b0025 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -51,6 +51,8 @@ enum static GParamSpec * properties[LAST_PROP]; +static int instance_count = 0; + /** *** **/ @@ -276,11 +278,12 @@ my_dispose (GObject * o) } static void -my_finalize (GObject * o) +my_finalize (GObject * o G_GNUC_UNUSED) { - IndicatorPowerNotifierClass * klass = INDICATOR_POWER_NOTIFIER_GET_CLASS(o); - - if (!--klass->instance_count) + /* FIXME: This is an awkward place to put this. + Ordinarily something like this would go in main(), but we need libnotify + to clean itself up before shutting down the bus in the unit tests as well. */ + if (!--instance_count) notify_uninit(); } @@ -292,7 +295,6 @@ static void indicator_power_notifier_init (IndicatorPowerNotifier * self) { priv_t * p; - IndicatorPowerNotifierClass * klass; p = G_TYPE_INSTANCE_GET_PRIVATE (self, INDICATOR_TYPE_POWER_NOTIFIER, @@ -315,9 +317,7 @@ indicator_power_notifier_init (IndicatorPowerNotifier * self) PROP_POWER_LEVEL_NAME, G_BINDING_SYNC_CREATE); - klass = INDICATOR_POWER_NOTIFIER_GET_CLASS(self); - - if (!klass->instance_count++) + if (!instance_count++) { if (!notify_init("indicator-power-service")) { @@ -364,8 +364,6 @@ indicator_power_notifier_class_init (IndicatorPowerNotifierClass * klass) G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); g_object_class_install_properties (object_class, LAST_PROP, properties); - - klass->instance_count = 0; } /*** diff --git a/src/notifier.h b/src/notifier.h index dddb6e9..8763ad6 100644 --- a/src/notifier.h +++ b/src/notifier.h @@ -60,9 +60,6 @@ struct _IndicatorPowerNotifier struct _IndicatorPowerNotifierClass { GObjectClass parent_class; - - /*< private >*/ - gint instance_count; }; /*** -- cgit v1.2.3 From 8b9a80174263c601952d6c31cbcd344c85d096dd Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 24 Jul 2014 09:20:35 -0500 Subject: in tests/glib-fixture.h, sync with the glib-fixture.h from indicator-transfer, it's got less '#if 0' cruft --- tests/glib-fixture.h | 77 +++++++++++++++++++++++++++------------------------- 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/tests/glib-fixture.h b/tests/glib-fixture.h index 46b9640..4c53244 100644 --- a/tests/glib-fixture.h +++ b/tests/glib-fixture.h @@ -1,8 +1,5 @@ /* - * Copyright 2013 Canonical Ltd. - * - * Authors: - * Charles Kerr + * Copyright 2014 Canonical Ltd. * * This program is free software: you can redistribute it and/or modify it * under the terms of the GNU General Public License version 3, as published @@ -15,6 +12,9 @@ * * You should have received a copy of the GNU General Public License along * with this program. If not, see . + * + * Authors: + * Charles Kerr */ #include @@ -31,64 +31,67 @@ class GlibFixture : public ::testing::Test { private: - //GLogFunc realLogHandler; - - protected: + GLogFunc realLogHandler; - std::map logCounts; + std::map expected_log; + std::map> log; - void testLogCount(GLogLevelFlags log_level, int /*expected*/) + void test_log_counts() { -#if 0 - EXPECT_EQ(expected, logCounts[log_level]); -#endif + const GLogLevelFlags levels_to_test[] = { G_LOG_LEVEL_ERROR, + G_LOG_LEVEL_CRITICAL, + G_LOG_LEVEL_MESSAGE, + G_LOG_LEVEL_WARNING }; - logCounts.erase(log_level); - } + for(const auto& level : levels_to_test) + { + const auto& v = log[level]; + const auto n = v.size(); - private: + EXPECT_EQ(expected_log[level], n); + + if (expected_log[level] != n) + for (size_t i=0; i(self)->logCounts[log_level]++; + auto tmp = g_strdup_printf ("%s:%d \"%s\"", log_domain, (int)log_level, message); + static_cast(self)->log[log_level].push_back(tmp); + g_free(tmp); } protected: + void increment_expected_errors(GLogLevelFlags level, size_t n=1) + { + expected_log[level] += n; + } + virtual void SetUp() { setlocale(LC_ALL, "C.UTF-8"); loop = g_main_loop_new(nullptr, false); - //g_log_set_default_handler(default_log_handler, this); - - // only use local, temporary settings - g_assert(g_setenv("GSETTINGS_SCHEMA_DIR", SCHEMA_DIR, true)); - g_assert(g_setenv("GSETTINGS_BACKEND", "memory", true)); - g_debug("SCHEMA_DIR is %s", SCHEMA_DIR); + g_log_set_default_handler(default_log_handler, this); g_unsetenv("DISPLAY"); - } virtual void TearDown() { -#if 0 - // confirm there aren't any unexpected log messages - EXPECT_EQ(0, logCounts[G_LOG_LEVEL_ERROR]); - EXPECT_EQ(0, logCounts[G_LOG_LEVEL_CRITICAL]); - EXPECT_EQ(0, logCounts[G_LOG_LEVEL_WARNING]); - EXPECT_EQ(0, logCounts[G_LOG_LEVEL_MESSAGE]); - EXPECT_EQ(0, logCounts[G_LOG_LEVEL_INFO]); -#endif - - // revert to glib's log handler - //g_log_set_default_handler(realLogHandler, this); + test_log_counts(); + + g_log_set_default_handler(realLogHandler, this); g_clear_pointer(&loop, g_main_loop_unref); } @@ -112,7 +115,7 @@ class GlibFixture : public ::testing::Test protected: /* convenience func to loop while waiting for a GObject's signal */ - void wait_for_signal(gpointer o, const gchar * signal, unsigned int timeout_seconds=5u) + void wait_for_signal(gpointer o, const gchar * signal, const guint timeout_seconds=5) { // wait for the signal or for timeout, whichever comes first const auto handler_id = g_signal_connect_swapped(o, signal, @@ -127,7 +130,7 @@ class GlibFixture : public ::testing::Test } /* convenience func to loop for N msec */ - void wait_msec(unsigned int msec=50u) + void wait_msec(guint msec=50) { const auto id = g_timeout_add(msec, wait_msec__timeout, loop); g_main_loop_run(loop); -- cgit v1.2.3 From f4e1a83d20cad3afa6dd51a4a520ef21ccaff5cd Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 24 Jul 2014 15:14:20 -0500 Subject: in notifier.c, don't keep pointers to the bindings around as they're cleaned up automatically when either of the objects is destroyed. --- src/notifier.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index c6b0025..1986f8c 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -73,8 +73,6 @@ struct _IndicatorPowerNotifierPrivate GDBusConnection * bus; DbusBattery * dbus_battery; /* com.canonical.indicator.power.Battery skeleton */ - GBinding * is_warning_binding; /* pushes our property to dbus_battery */ - GBinding * power_level_binding; /* pushes our property to dbus_battery */ }; typedef IndicatorPowerNotifierPrivate priv_t; @@ -269,8 +267,6 @@ my_dispose (GObject * o) indicator_power_notifier_set_bus (self, NULL); notification_clear (self); - g_clear_object (&p->power_level_binding); - g_clear_object (&p->is_warning_binding); g_clear_object (&p->dbus_battery); indicator_power_notifier_set_battery (self, NULL); @@ -305,17 +301,17 @@ indicator_power_notifier_init (IndicatorPowerNotifier * self) p->dbus_battery = dbus_battery_skeleton_new (); - p->is_warning_binding = g_object_bind_property (self, - PROP_IS_WARNING_NAME, - p->dbus_battery, - PROP_IS_WARNING_NAME, - G_BINDING_SYNC_CREATE); - - p->power_level_binding = g_object_bind_property (self, - PROP_POWER_LEVEL_NAME, - p->dbus_battery, - PROP_POWER_LEVEL_NAME, - G_BINDING_SYNC_CREATE); + g_object_bind_property (self, + PROP_IS_WARNING_NAME, + p->dbus_battery, + PROP_IS_WARNING_NAME, + G_BINDING_SYNC_CREATE); + + g_object_bind_property (self, + PROP_POWER_LEVEL_NAME, + p->dbus_battery, + PROP_POWER_LEVEL_NAME, + G_BINDING_SYNC_CREATE); if (!instance_count++) { -- cgit v1.2.3 From 11784e2e354b5538b6fbccd9df1e6259c9ebc6f0 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 24 Jul 2014 15:15:09 -0500 Subject: don't set to zero fields in a struct that's been calloc()ed already. --- src/notifier.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index 1986f8c..c091f73 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -334,8 +334,6 @@ indicator_power_notifier_class_init (IndicatorPowerNotifierClass * klass) g_type_class_add_private (klass, sizeof (IndicatorPowerNotifierPrivate)); - properties[PROP_0] = NULL; - properties[PROP_BATTERY] = g_param_spec_object ( PROP_BATTERY_NAME, "Battery", -- cgit v1.2.3 From 62a35de9ca05c71d98680b1e49c2c345a498acda Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 24 Jul 2014 15:17:32 -0500 Subject: check the return value of g_dbus_interface_skeleton_export() directly instead of indirectly testing to see if the GError it used is nonnull. --- src/notifier.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index c091f73..30285d3 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -438,11 +438,10 @@ indicator_power_notifier_set_bus (IndicatorPowerNotifier * self, p->bus = g_object_ref (bus); error = NULL; - g_dbus_interface_skeleton_export(skel, - bus, - BUS_PATH"/Battery", - &error); - if (error != NULL) + if (!g_dbus_interface_skeleton_export(skel, + bus, + BUS_PATH"/Battery", + &error)) { g_warning ("Unable to export LowBattery properties: %s", error->message); g_error_free (error); -- cgit v1.2.3 From 34cf2baf977650c60b53bf5a1e90bfec8b642ec9 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 24 Jul 2014 15:20:16 -0500 Subject: remove redundant '#include glib', '#include gobject' calls --- src/device.h | 1 - src/notifier.c | 1 - src/notifier.h | 4 +--- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/device.h b/src/device.h index 77d34ef..d867707 100644 --- a/src/device.h +++ b/src/device.h @@ -24,7 +24,6 @@ License along with this library. If not, see #ifndef __INDICATOR_POWER_DEVICE_H__ #define __INDICATOR_POWER_DEVICE_H__ -#include #include /* GIcon */ G_BEGIN_DECLS diff --git a/src/notifier.c b/src/notifier.c index 30285d3..29f678a 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -24,7 +24,6 @@ #include #include -#include #define HINT_INTERACTIVE "x-canonical-switch-to-application" diff --git a/src/notifier.h b/src/notifier.h index 8763ad6..f473ee7 100644 --- a/src/notifier.h +++ b/src/notifier.h @@ -20,9 +20,7 @@ #ifndef __INDICATOR_POWER_NOTIFIER_H__ #define __INDICATOR_POWER_NOTIFIER_H__ -#include -#include -#include /* GDBusConnection */ +#include #include "device.h" -- cgit v1.2.3 From f7f949eb2d04e8dd4493cbd8e55f34c896efd9ea Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 24 Jul 2014 16:25:32 -0500 Subject: d'oh --- tests/glib-fixture.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/glib-fixture.h b/tests/glib-fixture.h index 4c53244..d333ab2 100644 --- a/tests/glib-fixture.h +++ b/tests/glib-fixture.h @@ -52,7 +52,7 @@ class GlibFixture : public ::testing::Test if (expected_log[level] != n) for (size_t i=0; i Date: Thu, 24 Jul 2014 21:25:22 -0500 Subject: in notifier, use G_DEFINE_TYPE_WITH_PRIVATE --- src/notifier.c | 51 +++++++++++++++++++++++---------------------------- src/notifier.h | 2 -- 2 files changed, 23 insertions(+), 30 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index 29f678a..1ac7e32 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -27,10 +27,6 @@ #define HINT_INTERACTIVE "x-canonical-switch-to-application" -G_DEFINE_TYPE(IndicatorPowerNotifier, - indicator_power_notifier, - G_TYPE_OBJECT) - /** *** GObject Properties **/ @@ -56,7 +52,7 @@ static int instance_count = 0; *** **/ -struct _IndicatorPowerNotifierPrivate +typedef struct { /* The battery we're currently watching. This may be a physical battery or it may be an aggregated @@ -72,10 +68,17 @@ struct _IndicatorPowerNotifierPrivate GDBusConnection * bus; DbusBattery * dbus_battery; /* com.canonical.indicator.power.Battery skeleton */ -}; +} +IndicatorPowerNotifierPrivate; typedef IndicatorPowerNotifierPrivate priv_t; +G_DEFINE_TYPE_WITH_PRIVATE(IndicatorPowerNotifier, + indicator_power_notifier, + G_TYPE_OBJECT) + +#define get_priv(o) ((priv_t*)indicator_power_notifier_get_instance_private(o)) + static void set_is_warning_property (IndicatorPowerNotifier*, gboolean is_warning); @@ -89,7 +92,7 @@ static void set_power_level_property (IndicatorPowerNotifier*, static void notification_clear (IndicatorPowerNotifier * self) { - priv_t * p = self->priv; + priv_t * const p = get_priv(self); if (p->notify_notification != NULL) { @@ -110,7 +113,7 @@ notification_show(IndicatorPowerNotifier * self) notification_clear (self); - p = self->priv; + p = get_priv (self); /* create the notification */ body = g_strdup_printf(_("%.0f%% charge remaining"), @@ -152,9 +155,8 @@ on_battery_property_changed (IndicatorPowerNotifier * self) gboolean new_discharging; g_return_if_fail(INDICATOR_IS_POWER_NOTIFIER(self)); - g_return_if_fail(INDICATOR_IS_POWER_DEVICE(self->priv->battery)); - - p = self->priv; + p = get_priv (self); + g_return_if_fail(INDICATOR_IS_POWER_DEVICE(p->battery)); old_power_level = p->power_level; new_power_level = indicator_power_notifier_get_power_level (p->battery); @@ -189,8 +191,8 @@ my_get_property (GObject * o, GValue * value, GParamSpec * pspec) { - IndicatorPowerNotifier * self = INDICATOR_POWER_NOTIFIER (o); - priv_t * p = self->priv; + IndicatorPowerNotifier * const self = INDICATOR_POWER_NOTIFIER (o); + priv_t * const p = get_priv (self); switch (property_id) { @@ -217,7 +219,7 @@ my_set_property (GObject * o, const GValue * value, GParamSpec * pspec) { - IndicatorPowerNotifier * self = INDICATOR_POWER_NOTIFIER (o); + IndicatorPowerNotifier * const self = INDICATOR_POWER_NOTIFIER (o); switch (property_id) { @@ -234,7 +236,7 @@ my_set_property (GObject * o, static void set_is_warning_property (IndicatorPowerNotifier * self, gboolean is_warning) { - priv_t * p = self->priv; + priv_t * const p = get_priv (self); if (p->is_warning != is_warning) { @@ -248,7 +250,7 @@ set_is_warning_property (IndicatorPowerNotifier * self, gboolean is_warning) static void set_power_level_property (IndicatorPowerNotifier * self, PowerLevel power_level) { - priv_t * p = self->priv; + priv_t * const p = get_priv (self); if (p->power_level != power_level) { @@ -261,8 +263,8 @@ set_power_level_property (IndicatorPowerNotifier * self, PowerLevel power_level) static void my_dispose (GObject * o) { - IndicatorPowerNotifier * self = INDICATOR_POWER_NOTIFIER(o); - priv_t * p = self->priv; + IndicatorPowerNotifier * const self = INDICATOR_POWER_NOTIFIER(o); + priv_t * const p = get_priv (self); indicator_power_notifier_set_bus (self, NULL); notification_clear (self); @@ -289,12 +291,7 @@ my_finalize (GObject * o G_GNUC_UNUSED) static void indicator_power_notifier_init (IndicatorPowerNotifier * self) { - priv_t * p; - - p = G_TYPE_INSTANCE_GET_PRIVATE (self, - INDICATOR_TYPE_POWER_NOTIFIER, - IndicatorPowerNotifierPrivate); - self->priv = p; + priv_t * const p = get_priv (self); /* bind the read-only properties so they'll get pushed to the bus */ @@ -331,8 +328,6 @@ indicator_power_notifier_class_init (IndicatorPowerNotifierClass * klass) object_class->get_property = my_get_property; object_class->set_property = my_set_property; - g_type_class_add_private (klass, sizeof (IndicatorPowerNotifierPrivate)); - properties[PROP_BATTERY] = g_param_spec_object ( PROP_BATTERY_NAME, "Battery", @@ -381,7 +376,7 @@ indicator_power_notifier_set_battery (IndicatorPowerNotifier * self, 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)); - p = self->priv; + p = get_priv (self); if (p->battery == battery) return; @@ -415,7 +410,7 @@ indicator_power_notifier_set_bus (IndicatorPowerNotifier * self, g_return_if_fail(INDICATOR_IS_POWER_NOTIFIER(self)); g_return_if_fail((bus == NULL) || G_IS_DBUS_CONNECTION(bus)); - p = self->priv; + p = get_priv (self); if (p->bus == bus) return; diff --git a/src/notifier.h b/src/notifier.h index f473ee7..c23c585 100644 --- a/src/notifier.h +++ b/src/notifier.h @@ -34,7 +34,6 @@ G_BEGIN_DECLS typedef struct _IndicatorPowerNotifier IndicatorPowerNotifier; typedef struct _IndicatorPowerNotifierClass IndicatorPowerNotifierClass; -typedef struct _IndicatorPowerNotifierPrivate IndicatorPowerNotifierPrivate; typedef enum { @@ -52,7 +51,6 @@ struct _IndicatorPowerNotifier { /*< private >*/ GObject parent; - IndicatorPowerNotifierPrivate * priv; }; struct _IndicatorPowerNotifierClass -- cgit v1.2.3 From e9ba47b83251f40234059b1bd2bc25e30b5aa9b2 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 24 Jul 2014 22:51:03 -0500 Subject: in notifier.c, fix potential callchain loop when closing a notification --- src/notifier.c | 68 ++++++++++++++++++++++++++++++++-------------------- tests/test-notify.cc | 33 +++++++++++++++++++++---- 2 files changed, 70 insertions(+), 31 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index 1ac7e32..dc3a186 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -89,56 +89,72 @@ static void set_power_level_property (IndicatorPowerNotifier*, **** Notifications ***/ +static void +on_notify_notification_finalized (gpointer gself, GObject * dead) +{ + IndicatorPowerNotifier * const self = INDICATOR_POWER_NOTIFIER(gself); + priv_t * const p = get_priv(self); + g_return_if_fail ((void*)(p->notify_notification) == (void*)dead); + p->notify_notification = NULL; + set_is_warning_property (self, FALSE); +} + static void notification_clear (IndicatorPowerNotifier * self) { priv_t * const p = get_priv(self); + NotifyNotification * nn; - if (p->notify_notification != NULL) + if ((nn = p->notify_notification)) { - set_is_warning_property (self, FALSE); + GError * error = NULL; + + g_object_weak_unref(G_OBJECT(nn), on_notify_notification_finalized, self); - notify_notification_clear_actions(p->notify_notification); - g_signal_handlers_disconnect_by_data(p->notify_notification, self); - g_clear_object(&p->notify_notification); + if (!notify_notification_close(nn, &error)) + { + g_warning("Unable to close notification: %s", error->message); + g_error_free(error); + } + + p->notify_notification = NULL; + set_is_warning_property(self, FALSE); } } static void notification_show(IndicatorPowerNotifier * self) { - priv_t * p; + priv_t * const p = get_priv(self); + gdouble pct; char * body; + NotifyNotification * nn; GError * error; - notification_clear (self); - - p = get_priv (self); + notification_clear(self); /* create the notification */ - body = g_strdup_printf(_("%.0f%% charge remaining"), - indicator_power_device_get_percentage(p->battery)); - p->notify_notification = notify_notification_new(_("Battery Low"), body, NULL); - notify_notification_set_hint(p->notify_notification, - HINT_INTERACTIVE, - g_variant_new_boolean(TRUE)); - g_signal_connect_swapped(p->notify_notification, "closed", - G_CALLBACK(notification_clear), self); - - /* show the notification */ + pct = indicator_power_device_get_percentage(p->battery); + body = g_strdup_printf(_("%.0f%% charge remaining"), pct); + nn = notify_notification_new(_("Battery Low"), body, NULL); + g_free (body); + notify_notification_set_hint(nn, HINT_INTERACTIVE, g_variant_new_boolean(TRUE)); + + /* if we can show it, keep it */ error = NULL; - notify_notification_show(p->notify_notification, &error); - if (error != NULL) + if (notify_notification_show(nn, &error)) { - g_critical("Unable to show snap decision for '%s': %s", body, error->message); - g_error_free(error); + p->notify_notification = nn; + g_signal_connect(nn, "closed", G_CALLBACK(g_object_unref), NULL); + g_object_weak_ref(G_OBJECT(nn), on_notify_notification_finalized, self); + set_is_warning_property(self, TRUE); } else { - set_is_warning_property (self, TRUE); + g_critical("Unable to show snap decision for '%s': %s", body, error->message); + g_error_free(error); + g_object_unref(nn); } - - g_free (body); } /*** diff --git a/tests/test-notify.cc b/tests/test-notify.cc index 1fc843e..a8d66d3 100644 --- a/tests/test-notify.cc +++ b/tests/test-notify.cc @@ -54,7 +54,7 @@ protected: DbusTestDbusMockObject * obj = nullptr; GDBusConnection * bus = nullptr; - static constexpr int NOTIFY_ID {1234}; + static constexpr int FIRST_NOTIFY_ID {1234}; static constexpr int NOTIFICATION_CLOSED_EXPIRED {1}; static constexpr int NOTIFICATION_CLOSED_DISMISSED {2}; @@ -63,9 +63,11 @@ protected: static constexpr char const * APP_NAME {"indicator-power-service"}; + static constexpr char const * METHOD_CLOSE {"CloseNotification"}; static constexpr char const * METHOD_NOTIFY {"Notify"}; static constexpr char const * METHOD_GET_CAPS {"GetCapabilities"}; static constexpr char const * METHOD_GET_INFO {"GetServerInformation"}; + static constexpr char const * SIGNAL_CLOSED {"NotificationClosed"}; static constexpr char const * HINT_TIMEOUT {"x-canonical-snap-decisions-timeout"}; @@ -86,7 +88,8 @@ protected: NOTIFY_INTERFACE, &error); g_assert_no_error (error); - + + // METHOD_GET_INFO dbus_test_dbus_mock_object_add_method(mock, obj, METHOD_GET_INFO, nullptr, G_VARIANT_TYPE("(ssss)"), @@ -94,14 +97,34 @@ protected: &error); g_assert_no_error (error); - auto python_str = g_strdup_printf ("ret = %d", NOTIFY_ID); + // METHOD_NOTIFY + auto str = g_strdup_printf("try:\n" + " self.NextNotifyId\n" + "except AttributeError:\n" + " self.NextNotifyId = %d\n" + "ret = self.NextNotifyId\n" + "self.NextNotifyId += 1\n", + FIRST_NOTIFY_ID); dbus_test_dbus_mock_object_add_method(mock, obj, METHOD_NOTIFY, G_VARIANT_TYPE("(susssasa{sv}i)"), G_VARIANT_TYPE_UINT32, - python_str, + str, &error); - g_free (python_str); g_assert_no_error (error); + g_free (str); + + // METHOD_CLOSE + str = g_strdup_printf("self.EmitSignal('%s', '%s', 'uu', [ args[0], %d ])", + NOTIFY_INTERFACE, + SIGNAL_CLOSED, + NOTIFICATION_CLOSED_API); + dbus_test_dbus_mock_object_add_method(mock, obj, METHOD_CLOSE, + G_VARIANT_TYPE("(u)"), + nullptr, + str, + &error); + g_assert_no_error (error); + g_free (str); dbus_test_service_add_task(service, DBUS_TEST_TASK(mock)); dbus_test_service_start_tasks(service); -- cgit v1.2.3 From 079ac51da565af4882b79c14b3a4782e5c919dd3 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 24 Jul 2014 23:13:52 -0500 Subject: in notifier.c, remove unnecessary middleman properties and deal with the dbus-battery properties directly. --- src/notifier.c | 89 +++++----------------------------------------------------- 1 file changed, 7 insertions(+), 82 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index dc3a186..60799f9 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -35,14 +35,10 @@ enum { PROP_0, PROP_BATTERY, - PROP_IS_WARNING, - PROP_POWER_LEVEL, LAST_PROP }; #define PROP_BATTERY_NAME "battery" -#define PROP_IS_WARNING_NAME "is-warning" -#define PROP_POWER_LEVEL_NAME "power-level" static GParamSpec * properties[LAST_PROP]; @@ -63,8 +59,7 @@ typedef struct PowerLevel power_level; gboolean discharging; - NotifyNotification* notify_notification; - gboolean is_warning; + NotifyNotification * notify_notification; GDBusConnection * bus; DbusBattery * dbus_battery; /* com.canonical.indicator.power.Battery skeleton */ @@ -79,12 +74,6 @@ G_DEFINE_TYPE_WITH_PRIVATE(IndicatorPowerNotifier, #define get_priv(o) ((priv_t*)indicator_power_notifier_get_instance_private(o)) -static void set_is_warning_property (IndicatorPowerNotifier*, - gboolean is_warning); - -static void set_power_level_property (IndicatorPowerNotifier*, - PowerLevel power_level); - /*** **** Notifications ***/ @@ -96,7 +85,7 @@ on_notify_notification_finalized (gpointer gself, GObject * dead) priv_t * const p = get_priv(self); g_return_if_fail ((void*)(p->notify_notification) == (void*)dead); p->notify_notification = NULL; - set_is_warning_property (self, FALSE); + dbus_battery_set_is_warning (p->dbus_battery, FALSE); } static void @@ -118,7 +107,7 @@ notification_clear (IndicatorPowerNotifier * self) } p->notify_notification = NULL; - set_is_warning_property(self, FALSE); + dbus_battery_set_is_warning (p->dbus_battery, FALSE); } } @@ -147,7 +136,7 @@ notification_show(IndicatorPowerNotifier * self) p->notify_notification = nn; g_signal_connect(nn, "closed", G_CALLBACK(g_object_unref), NULL); g_object_weak_ref(G_OBJECT(nn), on_notify_notification_finalized, self); - set_is_warning_property(self, TRUE); + dbus_battery_set_is_warning (p->dbus_battery, TRUE); } else { @@ -193,7 +182,7 @@ on_battery_property_changed (IndicatorPowerNotifier * self) notification_clear (self); } - set_power_level_property (self, new_power_level); + dbus_battery_set_power_level (p->dbus_battery, new_power_level); p->discharging = new_discharging; } @@ -216,14 +205,6 @@ my_get_property (GObject * o, g_value_set_object (value, p->battery); break; - case PROP_POWER_LEVEL: - g_value_set_int (value, p->power_level); - break; - - case PROP_IS_WARNING: - g_value_set_boolean (value, p->is_warning); - break; - default: G_OBJECT_WARN_INVALID_PROPERTY_ID (o, property_id, pspec); } @@ -248,34 +229,6 @@ 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 * const p = get_priv (self); - - 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 * const p = get_priv (self); - - 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) { @@ -284,8 +237,8 @@ my_dispose (GObject * o) indicator_power_notifier_set_bus (self, NULL); notification_clear (self); - g_clear_object (&p->dbus_battery); indicator_power_notifier_set_battery (self, NULL); + g_clear_object (&p->dbus_battery); G_OBJECT_CLASS (indicator_power_notifier_parent_class)->dispose (o); } @@ -313,18 +266,6 @@ indicator_power_notifier_init (IndicatorPowerNotifier * self) p->dbus_battery = dbus_battery_skeleton_new (); - g_object_bind_property (self, - PROP_IS_WARNING_NAME, - p->dbus_battery, - PROP_IS_WARNING_NAME, - G_BINDING_SYNC_CREATE); - - g_object_bind_property (self, - PROP_POWER_LEVEL_NAME, - p->dbus_battery, - PROP_POWER_LEVEL_NAME, - G_BINDING_SYNC_CREATE); - if (!instance_count++) { if (!notify_init("indicator-power-service")) @@ -351,22 +292,6 @@ indicator_power_notifier_class_init (IndicatorPowerNotifierClass * klass) G_TYPE_OBJECT, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); - properties[PROP_POWER_LEVEL] = g_param_spec_int ( - PROP_POWER_LEVEL_NAME, - "Power Level", - "The battery's power level", - POWER_LEVEL_OK, - POWER_LEVEL_CRITICAL, - POWER_LEVEL_OK, - G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); - - properties[PROP_IS_WARNING] = g_param_spec_boolean ( - PROP_IS_WARNING_NAME, - "Is Warning", - "Whether or not we're currently warning the user about a low battery", - FALSE, - G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); - g_object_class_install_properties (object_class, LAST_PROP, properties); } @@ -401,7 +326,7 @@ indicator_power_notifier_set_battery (IndicatorPowerNotifier * self, { g_signal_handlers_disconnect_by_data (p->battery, self); g_clear_object (&p->battery); - set_power_level_property (self, POWER_LEVEL_OK); + dbus_battery_set_power_level (p->dbus_battery, POWER_LEVEL_OK); notification_clear (self); } -- cgit v1.2.3 From f15482d3f189c378d4e4cf2dfc69eefa522b30aa Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 24 Jul 2014 23:31:11 -0500 Subject: on the bus, publish the battery's power_level as strings rather than ints --- data/com.canonical.indicator.power.Battery.xml | 4 +- src/notifier.c | 81 ++++++++++++++++++-------- src/notifier.h | 15 ++--- tests/test-notify.cc | 20 +++---- 4 files changed, 73 insertions(+), 47 deletions(-) diff --git a/data/com.canonical.indicator.power.Battery.xml b/data/com.canonical.indicator.power.Battery.xml index d2c8a2d..eca4524 100644 --- a/data/com.canonical.indicator.power.Battery.xml +++ b/data/com.canonical.indicator.power.Battery.xml @@ -3,10 +3,10 @@ - + - The battery's power level. 0==No Low Battery, 1==Low, 2==Very Low, 3==Critical + The battery's power level. Possible values: 'ok', 'low', 'very_low', 'critical' diff --git a/src/notifier.c b/src/notifier.c index 60799f9..7add139 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -27,6 +27,15 @@ #define HINT_INTERACTIVE "x-canonical-switch-to-application" +typedef enum +{ + POWER_LEVEL_OK, + POWER_LEVEL_LOW, + POWER_LEVEL_VERY_LOW, + POWER_LEVEL_CRITICAL +} +PowerLevel; + /** *** GObject Properties **/ @@ -74,6 +83,48 @@ G_DEFINE_TYPE_WITH_PRIVATE(IndicatorPowerNotifier, #define get_priv(o) ((priv_t*)indicator_power_notifier_get_instance_private(o)) +/*** +**** +***/ + +static const char * +power_level_to_dbus_string (const PowerLevel power_level) +{ + switch (power_level) + { + case POWER_LEVEL_LOW: return "low"; + case POWER_LEVEL_VERY_LOW: return "very_low"; + case POWER_LEVEL_CRITICAL: return "critical"; + default: return "ok"; + } +} + +PowerLevel +get_battery_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; +} + /*** **** Notifications ***/ @@ -164,7 +215,7 @@ on_battery_property_changed (IndicatorPowerNotifier * self) g_return_if_fail(INDICATOR_IS_POWER_DEVICE(p->battery)); old_power_level = p->power_level; - new_power_level = indicator_power_notifier_get_power_level (p->battery); + new_power_level = get_battery_power_level (p->battery); old_discharging = p->discharging; new_discharging = indicator_power_device_get_state(p->battery) == UP_DEVICE_STATE_DISCHARGING; @@ -182,7 +233,7 @@ on_battery_property_changed (IndicatorPowerNotifier * self) notification_clear (self); } - dbus_battery_set_power_level (p->dbus_battery, new_power_level); + dbus_battery_set_power_level (p->dbus_battery, power_level_to_dbus_string (new_power_level)); p->discharging = new_discharging; } @@ -326,7 +377,7 @@ indicator_power_notifier_set_battery (IndicatorPowerNotifier * self, { g_signal_handlers_disconnect_by_data (p->battery, self); g_clear_object (&p->battery); - dbus_battery_set_power_level (p->dbus_battery, POWER_LEVEL_OK); + dbus_battery_set_power_level (p->dbus_battery, power_level_to_dbus_string (POWER_LEVEL_OK)); notification_clear (self); } @@ -384,28 +435,8 @@ indicator_power_notifier_set_bus (IndicatorPowerNotifier * self, } } -PowerLevel +const char * 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; + return power_level_to_dbus_string (get_battery_power_level (battery)); } diff --git a/src/notifier.h b/src/notifier.h index c23c585..18e25d7 100644 --- a/src/notifier.h +++ b/src/notifier.h @@ -35,15 +35,6 @@ G_BEGIN_DECLS typedef struct _IndicatorPowerNotifier IndicatorPowerNotifier; typedef struct _IndicatorPowerNotifierClass IndicatorPowerNotifierClass; -typedef enum -{ - POWER_LEVEL_OK, - POWER_LEVEL_LOW, - POWER_LEVEL_VERY_LOW, - POWER_LEVEL_CRITICAL -} -PowerLevel; - /** * The Indicator Power Notifier. */ @@ -72,7 +63,11 @@ 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); +#define POWER_LEVEL_STR_OK "ok" +#define POWER_LEVEL_STR_LOW "low" +#define POWER_LEVEL_STR_VERY_LOW "very_low" +#define POWER_LEVEL_STR_CRITICAL "critical" +const char * indicator_power_notifier_get_power_level (IndicatorPowerDevice * battery); G_END_DECLS diff --git a/tests/test-notify.cc b/tests/test-notify.cc index a8d66d3..b5166a0 100644 --- a/tests/test-notify.cc +++ b/tests/test-notify.cc @@ -200,13 +200,13 @@ TEST_F(NotifyFixture, PercentageToLevel) const auto level = indicator_power_notifier_get_power_level(battery); if (i <= percent_critical) - EXPECT_EQ (POWER_LEVEL_CRITICAL, level); + EXPECT_STREQ (POWER_LEVEL_STR_CRITICAL, level); else if (i <= percent_very_low) - EXPECT_EQ (POWER_LEVEL_VERY_LOW, level); + EXPECT_STREQ (POWER_LEVEL_STR_VERY_LOW, level); else if (i <= percent_low) - EXPECT_EQ (POWER_LEVEL_LOW, level); + EXPECT_STREQ (POWER_LEVEL_STR_LOW, level); else - EXPECT_EQ (POWER_LEVEL_OK, level); + EXPECT_STREQ (POWER_LEVEL_STR_OK, level); } g_object_unref (battery); @@ -227,7 +227,7 @@ namespace struct ChangedParams { - int32_t power_level = POWER_LEVEL_OK; + std::string power_level = POWER_LEVEL_STR_OK; bool is_warning = false; uint32_t fields = 0; }; @@ -245,8 +245,8 @@ namespace g_return_if_fail (g_variant_is_of_type (dict, G_VARIANT_TYPE_DICTIONARY)); auto changed_params = static_cast(gchanged_params); - gint32 power_level; - if (g_variant_lookup (dict, "PowerLevel", "i", &power_level, nullptr)) + const char * power_level; + if (g_variant_lookup (dict, "PowerLevel", "&s", &power_level, nullptr)) { changed_params->power_level = power_level; changed_params->fields |= FIELD_POWER_LEVEL; @@ -362,7 +362,7 @@ TEST_F(NotifyFixture, EventsThatChangeNotifications) // test setup case wait_msec(); - EXPECT_EQ (0, changed_params.power_level); + EXPECT_STREQ (POWER_LEVEL_STR_OK, changed_params.power_level.c_str()); // change the percent past the 'low' threshold and confirm that // a) the power level changes @@ -379,7 +379,7 @@ TEST_F(NotifyFixture, EventsThatChangeNotifications) set_battery_percentage (battery, percent_very_low); wait_msec(); EXPECT_EQ (FIELD_POWER_LEVEL, changed_params.fields); - EXPECT_EQ (POWER_LEVEL_VERY_LOW, changed_params.power_level); + EXPECT_STREQ (POWER_LEVEL_STR_VERY_LOW, changed_params.power_level.c_str()); // ...and that the warning is taken down if the battery is plugged back in... changed_params = ChangedParams(); @@ -400,7 +400,7 @@ TEST_F(NotifyFixture, EventsThatChangeNotifications) set_battery_percentage (battery, percent_low+1); wait_msec(); EXPECT_EQ (FIELD_POWER_LEVEL|FIELD_IS_WARNING, changed_params.fields); - EXPECT_EQ (POWER_LEVEL_OK, changed_params.power_level); + EXPECT_STREQ (POWER_LEVEL_STR_OK, changed_params.power_level.c_str()); EXPECT_FALSE (changed_params.is_warning); // cleanup -- cgit v1.2.3 From 35a251324cc5256c459ff4287855cd510b30a026 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 24 Jul 2014 23:40:28 -0500 Subject: in notify, reverse the numerical order of the now-private PowerLevel enum so that they have the more intuitive behavior of higher integer values meaning a better power level. --- src/notifier.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/notifier.c b/src/notifier.c index 7add139..6557246 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -29,10 +29,10 @@ typedef enum { - POWER_LEVEL_OK, - POWER_LEVEL_LOW, + POWER_LEVEL_CRITICAL, POWER_LEVEL_VERY_LOW, - POWER_LEVEL_CRITICAL + POWER_LEVEL_LOW, + POWER_LEVEL_OK } PowerLevel; @@ -92,10 +92,10 @@ power_level_to_dbus_string (const PowerLevel power_level) { switch (power_level) { - case POWER_LEVEL_LOW: return "low"; - case POWER_LEVEL_VERY_LOW: return "very_low"; - case POWER_LEVEL_CRITICAL: return "critical"; - default: return "ok"; + case POWER_LEVEL_LOW: return POWER_LEVEL_STR_LOW; + case POWER_LEVEL_VERY_LOW: return POWER_LEVEL_STR_VERY_LOW; + case POWER_LEVEL_CRITICAL: return POWER_LEVEL_STR_CRITICAL; + default: return POWER_LEVEL_STR_OK; } } @@ -223,7 +223,7 @@ on_battery_property_changed (IndicatorPowerNotifier * self) /* pop up a 'low battery' notification if either: a) it's already discharging, and its PowerLevel worsens, OR b) it's already got a bad PowerLevel and its state becomes 'discharging */ - if ((new_discharging && (new_power_level > old_power_level)) || + if ((new_discharging && (old_power_level > new_power_level)) || ((new_power_level != POWER_LEVEL_OK) && new_discharging && !old_discharging)) { notification_show (self); @@ -317,6 +317,8 @@ indicator_power_notifier_init (IndicatorPowerNotifier * self) p->dbus_battery = dbus_battery_skeleton_new (); + p->power_level = POWER_LEVEL_OK; + if (!instance_count++) { if (!notify_init("indicator-power-service")) -- cgit v1.2.3 From eaf64b2f5d9561b4e113330d24d17c01dfbc92f0 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 25 Jul 2014 00:05:04 -0500 Subject: fix bug introduced in previous commit --- src/notifier.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/notifier.c b/src/notifier.c index 6557246..52d8854 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -234,6 +234,7 @@ on_battery_property_changed (IndicatorPowerNotifier * self) } dbus_battery_set_power_level (p->dbus_battery, power_level_to_dbus_string (new_power_level)); + p->power_level = new_power_level; p->discharging = new_discharging; } -- cgit v1.2.3 From 265ab08cace125fc1e307f137f62240f74ae2662 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 25 Jul 2014 00:28:23 -0500 Subject: disable the notification's interactive hint for now --- src/notifier.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/notifier.c b/src/notifier.c index 52d8854..496b416 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -178,7 +178,7 @@ notification_show(IndicatorPowerNotifier * self) body = g_strdup_printf(_("%.0f%% charge remaining"), pct); nn = notify_notification_new(_("Battery Low"), body, NULL); g_free (body); - notify_notification_set_hint(nn, HINT_INTERACTIVE, g_variant_new_boolean(TRUE)); + /*notify_notification_set_hint(nn, HINT_INTERACTIVE, g_variant_new_boolean(TRUE));*/ /* if we can show it, keep it */ error = NULL; -- cgit v1.2.3 From b46ec7d0cc35d10197a1a91f473e0a655188f6eb Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 25 Jul 2014 00:34:59 -0500 Subject: in notify.c, improve the logic for when to tear down a notification --- src/notifier.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/notifier.c b/src/notifier.c index 496b416..81cd6f1 100644 --- a/src/notifier.c +++ b/src/notifier.c @@ -228,7 +228,7 @@ on_battery_property_changed (IndicatorPowerNotifier * self) { notification_show (self); } - else + else if (!new_discharging || (new_power_level == POWER_LEVEL_OK)) { notification_clear (self); } -- cgit v1.2.3