Input: Process input events in batches (#7483)

Requires https://github.com/koreader/koreader-base/pull/1344 & https://github.com/koreader/koreader-base/pull/1346 (fix #7485)

Assorted input fixes:

* Actually handle errors in the "there's a callback timer" input polling loop.
* Don't break timerfd when the clock probe was inconclusive.

Not directly related, but noticed because of duplicate onInputEvent handlers:

* HookContainer: Fix deregistration to actually deregister properly. "Regression" extant since its inception in #2933 (!).
* Made sure the three plugins (basically the trio of AutoThingies ;p) that were using HookContainer actually unschedule their task on teardown.

base

@@ -1 +1 @@
-Subproject commit 528aa30b42b61596e1fbe4bdf553621a67efb7d5
+Subproject commit b3337e5352e7af58759031f218232cbcff0afa84

frontend/device/input.lua

@@ -361,6 +361,7 @@ function Input:setTimeout(slot, ges, cb, origin, delay)
         -- If GestureDetector's clock source probing was inconclusive, do this on the UI timescale instead.
         if clock_id == -1 then
             deadline = TimeVal:now() + delay
+            clock_id = C.CLOCK_MONOTONIC
         else
             deadline = origin + delay
         end
@@ -934,15 +935,16 @@ end
 function Input:waitEvent(now, deadline)
     -- On the first iteration of the loop, we don't need to update now, we're following closely (a couple ms at most) behind UIManager.
     local ok, ev
-    -- Wrapper around the platform-specific input.waitForEvent (which itself is generally poll-like).
+    -- Wrapper around the platform-specific input.waitForEvent (which itself is generally poll-like, and supposed to poll *once*).
     -- Speaking of input.waitForEvent, it can return:
-    -- * true, ev: When an input event was read. ev is a table mapped after the input_event <linux/input.h> struct.
+    -- * true, ev: When a batch of input events was read.
+    --             ev is an array of event tables, themselves mapped after the input_event <linux/input.h> struct.
     -- * false, errno, timerfd: When no input event was read, possibly for benign reasons.
-    --                        One such common case is after a polling timeout, in which case errno is C.ETIME.
-    --                        If the timerfd backend is in use, and the early return was caused by a timerfd expiring,
-    --                        it returns false, C.ETIME, timerfd; where timerfd is a C pointer (i.e., light userdata)
-    --                        to the timerfd node that expired (so as to be able to free it later, c.f., input/timerfd-callbacks.h).
-    --                        Otherwise, errno is the actual error code from the backend (e.g., select's errno for the C backend).
+    --                          One such common case is after a polling timeout, in which case errno is C.ETIME.
+    --                          If the timerfd backend is in use, and the early return was caused by a timerfd expiring,
+    --                          it returns false, C.ETIME, timerfd; where timerfd is a C pointer (i.e., light userdata)
+    --                          to the timerfd node that expired (so as to be able to free it later, c.f., input/timerfd-callbacks.h).
+    --                          Otherwise, errno is the actual error code from the backend (e.g., select's errno for the C backend).
     -- * nil: When something terrible happened (e.g., fatal poll/read failure). We abort in such cases.
     while true do
         if #self.timer_callbacks > 0 then
@@ -1028,14 +1030,17 @@ function Input:waitEvent(now, deadline)
                             -- This is why we clear the full list of timers on the first match ;).
                             self:clearTimeouts()
                             self:gestureAdjustHook(touch_ges)
-                            return Event:new("Gesture",
-                                self.gesture_detector:adjustGesCoordinate(touch_ges)
-                            )
+                            return {
+                                Event:new("Gesture", self.gesture_detector:adjustGesCoordinate(touch_ges))
+                            }
                         end -- if touch_ges
                     end -- if poll_deadline reached
+                else
+                    -- Something went wrong, jump to error handling *now*
+                    break
                 end -- if poll returned ETIME
 
-                -- Refresh now on the next iteration (e.g., when we have multiple timers to check)
+                -- Refresh now on the next iteration (e.g., when we have multiple timers to check, and we've just timed out)
                 now = nil
             end -- while #timer_callbacks > 0
         else
@@ -1071,7 +1076,7 @@ function Input:waitEvent(now, deadline)
                 -- Retry on EINTR
             else
                 -- Warn, report, and go back to UIManager
-                logger.warn("Polling for input events returned an error:", ev)
+                logger.warn("Polling for input events returned an error:", ev, "->", ffi.string(C.strerror(ev)))
                 break
             end
         elseif ok == nil then
@@ -1087,55 +1092,80 @@ function Input:waitEvent(now, deadline)
     end
 
     if ok and ev then
-        if DEBUG.is_on then
-            DEBUG:logEv(ev)
-            if ev.type == C.EV_KEY then
-                logger.dbg(string.format(
-                    "key event => code: %d (%s), value: %s, time: %d.%d",
-                    ev.code, self.event_map[ev.code] or linux_evdev_key_code_map[ev.code], ev.value,
-                    ev.time.sec, ev.time.usec))
-            elseif ev.type == C.EV_SYN then
-                logger.dbg(string.format(
-                    "input event => type: %d (%s), code: %d (%s), value: %s, time: %d.%d",
-                    ev.type, linux_evdev_type_map[ev.type], ev.code, linux_evdev_syn_code_map[ev.code], ev.value,
-                    ev.time.sec, ev.time.usec))
-            elseif ev.type == C.EV_ABS then
-                logger.dbg(string.format(
-                    "input event => type: %d (%s), code: %d (%s), value: %s, time: %d.%d",
-                    ev.type, linux_evdev_type_map[ev.type], ev.code, linux_evdev_abs_code_map[ev.code], ev.value,
-                    ev.time.sec, ev.time.usec))
-            elseif ev.type == C.EV_MSC then
-                logger.dbg(string.format(
-                    "input event => type: %d (%s), code: %d (%s), value: %s, time: %d.%d",
-                    ev.type, linux_evdev_type_map[ev.type], ev.code, linux_evdev_msc_code_map[ev.code], ev.value,
-                    ev.time.sec, ev.time.usec))
+        local handled = {}
+        -- We're guaranteed that ev is an array of event tables. Might be an array of *one* event, but an array nonetheless ;).
+        for __, event in ipairs(ev) do
+            if DEBUG.is_on then
+                DEBUG:logEv(event)
+                if event.type == C.EV_KEY then
+                    logger.dbg(string.format(
+                        "key event => code: %d (%s), value: %s, time: %d.%d",
+                        event.code, self.event_map[event.code] or linux_evdev_key_code_map[event.code], event.value,
+                        event.time.sec, event.time.usec))
+                elseif event.type == C.EV_SYN then
+                    logger.dbg(string.format(
+                        "input event => type: %d (%s), code: %d (%s), value: %s, time: %d.%d",
+                        event.type, linux_evdev_type_map[event.type], event.code, linux_evdev_syn_code_map[event.code], event.value,
+                        event.time.sec, event.time.usec))
+                elseif event.type == C.EV_ABS then
+                    logger.dbg(string.format(
+                        "input event => type: %d (%s), code: %d (%s), value: %s, time: %d.%d",
+                        event.type, linux_evdev_type_map[event.type], event.code, linux_evdev_abs_code_map[event.code], event.value,
+                        event.time.sec, event.time.usec))
+                elseif event.type == C.EV_MSC then
+                    logger.dbg(string.format(
+                        "input event => type: %d (%s), code: %d (%s), value: %s, time: %d.%d",
+                        event.type, linux_evdev_type_map[event.type], event.code, linux_evdev_msc_code_map[event.code], event.value,
+                        event.time.sec, event.time.usec))
+                else
+                    logger.dbg(string.format(
+                        "input event => type: %d (%s), code: %d, value: %s, time: %d.%d",
+                        event.type, linux_evdev_type_map[event.type], event.code, event.value,
+                        event.time.sec, event.time.usec))
+                end
+            end
+            self:eventAdjustHook(event)
+            if event.type == C.EV_KEY then
+                local handled_ev = self:handleKeyBoardEv(event)
+                if handled_ev then
+                    table.insert(handled, handled_ev)
+                end
+            elseif event.type == C.EV_ABS and event.code == ABS_OASIS_ORIENTATION then
+                local handled_ev = self:handleOasisOrientationEv(event)
+                if handled_ev then
+                    table.insert(handled, handled_ev)
+                end
+            elseif event.type == C.EV_ABS or event.type == C.EV_SYN then
+                local handled_ev = self:handleTouchEv(event)
+                -- We don't gnerate an Event for *every* input event, so, make sure we don't push nil values to the array
+                if handled_ev then
+                    table.insert(handled, handled_ev)
+                end
+            elseif event.type == C.EV_MSC then
+                local handled_ev = self:handleMiscEv(event)
+                if handled_ev then
+                    table.insert(handled, handled_ev)
+                end
+            elseif event.type == C.EV_SDL then
+                local handled_ev = self:handleSdlEv(event)
+                if handled_ev then
+                    table.insert(handled, handled_ev)
+                end
             else
-                logger.dbg(string.format(
-                    "input event => type: %d (%s), code: %d, value: %s, time: %d.%d",
-                    ev.type, linux_evdev_type_map[ev.type], ev.code, ev.value,
-                    ev.time.sec, ev.time.usec))
+                -- Received some other kind of event that we do not know how to specifically handle yet
+                table.insert(handled, Event:new("GenericInput", event))
             end
         end
-        self:eventAdjustHook(ev)
-        if ev.type == C.EV_KEY then
-            return self:handleKeyBoardEv(ev)
-        elseif ev.type == C.EV_ABS and ev.code == ABS_OASIS_ORIENTATION then
-            return self:handleOasisOrientationEv(ev)
-        elseif ev.type == C.EV_ABS or ev.type == C.EV_SYN then
-            return self:handleTouchEv(ev)
-        elseif ev.type == C.EV_MSC then
-            return self:handleMiscEv(ev)
-        elseif ev.type == C.EV_SDL then
-            return self:handleSdlEv(ev)
-        else
-            -- Received some other kind of event that we do not know how to specifically handle yet
-            return Event:new("GenericInput", ev)
-        end
+        return handled
     elseif ok == false and ev then
-        return Event:new("InputError", ev)
+        return {
+            Event:new("InputError", ev)
+        }
     elseif ok == nil then
         -- No ok and no ev? Hu oh...
-        return Event:new("InputError", "Catastrophic")
+        return {
+            Event:new("InputError", "Catastrophic")
+        }
     end
 end
 

frontend/device/kobo/device.lua

@@ -94,7 +94,8 @@ local KoboDahlia = Kobo:new{
     model = "Kobo_dahlia",
     hasFrontlight = yes,
     touch_phoenix_protocol = true,
-    -- There's no slot 0, the first finger gets assigned slot 1, and the second slot 2
+    -- There's no slot 0, the first finger gets assigned slot 1, and the second slot 2.
+    -- NOTE: Could be queried at runtime via EVIOCGABS on ABS_MT_TRACKING_ID (minimum field).
     main_finger_slot = 1,
     display_dpi = 265,
     -- the bezel covers the top 11 pixels:

frontend/pluginloader.lua

@@ -84,8 +84,8 @@ function PluginLoader:loadPlugins()
     for element in pairs(OBSOLETE_PLUGINS) do
         plugins_disabled[element] = true
     end
-    for _,lookup_path in ipairs(lookup_path_list) do
-        logger.info('Loading plugins from directory:', lookup_path)
+    for _, lookup_path in ipairs(lookup_path_list) do
+        logger.info("Loading plugins from directory:", lookup_path)
         for entry in lfs.dir(lookup_path) do
             local plugin_root = lookup_path.."/"..entry
             local mode = lfs.attributes(plugin_root, "mode")
@@ -124,7 +124,7 @@ function PluginLoader:loadPlugins()
     end
 
     -- set package path for all loaded plugins
-    for _,plugin in ipairs(self.enabled_plugins) do
+    for _, plugin in ipairs(self.enabled_plugins) do
         package.path = string.format("%s;%s/?.lua", package.path, plugin.path)
         package.cpath = string.format("%s;%s/lib/?.so", package.cpath, plugin.path)
     end
@@ -193,7 +193,7 @@ function PluginLoader:createPluginInstance(plugin, attr)
     if ok then  -- re is a plugin instance
         return ok, re
     else  -- re is the error message
-        logger.err('Failed to initialize', plugin.name, 'plugin: ', re)
+        logger.err("Failed to initialize", plugin.name, "plugin: ", re)
         return nil, re
     end
 end

frontend/ui/hook_container.lua

@@ -49,16 +49,18 @@ end
 function HookContainer:registerWidget(name, widget)
     self:_assertIsValidName(name)
     assert(type(widget) == "table")
-    self:register(name, function(args)
+    -- *That* is the function we actually register and need to unregister later, so keep a ref to it...
+    local hook_func = function(args)
         local f = widget["on" .. name]
         self:_assertIsValidFunction(f)
         f(widget, args)
-    end)
+    end
+    self:register(name, hook_func)
     local original_close_widget = widget.onCloseWidget
     self:_assertIsValidFunctionOrNil(original_close_widget)
     widget.onCloseWidget = function()
         if original_close_widget then original_close_widget(widget) end
-        self:unregister(name, widget["on" .. name])
+        self:unregister(name, hook_func)
     end
 end
 

frontend/ui/uimanager.lua

@@ -1549,7 +1549,7 @@ end
 -- Process all pending events on all registered ZMQs.
 function UIManager:processZMQs()
     for _, zeromq in ipairs(self._zeromqs) do
-        for input_event in zeromq.waitEvent,zeromq do
+        for input_event in zeromq.waitEvent, zeromq do
             self:handleInputEvent(input_event)
         end
     end
@@ -1611,12 +1611,15 @@ function UIManager:handleInput()
     -- Anywhere else breaks preventStandby/allowStandby invariants used by background jobs while UI is left running.
     self:_standbyTransition()
 
-    -- wait for next event
-    local input_event = Input:waitEvent(now, deadline)
+    -- wait for next batch of events
+    local input_events = Input:waitEvent(now, deadline)
 
-    -- delegate input_event to handler
-    if input_event then
-        self:handleInputEvent(input_event)
+    -- delegate each input event to handler
+    if input_events then
+        -- Handle the full batch of events
+        for __, ev in ipairs(input_events) do
+            self:handleInputEvent(ev)
+        end
     end
 
     if self.looper then
@@ -1637,7 +1640,7 @@ end
 
 
 function UIManager:onRotation()
-    self:setDirty('all', 'full')
+    self:setDirty("all", "full")
     self:forceRePaint()
 end
 

plugins/autostandby.koplugin/main.lua

@@ -44,6 +44,11 @@ function AutoStandby:init()
     self.ui.menu:registerToMainMenu(self)
 end
 
+function AutoStandby:onCloseWidget()
+    logger.dbg("AutoStandby:onCloseWidget() instance=", tostring(self))
+    UIManager:unschedule(AutoStandby.allow)
+end
+
 function AutoStandby:addToMainMenu(menu_items)
     menu_items.autostandby = {
         sorting_hint = "device",

plugins/autosuspend.koplugin/main.lua

@@ -100,6 +100,7 @@ function AutoSuspend:_restart()
 end
 
 function AutoSuspend:init()
+    logger.dbg("AutoSuspend: init")
     if Device:isPocketBook() and not Device:canSuspend() then return end
     UIManager.event_hook:registerWidget("InputEvent", self)
     self:_unschedule()
@@ -109,6 +110,13 @@ function AutoSuspend:init()
     self.ui.menu:registerToMainMenu(self)
 end
 
+-- For event_hook automagic deregistration purposes
+function AutoSuspend:onCloseWidget()
+    logger.dbg("AutoSuspend: onCloseWidget")
+    if Device:isPocketBook() and not Device:canSuspend() then return end
+    self:_unschedule()
+end
+
 function AutoSuspend:onInputEvent()
     logger.dbg("AutoSuspend: onInputEvent")
     self.last_action_tv = UIManager:getTime()

plugins/autoturn.koplugin/main.lua

@@ -94,6 +94,11 @@ function AutoTurn:init()
     self:_start()
 end
 
+function AutoTurn:onCloseWidget()
+    logger.dbg("AutoTurn: onCloseWidget")
+    self:_deprecateLastTask()
+end
+
 function AutoTurn:onCloseDocument()
     logger.dbg("AutoTurn: onCloseDocument")
     self:_deprecateLastTask()

spec/unit/device_spec.lua

@@ -274,13 +274,15 @@ describe("device module", function()
             mock_ffi_input = require('ffi/input')
             stub(mock_ffi_input, "waitForEvent")
             mock_ffi_input.waitForEvent.returns(true, {
-                type = 3,
-                time = {
-                    usec = 450565,
-                    sec = 1471081881
-                },
-                code = 24,
-                value = 16
+                {
+                    type = 3,
+                    time = {
+                        usec = 450565,
+                        sec = 1471081881
+                    },
+                    code = 24,
+                    value = 16
+                }
             })
 
             local UIManager = require("ui/uimanager")