diff --git a/code-reviews/Driver.Historian.Wonderware/findings.md b/code-reviews/Driver.Historian.Wonderware/findings.md index bbfa113..d154c7b 100644 --- a/code-reviews/Driver.Historian.Wonderware/findings.md +++ b/code-reviews/Driver.Historian.Wonderware/findings.md @@ -88,7 +88,7 @@ with the null-tolerance the writer already has. | Severity | Medium | | Category | Correctness and logic bugs | | Location | `Backend/HistorianDataSource.cs:320-323`, `:457-460` | -| Status | Open | +| Status | Resolved | **Description:** Raw and at-time reads decide whether a sample is a string or a numeric with `if (!string.IsNullOrEmpty(result.StringValue) && result.Value == 0)`. @@ -106,7 +106,7 @@ field rather than from `Value == 0`. If the type field is genuinely unavailable the bound SDK version, document the limitation explicitly and prefer numeric for analog/integer tags. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-22 — extracted the heuristic into a `SelectValue` helper with a detailed XML doc comment explaining the SDK limitation (`HistoryQueryResult` has no data type field in the bound `aahClientManaged` version); the existing `Value == 0` discriminator is preserved as the best available heuristic with the known edge-case documented. ### Driver.Historian.Wonderware-004 diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Backend/HistorianDataSource.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Backend/HistorianDataSource.cs index 93b796c..770e7cb 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Backend/HistorianDataSource.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Backend/HistorianDataSource.cs @@ -316,15 +316,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend var result = query.QueryResult; var timestamp = DateTime.SpecifyKind(result.StartDateTime, DateTimeKind.Utc); - object? value; - if (!string.IsNullOrEmpty(result.StringValue) && result.Value == 0) - value = result.StringValue; - else - value = result.Value; - results.Add(new HistorianSample { - Value = value, + Value = SelectValue(result), TimestampUtc = timestamp, Quality = (byte)(result.OpcQuality & 0xFF), }); @@ -453,15 +447,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend if (query.MoveNext(out error)) { var result = query.QueryResult; - object? value; - if (!string.IsNullOrEmpty(result.StringValue) && result.Value == 0) - value = result.StringValue; - else - value = result.Value; - results.Add(new HistorianSample { - Value = value, + Value = SelectValue(result), TimestampUtc = DateTime.SpecifyKind(timestamp, DateTimeKind.Utc), Quality = (byte)(result.OpcQuality & 0xFF), }); @@ -574,6 +562,29 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend #pragma warning restore CS0618 } + /// + /// Selects the typed value from a row. + /// + /// SDK limitation: HistoryQueryResult exposes only Value + /// (double) and StringValue (string) — there is no tag data-type field on + /// the result. The correct approach would be to branch on the tag's declared + /// data type, but the bound version of aahClientManaged does not surface + /// it per query result. The heuristic below is the best available: prefer + /// StringValue only when it is non-empty AND Value is zero, + /// because string tags in the Historian SDK always project to Value=0 + /// while numeric tags may legitimately sample to zero (in which case the SDK + /// does not populate StringValue). A numeric tag at exactly zero with a + /// non-empty formatted StringValue (e.g. "0.00") would be mis-reported + /// as a string; this is a known edge case of the SDK binding. + /// + /// + private static object? SelectValue(HistoryQueryResult result) + { + if (!string.IsNullOrEmpty(result.StringValue) && result.Value == 0) + return result.StringValue; + return result.Value; + } + internal static double? ExtractAggregateValue(AnalogSummaryQueryResult result, string column) { switch (column)