From 994c0b3b19a184f156e5590487b11d92e9282f93 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 15:42:32 -0400 Subject: [PATCH] refactor(mtconnect): delete dead parsed fields (R06:T-3) MTConnectObservation.SampleRate was parsed per observation and never read; MTConnectTagDefinition.MtType/MtSubType/Units were carried (factory blob DTO, tags[] DTO, probe inference) and only MtCategory was ever read. Deleted. Stored configs and TagConfig blobs still carrying mtType/mtSubType/units keep loading (System.Text.Json skips unmapped members) - pinned by a new test. The AdminUI tag editor reads those keys straight off the blob and is unaffected. Refs #743 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../MTConnectTagDefinition.cs | 26 ++++------ .../MTConnectTagDefinitionFactory.cs | 11 +---- .../MTConnectDriver.cs | 19 ++------ .../MTConnectDtos.cs | 6 --- .../MTConnectStreamsParser.cs | 7 --- .../MTConnectDriverOptionsTests.cs | 47 +++++++++++++------ .../MTConnectObservationIndexTests.cs | 5 +- 7 files changed, 49 insertions(+), 72 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinition.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinition.cs index aaf040b8..d2132e14 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinition.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinition.cs @@ -26,27 +26,17 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; /// /// /// The DataItem's MTConnect category attribute — SAMPLE, EVENT, or -/// CONDITION. Optional metadata carried through for browse / display; not consulted by -/// the value-mapping path. Default null. -/// -/// -/// The DataItem's MTConnect type attribute (e.g. POSITION, EXECUTION). -/// Optional metadata. Default null. -/// -/// -/// The DataItem's MTConnect subType attribute (e.g. ACTUAL, COMMANDED). -/// Optional metadata. Default null. -/// -/// -/// The DataItem's MTConnect units attribute (e.g. MILLIMETER, -/// REVOLUTION/MINUTE). Optional metadata. Default null. +/// CONDITION. Read by the write path (a CONDITION is never writable). Default null. /// +/// +/// Review 2026-10-21 T-3: the MtType / MtSubType / Units members were carried here and +/// read by nothing — deleted. A stored blob or tags[] entry that still carries mtType / +/// mtSubType / units keeps loading (unknown JSON keys are skipped); the AdminUI tag editor reads +/// those keys straight off the blob for its display and type-inference advisory, not through this record. +/// public sealed record MTConnectTagDefinition( string FullName, DriverDataType DriverDataType, bool IsArray = false, int ArrayDim = 0, - string? MtCategory = null, - string? MtType = null, - string? MtSubType = null, - string? Units = null); + string? MtCategory = null); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinitionFactory.cs index 0458764b..5ff0a3ac 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Contracts/MTConnectTagDefinitionFactory.cs @@ -87,10 +87,7 @@ public static class MTConnectTagDefinitionFactory parsed, blob.IsArray ?? false, blob.ArrayDim ?? 0, - blob.MtCategory, - blob.MtType, - blob.MtSubType, - blob.Units); + blob.MtCategory); return true; } @@ -264,11 +261,5 @@ public static class MTConnectTagDefinitionFactory public int? ArrayDim { get; init; } /// The MTConnect DataItem category. public string? MtCategory { get; init; } - /// The MTConnect DataItem type. - public string? MtType { get; init; } - /// The MTConnect DataItem sub-type. - public string? MtSubType { get; init; } - /// The engineering units, when authored. - public string? Units { get; init; } } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs index 0440c81c..717cf2c0 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -3408,10 +3408,7 @@ public sealed partial class MTConnectDriver : ParseEnum(dto.DriverDataType, dto.FullName, driverInstanceId, nameof(MTConnectTagDefinition.DriverDataType)), dto.IsArray ?? false, dto.ArrayDim ?? 0, - dto.MtCategory, - dto.MtType, - dto.MtSubType, - dto.Units); + dto.MtCategory); } // ---- v3 tag binding: RawPath ↔ dataItemId, and the definitions the index coerces against ---- @@ -3680,10 +3677,7 @@ public sealed partial class MTConnectDriver inferred.DataType, inferred.IsArray, (int)(inferred.ArrayDim ?? 0), - dataItem.Category, - dataItem.Type, - dataItem.SubType, - dataItem.Units); + dataItem.Category); } /// Flattens a device model into a dataItemId → DataItem lookup; null ⇒ empty. @@ -3953,12 +3947,9 @@ public sealed partial class MTConnectDriver public int? ArrayDim { get; init; } /// The MTConnect DataItem category. public string? MtCategory { get; init; } - /// The MTConnect DataItem type. - public string? MtType { get; init; } - /// The MTConnect DataItem sub-type. - public string? MtSubType { get; init; } - /// The engineering units. - public string? Units { get; init; } + + // mtType / mtSubType / units were bound here and read by nothing (review 2026-10-21 T-3); a document that + // still carries them parses — System.Text.Json skips unmapped members. } /// Background connectivity-probe knobs (consumed by the probe loop). diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDtos.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDtos.cs index 9672d455..2e012d22 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDtos.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDtos.cs @@ -162,11 +162,6 @@ public sealed record MTConnectStreamsResult( /// The observation's own sampleCount attribute — the number of values a TIME_SERIES /// vector claims to carry. The index checks the parsed vector against it. null when absent. /// -/// -/// The observation's own sampleRate attribute (samples per second) on a TIME_SERIES -/// observation. Carried for the parser contract; not yet surfaced on the served node (see -/// docs/drivers/MTConnect.md). null when absent. -/// /// /// The parsed <Entry key="…"> children of a DATA_SET / TABLE observation, in /// document order, or null when the observation had none. A TABLE entry carries @@ -195,7 +190,6 @@ public sealed record MTConnectObservation( DateTime TimestampUtc, bool IsStructured = false, int? SampleCount = null, - double? SampleRate = null, MTConnectConditionDetail? Condition = null, IReadOnlyList? Entries = null, bool ResetTriggered = false, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs index 19b3df29..d2a0fc72 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs @@ -383,7 +383,6 @@ internal static class MTConnectStreamsParser var timestampUtc = default(DateTime); string? conditionState = null; int? sampleCount = null; - double? sampleRate = null; var resetTriggered = false; var hasCount = false; List? entries = null; @@ -421,11 +420,6 @@ internal static class MTConnectStreamsParser NumberStyles.Integer, CultureInfo.InvariantCulture, out var count) && count >= 0 ? count : null; - sampleRate = double.TryParse( - MTConnectXml.OptionalAttribute(reader, "sampleRate"), - NumberStyles.Float, - CultureInfo.InvariantCulture, - out var rate) && double.IsFinite(rate) ? rate : null; } // The slot is reserved BEFORE the subtree is read, and filled after: an observation is @@ -485,7 +479,6 @@ internal static class MTConnectStreamsParser // away a perfectly well-determined state. IsStructured: !isCondition && hasElements, SampleCount: sampleCount, - SampleRate: sampleRate, Condition: condition, Entries: entries is { Count: > 0 } ? entries : null, diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectDriverOptionsTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectDriverOptionsTests.cs index aa2d989f..db875d10 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectDriverOptionsTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectDriverOptionsTests.cs @@ -95,14 +95,11 @@ public sealed class MTConnectDriverOptionsTests var pos = new MTConnectTagDefinition( FullName: "dev1_pos", DriverDataType: ZB.MOM.WW.OtOpcUa.Core.Abstractions.DriverDataType.Float64, - MtCategory: "SAMPLE", - MtType: "POSITION", - Units: "MILLIMETER"); + MtCategory: "SAMPLE"); var partCount = new MTConnectTagDefinition( FullName: "dev1_partcount", DriverDataType: ZB.MOM.WW.OtOpcUa.Core.Abstractions.DriverDataType.Int64, - MtCategory: "EVENT", - MtType: "PART_COUNT"); + MtCategory: "EVENT"); var o = new MTConnectDriverOptions { AgentUri = "http://agent:5000", Tags = [pos, partCount] }; @@ -111,25 +108,47 @@ public sealed class MTConnectDriverOptionsTests o.Tags[1].ShouldBe(partCount); } - /// MTConnectTagDefinition round-trips its positional fields, including MTConnect metadata. + /// MTConnectTagDefinition round-trips its positional fields, including the category the write path reads. [Fact] - public void TagDefinition_carries_dataItemId_and_mtconnect_metadata() + public void TagDefinition_carries_dataItemId_and_mtconnect_category() { var tag = new MTConnectTagDefinition( FullName: "dtop_2", DriverDataType: ZB.MOM.WW.OtOpcUa.Core.Abstractions.DriverDataType.Float64, - MtCategory: "SAMPLE", - MtType: "POSITION", - MtSubType: "ACTUAL", - Units: "MILLIMETER"); + MtCategory: "SAMPLE"); tag.FullName.ShouldBe("dtop_2"); tag.DriverDataType.ShouldBe(ZB.MOM.WW.OtOpcUa.Core.Abstractions.DriverDataType.Float64); tag.IsArray.ShouldBeFalse(); tag.ArrayDim.ShouldBe(0); tag.MtCategory.ShouldBe("SAMPLE"); - tag.MtType.ShouldBe("POSITION"); - tag.MtSubType.ShouldBe("ACTUAL"); - tag.Units.ShouldBe("MILLIMETER"); + } + + /// + /// Review 2026-10-21 T-3: mtType / mtSubType / units were deleted from the definition as dead, but a stored + /// config that still carries them — in tags[] or in a raw tag's TagConfig blob — must keep loading. + /// + [Fact] + public void Stored_configs_carrying_the_deleted_metadata_keys_still_load() + { + const string config = """ + { + "agentUri": "http://agent:5000", + "tags": [ { "fullName": "dev1_pos", "driverDataType": "Float64", "mtCategory": "SAMPLE", + "mtType": "POSITION", "mtSubType": "ACTUAL", "units": "MILLIMETER" } ] + } + """; + + var options = MTConnectDriver.ParseOptions("mt-t3", config); + + options.Tags.ShouldHaveSingleItem().ShouldBe(new MTConnectTagDefinition( + "dev1_pos", ZB.MOM.WW.OtOpcUa.Core.Abstractions.DriverDataType.Float64, MtCategory: "SAMPLE")); + + MTConnectTagDefinitionFactory.TryFromTagConfig( + """{ "dataItemId": "dev1_pos", "dataType": "Float64", "mtCategory": "SAMPLE", "mtType": "POSITION", "mtSubType": "ACTUAL", "units": "MILLIMETER" }""", + out var dataItemId, out var authored) + .ShouldBeTrue(); + dataItemId.ShouldBe("dev1_pos"); + authored.ShouldNotBeNull().MtCategory.ShouldBe("SAMPLE"); } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectObservationIndexTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectObservationIndexTests.cs index 8b2c614a..d8aab2f8 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectObservationIndexTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectObservationIndexTests.cs @@ -502,14 +502,13 @@ public sealed class MTConnectObservationIndexTests idx.Get("dev1_vibration_ts").StatusCode.ShouldBe(BadNotSupported); } - /// The parser carries the observation's own sampleCount / sampleRate attributes. + /// The parser carries the observation's own sampleCount (sampleRate is not parsed — T-3: nothing read it). [Fact] - public void Parser_carries_time_series_sampleCount_and_sampleRate() + public void Parser_carries_time_series_sampleCount() { var obs = ParseFixture(CurrentFixture).Observations.Single(o => o.DataItemId == "dev1_vibration_ts"); obs.SampleCount.ShouldBe(10); - obs.SampleRate.ShouldBe(100.0); ParseFixture(CurrentFixture).Observations.Single(o => o.DataItemId == "dev1_pos").SampleCount.ShouldBeNull(); }