Merge pull request #20 from guh/fix-upnp

improve UPnP discovery

libmea-core/discovery/nymeadiscovery.cpp

@@ -21,13 +21,10 @@ void NymeaDiscovery::setDiscovering(bool discovering)
     if (m_discovering != discovering) {
         m_discovering = discovering;
         // For zeroconf we'll ignore it as zeroconf doesn't do active discovery but just listens for changes in the net all the time
-        // If we don't have zeroconf available, start an active upnp discovery
+        if (discovering) {
-        if (!m_zeroConf->available()) {
+            m_upnp->discover();
-            if (discovering) {
+        } else {
-                m_upnp->discover();
+            m_upnp->stopDiscovery();
-            } else {
-                m_upnp->stopDiscovery();
-            }
         }
         emit discoveringChanged();
     }

libmea-core/discovery/upnpdiscovery.cpp

@@ -23,12 +23,11 @@
 #include <QDebug>
 #include <QUrl>
 #include <QXmlStreamReader>
+#include <QNetworkInterface>
 
 UpnpDiscovery::UpnpDiscovery(DiscoveryModel *discoveryModel, QObject *parent) :
-    QUdpSocket(parent),
+    QObject(parent),
-    m_discoveryModel(discoveryModel),
+    m_discoveryModel(discoveryModel)
-    m_discovering(false),
-    m_available(false)
 {
     m_networkAccessManager = new QNetworkAccessManager(this);
     connect(m_networkAccessManager, &QNetworkAccessManager::finished, this, &UpnpDiscovery::networkReplyFinished);
@@ -36,76 +35,65 @@ UpnpDiscovery::UpnpDiscovery(DiscoveryModel *discoveryModel, QObject *parent) :
     m_repeatTimer.setInterval(500);
     connect(&m_repeatTimer, &QTimer::timeout, this, &UpnpDiscovery::writeDiscoveryPacket);
 
-    // bind udp socket and join multicast group
+    foreach (const QNetworkInterface &iface, QNetworkInterface::allInterfaces()) {
-    m_port = 1900;
+        if (!iface.flags().testFlag(QNetworkInterface::CanMulticast)) {
-    m_host = QHostAddress("239.255.255.250");
+            continue;
-
+        }
-    setSocketOption(QAbstractSocket::MulticastTtlOption,QVariant(1));
+        foreach (const QNetworkAddressEntry &netAddressEntry, iface.addressEntries()) {
-    setSocketOption(QAbstractSocket::MulticastLoopbackOption,QVariant(1));
+            if (netAddressEntry.ip().protocol() == QAbstractSocket::IPv4Protocol) {
-
+                QUdpSocket *socket = new QUdpSocket(this);
-    if(!bind(QHostAddress::AnyIPv4, m_port, QUdpSocket::ShareAddress)){
+                int port = -1;
-        qWarning() << "UPnP discovery could not bind to port" << m_port;
+                for (int i = 49125; i < 65535; i++) {
-        setAvailable(false);
+                    if(socket->bind(netAddressEntry.ip(), i, QUdpSocket::DontShareAddress)){
-        return;
+                        port = i;
+                        break;
+                    }
+                }
+                if (port == 65535 || socket->state() != QUdpSocket::BoundState) {
+                    socket->deleteLater();
+                    qWarning() << "UPnP discovery could not bind to interface" << netAddressEntry.ip();
+                    continue;
+                }
+                qDebug() << "Discovering on" << netAddressEntry.ip() << port;
+                m_sockets.append(socket);
+                connect(socket, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(error(QAbstractSocket::SocketError)));
+                connect(socket, &QUdpSocket::readyRead, this, &UpnpDiscovery::readData);
+            }
+        }
     }
-
-    if(!joinMulticastGroup(m_host)){
-        qWarning() << "UPnP discovery could not join multicast group" << m_host;
-        setAvailable(false);
-        return;
-    }
-
-    connect(this, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(error(QAbstractSocket::SocketError)));
-    connect(this, &UpnpDiscovery::readyRead, this, &UpnpDiscovery::readData);
-    setAvailable(true);
 }
 
 bool UpnpDiscovery::discovering() const
 {
-    return m_discovering;
+    return m_repeatTimer.isActive();
 }
 
 bool UpnpDiscovery::available() const
 {
-    return m_available;
+    return !m_sockets.isEmpty();
 }
 
 void UpnpDiscovery::discover()
 {
-    if (!m_available) {
+    if (!available()) {
         qWarning() << "Could not discover. UPnP not available.";
         return;
     }
 
     qDebug() << "start discovering...";
     m_repeatTimer.start();
-//    m_discoveryModel->clearModel();
     m_foundDevices.clear();
-
-    setDiscovering(true);
-
     writeDiscoveryPacket();
+    emit discoveringChanged();
 }
 
 void UpnpDiscovery::stopDiscovery()
 {
     qDebug() << "stop discovering";
     m_repeatTimer.stop();
-    setDiscovering(false);
-}
-
-void UpnpDiscovery::setDiscovering(const bool &discovering)
-{
-    m_discovering = discovering;
     emit discoveringChanged();
 }
 
-void UpnpDiscovery::setAvailable(const bool &available)
-{
-    m_available = available;
-    emit availableChanged();
-}
-
 void UpnpDiscovery::writeDiscoveryPacket()
 {
     QByteArray ssdpSearchMessage = QByteArray("M-SEARCH * HTTP/1.1\r\n"
@@ -114,33 +102,43 @@ void UpnpDiscovery::writeDiscoveryPacket()
                                               "MX:2\r\n"
                                               "ST: ssdp:all\r\n\r\n");
 
-//    qDebug() << "sending discovery packet";
+    qDebug() << "sending discovery package";
-    writeDatagram(ssdpSearchMessage, m_host, m_port);
+    foreach (QUdpSocket* socket, m_sockets) {
+        quint64 ret = socket->writeDatagram(ssdpSearchMessage, QHostAddress("239.255.255.250"), 1900);
+        if (ret != ssdpSearchMessage.length()) {
+            qWarning() << "Error sending SSDP query on socket" << socket->localAddress();
+        }
+
+    }
 }
 
 void UpnpDiscovery::error(QAbstractSocket::SocketError error)
 {
-    qWarning() << "UPnP socket error:" << error << errorString();
+    QUdpSocket* socket = static_cast<QUdpSocket*>(sender());
+    qWarning() << "UPnP socket error:" << error << socket->errorString();
 }
 
 void UpnpDiscovery::readData()
 {
+    QUdpSocket* socket = static_cast<QUdpSocket*>(sender());
     QByteArray data;
     quint16 port;
     QHostAddress hostAddress;
 
     // read the answere from the multicast
-    while (hasPendingDatagrams()) {
+    while (socket->hasPendingDatagrams()) {
-        data.resize(pendingDatagramSize());
+        data.resize(socket->pendingDatagramSize());
-        readDatagram(data.data(), data.size(), &hostAddress, &port);
+        socket->readDatagram(data.data(), data.size(), &hostAddress, &port);
     }
 
     if (!discovering()) {
         return;
     }
 
+//    qDebug() << "upnp packet" << data;
+
     // if the data contains the HTTP OK header...
-    if (data.contains("HTTP/1.1 200 OK") || data.contains("NOTIFY * HTTP/1.1")) {
+    if (data.contains("HTTP/1.1 200 OK")) {
         QUrl location;
         bool isNymea = false;
 
@@ -206,42 +204,63 @@ void UpnpDiscovery::networkReplyFinished(QNetworkReply *reply)
         if (xml.isStartDocument())
             continue;
 
+
         if (xml.isStartElement()) {
+
+            // Check for old style websocketURL and nymeaRpcURL
             if (xml.name().toString() == "websocketURL") {
                 QUrl u(xml.readElementText());
                 PortConfig *pc = new PortConfig(u.port());
                 pc->setProtocol(PortConfig::ProtocolWebSocket);
-                pc->setSslEnabled(u.scheme().endsWith('s'));
+                pc->setSslEnabled(u.scheme() == "wss");
                 portConfigList.append(pc);
             }
-        }
 
-        if (xml.isStartElement()) {
             if (xml.name().toString() == "nymeaRpcURL") {
                 QUrl u(xml.readElementText());
                 qDebug() << "have url" << u << u.scheme();
                 PortConfig *pc = new PortConfig(u.port());
                 pc->setProtocol(PortConfig::ProtocolNymeaRpc);
-                pc->setSslEnabled(u.scheme().endsWith('s'));
+                pc->setSslEnabled(u.scheme() == "nymeas");
                 portConfigList.append(pc);
             }
-        }
 
-        if (xml.isStartElement()) {
+            if (xml.name().toString() == "guhRpcURL") {
-            if (xml.name().toString() == "device") {
+                QUrl u(xml.readElementText());
-                while (!xml.atEnd()) {
+                qDebug() << "have url" << u << u.scheme();
-                    if (xml.name() == "friendlyName" && xml.isStartElement()) {
+                PortConfig *pc = new PortConfig(u.port());
-                        name = xml.readElementText();
+                pc->setProtocol(PortConfig::ProtocolNymeaRpc);
-                    }
+                pc->setSslEnabled(u.scheme() == "guhs");
-                    if (xml.name() == "modelNumber" && xml.isStartElement()) {
+                portConfigList.append(pc);
-                        version = xml.readElementText();
+            }
-                    }
+
-                    if (xml.name() == "UDN" && xml.isStartElement()) {
+            // But also for new style serviceList
-                        uuid = xml.readElementText().split(':').last();
+            if (xml.name().toString() == "serviceList") {
-                    }
+                while (!(xml.isEndElement() && xml.name().toString() == "serviceList") && !xml.atEnd()) {
                     xml.readNext();
+                    if (xml.name().toString() == "service") {
+                        while (!(xml.isEndElement() && xml.name().toString() == "service") && !xml.atEnd()) {
+                            xml.readNext();
+                            if (xml.name().toString() == "SCPDURL") {
+                                QUrl u(xml.readElementText());
+                                PortConfig *pc = new PortConfig(u.port());
+                                pc->setProtocol(u.scheme().startsWith("nymea") ? PortConfig::ProtocolNymeaRpc : PortConfig::ProtocolWebSocket);
+                                pc->setSslEnabled(u.scheme() == "nymeas" || u.scheme() == "wss");
+                                portConfigList.append(pc);
+                            }
+                        }
+                    }
                 }
-                xml.readNext();
+            }
+
+            if (xml.name() == "friendlyName") {
+                name = xml.readElementText();
+            }
+            if (xml.name() == "modelNumber") {
+                version = xml.readElementText();
+            }
+            if (xml.name() == "UDN") {
+                uuid = xml.readElementText().split(':').last();
             }
         }
     }

libmea-core/discovery/upnpdiscovery.h

@@ -30,7 +30,7 @@
 #include "discoverydevice.h"
 #include "discoverymodel.h"
 
-class UpnpDiscovery : public QUdpSocket
+class UpnpDiscovery : public QObject
 {
     Q_OBJECT
 public:
@@ -44,23 +44,16 @@ public:
     Q_INVOKABLE void stopDiscovery();
 
 private:
+    QList<QUdpSocket*> m_sockets;
     QNetworkAccessManager *m_networkAccessManager;
 
     QTimer m_repeatTimer;
 
-    QHostAddress m_host;
-    qint16 m_port;
-
     DiscoveryModel *m_discoveryModel;
 
-    bool m_discovering;
-    bool m_available;
     QHash<QNetworkReply *, QHostAddress> m_runningReplies;
     QList<QUrl> m_foundDevices;
 
-    void setDiscovering(const bool &discovering);
-    void setAvailable(const bool &available);
-
 signals:
     void discoveringChanged();
     void availableChanged();

mea/ui/ConnectPage.qml

@@ -582,6 +582,7 @@ Page {
                                 }
                                 onClicked: {
                                     Engine.connection.connect(dialog.discoveryDevice.toUrl(index))
+                                    dialog.close()
                                 }
                             }
                         }