Merge pull request #1939 from koreader/houqp-master

kobo: fix screen probe for touch

.ci/script.sh

@@ -8,4 +8,4 @@ make all
 retry_cmd 6 make testfront
 set +o pipefail
 luajit $(which luacheck) --no-color -q frontend | tee ./luacheck.out
-test $(grep Total ./luacheck.out | awk '{print $2}') -le 59
+test $(grep Total ./luacheck.out | awk '{print $2}') -le 54

frontend/apps/reader/modules/readerbookmark.lua

@@ -244,7 +244,8 @@ function ReaderBookmark:isBookmarkMatch(item, pn_or_xp)
 end
 
 function ReaderBookmark:getDogearBookmarkIndex(pn_or_xp)
-    local _start, _middle, _end = 1, 1, #self.bookmarks
+    local _middle
+    local _start, _end = 1, #self.bookmarks
     while _start <= _end do
         _middle = math.floor((_start + _end)/2)
         local v = self.bookmarks[_middle]
@@ -295,7 +296,8 @@ end
 
 -- binary search of sorted bookmarks
 function ReaderBookmark:isBookmarkAdded(item)
-    local _start, _middle, _end = 1, 1, #self.bookmarks
+    local _middle
+    local _start, _end = 1, #self.bookmarks
     while _start <= _end do
         _middle = math.floor((_start + _end)/2)
         if self:isBookmarkSame(item, self.bookmarks[_middle]) then
@@ -312,7 +314,8 @@ end
 
 -- binary search to remove bookmark
 function ReaderBookmark:removeBookmark(item)
-    local _start, _middle, _end = 1, 1, #self.bookmarks
+    local _middle
+    local _start, _end = 1, #self.bookmarks
     while _start <= _end do
         _middle = math.floor((_start + _end)/2)
         local v = self.bookmarks[_middle]

frontend/device/android/device.lua

@@ -21,14 +21,14 @@ function Device:init()
     self.input = require("device/input"):new{
         device = self,
         event_map = require("device/android/event_map"),
-        handleMiscEv = function(self, ev)
+        handleMiscEv = function(this, ev)
             DEBUG("Android application event", ev.code)
             if ev.code == ffi.C.APP_CMD_SAVE_STATE then
                 return "SaveState"
             elseif ev.code == ffi.C.APP_CMD_GAINED_FOCUS then
-                self.device.screen:refreshFull()
+                this.device.screen:refreshFull()
             elseif ev.code == ffi.C.APP_CMD_WINDOW_REDRAW_NEEDED then
-                self.device.screen:refreshFull()
+                this.device.screen:refreshFull()
             end
         end,
     }

frontend/device/generic/device.lua

@@ -20,6 +20,7 @@ local Device = {
     hasDPad = no,
     isTouchDevice = no,
     hasFrontlight = no,
+    needsTouchScreenProbe = no,
 
     -- use these only as a last resort. We should abstract the functionality
     -- and have device dependent implementations in the corresponting

frontend/device/input.lua

@@ -153,14 +153,16 @@ and register them.
 --]]
 function Input:registerEventAdjustHook(hook, hook_params)
     local old = self.eventAdjustHook
-    self.eventAdjustHook = function(self, ev)
-        old(self, ev)
-        hook(self, ev, hook_params)
+    self.eventAdjustHook = function(this, ev)
+        old(this, ev)
+        hook(this, ev, hook_params)
     end
 end
+
 function Input:eventAdjustHook(ev)
     -- do nothing by default
 end
+
 -- catalogue of predefined hooks:
 function Input:adjustTouchSwitchXY(ev)
     if ev.type == EV_ABS then
@@ -175,6 +177,7 @@ function Input:adjustTouchSwitchXY(ev)
         end
     end
 end
+
 function Input:adjustTouchScale(ev, by)
     if ev.type == EV_ABS then
         if ev.code == ABS_X or ev.code == ABS_MT_POSITION_X then
@@ -185,18 +188,21 @@ function Input:adjustTouchScale(ev, by)
         end
     end
 end
+
 function Input:adjustTouchMirrorX(ev, width)
     if ev.type == EV_ABS
     and (ev.code == ABS_X or ev.code == ABS_MT_POSITION_X) then
         ev.value = width - ev.value
     end
 end
+
 function Input:adjustTouchMirrorY(ev, height)
     if ev.type == EV_ABS
     and (ev.code == ABS_Y or ev.code == ABS_MT_POSITION_Y) then
         ev.value = height - ev.value
     end
 end
+
 function Input:adjustTouchTranslate(ev, by)
     if ev.type == EV_ABS then
         if ev.code == ABS_X or ev.code == ABS_MT_POSITION_X then
@@ -207,6 +213,7 @@ function Input:adjustTouchTranslate(ev, by)
         end
     end
 end
+
 function Input:adjustTouchAlyssum(ev)
     ev.time = TimeVal:now()
     if ev.type == EV_ABS and ev.code == ABS_MT_TRACKING_ID then

frontend/device/kobo/device.lua

@@ -1,6 +1,6 @@
 local Generic = require("device/generic/device")
 local Geom = require("ui/geometry")
-local DEBUG = require("dbg")
+local dbg = require("dbg")
 
 local function yes() return true end
 
@@ -22,6 +22,7 @@ local Kobo = Generic:new{
 -- Kobo Touch:
 local KoboTrilogy = Kobo:new{
     model = "Kobo_trilogy",
+    needsTouchScreenProbe = yes,
     touch_switch_xy = false,
     hasKeys = yes,
 }
@@ -78,7 +79,7 @@ local KoboAlyssum = Kobo:new{
 }
 
 function Kobo:init()
-    self.screen = require("ffi/framebuffer_mxcfb"):new{device = self, debug = DEBUG}
+    self.screen = require("ffi/framebuffer_mxcfb"):new{device = self, debug = dbg}
     self.powerd = require("device/kobo/powerd"):new{device = self}
     self.input = require("device/input"):new{
         device = self,
@@ -90,10 +91,43 @@ function Kobo:init()
         }
     }
 
+    Generic.init(self)
+
+    self.input.open("/dev/input/event0") -- Light button and sleep slider
+    self.input.open("/dev/input/event1")
+
+    if not self.needsTouchScreenProbe() then
+        self:initEventAdjustHooks()
+    else
+        -- if touch probe is required, we postpone EventAdjustHook
+        -- initialization to when self:touchScreenProbe is called
+        self.touchScreenProbe = function()
+            -- if user has not set KOBO_TOUCH_MIRRORED yet
+            if KOBO_TOUCH_MIRRORED == nil then
+                local switch_xy = G_reader_settings:readSetting("kobo_touch_switch_xy")
+                -- and has no probe before
+                if switch_xy == nil then
+                    local TouchProbe = require("utils/kobo_touch_probe")
+                    local UIManager = require("ui/uimanager")
+                    UIManager:show(TouchProbe:new{})
+                    UIManager:run()
+                    -- assuming TouchProbe sets kobo_touch_switch_xy config
+                    switch_xy = G_reader_settings:readSetting("kobo_touch_switch_xy")
+                end
+                self.touch_switch_xy = switch_xy
+            end
+            self:initEventAdjustHooks()
+        end
+    end
+end
+
+function Kobo:initEventAdjustHooks()
     -- it's called KOBO_TOUCH_MIRRORED in defaults.lua, but what it
     -- actually did in its original implementation was to switch X/Y.
-    if self.touch_switch_xy and not KOBO_TOUCH_MIRRORED
-    or not self.touch_switch_xy and KOBO_TOUCH_MIRRORED
+    -- NOTE: for kobo touch, adjustTouchSwitchXY needs to be called before
+    -- adjustTouchMirrorX
+    if (self.touch_switch_xy and not KOBO_TOUCH_MIRRORED)
+            or (not self.touch_switch_xy and KOBO_TOUCH_MIRRORED)
     then
         self.input:registerEventAdjustHook(self.input.adjustTouchSwitchXY)
     end
@@ -101,7 +135,8 @@ function Kobo:init()
     if self.touch_mirrored_x then
         self.input:registerEventAdjustHook(
             self.input.adjustTouchMirrorX,
-            self.screen:getScreenWidth()
+            -- FIXME: what if we change the screen protrait mode?
+            self.screen:getWidth()
         )
     end
 
@@ -112,11 +147,6 @@ function Kobo:init()
     if self.touch_phoenix_protocol then
         self.input.handleTouchEv = self.input.handleTouchEvPhoenix
     end
-
-    Generic.init(self)
-
-    self.input.open("/dev/input/event0") -- Light button and sleep slider
-    self.input.open("/dev/input/event1")
 end
 
 function Kobo:getCodeName()

reader.lua

@@ -137,9 +137,10 @@ if Device:isKobo() then
             end
         end
     end
-    if Device:getCodeName() == "trilogy" then
-        require("utils/kobo_touch_probe")
-    end
+end
+
+if Device:needsTouchScreenProbe() then
+    Device:touchScreenProbe()
 end
 
 if ARGV[argidx] and ARGV[argidx] ~= "" then

spec/unit/device_spec.lua

@@ -0,0 +1,98 @@
+describe("device module", function()
+    local mock_fb, mock_input
+    setup(function()
+        mock_fb = {
+            new = function()
+                return {
+                    getSize = function() return {w = 600, h = 800} end,
+                    getWidth = function() return 600 end,
+                    getDPI = function() return 72 end,
+                    setViewport = function() end
+                }
+            end
+        }
+        require("commonrequire")
+    end)
+
+    describe("kobo", function()
+        setup(function()
+            mock_fb = {
+                new = function()
+                    return {
+                        getSize = function() return {w = 600, h = 800} end,
+                        getWidth = function() return 600 end,
+                        getDPI = function() return 72 end,
+                        setViewport = function() end
+                    }
+                end
+            }
+        end)
+
+        it("should initialize properly on Kobo dahlia", function()
+            package.loaded['ffi/framebuffer_mxcfb'] = mock_fb
+            stub(os, "getenv")
+            os.getenv.returns("dahlia")
+            local kobo_dev = require("device/kobo/device")
+
+            mock_input = require('device/input')
+            stub(mock_input, "open")
+            kobo_dev:init()
+            assert.is.same("Kobo_dahlia", kobo_dev.model)
+
+            package.loaded['ffi/framebuffer_mxcfb'] = nil
+            os.getenv:revert()
+            mock_input.open:revert()
+        end)
+
+        it("should setup eventAdjustHooks properly for input in trilogy", function()
+            local saved_getenv = os.getenv
+            stub(os, "getenv")
+            os.getenv.invokes(function(key)
+                if key == "PRODUCT" then
+                    return "trilogy"
+                else
+                    return saved_getenv(key)
+                end
+            end)
+            package.loaded['device/kobo/device'] = nil
+            package.loaded['ffi/framebuffer_mxcfb'] = mock_fb
+            mock_input = require('device/input')
+            stub(mock_input, "open")
+
+            local kobo_dev = require("device/kobo/device")
+            kobo_dev:init()
+            local Screen = kobo_dev.screen
+
+            assert.is.same("Kobo_trilogy", kobo_dev.model)
+            assert.truthy(kobo_dev:needsTouchScreenProbe())
+            G_reader_settings:saveSetting("kobo_touch_switch_xy", true)
+            kobo_dev:touchScreenProbe()
+            local x, y = Screen:getWidth()-5, 10
+            local EV_ABS = 3
+            local ABS_X = 00
+            local ABS_Y = 01
+            -- mirror x, then switch_xy
+            local ev_x = {
+                type = EV_ABS,
+                code = ABS_X,
+                value = y,
+            }
+            local ev_y = {
+                type = EV_ABS,
+                code = ABS_Y,
+                value = Screen:getWidth()-x,
+            }
+
+            kobo_dev.input:eventAdjustHook(ev_x)
+            kobo_dev.input:eventAdjustHook(ev_y)
+            assert.is.same(x, ev_y.value)
+            assert.is.same(ABS_X, ev_y.code)
+            assert.is.same(y, ev_x.value)
+            assert.is.same(ABS_Y, ev_x.code)
+
+            package.loaded['ffi/framebuffer_mxcfb'] = nil
+            os.getenv:revert()
+            mock_input.open:revert()
+        end)
+    end)
+end)

spec/unit/touch_probe_spec.lua

@@ -0,0 +1,62 @@
+describe("touch probe module", function()
+    setup(function()
+        require("commonrequire")
+    end)
+
+    it("should probe properly for kobo touch", function()
+        local Device = require("device")
+        local TouchProbe = require("utils/kobo_touch_probe"):new{}
+        local need_to_switch_xy
+        TouchProbe.saveSwitchXYSetting = function(_, new_need_to_switch_xy)
+            need_to_switch_xy = new_need_to_switch_xy
+        end
+        -- for kobo touch, we have mirror_x, then switch_xy
+        -- tap lower right corner
+        local x, y = Device.screen:getWidth()-40, Device.screen:getHeight()-40
+        need_to_switch_xy = nil
+        TouchProbe:onTapProbe(nil, {
+            pos = {
+                x = y,
+                y = Device.screen:getWidth()-x,
+            }
+        })
+        assert.is.same(TouchProbe.curr_probe_step, 1)
+        assert.truthy(need_to_switch_xy)
+
+        -- now only test mirror_x
+        -- tap lower right corner
+        local x, y = Device.screen:getWidth()-40, Device.screen:getHeight()-40
+        need_to_switch_xy = nil
+        TouchProbe:onTapProbe(nil, {
+            pos = {
+                x = Device.screen:getWidth()-x,
+                y = y,
+            }
+        })
+        assert.is.same(TouchProbe.curr_probe_step, 1)
+        assert.falsy(need_to_switch_xy)
+
+        -- now only test switch_xy
+        -- tap lower right corner
+        local x, y = Device.screen:getWidth()-40, Device.screen:getHeight()-40
+        need_to_switch_xy = nil
+        TouchProbe:onTapProbe(nil, {
+            pos = {
+                x = y,
+                y = x,
+            }
+        })
+        assert.is.same(TouchProbe.curr_probe_step, 2)
+        assert.falsy(need_to_switch_xy)
+        -- tap upper right corner
+        local x, y = Device.screen:getWidth()-40, 40
+        TouchProbe:onTapProbe(nil, {
+            pos = {
+                x = y,
+                y = x,
+            }
+        })
+        assert.is.same(TouchProbe.curr_probe_step, 2)
+        assert.truthy(need_to_switch_xy)
+    end)
+end)

spec/unit/util_spec.lua

@@ -1,8 +1,10 @@
-require("commonrequire")
-
-local util = require("util")
-
 describe("util module", function()
+    local util
+    setup(function()
+        require("commonrequire")
+        util = require("util")
+    end)
+
     it("should strip punctuations around word", function()
         assert.is_equal(util.stripePunctuations("\"hello world\""), "hello world")
         assert.is_equal(util.stripePunctuations("\"hello world?\""), "hello world")
@@ -10,6 +12,7 @@ describe("util module", function()
         assert.is_equal(util.stripePunctuations("“你好“"), "你好")
         assert.is_equal(util.stripePunctuations("“你好?“"), "你好")
     end)
+
     it("should split string with patterns", function()
         local sentence = "Hello world, welcome to KoReader!"
         local words = {}
@@ -18,6 +21,7 @@ describe("util module", function()
         end
         assert.are_same(words, {"Hello", "world,", "welcome", "to", "KoReader!"})
     end)
+
     it("should split command line arguments with quotation", function()
         local command = "./sdcv -nj \"words\" \"a lot\" 'more or less' --data-dir=dict"
         local argv = {}

utils/kobo_touch_probe.lua

@@ -11,13 +11,16 @@ local _ = require("gettext")
 -- read settings and check for language override
 -- has to be done before requiring other files because
 -- they might call gettext on load
-G_reader_settings = DocSettings:open(".reader")
+if G_reader_settings == nil then
+    G_reader_settings = DocSettings:open(".reader")
+end
 local lang_locale = G_reader_settings:readSetting("language")
 if lang_locale then
     _.changeLang(lang_locale)
 end
 local InputContainer = require("ui/widget/container/inputcontainer")
 local CenterContainer = require("ui/widget/container/centercontainer")
+local FrameContainer = require("ui/widget/container/framecontainer")
 local RightContainer = require("ui/widget/container/rightcontainer")
 local OverlapGroup = require("ui/widget/overlapgroup")
 local ImageWidget = require("ui/widget/imagewidget")
@@ -27,13 +30,14 @@ local UIManager = require("ui/uimanager")
 local Blitbuffer = require("ffi/blitbuffer")
 local Geom = require("ui/geometry")
 local Device = require("device")
-local Screen = require("device").screen
-local Input = require("device").input
+local Screen = Device.screen
+local Input = Device.input
 local Font = require("ui/font")
-local DEBUG = require("dbg")
---DEBUG:turnOn()
+local dbg = require("dbg")
+--dbg:turnOn()
 
 local TouchProbe = InputContainer:new{
+    curr_probe_step = 1,
 }
 
 function TouchProbe:init()
@@ -49,49 +53,82 @@ function TouchProbe:init()
             }
         },
     }
-    local image_widget = ImageWidget:new{
+    self.image_widget = ImageWidget:new{
         file = "resources/kobo-touch-probe.png",
     }
-    self[1] = OverlapGroup:new{
-        dimen = Screen:getSize(),
-        CenterContainer:new{
-            dimen = Screen:getSize(),
-            TextWidget:new{
-                text = _("Tap the upper right corner"),
-                face = Font:getFace("cfont", 30),
-            },
+    local screen_w, screen_h = Screen:getWidth(), Screen:getHeight()
+    local img_w, img_h = self.image_widget:getSize().w, self.image_widget:getSize().h
+    self.probe_steps = {
+        {
+            hint_text = _("Tap the lower right corner"),
+            hint_icon_pos = {
+                x = screen_w-img_w,
+                y = screen_h-img_h,
+            }
         },
-        RightContainer:new{
-            dimen = {
-                h = image_widget:getSize().h,
-                w = Screen:getSize().w,
-            },
-            image_widget,
+        {
+            hint_text = _("Tap the upper right corner"),
+            hint_icon_pos = {
+                x = screen_w-img_w,
+                y = 0,
+            }
         },
     }
+    self.hint_text_widget = TextWidget:new{
+        text = '',
+        face = Font:getFace("cfont", 30),
+    }
+    self[1] = FrameContainer:new{
+        bordersize = 0,
+        background = Blitbuffer.COLOR_WHITE,
+        OverlapGroup:new{
+            dimen = Screen:getSize(),
+            CenterContainer:new{
+                dimen = Screen:getSize(),
+                self.hint_text_widget,
+            },
+            self.image_widget,
+        },
+    }
+    self:updateProbeInstruction()
 end
 
-function TouchProbe:onTapProbe(arg, ges)
-    --DEBUG("onTapProbe", ges)
-    local need_to_switch_xy = ges.pos.x < ges.pos.y
-    --DEBUG("Need to switch xy", need_to_switch_xy)
+function TouchProbe:updateProbeInstruction()
+    local probe_step = self.probe_steps[self.curr_probe_step]
+    self.image_widget.overlap_offset = {
+        probe_step.hint_icon_pos.x,
+        probe_step.hint_icon_pos.y,
+    }
+    self.hint_text_widget:setText(probe_step.hint_text)
+end
+
+function TouchProbe:saveSwitchXYSetting(need_to_switch_xy)
+    -- save the settings here so device.input can pick it up
     G_reader_settings:saveSetting("kobo_touch_switch_xy", need_to_switch_xy)
-    G_reader_settings:close()
-    if need_to_switch_xy then
-        Input:registerEventAdjustHook(Input.adjustTouchSwitchXY)
-    end
+    G_reader_settings:flush()
     UIManager:quit()
 end
 
--- if user has not set KOBO_TOUCH_MIRRORED yet
-if KOBO_TOUCH_MIRRORED == nil then
-    local switch_xy = G_reader_settings:readSetting("kobo_touch_switch_xy")
-    -- and has no probe before
-    if switch_xy == nil then
-        UIManager:show(TouchProbe:new{})
-        UIManager:run()
-    -- otherwise, we will use probed result
-    elseif switch_xy then
-        Input:registerEventAdjustHook(Input.adjustTouchSwitchXY)
+function TouchProbe:onTapProbe(arg, ges)
+    if self.curr_probe_step == 1 then
+        local shorter_edge = math.min(Screen:getHeight(), Screen:getWidth())
+        if math.min(ges.pos.x, ges.pos.y) < shorter_edge/2 then
+            -- x mirrored, x should be close to zero and y should be close to
+            -- screen height
+            local need_to_switch_xy = ges.pos.x > ges.pos.y
+            self:saveSwitchXYSetting(need_to_switch_xy)
+        else
+            -- x not mirroed, need one more probe
+            self.curr_probe_step = 2
+            self:updateProbeInstruction()
+            UIManager:setDirty(self)
+        end
+    elseif self.curr_probe_step == 2 then
+        -- x not mirrored, y should be close to zero and x should be close
+        -- TouchProbe screen width
+        local need_to_switch_xy = ges.pos.x < ges.pos.y
+        self:saveSwitchXYSetting(need_to_switch_xy)
     end
 end
+
+return TouchProbe

utils/wbuilder.lua

@@ -392,6 +392,11 @@ function testBookStatus()
     UIManager:show(status_page)
 end
 
+function testTouchProbe()
+    local TouchProbe = require("utils/kobo_touch_probe")
+    UIManager:show(TouchProbe:new{})
+end
+
 -----------------------------------------------------------------------
 -- you may want to uncomment following show calls to see the changes
 -----------------------------------------------------------------------
@@ -407,6 +412,6 @@ UIManager:show(Clock:new())
 --UIManager:show(TestInputText)
 --TestInputText:onShowKeyboard()
 -- testKeyValuePage()
+-- testTouchProbe()
 testBookStatus()
-
 UIManager:run()