fix(aggregates): live PercentGood/DurationGood count Good time only (#809)
The frozen 1.5.378.182 StatusAggregateCalculator counts Uncertain regions as Good under TreatUncertainAsBad=false, so live AggregateFilter PercentGood / DurationGood disagreed with HistoryRead (Part 13 §5.4.3.31/§5.4.3.33, Mantis 0006166) whenever Uncertain data was present. Owner-approved 2026-11-11. - New app-owned GoodOnlyStatusAggregateCalculator subclasses the baseline and replaces only the PercentGood/DurationGood computation (Good regions only); CompatibilityAggregateFactory routes exactly those two IDs to it. The copied SDK files are untouched; the other 35 aggregates are byte-identical. - Golden test: the 64 changed vectors (38 PercentGood + 26 DurationGood) are exempted exactly — those whose baseline output differs from the same case's baseline TreatUncertainAsBad=true twin — and pinned to that twin (baseline evidence, not regenerated from the implementation); plus a full Good-only pin over all 352 PercentGood/DurationGood vectors and a hand-computed case. - Live-vs-history parity over Good/Uncertain/Bad mixes (no masks), with a discrimination check against the pre-#809 baseline; the booted-runtime facts test now expects 8000 ms / 40 %. - SdkAggregateOracle: Run/RunLive = the registered factory; RunSdkAsIs = the untouched baseline; its Part13GoodCalculator duplicate is removed. - README (recorded decision), ServerAggregateConfiguration remarks, docs/Historian.md and .claude/rules/historian.md updated. Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk
This commit is contained in:
@@ -2,7 +2,7 @@ using Opc.Ua;
|
||||
using Opc.Ua.Server;
|
||||
using ZB.MOM.WW.OtOpcUa.OpcUaServer.Compatibility.Aggregates;
|
||||
using BaselineAggregateCalculator = ZB.MOM.WW.OtOpcUa.OpcUaServer.Compatibility.Aggregates.AggregateCalculator;
|
||||
using BaselineStatusAggregateCalculator = ZB.MOM.WW.OtOpcUa.OpcUaServer.Compatibility.Aggregates.StatusAggregateCalculator;
|
||||
using BaselineAggregators = ZB.MOM.WW.OtOpcUa.OpcUaServer.Compatibility.Aggregates.Aggregators;
|
||||
using Shouldly;
|
||||
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
||||
|
||||
@@ -18,17 +18,18 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Conformance;
|
||||
/// <remarks>
|
||||
/// <para>Shared (source-linked) by <c>Sql.Common.Tests</c> and both SQL backends' integration suites.</para>
|
||||
/// <para>
|
||||
/// <b>PercentGood (and, since #820, DurationGood) are the aggregates not taken as-is (#796).</b> The 1.5.378.182 baseline changed
|
||||
/// <b>PercentGood / DurationGood (#796, #820, #809).</b> The 1.5.378.182 baseline changed
|
||||
/// <c>StatusAggregateCalculator.ComputeDurationGoodBad</c> (1.5.378.106 and earlier counted Good regions only):
|
||||
/// with TreatUncertainAsBad = false it counts Uncertain regions as <b>Good</b> time (as Bad when true). Part 13
|
||||
/// keeps Uncertain out of both (§5.4.3.31 DurationGood counts Good regions; §5.4.3.33 PercentGood =
|
||||
/// DurationGood ÷ interval; Mantis 0006166, closed "no change required": an all-Uncertain interval is
|
||||
/// PercentGood 0 <i>and</i> PercentBad 0), and so does ours. <see cref="Run"/> therefore answers PercentGood
|
||||
/// (and DurationGood) through <see cref="Part13GoodCalculator"/>: the SAME baseline calculator, region machinery (simple
|
||||
/// bounds, end-of-data 1 ms region, status and Partial bits) and server-default configuration, counting only
|
||||
/// Good regions — exactly what the pre-.182 oracle computed. PercentBad / DurationBad still count Bad regions only
|
||||
/// (unchanged by .182) and are compared as-is. <see cref="RunSdkAsIs"/> runs the baseline untouched, for the test that pins
|
||||
/// the divergence.
|
||||
/// PercentGood 0 <i>and</i> PercentBad 0), and so does ours. Since #809 the LIVE calculators follow Part 13 too:
|
||||
/// <c>CompatibilityAggregateFactory</c> serves those two through the application's
|
||||
/// <c>GoodOnlyStatusAggregateCalculator</c> (the same baseline region machinery — simple bounds, end-of-data 1 ms
|
||||
/// region, status and Partial bits — counting only Good regions), so <see cref="Run"/> / <see cref="RunLive"/> are
|
||||
/// exactly what a live AggregateFilter computes, for every aggregate. PercentBad / DurationBad count Bad regions only
|
||||
/// (unchanged by .182). <see cref="RunSdkAsIs"/> runs the untouched baseline (what live served before #809), for the
|
||||
/// tests that pin that divergence.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Not <c>100 − PercentBad</c> under TreatUncertainAsBad = true: that flag also changes the region machinery
|
||||
@@ -54,8 +55,9 @@ public static class SdkAggregateOracle
|
||||
};
|
||||
|
||||
/// <summary>
|
||||
/// Runs the Part 13 oracle for <paramref name="aggregate"/> over <paramref name="samples"/>: the baseline calculator,
|
||||
/// except PercentGood / DurationGood, which count Good regions only (see the remarks, #796/#820).
|
||||
/// Runs the Part 13 oracle for <paramref name="aggregate"/> over <paramref name="samples"/>: the live calculator
|
||||
/// (<see cref="RunLive"/>) — the baseline, except PercentGood / DurationGood, which count Good regions only (see the
|
||||
/// remarks, #796/#820/#809).
|
||||
/// </summary>
|
||||
/// <param name="aggregate">The aggregate.</param>
|
||||
/// <param name="start">Request start (later than <paramref name="end"/> for a reverse read).</param>
|
||||
@@ -65,16 +67,29 @@ public static class SdkAggregateOracle
|
||||
/// <returns>The oracle'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) =>
|
||||
aggregate is HistoryAggregateType.PercentGood or HistoryAggregateType.DurationGood
|
||||
? RunCore(new LegacyCalculatorAdapter(new Part13GoodCalculator(AggregateId(aggregate),
|
||||
start, end, interval.TotalMilliseconds, false, DefaultConfiguration, Telemetry,
|
||||
usePercent: aggregate == HistoryAggregateType.PercentGood)), start, end, samples)
|
||||
: RunSdkAsIs(aggregate, start, end, interval, samples);
|
||||
RunLive(aggregate, start, end, interval, samples);
|
||||
|
||||
/// <summary>
|
||||
/// Runs the baseline (1.5.378.182) calculator for <paramref name="aggregate"/> exactly as it computes it, under
|
||||
/// <see cref="DefaultConfiguration"/> — including its PercentGood rule (Uncertain counted as Good) that
|
||||
/// <see cref="Run"/> deliberately does not use (#796).
|
||||
/// Runs exactly the calculator the server registers for live AggregateFilter monitored items
|
||||
/// (<c>CompatibilityAggregateFactory.Create</c>, the #809 Good-only PercentGood / DurationGood included).
|
||||
/// </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>
|
||||
/// <param name="configuration">The configuration (default <see cref="DefaultConfiguration"/>).</param>
|
||||
/// <returns>The calculator's processed values, in the order it returns them.</returns>
|
||||
public static IReadOnlyList<Result> RunLive(HistoryAggregateType aggregate, DateTime start, DateTime end,
|
||||
TimeSpan interval, IEnumerable<(DateTime Time, HistorianValue Value, uint Status)> samples,
|
||||
AggregateConfiguration? configuration = null) =>
|
||||
RunCore(CompatibilityAggregateFactory.Create(AggregateId(aggregate), start, end, interval.TotalMilliseconds, false,
|
||||
configuration ?? DefaultConfiguration, Telemetry), start, end, samples);
|
||||
|
||||
/// <summary>
|
||||
/// Runs the untouched baseline (1.5.378.182) calculator for <paramref name="aggregate"/> exactly as it computes it,
|
||||
/// under <see cref="DefaultConfiguration"/> — including its PercentGood / DurationGood rule (Uncertain counted as
|
||||
/// Good) that neither HistoryRead nor, since #809, the live calculators use.
|
||||
/// </summary>
|
||||
/// <param name="aggregate">The aggregate.</param>
|
||||
/// <param name="start">Request start (later than <paramref name="end"/> for a reverse read).</param>
|
||||
@@ -86,8 +101,9 @@ public static class SdkAggregateOracle
|
||||
public static IReadOnlyList<Result> RunSdkAsIs(HistoryAggregateType aggregate, DateTime start, DateTime end,
|
||||
TimeSpan interval, IEnumerable<(DateTime Time, HistorianValue Value, uint Status)> samples,
|
||||
AggregateConfiguration? configuration = null) =>
|
||||
RunCore(CompatibilityAggregateFactory.Create(AggregateId(aggregate), start, end, interval.TotalMilliseconds, false,
|
||||
configuration ?? DefaultConfiguration, Telemetry), start, end, samples);
|
||||
RunCore(new LegacyCalculatorAdapter(BaselineAggregators.CreateStandardCalculator(AggregateId(aggregate), start, end,
|
||||
interval.TotalMilliseconds, false, configuration ?? DefaultConfiguration, Telemetry)
|
||||
?? throw new ArgumentOutOfRangeException(nameof(aggregate))), start, end, samples);
|
||||
|
||||
private static IReadOnlyList<Result> RunCore(IAggregateCalculator calc, DateTime start, DateTime end,
|
||||
IEnumerable<(DateTime Time, HistorianValue Value, uint Status)> samples)
|
||||
@@ -155,42 +171,7 @@ public static class SdkAggregateOracle
|
||||
problems.ShouldBeEmpty($"{label} differs from the SDK:\n" + string.Join("\n", problems));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The baseline <see cref="BaselineStatusAggregateCalculator"/> with PercentGood / DurationGood counting <b>Good</b>
|
||||
/// regions only — Part 13 §5.4.3.31/§5.4.3.33, and the 1.5.378.106 rule. Everything else (bounds, regions, Partial,
|
||||
/// status, the millisecond Duration unit) is the baseline's own code, run under the same configuration as
|
||||
/// PercentBad / DurationBad.
|
||||
/// </summary>
|
||||
private sealed class Part13GoodCalculator(NodeId aggregateId, DateTime startTime, DateTime endTime,
|
||||
double processingInterval, bool stepped, AggregateConfiguration configuration, ITelemetryContext telemetry,
|
||||
bool usePercent)
|
||||
: BaselineStatusAggregateCalculator(aggregateId, startTime, endTime, processingInterval, stepped, configuration, telemetry)
|
||||
{
|
||||
protected override LegacyDataValue ComputeValue(TimeSlice slice)
|
||||
{
|
||||
var values = GetValuesWithSimpleBounds(slice);
|
||||
if (values == null || values.Count == 0) return GetNoDataValue(slice);
|
||||
|
||||
var regions = GetRegionsInValueSet(values, false, true);
|
||||
double good = 0, total = 0;
|
||||
foreach (var region in regions)
|
||||
{
|
||||
total += region.Duration;
|
||||
if (StatusCode.IsGood(region.StatusCode)) good += region.Duration;
|
||||
}
|
||||
|
||||
var value = new LegacyDataValue
|
||||
{
|
||||
WrappedValue = ZB.MOM.WW.OtOpcUa.OpcUaServer.SdkValueBoundary.FromClr(usePercent ? good / total * 100 : good),
|
||||
SourceTimestamp = GetTimestamp(slice),
|
||||
ServerTimestamp = GetTimestamp(slice),
|
||||
};
|
||||
value.StatusCode = value.StatusCode.WithAggregateBits(AggregateBits.Calculated);
|
||||
return value;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>The <see cref="CompatibilityAggregateFactory"/> adapter, for a calculator built here.</summary>
|
||||
/// <summary>The <see cref="CompatibilityAggregateFactory"/> adapter, for a baseline calculator built here.</summary>
|
||||
private sealed class LegacyCalculatorAdapter(BaselineAggregateCalculator calculator) : IAggregateCalculator
|
||||
{
|
||||
public NodeId AggregateId => calculator.AggregateId;
|
||||
|
||||
Reference in New Issue
Block a user