diff --git a/frontend/device/devicelistener.lua b/frontend/device/devicelistener.lua index 5c964dee7..9da2122e3 100644 --- a/frontend/device/devicelistener.lua +++ b/frontend/device/devicelistener.lua @@ -116,6 +116,7 @@ if Device:hasFrontlight() then else powerd:setIntensity(new_intensity) end + powerd:updateResumeFrontlightState() return true end @@ -183,6 +184,7 @@ if Device:hasFrontlight() then if not powerd:toggleFrontlight(notif_cb) then Notification:notify(_("Frontlight unchanged."), notif_source) end + powerd:updateResumeFrontlightState() end function DeviceListener:onShowFlDialog() diff --git a/frontend/device/generic/powerd.lua b/frontend/device/generic/powerd.lua index 5f41fa2d1..4cd88ed29 100644 --- a/frontend/device/generic/powerd.lua +++ b/frontend/device/generic/powerd.lua @@ -18,6 +18,7 @@ local BasePowerD = { last_aux_capacity_pull_time = time.s(-61), -- timestamp of last pull is_fl_on = false, -- whether the frontlight is on + fl_was_on = nil, -- whether the frontlight *was* on before suspend } function BasePowerD:new(o) @@ -29,6 +30,7 @@ function BasePowerD:new(o) if o.device and o.device:hasFrontlight() then o.fl_intensity = o:frontlightIntensityHW() o:_decideFrontlightState() + o:updateResumeFrontlightState() end --- @note: Post-init, as the min/max values may be computed at runtime on some platforms assert(o.fl_warmth_min < o.fl_warmth_max) @@ -112,6 +114,11 @@ function BasePowerD:_decideFrontlightState() self.is_fl_on = self:isFrontlightOnHW() end +-- Separate from _decideFrontlightState, as this is only called by *interactive* codepaths +function BasePowerD:updateResumeFrontlightState() + self.fl_was_on = self:isFrontlightOn() +end + function BasePowerD:isFrontlightOff() return not self:isFrontlightOn() end diff --git a/frontend/device/kobo/powerd.lua b/frontend/device/kobo/powerd.lua index acc98ba62..f3c65e824 100644 --- a/frontend/device/kobo/powerd.lua +++ b/frontend/device/kobo/powerd.lua @@ -3,6 +3,7 @@ local Math = require("optmath") local NickelConf = require("device/kobo/nickel_conf") local SysfsLight = require ("device/sysfs_light") local UIManager +local logger = require("logger") local RTC = require("ffi/rtc") -- Here, we only deal with the real hw intensity. @@ -17,7 +18,6 @@ local KoboPowerD = BasePowerD:new{ battery_sysfs = nil, aux_battery_sysfs = nil, fl_warmth_min = 0, fl_warmth_max = 100, - fl_was_on = nil, } --- @todo Remove G_defaults:readSetting("KOBO_LIGHT_ON_START") @@ -84,7 +84,7 @@ function KoboPowerD:_syncKoboLightOnStart() self.fl_warmth = new_warmth end - -- In any case frontlight is off, ensure intensity is non-zero so untoggle works + -- In case frontlight is off, ensure hw_intensity is non-zero so toggle on works if self.initial_is_fl_on == false and self.hw_intensity == 0 then self.hw_intensity = 1 end @@ -244,8 +244,8 @@ end function KoboPowerD:isFrontlightOnHW() if self.initial_is_fl_on ~= nil then -- happens only once after init() - -- give initial state to BasePowerD, which will - -- reset our self.hw_intensity to 0 if self.initial_is_fl_on is false + -- Pass our initial state to BasePowerD, + -- which will reset our self.hw_intensity to 0 if self.initial_is_fl_on is false local ret = self.initial_is_fl_on self.initial_is_fl_on = nil return ret @@ -263,9 +263,9 @@ function KoboPowerD:_setIntensityHW(intensity) self.fl:setNaturalBrightness(intensity, self.fl_warmth) end self.hw_intensity = intensity - -- Now that we have set intensity, we need to let BasePowerD - -- know about possibly changed frontlight state (if we came - -- from light toggled off to some intensity > 0). + -- Now that we have set the intensity, + -- we need to let BasePowerD know about the possibly new frontlight state + -- (if we came from light toggled off to some intensity > 0). self:_decideFrontlightState() end @@ -314,8 +314,11 @@ function KoboPowerD:isChargedHW() return false end +-- NOTE: When ramping down, we start from the *actual* intensity (hw_intensity), +-- instead of the expected one (fl_intensity), +-- in case a previously incomplete ramp was canceled and left us in an inconsistent state. function KoboPowerD:_startRampDown(done_callback) - self:turnOffFrontlightRamp(self.fl_intensity, self.fl_min, done_callback) + self:turnOffFrontlightRamp(self.hw_intensity, self.fl_min, done_callback) self.fl_ramp_down_running = true end @@ -372,7 +375,7 @@ function KoboPowerD:turnOffFrontlightHW(done_callback) -- otherwise you just see a single delayed step (1%) or two stuttery ones (2%) ;). -- FWIW, modern devices with a different PWM controller (i.e., with no controller-specific ramp_off_delay workarounds) -- deal with our 2% ramp without stuttering. - if self.device.frontlight_settings.ramp_off_delay > 0.0 and self.fl_intensity <= 2 then + if self.device.frontlight_settings.ramp_off_delay > 0.0 and self.hw_intensity <= 2 then UIManager:scheduleIn(self.device.frontlight_settings.ramp_delay, self._endRampDown, self, self.fl_min, done_callback) else -- NOTE: Similarly, some controllers *really* don't like to be interleaved with screen refreshes, @@ -380,7 +383,7 @@ function KoboPowerD:turnOffFrontlightHW(done_callback) if self.device.frontlight_settings.delay_ramp_start then UIManager:nextTick(self._startRampDown, self, done_callback) else - self:turnOffFrontlightRamp(self.fl_intensity, self.fl_min, done_callback) + self:turnOffFrontlightRamp(self.hw_intensity, self.fl_min, done_callback) self.fl_ramp_down_running = true end end @@ -462,15 +465,7 @@ function KoboPowerD:turnOnFrontlightHW(done_callback) return true end --- NOTE: We delay those *slightly*, so tracking the fl_was_on state needs to be delayed with it, --- or stuff gets wonky if you trip a resume/suspend in quick succession... --- c.f., https://github.com/koreader/koreader/issues/12246#issuecomment-2261334603 function KoboPowerD:_suspendFrontlight() - -- Remember the current frontlight state - -- NOTE: self.is_fl_on flips to false as soon as we engage the ramp down, so, ideally, - -- we'd delay setting self.fl_was_on to the pre-ramp value at the end of the ramp (via the ramp's done_callback), - -- except for the fact that if the frontlight is off, turnOffFrontlight will abort early, so we can't ;). - self.fl_was_on = self.is_fl_on self:turnOffFrontlight() end @@ -482,12 +477,10 @@ function KoboPowerD:beforeSuspend() -- Handle the frontlight last, -- to prevent as many things as we can from interfering with the smoothness of the ramp if self.fl then - -- NOTE: We *cannot* cancel any pending frontlight tasks, - -- because it risks breaking self.fl_was_on tracking... - --[[ + -- We only want the *last* scheduled suspend/resume frontlight task to run to avoid ramps running amok... + UIManager:unschedule(self._suspendFrontlight) UIManager:unschedule(self._resumeFrontlight) self:_stopFrontlightRamp() - --]] -- Turn off the frontlight -- NOTE: Funky delay mainly to yield to the EPDC's refresh on UP systems. @@ -498,7 +491,19 @@ end function KoboPowerD:_resumeFrontlight() -- Don't bother if the light was already off on suspend + -- NOTE: Things gan go sideways quick when you mix the userland ramp, + -- delays all over the place, and quick successions of suspend/resume requests (e.g., jittery sleepcovers), + -- so trust fl_was_on over the actual state on beforeSuspend, + -- as said state might no longer actually represent the pre-suspend reality... + -- c.f., #12246 + -- Note that fl_was_on is updated by *interactive* callers via `BasePowerD:updateResumeFrontlightState` if self.fl_was_on then + -- If the frontlight is currently on because of madness resulting from multiple concurrent suspend/resume requests, + -- but at the wrong intensity, turn it straight off first so that turnOnFrontlight doesn't abort early... + if self.is_fl_on and self.hw_intensity ~= self.fl_intensity then + logger.warn("KoboPowerD:_resumeFrontlight: frontlight intensity is at", self.hw_intensity, "instead of the expected", self.fl_intensity) + self:setIntensityHW(self.fl_min) + end -- Turn the frontlight back on self:turnOnFrontlight() end @@ -517,11 +522,10 @@ function KoboPowerD:afterResume() -- There's a whole bunch of stuff happening before us in Generic:onPowerEvent, -- so we'll delay this ever so slightly so as to appear as smooth as possible... if self.fl then - -- NOTE: Same as above, can't cancel any pending fl tasks. - --[[ + -- Same reasoning as on suspend UIManager:unschedule(self._suspendFrontlight) + UIManager:unschedule(self._resumeFrontlight) self:_stopFrontlightRamp() - --]] -- Turn the frontlight back on -- NOTE: There's quite likely *more* resource contention than on suspend here :/. diff --git a/frontend/ui/widget/frontlightwidget.lua b/frontend/ui/widget/frontlightwidget.lua index f7964f229..576cf53f7 100644 --- a/frontend/ui/widget/frontlightwidget.lua +++ b/frontend/ui/widget/frontlightwidget.lua @@ -512,6 +512,7 @@ function FrontLightWidget:setFrontLightIntensity(intensity) else self.powerd:setIntensity(self.fl.cur) end + self.powerd:updateResumeFrontlightState() -- Retrieve the real level set by PowerD (will be different from `intensity` on toggle) self.fl.cur = self.powerd:frontlightIntensity()