dohertj2/lmxopcua
1
0
Fork 0
Code Issues 17 Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(mtconnect): deterministic concurrency test + DATA_SET -> BadNotSupported

Browse Source 
The concurrency test asserted a post-Apply post-condition from inside the
race window: dev1_pos is authored, so a reader beating the first Apply
correctly observes BadWaitingForInitialData, which the test's two-member
"legal" set omitted. Consistently red (10/10) rather than intermittent.

Rewritten to assert only properties that hold at every instant — the whole
(status, value, source timestamp) triple against the three legal snapshots,
which is the real torn-snapshot check — plus a read counter guarding a
vacuous pass and one deterministic closing Apply for an exact end state.
The index is unchanged here; BadWaitingForInitialData is a designed state.

Also codes structured (DATA_SET / TABLE) observations BadNotSupported now
that the parser supplies MTConnectObservation.IsStructured, so a two-entry
data set no longer surfaces as the Good string "12".
This commit is contained in:
Joseph Doherty
2026-07-24 15:09:42 -04:00
parent 5ba9f1be6f
commit 908bd7ba4a
2 changed files with 156 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectObservationIndex.cs
+23 -14
View File
@@ -48,20 +48,18 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect;
/// would serialize the pump against every read for no semantic gain.
/// </para>
/// <para>
/// <b>KNOWN LIMITATION — <c>DATA_SET</c> / <c>TABLE</c> observations.</b> Their real content
/// is <c>&lt;Entry key=…&gt;</c> child elements, but the parser reads an observation's value
/// with <c>XElement.Value</c>, which concatenates all descendant text. Such an observation
/// therefore reaches this index as a run-together string with the keys discarded, and —
/// because the type inference correctly demotes <c>DATA_SET</c>/<c>TABLE</c> to
/// <see cref="DriverDataType.String"/> — it coerces "successfully" into a <b>Good-coded
/// snapshot carrying noise</b>. This index <b>cannot</b> detect the shape: neither
/// <see cref="MTConnectObservation"/> nor <see cref="MTConnectTagDefinition"/> carries the
/// DataItem's <c>representation</c>, and no value-shape heuristic can separate a
/// concatenated entry list from a legitimate space-bearing EVENT (a <c>Message</c> reads
/// "Coolant level low"). The discriminating signal — the observation element having child
/// elements — exists only in <see cref="MTConnectStreamsParser"/>, so the fix belongs there
/// (flag structured observations at parse time; this index then codes them
/// <c>BadNotSupported</c> alongside the TIME_SERIES deferral below).
/// <b>Shapes this build does not represent</b> are coded <c>BadNotSupported</c> with a null
/// value rather than approximated: a <c>TIME_SERIES</c> vector (array materialization is
/// deferred to P1.5) and a <c>DATA_SET</c> / <c>TABLE</c> observation. The latter carries its
/// real content in <c>&lt;Entry key=…&gt;</c> child elements, which reach a text-valued
/// observation concatenated with the keys discarded — a two-entry set reads as the single
/// token <c>"12"</c> — and because the type inference correctly demotes those
/// representations to <see cref="DriverDataType.String"/>, coercion would otherwise
/// "succeed" into a Good-coded snapshot carrying noise. This index cannot detect that shape
/// itself (no value heuristic can separate concatenated entries from a legitimate
/// space-bearing EVENT such as a <c>Message</c> reading "Coolant level low"), so it relies on
/// <see cref="MTConnectObservation.IsStructured"/>, which
/// <see cref="MTConnectStreamsParser"/> sets while the XML is still XML.
/// </para>
/// </remarks>
internal sealed class MTConnectObservationIndex
@@ -257,6 +255,17 @@ internal sealed class MTConnectObservationIndex
return new DataValueSnapshot(null, BadNoCommunication, sourceTimestamp, serverTimestamp);
}
// DATA_SET / TABLE: the Agent carried the content in <Entry key="…"> child elements, so
// Value is their concatenated text with the keys discarded (a two-entry set reads "12").
// Publishing that as a Good string would be noise an operator would trust. Checked before
// the tag lookup because it is a fact about the wire shape, true whether or not the data
// item is authored — and after the sentinel, so an unavailable data set still reports the
// truthful no-comms.
if (observation.IsStructured)
{
return new DataValueSnapshot(null, BadNotSupported, sourceTimestamp, serverTimestamp);
}
_tags.TryGetValue(observation.DataItemId, out var tag);
// TIME_SERIES vectors arrive as one space-separated string. P1 does not materialize OPC UA
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectObservationIndexTests.cs
+133 -16
View File
@@ -47,7 +47,8 @@ public sealed class MTConnectObservationIndexTests
MTConnectStreamsParser.Parse(File.ReadAllText(path));
/// <summary>Wraps one synthetic observation in the minimal legal streams result.</summary>
private static MTConnectStreamsResult Synthetic(string dataItemId, string value, DateTime? timestampUtc = null) =>
private static MTConnectStreamsResult Synthetic(
string dataItemId, string value, DateTime? timestampUtc = null, bool isStructured = false) =>
new(
InstanceId: 1L,
NextSequence: 2L,
@@ -55,7 +56,8 @@ public sealed class MTConnectObservationIndexTests
Observations: [new MTConnectObservation(
dataItemId,
value,
timestampUtc ?? new DateTime(2026, 7, 24, 12, 0, 0, DateTimeKind.Utc))]);
timestampUtc ?? new DateTime(2026, 7, 24, 12, 0, 0, DateTimeKind.Utc),
isStructured)]);
private static MTConnectTagDefinition Tag(string id, DriverDataType type) => new(id, type);
@@ -475,6 +477,92 @@ public sealed class MTConnectObservationIndexTests
snap.Value.ShouldBeOfType<string>().ShouldBe("1.1 1.2 1.3 1.4 1.5 1.6 1.7 1.8 1.9 2.0");
}
// --------------------------------------------------------------------- DATA_SET / TABLE
/// <summary>
/// A structured (DATA_SET / TABLE) observation's real content is its <c>&lt;Entry&gt;</c>
/// children, which reach the index concatenated with the keys discarded. It is coded
/// BadNotSupported rather than published as that noise — regardless of whether the data item
/// is authored, because the shape is a fact about the wire, not about the tag.
/// </summary>
[Fact]
public void Structured_observation_reports_BadNotSupported_when_unauthored()
{
var idx = new MTConnectObservationIndex();
idx.Apply(Synthetic("ds1", "12", isStructured: true));
var snap = idx.Get("ds1");
snap.StatusCode.ShouldBe(BadNotSupported);
snap.Value.ShouldBeNull();
}
/// <summary>The String authoring the type inference gives a DATA_SET must not smuggle the noise through.</summary>
[Fact]
public void Structured_observation_is_never_published_as_concatenated_text()
{
var idx = new MTConnectObservationIndex([Tag("ds1", DriverDataType.String)]);
idx.Apply(Synthetic("ds1", "12", isStructured: true));
var snap = idx.Get("ds1");
snap.Value.ShouldNotBe("12");
snap.Value.ShouldBeNull();
snap.StatusCode.ShouldBe(BadNotSupported);
}
/// <summary>No-comms outranks the unsupported shape, exactly as it does for a TIME_SERIES array.</summary>
[Fact]
public void Unavailable_wins_over_the_structured_shape()
{
var idx = new MTConnectObservationIndex([Tag("ds1", DriverDataType.String)]);
idx.Apply(Synthetic("ds1", "UNAVAILABLE", isStructured: true));
idx.Get("ds1").StatusCode.ShouldBe(BadNoCommunication);
}
/// <summary>
/// End-to-end through the real parser: the entry list that concatenates to the single token
/// "12" is caught by the parser's structured flag and never surfaces as a Good value. This is
/// the leg that would regress silently if the parser stopped setting the flag.
/// </summary>
[Fact]
public void A_parsed_data_set_observation_never_surfaces_as_a_good_value()
{
const string xml = """
<MTConnectStreams>
<Header instanceId="1" nextSequence="2" firstSequence="1"/>
<Streams><DeviceStream><ComponentStream><Events>
<VariableDataSet dataItemId="ds1" timestamp="2026-07-24T12:00:00Z" count="2">
<Entry key="V1">1</Entry>
<Entry key="V2">2</Entry>
</VariableDataSet>
</Events></ComponentStream></DeviceStream></Streams>
</MTConnectStreams>
""";
var idx = new MTConnectObservationIndex([Tag("ds1", DriverDataType.String)]);
idx.Apply(MTConnectStreamsParser.Parse(xml));
var snap = idx.Get("ds1");
snap.StatusCode.ShouldBe(BadNotSupported);
snap.Value.ShouldBeNull();
}
/// <summary>An ordinary text observation is not structured — the flag must not be over-applied.</summary>
[Fact]
public void An_ordinary_observation_is_not_treated_as_structured()
{
var idx = new MTConnectObservationIndex([Tag("dev1_program", DriverDataType.String)]);
idx.Apply(ParseFixture(CurrentFixture));
var snap = idx.Get("dev1_program");
snap.StatusCode.ShouldBe(Good);
snap.Value.ShouldBe("O1234");
}
// -------------------------------------------------------------------------------- ordering
/// <summary>Within one Apply, the later observation for a dataItemId wins (Agent order is ascending sequence).</summary>
@@ -652,8 +740,23 @@ public sealed class MTConnectObservationIndexTests
/// <summary>
/// /current (Task 10) and the /sample pump (Task 11) write concurrently while OPC UA reads
/// snapshot. Per-key atomicity is the contract: no throw, no torn snapshot (a Good value
/// paired with a Bad code), and every read observes one whole snapshot.
/// snapshot. Three properties, each stated so it holds at <b>every instant</b> rather than
/// only once the race settles: no call throws; no read observes a torn snapshot; and the
/// state after a deterministic closing Apply is exact.
/// <para>
/// <b>Torn-snapshot check.</b> The whole <c>(status, value, source timestamp)</c> triple
/// is matched against the closed set of snapshots <c>dev1_pos</c> may legally present —
/// never a field in isolation. A Good code beside the unavailable document's timestamp,
/// or beside a null value, matches no member and fails.
/// </para>
/// <para>
/// <b>The legal set has THREE members, not two.</b> <c>dev1_pos</c> is authored, so a
/// reader that beats the first Apply correctly observes <c>BadWaitingForInitialData</c> —
/// the designed "authored, not yet reported" state. An earlier version of this test
/// asserted a post-Apply post-condition from inside the race window and failed whenever
/// a reader got there first: it was asserting something the index never promised, and it
/// verified nothing its name claimed.
/// </para>
/// </summary>
[Fact]
public async Task Concurrent_applies_and_reads_stay_consistent_and_never_throw()
@@ -662,14 +765,24 @@ public sealed class MTConnectObservationIndexTests
var current = ParseFixture(CurrentFixture);
var unavailable = ParseFixture(UnavailableFixture);
var goodAt = new DateTime(2026, 7, 24, 12, 0, 0, 200, DateTimeKind.Utc);
(uint Status, object? Value, DateTime? Source)[] legalSnapshots =
[
(BadWaitingForInitialData, null, null),
(Good, 123.4567, goodAt),
(BadNoCommunication, null, new DateTime(2026, 7, 24, 12, 10, 0, 100, DateTimeKind.Utc)),
];
var reads = 0L;
using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(2));
var writers = Enumerable.Range(0, 4).Select(i => Task.Run(() =>
{
while (!cts.IsCancellationRequested)
{
idx.Apply(i % 2 == 0 ? current : unavailable);
}
}));
})).ToArray();
var readers = Enumerable.Range(0, 4).Select(_ => Task.Run(() =>
{
@@ -677,21 +790,25 @@ public sealed class MTConnectObservationIndexTests
{
var snap = idx.Get("dev1_pos");
// Exactly two snapshots are legal for this tag; anything else is a tear.
if (snap.StatusCode == 0u)
{
snap.Value.ShouldBe(123.4567);
}
else
{
snap.StatusCode.ShouldBe(BadNoCommunication);
snap.Value.ShouldBeNull();
}
legalSnapshots.ShouldContain((snap.StatusCode, snap.Value, snap.SourceTimestampUtc));
Interlocked.Increment(ref reads);
}
}));
})).ToArray();
await Task.WhenAll(writers.Concat(readers));
// Guards a vacuous pass: a reader loop that never ran would have asserted nothing.
Interlocked.Read(ref reads).ShouldBeGreaterThan(0L);
// The racing writers alternate documents, so the state DURING the race is unknowable by
// construction — pinning a specific one there is exactly the defect this test used to have.
// One deterministic apply, after every writer has stopped, gives an exact end state.
idx.Apply(current);
var final = idx.Get("dev1_pos");
final.StatusCode.ShouldBe(Good);
final.Value.ShouldBe(123.4567);
final.SourceTimestampUtc.ShouldBe(goodAt);
idx.Count.ShouldBe(AllFixtureDataItemIds.Length);
}