From 9af3e95d9d56a69fa6e275834aee70f3f7f966d9 Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Sat, 25 Nov 2023 06:30:37 +0100 Subject: [PATCH] Kindle: Fix a smattering of frontlight bugs * afterResume had *two* different implementations, so the historical one that handled frontlight fixups no longer ran (regression since #10426) * isFrontlightOn was completely broken, for a couple of reasons: * There was no is isFrontlightOnHW implementation, so when it ran, it mostly always thought the frontlight was on, because self.fl_intensity doesn't change on toggle off. * _decideFrontlightState was never called on Kindle, so isFrontlightOnHW was never really called, making isFrontlightOn completely useless. Call it in setIntensityHW's coda, as it ought to be. And properly document that. Generic *was* calling _decideFrontlightState is setIntensity, but *before* actually setting the frontlight, which makes no goddamn sense, so get rid of that, too. * Also fix frontlight toggle notifications (regression since #10305) TL;DR: The PowerD API being a mess strikes again. --- frontend/device/android/powerd.lua | 4 ++- frontend/device/devicelistener.lua | 2 +- frontend/device/generic/powerd.lua | 6 ++-- frontend/device/kindle/powerd.lua | 52 +++++++++++++++++---------- frontend/device/pocketbook/powerd.lua | 3 ++ frontend/device/sdl/powerd.lua | 1 + spec/unit/frontlight_spec.lua | 1 + 7 files changed, 46 insertions(+), 23 deletions(-) diff --git a/frontend/device/android/powerd.lua b/frontend/device/android/powerd.lua index 3cdd01f35..e121e25c2 100644 --- a/frontend/device/android/powerd.lua +++ b/frontend/device/android/powerd.lua @@ -60,7 +60,7 @@ function AndroidPowerD:turnOffFrontlightHW() android.setScreenBrightness(self.fl_min) end -function AndroidPowerD:turnOnFrontlightHW() +function AndroidPowerD:turnOnFrontlightHW(done_callback) if self:isFrontlightOn() and self:isFrontlightOnHW() then return end @@ -68,6 +68,8 @@ function AndroidPowerD:turnOnFrontlightHW() android.enableFrontlightSwitch() android.setScreenBrightness(math.floor(self.fl_intensity * self.bright_diff / self.fl_max)) + + return false end return AndroidPowerD diff --git a/frontend/device/devicelistener.lua b/frontend/device/devicelistener.lua index 3fccf3057..823d40faa 100644 --- a/frontend/device/devicelistener.lua +++ b/frontend/device/devicelistener.lua @@ -187,7 +187,7 @@ if Device:hasFrontlight() then function DeviceListener:onToggleFrontlight() local powerd = Device:getPowerDevice() local new_text - if powerd.is_fl_on then + if powerd:isFrontlightOn() then new_text = _("Frontlight disabled.") else new_text = _("Frontlight enabled.") diff --git a/frontend/device/generic/powerd.lua b/frontend/device/generic/powerd.lua index 24625aa6f..5f41fa2d1 100644 --- a/frontend/device/generic/powerd.lua +++ b/frontend/device/generic/powerd.lua @@ -43,7 +43,10 @@ function BasePowerD:new(o) end function BasePowerD:init() end -function BasePowerD:setIntensityHW(intensity) end +--- @note: This should *always* call self:_decideFrontlightState() in its coda (unless you have a custom isFrontlightOn implementation)! +function BasePowerD:setIntensityHW(intensity) + self:_decideFrontlightState() +end --- @note: Unlike the "public" setWarmth, this one takes a value in the *native* scale! function BasePowerD:setWarmthHW(warmth) end function BasePowerD:getCapacityHW() return 0 end @@ -207,7 +210,6 @@ function BasePowerD:setIntensity(intensity) if not self.device:hasFrontlight() then return false end if intensity == self:frontlightIntensity() then return false end self.fl_intensity = self:normalizeIntensity(intensity) - self:_decideFrontlightState() logger.dbg("set light intensity", self.fl_intensity) self:setIntensityHW(self.fl_intensity) self:stateChanged() diff --git a/frontend/device/kindle/powerd.lua b/frontend/device/kindle/powerd.lua index 0a2acf3c3..6b0b04bea 100644 --- a/frontend/device/kindle/powerd.lua +++ b/frontend/device/kindle/powerd.lua @@ -29,17 +29,22 @@ end -- If we start with the light off (fl_intensity is fl_min), ensure a toggle will set it to the lowest "on" step, -- and that we update fl_intensity (by using setIntensity and not setIntensityHW). -function KindlePowerD:turnOnFrontlightHW() +function KindlePowerD:turnOnFrontlightHW(done_callback) self:setIntensity(self.fl_intensity == self.fl_min and self.fl_min + 1 or self.fl_intensity) + + return false end -- Which means we need to get rid of the insane fl_intensity == fl_min shortcut in turnOnFrontlight, too... -- That dates back to #2941, and I have no idea what it's supposed to help with. -function BasePowerD:turnOnFrontlight() +function KindlePowerD:turnOnFrontlight(done_callback) if not self.device:hasFrontlight() then return end if self:isFrontlightOn() then return false end - self:turnOnFrontlightHW() + local cb_handled = self:turnOnFrontlightHW(done_callback) self.is_fl_on = true self:stateChanged() + if not cb_handled and done_callback then + done_callback() + end return true end @@ -79,6 +84,14 @@ function KindlePowerD:frontlightIntensityHW() end end +-- Make sure isFrontlightOn reflects the actual HW state, +-- as self.fl_intensity is kept as-is when toggling the light off, +-- in order to be able to toggle it back on at the right intensity. +function KindlePowerD:isFrontlightOnHW() + local hw_intensity = self:frontlightIntensityHW() + return hw_intensity > self.fl_min +end + function KindlePowerD:setIntensityHW(intensity) -- Handle the synthetic step switcheroo on ! canTurnFrontlightOff devices... local turn_it_off = false @@ -108,6 +121,9 @@ function KindlePowerD:setIntensityHW(intensity) ffiUtil.writeToSysfs(intensity, self.warmth_intensity_file) end end + + -- The state might have changed, make sure we don't break isFrontlightOn + self:_decideFrontlightState() end function KindlePowerD:frontlightWarmthHW() @@ -182,22 +198,6 @@ function KindlePowerD:_readFLIntensity() return self:read_int_file(self.fl_intensity_file) end -function KindlePowerD:afterResume() - if not self.device:hasFrontlight() then - return - end - if self:isFrontlightOn() then - -- The Kindle framework should turn the front light back on automatically. - -- The following statement ensures consistency of intensity, but should basically always be redundant, - -- since we set intensity via lipc and not sysfs ;). - -- NOTE: This is race-y, and we want to *lose* the race, hence the use of the scheduler (c.f., #4392) - UIManager:tickAfterNext(function() self:turnOnFrontlightHW() end) - else - -- But in the off case, we *do* use sysfs, so this one actually matters. - UIManager:tickAfterNext(function() self:turnOffFrontlightHW() end) - end -end - function KindlePowerD:toggleSuspend() if self.lipc_handle then self.lipc_handle:set_int_property("com.lab126.powerd", "powerButton", 1) @@ -293,6 +293,20 @@ function KindlePowerD:afterResume() -- Restore user input and emit the Resume event. self.device:_afterResume() + + if not self.device:hasFrontlight() then + return + end + if self:isFrontlightOn() then + -- The Kindle framework should turn the front light back on automatically. + -- The following statement ensures consistency of intensity, but should basically always be redundant, + -- since we set intensity via lipc and not sysfs ;). + -- NOTE: This is race-y, and we want to *lose* the race, hence the use of the scheduler (c.f., #4392) + UIManager:tickAfterNext(function() self:turnOnFrontlightHW() end) + else + -- But in the off case, we *do* use sysfs, so this one actually matters. + UIManager:tickAfterNext(function() self:turnOffFrontlightHW() end) + end end function KindlePowerD:UIManagerReadyHW(uimgr) diff --git a/frontend/device/pocketbook/powerd.lua b/frontend/device/pocketbook/powerd.lua index 11f9dc94d..9e82d1eac 100644 --- a/frontend/device/pocketbook/powerd.lua +++ b/frontend/device/pocketbook/powerd.lua @@ -41,6 +41,9 @@ function PocketBookPowerD:setIntensityHW(intensity) else inkview.SetFrontlightState(intensity) end + + -- We have a custom isFrontlightOn implementation, so this is redundant + self:_decideFrontlightState() end function PocketBookPowerD:isFrontlightOn() diff --git a/frontend/device/sdl/powerd.lua b/frontend/device/sdl/powerd.lua index 6fa015b9c..5456bccbf 100644 --- a/frontend/device/sdl/powerd.lua +++ b/frontend/device/sdl/powerd.lua @@ -18,6 +18,7 @@ end function SDLPowerD:setIntensityHW(intensity) require("logger").info("set brightness to", intensity) self.hw_intensity = intensity or self.hw_intensity + self:_decideFrontlightState() end function SDLPowerD:frontlightWarmthHW() diff --git a/spec/unit/frontlight_spec.lua b/spec/unit/frontlight_spec.lua index 36466ed50..f3412f43a 100644 --- a/spec/unit/frontlight_spec.lua +++ b/spec/unit/frontlight_spec.lua @@ -23,6 +23,7 @@ describe("Frontlight function in PowerD", function() end PowerD.setIntensityHW = function(self, intensity) self.frontlight = intensity + self:_decideFrontlightState() end end)