Kobo: Refactor frontlight toggle ramp behavior (#10305)

* Rewrite the loop mechanism to use scheduled tasks instead of a single blocking-ish subprocess.
* Change the actual logic to be more pleasing to the eye, especially on newer devices, as those *may* natively ramp on set; and fix a bad interaction with that behavior that could lead to no ramp at all on ramp down.
* Simplify Generic's Suspend handling to deal with the refresh ordering in a saner manner. The screensaver might be visible a tad longer than before this change before the frontlight actually ramps off.

frontend/device/devicelistener.lua

@@ -186,14 +186,18 @@ if Device:hasFrontlight() then
 
     function DeviceListener:onToggleFrontlight()
         local powerd = Device:getPowerDevice()
-        powerd:toggleFrontlight()
         local new_text
         if powerd.is_fl_on then
-            new_text = _("Frontlight enabled.")
-        else
             new_text = _("Frontlight disabled.")
+        else
+            new_text = _("Frontlight enabled.")
         end
-        Notification:notify(new_text)
+        -- We defer displaying the Notification to PowerD, as the toggle may be a ramp, and we both want to make sure the refresh fencing won't affect it, and that we only display the Notification at the end...
+        local notif_source = Notification.notify_source
+        local notif_cb = function()
+            Notification:notify(new_text, notif_source)
+        end
+        powerd:toggleFrontlight(notif_cb)
     end
 
     function DeviceListener:onShowFlDialog()

frontend/device/generic/device.lua

@@ -308,7 +308,6 @@ function Device:onPowerEvent(ev)
         end
     -- else we were not in screensaver mode
     elseif ev == "Power" or ev == "Suspend" then
-        self.powerd:beforeSuspend()
         local UIManager = require("ui/uimanager")
         logger.dbg("Suspending...")
         -- Add the current state of the SleepCover flag...
@@ -325,28 +324,25 @@ function Device:onPowerEvent(ev)
         if self:needsScreenRefreshAfterResume() then
             self.screen:refreshFull()
         end
-        -- NOTE: In the same vein as above, this is delayed to make sure we update the screen first.
-        --       (This, unfortunately, means we can't just move this to Device:_beforeSuspend :/).
-        UIManager:scheduleIn(0.1, function()
-            -- NOTE: This side of the check needs to be laxer, some platforms can handle Wi-Fi without WifiManager ;).
-            if self:hasWifiToggle() then
-                local network_manager = require("ui/network/manager")
-                -- NOTE: wifi_was_on does not necessarily mean that Wi-Fi is *currently* on! It means *we* enabled it.
-                --       This is critical on Kobos (c.f., #3936), where it might still be on from KSM or Nickel,
-                --       without us being aware of it (i.e., wifi_was_on still unset or false),
-                --       because suspend will at best fail, and at worst deadlock the system if Wi-Fi is on,
-                --       regardless of who enabled it!
-                if network_manager:isWifiOn() then
-                    network_manager:releaseIP()
-                    network_manager:turnOffWifi()
-                end
-            end
-            -- Only actually schedule suspension if we're still supposed to go to sleep,
-            -- because the Wi-Fi stuff above may have blocked for a significant amount of time...
-            if self.screen_saver_mode then
-                self:rescheduleSuspend()
+        -- NOTE: In the same vein as above, make sure we update the screen *now*, before dealing with Wi-Fi.
+        UIManager:forceRePaint()
+        -- NOTE: This side of the check needs to be laxer, some platforms can handle Wi-Fi without WifiManager ;).
+        if self:hasWifiToggle() then
+            local network_manager = require("ui/network/manager")
+            -- NOTE: wifi_was_on does not necessarily mean that Wi-Fi is *currently* on! It means *we* enabled it.
+            --       This is critical on Kobos (c.f., #3936), where it might still be on from KSM or Nickel,
+            --       without us being aware of it (i.e., wifi_was_on still unset or false),
+            --       because suspend will at best fail, and at worst deadlock the system if Wi-Fi is on,
+            --       regardless of who enabled it!
+            if network_manager:isWifiOn() then
+                network_manager:releaseIP()
+                network_manager:turnOffWifi()
             end
-        end)
+        end
+        -- Only turn off the frontlight *after* we've displayed the screensaver and dealt with Wi-Fi,
+        -- to prevent that from affecting the smoothness of the frontlight ramp down.
+        self.powerd:beforeSuspend()
+        self:rescheduleSuspend()
     end
 end
 

frontend/device/generic/powerd.lua

@@ -1,4 +1,4 @@
-local UIManager -- will be updated when available
+local UIManager = nil -- will be updated when available
 local Math = require("optmath")
 local logger = require("logger")
 local time = require("ui/time")
@@ -43,6 +43,7 @@ end
 
 function BasePowerD:readyUI()
     UIManager = require("ui/uimanager")
+    self:readyUIHW(UIManager)
 end
 
 function BasePowerD:init() end
@@ -62,9 +63,10 @@ function BasePowerD:isAuxChargingHW() return false end
 function BasePowerD:isAuxChargedHW() return false end
 function BasePowerD:frontlightIntensityHW() return 0 end
 function BasePowerD:isFrontlightOnHW() return self.fl_intensity > self.fl_min end
-function BasePowerD:turnOffFrontlightHW() self:setIntensityHW(self.fl_min) end
-function BasePowerD:turnOnFrontlightHW() self:setIntensityHW(self.fl_intensity) end --- @fixme: what if fl_intensity == fl_min (c.f., kindle)?
+function BasePowerD:turnOffFrontlightHW(done_callback) self:setIntensityHW(self.fl_min) end
+function BasePowerD:turnOnFrontlightHW(done_callback) self:setIntensityHW(self.fl_intensity) end --- @fixme: what if fl_intensity == fl_min (c.f., kindle)?
 function BasePowerD:frontlightWarmthHW() return 0 end
+function BasePowerD:readyUIHW(uimgr) end
 -- Anything that needs to be done before doing a real hardware suspend.
 -- (Such as turning the front light off).
 function BasePowerD:beforeSuspend() end
@@ -94,29 +96,29 @@ function BasePowerD:frontlightIntensity()
     return self.fl_intensity
 end
 
-function BasePowerD:toggleFrontlight()
+function BasePowerD:toggleFrontlight(done_callback)
     if not self.device:hasFrontlight() then return false end
     if self:isFrontlightOn() then
-        return self:turnOffFrontlight()
+        return self:turnOffFrontlight(done_callback)
     else
-        return self:turnOnFrontlight()
+        return self:turnOnFrontlight(done_callback)
     end
 end
 
-function BasePowerD:turnOffFrontlight()
+function BasePowerD:turnOffFrontlight(done_callback)
     if not self.device:hasFrontlight() then return end
     if self:isFrontlightOff() then return false end
-    self:turnOffFrontlightHW()
+    self:turnOffFrontlightHW(done_callback)
     self.is_fl_on = false
     self:stateChanged()
     return true
 end
 
-function BasePowerD:turnOnFrontlight()
+function BasePowerD:turnOnFrontlight(done_callback)
     if not self.device:hasFrontlight() then return end
     if self:isFrontlightOn() then return false end
     if self.fl_intensity == self.fl_min then return false end  --- @fixme what the hell?
-    self:turnOnFrontlightHW()
+    self:turnOnFrontlightHW(done_callback)
     self.is_fl_on = true
     self:stateChanged()
     return true

frontend/device/kobo/device.lua

@@ -166,7 +166,7 @@ local Kobo = Generic:extend{
     -- Device ships with various hardware revisions under the same device code, requirign automatic hardware detection...
     automagic_sysfs = false,
 
-    unexpected_wakeup_count = 0
+    unexpected_wakeup_count = 0,
 }
 
 local KoboTrilogyA = Kobo:extend{
@@ -449,6 +449,11 @@ local KoboCadmus = Kobo:extend{
         nl_min = 0,
         nl_max = 10,
         nl_inverted = false,
+        --- @note: The Sage natively ramps when setting the frontlight intensity.
+        ---        A side-effect of this behavior is that if you queue a series of intensity changes ending at 0,
+        ---        it won't ramp *at all*, and jump straight to zero.
+        ---        So we delay the final ramp off step to prevent (both) the native and our ramping from being optimized out
+        ramp_off_delay = 0.5,
     },
     boot_rota = C.FB_ROTATE_CW,
     battery_sysfs = "/sys/class/power_supply/battery",

frontend/device/kobo/powerd.lua

@@ -1,8 +1,8 @@
+local UIManager = nil -- will be updated when available
 local BasePowerD = require("device/generic/powerd")
 local Math = require("optmath")
 local NickelConf = require("device/kobo/nickel_conf")
 local SysfsLight = require ("device/sysfs_light")
-local ffiUtil = require("ffi/util")
 local RTC = require("ffi/rtc")
 
 -- Here, we only deal with the real hw intensity.
@@ -133,6 +133,12 @@ function KoboPowerD:init()
     self.initial_is_fl_on = true
 
     if self.device:hasFrontlight() then
+        self.device.frontlight_settings = self.device.frontlight_settings or {}
+        -- Does this device require non-standard ramping behavior?
+        self.device.frontlight_settings.ramp_off_delay = self.device.frontlight_settings.ramp_off_delay or 0.0
+        --- @note: Newer devices appear to block slightly longer on FL ioctls/sysfs, so we only really need a delay on older devices.
+        self.device.frontlight_settings.ramp_delay = self.device.frontlight_settings.ramp_delay or (self.device:hasNaturalLight() and 0.0 or 0.025)
+
         -- If this device has natural light (currently only KA1 & Forma)
         -- Use the SysFS interface, and ioctl otherwise.
         -- NOTE: On the Forma, nickel still appears to prefer using ntx_io to handle the FL,
@@ -241,10 +247,10 @@ function KoboPowerD:isFrontlightOnHW()
         self.initial_is_fl_on = nil
         return ret
     end
-    return self.hw_intensity > 0
+    return self.hw_intensity > 0 and not self.fl_ramp_down_running
 end
 
-function KoboPowerD:setIntensityHW(intensity)
+function KoboPowerD:_setIntensityHW(intensity)
     if self.fl == nil then return end
     if self.fl_warmth == nil or self.device:hasNaturalLightMixer() then
         -- We either don't have NL, or we have a mixer: we only want to set the intensity (c.f., #5429)
@@ -260,6 +266,11 @@ function KoboPowerD:setIntensityHW(intensity)
     self:_decideFrontlightState()
 end
 
+function KoboPowerD:setIntensityHW(intensity)
+    self:_stopFrontlightRamp()
+    self:_setIntensityHW(intensity)
+end
+
 -- NOTE: We *can* actually read this from the system (as well as frontlight level, since Mk. 7),
 --       but this is already a huge mess, so, keep ignoring it...
 function KoboPowerD:frontlightWarmthHW()
@@ -300,38 +311,84 @@ function KoboPowerD:isChargedHW()
     return false
 end
 
-function KoboPowerD:turnOffFrontlightHW()
+function KoboPowerD:_postponedSetIntensityHW(end_intensity, done_callback)
+    self:_setIntensityHW(end_intensity)
+    self.fl_ramp_down_running = false
+
+    if done_callback then
+        done_callback()
+    end
+end
+
+function KoboPowerD:_stopFrontlightRamp()
+    if self.fl_ramp_up_running or self.fl_ramp_down_running then
+        -- Make sure we have no other ramp running.
+        UIManager:unschedule(self.turnOffFrontlightRamp)
+        UIManager:unschedule(self.turnOnFrontlightRamp)
+        UIManager:unschedule(self._postponedSetIntensityHW)
+        self.fl_ramp_up_running = false
+        self.fl_ramp_down_running = false
+    end
+end
+
+-- This will ramp down faster at high intensity values (i.e., start), and slower at lower intensity values (i.e., end).
+-- That's an attempt at making the *perceived* effect appear as a more linear brightness change.
+-- The whole function gets called at most log(100)/log(0.75) = 17 times,
+-- leading to a 0.025*17 + 0.5 = 0.925s ramp down time (non blocking); can be aborted.
+function KoboPowerD:turnOffFrontlightRamp(curr_ramp_intensity, end_intensity, done_callback)
+    curr_ramp_intensity = math.floor(math.max(curr_ramp_intensity * .75, self.fl_min))
+
+    if curr_ramp_intensity > end_intensity then
+        self:_setIntensityHW(curr_ramp_intensity)
+        UIManager:scheduleIn(self.device.frontlight_settings.ramp_delay, self.turnOffFrontlightRamp, self, curr_ramp_intensity, end_intensity, done_callback)
+    else
+        -- Some devices require delaying the final step, to prevent them from jumping straight to zero and messing up the ramp.
+        UIManager:scheduleIn(self.device.frontlight_settings.ramp_off_delay, self._postponedSetIntensityHW, self, end_intensity, done_callback)
+        -- no reschedule here, as we are done
+    end
+end
+
+function KoboPowerD:turnOffFrontlightHW(done_callback)
     if not self:isFrontlightOnHW() then
         return
     end
-    ffiUtil.runInSubProcess(function()
-        for i = 1, 5 do
-            -- NOTE: Do *not* switch to (self.fl_intensity * (1/5) * i) here, it may lead to rounding errors,
-            --       which is problematic paired w/ math.floor because it doesn't round towards zero,
-            --       which means we may end up passing -1 to setIntensityHW, which will fail,
-            --       because we're bypassing the clamping usually done by setIntensity...
-            self:setIntensityHW(math.floor(self.fl_intensity - (self.fl_intensity / 5 * i)))
-            --- @note: Newer devices appear to block slightly longer on FL ioctls/sysfs, so only sleep on older devices,
-            ---        otherwise we get a jump and not a ramp ;).
-            if not self.device:hasNaturalLight() then
-                if (i < 5) then
-                    ffiUtil.usleep(35 * 1000)
-                end
-            end
+
+    if UIManager then
+        -- We've got nothing to do if we're already ramping down
+        if not self.fl_ramp_down_running then
+            self:_stopFrontlightRamp()
+            self:turnOffFrontlightRamp(self.fl_intensity, self.fl_min, done_callback)
+            self.fl_ramp_down_running = true
         end
-    end, false, true)
-    -- NOTE: This is essentially what setIntensityHW does, except we don't actually touch the FL,
-    --       we only sync the state of the main process with the final state of what we're doing in the forks.
-    -- And update hw_intensity in our actual process ;).
-    self.hw_intensity = self.fl_min
-    -- NOTE: And don't forget to update sysfs_light, too, as a real setIntensityHW would via setBrightness
-    if self.fl then
-        self.fl.current_brightness = self.fl_min
+    else
+        -- If UIManager is not initialized yet, just turn it off immediately
+        self:setIntensityHW(self.fl_min)
     end
-    self:_decideFrontlightState()
 end
 
-function KoboPowerD:turnOnFrontlightHW()
+-- Similar functionality as `Kobo:turnOnFrontlightHW`, but the other way around ;).
+function KoboPowerD:turnOnFrontlightRamp(curr_ramp_intensity, end_intensity, done_callback)
+    if curr_ramp_intensity == 0 then
+        curr_ramp_intensity = 1
+    else
+        curr_ramp_intensity = math.ceil(math.min(curr_ramp_intensity * 1.5, self.fl_max))
+    end
+
+    if curr_ramp_intensity < end_intensity then
+        self:_setIntensityHW(curr_ramp_intensity)
+        UIManager:scheduleIn(self.device.frontlight_settings.ramp_delay, self.turnOnFrontlightRamp, self, curr_ramp_intensity, end_intensity, done_callback)
+    else
+        self:_setIntensityHW(end_intensity)
+        self.fl_ramp_up_running = false
+
+        if done_callback then
+            done_callback()
+        end
+        -- no reschedule here, as we are done
+    end
+end
+
+function KoboPowerD:turnOnFrontlightHW(done_callback)
     -- NOTE: Insane workaround for the first toggle after a startup with the FL off.
     -- The light is actually off, but hw_intensity couldn't have been set to a sane value because of a number of interactions.
     -- So, fix it now, so we pass the isFrontlightOnHW check (which checks if hw_intensity > fl_min).
@@ -341,27 +398,19 @@ function KoboPowerD:turnOnFrontlightHW()
     if self:isFrontlightOnHW() then
         return
     end
-    ffiUtil.runInSubProcess(function()
-        for i = 1, 5 do
-            self:setIntensityHW(math.ceil(self.fl_min + (self.fl_intensity / 5 * i)))
-            --- @note: Newer devices appear to block slightly longer on FL ioctls/sysfs, so only sleep on older devices,
-            ---        otherwise we get a jump and not a ramp ;).
-            if not self.device:hasNaturalLight() then
-                if (i < 5) then
-                    ffiUtil.usleep(35 * 1000)
-                end
-            end
+
+    if UIManager then
+        -- We've got nothing to do if we're already ramping up
+        if not self.fl_ramp_up_running then
+            self:_stopFrontlightRamp()
+            self.fl_ramp_up_running = true
+
+            self:turnOnFrontlightRamp(self.fl_min, self.fl_intensity, done_callback)
         end
-    end, false, true)
-    -- NOTE: This is essentially what setIntensityHW does, except we don't actually touch the FL,
-    --       we only sync the state of the main process with the final state of what we're doing in the forks.
-    -- And update hw_intensity in our actual process ;).
-    self.hw_intensity = self.fl_intensity
-    -- NOTE: And don't forget to update sysfs_light, too, as a real setIntensityHW would via setBrightness
-    if self.fl then
-        self.fl.current_brightness = self.fl_intensity
+    else
+        -- If UIManager is not initialized yet, just turn it on immediately
+        self:setIntensityHW(self.fl_intensity)
     end
-    self:_decideFrontlightState()
 end
 
 -- Turn off front light before suspend.
@@ -385,4 +434,8 @@ function KoboPowerD:afterResume()
     RTC:HCToSys()
 end
 
+function KoboPowerD:readyUIHW(uimgr)
+    UIManager = uimgr
+end
+
 return KoboPowerD

frontend/ui/widget/notification.lua

@@ -156,7 +156,7 @@ end
 function Notification:notify(arg, source, refresh_after)
     source = source or self.notify_source
     local mask = G_reader_settings:readSetting("notification_sources_to_show_mask") or self.SOURCE_DEFAULT
-    if source and band(mask, self.notify_source) ~= 0 then
+    if source and band(mask, source) ~= 0 then
         UIManager:show(Notification:new{
             text = arg,
          })