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

[feat] Update check #4620

Merged
merged 3 commits into from Feb 21, 2019

Conversation

Projects
None yet
2 participants
@Frenzie
Copy link
Member

Frenzie commented Feb 19, 2019

The concept is quite simple: stick a file on the OTA server named
something like koreader-appimage-latest-stable (by analogy with
koreader-cervantes-latest-stable.zsync), which contains nothing
but a filename.

The difference with the zsync update is that the link is then launched
in the user's browser (AppImage) or DownloadManager (Android, not yet
implemented).

[feat] Update check
The concept is quite simple: stick a file on the OTA server named
something like `koreader-appimage-latest-stable` (by analogy with
`koreader-cervantes-latest-stable.zsync`), which contains nothing
but a filename.

The difference with the zsync update is that the link is then launched
in the user's browser (AppImage) or DownloadManager (Android, not yet
implemented).

@Frenzie Frenzie requested a review from pazos Feb 19, 2019

Frenzie added a commit to Frenzie/koreader-misc that referenced this pull request Feb 19, 2019

@Frenzie

This comment has been minimized.

Copy link
Member Author

Frenzie commented Feb 19, 2019

Rationale for Android zsync removal: #4603 (comment)

Setting aside that it's not currently working, I envision the best case scenario something like this:

  1. User downloads OTA update.
  2. Restart to update…
  3. Update says KOReader APK too old, download a new one… (nothing like this is currently implemented, broken or otherwise)
  4. User: 😬 😬 😬
@pazos

This comment has been minimized.

Copy link
Contributor

pazos commented Feb 19, 2019

On android there are a few (probably broken) options.

DownloadManager is available since API9. It will start the download in the background and trigger a nofication (like the one you'll get with aosp browser when you download). The download can be paused, stopped and/or restarted by usual means (ie: using the downloads application on your phone).

The limitation of this method is that we can't easily provide feedback on koreader activity and even when the application is downloaded we can't trigger an installation without the user doing a few clicks.

The other limitation is how to test this feature. Mainly two things.

  1. Do not download more than once the same version of the apk.
  2. Testing will require that installed app and updated one are signed with the same keys.
@Frenzie

This comment has been minimized.

Copy link
Member Author

Frenzie commented Feb 19, 2019

The limitation of this method is that we can't easily provide feedback on koreader activity and even when the application is downloaded we can't trigger an installation without the user doing a few clicks.

Just two clicks. Open file and install. It's just the more streamlined version of going to https://github.com/koreader/koreader/releases or http://build.koreader.rocks/download/nightly/

Testing will require that installed app and updated one are signed with the same keys.

Your localhost will also do just fine as an OTA server. The only "tricky" bit is you'll need your IP within your LAN.

screenshot_2019-02-19_21-27-43

@pazos

This comment has been minimized.

Copy link
Contributor

pazos commented Feb 19, 2019

@Frenzie: okay, after a quick test it works. Didn't try this PR, but modifying frontend;

android-luajit-launcher modifications are, for the moment, on the codacy PR if you wish to test.

--- a/frontend/device/android/device.lua
+++ b/frontend/device/android/device.lua
@@ -109,6 +109,12 @@ function Device:retrieveNetworkInfo()
     end
 end
 
+function Device:update()
+    local test_url = "http://build.koreader.rocks/download/nightly/v2019.02-59-g388871a_2019-02-19/koreader-android-arm-linux-androideabi-v2019.02-59-g388871a_2019-02-19.apk"
+    local test_name = "koreader-lastest.apk"
+    android.download(test_url, test_name)
+end
+

--- a/frontend/ui/elements/common_settings_menu_table.lua
+++ b/frontend/ui/elements/common_settings_menu_table.lua
@@ -153,6 +153,12 @@ if Device:isAndroid() then
             callback = function() require("ui/elements/screen_android"):toggleWakelock() end,
         })
 
+    table.insert(common_settings.screen.sub_item_table,
+        {
+            text = _("Test download"),
+            callback = function() Device:update() end,
+        })
+

My idea is basically feed these two arguments to Device:update (full url to download and name for the downloaded package) from OTAManager if Device:isAndroid().

There is a issue because we can't get the "downloading notification" since we're in fullscreen mode. So I guess we need to show some feedback saying that we started to download the updated package, please check your downloads and install the new package once it is downloaded).

),
ok_text = _("Update"),
ok_callback = function()
if Device:isSDL() then

This comment has been minimized.

@Frenzie

Frenzie Feb 19, 2019

Author Member

@pazos quoting from #4620 (comment)

My idea is basically feed these two arguments to Device:update (full url to download and name for the downloaded package) from OTAManager if Device:isAndroid().

And getting that URL is exactly what this PR is about. Android would just need another two or three lines right here.

This comment has been minimized.

@pazos

pazos Feb 19, 2019

Contributor

yeah, I know, but I have little time right now to test, so I went for the quick & dirty way.

This comment has been minimized.

@Frenzie

Frenzie Feb 19, 2019

Author Member

Oh right, sure.

@Frenzie

This comment has been minimized.

Copy link
Member Author

Frenzie commented Feb 19, 2019

There is a issue because we can't get the "downloading notification" since we're in fullscreen mode.

Is that an issue on older Android? My browser just pops up a notification about the download being finished on screen. Although it occurs to me that my browser's fullscreen doesn't actually hide the statusbar.

@pazos

This comment has been minimized.

Copy link
Contributor

pazos commented Feb 19, 2019

Is that an issue on older Android? My browser just pops up a notification about the download being finished on screen. Although it occurs to me that my browser's fullscreen doesn't actually hide the statusbar.

I don't know, I'm using the emulator (API17) and https://developer.android.com/reference/android/app/DownloadManager.Request.html#VISIBILITY_VISIBLE. In fact the notification is visible on the statusbar during download and stays visible once download is complete, but as you said, the notification is on the statusbar, so it is hidden and we need to show a notification after Device:update.

Probably on API19+ a new download will trigger a visibility change that shows the statusbar for a few seconds, but I didn't test it yet.

@Frenzie

This comment has been minimized.

Copy link
Member Author

Frenzie commented Feb 19, 2019

but as you said, the notification is on the statusbar

I meant that what it calls fullscreen isn't truly fullscreen. The notification pops up in the bottom center (of course there's also an icon for it in the statusbar).

@pazos

This comment has been minimized.

Copy link
Contributor

pazos commented Feb 20, 2019

The notification pops up in the bottom center (of course there's also an icon for it in the statusbar).

We can use toasts and other widgets. I am against bloating the ui with android widgets but in this case comes handy:
untitled

@Frenzie

This comment has been minimized.

Copy link
Member Author

Frenzie commented Feb 20, 2019

Yes, that's exactly what I meant. When it finishes it does the same thing with an "open" button on the right.

@pazos

This comment has been minimized.

Copy link
Contributor

pazos commented Feb 20, 2019

Yes, that's exactly what I meant. When it finishes it does the same thing with an "open" button on the right.

Ok, will try to open it automatically on download complete, since we can't interact with widgets.

@pazos

pazos approved these changes Feb 21, 2019

Copy link
Contributor

pazos left a comment

I like how it looks but I didn't test yet. I would say lgtm and set hasOTAUpdates = no on android for the moment. You know, it is already broken so it shouldn't hurt.

@Frenzie Frenzie merged commit 8e5c1ad into koreader:master Feb 21, 2019

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@Frenzie Frenzie deleted the Frenzie:full-update branch Feb 21, 2019

Frenzie added a commit to koreader/koreader-misc that referenced this pull request Feb 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.