fix(driver-historian-wonderware): resolve Medium code-review finding (Driver.Historian.Wonderware-003)

Extract the string-vs-numeric value selection from raw and at-time read
loops into a SelectValue helper method. aahClientManaged's HistoryQueryResult
has no data-type field in the bound SDK version, so the heuristic (prefer
StringValue when non-empty and Value==0) is unavoidable; the helper now
documents the limitation explicitly in its XML doc so the known edge case
(numeric tag at exactly zero with a formatted StringValue) is self-evident.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-05-22 09:23:00 -04:00
parent 205b07f6aa
commit 02daacbfd0
2 changed files with 27 additions and 16 deletions

View File

@@ -88,7 +88,7 @@ with the null-tolerance the writer already has.
| Severity | Medium | | Severity | Medium |
| Category | Correctness and logic bugs | | Category | Correctness and logic bugs |
| Location | `Backend/HistorianDataSource.cs:320-323`, `:457-460` | | 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 **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)`. 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 the bound SDK version, document the limitation explicitly and prefer numeric for
analog/integer tags. 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 ### Driver.Historian.Wonderware-004

View File

@@ -316,15 +316,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
var result = query.QueryResult; var result = query.QueryResult;
var timestamp = DateTime.SpecifyKind(result.StartDateTime, DateTimeKind.Utc); 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 results.Add(new HistorianSample
{ {
Value = value, Value = SelectValue(result),
TimestampUtc = timestamp, TimestampUtc = timestamp,
Quality = (byte)(result.OpcQuality & 0xFF), Quality = (byte)(result.OpcQuality & 0xFF),
}); });
@@ -453,15 +447,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
if (query.MoveNext(out error)) if (query.MoveNext(out error))
{ {
var result = query.QueryResult; 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 results.Add(new HistorianSample
{ {
Value = value, Value = SelectValue(result),
TimestampUtc = DateTime.SpecifyKind(timestamp, DateTimeKind.Utc), TimestampUtc = DateTime.SpecifyKind(timestamp, DateTimeKind.Utc),
Quality = (byte)(result.OpcQuality & 0xFF), Quality = (byte)(result.OpcQuality & 0xFF),
}); });
@@ -574,6 +562,29 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
#pragma warning restore CS0618 #pragma warning restore CS0618
} }
/// <summary>
/// Selects the typed value from a <see cref="HistoryQueryResult"/> row.
/// <para>
/// <b>SDK limitation:</b> <c>HistoryQueryResult</c> exposes only <c>Value</c>
/// (double) and <c>StringValue</c> (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 <c>aahClientManaged</c> does not surface
/// it per query result. The heuristic below is the best available: prefer
/// <c>StringValue</c> only when it is non-empty AND <c>Value</c> is zero,
/// because string tags in the Historian SDK always project to <c>Value=0</c>
/// while numeric tags may legitimately sample to zero (in which case the SDK
/// does not populate <c>StringValue</c>). A numeric tag at exactly zero with a
/// non-empty formatted <c>StringValue</c> (e.g. "0.00") would be mis-reported
/// as a string; this is a known edge case of the SDK binding.
/// </para>
/// </summary>
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) internal static double? ExtractAggregateValue(AnalogSummaryQueryResult result, string column)
{ {
switch (column) switch (column)