From 113b741bfa973132f98d6d0b1b15a6413f5544b9 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 11 Feb 2015 18:20:20 -0600 Subject: change the LiveClock's private impl to detect time changes from timerfd's TFD_TIMER_CANCEL_ON_SET, e.g. when ntp<-->manual is toggled --- src/clock-live.cpp | 125 +++++++++++++++++++++++++++++++++-------------------- 1 file changed, 77 insertions(+), 48 deletions(-) diff --git a/src/clock-live.cpp b/src/clock-live.cpp index 68a8a8b..7827444 100644 --- a/src/clock-live.cpp +++ b/src/clock-live.cpp @@ -20,6 +20,15 @@ #include #include +#include // g_unix_fd_add() + +#include +#include // close() + +#ifndef TFD_TIMER_CANCEL_ON_SET + #define TFD_TIMER_CANCEL_ON_SET (1 << 1) +#endif + namespace unity { namespace indicator { namespace datetime { @@ -28,33 +37,6 @@ 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: @@ -70,12 +52,50 @@ public: setter(m_timezone->timezone.get()); } - restart_minute_timer(); + m_timerfd = timerfd_create(CLOCK_REALTIME, 0); + if (m_timerfd == -1) + { + g_warning("unable to create realtime timer: %s", g_strerror(errno)); + } + else + { + struct itimerspec timerval; + // set args to fire at the beginning of the next minute... + int flags = TFD_TIMER_ABSTIME; + auto now = g_date_time_new_now(m_gtimezone); + auto next = g_date_time_add_minutes(now, 1); + auto start_of_next = g_date_time_add_seconds(next, -g_date_time_get_seconds(next)); + timerval.it_value.tv_sec = g_date_time_to_unix(start_of_next); + timerval.it_value.tv_nsec = 0; + g_date_time_unref(start_of_next); + g_date_time_unref(next); + g_date_time_unref(now); + // ...and also to fire at the beginning of every subsequent minute... + timerval.it_interval.tv_sec = 60; + timerval.it_interval.tv_nsec = 0; + // ...and also to fire if someone changes the time + // manually (eg toggling from manual<->ntp) + flags |= TFD_TIMER_CANCEL_ON_SET; + + if (timerfd_settime(m_timerfd, flags, &timerval, NULL) == -1) + g_error("timerfd_settime failed: %s", g_strerror(errno)); + + m_timerfd_tag = g_unix_fd_add(m_timerfd, + (GIOCondition)(G_IO_IN|G_IO_HUP|G_IO_ERR), + on_timerfd_cond, + this); + } + + refresh(); } ~Impl() { - clearTimer(m_timer); + if (m_timerfd_tag != 0) + g_source_remove(m_timerfd_tag); + + if (m_timerfd != -1) + close(m_timerfd); g_clear_pointer(&m_gtimezone, g_time_zone_unref); } @@ -92,6 +112,30 @@ public: private: + static gboolean on_timerfd_cond (gint fd, GIOCondition cond G_GNUC_UNUSED, gpointer gself) + { + // let's see what triggered this event + auto self = static_cast(gself); + uint64_t n_interrupts = 0; + auto s = read(fd, &n_interrupts, sizeof(uint64_t)); + + // make a debug log of what just happened + auto now = g_date_time_new_now(self->m_gtimezone); + auto now_str = g_date_time_format(now, "%F %T"); + g_debug("%s at %s (%f), read %zd bytes to get n_interrupts %zu", + G_STRFUNC, now_str, g_date_time_get_seconds(now), + s, n_interrupts); + g_free(now_str); + g_date_time_unref(now); + + self->refresh(); + return G_SOURCE_CONTINUE; + } + + /*** + **** + ***/ + void setTimezone(const std::string& str) { g_clear_pointer(&m_gtimezone, g_time_zone_unref); @@ -103,33 +147,17 @@ private: **** ***/ - void restart_minute_timer() + void refresh() { - clearTimer(m_timer); + const auto now = localtime(); // maybe emit change signals - const auto now = localtime(); if (!DateTime::is_same_minute(m_prev_datetime, now)) m_owner.minute_changed(); if (!DateTime::is_same_day(m_prev_datetime, now)) m_owner.date_changed(); - // 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: @@ -139,7 +167,8 @@ protected: std::shared_ptr m_timezone; DateTime m_prev_datetime; - unsigned int m_timer = 0; + int m_timerfd = -1; + guint m_timerfd_tag = 0; }; LiveClock::LiveClock(const std::shared_ptr& timezone_): -- cgit v1.2.3 From 5df082224c0f4664b8798261abce1d0b1b5ab2d9 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 12 Feb 2015 14:22:11 -0600 Subject: add manual test: indicator-datetime/manual-time --- tests/manual | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/manual b/tests/manual index 007f967..95e5401 100644 --- a/tests/manual +++ b/tests/manual @@ -137,6 +137,12 @@ Test-case indicator-datetime/silent-mode
The calendar event notification should play a sound.
+Test-case indicator-datetime/manual-time +
+
In System Settings > Time & Date, manually change the time to an arbitrary time.
+
The indicator's timestamp should change right after the time manual time is set.
+
+ If all actions produce the expected results listed, please submit a 'passed' result. If an action fails, or produces an unexpected result, please submit a 'failed' result and file a bug. Please be sure to include the bug number when you submit your result. -- cgit v1.2.3 From de6bc91f29122242fab6da7f5120f5bc8c64a1d6 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 12 Feb 2015 14:38:49 -0600 Subject: extract the timerfd_settime() code into its own method. Add a new call to it when we might have triggered via TFD_TIMER_CANCEL_ON_SET --- src/clock-live.cpp | 49 +++++++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/src/clock-live.cpp b/src/clock-live.cpp index 7827444..b91fc62 100644 --- a/src/clock-live.cpp +++ b/src/clock-live.cpp @@ -59,26 +59,7 @@ public: } else { - struct itimerspec timerval; - // set args to fire at the beginning of the next minute... - int flags = TFD_TIMER_ABSTIME; - auto now = g_date_time_new_now(m_gtimezone); - auto next = g_date_time_add_minutes(now, 1); - auto start_of_next = g_date_time_add_seconds(next, -g_date_time_get_seconds(next)); - timerval.it_value.tv_sec = g_date_time_to_unix(start_of_next); - timerval.it_value.tv_nsec = 0; - g_date_time_unref(start_of_next); - g_date_time_unref(next); - g_date_time_unref(now); - // ...and also to fire at the beginning of every subsequent minute... - timerval.it_interval.tv_sec = 60; - timerval.it_interval.tv_nsec = 0; - // ...and also to fire if someone changes the time - // manually (eg toggling from manual<->ntp) - flags |= TFD_TIMER_CANCEL_ON_SET; - - if (timerfd_settime(m_timerfd, flags, &timerval, NULL) == -1) - g_error("timerfd_settime failed: %s", g_strerror(errno)); + reset_timer(); m_timerfd_tag = g_unix_fd_add(m_timerfd, (GIOCondition)(G_IO_IN|G_IO_HUP|G_IO_ERR), @@ -112,6 +93,30 @@ public: private: + void reset_timer() + { + struct itimerspec timerval; + // set args to fire at the beginning of the next minute... + int flags = TFD_TIMER_ABSTIME; + auto now = g_date_time_new_now(m_gtimezone); + auto next = g_date_time_add_minutes(now, 1); + auto start_of_next = g_date_time_add_seconds(next, -g_date_time_get_seconds(next)); + timerval.it_value.tv_sec = g_date_time_to_unix(start_of_next); + timerval.it_value.tv_nsec = 0; + g_date_time_unref(start_of_next); + g_date_time_unref(next); + g_date_time_unref(now); + // ...and also to fire at the beginning of every subsequent minute... + timerval.it_interval.tv_sec = 60; + timerval.it_interval.tv_nsec = 0; + // ...and also to fire if someone changes the time + // manually (eg toggling from manual<->ntp) + flags |= TFD_TIMER_CANCEL_ON_SET; + + if (timerfd_settime(m_timerfd, flags, &timerval, NULL) == -1) + g_error("timerfd_settime failed: %s", g_strerror(errno)); + } + static gboolean on_timerfd_cond (gint fd, GIOCondition cond G_GNUC_UNUSED, gpointer gself) { // let's see what triggered this event @@ -127,6 +132,10 @@ private: s, n_interrupts); g_free(now_str); g_date_time_unref(now); + + // if we weren't triggered because of a timeout, then it may have + // happened due to time being set, so reset the timer + self->reset_timer(); self->refresh(); return G_SOURCE_CONTINUE; -- cgit v1.2.3 From b6899cbf72d3f1596fc466b1aa365cea83e55410 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 12 Feb 2015 17:03:38 -0600 Subject: add a g_message() when updating due to anything that's not a periodic timer --- src/clock-live.cpp | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/clock-live.cpp b/src/clock-live.cpp index b91fc62..988d47f 100644 --- a/src/clock-live.cpp +++ b/src/clock-live.cpp @@ -117,25 +117,28 @@ private: g_error("timerfd_settime failed: %s", g_strerror(errno)); } - static gboolean on_timerfd_cond (gint fd, GIOCondition cond G_GNUC_UNUSED, gpointer gself) + static gboolean on_timerfd_cond (gint fd, GIOCondition cond, gpointer gself) { - // let's see what triggered this event auto self = static_cast(gself); + + int n_bytes = 0; uint64_t n_interrupts = 0; - auto s = read(fd, &n_interrupts, sizeof(uint64_t)); - - // make a debug log of what just happened - auto now = g_date_time_new_now(self->m_gtimezone); - auto now_str = g_date_time_format(now, "%F %T"); - g_debug("%s at %s (%f), read %zd bytes to get n_interrupts %zu", - G_STRFUNC, now_str, g_date_time_get_seconds(now), - s, n_interrupts); - g_free(now_str); - g_date_time_unref(now); + if (cond & G_IO_IN) + n_bytes = read(fd, &n_interrupts, sizeof(uint64_t)); - // if we weren't triggered because of a timeout, then it may have - // happened due to time being set, so reset the timer - self->reset_timer(); + if ((n_interrupts==0) || (n_bytes!=sizeof(uint64_t))) + { + auto now = g_date_time_new_now(self->m_gtimezone); + auto now_str = g_date_time_format(now, "%F %T"); + g_message("%s triggered at %s.%06d by GIOCondition %d, read %zd bytes, found %zu interrupts", + G_STRFUNC, now_str, g_date_time_get_microsecond(now), + (int)cond, n_bytes, n_interrupts); + g_free(now_str); + g_date_time_unref(now); + + // reset the timer in case someone changed the system clock + self->reset_timer(); + } self->refresh(); return G_SOURCE_CONTINUE; -- cgit v1.2.3