fix(mtconnect): resetTriggered only resets data sets; empty set is Good {}; re-baseline test (#650 review)
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
@@ -291,6 +291,7 @@ type inference already type these items String):
|
|||||||
`resetTriggered` observation with no entries is the empty set `{}`. Several observations of one item in one
|
`resetTriggered` observation with no entries is the empty set `{}`. Several observations of one item in one
|
||||||
document fold in document order. The accumulated set is dropped on an Agent restart (`instanceId` change).
|
document fold in document order. The accumulated set is dropped on an Agent restart (`instanceId` change).
|
||||||
- `removed="true"` deletes the key (or row): it is **absent** from the JSON, never `null`.
|
- `removed="true"` deletes the key (or row): it is **absent** from the JSON, never `null`.
|
||||||
|
- **Empty set is Good `{}`**: a data-set-shaped observation (it carries the `count` attribute, which only DATA_SET/TABLE have) with no entries - `count="0"` in a `/current`, or `resetTriggered` - is the empty set, `{}`, never an absence; an all-removed set renders the same. `resetTriggered` on any other observation (e.g. an accumulating `PartCount`) is ignored.
|
||||||
- `UNAVAILABLE` is `BadNoCommunication` as for any observation, and also drops the accumulated set, so a
|
- `UNAVAILABLE` is `BadNoCommunication` as for any observation, and also drops the accumulated set, so a
|
||||||
later delta cannot resurrect pre-outage entries.
|
later delta cannot resurrect pre-outage entries.
|
||||||
- An observation whose children are not `<Entry>` elements stays `BadNotSupported`. Writes to these items stay
|
- An observation whose children are not `<Entry>` elements stays `BadNotSupported`. Writes to these items stay
|
||||||
|
|||||||
@@ -178,6 +178,12 @@ public sealed record MTConnectStreamsResult(
|
|||||||
/// <c>true</c> when the observation carried a non-empty <c>resetTriggered</c> attribute — the Agent's statement that
|
/// <c>true</c> when the observation carried a non-empty <c>resetTriggered</c> attribute — the Agent's statement that
|
||||||
/// the set was reset, so this observation <b>replaces</b> the accumulated set rather than merging into it.
|
/// the set was reset, so this observation <b>replaces</b> the accumulated set rather than merging into it.
|
||||||
/// </param>
|
/// </param>
|
||||||
|
/// <param name="IsDataSet">
|
||||||
|
/// <c>true</c> when the element is data-set shaped: it carries the <c>count</c> attribute every DATA_SET / TABLE
|
||||||
|
/// observation has (and no SAMPLE or ordinary EVENT does) or has <c><Entry></c> children. This — not
|
||||||
|
/// <see cref="ResetTriggered"/> — is what lets the index serve an entry-less set as <c>{}</c> rather than as
|
||||||
|
/// an absent value; <c>resetTriggered</c> also appears on ordinary accumulating SAMPLEs and must not matter there.
|
||||||
|
/// </param>
|
||||||
/// <param name="Condition">
|
/// <param name="Condition">
|
||||||
/// The CONDITION-only detail (#626) — set exactly when the observation came from a
|
/// The CONDITION-only detail (#626) — set exactly when the observation came from a
|
||||||
/// <c><Condition></c> container, <c>null</c> for every Sample/Event. Its presence is what marks an
|
/// <c><Condition></c> container, <c>null</c> for every Sample/Event. Its presence is what marks an
|
||||||
@@ -192,7 +198,8 @@ public sealed record MTConnectObservation(
|
|||||||
double? SampleRate = null,
|
double? SampleRate = null,
|
||||||
MTConnectConditionDetail? Condition = null,
|
MTConnectConditionDetail? Condition = null,
|
||||||
IReadOnlyList<MTConnectDataSetEntry>? Entries = null,
|
IReadOnlyList<MTConnectDataSetEntry>? Entries = null,
|
||||||
bool ResetTriggered = false);
|
bool ResetTriggered = false,
|
||||||
|
bool IsDataSet = false);
|
||||||
|
|
||||||
/// <summary>One <c><Cell key="…"></c> of a <c>TABLE</c> row (#650).</summary>
|
/// <summary>One <c><Cell key="…"></c> of a <c>TABLE</c> row (#650).</summary>
|
||||||
public sealed record MTConnectDataSetCell(string Key, string Value);
|
public sealed record MTConnectDataSetCell(string Key, string Value);
|
||||||
|
|||||||
@@ -121,6 +121,7 @@ internal sealed class MTConnectObservationIndex
|
|||||||
// Apply, but the lock keeps a concurrent one-shot read-through from corrupting a shared index.
|
// Apply, but the lock keeps a concurrent one-shot read-through from corrupting a shared index.
|
||||||
private readonly Dictionary<string, SortedDictionary<string, SetEntry>> _sets = new(StringComparer.Ordinal);
|
private readonly Dictionary<string, SortedDictionary<string, SetEntry>> _sets = new(StringComparer.Ordinal);
|
||||||
private readonly object _setsLock = new();
|
private readonly object _setsLock = new();
|
||||||
|
private bool _anySets;
|
||||||
|
|
||||||
private sealed record SetEntry(string? Value, SortedDictionary<string, string>? Cells);
|
private sealed record SetEntry(string? Value, SortedDictionary<string, string>? Cells);
|
||||||
|
|
||||||
@@ -264,18 +265,37 @@ internal sealed class MTConnectObservationIndex
|
|||||||
|
|
||||||
if (observation.Entries is not { Count: > 0 } entries)
|
if (observation.Entries is not { Count: > 0 } entries)
|
||||||
{
|
{
|
||||||
if (observation.ResetTriggered && !observation.IsStructured)
|
var unavailable = string.IsNullOrWhiteSpace(observation.Value) && !observation.IsDataSet ||
|
||||||
|
string.Equals(observation.Value, UnavailableSentinel, StringComparison.Ordinal);
|
||||||
|
|
||||||
|
if (unavailable)
|
||||||
{
|
{
|
||||||
// resetTriggered with no entries: an empty set, which is a legitimate value rather than an absence.
|
// Hot path for every ordinary UNAVAILABLE observation: skip the lock until a set exists at all.
|
||||||
lock (_setsLock) _sets[id] = new SortedDictionary<string, SetEntry>(StringComparer.Ordinal);
|
if (Volatile.Read(ref _anySets)) lock (_setsLock) _sets.Remove(id);
|
||||||
seenSets.Add(id);
|
|
||||||
return observation with { Value = "{}", ResetTriggered = false };
|
return observation;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (string.IsNullOrWhiteSpace(observation.Value) ||
|
if (observation.IsDataSet && !observation.IsStructured)
|
||||||
string.Equals(observation.Value, UnavailableSentinel, StringComparison.Ordinal))
|
|
||||||
{
|
{
|
||||||
lock (_setsLock) _sets.Remove(id);
|
// An entry-less data-set-shaped observation (count="0", or resetTriggered) is the EMPTY set - a
|
||||||
|
// legitimate value, Good "{}", not an absence. A reset or a /current baseline discards what
|
||||||
|
// was accumulated; a bare empty delta leaves the set as it is.
|
||||||
|
string emptyJson;
|
||||||
|
lock (_setsLock)
|
||||||
|
{
|
||||||
|
var first = seenSets.Add(id);
|
||||||
|
if (!_sets.TryGetValue(id, out var set) || observation.ResetTriggered || (baseline && first))
|
||||||
|
{
|
||||||
|
set = new SortedDictionary<string, SetEntry>(StringComparer.Ordinal);
|
||||||
|
_sets[id] = set;
|
||||||
|
Volatile.Write(ref _anySets, true);
|
||||||
|
}
|
||||||
|
|
||||||
|
emptyJson = RenderSet(set);
|
||||||
|
}
|
||||||
|
|
||||||
|
return observation with { Value = emptyJson, ResetTriggered = false, IsDataSet = false };
|
||||||
}
|
}
|
||||||
|
|
||||||
return observation;
|
return observation;
|
||||||
@@ -289,6 +309,7 @@ internal sealed class MTConnectObservationIndex
|
|||||||
{
|
{
|
||||||
set = new SortedDictionary<string, SetEntry>(StringComparer.Ordinal);
|
set = new SortedDictionary<string, SetEntry>(StringComparer.Ordinal);
|
||||||
_sets[id] = set;
|
_sets[id] = set;
|
||||||
|
Volatile.Write(ref _anySets, true);
|
||||||
}
|
}
|
||||||
|
|
||||||
foreach (var entry in entries)
|
foreach (var entry in entries)
|
||||||
@@ -312,7 +333,7 @@ internal sealed class MTConnectObservationIndex
|
|||||||
json = RenderSet(set);
|
json = RenderSet(set);
|
||||||
}
|
}
|
||||||
|
|
||||||
return observation with { Value = json, IsStructured = false, Entries = null, ResetTriggered = false };
|
return observation with { Value = json, IsStructured = false, Entries = null, ResetTriggered = false, IsDataSet = false };
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|||||||
@@ -382,6 +382,7 @@ internal static class MTConnectStreamsParser
|
|||||||
int? sampleCount = null;
|
int? sampleCount = null;
|
||||||
double? sampleRate = null;
|
double? sampleRate = null;
|
||||||
var resetTriggered = false;
|
var resetTriggered = false;
|
||||||
|
var hasCount = false;
|
||||||
List<MTConnectDataSetEntry>? entries = null;
|
List<MTConnectDataSetEntry>? entries = null;
|
||||||
(string? NativeCode, string? NativeSeverity, string? Qualifier, string? Type) conditionAttributes = default;
|
(string? NativeCode, string? NativeSeverity, string? Qualifier, string? Type) conditionAttributes = default;
|
||||||
|
|
||||||
@@ -410,6 +411,7 @@ internal static class MTConnectStreamsParser
|
|||||||
if (!isCondition)
|
if (!isCondition)
|
||||||
{
|
{
|
||||||
resetTriggered = !string.IsNullOrWhiteSpace(MTConnectXml.OptionalAttribute(reader, "resetTriggered"));
|
resetTriggered = !string.IsNullOrWhiteSpace(MTConnectXml.OptionalAttribute(reader, "resetTriggered"));
|
||||||
|
hasCount = MTConnectXml.OptionalAttribute(reader, "count") is not null;
|
||||||
entries = [];
|
entries = [];
|
||||||
sampleCount = int.TryParse(
|
sampleCount = int.TryParse(
|
||||||
MTConnectXml.OptionalAttribute(reader, "sampleCount"),
|
MTConnectXml.OptionalAttribute(reader, "sampleCount"),
|
||||||
@@ -483,7 +485,11 @@ internal static class MTConnectStreamsParser
|
|||||||
SampleRate: sampleRate,
|
SampleRate: sampleRate,
|
||||||
Condition: condition,
|
Condition: condition,
|
||||||
Entries: entries is { Count: > 0 } ? entries : null,
|
Entries: entries is { Count: > 0 } ? entries : null,
|
||||||
ResetTriggered: resetTriggered);
|
|
||||||
|
// resetTriggered only means "replace the set" on a data-set-shaped observation; on an ordinary
|
||||||
|
// accumulating SAMPLE/EVENT (PartCount resetTriggered="DAILY") it is not our concern.
|
||||||
|
ResetTriggered: resetTriggered && (hasCount || entries is { Count: > 0 }),
|
||||||
|
IsDataSet: !isCondition && (hasCount || entries is { Count: > 0 }));
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|||||||
@@ -23,10 +23,10 @@ public sealed class MTConnectDataSetTests
|
|||||||
""";
|
""";
|
||||||
|
|
||||||
private static string DataSet(string body, string attrs = "") =>
|
private static string DataSet(string body, string attrs = "") =>
|
||||||
$"""<VariableDataSet dataItemId="ds" timestamp="2026-07-24T12:00:00Z"{attrs}>{body}</VariableDataSet>""";
|
$"""<VariableDataSet dataItemId="ds" timestamp="2026-07-24T12:00:00Z" count="0"{attrs}>{body}</VariableDataSet>""";
|
||||||
|
|
||||||
private static string Table(string body, string attrs = "") =>
|
private static string Table(string body, string attrs = "") =>
|
||||||
$"""<WorkOffsetTable dataItemId="tb" timestamp="2026-07-24T12:00:00Z"{attrs}>{body}</WorkOffsetTable>""";
|
$"""<WorkOffsetTable dataItemId="tb" timestamp="2026-07-24T12:00:00Z" count="0"{attrs}>{body}</WorkOffsetTable>""";
|
||||||
|
|
||||||
private static MTConnectObservationIndex NewIndex() =>
|
private static MTConnectObservationIndex NewIndex() =>
|
||||||
new([new MTConnectTagDefinition("ds", DriverDataType.String), new MTConnectTagDefinition("tb", DriverDataType.String)]);
|
new([new MTConnectTagDefinition("ds", DriverDataType.String), new MTConnectTagDefinition("tb", DriverDataType.String)]);
|
||||||
@@ -172,4 +172,43 @@ public sealed class MTConnectDataSetTests
|
|||||||
|
|
||||||
idx.Get("ds").StatusCode.ShouldBe(0x803D0000u);
|
idx.Get("ds").StatusCode.ShouldBe(0x803D0000u);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ResetTriggered_on_an_ordinary_sample_keeps_its_numeric_value()
|
||||||
|
{
|
||||||
|
// Regression: resetTriggered is also a standard attribute on accumulating SAMPLEs / counters. It must
|
||||||
|
// never turn one into an empty set.
|
||||||
|
var idx = new MTConnectObservationIndex(
|
||||||
|
[new MTConnectTagDefinition("pc", DriverDataType.Int64), new MTConnectTagDefinition("amt", DriverDataType.Float64)]);
|
||||||
|
idx.Apply(MTConnectStreamsParser.Parse(Doc(
|
||||||
|
"""<PartCount dataItemId="pc" timestamp="2026-07-24T12:00:00Z" resetTriggered="DAILY">5</PartCount>""" +
|
||||||
|
"""<Amount dataItemId="amt" timestamp="2026-07-24T12:00:00Z" resetTriggered="DAILY">2.5</Amount>""")));
|
||||||
|
|
||||||
|
idx.Get("pc").StatusCode.ShouldBe(Good);
|
||||||
|
idx.Get("pc").Value.ShouldBe(5L);
|
||||||
|
idx.Get("amt").Value.ShouldBe(2.5d);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void An_empty_current_data_set_is_Good_empty_json_like_a_reset_one()
|
||||||
|
{
|
||||||
|
var idx = NewIndex();
|
||||||
|
Apply(idx, DataSet("""<Entry key="a">1</Entry>"""), baseline: true);
|
||||||
|
|
||||||
|
Apply(idx, DataSet(""), baseline: true);
|
||||||
|
|
||||||
|
var snap = idx.Get("ds");
|
||||||
|
snap.StatusCode.ShouldBe(Good);
|
||||||
|
snap.Value.ShouldBe("{}");
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void An_all_removed_set_is_the_same_empty_json()
|
||||||
|
{
|
||||||
|
var idx = NewIndex();
|
||||||
|
Apply(idx, DataSet("""<Entry key="a">1</Entry>"""), baseline: true);
|
||||||
|
Apply(idx, DataSet("""<Entry key="a" removed="true"/>"""));
|
||||||
|
|
||||||
|
idx.Get("ds").Value.ShouldBe("{}");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -377,6 +377,42 @@ public sealed class MTConnectSubscribeTests
|
|||||||
For(seen, "dev1_pos").Last().Snapshot.Value.ShouldBe(77.5d);
|
For(seen, "dev1_pos").Last().Snapshot.Value.ShouldBe(77.5d);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// #650: a re-baseline <c>/current</c> states the WHOLE data set, so it must REPLACE what the deltas
|
||||||
|
/// accumulated - a key removed while the stream was down must not linger. Reverting the
|
||||||
|
/// <c>baseline: true</c> argument at the re-baseline call site leaves {a,b} merged and fails this.
|
||||||
|
/// (The init and one-shot-read call sites build a fresh index, so their flag is not observable.)
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public async Task Rebaseline_replaces_the_accumulated_data_set_rather_than_merging()
|
||||||
|
{
|
||||||
|
var (driver, client) = await InitializedDriverAsync();
|
||||||
|
|
||||||
|
static MTConnectStreamsResult SetChunk(long first, long next, params (string Key, string Value)[] entries) =>
|
||||||
|
new(FixtureInstanceId, next, first,
|
||||||
|
[
|
||||||
|
new MTConnectObservation(
|
||||||
|
"ds", "", ObservedAt, IsStructured: true, IsDataSet: true,
|
||||||
|
Entries: [.. entries.Select(e => new MTConnectDataSetEntry(e.Key, e.Value, false))]),
|
||||||
|
]);
|
||||||
|
|
||||||
|
const string Oor = "MTConnect /sample answered an MTConnectError document (OUT_OF_RANGE) under HTTP 200";
|
||||||
|
|
||||||
|
// First re-baseline seeds {a,b}; the stream then resumes from its nextSequence (500).
|
||||||
|
client.SampleFailures.Enqueue(new InvalidDataException(Oor));
|
||||||
|
client.CurrentAnswers.Enqueue(SetChunk(490, 500, ("a", "1"), ("b", "2")));
|
||||||
|
await driver.SubscribeAsync(["dev1_pos"], TimeSpan.FromMilliseconds(50), Ct);
|
||||||
|
await client.PumpAsync(Chunk(500, 505, ("dev1_pos", "1.5"))).WaitAsync(Watchdog, Ct);
|
||||||
|
driver.ObservationIndex.Get("ds").Value.ShouldBe("""{"a":1,"b":2}""");
|
||||||
|
|
||||||
|
client.SampleFailures.Enqueue(new InvalidDataException(Oor));
|
||||||
|
client.CurrentAnswers.Enqueue(SetChunk(600, 610, ("a", "1")));
|
||||||
|
|
||||||
|
await client.PumpAsync(Chunk(610, 615, ("dev1_pos", "77.5"))).WaitAsync(Watchdog, Ct);
|
||||||
|
|
||||||
|
driver.ObservationIndex.Get("ds").Value.ShouldBe("""{"a":1}""");
|
||||||
|
}
|
||||||
|
|
||||||
// ---- agent restart ----
|
// ---- agent restart ----
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|||||||
Reference in New Issue
Block a user