From 961008027d1e276f07d84bb175c1907a1da748ec Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Mon, 21 Feb 2022 00:02:05 +0100 Subject: [PATCH] Consolidate permit joining timer Starting/stopping the permit joining duration timer is not something each backend should do on it's own. So making the timer a private member of ZigbeeNetwork and taking control over it internally. Also reduce some logic about the remaining duration by merging the related methods into one and hiding the "remaining" duration from backend implementations completely. --- .../backends/deconz/zigbeenetworkdeconz.cpp | 27 +++------ .../backends/nxp/zigbeenetworknxp.cpp | 30 ++-------- .../backends/ti/zigbeenetworkti.cpp | 17 +++--- libnymea-zigbee/zigbeenetwork.cpp | 58 ++++++++----------- libnymea-zigbee/zigbeenetwork.h | 16 +++-- 5 files changed, 51 insertions(+), 97 deletions(-) diff --git a/libnymea-zigbee/backends/deconz/zigbeenetworkdeconz.cpp b/libnymea-zigbee/backends/deconz/zigbeenetworkdeconz.cpp index 866c645..96bc5cd 100644 --- a/libnymea-zigbee/backends/deconz/zigbeenetworkdeconz.cpp +++ b/libnymea-zigbee/backends/deconz/zigbeenetworkdeconz.cpp @@ -99,11 +99,6 @@ void ZigbeeNetworkDeconz::setPermitJoining(quint8 duration, quint16 address) qCDebug(dcZigbeeNetwork()) << "Disable permit join on"<< ZigbeeUtils::convertUint16ToHexString(address); } - // Note: will be reseted if permit join will not work - setPermitJoiningEnabled(duration > 0); - setPermitJoiningDuration(duration); - setPermitJoiningRemaining(duration); - // Note: since compliance version >= 21 the value 255 is not any more endless. // We need to refresh the command on timeout repeatedly if the duration is longer than 254 s @@ -111,20 +106,12 @@ void ZigbeeNetworkDeconz::setPermitJoining(quint8 duration, quint16 address) connect(reply, &ZigbeeNetworkReply::finished, this, [this, reply, duration, address](){ if (reply->zigbeeApsStatus() != Zigbee::ZigbeeApsStatusSuccess) { qCWarning(dcZigbeeNetwork()) << "Could not set permit join to" << duration << ZigbeeUtils::convertUint16ToHexString(address) << reply->zigbeeApsStatus(); - setPermitJoiningEnabled(false); - setPermitJoiningDuration(duration); + setPermitJoiningState(false); return; } qCDebug(dcZigbeeNetwork()) << "Permit join request finished successfully"; - setPermitJoiningEnabled(duration > 0); - setPermitJoiningDuration(duration); - setPermitJoiningRemaining(duration); - if (duration > 0) { - m_permitJoinTimer->start(); - } else { - m_permitJoinTimer->stop(); - } + setPermitJoiningState(duration > 0, duration); // Set the permit joining timeout network configuration parameter QByteArray parameterData; @@ -530,7 +517,7 @@ void ZigbeeNetworkDeconz::runNetworkInitProcess() if (m_controller->networkState() == Deconz::NetworkStateConnected) { qCDebug(dcZigbeeNetwork()) << "The network is already running."; m_initializing = false; - setPermitJoiningEnabled(false); + setPermitJoiningState(false); // Set the permit joining timeout network configuration parameter QByteArray parameterData; QDataStream stream(¶meterData, QIODevice::WriteOnly); @@ -585,11 +572,11 @@ void ZigbeeNetworkDeconz::onControllerAvailableChanged(bool available) qCWarning(dcZigbeeNetwork()) << "Hardware controller is not available any more."; setError(ErrorHardwareUnavailable); m_initializing = false; - setPermitJoiningEnabled(false); + setPermitJoiningState(false); setState(StateOffline); } else { m_error = ErrorNoError; - setPermitJoiningEnabled(false); + setPermitJoiningState(false); setState(StateStarting); qCDebug(dcZigbeeNetwork()) << "Hardware controller is now available."; startNetworkInternally(); @@ -696,14 +683,14 @@ void ZigbeeNetworkDeconz::startNetwork() loadNetwork(); if (!m_controller->enable(serialPortName(), serialBaudrate())) { - setPermitJoiningEnabled(false); + setPermitJoiningState(false); setState(StateOffline); setCreateNetworkState(CreateNetworkStateIdle); setError(ErrorHardwareUnavailable); return; } - setPermitJoiningEnabled(false); + setPermitJoiningState(false); m_initializing = true; // Note: wait for the controller available signal and start the initialization there diff --git a/libnymea-zigbee/backends/nxp/zigbeenetworknxp.cpp b/libnymea-zigbee/backends/nxp/zigbeenetworknxp.cpp index af0f17e..7b633f5 100644 --- a/libnymea-zigbee/backends/nxp/zigbeenetworknxp.cpp +++ b/libnymea-zigbee/backends/nxp/zigbeenetworknxp.cpp @@ -114,11 +114,6 @@ void ZigbeeNetworkNxp::setPermitJoining(quint8 duration, quint16 address) qCDebug(dcZigbeeNetwork()) << "Disable permit join on"<< ZigbeeUtils::convertUint16ToHexString(address); } - // Note: will be reseted if permit join will not work - setPermitJoiningEnabled(duration > 0); - setPermitJoiningDuration(duration); - setPermitJoiningRemaining(duration); - if (address == 0x0000) { // Only the coordinator is allowed to join the network qCDebug(dcZigbeeNetwork()) << "Set permit join in the coordinator node only to" << duration << "[s]"; @@ -127,15 +122,9 @@ void ZigbeeNetworkNxp::setPermitJoining(quint8 duration, quint16 address) qCDebug(dcZigbeeNetwork()) << "Set permit join in the coordinator finished" << reply->status(); if (reply->status() != Nxp::StatusSuccess) { qCWarning(dcZigbeeNetwork()) << "Failed to set permit join status in coordinator"; - setPermitJoiningEnabled(false); - setPermitJoiningDuration(duration); + setPermitJoiningState(false); } else { - setPermitJoiningEnabled(duration > 0); - setPermitJoiningDuration(duration); - setPermitJoiningRemaining(duration); - if (duration > 0) { - m_permitJoinTimer->start(); - } + setPermitJoiningState(duration > 0, duration); } }); @@ -149,21 +138,12 @@ void ZigbeeNetworkNxp::setPermitJoining(quint8 duration, quint16 address) connect(reply, &ZigbeeNetworkReply::finished, this, [this, reply, duration, address](){ if (reply->zigbeeApsStatus() != Zigbee::ZigbeeApsStatusSuccess) { qCWarning(dcZigbeeNetwork()) << "Could not set permit join to" << duration << ZigbeeUtils::convertUint16ToHexString(address) << reply->zigbeeApsStatus(); - setPermitJoiningEnabled(false); - setPermitJoiningDuration(duration); - m_permitJoinTimer->stop(); + setPermitJoiningState(false); return; } qCDebug(dcZigbeeNetwork()) << "Permit join request finished successfully"; - setPermitJoiningEnabled(duration > 0); - setPermitJoiningDuration(duration); - setPermitJoiningRemaining(duration); - if (duration > 0) { - m_permitJoinTimer->start(); - } else { - m_permitJoinTimer->stop(); - } + setPermitJoiningState(duration > 0, duration); if (address == Zigbee::BroadcastAddressAllRouters || address == 0x0000) { qCDebug(dcZigbeeNetwork()) << "Set permit join in the coordinator node to" << duration << "[s]"; @@ -616,7 +596,7 @@ void ZigbeeNetworkNxp::startNetwork() { loadNetwork(); - setPermitJoiningEnabled(false); + setPermitJoiningState(false); if (!m_controller->enable(serialPortName(), serialBaudrate())) { setState(StateOffline); diff --git a/libnymea-zigbee/backends/ti/zigbeenetworkti.cpp b/libnymea-zigbee/backends/ti/zigbeenetworkti.cpp index 8e02181..0d5be41 100644 --- a/libnymea-zigbee/backends/ti/zigbeenetworkti.cpp +++ b/libnymea-zigbee/backends/ti/zigbeenetworkti.cpp @@ -98,15 +98,15 @@ void ZigbeeNetworkTi::setPermitJoining(quint8 duration, quint16 address) qCDebug(dcZigbeeNetwork()) << "Disable permit join on"<< ZigbeeUtils::convertUint16ToHexString(address); } - setPermitJoiningDuration(duration); - ZigbeeInterfaceTiReply *requestPermitJoinReply = m_controller->requestPermitJoin(duration, address); connect(requestPermitJoinReply, &ZigbeeInterfaceTiReply::finished, this, [=](){ if (requestPermitJoinReply->statusCode() != Ti::StatusCodeSuccess) { qCWarning(dcZigbeeNetwork()) << "Could not set permit join to" << duration << ZigbeeUtils::convertUint16ToHexString(address) << requestPermitJoinReply->statusCode(); return; } - qCDebug(dcZigbeeNetwork()) << "Permit join request finished successfully"; + qCDebug(dcZigbeeNetwork()) << "Permit join request finished successfully:" << duration; + + setPermitJoiningState(true, duration); // Opening the green power network too // Todo: This should probably be somewhere else, but not yet sure how other backeds deal with this @@ -325,11 +325,11 @@ void ZigbeeNetworkTi::onControllerAvailableChanged(bool available) if (!available) { qCWarning(dcZigbeeNetwork()) << "Hardware controller is not available any more."; setError(ErrorHardwareUnavailable); - setPermitJoiningEnabled(false); + setPermitJoiningState(false); setState(StateOffline); setError(ErrorHardwareUnavailable); } else { - setPermitJoiningEnabled(false); + setPermitJoiningState(false); setState(StateOffline); setError(ErrorNoError); qCDebug(dcZigbeeNetwork()) << "Hardware controller is now available."; @@ -442,8 +442,7 @@ void ZigbeeNetworkTi::onControllerStateChanged(ZigbeeBridgeControllerTi::Control void ZigbeeNetworkTi::onPermitJoinStateChanged(quint8 duration) { - setPermitJoiningRemaining(duration); - setPermitJoiningEnabled(duration > 0); + setPermitJoiningState(duration > 0, duration); m_controller->setLed(duration > 0); } @@ -493,13 +492,13 @@ void ZigbeeNetworkTi::startNetwork() loadNetwork(); if (!m_controller->enable(serialPortName(), serialBaudrate())) { - setPermitJoiningEnabled(false); + setPermitJoiningState(false); setState(StateOffline); setError(ErrorHardwareUnavailable); return; } - setPermitJoiningEnabled(false); + setPermitJoiningState(false); } void ZigbeeNetworkTi::stopNetwork() diff --git a/libnymea-zigbee/zigbeenetwork.cpp b/libnymea-zigbee/zigbeenetwork.cpp index 2c76c3c..b8ec2ee 100644 --- a/libnymea-zigbee/zigbeenetwork.cpp +++ b/libnymea-zigbee/zigbeenetwork.cpp @@ -49,7 +49,7 @@ ZigbeeNetwork::ZigbeeNetwork(const QUuid &networkUuid, QObject *parent) : m_permitJoinTimer->stop(); setPermitJoining(0); } else { - setPermitJoiningRemaining(m_permitJoiningRemaining); + setPermitJoiningState(m_permitJoiningRemaining, m_permitJoiningRemaining); } }); @@ -246,38 +246,6 @@ quint8 ZigbeeNetwork::permitJoiningRemaining() const return m_permitJoiningRemaining; } -void ZigbeeNetwork::setPermitJoiningEnabled(bool permitJoiningEnabled) -{ - if (m_permitJoiningEnabled == permitJoiningEnabled) - return; - - m_permitJoiningEnabled = permitJoiningEnabled; - emit permitJoiningEnabledChanged(m_permitJoiningEnabled); - - if (!m_permitJoiningEnabled) { - m_permitJoinTimer->stop(); - setPermitJoiningRemaining(0); - } -} - -void ZigbeeNetwork::setPermitJoiningDuration(quint8 duration) -{ - if (m_permitJoiningDuration == duration) - return; - - m_permitJoiningDuration = duration; - emit permitJoinDurationChanged(m_permitJoiningDuration); -} - -void ZigbeeNetwork::setPermitJoiningRemaining(quint8 remaining) -{ - if (m_permitJoiningRemaining == remaining) - return; - - m_permitJoiningRemaining = remaining; - emit permitJoinRemainingChanged(m_permitJoiningRemaining); -} - quint8 ZigbeeNetwork::generateSequenceNumber() { return m_sequenceNumber++; @@ -514,6 +482,28 @@ void ZigbeeNetwork::evaluateNextNodeReachableState() }); } +void ZigbeeNetwork::setPermitJoiningState(bool permitJoiningEnabled, quint8 duration) +{ + if (permitJoiningEnabled) { + if (m_permitJoiningDuration != duration) { + m_permitJoiningDuration = duration; + emit permitJoinDurationChanged(duration); + } + m_permitJoiningRemaining = duration; + m_permitJoinTimer->start(); + } else { + m_permitJoiningDuration = 0; + emit permitJoinDurationChanged(0); + m_permitJoiningRemaining = 0; + m_permitJoinTimer->stop(); + } + + if (m_permitJoiningEnabled != permitJoiningEnabled) { + m_permitJoiningEnabled = permitJoiningEnabled; + emit permitJoiningEnabledChanged(permitJoiningEnabled); + } +} + void ZigbeeNetwork::loadNetwork() { if (m_networkLoaded) { @@ -568,7 +558,7 @@ void ZigbeeNetwork::clearSettings() setChannel(0); setSecurityConfiguration(ZigbeeSecurityConfiguration()); setState(StateUninitialized); - setPermitJoiningEnabled(false); + setPermitJoiningState(false); m_nodeType = ZigbeeDeviceProfile::NodeTypeCoordinator; } diff --git a/libnymea-zigbee/zigbeenetwork.h b/libnymea-zigbee/zigbeenetwork.h index e63e026..ba7c663 100644 --- a/libnymea-zigbee/zigbeenetwork.h +++ b/libnymea-zigbee/zigbeenetwork.h @@ -163,6 +163,12 @@ private: void printNetwork(); + // Permit join + QTimer *m_permitJoinTimer = nullptr; + bool m_permitJoiningEnabled = false; + quint8 m_permitJoiningDuration = 120; + quint8 m_permitJoiningRemaining = 0; + private: void addNodeInternally(ZigbeeNode *node); void removeNodeInternally(ZigbeeNode *node); @@ -177,19 +183,11 @@ protected: ZigbeeNode *createNode(quint16 shortAddress, const ZigbeeAddress &extendedAddress, QObject *parent); ZigbeeNode *createNode(quint16 shortAddress, const ZigbeeAddress &extendedAddress, quint8 macCapabilities, QObject *parent); - // Permit join - QTimer *m_permitJoinTimer = nullptr; - bool m_permitJoiningEnabled = false; - quint8 m_permitJoiningDuration = 120; - quint8 m_permitJoiningRemaining = 0; - QTimer *m_reachableRefreshTimer = nullptr; QList m_reachableRefreshAddresses; void evaluateNextNodeReachableState(); - void setPermitJoiningEnabled(bool permitJoiningEnabled); - void setPermitJoiningDuration(quint8 duration); - void setPermitJoiningRemaining(quint8 remaining); + void setPermitJoiningState(bool permitJoiningEnabled, quint8 duration = 0); void clearSettings();