From a660b6e5dd1d53bdbe9bdc5239665a647a2e2080 Mon Sep 17 00:00:00 2001
From: Andreas Grois <andi@grois.info>
Date: Thu, 22 Apr 2021 21:07:46 +0200
Subject: Fix rounding issue in clock:

Sleep a millisecond if otherwise an update would get skipped. This
happens if a refresh is requested in the last millisecond before the
next synchronized update is expected.
---
 TODO             |  4 ----
 clock/src/lib.rs | 23 +++++++++++++++++++++--
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/TODO b/TODO
index 62b91b1..36ff903 100644
--- a/TODO
+++ b/TODO
@@ -1,7 +1,3 @@
 o) Factor out lib- and plugin-loading in a way that still gives a vector of libs, but an Iterator over the plugins (so caller can choose if Vec or something else)
 o) Add --plugin-help parameter that takes a list of plugins for which to display help (or if none given for all of them). This also means changing the Plugin trait once more.
 o) Add --list-plugins parameter that just lists all loaded plugins.
-
-Clock:
-o) Add a corrective sleep of 1 ms if rounding errors are detected at startup or manual update. (current time + 1 ms rounds to a different update-fraction than current time)
-o) Change configuration format. Instead of directly entering seconds as a float, update rate should be "how often to update within 30 minutes" as u16. That way the _data type_ already enforces the precision limits :D
diff --git a/clock/src/lib.rs b/clock/src/lib.rs
index 79426ce..3c551a6 100644
--- a/clock/src/lib.rs
+++ b/clock/src/lib.rs
@@ -14,6 +14,10 @@ impl<'c> ClockRunnable<'c> {
         let now = chrono::offset::Local::now();
         now.format(&self.config.format).to_string()
     }
+
+    /// Simple, non-synchronized loop. Just sleeps the configured duration between sending the
+    /// current time to the main module. Because thread sleeping is inaccurrate, this will alias
+    /// sooner or later. Probably sooner.
     fn simple_loop(&self, timeout : std::time::Duration) {
         loop {
             self.to_main.send_update(Ok(self.print_current_time_with_format())).expect("Clock plugin tried to send the current time to the main program, but the main program doesn't listen any more.");
@@ -24,13 +28,25 @@ impl<'c> ClockRunnable<'c> {
         }
     }
 
+    fn fix_rounding_error_if_bad_refresh(fraction_of_thirty_mins : u128) {
+        let now = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).expect("System time before beginning of UNIX epoch?!?");
+        let now_millis = now.as_millis();
+        let now_millis_up = now_millis + 1;
+        let now_fraction_millis = now_millis * fraction_of_thirty_mins;
+        let now_fraction_millis_up = now_millis_up * fraction_of_thirty_mins;
+        if now_fraction_millis_up / 1800000 != now_fraction_millis / 1800000 {
+            std::thread::sleep(std::time::Duration::from_millis(1));
+        }
+    }
+
 
     fn synchronized_loop(&self, fraction_of_thirty_mins : u128) {
+        Self::fix_rounding_error_if_bad_refresh(fraction_of_thirty_mins);
         loop {
              self.to_main.send_update(Ok(self.print_current_time_with_format())).expect("Clock plugin tried to send the current time to the main program, but the main program doesn't listen any more.");
              let now = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).expect("System time before beginning of UNIX epoch?!?");
              
-             let now_millis = now.as_millis() +1; //+1 for rounding up. TODO: Add a conditional millisecond of sleep after startup or refresh to compensate if now_fraction != now_plus_1_fraction
+             let now_millis = now.as_millis() +1; //+1 for rounding up. 
              let now_fraction_millis = now_millis * fraction_of_thirty_mins;
              let now_fraction = now_fraction_millis / (1800000);
              let target_fraction = now_fraction + 1; //Adds one fraction_of_thirty_mins
@@ -39,7 +55,10 @@ impl<'c> ClockRunnable<'c> {
              let timeout_millis = target_millis - now_millis +1; //the 1 from above again, this time to ensure timeout_millis is actually rounded _up_
              let timeout = std::time::Duration::from_millis(timeout_millis as u64);
              match self.from_main.recv_timeout(timeout) {
-                 Ok(MessagesFromMain::Refresh) | Err(RecvTimeoutError::Timeout) => {},
+                 Ok(MessagesFromMain::Refresh) => {
+                     Self::fix_rounding_error_if_bad_refresh(fraction_of_thirty_mins)
+                 },
+                 Err(RecvTimeoutError::Timeout) => {},
                  Ok(MessagesFromMain::Quit) | Err(RecvTimeoutError::Disconnected) => { break; },
              }
         }
-- 
cgit v1.2.3