From 514635860553472c65d80dd1b63dbb4966fb226a Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Mon, 14 Aug 2023 08:22:22 +0200 Subject: [PATCH] NetworkManager: Allow backends to report connection failures early As early as turnOnWifi. Implement it on hasWifiManager platforms, preventing useless connectivity checks to run when they're obviously never going to work because you're out of range of your AP. Also implemented a flag to notify the backend if the connection attempt was interactive or not. Right now, interactive is extremely restricted, it basically means the menu checkmark, or a gesture. The intent being that for stuff like the beforeWifiAction framework, we don't want the backend to spawn extra UI. Specifically, for hasWifiManager platforms, we no longer spawn the AP scan list on failure unless the caller was interactive. TL;DR: beforeWifiAction is now much less obnoxious when you're obviously not able to connect. --- frontend/device/cervantes/device.lua | 4 +- frontend/device/kobo/device.lua | 4 +- frontend/device/remarkable/device.lua | 6 +- frontend/device/sony-prstux/device.lua | 4 +- frontend/ui/network/manager.lua | 73 +++++++++++++++++++------ frontend/ui/network/networklistener.lua | 2 +- 6 files changed, 64 insertions(+), 29 deletions(-) diff --git a/frontend/device/cervantes/device.lua b/frontend/device/cervantes/device.lua index c879e66f3..f6fdd42f6 100644 --- a/frontend/device/cervantes/device.lua +++ b/frontend/device/cervantes/device.lua @@ -159,10 +159,10 @@ function Cervantes:initNetworkManager(NetworkMgr) complete_callback() end end - function NetworkMgr:turnOnWifi(complete_callback) + function NetworkMgr:turnOnWifi(complete_callback, interactive) logger.info("Cervantes: enabling Wi-Fi") os.execute("./enable-wifi.sh") - self:reconnectOrShowNetworkMenu(complete_callback) + return self:reconnectOrShowNetworkMenu(complete_callback, interactive) end function NetworkMgr:getNetworkInterfaceName() return "eth0" diff --git a/frontend/device/kobo/device.lua b/frontend/device/kobo/device.lua index c89a4a5dc..4ef88f7f7 100644 --- a/frontend/device/kobo/device.lua +++ b/frontend/device/kobo/device.lua @@ -932,9 +932,9 @@ function Kobo:initNetworkManager(NetworkMgr) end end - function NetworkMgr:turnOnWifi(complete_callback) + function NetworkMgr:turnOnWifi(complete_callback, interactive) koboEnableWifi(true) - self:reconnectOrShowNetworkMenu(complete_callback) + return self:reconnectOrShowNetworkMenu(complete_callback, interactive) end local net_if = os.getenv("INTERFACE") or "eth0" diff --git a/frontend/device/remarkable/device.lua b/frontend/device/remarkable/device.lua index d58ceed59..7e15e942e 100644 --- a/frontend/device/remarkable/device.lua +++ b/frontend/device/remarkable/device.lua @@ -218,11 +218,9 @@ end function Remarkable:supportsScreensaver() return true end function Remarkable:initNetworkManager(NetworkMgr) - function NetworkMgr:turnOnWifi(complete_callback) + function NetworkMgr:turnOnWifi(complete_callback, interactive) os.execute("./enable-wifi.sh") - self:reconnectOrShowNetworkMenu(function() - self:connectivityCheck(1, complete_callback) - end) + return self:reconnectOrShowNetworkMenu(complete_callback, interactive) end function NetworkMgr:turnOffWifi(complete_callback) diff --git a/frontend/device/sony-prstux/device.lua b/frontend/device/sony-prstux/device.lua index aa3224f9b..ee26dc9fd 100644 --- a/frontend/device/sony-prstux/device.lua +++ b/frontend/device/sony-prstux/device.lua @@ -147,9 +147,9 @@ function SonyPRSTUX:initNetworkManager(NetworkMgr) end end - function NetworkMgr:turnOnWifi(complete_callback) + function NetworkMgr:turnOnWifi(complete_callback, interactive) os.execute("./set-wifi.sh on") - self:reconnectOrShowNetworkMenu(complete_callback) + return self:reconnectOrShowNetworkMenu(complete_callback, interactive) end function NetworkMgr:getNetworkInterfaceName() diff --git a/frontend/ui/network/manager.lua b/frontend/ui/network/manager.lua index af4e78491..6707af423 100644 --- a/frontend/ui/network/manager.lua +++ b/frontend/ui/network/manager.lua @@ -110,10 +110,13 @@ function NetworkMgr:init() return self end --- Following methods are Device specific which need to be initialized in --- Device:initNetworkManager. Some of them can be set by calling --- NetworkMgr:setWirelessBackend -function NetworkMgr:turnOnWifi(complete_callback) end +-- The following methods are Device specific, and need to be initialized in Device:initNetworkManager. +-- Some of them can be set by calling NetworkMgr:setWirelessBackend +-- NOTE: The interactive flag is set by callers when the toggle was a *direct* user prompt (i.e., Menu or Gesture), +-- as opposed to an indirect one (like the beforeWifiAction framework). +-- It allows the backend to skip UI prompts for non-interactive use-cases. +-- NOTE: May optionally return a boolean, e.g., return false if the backend can guarantee the connection failed. +function NetworkMgr:turnOnWifi(complete_callback, interactive) end function NetworkMgr:turnOffWifi(complete_callback) end -- This function returns the current status of the WiFi radio -- NOTE: On !hasWifiToggle platforms, we assume networking is always available, @@ -232,16 +235,28 @@ function NetworkMgr:ifHasAnAddress() end -- Wrappers around turnOnWifi & turnOffWifi with proper Event signaling -function NetworkMgr:enableWifi(wifi_cb, connectivity_cb, connectivity_widget) +function NetworkMgr:enableWifi(wifi_cb, connectivity_cb, connectivity_widget, interactive) -- Connecting will take a few seconds, broadcast that information so affected modules/plugins can react. UIManager:broadcastEvent(Event:new("NetworkConnecting")) - self:turnOnWifi(wifi_cb) + local status = self:turnOnWifi(wifi_cb, interactive) + -- If turnOnWifi failed, abort early + if status == false then + -- And cancel any pending connectivity check, because it wouldn't achieve anything + if self.pending_connectivity_check then + UIManager:unschedule(self.connectivityCheck) + self.pending_connectivity_check = false + end + logger.warn("NetworkMgr:enableWifi: Connection failed!") + return false + end -- Some turnOnWifi implementations may already have fired a connectivity check... if not self.pending_connectivity_check then -- This will handle sending the proper Event, manage wifi_was_on, as well as tearing down Wi-Fi in case of failures. self:scheduleConnectivityCheck(connectivity_cb, connectivity_widget) end + + return true end function NetworkMgr:disableWifi(cb) @@ -255,7 +270,7 @@ function NetworkMgr:disableWifi(cb) self:turnOffWifi(complete_callback) end -function NetworkMgr:toggleWifiOn(complete_callback, long_press) +function NetworkMgr:toggleWifiOn(complete_callback, long_press, interactive) local toggle_im = InfoMessage:new{ text = _("Turning on Wi-Fi…"), } @@ -266,7 +281,7 @@ function NetworkMgr:toggleWifiOn(complete_callback, long_press) G_reader_settings:makeTrue("wifi_was_on") self.wifi_toggle_long_press = long_press - self:enableWifi(complete_callback) + self:enableWifi(complete_callback, nil, nil, interactive) UIManager:close(toggle_im) end @@ -286,6 +301,7 @@ function NetworkMgr:toggleWifiOff(complete_callback) UIManager:close(toggle_im) end +-- NOTE: Only used by the beforeWifiAction framework, so, can never be flagged as "interactive" ;). function NetworkMgr:promptWifiOn(complete_callback, long_press) UIManager:show(ConfirmBox:new{ text = _("Do you want to turn on Wi-Fi?"), @@ -306,7 +322,7 @@ function NetworkMgr:promptWifiOff(complete_callback) }) end -function NetworkMgr:promptWifi(complete_callback, long_press) +function NetworkMgr:promptWifi(complete_callback, long_press, interactive) UIManager:show(MultiConfirmBox:new{ text = _("Wi-Fi is enabled, but you're currently not connected to a network.\nHow would you like to proceed?"), choice1_text = _("Turn Wi-Fi off"), @@ -315,7 +331,7 @@ function NetworkMgr:promptWifi(complete_callback, long_press) end, choice2_text = _("Connect"), choice2_callback = function() - self:toggleWifiOn(complete_callback, long_press) + self:toggleWifiOn(complete_callback, long_press, interactive) end, }) end @@ -338,13 +354,22 @@ function NetworkMgr:turnOnWifiAndWaitForConnection(callback) -- This is a slightly tweaked variant of enableWifi, because of our peculiar connectivityCheck usage... UIManager:broadcastEvent(Event:new("NetworkConnecting")) - self:turnOnWifi() + -- Some implementations (usually, hasWifiManager) can report whether they were successfull + local status = self:turnOnWifi() -- Some implementations may fire a connectivity check, -- but we *need* our own, because of the callback & widget passing. if self.pending_connectivity_check then UIManager:unschedule(self.connectivityCheck) self.pending_connectivity_check = false end + + -- If turnOnWifi failed, abort early + if status == false then + logger.warn("NetworkMgr:turnOnWifiAndWaitForConnection: Connection failed!") + UIManager:close(info) + return false + end + self:scheduleConnectivityCheck(callback, info) return info @@ -529,9 +554,16 @@ function NetworkMgr:goOnlineToRun(callback) -- In case we abort before the beforeWifiAction, we won't pass it the callback, but run it ourselves, -- to avoid it firing too late (or at the very least being pinned for too long). local info = self:beforeWifiAction() + -- We'll basically do the same but in a blocking manner... UIManager:unschedule(self.connectivityCheck) self.pending_connectivity_check = false + + -- If connecting just plain failed, we're done + if info == false then + return false + end + -- Throw in a connectivity check now, for the sake of hasWifiManager platforms, -- where we manage Wi-Fi ourselves, meaning turnOnWifi, and as such beforeWifiAction, -- is *blocking*, so if all went well, we'll already have blocked a while, @@ -646,9 +678,9 @@ function NetworkMgr:getWifiToggleMenuTable() self:toggleWifiOff(complete_callback) elseif self.is_wifi_on and not self.is_connected then -- ask whether user wants to connect or turn off wifi - self:promptWifi(complete_callback, long_press) + self:promptWifi(complete_callback, long_press, true) else -- if not connected at all - self:toggleWifiOn(complete_callback, long_press) + self:toggleWifiOn(complete_callback, long_press, true) end end -- toggleCallback() @@ -833,7 +865,7 @@ function NetworkMgr:getMenuTable(common_settings) end end -function NetworkMgr:reconnectOrShowNetworkMenu(complete_callback) +function NetworkMgr:reconnectOrShowNetworkMenu(complete_callback, interactive) local info = InfoMessage:new{text = _("Scanning for networks…")} UIManager:show(info) UIManager:forceRePaint() @@ -915,11 +947,16 @@ function NetworkMgr:reconnectOrShowNetworkMenu(complete_callback) if not success then -- NOTE: Also supports a disconnect_callback, should we use it for something? -- Tearing down Wi-Fi completely when tapping "disconnect" would feel a bit harsh, though... - UIManager:show(require("ui/widget/networksetting"):new{ - network_list = network_list, - connect_callback = complete_callback, - }) + if interactive then + -- We don't want this for non-interactive callers, like the beforeWifiAction framework... + UIManager:show(require("ui/widget/networksetting"):new{ + network_list = network_list, + connect_callback = complete_callback, + }) + end end + + return success end function NetworkMgr:saveNetwork(setting) diff --git a/frontend/ui/network/networklistener.lua b/frontend/ui/network/networklistener.lua index 473885366..cf783683a 100644 --- a/frontend/ui/network/networklistener.lua +++ b/frontend/ui/network/networklistener.lua @@ -26,7 +26,7 @@ local function enableWifi() -- NB Normal widgets should use NetworkMgr:promptWifiOn() -- (or, better yet, the NetworkMgr:beforeWifiAction wrappers: NetworkMgr:runWhenOnline() & co.) -- This is specifically the toggle Wi-Fi action, so consent is implied. - NetworkMgr:enableWifi() + NetworkMgr:enableWifi(nil, nil, nil, true) -- flag it as interactive, though UIManager:close(toggle_im) end