-
Notifications
You must be signed in to change notification settings - Fork 85
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
Android detects VPN (wireguard) connection as WI-FI enabled #291
Comments
See android-luajit-launcher/app/src/org/koreader/launcher/BaseActivity.kt Lines 338 to 402 in e3d58c6
It is expected. VPN will prevail over wifi (ie: if both are active). Maybe if connection type is VPN do aditional online check?. I wouldn't want to remove VPN as a valid network type. I use gnirehtet a lot (which is recongized as VPN network). |
Perhaps it's enough to just not android-luajit-launcher/app/src/org/koreader/launcher/BaseActivity.kt Lines 364 to 367 in e3d58c6
android-luajit-launcher/app/src/org/koreader/launcher/BaseActivity.kt Lines 392 to 395 in e3d58c6
I'm not actually too clear on how the when works when multiple conditions are true, but my idea is to not change "connected" state with VPN connection, because user being connected to VPN doesn't imply internet connection. Hope it makes sense
|
@Galunid: the real problem is android != all the other platforms. On embedded linux devices network == wifi On android network means a lot of things and VPN is a valid kind of network. You might want to try --- a/frontend/device/android/device.lua
+++ b/frontend/device/android/device.lua
@@ -259,9 +259,10 @@ function Device:initNetworkManager(NetworkMgr)
end
function NetworkMgr:isWifiOn()
- local ok = android.getNetworkInfo()
+ local ok, transport = android.getNetworkInfo()
ok = tonumber(ok)
if not ok then return false end
+ if transport == C.ANETWORK_VPN then return false end
return ok == 1
end
end A snippet like that is cool for own usage, but I don't like it as is even more broken than before :) Hence my comment
|
Of course the proper implementation would be function NetworkMgr:isWifiOn()
local ok, transport = android.getNetworkInfo()
ok = tonumber(ok)
if not ok then return false end
return ok == 1 and transport == C.ANETWORK_WIFI
end But that will break wikipedia over mobile network, among other things. |
@pazos EDIT: Still doesn't work with KOSync... |
There's a good reason for that: an online check will, by definition, be blocking. (Remember gitter? ;p) IIRC, it also doesn't actually check anything most of the time, because it can be triggered approximately seven billion different ways. The one thing it tries to rely on (the |
It makes sense, except it doesn't. Since if network is offline, KOSync starts doing network stuff that's blocking (at least on android and kobo) and hangs device for ~30 seconds (till request timeouts). Am I missing something? |
IIRC, it shouldn't happen on Kobo, at least in most cases. But, like I said, this thing is full of corner cases, so, who knows ;). (Also, I don't actually use it, so I have no first hand experience with it). |
It happens on Kobo when autosync is enabled and it's pretty random I didn't really have trouble when I triggered it manually. On android it causes ANR (doesn't matter if triggered manually or automatically). |
Partially cool. If active network is VPN then see in all networks if has a different transport (wifi, ethernet, bluetooth) and return that as the active network. In any cases that function should return true if an active network is found. Anything that changes the logic belongs to the frontend
Still unimplemented. See koreader/koreader#6438 (comment) and koreader/koreader#6438 (comment)
What causes ANR is a input event not consumed in 5 seconds. There's a workaround for that but I just implemented it for ANRs I found. See koreader/koreader#6266 |
Something like that might help, but it'll probably also break stuff in fun and interesting ways in some cases? (I have a vague memory of someone trying something like that and failing horribly, but I don't recall the details...). diff --git a/plugins/kosync.koplugin/main.lua b/plugins/kosync.koplugin/main.lua
index 6fcf4744..b56c0d59 100644
--- a/plugins/kosync.koplugin/main.lua
+++ b/plugins/kosync.koplugin/main.lua
@@ -494,6 +494,10 @@ function KOSync:updateProgress(manual)
return
end
+ if NetworkMgr:willRerunWhenOnline(function() self:updateProgress(manual) end) then
+ return
+ end
+
local KOSyncClient = require("KOSyncClient")
local client = KOSyncClient:new{
custom_url = self.kosync_custom_server,
@@ -538,6 +542,10 @@ function KOSync:getProgress(manual)
return
end
+ if NetworkMgr:willRerunWhenOnline(function() self:getProgress(manual) end) then
+ return
+ end
+
local KOSyncClient = require("KOSyncClient")
local client = KOSyncClient:new{
custom_url = self.kosync_custom_server, |
@NiLuJe It was me in koreader/koreader#6489, which resulted in koreader/koreader#6535 ;-) diff --git a/plugins/kosync.koplugin/main.lua b/plugins/kosync.koplugin/main.lua
index 6fcf4744..b56c0d59 100644
--- a/plugins/kosync.koplugin/main.lua
+++ b/plugins/kosync.koplugin/main.lua
@@ -494,6 +494,10 @@ function KOSync:updateProgress(manual)
return
end
+ if manual and NetworkMgr:willRerunWhenOnline(function() self:updateProgress(manual) end) then
+ return
+ end
+
local KOSyncClient = require("KOSyncClient")
local client = KOSyncClient:new{
custom_url = self.kosync_custom_server,
@@ -538,6 +542,10 @@ function KOSync:getProgress(manual)
return
end
+ if manual and NetworkMgr:willRerunWhenOnline(function() self:getProgress(manual) end) then
+ return
+ end
+
local KOSyncClient = require("KOSyncClient")
local client = KOSyncClient:new{
custom_url = self.kosync_custom_server, |
Right, that'd be a start ;). That, and allowing to tweak the period like the TODO says ;p. |
@pazos ANR is still present in KOSync, not sure if you're interested in fixing that, but what koreader/koreader#6266 did was fix ANR in login/register functions, the more important ones getProgress/updateProgress, don't have your safeguard. I'm assume that's intentional to prevent reader from suddenly becoming unresponsive.
I don't really think it's possible to do this on frontend, since android will return the network type VPN, and it's not possible to check if there actually is an internet connection, so I'd be stuck with detecting VPN as either connected or disconnected, which is pretty much out of question, since I have VPN set as
to return different value if connected to VPN and online, vs connected to VPN offline, but that's probably less than ideal. Anyway, let me know your thoughts, feel free to close this issue, if you don't want to bother with this, since it probably doesn't affect too many people. |
Mmm. Nobody faced that issue. The safeguards were added to prevent koreader/koreader#6237. I would suggest to focus now on your original issue with VPNs here. Other issues with KoSync can be fixed later.
Use a g_setting called "vpn_alone_is_a_valid_network" paired with https://github.com/koreader/koreader/issues/6726#issuecomment-700340138. So the backend network info will report true, VPN and the frontend will decide if isWifiOn based on Gsettings. If you make the changes in the backend I can fix the frontend, if you want. So, the backend will return: on wifi (with or without VPN): true, WIFI and the frontend will do: if vpn_is_valid_network then |
Issue
When wifi is disabled, and device has VPN (in my case wireguard) enabled KOReader detects wifi as enabled. It's problematic because KOSync plugin/Calibre plugin have to timeout if triggered in this state.
Steps to reproduce
Connect to wireguard -> Disconnect wifi -> KOReader still detects wifi enabled (because VPN connection is still active)
The text was updated successfully, but these errors were encountered: