refactor(server): Cache absolute deadband percentage deadband filter

The percentage deadband refers to the EURange property of the variable.
Compute the resulting absolute deadband ahead of time. This simplifies
the cyclic "hot path".

src/server/ua_services_monitoreditem.c

@@ -21,8 +21,61 @@
 
 #ifdef UA_ENABLE_SUBSCRIPTIONS /* conditional compilation */
 
+#ifdef UA_ENABLE_DA
+
+/* Translate a percentage deadband into an absolute deadband based on the
+ * UARange property of the variable */
+static UA_StatusCode
+setAbsoluteFromPercentageDeadband(UA_Server *server, UA_Session *session,
+                                  UA_MonitoredItem *mon, UA_DataChangeFilter *filter) {
+    /* A valid deadband? */
+    if(filter->deadbandValue < 0.0 || filter->deadbandValue > 100.0)
+        return UA_STATUSCODE_BADDEADBANDFILTERINVALID;
+
+    /* Browse for the percent range */
+    UA_QualifiedName qn = UA_QUALIFIEDNAME(0, "EURange");
+    UA_BrowsePathResult bpr =
+        browseSimplifiedBrowsePath(server, mon->monitoredNodeId, 1, &qn);
+    if(bpr.statusCode != UA_STATUSCODE_GOOD || bpr.targetsSize < 1) {
+        UA_BrowsePathResult_clear(&bpr);
+        return UA_STATUSCODE_BADFILTERNOTALLOWED;
+    }
+
+    /* Read the range */
+    UA_ReadValueId rvi;
+    UA_ReadValueId_init(&rvi);
+    rvi.nodeId = bpr.targets->targetId.nodeId;
+    rvi.attributeId = UA_ATTRIBUTEID_VALUE;
+    UA_DataValue rangeVal = UA_Server_readWithSession(server, session, &rvi,
+                                                      UA_TIMESTAMPSTORETURN_NEITHER);
+    UA_BrowsePathResult_clear(&bpr);
+    if(!UA_Variant_isScalar(&rangeVal.value) ||
+       rangeVal.value.type != &UA_TYPES[UA_TYPES_RANGE]) {
+        UA_DataValue_clear(&rangeVal);
+        return UA_STATUSCODE_BADFILTERNOTALLOWED;
+    }
+
+    /* Compute the abs deadband */
+    UA_Range* euRange = (UA_Range*)rangeVal.value.data;
+    UA_Double absDeadband =
+        (filter->deadbandValue/100.0) * (euRange->high - euRange->low);
+
+    /* EURange invalid or NaN? */
+    if(absDeadband < 0.0 || absDeadband != absDeadband) {
+        UA_DataValue_clear(&rangeVal);
+        return UA_STATUSCODE_BADFILTERNOTALLOWED;
+    }
+
+    mon->filter.dataChangeFilter.trigger = filter->trigger;
+    mon->filter.dataChangeFilter.deadbandType = UA_DEADBANDTYPE_ABSOLUTE;
+    mon->filter.dataChangeFilter.deadbandValue = absDeadband;
+    return UA_STATUSCODE_GOOD;
+}
+
+#endif /* UA_ENABLE_DA */
+
 static UA_StatusCode
-setMonitoredItemSettings(UA_Server *server, UA_MonitoredItem *mon,
+setMonitoredItemSettings(UA_Server *server, UA_Session *session, UA_MonitoredItem *mon,
                          UA_MonitoringMode monitoringMode,
                          const UA_MonitoringParameters *params,
                          const UA_DataType* dataType) {
@@ -53,20 +106,27 @@ setMonitoredItemSettings(UA_Server *server, UA_MonitoredItem *mon,
             mon->filter.dataChangeFilter.trigger = UA_DATACHANGETRIGGER_STATUSVALUE;
         } else if(params->filter.content.decoded.type == &UA_TYPES[UA_TYPES_DATACHANGEFILTER]) {
             UA_DataChangeFilter *filter = (UA_DataChangeFilter *)params->filter.content.decoded.data;
-            // TODO implement EURange to support UA_DEADBANDTYPE_PERCENT
             switch(filter->deadbandType) {
             case UA_DEADBANDTYPE_NONE:
+                mon->filter.dataChangeFilter = *filter;
                 break;
             case UA_DEADBANDTYPE_ABSOLUTE:
                 if(!dataType || !UA_DataType_isNumeric(dataType))
                     return UA_STATUSCODE_BADFILTERNOTALLOWED;
+                mon->filter.dataChangeFilter = *filter;
                 break;
             case UA_DEADBANDTYPE_PERCENT:
+#ifdef UA_ENABLE_DA
+                if(!dataType || !UA_DataType_isNumeric(dataType))
+                    return UA_STATUSCODE_BADFILTERNOTALLOWED;
+                retval = setAbsoluteFromPercentageDeadband(server, session, mon, filter);
+                break;
+#else
                 return UA_STATUSCODE_BADMONITOREDITEMFILTERUNSUPPORTED;
+#endif
             default:
                 return UA_STATUSCODE_BADMONITOREDITEMFILTERUNSUPPORTED;
             }
-            retval = UA_DataChangeFilter_copy(filter, &mon->filter.dataChangeFilter);
         } else {
             return UA_STATUSCODE_BADMONITOREDITEMFILTERUNSUPPORTED;
         }
@@ -214,7 +274,7 @@ Operation_CreateMonitoredItem(UA_Server *server, UA_Session *session, struct cre
     UA_StatusCode retval = UA_STATUSCODE_GOOD;
     retval |= UA_NodeId_copy(&request->itemToMonitor.nodeId, &newMon->monitoredNodeId);
     retval |= UA_String_copy(&request->itemToMonitor.indexRange, &newMon->indexRange);
-    retval |= setMonitoredItemSettings(server, newMon, request->monitoringMode,
+    retval |= setMonitoredItemSettings(server, session, newMon, request->monitoringMode,
                                        &request->requestedParameters, v.value.type);
     UA_DataValue_clear(&v);
     if(retval != UA_STATUSCODE_GOOD) {
@@ -353,7 +413,7 @@ Operation_ModifyMonitoredItem(UA_Server *server, UA_Session *session, UA_Subscri
     rvid.attributeId = mon->attributeId;
     rvid.indexRange = mon->indexRange;
     UA_DataValue v = UA_Server_readWithSession(server, session, &rvid, mon->timestampsToReturn);
-    UA_StatusCode retval = setMonitoredItemSettings(server, mon, mon->monitoringMode,
+    UA_StatusCode retval = setMonitoredItemSettings(server, session, mon, mon->monitoringMode,
                                                     &request->requestedParameters,
                                                     v.value.type);
     UA_DataValue_clear(&v);

src/server/ua_subscription.h

@@ -111,12 +111,24 @@ struct UA_MonitoredItem {
     UA_Boolean discardOldest;
     union {
 #ifdef UA_ENABLE_SUBSCRIPTIONS_EVENTS
-        UA_EventFilter eventFilter; /* If attributeId == UA_ATTRIBUTEID_EVENTNOTIFIER */
+        /* If attributeId == UA_ATTRIBUTEID_EVENTNOTIFIER */
+        UA_EventFilter eventFilter;
 #endif
+        /* The DataChangeFilter always contains an absolute deadband definition.
+         * Part 8, §6.2 gives the following formula to test for percentage
+         * deadbands:
+         *
+         * DataChange if (absolute value of (last cached value - current value)
+         *                > (deadbandValue/100.0) * ((high–low) of EURange)))
+         *
+         * So we can convert from a percentage to an absolute deadband and keep
+         * the hot code path simple.
+         *
+         * TODO: Store the percentage deadband to recompute when the UARange is
+         * changed at runtime of the MonitoredItem */
         UA_DataChangeFilter dataChangeFilter;
     } filter;
-    UA_Variant lastValue;
-    // TODO: dataEncoding is hardcoded to UA binary
+    UA_Variant lastValue; // TODO: dataEncoding is hardcoded to UA binary
 
     /* Sample Callback */
     UA_UInt64 sampleCallbackId;

src/server/ua_subscription_datachange.c

@@ -13,10 +13,6 @@
 #include "ua_subscription.h"
 #include "ua_types_encoding_binary.h"
 
-#ifdef UA_ENABLE_DA
-#include <math.h> // fabs
-#endif
-
 #ifdef UA_ENABLE_SUBSCRIPTIONS /* conditional compilation */
 
 #define UA_VALUENCODING_MAXSTACK 512
@@ -85,16 +81,6 @@ updateNeededForFilteredValue(const UA_Variant *value, const UA_Variant *oldValue
     return false;
 }
 
-#ifdef UA_ENABLE_DA
-static UA_Boolean
-updateNeededForStatusCode(const UA_DataValue *value, const UA_MonitoredItem *mon) {
-    if(UA_Variant_isScalar(&value->value) && value->status != mon->lastStatus)
-        return true;
-    return false;
-}
-#endif
-
-
 /* When a change is detected, encoding contains the heap-allocated binary
  * encoded value. The default for changed is false. */
 static UA_StatusCode
@@ -108,38 +94,6 @@ detectValueChangeWithFilter(UA_Server *server, UA_Session *session, UA_Monitored
                                              mon->filter.dataChangeFilter.deadbandValue))
                 return UA_STATUSCODE_GOOD;
         }
-#ifdef UA_ENABLE_DA
-        else if(mon->filter.dataChangeFilter.deadbandType == UA_DEADBANDTYPE_PERCENT) {
-            /* Browse for the percent range */
-            UA_QualifiedName qn = UA_QUALIFIEDNAME(0, "EURange");
-            UA_BrowsePathResult bpr = browseSimplifiedBrowsePath(server, mon->monitoredNodeId, 1, &qn);
-            if(bpr.statusCode != UA_STATUSCODE_GOOD || bpr.targetsSize < 1) {
-                  UA_BrowsePathResult_clear(&bpr);
-                  return UA_STATUSCODE_GOOD;
-            }
-
-            /* Read the range */
-            UA_ReadValueId rvi;
-            UA_ReadValueId_init(&rvi);
-            rvi.nodeId = bpr.targets->targetId.nodeId;
-            rvi.attributeId = UA_ATTRIBUTEID_VALUE;
-            UA_DataValue rangeVal = UA_Server_readWithSession(server, session, &rvi, UA_TIMESTAMPSTORETURN_NEITHER);
-            if(!UA_Variant_isScalar(&rangeVal.value) || rangeVal.value.type != &UA_TYPES[UA_TYPES_RANGE]) {
-                UA_DataValue_clear(&rangeVal);
-                return UA_STATUSCODE_GOOD;
-            }
-
-            /* Compute the max change */
-            UA_Range* euRange = (UA_Range*)rangeVal.value.data;
-            UA_Double maxDist = (mon->filter.dataChangeFilter.deadbandValue/100.0) * (euRange->high - euRange->low);
-            UA_DataValue_clear(&rangeVal);
-
-            /* Relevant change? */
-            if(!updateNeededForFilteredValue(&value->value, &mon->lastValue, maxDist) &&
-               !updateNeededForStatusCode(value, mon))
-                return UA_STATUSCODE_GOOD;
-        }
-#endif
     }
 
     /* Stack-allocate some memory for the value encoding. We might heap-allocate

tests/server/check_monitoreditem_filter.c

@@ -559,7 +559,7 @@ START_TEST(Server_MonitoredItemsPercentFilterSetLater) {
        UA_Client_MonitoredItems_modify(client, modifyRequest);
 
     ck_assert_uint_eq(modifyResponse.resultsSize, 1);
-    ck_assert_uint_eq(modifyResponse.results[0].statusCode, UA_STATUSCODE_BADMONITOREDITEMFILTERUNSUPPORTED);
+    ck_assert_uint_eq(modifyResponse.results[0].statusCode, UA_STATUSCODE_BADFILTERNOTALLOWED);
 
     UA_ModifyMonitoredItemsResponse_deleteMembers(&modifyResponse);
 
@@ -837,7 +837,7 @@ START_TEST(Server_MonitoredItemsPercentFilterSetOnCreate) {
 
     ck_assert_uint_eq(createResponse.responseHeader.serviceResult, UA_STATUSCODE_GOOD);
     ck_assert_uint_eq(createResponse.resultsSize, 1);
-    ck_assert_uint_eq(createResponse.results[0].statusCode, UA_STATUSCODE_BADMONITOREDITEMFILTERUNSUPPORTED);
+    ck_assert_uint_eq(createResponse.results[0].statusCode, UA_STATUSCODE_BADFILTERNOTALLOWED);
     newMonitoredItemIds[0] = createResponse.results[0].monitoredItemId;
     UA_CreateMonitoredItemsResponse_deleteMembers(&createResponse);