[fix, Kobo] Don't crash trying to sync frontlight with Nickel. (#5361)

When reasonably possible, the program should only crash in debug mode. Adds a couple of extra unit tests to prevent regressions and adds docs. Fixes <https://github.com/koreader/koreader/issues/5356>.
5 years ago · 0dad707e2e
parent deaa5d98e6
commit 0dad707e2e
3 changed files with 139 additions and 23 deletions
--- a/frontend/device/kobo/nickel_conf.lua
+++ b/frontend/device/kobo/nickel_conf.lua
@ -1,6 +1,6 @@
--[[
-    Access and modify values in 'Kobo eReader.conf' used by Nickel.
-    Only PowerOptions:FrontLightLevel is currently supported .
+--[[--
+Access and modify values in `Kobo eReader.conf` used by Nickel.
+Only PowerOptions:FrontLightLevel is currently supported.
 ]]

 local dbg = require("dbg")
@ -57,6 +57,11 @@ function NickelConf._read_kobo_conf(re_Match)
    return value
 end

+--[[--
+Get frontlight level.
+
+@treturn int Frontlight level.
+--]]
 function NickelConf.frontLightLevel.get()
    local new_intensity = NickelConf._read_kobo_conf(re_FrontLightLevel)
    if new_intensity then
@ -70,16 +75,29 @@ function NickelConf.frontLightLevel.get()
    end
 end

+--[[--
+Get frontlight state.
+
+This entry will be missing for devices that do not have a hardware toggle button.
+We return nil in this case.
+
+@treturn int Frontlight state (or nil).
+--]]
 function NickelConf.frontLightState.get()
    local new_state = NickelConf._read_kobo_conf(re_FrontLightState)
+
    if new_state then
        new_state = (new_state == "true") or false
    end
-    -- for devices that do not have toggle button, the entry will be missing
-    -- and we return nil in this case.
+
    return new_state
 end

+--[[--
+Get color setting.
+
+@treturn int Color setting.
+--]]
 function NickelConf.colorSetting.get()
    local new_colorsetting = NickelConf._read_kobo_conf(re_ColorSetting)
    if new_colorsetting then
@ -87,6 +105,11 @@ function NickelConf.colorSetting.get()
    end
 end

+--[[--
+Get auto color enabled.
+
+@treturn bool Auto color enabled.
+--]]
 function NickelConf.autoColorEnabled.get()
    local new_autocolor = NickelConf._read_kobo_conf(re_AutoColorEnabled)
    if new_autocolor then
@ -94,6 +117,14 @@ function NickelConf.autoColorEnabled.get()
    end
 end

+--[[--
+Write Kobo configuration.
+
+@string re_Match Lua pattern.
+@string key Kobo conf key.
+@param value
+@bool dontcreate Don't create if key doesn't exist.
+--]]
 function NickelConf._write_kobo_conf(re_Match, key, value, dont_create)
    local kobo_conf = io.open(kobo_conf_path, "r")
    local lines = {}
@ -150,46 +181,76 @@ function NickelConf._write_kobo_conf(re_Match, key, value, dont_create)
    return true
 end

+--[[--
+Set frontlight level.
+
+@int new_intensity
+--]]
 function NickelConf.frontLightLevel.set(new_intensity)
-    assert(new_intensity >= 0 and new_intensity <= 100,
-           "Wrong brightness value given!")
+    if type(new_intensity) ~= "number" or (not (new_intensity >= 0) and (not new_intensity <= 100)) then return end
    return NickelConf._write_kobo_conf(re_FrontLightLevel,
                                       front_light_level_str,
                                       new_intensity)
 end
+dbg:guard(NickelConf.frontLightLevel, "set",
+    function(new_intensity)
+        assert(type(new_intensity) == "number",
+               "Wrong brightness value type (expected number)!")
+        assert(new_intensity >= 0 and new_intensity <= 100,
+               "Wrong brightness value given!")
+    end)
+
+--[[--
+Set frontlight state.

+@bool new_state
+--]]
 function NickelConf.frontLightState.set(new_state)
-    assert(type(new_state) == "boolean",
-           "Wrong front light state value type (expect boolean)!")
+    if new_state == nil or type(new_state) ~= "boolean" then return end
    return NickelConf._write_kobo_conf(re_FrontLightState,
                                       front_light_state_str,
                                       new_state,
-                                       -- do not create this entry is missing
+                                       -- Do not create if this entry is missing.
                                       true)
 end
+dbg:guard(NickelConf.frontLightState, "set",
+    function(new_state)
+        assert(type(new_state) == "boolean",
+            "Wrong front light state value type (expected boolean)!")
+    end)

+--[[--
+Set color setting.
+
+@int new_color >= 1500 and <= 6400
+--]]
 function NickelConf.colorSetting.set(new_color)
    return NickelConf._write_kobo_conf(re_ColorSetting,
                                       color_setting_str,
                                       new_color)
 end
+dbg:guard(NickelConf.colorSetting, "set",
+    function(new_color)
+        assert(type(new_color) == "number",
+            "Wrong color value type (expected number)!")
+        assert(new_color >= 1500 and new_color <= 6400,
+            "Wrong colorSetting value given!")
+    end)
+
+--[[--
+Set auto color enabled.

+@bool new_autocolor
+--]]
 function NickelConf.autoColorEnabled.set(new_autocolor)
    return NickelConf._write_kobo_conf(re_AutoColorEnabled,
                                       auto_color_enabled_str,
                                       new_autocolor)
 end
-
-dbg:guard(NickelConf.colorSetting, 'set',
-          function(new_color)
-              assert(new_color >= 1500 and new_color <= 6400,
-                     "Wrong colorSetting value given!")
-          end)
-
-dbg:guard(NickelConf.autoColorEnabled, 'set',
-          function(new_autocolor)
-              assert(type(new_autocolor) == "boolean",
-                     "Wrong type for autocolor (expected boolean)!")
-          end)
+dbg:guard(NickelConf.autoColorEnabled, "set",
+    function(new_autocolor)
+        assert(type(new_autocolor) == "boolean",
+            "Wrong type for autocolor (expected boolean)!")
+    end)

 return NickelConf
--- a/frontend/device/kobo/powerd.lua
+++ b/frontend/device/kobo/powerd.lua
@ -168,7 +168,7 @@ function KoboPowerD:saveSettings()
        local cur_intensity = self.fl_intensity
        -- If we're shutting down straight from suspend then the frontlight won't
        -- be turned on but we still want to save its state.
-        local cur_is_fl_on = self.is_fl_on or self.fl_was_on
+        local cur_is_fl_on = self.is_fl_on or self.fl_was_on or false
        local cur_warmth = self.fl_warmth
        local cur_auto_warmth = self.auto_warmth
        local cur_max_warmth_hour = self.max_warmth_hour
--- a/spec/unit/nickel_conf_spec.lua
+++ b/spec/unit/nickel_conf_spec.lua
@ -183,5 +183,60 @@ FrontLightLevel=15
                          fd:read("*a"))
            fd:close()
        end)
+
+        it("should not crash on nil values for regular users", function()
+            local fn = os.tmpname()
+            local fd = io.open(fn, "w")
+            fd:write([[
+[PowerOptions]
+foo=bar
+[OtherThing]
+bar=baz
+]])
+            fd:close()
+
+            NickelConf._set_kobo_conf_path(fn)
+            NickelConf.frontLightLevel.set()
+            NickelConf.frontLightState.set()
+
+            fd = io.open(fn, "r")
+            assert.Equals([[
+[PowerOptions]
+foo=bar
+[OtherThing]
+bar=baz
+]], fd:read("*a"))
+            fd:close()
+            os.remove(fn)
+        end)
+
+        it("should crash on nil values in debug mode", function()
+            local dbg = require("dbg")
+            dbg:turnOn()
+            NickelConf = package.reload("device/kobo/nickel_conf")
+            local fn = os.tmpname()
+            local fd = io.open(fn, "w")
+            fd:write([[
+[PowerOptions]
+foo=bar
+[OtherThing]
+bar=baz
+]])
+            fd:close()
+
+            NickelConf._set_kobo_conf_path(fn)
+            assert.has_error(function() NickelConf.frontLightLevel.set() end)
+            assert.has_error(function() NickelConf.frontLightState.set() end)
+
+            fd = io.open(fn, "r")
+            assert.Equals([[
+[PowerOptions]
+foo=bar
+[OtherThing]
+bar=baz
+]], fd:read("*a"))
+            fd:close()
+            os.remove(fn)
+        end)
    end)
 end)