From 5d753a34dcb898d1572f527a95b7ec9b58be1803 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Sat, 13 Sep 2014 13:12:04 -0500 Subject: hide Clock's implementation details into an Impl class. --- include/datetime/clock.h | 9 ++-- src/clock.cpp | 138 +++++++++++++++++++++++++++++++++-------------- tests/test-clock.cpp | 50 ++++++++++------- 3 files changed, 132 insertions(+), 65 deletions(-) diff --git a/include/datetime/clock.h b/include/datetime/clock.h index 1d488d1..0b2a543 100644 --- a/include/datetime/clock.h +++ b/include/datetime/clock.h @@ -55,12 +55,9 @@ protected: void maybe_emit (const DateTime& a, const DateTime& b); private: - static void on_system_bus_ready(GObject*, GAsyncResult*, gpointer); - static void on_prepare_for_sleep(GDBusConnection*, const gchar*, const gchar*, const gchar*, const gchar*, GVariant*, gpointer); - - GCancellable * m_cancellable = nullptr; - GDBusConnection * m_system_bus = nullptr; - unsigned int m_sleep_subscription_id = 0; + class Impl; + friend class Impl; + std::unique_ptr m_impl; // we've got raw pointers and GSignal tags in here, so disable copying Clock(const Clock&) =delete; diff --git a/src/clock.cpp b/src/clock.cpp index f41a0cc..748174b 100644 --- a/src/clock.cpp +++ b/src/clock.cpp @@ -22,6 +22,11 @@ #include #include +#include +#include +#include +#include + namespace unity { namespace indicator { namespace datetime { @@ -30,58 +35,109 @@ namespace datetime { **** ***/ -Clock::Clock(): - m_cancellable(g_cancellable_new()) +class Clock::Impl { - g_bus_get(G_BUS_TYPE_SYSTEM, m_cancellable, on_system_bus_ready, this); -} +public: -Clock::~Clock() -{ - g_cancellable_cancel(m_cancellable); - g_clear_object(&m_cancellable); + Impl(Clock& owner): + m_owner(owner) + { + auto tag = g_bus_watch_name(G_BUS_TYPE_SYSTEM, + "org.freedesktop.login1", + G_BUS_NAME_WATCHER_FLAGS_NONE, + on_login1_appeared, + on_login1_vanished, + this, nullptr); + m_watched_names.insert(tag); + } - if (m_sleep_subscription_id) - g_dbus_connection_signal_unsubscribe(m_system_bus, m_sleep_subscription_id); - g_clear_object(&m_system_bus); -} + ~Impl() + { + for(const auto& tag : m_watched_names) + g_bus_unwatch_name(tag); + } -void -Clock::on_system_bus_ready(GObject*, GAsyncResult * res, gpointer gself) -{ - GDBusConnection * system_bus; +private: + + void remember_subscription(const std::string& name, + GDBusConnection* bus, + guint tag) + { + auto subscription = std::shared_ptr( + G_DBUS_CONNECTION(g_object_ref(bus)), + [tag](GDBusConnection* bus){ + g_dbus_connection_signal_unsubscribe(bus, tag); + g_object_unref(G_OBJECT(bus)); + } + ); - if ((system_bus = g_bus_get_finish(res, nullptr))) + m_subscriptions[name].push_back(subscription); + } + + /** + *** DBus Chatter: org.freedesktop.login1 + *** + *** Fire Clock::minute_changed() signal on login1's PrepareForSleep signal + **/ + + static void on_login1_appeared(GDBusConnection * bus, + const gchar * name, + const gchar * name_owner, + gpointer gself) { - auto self = static_cast(gself); - - self->m_system_bus = system_bus; - - self->m_sleep_subscription_id = g_dbus_connection_signal_subscribe( - system_bus, - nullptr, - "org.freedesktop.login1.Manager", // interface - "PrepareForSleep", // signal name - "/org/freedesktop/login1", // object path - nullptr, // arg0 - G_DBUS_SIGNAL_FLAGS_NONE, - on_prepare_for_sleep, - self, - nullptr); + auto tag = g_dbus_connection_signal_subscribe(bus, + name_owner, + "org.freedesktop.login1.Manager", // interface + "PrepareForSleep", // signal name + "/org/freedesktop/login1", // object path + nullptr, // arg0 + G_DBUS_SIGNAL_FLAGS_NONE, + on_prepare_for_sleep, + gself, + nullptr); + + static_cast(gself)->remember_subscription(name, bus, tag); } + + static void on_login1_vanished(GDBusConnection * /*system_bus*/, + const gchar * name, + gpointer gself) + { + static_cast(gself)->m_subscriptions[name].clear(); + } + + static void on_prepare_for_sleep(GDBusConnection* /*connection*/, + const gchar* /*sender_name*/, + const gchar* /*object_path*/, + const gchar* /*interface_name*/, + const gchar* /*signal_name*/, + GVariant* /*parameters*/, + gpointer gself) + { + static_cast(gself)->m_owner.minute_changed(); + } + + /*** + **** + ***/ + + Clock& m_owner; + std::set m_watched_names; + std::map>> m_subscriptions; +}; + +/*** +**** +***/ + +Clock::Clock(): + m_impl(new Impl{*this}) +{ } -void -Clock::on_prepare_for_sleep(GDBusConnection* /*connection*/, - const gchar* /*sender_name*/, - const gchar* /*object_path*/, - const gchar* /*interface_name*/, - const gchar* /*signal_name*/, - GVariant* /*parameters*/, - gpointer gself) +Clock::~Clock() { - static_cast(gself)->minute_changed(); } /*** diff --git a/tests/test-clock.cpp b/tests/test-clock.cpp index a4924b3..82f8cb0 100644 --- a/tests/test-clock.cpp +++ b/tests/test-clock.cpp @@ -32,18 +32,6 @@ class ClockFixture: public TestDBusFixture { private: typedef TestDBusFixture super; - - public: - void emitPrepareForSleep() - { - g_dbus_connection_emit_signal(g_bus_get_sync(G_BUS_TYPE_SYSTEM, nullptr, nullptr), - nullptr, - "/org/freedesktop/login1", // object path - "org.freedesktop.login1.Manager", // interface - "PrepareForSleep", // signal name - g_variant_new("(b)", FALSE), - nullptr); - } }; TEST_F(ClockFixture, MinuteChangedSignalShouldTriggerOncePerMinute) @@ -113,6 +101,27 @@ TEST_F(ClockFixture, TimezoneChangeTriggersSkew) g_time_zone_unref(tz_la); } +/*** +**** +***/ + +namespace +{ + void on_login1_name_acquired(GDBusConnection * connection, + const gchar * /*name*/, + gpointer /*user_data*/) + { + g_dbus_connection_emit_signal(connection, + nullptr, + "/org/freedesktop/login1", // object path + "org.freedesktop.login1.Manager", // interface + "PrepareForSleep", // signal name + g_variant_new("(b)", FALSE), + nullptr); + } +} + + /** * Confirm that a "PrepareForSleep" event wil trigger a skew event */ @@ -121,7 +130,7 @@ TEST_F(ClockFixture, SleepTriggersSkew) std::shared_ptr zones(new Timezones); zones->timezone.set("America/New_York"); LiveClock clock(zones); - wait_msec(500); // wait for the bus to set up + wait_msec(250); // wait for the bus to set up bool skewed = false; clock.minute_changed.connect([&skewed, this](){ @@ -130,11 +139,16 @@ TEST_F(ClockFixture, SleepTriggersSkew) return G_SOURCE_REMOVE; }); - g_idle_add([](gpointer gself){ - static_cast(gself)->emitPrepareForSleep(); - return G_SOURCE_REMOVE; - }, this); - + auto name_tag = g_bus_own_name(G_BUS_TYPE_SYSTEM, + "org.freedesktop.login1", + G_BUS_NAME_OWNER_FLAGS_NONE, + nullptr /* bus acquired */, + on_login1_name_acquired, + nullptr /* name lost */, + nullptr /* user_data */, + nullptr /* user_data closure */); g_main_loop_run(loop); EXPECT_TRUE(skewed); + + g_bus_unown_name(name_tag); } -- cgit v1.2.3 From 187b660a2a80f4aabf0b87c3fae02cb0841dd2e0 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Sat, 13 Sep 2014 13:39:19 -0500 Subject: update the time string when a powerd.Wakeup signal is seen. --- include/datetime/dbus-shared.h | 5 +++ src/clock.cpp | 79 +++++++++++++++++++++++++++++++++++------- 2 files changed, 72 insertions(+), 12 deletions(-) diff --git a/include/datetime/dbus-shared.h b/include/datetime/dbus-shared.h index db10c1d..c09caa2 100644 --- a/include/datetime/dbus-shared.h +++ b/include/datetime/dbus-shared.h @@ -24,4 +24,9 @@ #define BUS_DATETIME_NAME "com.canonical.indicator.datetime" #define BUS_DATETIME_PATH "/com/canonical/indicator/datetime" +#define BUS_POWERD_NAME "com.canonical.powerd" +#define BUS_POWERD_PATH "/com/canonical/powerd" +#define BUS_POWERD_INTERFACE "com.canonical.powerd" + + #endif /* _INDICATOR_DATETIME_DBUS_SHARED_H_ */ diff --git a/src/clock.cpp b/src/clock.cpp index 748174b..6831732 100644 --- a/src/clock.cpp +++ b/src/clock.cpp @@ -18,6 +18,7 @@ */ #include +#include #include #include @@ -49,6 +50,14 @@ public: on_login1_vanished, this, nullptr); m_watched_names.insert(tag); + + tag = g_bus_watch_name(G_BUS_TYPE_SYSTEM, + BUS_POWERD_NAME, + G_BUS_NAME_WATCHER_FLAGS_NONE, + on_powerd_appeared, + on_powerd_vanished, + this, nullptr); + m_watched_names.insert(tag); } @@ -60,19 +69,18 @@ public: private: - void remember_subscription(const std::string& name, - GDBusConnection* bus, - guint tag) + void remember_subscription(const std::string & name, + GDBusConnection * bus, + guint tag) { - auto subscription = std::shared_ptr( - G_DBUS_CONNECTION(g_object_ref(bus)), - [tag](GDBusConnection* bus){ - g_dbus_connection_signal_unsubscribe(bus, tag); - g_object_unref(G_OBJECT(bus)); - } - ); - - m_subscriptions[name].push_back(subscription); + g_object_ref(bus); + + auto deleter = [tag](GDBusConnection* bus){ + g_dbus_connection_signal_unsubscribe(bus, tag); + g_object_unref(G_OBJECT(bus)); + }; + + m_subscriptions[name].push_back(std::shared_ptr(bus, deleter)); } /** @@ -115,6 +123,53 @@ private: GVariant* /*parameters*/, gpointer gself) { + g_debug("firing clock.minute_changed() due to PrepareForSleep"); + static_cast(gself)->m_owner.minute_changed(); + } + + /** + *** DBus Chatter: com.canonical.powerd + *** + *** Fire Clock::minute_changed() signal when powerd says the system's + *** has awoken from sleep -- the old timestamp is likely out-of-date + **/ + + static void on_powerd_appeared(GDBusConnection * bus, + const gchar * name, + const gchar * name_owner, + gpointer gself) + { + auto tag = g_dbus_connection_signal_subscribe(bus, + name_owner, + BUS_POWERD_INTERFACE, + "Wakeup", // signal name + BUS_POWERD_PATH, + nullptr, // arg0 + G_DBUS_SIGNAL_FLAGS_NONE, + on_wakeup, + gself, // user_data + nullptr); // user_data closure + + + static_cast(gself)->remember_subscription(name, bus, tag); + } + + static void on_powerd_vanished(GDBusConnection * /*bus*/, + const gchar * name, + gpointer gself) + { + static_cast(gself)->m_subscriptions[name].clear(); + } + + static void on_wakeup(GDBusConnection* /*connection*/, + const gchar* /*sender_name*/, + const gchar* /*object_path*/, + const gchar* /*interface_name*/, + const gchar* /*signal_name*/, + GVariant* /*parameters*/, + gpointer gself) + { + g_debug("firing clock.minute_changed() due to powerd.Wakeup"); static_cast(gself)->m_owner.minute_changed(); } -- cgit v1.2.3 From a705cb47912f6785a3dda0f67eee264938cdf4e0 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 15 Sep 2014 09:56:29 -0500 Subject: add translator comments for the date format string in snap.cpp --- src/snap.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/snap.cpp b/src/snap.cpp index b3368a1..c2cbc0a 100644 --- a/src/snap.cpp +++ b/src/snap.cpp @@ -94,7 +94,16 @@ public: b.add_hint (uin::Builder::HINT_TINT); b.add_hint (uin::Builder::HINT_NONSHAPEDICON); - const auto timefmt = is_locale_12h() ? _("%a, %l:%M %p") : _("%a, %H:%M"); + const char * timefmt; + if (is_locale_12h()) { + /** strftime(3) format for abbreviated weekday, + hours, minutes in a 12h locale; e.g. Wed, 2:00 PM */ + timefmt = _("%a, %l:%M %p"); + } else { + /** A strftime(3) format for abbreviated weekday, + hours, minutes in a 24h locale; e.g. Wed, 14:00 */ + timefmt = _("%a, %H:%M"); + } const auto timestr = appointment.begin.format(timefmt); auto title = g_strdup_printf(_("Alarm %s"), timestr.c_str()); b.set_title (title); -- cgit v1.2.3 From d52c7d1b04e5c4353533b9927617e1753784d2c8 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 15 Sep 2014 09:56:52 -0500 Subject: listen to powerd SysPowerStateChange instead of Wakeup --- src/clock.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/clock.cpp b/src/clock.cpp index 35690dc..a04e074 100644 --- a/src/clock.cpp +++ b/src/clock.cpp @@ -169,11 +169,11 @@ private: auto tag = g_dbus_connection_signal_subscribe(bus, name_owner, BUS_POWERD_INTERFACE, - "Wakeup", // signal name + "SysPowerStateChange", BUS_POWERD_PATH, nullptr, // arg0 G_DBUS_SIGNAL_FLAGS_NONE, - on_wakeup, + on_sys_power_state_change, gself, // user_data nullptr); // user_data closure @@ -188,15 +188,15 @@ private: static_cast(gself)->m_subscriptions[name].clear(); } - static void on_wakeup(GDBusConnection* /*connection*/, - const gchar* /*sender_name*/, - const gchar* /*object_path*/, - const gchar* /*interface_name*/, - const gchar* /*signal_name*/, - GVariant* /*parameters*/, - gpointer gself) + static void on_sys_power_state_change(GDBusConnection* /*connection*/, + const gchar* /*sender_name*/, + const gchar* /*object_path*/, + const gchar* /*interface_name*/, + const gchar* /*signal_name*/, + GVariant* /*parameters*/, + gpointer gself) { - g_debug("firing clock.minute_changed() due to powerd.Wakeup"); + g_debug("firing clock.minute_changed() due to state change"); static_cast(gself)->m_owner.minute_changed(); } -- cgit v1.2.3 From 321873e4c41328e6ef61ca11362abdc753cf1178 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 15 Sep 2014 12:02:50 -0500 Subject: add manual test indicator-datetime/timestamp-wakeup --- tests/manual | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/manual b/tests/manual index e9f858e..2b16841 100644 --- a/tests/manual +++ b/tests/manual @@ -22,6 +22,13 @@ Test-case indicator-datetime/unity8-items-check
The menu is populated with items
+Test-case indicator-datetime/timestamp-wakeup +
+
Unplug the phone from any USB connection and put it to sleep
+
Reawaken the device. +
The indicator should be showing the correct time.
+
+ Test-case indicator-datetime/new-alarm-wakeup
Create and save an upcoming alarm in ubuntu-clock-app
-- cgit v1.2.3 From c0055ef9118a7a284b26d164617985236a2d432a Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 15 Sep 2014 14:49:20 -0500 Subject: add unit tests for the powerd monitor --- include/datetime/clock-mock.h | 12 +++++++-- tests/test-clock.cpp | 61 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/include/datetime/clock-mock.h b/include/datetime/clock-mock.h index fb9b52f..0e24377 100644 --- a/include/datetime/clock-mock.h +++ b/include/datetime/clock-mock.h @@ -41,15 +41,23 @@ public: DateTime localtime() const { return m_localtime; } - void set_localtime(const DateTime& dt) { + void set_localtime(const DateTime& dt) + { const auto old = m_localtime; - m_localtime = dt; + + set_localtime_quietly(dt); + if (!DateTime::is_same_minute(old, m_localtime)) minute_changed(); if (!DateTime::is_same_day(old, m_localtime)) date_changed(); } + void set_localtime_quietly(const DateTime& dt) + { + m_localtime = dt; + } + private: DateTime m_localtime; }; diff --git a/tests/test-clock.cpp b/tests/test-clock.cpp index 82f8cb0..62281fb 100644 --- a/tests/test-clock.cpp +++ b/tests/test-clock.cpp @@ -18,8 +18,11 @@ */ #include +#include #include +#include + #include "test-dbus-fixture.h" /*** @@ -152,3 +155,61 @@ TEST_F(ClockFixture, SleepTriggersSkew) g_bus_unown_name(name_tag); } + +namespace +{ + void on_powerd_name_acquired(GDBusConnection * /*connection*/, + const gchar * /*name*/, + gpointer is_owned) + { + *static_cast(is_owned) = true; + } +} + +/** + * Confirm that powerd's SysPowerStateChange triggers + * a timestamp change + */ +TEST_F(ClockFixture, SysPowerStateChange) +{ + // set up the mock clock + bool minute_changed = false; + auto clock = std::make_shared(DateTime::NowLocal()); + clock->minute_changed.connect([&minute_changed]() { + minute_changed = true; + }); + + // control test -- minute_changed shouldn't get triggered + // when the clock is silently changed + gboolean is_owned = false; + auto tag = g_bus_own_name_on_connection(system_bus, + BUS_POWERD_NAME, + G_BUS_NAME_OWNER_FLAGS_NONE, + on_powerd_name_acquired, + nullptr, + &is_owned /* user_data */, + nullptr /* user_data closure */); + const DateTime not_now {DateTime::Local(1999, 12, 31, 23, 59, 59)}; + clock->set_localtime_quietly(not_now); + wait_msec(); + ASSERT_TRUE(is_owned); + ASSERT_FALSE(minute_changed); + + // now for the actual test, + // confirm that SysPowerStateChange triggers a minute_changed() signal + GError * error = nullptr; + auto emitted = g_dbus_connection_emit_signal(system_bus, + nullptr, + BUS_POWERD_PATH, + BUS_POWERD_INTERFACE, + "SysPowerStateChange", + g_variant_new("(i)", 1), + &error); + wait_msec(); + EXPECT_TRUE(emitted); + EXPECT_EQ(nullptr, error); + EXPECT_TRUE(minute_changed); + + // cleanup + g_bus_unown_name(tag); +} -- cgit v1.2.3 From 790f43bcea23f926f6617e0fe41026a3adaee8cc Mon Sep 17 00:00:00 2001 From: CI bot Date: Mon, 15 Sep 2014 21:03:01 +0000 Subject: Releasing 13.10.0+14.10.20140915-0ubuntu1 --- debian/changelog | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/debian/changelog b/debian/changelog index 04fcc14..a27953d 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,11 @@ +indicator-datetime (13.10.0+14.10.20140915-0ubuntu1) utopic; urgency=low + + [ Charles Kerr ] + * Update the time strings when a powerd Wakeup signal is detected. + (LP: #1359802) + + -- Ubuntu daily release Mon, 15 Sep 2014 21:03:00 +0000 + indicator-datetime (13.10.0+14.10.20140908.1-0ubuntu1) utopic; urgency=low [ Charles Kerr ] -- cgit v1.2.3