Hold "Wi-Fi" menu to show network selection instead of auto-connecting (#7395)

* Hold "Wi-Fi connection" to show network connection options.
* Honor backend connections (e.g., if wpa_supplicant found a matching AP in its own config first).
* When user clicks "Wi-Fi connection" in menu, only prompt if state is ambiguous.

frontend/device/kobo/device.lua

@@ -388,7 +388,7 @@ function Kobo:initNetworkManager(NetworkMgr)
 
     function NetworkMgr:turnOnWifi(complete_callback)
         koboEnableWifi(true)
-        self:showNetworkMenu(complete_callback)
+        self:reconnectOrShowNetworkMenu(complete_callback)
     end
 
     local net_if = os.getenv("INTERFACE")

frontend/device/sony-prstux/device.lua

@@ -153,7 +153,7 @@ function SonyPRSTUX:initNetworkManager(NetworkMgr)
 
     function NetworkMgr:turnOnWifi(complete_callback)
        os.execute("./set-wifi.sh on")
-       self:showNetworkMenu(complete_callback)
+       self:reconnectOrShowNetworkMenu(complete_callback)
     end
 
     function NetworkMgr:getNetworkInterfaceName()

frontend/ui/network/manager.lua

@@ -112,14 +112,25 @@ function NetworkMgr:releaseIP() end
 function NetworkMgr:restoreWifiAsync() end
 -- End of device specific methods
 
-function NetworkMgr:promptWifiOn(complete_callback)
+function NetworkMgr:toggleWifiOn(complete_callback, long_press)
+    self.wifi_was_on = true
+    G_reader_settings:makeTrue("wifi_was_on")
+    self.wifi_toggle_long_press = long_press
+    self:turnOnWifi(complete_callback)
+end
+
+function NetworkMgr:toggleWifiOff(complete_callback)
+    self.wifi_was_on = false
+    G_reader_settings:makeFalse("wifi_was_on")
+    self:turnOffWifi(complete_callback)
+end
+
+function NetworkMgr:promptWifiOn(complete_callback, long_press)
     UIManager:show(ConfirmBox:new{
         text = _("Do you want to turn on Wi-Fi?"),
         ok_text = _("Turn on"),
         ok_callback = function()
-            self.wifi_was_on = true
+            self:toggleWifiOn(complete_callback, long_press)
-            G_reader_settings:makeTrue("wifi_was_on")
-            self:turnOnWifi(complete_callback)
         end,
     })
 end
@@ -129,27 +140,21 @@ function NetworkMgr:promptWifiOff(complete_callback)
         text = _("Do you want to turn off Wi-Fi?"),
         ok_text = _("Turn off"),
         ok_callback = function()
-            self.wifi_was_on = false
+            self:toggleWifiOff(complete_callback)
-            G_reader_settings:makeFalse("wifi_was_on")
-            self:turnOffWifi(complete_callback)
         end,
     })
 end
 
-function NetworkMgr:promptWifi(complete_callback)
+function NetworkMgr:promptWifi(complete_callback, long_press)
     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"),
         choice1_callback = function()
-            self.wifi_was_on = false
+            self:toggleWifiOff(complete_callback)
-            G_reader_settings:makeFalse("wifi_was_on")
-            self:turnOffWifi(complete_callback)
         end,
         choice2_text = _("Connect"),
         choice2_callback = function()
-            self.wifi_was_on = true
+            self:toggleWifiOn(complete_callback, long_press)
-            G_reader_settings:makeTrue("wifi_was_on")
-            self:turnOnWifi(complete_callback)
         end,
     })
 end
@@ -349,61 +354,67 @@ function NetworkMgr:getWifiMenuTable()
 end
 
 function NetworkMgr:getWifiToggleMenuTable()
-    return {
+    local toggleCallback = function(touchmenu_instance, long_press)
-        text = _("Wi-Fi connection"),
+        local is_wifi_on = NetworkMgr:isWifiOn()
-        enabled_func = function() return Device:hasWifiToggle() end,
+        local is_connected = NetworkMgr:isConnected()
-        checked_func = function() return NetworkMgr:isWifiOn() end,
+        local fully_connected = is_wifi_on and is_connected
-        callback = function(touchmenu_instance)
+        local complete_callback = function()
-            local is_wifi_on = NetworkMgr:isWifiOn()
+            -- Notify TouchMenu to update item check state
-            local is_connected = NetworkMgr:isConnected()
+            touchmenu_instance:updateItems()
-            local fully_connected = is_wifi_on and is_connected
+            -- If Wi-Fi was on when the menu was shown, this means the tap meant to turn the Wi-Fi *off*,
-            local complete_callback = function()
+            -- as such, this callback will only be executed *after* the network has been disconnected.
-                -- Notify TouchMenu to update item check state
+            if fully_connected then
-                touchmenu_instance:updateItems()
+                UIManager:broadcastEvent(Event:new("NetworkDisconnected"))
-                -- If Wi-Fi was on when the menu was shown, this means the tap meant to turn the Wi-Fi *off*,
+            else
-                -- as such, this callback will only be executed *after* the network has been disconnected.
+                -- On hasWifiManager devices that play with kernel modules directly,
-                if fully_connected then
+                -- double-check that the connection attempt was actually successful...
-                    UIManager:broadcastEvent(Event:new("NetworkDisconnected"))
+                if Device:isKobo() or Device:isCervantes() then
-                else
+                    if NetworkMgr:isWifiOn() and NetworkMgr:isConnected() then
-                    -- On hasWifiManager devices that play with kernel modules directly,
-                    -- double-check that the connection attempt was actually successful...
-                    if Device:isKobo() or Device:isCervantes() then
-                        if NetworkMgr:isWifiOn() and NetworkMgr:isConnected() then
-                            UIManager:broadcastEvent(Event:new("NetworkConnected"))
-                        elseif NetworkMgr:isWifiOn() and not NetworkMgr:isConnected() then
-                            -- Don't leave Wi-Fi in an inconsistent state if the connection failed.
-                            -- NOTE: Keep in mind that NetworkSetting only runs this callback on *successful* connections!
-                            --       (It's called connect_callback there).
-                            --       This makes this branch somewhat hard to reach, which is why it gets a dedicated prompt below...
-                            self.wifi_was_on = false
-                            G_reader_settings:makeFalse("wifi_was_on")
-                            -- NOTE: We're limiting this to only a few platforms, as it might be actually harmful on some devices.
-                            --       The intent being to unload kernel modules, and make a subsequent turnOnWifi behave sanely.
-                            --       PB: Relies on netagent, no idea what it does, but it's not using this codepath anyway (!hasWifiToggle)
-                            --       Android: Definitely shouldn't do it.
-                            --       Sony: Doesn't play with modules, don't do it.
-                            --       Kobo: Yes, please.
-                            --       Cervantes: Loads/unloads module, probably could use it like Kobo.
-                            --       Kindle: Probably could use it, if only because leaving Wireless on is generally a terrible idea on Kindle,
-                            --               except that we defer to lipc, which makes Wi-Fi handling asynchronous, and the callback is simply delayed by 1s,
-                            --               so we can't be sure the system will actually have finished bringing Wi-Fi up by then...
-                            NetworkMgr:turnOffWifi()
-                            touchmenu_instance:updateItems()
-                        end
-                    else
-                        -- Assume success on other platforms
                         UIManager:broadcastEvent(Event:new("NetworkConnected"))
+                    elseif NetworkMgr:isWifiOn() and not NetworkMgr:isConnected() then
+                        -- Don't leave Wi-Fi in an inconsistent state if the connection failed.
+                        -- NOTE: Keep in mind that NetworkSetting only runs this callback on *successful* connections!
+                        --       (It's called connect_callback there).
+                        --       This makes this branch somewhat hard to reach, which is why it gets a dedicated prompt below...
+                        self.wifi_was_on = false
+                        G_reader_settings:makeFalse("wifi_was_on")
+                        -- NOTE: We're limiting this to only a few platforms, as it might be actually harmful on some devices.
+                        --       The intent being to unload kernel modules, and make a subsequent turnOnWifi behave sanely.
+                        --       PB: Relies on netagent, no idea what it does, but it's not using this codepath anyway (!hasWifiToggle)
+                        --       Android: Definitely shouldn't do it.
+                        --       Sony: Doesn't play with modules, don't do it.
+                        --       Kobo: Yes, please.
+                        --       Cervantes: Loads/unloads module, probably could use it like Kobo.
+                        --       Kindle: Probably could use it, if only because leaving Wireless on is generally a terrible idea on Kindle,
+                        --               except that we defer to lipc, which makes Wi-Fi handling asynchronous, and the callback is simply delayed by 1s,
+                        --               so we can't be sure the system will actually have finished bringing Wi-Fi up by then...
+                        NetworkMgr:turnOffWifi()
+                        touchmenu_instance:updateItems()
                     end
+                else
+                    -- Assume success on other platforms
+                    UIManager:broadcastEvent(Event:new("NetworkConnected"))
                 end
             end
-            if fully_connected then
-                NetworkMgr:promptWifiOff(complete_callback)
-            elseif is_wifi_on and not is_connected then
-                NetworkMgr:promptWifi(complete_callback)
-            else
-                NetworkMgr:promptWifiOn(complete_callback)
-            end
         end
+        if fully_connected then
+            NetworkMgr:toggleWifiOff(complete_callback)
+        elseif is_wifi_on and not is_connected then
+            -- ask whether user wants to connect or turn off wifi
+            NetworkMgr:promptWifi(complete_callback, long_press)
+        else
+            NetworkMgr:toggleWifiOn(complete_callback, long_press)
+        end
+    end
+
+    return {
+        text = _("Wi-Fi connection"),
+        enabled_func = function() return Device:hasWifiToggle() end,
+        checked_func = function() return NetworkMgr:isWifiOn() end,
+        callback = toggleCallback,
+        hold_callback = function(touchmenu_instance)
+            toggleCallback(touchmenu_instance, true)
+        end,
     }
 end
 
@@ -580,7 +591,7 @@ function NetworkMgr:getMenuTable(common_settings)
     end
 end
 
-function NetworkMgr:showNetworkMenu(complete_callback)
+function NetworkMgr:reconnectOrShowNetworkMenu(complete_callback)
     local info = InfoMessage:new{text = _("Scanning for networks…")}
     UIManager:show(info)
     UIManager:nextTick(function()
@@ -593,38 +604,30 @@ function NetworkMgr:showNetworkMenu(complete_callback)
         -- NOTE: Fairly hackish workaround for #4387,
         --       rescan if the first scan appeared to yield an empty list.
         --- @fixme This *might* be an issue better handled in lj-wpaclient...
-        if (table.getn(network_list) == 0) then
+        if #network_list == 0 then
             network_list, err = self:getNetworkList()
             if network_list == nil then
                 UIManager:show(InfoMessage:new{text = err})
                 return
             end
         end
-        -- 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,
-        })
-    end)
-end
 
-function NetworkMgr:reconnectOrShowNetworkMenu(complete_callback)
-    local info = InfoMessage:new{text = _("Scanning for networks…")}
-    UIManager:show(info)
-    UIManager:nextTick(function()
-        local network_list, err = self:getNetworkList()
-        UIManager:close(info)
-        if network_list == nil then
-            UIManager:show(InfoMessage:new{text = err})
-            return
-        end
         table.sort(network_list,
            function(l, r) return l.signal_quality > r.signal_quality end)
+
         local success = false
-        for dummy, network in ipairs(network_list) do
+        if self.wifi_toggle_long_press then
-            if network.password then
+            self.wifi_toggle_long_press = nil
-                success = NetworkMgr:authenticateNetwork(network)
+        else
+            for dummy, network in ipairs(network_list) do
+                if network.connected then
+                    -- On platforms where we use wpa_supplicant (if we're calling this, we are),
+                    -- the invocation will check its global config, and if an AP configured there is reachable,
+                    -- it'll already have connected to it on its own.
+                    success = true
+                elseif network.password then
+                    success = NetworkMgr:authenticateNetwork(network)
+                end
                 if success then
                     NetworkMgr:obtainIP()
                     if complete_callback then
@@ -639,6 +642,8 @@ function NetworkMgr:reconnectOrShowNetworkMenu(complete_callback)
             end
         end
         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,