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

Add gesture - poweroff and restart device #5202

Merged
merged 6 commits into from Aug 15, 2019

Conversation

@robert00s
Copy link
Contributor

commented Aug 13, 2019

Ref: #5021

Two new gesture:

  • Power off
  • Reboot the device

@Frenzie Frenzie added this to the 2019.09 milestone Aug 13, 2019

@@ -686,6 +688,8 @@ function ReaderGesture:buildMenu(ges, default)
{"suspend", true},
{"exit", true},
{"restart", not Device:isAndroid()},
{"poweroff", Device:isCervantes() or Device:isKobo() or Device:isSonyPRSTUX()},

This comment has been minimized.

Copy link
@Frenzie

Frenzie Aug 13, 2019

Member

With this being duplicated twice here and also in

if Device:isCervantes() or Device:isKobo() or Device:isSonyPRSTUX() then
maybe it should be abstracted into a Device:canPowerOff() and Device:canReboot() or some such?

This comment has been minimized.

Copy link
@robert00s

robert00s Aug 13, 2019

Author Contributor

Good tip. Thanks.

@Frenzie
Copy link
Member

left a comment

Pinging @poire-z @NiLuJe for a second opinion on the abstraction.

@NiLuJe

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

Sure, it's been used in a couple of places by now ;).

@poire-z

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

Lgtm.
I'm fine with the abstraction, but we may as well add canRestart, which Android can't :)
{"restart", not Device:isAndroid()}, just above the added ones.

(May be in the menu order, PowerOff after Reboot, ordered by bye'ness :) ?)

UIManager:nextTick(UIManager.poweroff_action)
end,
})
elseif action == "reboot" then

This comment has been minimized.

Copy link
@Frenzie

Frenzie Aug 13, 2019

Member

Bit of a nitpick that I didn't think of before @poire-z mentioned it, but the reboot/poweroff order also happens here

@@ -41,6 +41,8 @@ local Cervantes = Generic:new{
hasOTAUpdates = yes,
hasKeys = yes,
hasWifiManager = yes,
canPowerOff = yes,

This comment has been minimized.

Copy link
@Frenzie

Frenzie Aug 13, 2019

Member

and here (and in the other device files)

This comment has been minimized.

Copy link
@robert00s

robert00s Aug 14, 2019

Author Contributor

Done.

@poire-z

This comment has been minimized.

Copy link
Contributor

commented Aug 14, 2019

but we may as well add canRestart, which Android can't :)
{"restart", not Device:isAndroid()}, => {"restart", Device:canRestart()}, just above the added ones.

Can you add that one in this PR, for coherency/completeness sake?

@robert00s

This comment has been minimized.

Copy link
Contributor Author

commented Aug 14, 2019

Yes, soon.

@poire-z

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2019

And some last bits there, and we're done, thanks :)

if Device:isAndroid() then
self.menu_items.exit_menu = self.menu_items.exit
self.menu_items.exit = nil
self.menu_items.restart_koreader = nil
end

if Device:isAndroid() then
self.menu_items.exit_menu = self.menu_items.exit
self.menu_items.exit = nil
self.menu_items.restart_koreader = nil
end

@Frenzie Frenzie merged commit 939538c into koreader:master Aug 15, 2019

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@Frenzie Frenzie referenced this pull request Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.