From ed5517fbf1a4083bc89b4857d33097b984e406f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=BCrz?= Date: Tue, 30 Jun 2020 16:49:09 +0200 Subject: [PATCH] Implement reply timeout and restructure uart communication queue --- .../interface/zigbeeinterfacedeconz.cpp | 2 +- .../interface/zigbeeinterfacedeconzreply.cpp | 2 +- .../deconz/zigbeebridgecontrollerdeconz.cpp | 54 ++++++++++++------- .../backends/deconz/zigbeenetworkdeconz.cpp | 10 ++++ .../lighting/zigbeeclustercolorcontrol.cpp | 1 - .../zcl/lighting/zigbeeclustercolorcontrol.h | 1 - libnymea-zigbee/zcl/zigbeecluster.cpp | 3 ++ libnymea-zigbee/zdo/zigbeedeviceobject.cpp | 3 ++ libnymea-zigbee/zigbeenetwork.cpp | 30 +++++++++++ libnymea-zigbee/zigbeenetwork.h | 4 ++ libnymea-zigbee/zigbeenetworkdatabase.cpp | 5 +- libnymea-zigbee/zigbeenetworkmanager.h | 6 ++- libnymea-zigbee/zigbeenetworkreply.cpp | 9 +++- libnymea-zigbee/zigbeenetworkreply.h | 3 ++ libnymea-zigbee/zigbeenode.cpp | 6 +-- 15 files changed, 110 insertions(+), 29 deletions(-) diff --git a/libnymea-zigbee/backends/deconz/interface/zigbeeinterfacedeconz.cpp b/libnymea-zigbee/backends/deconz/interface/zigbeeinterfacedeconz.cpp index 505b106..cb18849 100644 --- a/libnymea-zigbee/backends/deconz/interface/zigbeeinterfacedeconz.cpp +++ b/libnymea-zigbee/backends/deconz/interface/zigbeeinterfacedeconz.cpp @@ -227,7 +227,7 @@ void ZigbeeInterfaceDeconz::sendPackage(const QByteArray &package) qCWarning(dcZigbeeInterface()) << "Could not stream byte" << ZigbeeUtils::convertByteArrayToHexString(data); } - m_serialPort->flush(); + //m_serialPort->flush(); } bool ZigbeeInterfaceDeconz::enable(const QString &serialPort, qint32 baudrate) diff --git a/libnymea-zigbee/backends/deconz/interface/zigbeeinterfacedeconzreply.cpp b/libnymea-zigbee/backends/deconz/interface/zigbeeinterfacedeconzreply.cpp index cd46c24..66015d4 100644 --- a/libnymea-zigbee/backends/deconz/interface/zigbeeinterfacedeconzreply.cpp +++ b/libnymea-zigbee/backends/deconz/interface/zigbeeinterfacedeconzreply.cpp @@ -84,7 +84,7 @@ ZigbeeInterfaceDeconzReply::ZigbeeInterfaceDeconzReply(Deconz::Command command, m_timer(new QTimer(this)), m_command(command) { - m_timer->setInterval(3000); + m_timer->setInterval(5000); m_timer->setSingleShot(true); connect(m_timer, &QTimer::timeout, this, &ZigbeeInterfaceDeconzReply::onTimeout); } diff --git a/libnymea-zigbee/backends/deconz/zigbeebridgecontrollerdeconz.cpp b/libnymea-zigbee/backends/deconz/zigbeebridgecontrollerdeconz.cpp index 7712c15..d990501 100644 --- a/libnymea-zigbee/backends/deconz/zigbeebridgecontrollerdeconz.cpp +++ b/libnymea-zigbee/backends/deconz/zigbeebridgecontrollerdeconz.cpp @@ -214,7 +214,7 @@ void ZigbeeBridgeControllerDeconz::sendNextRequest() if (m_currentReply) return; -// // If the controler request queue is full, wait until it's free again +// // FIXME: If the controler request queue is full, wait until it's free again // if (!m_apsFreeSlotsAvailable) // return; @@ -241,15 +241,24 @@ ZigbeeInterfaceDeconzReply *ZigbeeBridgeControllerDeconz::createReply(Deconz::Co // Make sure we clean up on timeout connect(reply, &ZigbeeInterfaceDeconzReply::timeout, this, [this, reply](){ qCWarning(dcZigbeeController()) << "Reply timeout" << reply; - if (m_currentReply == reply) { - m_currentReply = nullptr; - QMetaObject::invokeMethod(this, "sendNextRequest", Qt::QueuedConnection); + + // Make sure we can send the next read confirm reply + if (m_readConfirmReply == reply) { + m_readConfirmReply = nullptr; } + + // Make sure we can send the next read indication reply + if (m_readIndicationReply == reply) { + m_readIndicationReply = nullptr; + } + + // Note: send next reply with the finished signal }); // Auto delete the object on finished connect(reply, &ZigbeeInterfaceDeconzReply::finished, reply, [this, reply](){ reply->deleteLater(); + if (m_currentReply == reply) { m_currentReply = nullptr; QMetaObject::invokeMethod(this, "sendNextRequest", Qt::QueuedConnection); @@ -257,8 +266,16 @@ ZigbeeInterfaceDeconzReply *ZigbeeBridgeControllerDeconz::createReply(Deconz::Co }); // Enqueu this reply and send it once the current reply slot is free - m_replyQueue.enqueue(reply); - qCDebug(dcZigbeeController()) << "Enqueue request:" << reply->requestName(); + + // If this is a data indication or a confirmation, prepend the reply since responses have higher priority than new requests + if (command == Deconz::CommandApsDataConfirm || command == Deconz::CommandApsDataIndication) { + m_replyQueue.prepend(reply); + qCDebug(dcZigbeeController()) << "Prepend request to queue:" << reply->requestName(); + } else { + m_replyQueue.enqueue(reply); + qCDebug(dcZigbeeController()) << "Enqueue request:" << reply->requestName(); + } + QMetaObject::invokeMethod(this, "sendNextRequest", Qt::QueuedConnection); return reply; } @@ -737,15 +754,15 @@ void ZigbeeBridgeControllerDeconz::readDataIndication() return; } - m_readIndicationReply = requestReadReceivedDataIndication(); - connect(m_readIndicationReply, &ZigbeeInterfaceDeconzReply::finished, this, [this](){ - ZigbeeInterfaceDeconzReply *reply = m_readIndicationReply; - + ZigbeeInterfaceDeconzReply *reply = requestReadReceivedDataIndication(); + // Set this as the current read indication reply so we don't request more than one at the time + m_readIndicationReply = reply; + connect(reply, &ZigbeeInterfaceDeconzReply::finished, this, [this, reply](){ // Allow to send the next read indication reply if required m_readIndicationReply = nullptr; if (reply->statusCode() != Deconz::StatusCodeSuccess) { - qCWarning(dcZigbeeController()) << "Could not read data indication." << "SQN:" << m_readIndicationReply->sequenceNumber() << m_readIndicationReply->statusCode(); + qCWarning(dcZigbeeController()) << "Could not read data indication." << "SQN:" << reply->sequenceNumber() << reply->statusCode(); // FIXME: set an appropriate error return; } @@ -764,10 +781,10 @@ void ZigbeeBridgeControllerDeconz::readDataConfirm() return; } - m_readConfirmReply = requestQuerySendDataConfirm(); - connect(m_readConfirmReply, &ZigbeeInterfaceDeconzReply::finished, this, [this](){ - ZigbeeInterfaceDeconzReply *reply = m_readConfirmReply; - + ZigbeeInterfaceDeconzReply *reply = requestQuerySendDataConfirm(); + // Set this as the current read confirm reply so we don't request more than one at the time + m_readConfirmReply = reply; + connect(reply, &ZigbeeInterfaceDeconzReply::finished, this, [this, reply](){ // Allow to send the next read confirm reply if required m_readConfirmReply = nullptr; @@ -939,13 +956,13 @@ void ZigbeeBridgeControllerDeconz::onInterfacePackageReceived(const QByteArray & m_currentReply->m_responseData = data; m_currentReply->m_statusCode = status; m_currentReply->finished(); - // Note: the current reply will be cleand up in the finished slot + // Note: the current reply will be cleaned up in the finished slot return; } // We got a notification, lets set the current sequence number to the notification id, // so the next request will be a continuouse increase - m_sequenceNumber = sequenceNumber; + m_sequenceNumber = sequenceNumber + 1; // No request for this data, lets check which notification and process the data switch (command) { @@ -956,7 +973,8 @@ void ZigbeeBridgeControllerDeconz::onInterfacePackageReceived(const QByteArray & break; } case Deconz::CommandMacPoll: { - qCDebug(dcZigbeeController()) << "MAC Poll command received" << ZigbeeUtils::convertByteArrayToHexString(data);// FIXME: parse the data and print info + // FIXME: parse the data and print info + qCDebug(dcZigbeeController()) << "MAC Poll command received" << ZigbeeUtils::convertByteArrayToHexString(data); break; } case Deconz::CommandSimplifiedBeacon: { diff --git a/libnymea-zigbee/backends/deconz/zigbeenetworkdeconz.cpp b/libnymea-zigbee/backends/deconz/zigbeenetworkdeconz.cpp index 4006326..13e2a01 100644 --- a/libnymea-zigbee/backends/deconz/zigbeenetworkdeconz.cpp +++ b/libnymea-zigbee/backends/deconz/zigbeenetworkdeconz.cpp @@ -81,6 +81,9 @@ ZigbeeNetworkReply *ZigbeeNetworkDeconz::sendRequest(const ZigbeeNetworkRequest finishNetworkReply(reply, ZigbeeNetworkReply::ErrorInterfaceError); return; } + + // The request has been sent successfully to the device, start the timeout timer now + startWaitingReply(reply); }); return reply; @@ -652,6 +655,13 @@ void ZigbeeNetworkDeconz::onDeviceAnnounced(quint16 shortAddress, ZigbeeAddress { qCDebug(dcZigbeeNetwork()) << "Device announced" << ZigbeeUtils::convertUint16ToHexString(shortAddress) << ieeeAddress.toString() << ZigbeeUtils::convertByteToHexString(macCapabilities); + // Lets check if this device is in the uninitialized node list, if so, remove it and recreate the device + if (hasUninitializedNode(ieeeAddress)) { + qCWarning(dcZigbeeNetwork()) << "Device announced but there is already an initialization running for it. Remove the device and restart the initialization."; + ZigbeeNode *uninitializedNode = getZigbeeNode(ieeeAddress); + removeUninitializedNode(uninitializedNode); + } + if (hasNode(ieeeAddress)) { qCWarning(dcZigbeeNetwork()) << "Already known device announced. FIXME: Ignoring announcement" << ieeeAddress.toString(); return; diff --git a/libnymea-zigbee/zcl/lighting/zigbeeclustercolorcontrol.cpp b/libnymea-zigbee/zcl/lighting/zigbeeclustercolorcontrol.cpp index 81880c5..3f300a3 100644 --- a/libnymea-zigbee/zcl/lighting/zigbeeclustercolorcontrol.cpp +++ b/libnymea-zigbee/zcl/lighting/zigbeeclustercolorcontrol.cpp @@ -224,7 +224,6 @@ void ZigbeeClusterColorControl::setAttribute(const ZigbeeClusterAttribute &attri emit attributeChanged(attribute); } - } void ZigbeeClusterColorControl::processDataIndication(ZigbeeClusterLibrary::Frame frame) diff --git a/libnymea-zigbee/zcl/lighting/zigbeeclustercolorcontrol.h b/libnymea-zigbee/zcl/lighting/zigbeeclustercolorcontrol.h index f83dfa9..f98b1b4 100644 --- a/libnymea-zigbee/zcl/lighting/zigbeeclustercolorcontrol.h +++ b/libnymea-zigbee/zcl/lighting/zigbeeclustercolorcontrol.h @@ -128,7 +128,6 @@ public: }; Q_ENUM(EnhancedColorMode) - enum ColorCapability { ColorCapabilityHueSaturation = 0x01, ColorCapabilityEnhancedHue = 0x02, diff --git a/libnymea-zigbee/zcl/zigbeecluster.cpp b/libnymea-zigbee/zcl/zigbeecluster.cpp index fe68579..a75baea 100644 --- a/libnymea-zigbee/zcl/zigbeecluster.cpp +++ b/libnymea-zigbee/zcl/zigbeecluster.cpp @@ -267,6 +267,9 @@ bool ZigbeeCluster::verifyNetworkError(ZigbeeClusterReply *zclReply, ZigbeeNetwo zclReply->m_zigbeeNwkStatus = networkReply->zigbeeNwkStatus(); success = true; break; + case ZigbeeNetworkReply::ErrorTimeout: + zclReply->m_error = ZigbeeClusterReply::ErrorTimeout; + break; case ZigbeeNetworkReply::ErrorInterfaceError: zclReply->m_error = ZigbeeClusterReply::ErrorInterfaceError; break; diff --git a/libnymea-zigbee/zdo/zigbeedeviceobject.cpp b/libnymea-zigbee/zdo/zigbeedeviceobject.cpp index b5b28b2..d47cfdb 100644 --- a/libnymea-zigbee/zdo/zigbeedeviceobject.cpp +++ b/libnymea-zigbee/zdo/zigbeedeviceobject.cpp @@ -369,6 +369,9 @@ bool ZigbeeDeviceObject::verifyNetworkError(ZigbeeDeviceObjectReply *zdoReply, Z case ZigbeeNetworkReply::ErrorInterfaceError: zdoReply->m_error = ZigbeeDeviceObjectReply::ErrorInterfaceError; break; + case ZigbeeNetworkReply::ErrorTimeout: + zdoReply->m_error = ZigbeeDeviceObjectReply::ErrorTimeout; + break; case ZigbeeNetworkReply::ErrorNetworkOffline: zdoReply->m_error = ZigbeeDeviceObjectReply::ErrorNetworkOffline; break; diff --git a/libnymea-zigbee/zigbeenetwork.cpp b/libnymea-zigbee/zigbeenetwork.cpp index 8123be5..8e8c0c2 100644 --- a/libnymea-zigbee/zigbeenetwork.cpp +++ b/libnymea-zigbee/zigbeenetwork.cpp @@ -282,6 +282,10 @@ void ZigbeeNetwork::removeNodeInternally(ZigbeeNode *node) return; } + if (node == m_coordinatorNode) { + m_coordinatorNode = nullptr; + } + m_nodes.removeAll(node); emit nodeRemoved(node); node->deleteLater(); @@ -366,6 +370,17 @@ void ZigbeeNetwork::clearSettings() m_nodeType = ZigbeeDeviceProfile::NodeTypeCoordinator; } +bool ZigbeeNetwork::hasUninitializedNode(const ZigbeeAddress &address) const +{ + foreach (ZigbeeNode *node, m_uninitializedNodes) { + if (node->extendedAddress() == address) { + return true; + } + } + + return false; +} + void ZigbeeNetwork::addNode(ZigbeeNode *node) { qCDebug(dcZigbeeNetwork()) << "Add node" << node; @@ -402,6 +417,13 @@ void ZigbeeNetwork::removeNode(ZigbeeNode *node) m_database->removeNode(node); } +void ZigbeeNetwork::removeUninitializedNode(ZigbeeNode *node) +{ + qCDebug(dcZigbeeNetwork()) << "Remove uninitialized node" << node; + m_uninitializedNodes.removeAll(node); + node->deleteLater(); +} + void ZigbeeNetwork::setState(ZigbeeNetwork::State state) { if (m_state == state) @@ -480,10 +502,18 @@ void ZigbeeNetwork::finishNetworkReply(ZigbeeNetworkReply *reply, ZigbeeNetworkR qCWarning(dcZigbeeNetwork()) << "Failed to send request to device" << reply->request() << reply->error(); break; } + // Stop the timer + reply->m_timer->stop(); + // Finish the reply reply->finished(); } +void ZigbeeNetwork::startWaitingReply(ZigbeeNetworkReply *reply) +{ + reply->m_timer->start(); +} + void ZigbeeNetwork::onNodeStateChanged(ZigbeeNode::State state) { ZigbeeNode *node = qobject_cast(sender()); diff --git a/libnymea-zigbee/zigbeenetwork.h b/libnymea-zigbee/zigbeenetwork.h index db872e6..6cfb801 100644 --- a/libnymea-zigbee/zigbeenetwork.h +++ b/libnymea-zigbee/zigbeenetwork.h @@ -156,9 +156,12 @@ protected: void loadNetwork(); void clearSettings(); + bool hasUninitializedNode(const ZigbeeAddress &address) const; + void addNode(ZigbeeNode *node); void addUnitializedNode(ZigbeeNode *node); void removeNode(ZigbeeNode *node); + void removeUninitializedNode(ZigbeeNode *node); void setState(State state); void setError(Error error); @@ -171,6 +174,7 @@ protected: ZigbeeNetworkReply *createNetworkReply(const ZigbeeNetworkRequest &request = ZigbeeNetworkRequest()); void setReplyResponseError(ZigbeeNetworkReply *reply, Zigbee::ZigbeeApsStatus zigbeeApsStatus = Zigbee::ZigbeeApsStatusSuccess); void finishNetworkReply(ZigbeeNetworkReply *reply, ZigbeeNetworkReply::Error error = ZigbeeNetworkReply::ErrorNoError); + void startWaitingReply(ZigbeeNetworkReply *reply); signals: void settingsFileNameChanged(const QString &settingsFileName); diff --git a/libnymea-zigbee/zigbeenetworkdatabase.cpp b/libnymea-zigbee/zigbeenetworkdatabase.cpp index 966d12e..f8c6819 100644 --- a/libnymea-zigbee/zigbeenetworkdatabase.cpp +++ b/libnymea-zigbee/zigbeenetworkdatabase.cpp @@ -193,7 +193,6 @@ bool ZigbeeNetworkDatabase::initDatabase() if (m_db.tables().isEmpty()) { // Write pragmas m_db.exec("PRAGMA foreign_keys = ON;"); - m_db.exec(QString("PRAGMA schema_version = %1;").arg(DB_VERSION)); m_db.exec(QString("PRAGMA user_version = %1;").arg(DB_VERSION)); } @@ -203,7 +202,9 @@ bool ZigbeeNetworkDatabase::initDatabase() "(ieeeAddress TEXT PRIMARY KEY, " // ieeeAddress to string "shortAddress INTEGER NOT NULL, " // uint16 "nodeDescriptor BLOB NOT NULL, " // bytes as received from the node - "powerDescriptor INTEGER NOT NULL)"); // uint16 + "powerDescriptor INTEGER NOT NULL, " + "lqi INTEGER," + "timestamp )"); // uint16 createIndices("ieeeAddressIndex", "nodes", "ieeeAddress"); } diff --git a/libnymea-zigbee/zigbeenetworkmanager.h b/libnymea-zigbee/zigbeenetworkmanager.h index a3b9f50..a78e058 100644 --- a/libnymea-zigbee/zigbeenetworkmanager.h +++ b/libnymea-zigbee/zigbeenetworkmanager.h @@ -35,13 +35,17 @@ class ZigbeeNetworkManager { public: + //Q_GADGET + enum BackendType { BackendTypeDeconz }; + //Q_ENUM(BackendType) static QStringList availableBackendTypes(); - static ZigbeeNetwork *createZigbeeNetwork(BackendType backend, QObject *parent = nullptr); }; +Q_DECLARE_METATYPE(ZigbeeNetworkManager) + #endif // ZIGBEENETWORKMANAGER_H diff --git a/libnymea-zigbee/zigbeenetworkreply.cpp b/libnymea-zigbee/zigbeenetworkreply.cpp index 8077a17..f9e9ea6 100644 --- a/libnymea-zigbee/zigbeenetworkreply.cpp +++ b/libnymea-zigbee/zigbeenetworkreply.cpp @@ -51,5 +51,12 @@ ZigbeeNetworkReply::ZigbeeNetworkReply(const ZigbeeNetworkRequest &request, QObj QObject(parent), m_request(request) { - + m_timer = new QTimer(this); + m_timer->setSingleShot(true); + m_timer->setInterval(8000); + connect(m_timer, &QTimer::timeout, this, [this](){ + m_error = ErrorTimeout; + emit finished(); + }); } + diff --git a/libnymea-zigbee/zigbeenetworkreply.h b/libnymea-zigbee/zigbeenetworkreply.h index 6018d25..f32f79c 100644 --- a/libnymea-zigbee/zigbeenetworkreply.h +++ b/libnymea-zigbee/zigbeenetworkreply.h @@ -29,6 +29,7 @@ #define ZIGBEENETWORKREPLY_H #include +#include #include "zigbee.h" #include "zigbeenetworkrequest.h" @@ -43,6 +44,7 @@ class ZigbeeNetworkReply : public QObject public: enum Error { ErrorNoError, + ErrorTimeout, ErrorZigbeeApsStatusError, ErrorZigbeeNwkStatusError, ErrorInterfaceError, @@ -59,6 +61,7 @@ public: private: explicit ZigbeeNetworkReply(const ZigbeeNetworkRequest &request, QObject *parent = nullptr); ZigbeeNetworkRequest m_request; + QTimer *m_timer = nullptr; Error m_error = ErrorNoError; Zigbee::ZigbeeApsStatus m_zigbeeApsStatus = Zigbee::ZigbeeApsStatusSuccess; diff --git a/libnymea-zigbee/zigbeenode.cpp b/libnymea-zigbee/zigbeenode.cpp index 4a77fe6..baf1585 100644 --- a/libnymea-zigbee/zigbeenode.cpp +++ b/libnymea-zigbee/zigbeenode.cpp @@ -166,7 +166,7 @@ void ZigbeeNode::initNodeDescriptor() // The request finished, but we received a ZDP error. if (reply->responseAdpu().status != ZigbeeDeviceProfile::StatusSuccess) { qCWarning(dcZigbeeNode()) << this << "failed to read node descriptor" << reply->responseAdpu().status; - // FIXME: decide what to do, remove the node again from network + emit nodeInitializationFailed(); return; } @@ -240,7 +240,7 @@ void ZigbeeNode::initEndpoints() if (reply->responseAdpu().status != ZigbeeDeviceProfile::StatusSuccess) { qCWarning(dcZigbeeNode()) << "Failed to read active endpoints" << reply->responseAdpu().status; - // FIXME: decide what to do, retry or stop initialization + emit nodeInitializationFailed(); return; } @@ -296,7 +296,7 @@ void ZigbeeNode::initEndpoint(quint8 endpointId) if (reply->responseAdpu().status != ZigbeeDeviceProfile::StatusSuccess) { qCWarning(dcZigbeeNode()) << this << "failed to read simple descriptor from endpoint" << endpointId << reply->responseAdpu().status; - // FIXME: decide what to do, retry or stop initialization + emit nodeInitializationFailed(); return; }