ReadTimer: More QoL tweaks (#8107)

* Cleanup util.secondsFrom*Clock stuff (simpler maths, tail calls, meaningful printf tokens).
* Use util.secondsToClockDuration in ReadTimer instead of reinventing the wheel three different ways.
* Reschedule unexpired timers properly on resume (as best as we can, given the unreliable nature of REALTIME).
* Make clock timers tick on the dot, instead of at the same second as when being set.
* Speaking of clock timers, leave the math to os.date & os.time, don't reinvent the wheel yet again.

frontend/util.lua

@@ -98,6 +98,11 @@ function util.gsplit(str, pattern, capture, capture_empty_entity)
     end)
 end
 
+-- Stupid helper for the duration stuff
+local function passthrough(n)
+    return n
+end
+
 --[[--
 Converts seconds to a clock string.
 
@@ -115,18 +120,20 @@ function util.secondsToClock(seconds, withoutSeconds)
             return "00:00:00"
         end
     else
-        local round = withoutSeconds and require("optmath").round or math.floor
-        local hours = string.format("%02.f", math.floor(seconds / 3600))
-        local mins = string.format("%02.f", round(seconds / 60 - (hours * 60)))
-        if mins == "60" then
-            mins = string.format("%02.f", 0)
-            hours = string.format("%02.f", hours + 1)
-        end
+        local round = withoutSeconds and require("optmath").round or passthrough
+        local hours = string.format("%02d", seconds / 3600)
+        local mins = string.format("%02d", round(seconds % 3600 / 60))
         if withoutSeconds then
+            if mins == "60" then
+                -- Can only happen because of rounding, which only happens if withoutSeconds...
+                mins = string.format("%02d", 0)
+                hours = string.format("%02d", hours + 1)
+            end
             return hours .. ":" .. mins
+        else
+            local secs = string.format("%02d", seconds % 60)
+            return hours .. ":" .. mins .. ":" .. secs
         end
-        local secs = string.format("%02.f", math.floor(seconds - hours * 3600 - mins * 60))
-        return hours .. ":" .. mins .. ":" .. secs
     end
 end
 
@@ -166,22 +173,23 @@ function util.secondsToHClock(seconds, withoutSeconds, hmsFormat)
             end
         else
             if hmsFormat then
-                return T(_("%1m%2s"), "0", string.format("%02.f", math.floor(seconds)))
+                return T(_("%1m%2s"), "0", string.format("%02d", seconds))
             else
-                return "0'" .. string.format("%02.f", seconds) .. "''"
+                return "0'" .. string.format("%02d", seconds) .. "''"
             end
         end
     else
-        local round = withoutSeconds and require("optmath").round or math.floor
-        local hours = string.format("%.f", math.floor(seconds / 3600))
-        local mins = string.format("%02.f", round(seconds / 60 - (hours * 60)))
-        if mins == "60" then
-            mins = string.format("%02.f", 0)
-            hours = string.format("%.f", hours + 1)
-        end
+        local round = withoutSeconds and require("optmath").round or passthrough
+        local hours = string.format("%d", seconds / 3600)
+        local mins = string.format("%02d", round(seconds % 3600 / 60))
         if withoutSeconds then
+            if mins == "60" then
+                mins = string.format("%02d", 0)
+                hours = string.format("%d", hours + 1)
+            end
             if hours == "0" then
-                mins = string.format("%.f", round(seconds / 60))
+                -- We can optimize out the % 3600 since the branch ensures we're < than 3600
+                mins = string.format("%d", round(seconds / 60))
                 if hmsFormat then
                     return T(_("%1m"), mins)
                 else
@@ -190,31 +198,32 @@ function util.secondsToHClock(seconds, withoutSeconds, hmsFormat)
             end
             -- @translators This is the 'h' for hour, like in 1h30. This is a duration.
             return T(_("%1h%2"), hours, mins)
-        end
-        local secs = string.format("%02.f", math.floor(seconds - hours * 3600 - mins * 60))
-        if hours == "0" then
-            mins = string.format("%.f", round(seconds / 60))
-            if hmsFormat then
-                -- @translators This is the 'm' for minute and the 's' for second, like in 1m30s. This is a duration.
-                return T(_("%1m%2s"), mins, secs)
-            else
-                return mins .. "'" .. secs .. "''"
-            end
-        end
-        if hmsFormat then
-            if secs == "00" then
-                -- @translators This is the 'h' for hour and the 'm' for minute, like in 1h30m. This is a duration.
-                return T(_("%1h%2m"), hours, mins)
-            else
-                -- @translators This is the 'h' for hour, the 'm' for minute and the 's' for second, like in 1h30m30s. This is a duration.
-                return T(_("%1h%2m%3s"), hours, mins, secs)
+        else
+            local secs = string.format("%02d", seconds % 60)
+            if hours == "0" then
+                mins = string.format("%d", round(seconds / 60))
+                if hmsFormat then
+                    -- @translators This is the 'm' for minute and the 's' for second, like in 1m30s. This is a duration.
+                    return T(_("%1m%2s"), mins, secs)
+                else
+                    return mins .. "'" .. secs .. "''"
+                end
             end
+            if hmsFormat then
+                if secs == "00" then
+                    -- @translators This is the 'h' for hour and the 'm' for minute, like in 1h30m. This is a duration.
+                    return T(_("%1h%2m"), hours, mins)
+                else
+                    -- @translators This is the 'h' for hour, the 'm' for minute and the 's' for second, like in 1h30m30s. This is a duration.
+                    return T(_("%1h%2m%3s"), hours, mins, secs)
+                end
 
-        else
-            if secs == "00" then
-                return T(_("%1h%2'"), hours, mins)
             else
-                return T(_("%1h%2'%3''"), hours, mins, secs)
+                if secs == "00" then
+                    return T(_("%1h%2'"), hours, mins)
+                else
+                    return T(_("%1h%2'%3''"), hours, mins, secs)
+                end
             end
         end
     end

plugins/coverbrowser.koplugin/listmenu.lua

@@ -433,7 +433,7 @@ function ListMenuItem:update()
                         pages_str = T(_("%1, %2 to read"), pages_str, Math.round(pages-percent_finished*pages), pages)
                     end
                 else
-                    pages_str = string.format("%d %%", math.floor(100*percent_finished))
+                    pages_str = string.format("%d %%", 100*percent_finished)
                 end
             else
                 if pages then

plugins/readtimer.koplugin/main.lua

@@ -2,8 +2,9 @@ local InfoMessage = require("ui/widget/infomessage")
 local TimeWidget = require("ui/widget/timewidget")
 local UIManager = require("ui/uimanager")
 local WidgetContainer = require("ui/widget/container/widgetcontainer")
+local logger = require("logger")
+local util = require("util")
 local _ = require("gettext")
-local N_ = _.ngettext
 local T = require("ffi/util").template
 
 local ReadTimer = WidgetContainer:new{
@@ -28,9 +29,9 @@ function ReadTimer:scheduled()
     return self.time ~= 0
 end
 
-function ReadTimer:remainingMinutes()
+function ReadTimer:remaining()
     if self:scheduled() then
-        local td = os.difftime(self.time, os.time()) / 60
+        local td = os.difftime(self.time, os.time())
         if td > 0 then
             return td
         else
@@ -43,10 +44,11 @@ end
 
 function ReadTimer:remainingTime()
     if self:scheduled() then
-        local remain_time = self:remainingMinutes()
-        local remain_hours = math.floor(remain_time / 60)
-        local remain_minutes = math.floor(remain_time - 60 * remain_hours)
-        return remain_hours, remain_minutes
+        local remainder = self:remaining()
+        local hours = math.floor(remainder / 3600)
+        local minutes = math.floor(remainder % 3600 / 60)
+        local seconds = math.floor(remainder % 60)
+        return hours, minutes, seconds
     end
 end
 
@@ -57,12 +59,18 @@ function ReadTimer:unschedule()
     end
 end
 
+function ReadTimer:rescheduleIn(seconds)
+    self.time = os.time() + seconds
+    UIManager:scheduleIn(seconds, self.alarm_callback)
+end
+
 function ReadTimer:addToMainMenu(menu_items)
     menu_items.read_timer = {
         text_func = function()
             if self:scheduled() then
-                return T(_("Read timer (%1m)"),
-                    string.format("%.2f", self:remainingMinutes()))
+                local user_duration_format = G_reader_settings:readSetting("duration_format")
+                return T(_("Read timer (%1)"),
+                    util.secondsToClockDuration(user_duration_format, self:remaining(), false))
             else
                 return _("Read timer")
             end
@@ -78,7 +86,6 @@ function ReadTimer:addToMainMenu(menu_items)
                     local now_t = os.date("*t")
                     local curr_hour = now_t.hour
                     local curr_min = now_t.min
-                    local curr_sec_from_midnight = curr_hour*3600 + curr_min*60
                     local time_widget = TimeWidget:new{
                         hour = curr_hour,
                         min = curr_min,
@@ -87,38 +94,24 @@ function ReadTimer:addToMainMenu(menu_items)
                         callback = function(time)
                             touchmenu_instance:closeMenu()
                             self:unschedule()
-                            local timer_sec_from_mignight = time.hour*3600 + time.min*60
-                            local seconds
-                            if timer_sec_from_mignight > curr_sec_from_midnight then
-                                seconds = timer_sec_from_mignight - curr_sec_from_midnight
-                            else
-                                seconds = 24*3600 - (curr_sec_from_midnight - timer_sec_from_mignight)
-                            end
-                            if seconds > 0 and seconds < 18*3600 then
-                                self.time = os.time() + seconds
-                                UIManager:scheduleIn(seconds, self.alarm_callback)
-                                local hr_str = ""
-                                local min_str = ""
-                                local hr = math.floor(seconds/3600)
-                                if hr > 0 then
-                                    hr_str = T(N_("1 hour", "%1 hours", hr), hr)
-                                end
-                                local min = math.floor((seconds%3600)/60)
-                                if min > 0 then
-                                    min_str = T(N_("1 minute", "%1 minutes", min), min)
-                                    if hr_str ~= "" then
-                                        hr_str = hr_str .. " "
-                                    end
-                                end
+                            local then_t = now_t
+                            then_t.hour = time.hour
+                            then_t.min = time.min
+                            then_t.sec = 0
+                            local seconds = os.difftime(os.time(then_t), os.time())
+                            if seconds > 0 then
+                                self:rescheduleIn(seconds)
+                                local user_duration_format = G_reader_settings:readSetting("duration_format")
                                 UIManager:show(InfoMessage:new{
-                                    text = T(_("Timer set to: %1:%2.\n\nThat's %3%4 from now."),
+                                    -- @translators %1:%2 is a clock time (HH:MM), %3 is a duration
+                                    text = T(_("Timer set for %1:%2.\n\nThat's %3 from now."),
                                         string.format("%02d", time.hour), string.format("%02d", time.min),
-                                        hr_str, min_str),
+                                        util.secondsToClockDuration(user_duration_format, seconds, false)),
                                     timeout = 5,
                                 })
-                            elseif seconds <= 0 or seconds >= 18*3600 then
+                            else
                                 UIManager:show(InfoMessage:new{
-                                    text = _("Timer could not be set. The selected time is in the past or too far in the future."),
+                                    text = _("Timer could not be set. The selected time is in the past."),
                                     timeout = 5,
                                 })
                             end
@@ -151,26 +144,15 @@ function ReadTimer:addToMainMenu(menu_items)
                             self:unschedule()
                             local seconds = time.hour * 3600 + time.min * 60
                             if seconds > 0 then
-                                self.time = os.time() + seconds
-                                UIManager:scheduleIn(seconds, self.alarm_callback)
-                                local hr_str = ""
-                                local min_str = ""
-                                local hr = time.hour
-                                if hr > 0 then
-                                    hr_str = T(N_("1 hour", "%1 hours", hr), hr)
-                                end
-                                local min = time.min
-                                if min > 0 then
-                                    min_str = T(N_("1 minute", "%1 minutes", min), min)
-                                    if hr_str ~= "" then
-                                        hr_str = hr_str .. " "
-                                    end
-                                end
+                                self:rescheduleIn(seconds)
+                                local user_duration_format = G_reader_settings:readSetting("duration_format")
                                 UIManager:show(InfoMessage:new{
-                                    text = T(_("Timer set for %1%2."), hr_str, min_str),
+                                    -- @translators This is a duration
+                                    text = T(_("Timer will expire in %1."),
+                                             util.secondsToClockDuration(user_duration_format, seconds, true)),
                                     timeout = 5,
                                 })
-                                remain_time = {hr, min}
+                                remain_time = {time.hour, time.min}
                                 G_reader_settings:saveSetting("reader_timer_remain_time", remain_time)
                             end
                         end
@@ -193,12 +175,23 @@ function ReadTimer:addToMainMenu(menu_items)
     }
 end
 
--- The UI ticks on a monotonic time domain, while this plugin deals with real time.
--- Make sure we fire the alarm right away if it expired during suspend...
+-- The UI ticks on a MONOTONIC time domain, while this plugin deals with REAL wall clock time.
 function ReadTimer:onResume()
-    if self:remainingMinutes() == 0 then
-        self:alarm_callback()
-        self:unschedule()
+    if self:scheduled() then
+        logger.dbg("ReadTimer: onResume with an active timer")
+        local remainder = self:remaining()
+
+        if remainder == 0 then
+            -- Make sure we fire the alarm right away if it expired during suspend...
+            self:alarm_callback()
+            self:unschedule()
+        else
+            -- ...and that we re-schedule the timer against the REAL time if it's still ticking.
+            logger.dbg("ReadTimer: Rescheduling in", remainder, "seconds")
+            self:unschedule()
+            self:rescheduleIn(remainder)
+        end
+
     end
 end