Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow transitioning from *sleep* to *power off* #2431

Closed
baskerville opened this issue Dec 16, 2016 · 15 comments · Fixed by #5280
Closed

Allow transitioning from *sleep* to *power off* #2431

baskerville opened this issue Dec 16, 2016 · 15 comments · Fixed by #5280
Milestone

Comments

@baskerville
Copy link
Contributor

Some users might put the device to sleep by mistake and then try to power it off directly.

This will currently fail.

@robert00s
Copy link
Contributor

I think this is done by @poire-z in #3572. You can set deferent settings for sleep and power off.

@poire-z
Copy link
Contributor

poire-z commented Jul 18, 2018

Not sure this issue is about screensaver.
I guess it's about, after having put the device to sleep with a short press on the power button, to be able to power it off with a 3-seconds press.
Currently, one have to short press again to wake it up, and the long-press it to power it off (which does not sounds too complicated to me :)

@robert00s
Copy link
Contributor

@poire-z But can it be handled by KOReader?
@baskerville What did you mean exactly?

@poire-z
Copy link
Contributor

poire-z commented Jul 19, 2018

On Kobo, it is handled by KOreader:

if Device:isKobo() then
-- We do not want auto suspend procedure to waste battery during
-- suspend. So let's unschedule it when suspending, and restart it after
-- resume.
self.event_handlers["Suspend"] = function()
self:_beforeSuspend()
Device:onPowerEvent("Suspend")
end
self.event_handlers["Resume"] = function()
Device:onPowerEvent("Resume")
self:_afterResume()
end
self.event_handlers["PowerPress"] = function()
UIManager:scheduleIn(2, self.poweroff_action)
end
self.event_handlers["PowerRelease"] = function()
if not self._entered_poweroff_stage then
UIManager:unschedule(self.poweroff_action)
self:suspend()
end
end

self.poweroff_action = function()
self._entered_poweroff_stage = true;
Screen:setRotationMode(0)
require("ui/screensaver"):show("poweroff", _("Powered off"))
Screen:refreshFull()
UIManager:nextTick(function()
Device:saveSettings()
self:broadcastEvent(Event:new("Close"))
Device:powerOff()
end)
end

So, solving this issue would need some trickery around these (but I won't try it :)

@robert00s
Copy link
Contributor

Ok :) I don't have Kobo so I can't help.

@Frenzie Frenzie added wontfix PR are welcomed and removed need more info wontfix PR are welcomed labels Aug 29, 2019
@Frenzie Frenzie added this to the 2019.09 milestone Aug 29, 2019
Frenzie added a commit to Frenzie/koreader that referenced this issue Aug 29, 2019
This allows to shutdown straight from suspend just like Nickel.

Fixes <koreader#2431>.
Frenzie added a commit that referenced this issue Aug 29, 2019
This allows to shutdown straight from suspend just like Nickel.

Fixes <#2431>.
@avsej
Copy link
Contributor

avsej commented Sep 1, 2019

@Frenzie @poire-z @pazos This patch breaks power button on Cervantes! Is it possible to keep kobo-specific fixes separately?

@Frenzie
Copy link
Member

Frenzie commented Sep 1, 2019

@avsej Could you be a bit more specific? This is Kobo-specific code.

Frenzie added a commit to Frenzie/koreader that referenced this issue Sep 1, 2019
This reverts commit cfa73be.

Cervantes and Sony secretly depend on hacky Kobo code.
Reported by @avsej, see <koreader#2431 (comment)>.

Closes <koreader#5292>.
@NiLuJe
Copy link
Member

NiLuJe commented Sep 1, 2019

@Frenzie : From a very quick glance, I think the Cervantes/Sony blocks in uimanager rely on the "Resume" event, which went poof with this ;).

@Frenzie
Copy link
Member

Frenzie commented Sep 1, 2019

Indeed. We've got a case of confusing design or just a quick hack from years ago spreading out like an oil spill here. :-P

@pazos
Copy link
Member

pazos commented Sep 1, 2019

We've got a case of confusing design or just a quick hack from years ago spreading out like an oil spill here. :-P

Indeed. I might be still confused now as I don't understand why this code path should be reachable only for kobos.

@Frenzie
Copy link
Member

Frenzie commented Sep 1, 2019

The split in Press/Release is fine and generic enough in theory, but deciding that "Resume" is the correct action to take based on that input event is UIManager logic infiltrating in the Input module.

Back a few years ago this lack of separation was "Kobo only" and I just hadn't quite caught up to that fact. ;-)

@avsej
Copy link
Contributor

avsej commented Sep 1, 2019

Could you be a bit more specific?

My Cervantes 4, unable to resume after suspend with this patch. I had to reboot it. But even after reboot it does not wake up after suspend. While the change is kobo-specific, the fix goes into common code path, and affect other devices.

@Frenzie
Copy link
Member

Frenzie commented Sep 1, 2019

The change isn't Kobo-specific exactly. The common code path should definitely look the other way architecturally speaking. ;-)

Anyway, thanks for reporting!

@pazos
Copy link
Member

pazos commented Sep 1, 2019

Uh-oh. I think that I understand now: instead of returning "Resume" from input.lua return "PowerPress | PowerRelease" and let UiManager handle that?

Frenzie added a commit to Frenzie/koreader that referenced this issue Sep 1, 2019
Frenzie added a commit to Frenzie/koreader that referenced this issue Sep 1, 2019
@Frenzie
Copy link
Member

Frenzie commented Sep 1, 2019

Right, Input incorporates a couple of hacks related to converting keyboard combos to events but in principle it should just make sure input is turned into something that's digestible by other parts of the program. UIManager handles suspend/resume/shutdown with the aid of Device-specific implementations.

The original hack from b86aa5a made a lot more sense than the spaghetti monster we ended up with in subsequent rewrites:

if ev.value == EVENT_VALUE_KEY_RELEASE then
        if keycode == "Light" then
            return keycode
        elseif keycode == "Power" then
            -- Kobo generates Power keycode only, we need to decide whether it's
            -- power-on or power-off ourselves.
            if self.device.screen_saver_mode then
                return "Resume"
            else
                return "Suspend"
            end
        end
    end

It's still breaking the abstraction in a way I'd prefer to avoid in Input, but it does so sensibly.

Frenzie added a commit that referenced this issue Sep 1, 2019
mwoz123 pushed a commit to mwoz123/koreader that referenced this issue Mar 29, 2020
This allows to shutdown straight from suspend just like Nickel.

Fixes <koreader#2431>.
mwoz123 pushed a commit to mwoz123/koreader that referenced this issue Mar 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants