Wifi improvements (#2618)

We now detect wrong passwrods and request the correct password from the user. We also write "CONNECTING" if we are actively connecting to a network.

For a tiny usability upgrade, we now display "Refreshing networks" text, while loading the wifi networks available around us.
old-commit-hash: 7d37793434

selfdrive/ui/qt/offroad/wifi.cc

@@ -22,6 +22,7 @@ void clearLayout(QLayout* layout) {
 
 WifiUI::WifiUI(QWidget *parent) : QWidget(parent) {
   wifi = new WifiManager;
+  QObject::connect(wifi, SIGNAL(wrongPassword(QString)), this, SLOT(wrongPassword(QString)));
 
   QVBoxLayout * top_layout = new QVBoxLayout;
   swidget = new QStackedWidget;
@@ -46,15 +47,17 @@ WifiUI::WifiUI(QWidget *parent) : QWidget(parent) {
     }
   )");
 
-  // TODO: implement (not) connecting with wrong password
-
   // Update network list
   timer = new QTimer(this);
   QObject::connect(timer, SIGNAL(timeout()), this, SLOT(refresh()));
-  timer->start(400);
+  timer->start(2000);
 
   // Scan on startup
   wifi->request_scan();
+  QLabel* scanning = new QLabel(this);
+  scanning->setText("Scanning for networks");
+  vlayout->addWidget(scanning);
+  refresh();
   page = 0;
 }
 
@@ -74,7 +77,6 @@ void WifiUI::refresh() {
   int i = 0;
   for (Network &network : wifi->seen_networks){
     QHBoxLayout *hlayout = new QHBoxLayout;
-
     if(page * networks_per_page <= i && i < (page + 1) * networks_per_page){
       // SSID
       hlayout->addSpacing(50);
@@ -90,9 +92,9 @@ void WifiUI::refresh() {
       hlayout->addSpacing(20);
 
       // connect button
-      QPushButton* btn = new QPushButton(network.connected ? "Connected" : "Connect");
+      QPushButton* btn = new QPushButton(network.connected == ConnectedType::CONNECTED ? "Connected" : (network.connected == ConnectedType::CONNECTING ? "Connecting" : "Connect"));
       btn->setFixedWidth(300);
-      btn->setDisabled(network.connected || network.security_type == SecurityType::UNSUPPORTED);
+      btn->setDisabled(network.connected == ConnectedType::CONNECTED || network.connected == ConnectedType::CONNECTING || network.security_type == SecurityType::UNSUPPORTED);
       hlayout->addWidget(btn);
       hlayout->addSpacing(20);
 
@@ -154,11 +156,14 @@ void WifiUI::refresh() {
 
 void WifiUI::handleButton(QAbstractButton* button) {
   QPushButton* btn = static_cast<QPushButton*>(button);
-  qDebug() << connectButtons->id(btn);
   Network n = wifi->seen_networks[connectButtons->id(btn)];
 
   a->label->setText("Enter password for \"" + n.ssid  + "\"");
+  connectToNetwork(n);
+}
 
+void WifiUI::connectToNetwork(Network n){
+  timer->stop();
   if(n.security_type == SecurityType::OPEN){
     wifi->connect(n);
   } else if (n.security_type == SecurityType::WPA){
@@ -166,9 +171,9 @@ void WifiUI::handleButton(QAbstractButton* button) {
     if(password.size()){
       wifi->connect(n, password);
     }
-  } else {
-    qDebug() << "Cannot determine network's security type";
   }
+  refresh();
+  timer->start();
 }
 
 QString WifiUI::getStringFromUser(){
@@ -182,6 +187,20 @@ void WifiUI::receiveText(QString t) {
   loop.quit();
   text = t;
 }
+
+
+void WifiUI::wrongPassword(QString ssid){
+  if(loop.isRunning()){
+    return;
+  }
+  for(Network n : wifi->seen_networks){
+    if(n.ssid == ssid){
+      a->label->setText("Wrong password for \"" + n.ssid +"\"");
+      connectToNetwork(n);
+    }
+  }
+}
+
 void WifiUI::prevPage() {
   page--;
   refresh();

selfdrive/ui/qt/offroad/wifi.hpp

@@ -28,6 +28,7 @@ private:
   QString text;
   QButtonGroup *connectButtons;
 
+  void connectToNetwork(Network n);
   QString getStringFromUser();
 
 public:
@@ -37,6 +38,8 @@ private slots:
   void handleButton(QAbstractButton* m_button);
   void refresh();
   void receiveText(QString text);
+  void wrongPassword(QString ssid);
+
   void prevPage();
   void nextPage();
 };

selfdrive/ui/qt/offroad/wifiManager.cc

@@ -1,9 +1,11 @@
 #include <algorithm>
 #include <set>
-
 #include "wifiManager.hpp"
 
-
+/**
+ * We are using a NetworkManager DBUS API : https://developer.gnome.org/NetworkManager/1.26/spec.html
+ * */
+ 
 QString nm_path                = "/org/freedesktop/NetworkManager";
 QString nm_settings_path       = "/org/freedesktop/NetworkManager/Settings";
 
@@ -18,6 +20,9 @@ QString connection_iface       = "org.freedesktop.NetworkManager.Connection.Acti
 
 QString nm_service             = "org.freedesktop.NetworkManager";
 
+const int state_connected = 100;
+const int state_need_auth = 60;
+const int reason_wrong_password = 8;
 
 template <typename T>
 T get_response(QDBusMessage response){
@@ -28,16 +33,27 @@ T get_response(QDBusMessage response){
 }
 
 bool compare_by_strength(const Network &a, const Network &b){
-  if (a.connected) return true;
+  if (a.connected == ConnectedType::CONNECTED) return true;
-  if (b.connected) return false;
+  if (b.connected == ConnectedType::CONNECTED) return false;
+  if (a.connected == ConnectedType::CONNECTING) return true;
+  if (b.connected == ConnectedType::CONNECTING) return false;
   return a.strength > b.strength;
 }
 
 WifiManager::WifiManager(){
   qDBusRegisterMetaType<Connection>();
-
+  connecting_to_network = "";
   adapter = get_adapter();
   has_adapter = adapter != "";
+  if(has_adapter){
+    QDBusInterface nm(nm_service, adapter, device_iface, bus);
+    bus.connect(nm_service, adapter, device_iface, "StateChanged", this, SLOT(change(unsigned int, unsigned int, unsigned int)));
+    
+    QDBusInterface device_props(nm_service, adapter, props_iface, bus);
+    QDBusMessage response = device_props.call("Get", device_iface, "State");
+    raw_adapter_state = get_response<uint>(response);
+    change(raw_adapter_state, 0, 0);
+  }
 }
 
 void WifiManager::refreshNetworks(){
@@ -63,7 +79,6 @@ QList<Network> WifiManager::get_networks(){
   QVariant first =  response.arguments().at(0);
 
   QString active_ap = get_active_ap();
-
   const QDBusArgument &args = first.value<QDBusArgument>();
   args.beginArray();
   while (!args.atEnd()) {
@@ -73,7 +88,17 @@ QList<Network> WifiManager::get_networks(){
     QByteArray ssid = get_property(path.path(), "Ssid");
     unsigned int strength = get_ap_strength(path.path());
     SecurityType security = getSecurityType(path.path());
-    Network network = {path.path(), ssid, strength, path.path()==active_ap, security};
+    ConnectedType ctype;
+    if(path.path() != active_ap){
+      ctype = ConnectedType::DISCONNECTED;
+    }else{
+      if(ssid == connecting_to_network){
+        ctype = ConnectedType::CONNECTING;
+      }else{
+        ctype = ConnectedType::CONNECTED;
+      }
+    }
+    Network network = {path.path(), ssid, strength, ctype, security};
 
     if (ssid.length()){
       r.push_back(network);
@@ -96,10 +121,6 @@ SecurityType WifiManager::getSecurityType(QString path){
   } else if((sflag & 0x1) && (wpa_props & (0x333) && !(wpa_props & 0x200))) {
     return SecurityType::WPA;
   } else {
-    // qDebug() << "Cannot determine security type for " << get_property(path, "Ssid") << " with flags";
-    // qDebug() << "flag    " << sflag;
-    // qDebug() << "WpaFlag " << wpaflag;
-    // qDebug() << "RsnFlag " << rsnflag;
     return SecurityType::UNSUPPORTED;
   }
 }
@@ -113,12 +134,12 @@ void WifiManager::connect(Network n, QString password){
 }
 
 void WifiManager::connect(Network n, QString username, QString password){
+  connecting_to_network = n.ssid;
   QString active_ap = get_active_ap();
-  if (active_ap!="") {
+  if(active_ap!="" && active_ap!="/"){
-    clear_connections(get_property(active_ap, "Ssid"));
+    deactivate_connections(get_property(active_ap, "Ssid")); //Disconnect from any connected networks 
   }
-  clear_connections(n.ssid);
+  clear_connections(n.ssid); //Clear all connections that may already exist to the network we are connecting
-  qDebug() << "Connecting to"<< n.ssid << "with username, password =" << username << "," <<password;
   connect(n.ssid, username, password, n.security_type);
 }
 
@@ -127,7 +148,7 @@ void WifiManager::connect(QByteArray ssid, QString username, QString password, S
   connection["connection"]["type"] = "802-11-wireless";
   connection["connection"]["uuid"] = QUuid::createUuid().toString().remove('{').remove('}');
 
-  connection["connection"]["id"] = "OpenPilot connection "+QString::fromStdString(ssid.toStdString()); //TODO Add security type
+  connection["connection"]["id"] = "OpenPilot connection "+QString::fromStdString(ssid.toStdString());
 
   connection["802-11-wireless"]["ssid"] = ssid;
   connection["802-11-wireless"]["mode"] = "infrastructure";
@@ -143,28 +164,34 @@ void WifiManager::connect(QByteArray ssid, QString username, QString password, S
 
   QDBusInterface nm_settings(nm_service, nm_settings_path, nm_settings_iface, bus);
   QDBusReply<QDBusObjectPath> result = nm_settings.call("AddConnection", QVariant::fromValue(connection));
-  if (!result.isValid()) {
+}
-    qDebug() << result.error().name() << result.error().message();
+
-  } else {
+void WifiManager::deactivate_connections(QString ssid){
-    qDebug() << result.value().path();
+  for(QDBusObjectPath active_connection_raw : get_active_connections()){
+    QString active_connection = active_connection_raw.path();
+    QDBusInterface nm(nm_service, active_connection, props_iface, bus);
+    QDBusObjectPath pth = get_response<QDBusObjectPath>(nm.call("Get", connection_iface, "SpecificObject"));
+    QString Ssid = get_property(pth.path(), "Ssid");
+    if(Ssid == ssid){
+      QDBusInterface nm2(nm_service, nm_path, nm_iface, bus);
+      nm2.call("DeactivateConnection", QVariant::fromValue(active_connection_raw));
+    }
   }
 }
 
-void WifiManager::print_active_connections(){
+QVector<QDBusObjectPath> WifiManager::get_active_connections(){
-  //TO-DO clean up, the code is not currently in use.
   QDBusInterface nm(nm_service, nm_path, props_iface, bus);
   QDBusMessage response = nm.call("Get", nm_iface, "ActiveConnections");
-  QVariant first = response.arguments().at(0);
+  QDBusArgument arr = get_response<QDBusArgument>(response);
-  QDBusVariant dbvFirst = first.value<QDBusVariant>();
+  QVector<QDBusObjectPath> conns;
-  QVariant vFirst = dbvFirst.variant();
+
-  QDBusArgument step4 = vFirst.value<QDBusArgument>();
   QDBusObjectPath path;
-  step4.beginArray();
+  arr.beginArray();
-  while (!step4.atEnd()){
+  while (!arr.atEnd()){
-    step4 >> path;
+    arr >> path;
-    qDebug()<<path.path();
+    conns.push_back(path);
   }
-  step4.endArray();
+  return conns;
 }
 
 void WifiManager::clear_connections(QString ssid){
@@ -189,7 +216,6 @@ void WifiManager::clear_connections(QString ssid){
         if(inner_key == "ssid"){
           QString value = innerMap.value(inner_key).value<QString>();
           if(value == ssid){
-            // qDebug()<<"Deleting "<<value;
             nm2.call("Delete");
           }
         }
@@ -202,7 +228,7 @@ void WifiManager::request_scan(){
   if (!has_adapter) return;
 
   QDBusInterface nm(nm_service, adapter, wireless_device_iface, bus);
-  QDBusMessage response = nm.call("RequestScan",  QVariantMap());
+  nm.call("RequestScan",  QVariantMap());
 }
 
 uint WifiManager::get_wifi_device_state(){
@@ -259,3 +285,12 @@ QString WifiManager::get_adapter(){
 
   return adapter_path;
 }
+
+void WifiManager::change(unsigned int new_state,unsigned int previous_state,unsigned int change_reason){
+  raw_adapter_state = new_state;
+  if(new_state == state_need_auth && change_reason == reason_wrong_password){
+    emit wrongPassword(connecting_to_network);
+  }else if(new_state == state_connected){
+    connecting_to_network="";
+  }
+}

selfdrive/ui/qt/offroad/wifiManager.hpp

@@ -8,6 +8,11 @@ enum class SecurityType {
   WPA,
   UNSUPPORTED
 };
+enum class ConnectedType{
+  DISCONNECTED,
+  CONNECTING,
+  CONNECTED
+};
 
 typedef QMap<QString, QMap<QString, QVariant>> Connection;
 
@@ -15,11 +20,12 @@ struct Network {
   QString path;
   QByteArray ssid;
   unsigned int strength;
-  bool connected;
+  ConnectedType connected;
   SecurityType security_type;
 };
 
-class WifiManager{
+class WifiManager : public QWidget {
+  Q_OBJECT
 public:
   explicit WifiManager();
 
@@ -36,15 +42,24 @@ private:
   QVector<QByteArray> seen_ssids;
   QString adapter;//Path to network manager wifi-device
   QDBusConnection bus = QDBusConnection::systemBus();
+  unsigned int raw_adapter_state;//Connection status https://developer.gnome.org/NetworkManager/1.26/nm-dbus-types.html#NMDeviceState
+  QString connecting_to_network;
 
   QString get_adapter();
   QList<Network> get_networks();
   void connect(QByteArray ssid, QString username, QString password, SecurityType security_type);
   QString get_active_ap();
+  void deactivate_connections(QString ssid);
   void clear_connections(QString ssid);
-  void print_active_connections();
+  QVector<QDBusObjectPath> get_active_connections();
   uint get_wifi_device_state();
   QByteArray get_property(QString network_path, QString property);
   unsigned int get_ap_strength(QString network_path);
   SecurityType getSecurityType(QString ssid);
+
+private slots:
+  void change(unsigned int new_state, unsigned int previous_state, unsigned int change_reason);
+signals:
+  void wrongPassword(QString ssid);
+  void refresh();
 };