From aad7e86a109aeec75b3772cda20478363f966745 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 22 Jan 2014 14:28:20 -0600 Subject: Alarms is going to need to know when the clock's minute changes. We already have a timer for that in Formatter, so move it from there to Clock and add a corresponding public signal Clock.minuteChanged that both Formatter and Alarms can use. Sync unit tests. --- include/datetime/clock-mock.h | 8 +-- include/datetime/clock.h | 20 ++++---- include/datetime/date-time.h | 5 +- src/clock-live.cpp | 113 ++++++++++++++++++++++++------------------ src/clock.cpp | 2 +- src/date-time.cpp | 65 ++++++++++-------------- src/formatter.cpp | 106 ++++++++++++++++++--------------------- tests/test-clock.cpp | 32 ++++++------ 8 files changed, 169 insertions(+), 182 deletions(-) diff --git a/include/datetime/clock-mock.h b/include/datetime/clock-mock.h index 19a859b..27926ff 100644 --- a/include/datetime/clock-mock.h +++ b/include/datetime/clock-mock.h @@ -42,11 +42,11 @@ public: DateTime localtime() const { return m_localtime; } void set_localtime(const DateTime& dt) { - const auto old_day = m_localtime.day_of_year(); + const auto old = m_localtime; m_localtime = dt; - skewDetected(); - const auto new_day = m_localtime.day_of_year(); - if (old_day != new_day) + if (!DateTime::is_same_minute(old, m_localtime)) + minuteChanged(); + if (!DateTime::is_same_day(old, m_localtime)) dateChanged(); } diff --git a/include/datetime/clock.h b/include/datetime/clock.h index 9e87082..b3e3538 100644 --- a/include/datetime/clock.h +++ b/include/datetime/clock.h @@ -35,21 +35,25 @@ namespace datetime { /** * \brief A clock. - * - * Provides a signal to notify when clock skew is detected, such as - * when the timezone changes or when the system resumes from sleep. */ class Clock { public: virtual ~Clock(); virtual DateTime localtime() const =0; - core::Signal<> skewDetected; + + /** \brief A signal which fires when the clock's minute changes */ + core::Signal<> minuteChanged; + + /** \brief A signal which fires when the clock's date changes */ core::Signal<> dateChanged; protected: Clock(); + /** \brief Compares old and new times, emits minuteChanged() or dateChanged() signals if appropriate */ + void maybe_emit (const DateTime& a, const DateTime& b); + private: static void onSystemBusReady(GObject*, GAsyncResult*, gpointer); static void onPrepareForSleep(GDBusConnection*, const gchar*, const gchar*, const gchar*, const gchar*, GVariant*, gpointer); @@ -70,12 +74,7 @@ private: class Timezones; /** - * \brief A live clock that provides the actual system time. - * - * This subclass also adds another clock skew detection test: - * it wakes up every skewTestIntervalSec seconds to see how - * much time has passed since the last wakeup. If the answer - * isn't what it expected, the skewDetected signal is triggered. + * \brief A live #Clock that provides the actual system time. */ class LiveClock: public Clock { @@ -83,7 +82,6 @@ public: LiveClock (const std::shared_ptr& zones); virtual ~LiveClock(); virtual DateTime localtime() const; - core::Property skewTestIntervalSec; private: class Impl; diff --git a/include/datetime/date-time.h b/include/datetime/date-time.h index 145e34e..c2429eb 100644 --- a/include/datetime/date-time.h +++ b/include/datetime/date-time.h @@ -49,13 +49,14 @@ public: std::string format(const std::string& fmt) const; int day_of_month() const; int64_t to_unix() const; - int day_of_year() const; - gint64 difference(const DateTime& that) const; bool operator<(const DateTime& that) const; bool operator!=(const DateTime& that) const; bool operator==(const DateTime& that) const; + static bool is_same_day(const DateTime& a, const DateTime& b); + static bool is_same_minute(const DateTime& a, const DateTime& b); + private: std::shared_ptr m_dt; }; diff --git a/src/clock-live.cpp b/src/clock-live.cpp index 25623ae..69ebda7 100644 --- a/src/clock-live.cpp +++ b/src/clock-live.cpp @@ -24,6 +24,37 @@ namespace unity { namespace indicator { namespace datetime { +/*** +**** +***/ + +namespace +{ + +void clearTimer(guint& tag) +{ + if (tag) + { + g_source_remove(tag); + tag = 0; + } +} + +guint calculate_milliseconds_until_next_minute(const DateTime& now) +{ + auto next = g_date_time_add_minutes(now.get(), 1); + auto start_of_next = g_date_time_add_seconds (next, -g_date_time_get_seconds(next)); + const auto interval_usec = g_date_time_difference(start_of_next, now.get()); + const guint interval_msec = (interval_usec + 999) / 1000; + g_date_time_unref(start_of_next); + g_date_time_unref(next); + g_assert (interval_msec <= 60000); + return interval_msec; +} + +} // unnamed namespace + + class LiveClock::Impl { public: @@ -38,13 +69,12 @@ public: setTimezone(m_timezones->timezone.get()); } - m_owner.skewTestIntervalSec.changed().connect([this](unsigned int intervalSec) {setInterval(intervalSec);}); - setInterval(m_owner.skewTestIntervalSec.get()); + restart_minute_timer(); } ~Impl() { - clearTimer(); + clearTimer(m_timer); g_clear_pointer(&m_timezone, g_time_zone_unref); } @@ -64,70 +94,55 @@ private: void setTimezone(const std::string& str) { g_clear_pointer(&m_timezone, g_time_zone_unref); - m_timezone= g_time_zone_new(str.c_str()); - m_owner.skewDetected(); - } - -private: - - void clearTimer() - { - if (m_skew_timeout_id) - { - g_source_remove(m_skew_timeout_id); - m_skew_timeout_id = 0; - } - - m_prev_datetime.reset(); + m_timezone = g_time_zone_new(str.c_str()); + m_owner.minuteChanged(); } - void setInterval(unsigned int seconds) - { - clearTimer(); - - if (seconds > 0) - { - m_prev_datetime = localtime(); - m_skew_timeout_id = g_timeout_add_seconds(seconds, onTimerPulse, this); - } - } + /*** + **** + ***/ - static gboolean onTimerPulse(gpointer gself) + void restart_minute_timer() { - static_cast(gself)->onTimerPulse(); - return G_SOURCE_CONTINUE; - } + clearTimer(m_timer); - void onTimerPulse() - { - // check to see if too much time passed since the last check */ + // maybe emit change signals const auto now = localtime(); - const auto diff = now.difference (m_prev_datetime); - const GTimeSpan fuzz = 5; - const GTimeSpan max = (m_owner.skewTestIntervalSec.get() + fuzz) * G_USEC_PER_SEC; - if (abs(diff) > max) - m_owner.skewDetected(); - - // check to see if the day has changed - if (now.day_of_year() != m_prev_datetime.day_of_year()) + if (!DateTime::is_same_minute(m_prev_datetime, now)) + m_owner.minuteChanged(); + if (!DateTime::is_same_day(m_prev_datetime, now)) m_owner.dateChanged(); - // update m_prev_datetime + // queue up a timer to fire at the next minute m_prev_datetime = now; + auto interval_msec = calculate_milliseconds_until_next_minute(now); + interval_msec += 50; // add a small margin to ensure the callback + // fires /after/ next is reached + m_timer = g_timeout_add_full(G_PRIORITY_HIGH, + interval_msec, + on_minute_timer_reached, + this, + nullptr); + } + + static gboolean on_minute_timer_reached(gpointer gself) + { + static_cast(gself)->restart_minute_timer(); + return G_SOURCE_REMOVE; } protected: LiveClock& m_owner; - GTimeZone * m_timezone = nullptr; + GTimeZone* m_timezone = nullptr; std::shared_ptr m_timezones; DateTime m_prev_datetime; - unsigned int m_skew_timeout_id = 0; + unsigned int m_timer = 0; }; LiveClock::LiveClock(const std::shared_ptr& tzd): - p(new Impl(*this, tzd)) + p(new Impl(*this, tzd)) { } @@ -138,6 +153,10 @@ DateTime LiveClock::localtime() const return p->localtime(); } +/*** +**** +***/ + } // namespace datetime } // namespace indicator } // namespace unity diff --git a/src/clock.cpp b/src/clock.cpp index 7c3b8c7..d5293cc 100644 --- a/src/clock.cpp +++ b/src/clock.cpp @@ -81,7 +81,7 @@ Clock::onPrepareForSleep(GDBusConnection* /*connection*/, GVariant* /*parameters*/, gpointer gself) { - static_cast(gself)->skewDetected(); + static_cast(gself)->minuteChanged(); } /*** diff --git a/src/date-time.cpp b/src/date-time.cpp index 3842ac0..40c638f 100644 --- a/src/date-time.cpp +++ b/src/date-time.cpp @@ -76,11 +76,6 @@ int64_t DateTime::to_unix() const return g_date_time_to_unix(get()); } -int DateTime::day_of_year() const -{ - return m_dt ? g_date_time_get_day_of_year(get()) : -1; -} - void DateTime::reset(GDateTime* in) { if (in) @@ -95,39 +90,6 @@ void DateTime::reset(GDateTime* in) } } -#if 0 -DateTime& DateTime::operator=(GDateTime* in) -{ - reset(in); - return *this; -} - -DateTime& DateTime::operator=(const DateTime& in) -{ - m_dt = in.m_dt; - return *this; -} -#endif - -gint64 DateTime::difference(const DateTime& that) const -{ - const auto dt = get(); - const auto tdt = that.get(); - - gint64 ret; - - if (dt && tdt) - ret = g_date_time_difference(dt, tdt); - else if (dt) - ret = to_unix(); - else if (tdt) - ret = that.to_unix(); - else - ret = 0; - - return ret; -} - bool DateTime::operator<(const DateTime& that) const { return g_date_time_compare(get(), that.get()) < 0; @@ -141,13 +103,36 @@ bool DateTime::operator!=(const DateTime& that) const bool DateTime::operator==(const DateTime& that) const { - GDateTime * dt = get(); - GDateTime * tdt = that.get(); + auto dt = get(); + auto tdt = that.get(); if (!dt && !tdt) return true; if (!dt || !tdt) return false; return g_date_time_compare(get(), that.get()) == 0; } +bool DateTime::is_same_day(const DateTime& a, const DateTime& b) +{ + // it's meaningless to compare uninitialized dates + if (!a.m_dt || !b.m_dt) + return false; + + const auto adt = a.get(); + const auto bdt = b.get(); + return (g_date_time_get_year(adt) == g_date_time_get_year(bdt)) + && (g_date_time_get_day_of_year(adt) == g_date_time_get_day_of_year(bdt)); +} + +bool DateTime::is_same_minute(const DateTime& a, const DateTime& b) +{ + if (!is_same_day(a,b)) + return false; + + const auto adt = a.get(); + const auto bdt = b.get(); + return (g_date_time_get_hour(adt) == g_date_time_get_hour(bdt)) + && (g_date_time_get_minute(adt) == g_date_time_get_minute(bdt)); +} + /*** **** ***/ diff --git a/src/formatter.cpp b/src/formatter.cpp index 88a64df..a271ba0 100644 --- a/src/formatter.cpp +++ b/src/formatter.cpp @@ -28,9 +28,18 @@ #include // nl_langinfo() #include // strstr() +namespace unity { +namespace indicator { +namespace datetime { + +/*** +**** +***/ + namespace { -void clearTimer(guint& tag) + +void clear_timer(guint& tag) { if (tag) { @@ -39,35 +48,12 @@ void clearTimer(guint& tag) } } -guint calculate_milliseconds_until_next_minute(GDateTime * now) -{ - GDateTime * next; - GDateTime * start_of_next; - GTimeSpan interval_usec; - guint interval_msec; - - next = g_date_time_add_minutes(now, 1); - start_of_next = g_date_time_new_local(g_date_time_get_year(next), - g_date_time_get_month(next), - g_date_time_get_day_of_month(next), - g_date_time_get_hour(next), - g_date_time_get_minute(next), - 0.1); - - interval_usec = g_date_time_difference(start_of_next, now); - interval_msec = (interval_usec + 999) / 1000; - - g_date_time_unref(start_of_next); - g_date_time_unref(next); - return interval_msec; -} - -gint calculate_milliseconds_until_next_second(GDateTime * now) +gint calculate_milliseconds_until_next_second(const DateTime& now) { gint interval_usec; guint interval_msec; - interval_usec = G_USEC_PER_SEC - g_date_time_get_microsecond(now); + interval_usec = G_USEC_PER_SEC - g_date_time_get_microsecond(now.get()); interval_msec = (interval_usec + 999) / 1000; return interval_msec; } @@ -127,11 +113,6 @@ guint calculate_seconds_until_next_fifteen_minutes(GDateTime * now) } // unnamed namespace - -namespace unity { -namespace indicator { -namespace datetime { - class Formatter::Impl { public: @@ -140,57 +121,64 @@ public: m_owner(owner), m_clock(clock) { - m_owner->headerFormat.changed().connect([this](const std::string& /*fmt*/){updateHeader();}); - m_clock->skewDetected.connect([this](){updateHeader();}); - updateHeader(); + m_owner->headerFormat.changed().connect([this](const std::string& /*fmt*/){update_header();}); + m_clock->minuteChanged.connect([this](){update_header();}); + update_header(); restartRelativeTimer(); } ~Impl() { - clearTimer(m_header_timer); + clear_timer(m_header_seconds_timer); + clear_timer(m_relative_timer); } private: - void updateHeader() + static bool format_shows_seconds(const std::string& fmt) { + return (fmt.find("%s") != std::string::npos) + || (fmt.find("%S") != std::string::npos) + || (fmt.find("%T") != std::string::npos) + || (fmt.find("%X") != std::string::npos) + || (fmt.find("%c") != std::string::npos); + } + + void update_header() + { + // update the header property const auto fmt = m_owner->headerFormat.get(); const auto str = m_clock->localtime().format(fmt); m_owner->header.set(str); - restartHeaderTimer(); + // if the header needs to show seconds, set a timer. + if (format_shows_seconds(fmt)) + start_header_timer(); + else + clear_timer(m_header_seconds_timer); } - void restartHeaderTimer() + // we've got a header format that shows seconds, + // so we need to update it every second + void start_header_timer() { - clearTimer(m_header_timer); + clear_timer(m_header_seconds_timer); - const auto fmt = m_owner->headerFormat.get(); - const bool header_shows_seconds = (fmt.find("%s") != std::string::npos) - || (fmt.find("%S") != std::string::npos) - || (fmt.find("%T") != std::string::npos) - || (fmt.find("%X") != std::string::npos) - || (fmt.find("%c") != std::string::npos); - - guint interval_msec; const auto now = m_clock->localtime(); - auto str = now.format("%F %T"); - if (header_shows_seconds) - interval_msec = calculate_milliseconds_until_next_second(now.get()); - else - interval_msec = calculate_milliseconds_until_next_minute(now.get()); - + auto interval_msec = calculate_milliseconds_until_next_second(now); interval_msec += 50; // add a small margin to ensure the callback // fires /after/ next is reached - - m_header_timer = g_timeout_add_full(G_PRIORITY_HIGH, interval_msec, onHeaderTimer, this, nullptr); + m_header_seconds_timer = g_timeout_add_full(G_PRIORITY_HIGH, + interval_msec, + on_header_timer, + this, + nullptr); } - static gboolean onHeaderTimer(gpointer gself) + static gboolean on_header_timer(gpointer gself) { - static_cast(gself)->updateHeader(); + static_cast(gself)->update_header(); return G_SOURCE_REMOVE; } @@ -198,7 +186,7 @@ private: void restartRelativeTimer() { - clearTimer(m_relative_timer); + clear_timer(m_relative_timer); const auto now = m_clock->localtime(); const auto seconds = calculate_seconds_until_next_fifteen_minutes(now.get()); @@ -215,7 +203,7 @@ private: private: Formatter* const m_owner; - guint m_header_timer = 0; + guint m_header_seconds_timer = 0; guint m_relative_timer = 0; public: diff --git a/tests/test-clock.cpp b/tests/test-clock.cpp index 142ccad..4271374 100644 --- a/tests/test-clock.cpp +++ b/tests/test-clock.cpp @@ -46,28 +46,24 @@ class ClockFixture: public TestDBusFixture } }; -/** - * Confirm that normal time passing doesn't trigger a skew event. - * that idling changing the clock's time triggers a skew event - */ -TEST_F(ClockFixture, IdleDoesNotTriggerSkew) +TEST_F(ClockFixture, MinuteChangedSignalShouldTriggerOncePerMinute) { + // start up a live clock 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 - bool skewed = false; - clock.skewDetected.connect([&skewed](){ - skewed = true; - g_warn_if_reached(); - return G_SOURCE_REMOVE; - }); - - const unsigned int intervalSec = 3; - clock.skewTestIntervalSec.set(intervalSec); - wait_msec(intervalSec * 2.5 * 1000); - EXPECT_FALSE(skewed); + // count how many times clock.minuteChanged() is emitted over the next minute + const DateTime now = clock.localtime(); + const auto gnow = now.get(); + auto gthen = g_date_time_add_minutes(gnow, 1); + int count = 0; + clock.minuteChanged.connect([&count](){count++;}); + const auto msec = g_date_time_difference(gthen,gnow) / 1000; + wait_msec(msec); + EXPECT_EQ(1, count); + g_date_time_unref(gthen); } /*** @@ -99,7 +95,7 @@ TEST_F(ClockFixture, TimezoneChangeTriggersSkew) g_time_zone_unref(tz_nyc); /// change the timezones! - clock.skewDetected.connect([this](){ + clock.minuteChanged.connect([this](){ g_main_loop_quit(loop); }); g_idle_add([](gpointer gs){ @@ -128,7 +124,7 @@ TEST_F(ClockFixture, SleepTriggersSkew) wait_msec(500); // wait for the bus to set up bool skewed = false; - clock.skewDetected.connect([&skewed, this](){ + clock.minuteChanged.connect([&skewed, this](){ skewed = true; g_main_loop_quit(loop); return G_SOURCE_REMOVE; -- cgit v1.2.3