WifiManager: single source for known connections (#37262)

* temp

* rev

* reproduce race condition where connection removed signal takes a while to remove, then update networks keep is_saved true

* fix

* Revert "reproduce race condition where connection removed signal takes a while to remove, then update networks keep is_saved true"

This reverts commit cf7044ee955777db16434ab81c520bbe798c9164.

* not anymore

* more clear

* safe guards

nl

selfdrive/ui/mici/layouts/settings/network/wifi_ui.py

@@ -263,7 +263,7 @@ class NetworkInfoPage(NavWidget):
     if self._network is None:
       return
 
-    self._connect_btn.set_full(not self._network.is_saved and not self._is_connecting)
+    self._connect_btn.set_full(not self._wifi_manager.is_connection_saved(self._network.ssid) and not self._is_connecting)
     if self._is_connecting:
       self._connect_btn.set_label("connecting...")
       self._connect_btn.set_enabled(False)
@@ -425,7 +425,7 @@ class WifiUIMici(BigMultiOptionDialog):
       cloudlog.warning(f"Trying to connect to unknown network: {ssid}")
       return
 
-    if network.is_saved:
+    if self._wifi_manager.is_connection_saved(network.ssid):
       self._wifi_manager.activate_connection(network.ssid)
       self._update_buttons()
     elif network.security_type == SecurityType.OPEN:

system/ui/lib/wifi_manager.py

@@ -89,10 +89,9 @@ class Network:
   ssid: str
   strength: int
   security_type: SecurityType
-  is_saved: bool
 
   @classmethod
-  def from_dbus(cls, ssid: str, aps: list["AccessPoint"], is_saved: bool) -> "Network":
+  def from_dbus(cls, ssid: str, aps: list["AccessPoint"]) -> "Network":
     # we only want to show the strongest AP for each Network/SSID
     strongest_ap = max(aps, key=lambda ap: ap.strength)
     security_type = get_security_type(strongest_ap.flags, strongest_ap.wpa_flags, strongest_ap.rsn_flags)
@@ -101,7 +100,6 @@ class Network:
       ssid=ssid,
       strength=strongest_ap.strength,
       security_type=security_type,
-      is_saved=is_saved,
     )
 
 
@@ -620,8 +618,6 @@ class WifiManager:
         conn_addr = DBusAddress(conn_path, bus_name=NM, interface=NM_CONNECTION_IFACE)
         self._router_main.send_and_get_reply(new_method_call(conn_addr, 'Delete'))
 
-      # FIXME: race here where ConnectionRemoved signal may arrive after we update all Network is_saved
-      #  and keep the old ssid's is_saved=True
       self._update_networks()
       self._enqueue_callbacks(self._forgotten, ssid)
 
@@ -635,13 +631,17 @@ class WifiManager:
 
     def worker():
       conn_path = self._connections.get(ssid, None)
-      if conn_path is not None:
-        if self._wifi_device is None:
-          cloudlog.warning("No WiFi device found")
-          return
+      if conn_path is None or self._wifi_device is None:
+        cloudlog.warning(f"Failed to activate connection for {ssid}: conn_path={conn_path}, wifi_device={self._wifi_device}")
+        self._set_connecting(None)
+        return
 
-        self._router_main.send(new_method_call(self._nm, 'ActivateConnection', 'ooo',
-                                               (conn_path, self._wifi_device, "/")))
+      reply = self._router_main.send_and_get_reply(new_method_call(self._nm, 'ActivateConnection', 'ooo',
+                                                                   (conn_path, self._wifi_device, "/")))
+
+      if reply.header.message_type == MessageType.error:
+        cloudlog.warning(f"Failed to activate connection for {ssid}: {reply}")
+        self._set_connecting(None)
 
     if block:
       worker()
@@ -665,6 +665,9 @@ class WifiManager:
     # Check ssid, not connected_ssid, to also catch connecting state
     return self._wifi_state.ssid == self._tethering_ssid
 
+  def is_connection_saved(self, ssid: str) -> bool:
+    return ssid in self._connections
+
   def set_tethering_password(self, password: str):
     def worker():
       conn_path = self._connections.get(self._tethering_ssid, None)
@@ -804,8 +807,8 @@ class WifiManager:
             # catch all for parsing errors
             cloudlog.exception(f"Failed to parse AP properties for {ap_path}")
 
-        networks = [Network.from_dbus(ssid, ap_list, ssid in self._connections) for ssid, ap_list in aps.items()]
-        networks.sort(key=lambda n: (n.ssid != self._wifi_state.ssid, -n.is_saved, -n.strength, n.ssid.lower()))
+        networks = [Network.from_dbus(ssid, ap_list) for ssid, ap_list in aps.items()]
+        networks.sort(key=lambda n: (n.ssid != self._wifi_state.ssid, not self.is_connection_saved(n.ssid), -n.strength, n.ssid.lower()))
         self._networks = networks
 
         self._update_active_connection_info()

system/ui/widgets/network.py

@@ -383,7 +383,7 @@ class WifiManagerUI(Widget):
       gui_label(status_text_rect, status_text, font_size=48, alignment=rl.GuiTextAlignment.TEXT_ALIGN_CENTER)
     else:
       # If the network is saved, show the "Forget" button
-      if network.is_saved:
+      if self._wifi_manager.is_connection_saved(network.ssid):
         forget_btn_rect = rl.Rectangle(
           security_icon_rect.x - self.btn_width - spacing,
           rect.y + (ITEM_HEIGHT - 80) / 2,
@@ -396,7 +396,7 @@ class WifiManagerUI(Widget):
     self._draw_signal_strength_icon(signal_icon_rect, network)
 
   def _networks_buttons_callback(self, network):
-    if not network.is_saved and network.security_type != SecurityType.OPEN:
+    if not self._wifi_manager.is_connection_saved(network.ssid) and network.security_type != SecurityType.OPEN:
       self.state = UIState.NEEDS_AUTH
       self._state_network = network
       self._password_retry = False
@@ -432,7 +432,7 @@ class WifiManagerUI(Widget):
   def connect_to_network(self, network: Network, password=''):
     self.state = UIState.CONNECTING
     self._state_network = network
-    if network.is_saved and not password:
+    if self._wifi_manager.is_connection_saved(network.ssid) and not password:
       self._wifi_manager.activate_connection(network.ssid)
     else:
       self._wifi_manager.connect_to_network(network.ssid, password)