Battery stats plugin: fix and improvements (#5626)

- fix incorrectly shown awake, sleeping, charging and discharging,
- remove unneeded debug mode and logging to external file,
- prevent showing values like inf, -inf, nan in estimated times
  (we now show "n/a"), and values below zero,
- show extra confirm box when we want to reset data,
- show time in format xxhxxm instead of only pure minutes,
- check at initialization that the device was charging when it was
  turned off (battery level larger than at the time of exit).

plugins/batterystat.koplugin/main.lua

@@ -1,14 +1,12 @@
-
+local ConfirmBox = require("ui/widget/confirmbox")
 local DataStorage = require("datastorage")
 local KeyValuePage = require("ui/widget/keyvaluepage")
 local LuaSettings = require("luasettings")
 local PowerD = require("device"):getPowerDevice()
 local UIManager = require("ui/uimanager")
 local WidgetContainer = require("ui/widget/container/widgetcontainer")
-local T = require("ffi/util").template
 local dbg = require("dbg")
-local logger = require("logger")
+local util = require("util")
-local util = require("ffi/util")
 local _ = require("gettext")
 
 local State = {}
@@ -29,6 +27,7 @@ function State:toString()
 end
 
 local Usage = {}
+local INDENTATION = "   " -- Three spaces.
 
 function Usage:new(o)
     o = o or {}
@@ -43,7 +42,7 @@ end
 
 function Usage:append(state)
     local curr = State:new()
-    self.percentage = self.percentage + (state.percentage - curr.percentage)
+    self.percentage = self.percentage + math.abs(state.percentage - curr.percentage)
     self.time = self.time + os.difftime(curr.timestamp - state.timestamp)
 end
 
@@ -64,59 +63,64 @@ function Usage:percentagePerHour()
 end
 
 function Usage:remainingHours()
+    if self:percentagePerHour() == 0 then return "n/a" end
     local curr = State:new()
     return curr.percentage / self:percentagePerHour()
 end
 
 function Usage:chargingHours()
+    if self:percentagePerHour() == 0 then return "n/a" end
     local curr = State:new()
-    return (curr.percentage - 100) / self:percentagePerHour()
+    return math.abs(curr.percentage - 100) / self:percentagePerHour()
 end
 
 local function shorten(number)
+    if number == "n/a" then return _("n/a") end
     return string.format("%.2f", number);
 end
 
 function Usage:dump(kv_pairs)
-    table.insert(kv_pairs, {_("    Consumed %"), shorten(self.percentage)})
+    table.insert(kv_pairs, {INDENTATION .. _("Consumed %"), shorten(self.percentage)})
-    table.insert(kv_pairs, {_("    Total minutes"), shorten(self:minutes())})
+    table.insert(kv_pairs, {INDENTATION .. _("Total time"), util.secondsToHClock(self.time, true, true)})
-    table.insert(kv_pairs, {_("    % per hour"), shorten(self:percentagePerHour())})
+    table.insert(kv_pairs, {INDENTATION .. _("% per hour"), shorten(self:percentagePerHour())})
 end
 
 function Usage:dumpRemaining(kv_pairs)
-    table.insert(kv_pairs, {_("    Estimated remaining hours"), shorten(self:remainingHours())})
+    table.insert(kv_pairs, {INDENTATION .. _("Estimated remaining hours"), shorten(self:remainingHours())})
 end
 
 function Usage:dumpCharging(kv_pairs)
-    table.insert(kv_pairs, {_("    Estimated hours for charging"), shorten(self:chargingHours())})
+    table.insert(kv_pairs, {INDENTATION .. _("Estimated hours for charging"), shorten(self:chargingHours())})
 end
 
 local BatteryStat = {
-    settings = LuaSettings:open(DataStorage:getSettingsDir() .. "/batterstat.lua"),
+    settings = LuaSettings:open(DataStorage:getSettingsDir() .. "/battery_stats.lua"),
-    dump_file = util.realpath(DataStorage:getDataDir()) .. "/batterystat.log",
-    debugging = false,
     kv_page = nil,
 }
 
 function BatteryStat:init()
-    self.charging = Usage:new(self.settings:readSetting("charging"))
-    self.discharging = Usage:new(self.settings:readSetting("discharging"))
     self.awake = Usage:new(self.settings:readSetting("awake"))
     self.sleeping = Usage:new(self.settings:readSetting("sleeping"))
+    self.charging = Usage:new(self.settings:readSetting("charging"))
+    self.discharging = Usage:new(self.settings:readSetting("discharging"))
 
     -- Note: these fields are not the "real" timestamp and battery usage, but
     -- the unaccumulated values.
-    self.charging_state = State:new(self.settings:readSetting("charging_state"))
+    self.awake_state = State:new()
-    self.awake_state = State:new(self.settings:readSetting("awake_state"))
+    self.charging_state = State:new()
+
     -- Whether the device was suspending before current timestamp.
     self.was_suspending = false
     -- Whether the device was charging before current timestamp.
     self.was_charging = PowerD:isCharging()
-
+    if self.was_charging then
-    if self.debugging then
+        self:reset(true, false)
-        self.debugOutput = self._debugOutput
+    end
-    else
+    -- Check if the battery was charging when KO was turned off.
-        self.debugOutput = function() end
+    local battery_before_off = self.settings:readSetting("awake_state")
+    if battery_before_off and battery_before_off.percentage
+        and self.awake_state.percentage > battery_before_off.percentage then
+        self:reset(false, true)
     end
 end
 
@@ -133,10 +137,10 @@ function BatteryStat:onFlushSettings()
 end
 
 function BatteryStat:accumulate()
-    if self.was_suspending then
+    if self.was_suspending and not self.was_charging then
         -- Suspending to awake.
         self.sleeping:append(self.awake_state)
-    else
+    elseif not self.was_suspending and not self.was_charging then
         -- Awake to suspending, time between self.awake_state and now should belong to awake.
         self.awake:append(self.awake_state)
     end
@@ -150,24 +154,7 @@ function BatteryStat:accumulate()
     self.charging_state = State:new()
 end
 
-function BatteryStat:dumpOrLog(content)
-    local file = io.open(self.dump_file, "a")
-    if file then
-        file:write(content .. "\n")
-        file:close()
-    else
-        logger.warn("Failed to dump output ", content, " into ", self.dump_file )
-    end
-end
-
-function BatteryStat:_debugOutput(event)
-    self:dumpOrLog(event .. " @ " .. State:new():toString() ..
-                   ", awake_state " .. self.awake_state:toString() ..
-                   ", charging_state " .. self.charging_state:toString())
-end
-
 function BatteryStat:onSuspend()
-    self:debugOutput("onSuspend")
     if not self.was_suspending then
         self:accumulate()
     end
@@ -175,7 +162,6 @@ function BatteryStat:onSuspend()
 end
 
 function BatteryStat:onResume()
-    self:debugOutput("onResume")
     if self.was_suspending then
         self:accumulate()
     end
@@ -183,16 +169,14 @@ function BatteryStat:onResume()
 end
 
 function BatteryStat:onCharging()
-    self:debugOutput("onCharging")
     if not self.was_charging then
-        self:reset(true, false)
+        self:reset(true, true)
         self:accumulate()
     end
     self.was_charging = true
 end
 
 function BatteryStat:onNotCharging()
-    self:debugOutput("onNotCharging")
     if self.was_charging then
         self:reset(false, true)
         self:accumulate()
@@ -201,21 +185,33 @@ function BatteryStat:onNotCharging()
 end
 
 function BatteryStat:showStatistics()
+    local function askResetData()
+        UIManager:show(ConfirmBox:new{
+            text = _("Are you sure that you want to clear battery statistics?"),
+            ok_text = _("Clear"),
+            ok_callback = function()
+                self:resetAll()
+                self:restart()
+            end,
+        })
+    end
+
     self:accumulate()
     local kv_pairs = self:dump()
     table.insert(kv_pairs, "----------")
-    table.insert(kv_pairs, {_("Historical records are dumped to"), ""})
-    table.insert(kv_pairs, {self.dump_file, ""})
-    table.insert(kv_pairs, "----------")
     table.insert(kv_pairs, {_("If you would like to reset the data,"), "",
                             callback = function()
-                                self:resetAll()
+                                UIManager:setDirty(self.kv_page, "fast")
-                                self:restart()
+                                UIManager:scheduleIn(0.1, function()
+                                    askResetData()
+                                end)
                             end})
     table.insert(kv_pairs, {_("please tap here."), "",
                             callback = function()
-                                self:resetAll()
+                                UIManager:setDirty(self.kv_page, "fast")
-                                self:restart()
+                                UIManager:scheduleIn(0.1, function()
+                                    askResetData()
+                                end)
                             end})
     self.kv_page = KeyValuePage:new{
         title = _("Battery statistics"),
@@ -225,7 +221,6 @@ function BatteryStat:showStatistics()
 end
 
 function BatteryStat:reset(withCharging, withDischarging)
-    self:dumpToText()
     self.awake = Usage:new()
     self.sleeping = Usage:new()
 
@@ -235,6 +230,7 @@ function BatteryStat:reset(withCharging, withDischarging)
     if withDischarging then
         self.discharging = Usage:new()
     end
+    self.awake_state = State:new()
 end
 
 function BatteryStat:resetAll()
@@ -249,18 +245,6 @@ function BatteryStat:restart()
     self:showStatistics()
 end
 
-function BatteryStat:dumpToText()
-    local kv_pairs = self:dump()
-    local content = T(_("Dump at %1"), os.date("%c"))
-    for _, pair in ipairs(kv_pairs) do
-        content = content .. "\n" .. pair[1]
-        if pair[2] ~= nil and pair[2] ~= "" then
-            content = content .. "\t" .. pair[2]
-        end
-    end
-    self:dumpOrLog(content .. "\n-=-=-=-=-=-\n")
-end
-
 function BatteryStat:dump()
     local kv_pairs = {}
     table.insert(kv_pairs, {_("Awake since last charge"), ""})

spec/unit/batterystat_spec.lua

@@ -40,10 +40,10 @@ describe("BatteryState plugin tests #nocov", function()
         assert.is_false(widget.was_suspending)
         MockTime:increase(1)
         widget:accumulate()
-        -- awake & charging time should be reset.
+        -- Awake charging & discharging time should be reset.
-        assert.are.equal(1, widget.awake.time)
+        assert.are.equal(0, widget.awake.time)
         assert.are.equal(0, widget.sleeping.time)
-        assert.are.equal(1, widget.discharging.time)
+        assert.are.equal(0, widget.discharging.time)
         assert.are.equal(1, widget.charging.time)
 
         widget:onNotCharging()
@@ -62,10 +62,10 @@ describe("BatteryState plugin tests #nocov", function()
         assert.is_false(widget.was_suspending)
         MockTime:increase(1)
         widget:accumulate()
-        -- awake & charging time should be reset.
+        -- Awake charging & discharging time should be reset.
-        assert.are.equal(1, widget.awake.time)
+        assert.are.equal(0, widget.awake.time)
         assert.are.equal(0, widget.sleeping.time)
-        assert.are.equal(1, widget.discharging.time)
+        assert.are.equal(0, widget.discharging.time)
         assert.are.equal(1, widget.charging.time)
     end)
 
@@ -129,10 +129,10 @@ describe("BatteryState plugin tests #nocov", function()
         assert.is_false(widget.was_suspending)
         MockTime:increase(1)
         widget:accumulate()
-        -- awake & charging time should be reset.
+        -- Awake charging & discharging time should be reset.
-        assert.are.equal(1, widget.awake.time)
+        assert.are.equal(0, widget.awake.time)
         assert.are.equal(0, widget.sleeping.time)
-        assert.are.equal(1, widget.discharging.time)
+        assert.are.equal(0, widget.discharging.time)
         assert.are.equal(1, widget.charging.time)
 
         widget:onCharging()
@@ -140,9 +140,9 @@ describe("BatteryState plugin tests #nocov", function()
         assert.is_false(widget.was_suspending)
         MockTime:increase(1)
         widget:accumulate()
-        assert.are.equal(2, widget.awake.time)
+        assert.are.equal(0, widget.awake.time)
         assert.are.equal(0, widget.sleeping.time)
-        assert.are.equal(1, widget.discharging.time)
+        assert.are.equal(0, widget.discharging.time)
         assert.are.equal(2, widget.charging.time)
     end)