Historian aggregates: PercentGood diverges from the SDK after the 1.5.378.182 bump (10 SdkAggregateParityTests failures) #796

New Issue

2026-11-10T09:38:06-05:00

dohertj2 commented

2026-11-10 09:38:06 -05:00

SdkAggregateParityTests (tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Sql.Common.Tests) fails 10 of 275 on master.

It passes 275/275 at 2da58d03^ and fails at 2da58d03, the OPC Foundation SDK bump to 1.5.378.182 (#782).
Only PercentGood differs from the SDK; PercentBad still matches. It shows up in the Bounded, EndsInside, RecoveryEnds, OutageInside and OutageLast shapes. Example: EndsInside 0→50/20s [2] ours 75 | sdk 100, and [0] ours 0 | sdk 25.
Hypothesis, not verified: the SDK changed how PercentGood counts Uncertain time, or its TreatUncertainAsBad / AggregateConfiguration default changed, rather than the duration maths.

To do: find the SDK change, decide which side Part 13 supports, then either fix our evaluator or set the test's SDK configuration explicitly. Also check whether HistoryRead PercentGood results seen by clients (ScadaBridge) change.

`SdkAggregateParityTests` (`tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Sql.Common.Tests`) fails 10 of 275 on master. - It passes 275/275 at `2da58d03^` and fails at `2da58d03`, the OPC Foundation SDK bump to 1.5.378.182 (#782). - Only **PercentGood** differs from the SDK; PercentBad still matches. It shows up in the Bounded, EndsInside, RecoveryEnds, OutageInside and OutageLast shapes. Example: `EndsInside 0→50/20s [2] ours 75 | sdk 100`, and `[0] ours 0 | sdk 25`. - Hypothesis, not verified: the SDK changed how PercentGood counts Uncertain time, or its TreatUncertainAsBad / AggregateConfiguration default changed, rather than the duration maths. To do: find the SDK change, decide which side Part 13 supports, then either fix our evaluator or set the test's SDK configuration explicitly. Also check whether HistoryRead PercentGood results seen by clients (ScadaBridge) change.

dohertj2 added the type:bug priority:p2 labels 2026-11-10 09:38:06 -05:00

dohertj2 commented

2026-11-10 09:40:40 -05:00

Root cause, found by the investigation I stopped. Not committed.

Opc.Ua.Server 1.5.378.182 changed StatusAggregateCalculator.ComputeDurationGoodBad. With TreatUncertainAsBad = false it now counts Uncertain regions as Good time, and as Bad when the flag is true. 1.5.378.106 and earlier counted Uncertain as neither. Part 13 keeps Uncertain out of both (§5.4.3.31 DurationGood, §5.4.3.33 PercentGood). So does Mantis 0006166 (closed 2022, "Uncertain data does not affect the calculations"): an all-Uncertain interval is PercentGood 0 and PercentBad 0.

Our evaluator follows Part 13, so the SDK oracle is what moved. This is not a server bug. PercentBad is unaffected because Bad regions are counted as before.

Proposed fix (test side only): in tests/Drivers/Historian.Conformance/SdkAggregateOracle.cs, derive the oracle's PercentGood as 100 − PercentBad (run with TreatUncertainAsBad = true). Keep a RunSdkAsIs path plus a test that pins the SDK divergence. The work-in-progress patch is below. It is untested. Still to do: add the pinning test, run Sql.Common.Tests and both SQL backends' integration suites (the oracle is source-linked into them), and check that nothing at runtime uses the SDK's PercentGood.

diff --git a/tests/Drivers/Historian.Conformance/SdkAggregateOracle.cs b/tests/Drivers/Historian.Conformance/SdkAggregateOracle.cs
index b7dd2f78..4284d9b3 100644
--- a/tests/Drivers/Historian.Conformance/SdkAggregateOracle.cs
+++ b/tests/Drivers/Historian.Conformance/SdkAggregateOracle.cs
@@ -13,7 +13,20 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Conformance;
 ///     UseSlopedExtrapolation = false), sloped interpolation, source timestamps.
 /// </summary>
 /// <remarks>
-///     Shared (source-linked) by <c>Sql.Common.Tests</c> and both SQL backends' integration suites.
+///     <para>Shared (source-linked) by <c>Sql.Common.Tests</c> and both SQL backends' integration suites.</para>
+///     <para>
+///         <b>PercentGood is derived, not taken as-is (#796).</b> Opc.Ua.Server 1.5.378.182 changed
+///         <c>StatusAggregateCalculator.ComputeDurationGoodBad</c> to count Uncertain regions as Good time when
+///         TreatUncertainAsBad = false (as Bad when true); 1.5.378.106 and earlier counted them as neither. Part 13
+///         (§5.4.3.31 DurationGood: "regions of Good and non-Good data"; §5.4.3.33 PercentGood = DurationGood ÷ interval)
+///         and the OPC Foundation's own Annex A test data — re-confirmed by Mantis 0006166 (closed 2022, "no change
+///         required": "Uncertain data does not affect the calculations", an all-Uncertain interval is PercentGood 0 AND
+///         PercentBad 0) — keep Uncertain out of both. Ours follows Part 13, so <see cref="Run"/> answers PercentGood as
+///         <c>100 − PercentBad</c> under TreatUncertainAsBad = true: the same SDK region machinery (simple bounds, end-of-data
+///         1 ms region, status and Partial bits), with the Uncertain time on the non-Good side where Part 13 puts it.
+///         PercentBad under the server's false still counts Bad regions only, unchanged by .182, and is compared as-is.
+///         <see cref="RunSdkAsIs"/> runs the calculator untouched, for the test that pins the divergence.
+///     </para>
 /// </remarks>
 public static class SdkAggregateOracle
 {
@@ -30,7 +43,10 @@ public static class SdkAggregateOracle
         UseServerCapabilitiesDefaults = false,
     };
 
-    /// <summary>Runs the SDK calculator for <paramref name="aggregate"/> over <paramref name="samples"/>.</summary>
+    /// <summary>
+    ///     Runs the SDK calculator for <paramref name="aggregate"/> over <paramref name="samples"/> — the Part 13 oracle
+    ///     (PercentGood derived, see the remarks).
+    /// </summary>
     /// <param name="aggregate">The aggregate.</param>
     /// <param name="start">Request start (later than <paramref name="end"/> for a reverse read).</param>
     /// <param name="end">Request end.</param>
@@ -39,10 +55,37 @@ public static class SdkAggregateOracle
     /// <returns>The SDK's processed values, in the order it returns them.</returns>
     public static IReadOnlyList<Result> Run(HistoryAggregateType aggregate, DateTime start, DateTime end, TimeSpan interval,
         IEnumerable<(DateTime Time, HistorianValue Value, uint Status)> samples)
+    {
+        if (aggregate != HistoryAggregateType.PercentGood) return RunSdkAsIs(aggregate, start, end, interval, samples);
+        // Part 13 PercentGood (Good regions only, Uncertain excluded) = 100 − the SDK's PercentBad with Uncertain on the Bad
+        // side. See the remarks: the SDK's own PercentGood counts Uncertain as Good since 1.5.378.182.
+        var cfg = DefaultConfiguration;
+        cfg.TreatUncertainAsBad = true;
+        return RunCore(HistoryAggregateType.PercentBad, start, end, interval, samples, cfg)
+            .Select(r => r.Value is double bad ? r with { Value = 100d - bad } : r)
+            .ToList();
+    }
+
+    /// <summary>
+    ///     Runs the SDK calculator for <paramref name="aggregate"/> exactly as the SDK computes it, under
+    ///     <see cref="DefaultConfiguration"/> — including the 1.5.378.182 PercentGood rule (Uncertain counted as Good) that
+    ///     <see cref="Run"/> deliberately does not use.
+    /// </summary>
+    /// <param name="aggregate">The aggregate.</param>
+    /// <param name="start">Request start (later than <paramref name="end"/> for a reverse read).</param>
+    /// <param name="end">Request end.</param>
+    /// <param name="interval">Processing interval.</param>
+    /// <param name="samples">Every stored sample of the tag, any order (sorted here).</param>
+    /// <returns>The SDK's processed values, in the order it returns them.</returns>
+    public static IReadOnlyList<Result> RunSdkAsIs(HistoryAggregateType aggregate, DateTime start, DateTime end,
+        TimeSpan interval, IEnumerable<(DateTime Time, HistorianValue Value, uint Status)> samples) =>
+        RunCore(aggregate, start, end, interval, samples, DefaultConfiguration);
+
+    private static IReadOnlyList<Result> RunCore(HistoryAggregateType aggregate, DateTime start, DateTime end,
+        TimeSpan interval, IEnumerable<(DateTime Time, HistorianValue Value, uint Status)> samples, AggregateConfiguration cfg)
     {
         var id = AggregateId(aggregate);
         var ms = interval.TotalMilliseconds;
-        var cfg = DefaultConfiguration;
         IAggregateCalculator calc = aggregate switch
         {
             HistoryAggregateType.Average or HistoryAggregateType.TimeAverage or HistoryAggregateType.TimeAverage2

**Root cause, found by the investigation I stopped. Not committed.** Opc.Ua.Server 1.5.378.182 changed `StatusAggregateCalculator.ComputeDurationGoodBad`. With `TreatUncertainAsBad = false` it now counts Uncertain regions as **Good** time, and as Bad when the flag is true. 1.5.378.106 and earlier counted Uncertain as neither. Part 13 keeps Uncertain out of both (§5.4.3.31 DurationGood, §5.4.3.33 PercentGood). So does Mantis 0006166 (closed 2022, "Uncertain data does not affect the calculations"): an all-Uncertain interval is PercentGood 0 *and* PercentBad 0. **Our evaluator follows Part 13, so the SDK oracle is what moved. This is not a server bug.** PercentBad is unaffected because Bad regions are counted as before. Proposed fix (test side only): in `tests/Drivers/Historian.Conformance/SdkAggregateOracle.cs`, derive the oracle's PercentGood as `100 − PercentBad` (run with TreatUncertainAsBad = true). Keep a `RunSdkAsIs` path plus a test that pins the SDK divergence. The work-in-progress patch is below. It is **untested**. Still to do: add the pinning test, run Sql.Common.Tests and both SQL backends' integration suites (the oracle is source-linked into them), and check that nothing at runtime uses the SDK's PercentGood. ```diff diff --git a/tests/Drivers/Historian.Conformance/SdkAggregateOracle.cs b/tests/Drivers/Historian.Conformance/SdkAggregateOracle.cs index b7dd2f78..4284d9b3 100644 --- a/tests/Drivers/Historian.Conformance/SdkAggregateOracle.cs +++ b/tests/Drivers/Historian.Conformance/SdkAggregateOracle.cs @@ -13,7 +13,20 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Conformance; /// UseSlopedExtrapolation = false), sloped interpolation, source timestamps. /// </summary> /// <remarks> -/// Shared (source-linked) by <c>Sql.Common.Tests</c> and both SQL backends' integration suites. +/// <para>Shared (source-linked) by <c>Sql.Common.Tests</c> and both SQL backends' integration suites.</para> +/// <para> +/// <b>PercentGood is derived, not taken as-is (#796).</b> Opc.Ua.Server 1.5.378.182 changed +/// <c>StatusAggregateCalculator.ComputeDurationGoodBad</c> to count Uncertain regions as Good time when +/// TreatUncertainAsBad = false (as Bad when true); 1.5.378.106 and earlier counted them as neither. Part 13 +/// (§5.4.3.31 DurationGood: "regions of Good and non-Good data"; §5.4.3.33 PercentGood = DurationGood ÷ interval) +/// and the OPC Foundation's own Annex A test data — re-confirmed by Mantis 0006166 (closed 2022, "no change +/// required": "Uncertain data does not affect the calculations", an all-Uncertain interval is PercentGood 0 AND +/// PercentBad 0) — keep Uncertain out of both. Ours follows Part 13, so <see cref="Run"/> answers PercentGood as +/// <c>100 − PercentBad</c> under TreatUncertainAsBad = true: the same SDK region machinery (simple bounds, end-of-data +/// 1 ms region, status and Partial bits), with the Uncertain time on the non-Good side where Part 13 puts it. +/// PercentBad under the server's false still counts Bad regions only, unchanged by .182, and is compared as-is. +/// <see cref="RunSdkAsIs"/> runs the calculator untouched, for the test that pins the divergence. +/// </para> /// </remarks> public static class SdkAggregateOracle { @@ -30,7 +43,10 @@ public static class SdkAggregateOracle UseServerCapabilitiesDefaults = false, }; - /// <summary>Runs the SDK calculator for <paramref name="aggregate"/> over <paramref name="samples"/>.</summary> + /// <summary> + /// Runs the SDK calculator for <paramref name="aggregate"/> over <paramref name="samples"/> — the Part 13 oracle + /// (PercentGood derived, see the remarks). + /// </summary> /// <param name="aggregate">The aggregate.</param> /// <param name="start">Request start (later than <paramref name="end"/> for a reverse read).</param> /// <param name="end">Request end.</param> @@ -39,10 +55,37 @@ public static class SdkAggregateOracle /// <returns>The SDK's processed values, in the order it returns them.</returns> public static IReadOnlyList<Result> Run(HistoryAggregateType aggregate, DateTime start, DateTime end, TimeSpan interval, IEnumerable<(DateTime Time, HistorianValue Value, uint Status)> samples) + { + if (aggregate != HistoryAggregateType.PercentGood) return RunSdkAsIs(aggregate, start, end, interval, samples); + // Part 13 PercentGood (Good regions only, Uncertain excluded) = 100 − the SDK's PercentBad with Uncertain on the Bad + // side. See the remarks: the SDK's own PercentGood counts Uncertain as Good since 1.5.378.182. + var cfg = DefaultConfiguration; + cfg.TreatUncertainAsBad = true; + return RunCore(HistoryAggregateType.PercentBad, start, end, interval, samples, cfg) + .Select(r => r.Value is double bad ? r with { Value = 100d - bad } : r) + .ToList(); + } + + /// <summary> + /// Runs the SDK calculator for <paramref name="aggregate"/> exactly as the SDK computes it, under + /// <see cref="DefaultConfiguration"/> — including the 1.5.378.182 PercentGood rule (Uncertain counted as Good) that + /// <see cref="Run"/> deliberately does not use. + /// </summary> + /// <param name="aggregate">The aggregate.</param> + /// <param name="start">Request start (later than <paramref name="end"/> for a reverse read).</param> + /// <param name="end">Request end.</param> + /// <param name="interval">Processing interval.</param> + /// <param name="samples">Every stored sample of the tag, any order (sorted here).</param> + /// <returns>The SDK's processed values, in the order it returns them.</returns> + public static IReadOnlyList<Result> RunSdkAsIs(HistoryAggregateType aggregate, DateTime start, DateTime end, + TimeSpan interval, IEnumerable<(DateTime Time, HistorianValue Value, uint Status)> samples) => + RunCore(aggregate, start, end, interval, samples, DefaultConfiguration); + + private static IReadOnlyList<Result> RunCore(HistoryAggregateType aggregate, DateTime start, DateTime end, + TimeSpan interval, IEnumerable<(DateTime Time, HistorianValue Value, uint Status)> samples, AggregateConfiguration cfg) { var id = AggregateId(aggregate); var ms = interval.TotalMilliseconds; - var cfg = DefaultConfiguration; IAggregateCalculator calc = aggregate switch { HistoryAggregateType.Average or HistoryAggregateType.TimeAverage or HistoryAggregateType.TimeAverage2 ```

dohertj2 referenced this issue

2026-11-10 13:30:50 -05:00

WIP: Upgrade OPC UA SDK to 2.0.1 preserving application contracts #802

dohertj2 referenced this issue

2026-11-10 13:31:57 -05:00

Upgrade OPC Foundation SDK 1.5.378 → 2.0.x (public SecureChannelContext.PeerAddress replaces #751 reflection) #782

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#796