Fix status code handling for monitored items

The DataValue delivered by a data change notification may have a status code.
Such a status code should be reported to the node object instead of setting
it to Good.

[ChangeLog][QOpcUaNode] Report the right status code for monitored item updates

Change-Id: I2a62a475fca51eb0a95faef9bcc94037a58277c6
Pick-to: 6.7 6.6 6.5
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Frank Meerkoetter <frank.meerkoetter@basyskom.com>

5 files changed, 67 insertions, 1 deletions

diff --git a/src/plugins/opcua/open62541/qopen62541subscription.cpp b/src/plugins/opcua/open62541/qopen62541subscription.cpp
index 0128de3..f8a533d 100644
--- a/src/plugins/opcua/open62541/qopen62541subscription.cpp
+++ b/src/plugins/opcua/open62541/qopen62541subscription.cpp
@@ -470,7 +470,7 @@ void QOpen62541Subscription::monitoredValueUpdated(UA_UInt32 monId, UA_DataValue
         res.setServerTimestamp(QOpen62541ValueConverter::scalarToQt<QDateTime, UA_DateTime>(&value->serverTimestamp));
     if (value->hasSourceTimestamp)
         res.setSourceTimestamp(QOpen62541ValueConverter::scalarToQt<QDateTime, UA_DateTime>(&value->sourceTimestamp));
-    res.setStatusCode(QOpcUa::UaStatusCode::Good);
+    res.setStatusCode(value->hasStatus ? QOpcUa::UaStatusCode(value->status) : QOpcUa::UaStatusCode::Good);
     emit m_backend->dataChangeOccurred(item.value()->handle, res);
 }
 
diff --git a/tests/auto/qopcuaclient/tst_client.cpp b/tests/auto/qopcuaclient/tst_client.cpp
index a42e5d8..a1e3f22 100644
--- a/tests/auto/qopcuaclient/tst_client.cpp
+++ b/tests/auto/qopcuaclient/tst_client.cpp
@@ -606,6 +606,8 @@ private slots:
     void modifyMonitoredItem();
     defineDataMethod(addDuplicateMonitoredItem_data)
     void addDuplicateMonitoredItem();
+    defineDataMethod(subscriptionUnreadableNode_data);
+    void subscriptionUnreadableNode();
     defineDataMethod(checkMonitoredItemCleanup_data);
     void checkMonitoredItemCleanup();
     defineDataMethod(checkAttributeUpdated_data);
@@ -4433,6 +4435,39 @@ void Tst_QOpcUaClient::addDuplicateMonitoredItem()
     QCOMPARE(monitoringDisabledSpy.at(0).at(1).value<QOpcUa::UaStatusCode>(), QOpcUa::UaStatusCode::Good);
 }
 
+void Tst_QOpcUaClient::subscriptionUnreadableNode()
+{
+    QFETCH(QOpcUaClient *, opcuaClient);
+    OpcuaConnector connector(opcuaClient, m_endpoint);
+
+    QScopedPointer<QOpcUaNode> unreadableNode(opcuaClient->node("ns=3;s=VariableWithoutReadPermission"));
+    QVERIFY(unreadableNode != nullptr);
+
+    QOpcUaMonitoringParameters p(100);
+    p.setIndexRange(QStringLiteral("1"));
+    QSignalSpy monitoringEnabledSpy(unreadableNode.data(), &QOpcUaNode::enableMonitoringFinished);
+    QSignalSpy monitoringDisabledSpy(unreadableNode.data(), &QOpcUaNode::disableMonitoringFinished);
+    QSignalSpy dataChangeSpy(unreadableNode.data(), &QOpcUaNode::dataChangeOccurred);
+
+    unreadableNode->enableMonitoring(QOpcUa::NodeAttribute::Value, p);
+    monitoringEnabledSpy.wait(signalSpyTimeout);
+    QCOMPARE(monitoringEnabledSpy.size(), 1);
+    QCOMPARE(monitoringEnabledSpy.at(0).at(0).value<QOpcUa::NodeAttribute>(), QOpcUa::NodeAttribute::Value);
+    QCOMPARE(monitoringEnabledSpy.at(0).at(1).value<QOpcUa::UaStatusCode>(), QOpcUa::UaStatusCode::Good);
+
+     // Wait for the initial data change
+    dataChangeSpy.wait(signalSpyTimeout);
+    QCOMPARE(dataChangeSpy.size(), 1);
+    QCOMPARE(dataChangeSpy.at(0).at(0).value<QOpcUa::NodeAttribute>(), QOpcUa::NodeAttribute::Value);
+    QCOMPARE(dataChangeSpy.at(0).at(1), QVariant());
+    QCOMPARE(unreadableNode->valueAttribute(), QVariant());
+    QCOMPARE(unreadableNode->valueAttributeError(), QOpcUa::UaStatusCode::BadNotReadable);
+
+    unreadableNode->disableMonitoring(QOpcUa::NodeAttribute::Value);
+    monitoringDisabledSpy.wait(signalSpyTimeout);
+    QCOMPARE(monitoringDisabledSpy.size(), 1);
+}
+
 void Tst_QOpcUaClient::checkMonitoredItemCleanup()
 {
     QFETCH(QOpcUaClient *, opcuaClient);
diff --git a/tests/open62541-testserver/main.cpp b/tests/open62541-testserver/main.cpp
index 7d40f26..947bcbe 100644
--- a/tests/open62541-testserver/main.cpp
+++ b/tests/open62541-testserver/main.cpp
@@ -306,6 +306,8 @@ int main()
 
     server.addServerStatusTypeTestNodes(testFolder);
 
+    server.addUnreadableVariableNode(testFolder);
+
     // Add test nodes for the generic type decoder
     auto result = server.addEncoderTestModel();
 
diff --git a/tests/open62541-testserver/testserver.cpp b/tests/open62541-testserver/testserver.cpp
index 860eb0b..ea0ddbe 100644
--- a/tests/open62541-testserver/testserver.cpp
+++ b/tests/open62541-testserver/testserver.cpp
@@ -1374,6 +1374,33 @@ UA_StatusCode TestServer::addEncoderTestModel()
     return result;
 }
 
+UA_StatusCode TestServer::addUnreadableVariableNode(const UA_NodeId &parent)
+{
+    UA_VariableAttributes attr = UA_VariableAttributes_default;
+    attr.value = QOpen62541ValueConverter::toOpen62541Variant(42, QOpcUa::Int32);
+    attr.displayName = UA_LOCALIZEDTEXT_ALLOC("en-US", "VariableWithoutReadPermission");
+    attr.dataType = attr.value.type->typeId;
+    attr.accessLevel = UA_ACCESSLEVELMASK_WRITE; // Write only
+
+    UA_QualifiedName variableName;
+    variableName.namespaceIndex = parent.namespaceIndex;
+    variableName.name = attr.displayName.text;
+
+    auto nodeId = UA_NODEID_STRING_ALLOC(parent.namespaceIndex, "VariableWithoutReadPermission");
+    UA_StatusCode result = UA_Server_addVariableNode(m_server,
+                                                     nodeId,
+                                                     parent,
+                                                     UA_NODEID_NUMERIC(0, UA_NS0ID_ORGANIZES),
+                                                     variableName,
+                                                     UA_NODEID_NULL,
+                                                     attr,
+                                                     nullptr,
+                                                     nullptr);
+    UA_NodeId_clear(&nodeId);
+    UA_VariableAttributes_clear(&attr);
+    return result;
+}
+
 UA_StatusCode TestServer::addEventHistorian(const UA_NodeId &parent)
 {
     UA_ObjectAttributes attr = UA_ObjectAttributes_default;
diff --git a/tests/open62541-testserver/testserver.h b/tests/open62541-testserver/testserver.h
index cd59289..be5a012 100644
--- a/tests/open62541-testserver/testserver.h
+++ b/tests/open62541-testserver/testserver.h
@@ -50,6 +50,8 @@ public:
 
     UA_StatusCode addEncoderTestModel();
 
+    UA_StatusCode addUnreadableVariableNode(const UA_NodeId &parent);
+
     static UA_StatusCode multiplyMethod(UA_Server *server, const UA_NodeId *sessionId, void *sessionHandle,
                                             const UA_NodeId *methodId, void *methodContext,
                                             const UA_NodeId *objectId, void *objectContext,